Skip to content
This repository has been archived by the owner on Apr 10, 2024. It is now read-only.

Adding ADAL wrapper to msrestazure #8

Merged
merged 9 commits into from
Nov 30, 2016
Merged
39 changes: 37 additions & 2 deletions msrestazure/azure_active_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@
MismatchingStateError,
OAuth2Error,
TokenExpiredError)
from requests import RequestException
from requests import (
RequestException,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to make this span multiple lines. Same goes for other imports.

ConnectionError
)
import requests_oauthlib as oauth

from msrest.authentication import OAuthTokenAuthentication
from msrest.authentication import (
OAuthTokenAuthentication,
Authentication
)
from msrest.exceptions import TokenExpiredError as Expired
from msrest.exceptions import (
AuthenticationError,
Expand Down Expand Up @@ -525,3 +531,32 @@ def set_token(self, response_url):
raise_with_traceback(AuthenticationError, "", err)
else:
self.token = token

class AdalAuthentication(Authentication):#pylint: disable=too-few-public-methods
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two spaces between : and #.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a docstring that explains what the class is about. Same goes for methods.


def __init__(self, adal_method, *args, **kwargs):
self._adal_method = adal_method
self._args = args
self._kwargs = kwargs

def signed_session(self):
session = super(AdalAuthentication, self).signed_session()

import adal # Adal is not mandatory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make adal mandatory then this should move to the top of the file.


try:
raw_token = self._adal_method(*self._args, **self._kwargs)
scheme, token = raw_token['tokenType'], raw_token['accessToken']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved outside of the try block as it won't trigger an AdalError.

except adal.AdalError as err:
#pylint: disable=no-member
if (hasattr(err, 'error_response') and ('error_description' in err.error_response)
and ('AADSTS70008:' in err.error_response['error_description'])):
raise Expired("Credentials have expired due to inactivity. Please run 'az login'")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider changing this error message, since this is not the CLI and they can't run az login.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the blank line with an else: clause.

raise AuthenticationError(err)
except ConnectionError as err:
raise AuthenticationError('Please ensure you have network connection. Error detail: ' + str(err))

header = "{} {}".format(scheme, token)
session.headers['Authorization'] = header
return session
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a newline at the end of the file.