Merge pull request #82 from AzureAD/release-0.6.0

MSAL Python 0.6.0

Author: abhidnya13

msal/application.py

@@ -18,7 +18,7 @@
 
 
 # The __init__.py will import this. Not the other way around.
-__version__ = "0.5.1"
+__version__ = "0.6.0"
 
 logger = logging.getLogger(__name__)
 
@@ -400,20 +400,21 @@ def acquire_token_silent(
         #     authority,
         #     verify=self.verify, proxies=self.proxies, timeout=self.timeout,
         #     ) if authority else self.authority
-        result = self._acquire_token_silent(scopes, account, self.authority, **kwargs)
+        result = self._acquire_token_silent_from_cache_and_possibly_refresh_it(
+            scopes, account, self.authority, **kwargs)
         if result:
             return result
         for alias in self._get_authority_aliases(self.authority.instance):
             the_authority = Authority(
                 "https://" + alias + "/" + self.authority.tenant,
                 validate_authority=False,
                 verify=self.verify, proxies=self.proxies, timeout=self.timeout)
-            result = self._acquire_token_silent(
+            result = self._acquire_token_silent_from_cache_and_possibly_refresh_it(
                 scopes, account, the_authority, **kwargs)
             if result:
                 return result
 
-    def _acquire_token_silent(
+    def _acquire_token_silent_from_cache_and_possibly_refresh_it(
             self,
             scopes,  # type: List[str]
             account,  # type: Optional[Account]

msal/authority.py

@@ -38,10 +38,14 @@ def __init__(self, authority_url, validate_authority=True,
         self.proxies = proxies
         self.timeout = timeout
         canonicalized, self.instance, tenant = canonicalize(authority_url)
-        tenant_discovery_endpoint = (  # Hard code a V2 pattern as default value
-            'https://{}/{}/v2.0/.well-known/openid-configuration'
-            .format(self.instance, tenant))
-        if validate_authority and self.instance not in WELL_KNOWN_AUTHORITY_HOSTS:
+        tenant_discovery_endpoint = (
+            'https://{}/{}{}/.well-known/openid-configuration'.format(
+                self.instance,
+                tenant,
+                "" if tenant == "adfs" else "/v2.0" # the AAD v2 endpoint
+                ))
+        if (tenant != "adfs" and validate_authority
+                and self.instance not in WELL_KNOWN_AUTHORITY_HOSTS):
             tenant_discovery_endpoint = instance_discovery(
                 canonicalized + "/oauth2/v2.0/authorize",
                 verify=verify, proxies=proxies, timeout=timeout)

msal/oauth2cli/oauth2.py

@@ -380,26 +380,34 @@ def __init__(self,
         self.on_removing_rt = on_removing_rt
         self.on_updating_rt = on_updating_rt
 
-    def _obtain_token(self, grant_type, params=None, data=None, *args, **kwargs):
+    def _obtain_token(self, grant_type, params=None, data=None,
+            rt_getter=lambda token_item: token_item["refresh_token"],
+            *args, **kwargs):
+        RT = "refresh_token"
+        _data = data.copy()  # to prevent side effect
+        refresh_token = _data.get(RT)
+        if grant_type == RT and isinstance(refresh_token, dict):
+            _data[RT] = rt_getter(refresh_token)  # Put raw RT in _data
         resp = super(Client, self)._obtain_token(
-            grant_type, params, data, *args, **kwargs)
+            grant_type, params, _data, *args, **kwargs)
         if "error" not in resp:
             _resp = resp.copy()
-            if grant_type == "refresh_token" and "refresh_token" in _resp:
-                _resp.pop("refresh_token")  # We'll handle this in its own method
+            if grant_type == RT and RT in _resp and isinstance(refresh_token, dict):
+                _resp.pop(RT)  # So we skip it in on_obtaining_tokens(); it will
+                               # be handled in self.obtain_token_by_refresh_token()
             if "scope" in _resp:
                 scope = _resp["scope"].split()  # It is conceptually a set,
                     # but we represent it as a list which can be persisted to JSON
             else:
                 # TODO: Deal with absent scope in authorization grant
-                scope = data.get("scope")
+                scope = _data.get("scope")
             self.on_obtaining_tokens({
                 "client_id": self.client_id,
                 "scope": scope,
                 "token_endpoint": self.configuration["token_endpoint"],
                 "grant_type": grant_type,  # can be used to know an IdToken-less
                                            # response is for an app or for a user
-                "response": _resp, "params": params, "data": data,
+                "response": _resp, "params": params, "data": _data,
                 })
         return resp
 
@@ -411,26 +419,29 @@ def obtain_token_by_refresh_token(self, token_item, scope=None,
         """This is an "overload" which accepts a refresh token item as a dict,
         therefore this method can relay refresh_token item to event listeners.
 
-        :param token_item: A refresh token item came from storage
+        :param token_item:
+            A refresh token item as a dict, came from the cache managed by this lib.
+
+            Alternatively, you can still use a refresh token (RT) as a string,
+            supposedly came from a token cache managed by a different library,
+            then this library will store the new RT (if Authority Server issued one)
+            into this lib's cache. This is a way to migrate from other lib to us.
         :param scope: If omitted, is treated as equal to the scope originally
             granted by the resource ownser,
             according to https://tools.ietf.org/html/rfc6749#section-6
         :param rt_getter: A callable used to extract the RT from token_item
         :param on_removing_rt: If absent, fall back to the one defined in initialization
         """
-        if isinstance(token_item, str):
-            # Satisfy the L of SOLID, although we expect caller uses a dict
-            return super(Client, self).obtain_token_by_refresh_token(
-                    token_item, scope=scope, **kwargs)
+        resp = super(Client, self).obtain_token_by_refresh_token(
+            token_item, scope=scope,
+            rt_getter=rt_getter,  # Wire up this for _obtain_token()
+            **kwargs)
         if isinstance(token_item, dict):
-            resp = super(Client, self).obtain_token_by_refresh_token(
-                    rt_getter(token_item), scope=scope, **kwargs)
             if resp.get('error') == 'invalid_grant':
                 (on_removing_rt or self.on_removing_rt)(token_item)  # Discard old RT
             if 'refresh_token' in resp:
                 self.on_updating_rt(token_item, resp['refresh_token'])
-            return resp
-        raise ValueError("token_item should not be a type %s" % type(token_item))
+        return resp
 
     def obtain_token_by_assertion(
             self, assertion, grant_type, scope=None, **kwargs):

msal/token_cache.py

@@ -111,18 +111,25 @@ def add(self, event, now=None):
             event, indent=4, sort_keys=True,
             default=str,  # A workaround when assertion is in bytes in Python 3
             ))
+        environment = realm = None
+        if "token_endpoint" in event:
+            _, environment, realm = canonicalize(event["token_endpoint"])
         response = event.get("response", {})
         access_token = response.get("access_token")
         refresh_token = response.get("refresh_token")
         id_token = response.get("id_token")
+        id_token_claims = (
+            decode_id_token(id_token, client_id=event["client_id"])
+            if id_token else {})
         client_info = {}
-        home_account_id = None
-        if "client_info" in response:
+        home_account_id = None  # It would remain None in client_credentials flow
+        if "client_info" in response:  # We asked for it, and AAD will provide it
             client_info = json.loads(base64decode(response["client_info"]))
             home_account_id = "{uid}.{utid}".format(**client_info)
-        environment = realm = None
-        if "token_endpoint" in event:
-            _, environment, realm = canonicalize(event["token_endpoint"])
+        elif id_token_claims:  # This would be an end user on ADFS-direct scenario
+            client_info["uid"] = id_token_claims.get("sub")
+            home_account_id = id_token_claims.get("sub")
+
         target = ' '.join(event.get("scope", []))  # Per schema, we don't sort it
 
         with self._lock:
@@ -148,15 +155,15 @@ def add(self, event, now=None):
                 self.modify(self.CredentialType.ACCESS_TOKEN, at, at)
 
             if client_info:
-                decoded_id_token = decode_id_token(
-                    id_token, client_id=event["client_id"]) if id_token else {}
                 account = {
                     "home_account_id": home_account_id,
                     "environment": environment,
                     "realm": realm,
-                    "local_account_id": decoded_id_token.get(
-                        "oid", decoded_id_token.get("sub")),
-                    "username": decoded_id_token.get("preferred_username"),
+                    "local_account_id": id_token_claims.get(
+                        "oid", id_token_claims.get("sub")),
+                    "username": id_token_claims.get("preferred_username")  # AAD
+                        or id_token_claims.get("upn")  # ADFS 2019
+                        or "",  # The schema does not like null
                     "authority_type":
                         self.AuthorityType.ADFS if realm == "adfs"
                         else self.AuthorityType.MSSTS,

sample/device_flow_sample.py

@@ -51,10 +51,18 @@
 
 if not result:
     logging.info("No suitable token exists in cache. Let's get a new one from AAD.")
+
     flow = app.initiate_device_flow(scopes=config["scope"])
+    if "user_code" not in flow:
+        raise ValueError(
+            "Fail to create device flow. Err: %s" % json.dumps(flow, indent=4))
+
     print(flow["message"])
+    sys.stdout.flush()  # Some terminal needs this to ensure the message is shown
+
     # Ideally you should wait here, in order to save some unnecessary polling
-    # input("Press Enter after you successfully login from another device...")
+    # input("Press Enter after signing in from another device to proceed, CTRL+C to abort.")
+
     result = app.acquire_token_by_device_flow(flow)  # By default it will block
         # You can follow this instruction to shorten the block time
         #    https://msal-python.readthedocs.io/en/latest/#msal.PublicClientApplication.acquire_token_by_device_flow

tests/test_token_cache.py

@@ -16,30 +16,29 @@ class TokenCacheTestCase(unittest.TestCase):
     @staticmethod
     def build_id_token(
             iss="issuer", sub="subject", aud="my_client_id", exp=None, iat=None,
-            preferred_username="me", **claims):
+            **claims):  # AAD issues "preferred_username", ADFS issues "upn"
         return "header.%s.signature" % base64.b64encode(json.dumps(dict({
             "iss": iss,
             "sub": sub,
             "aud": aud,
             "exp": exp or (time.time() + 100),
             "iat": iat or time.time(),
-            "preferred_username": preferred_username,
             }, **claims)).encode()).decode('utf-8')
 
     @staticmethod
     def build_response(  # simulate a response from AAD
-            uid="uid", utid="utid",  # They will form client_info
+            uid=None, utid=None,  # If present, they will form client_info
             access_token=None, expires_in=3600, token_type="some type",
             refresh_token=None,
             foci=None,
             id_token=None,  # or something generated by build_id_token()
             error=None,
             ):
-        response = {
-            "client_info": base64.b64encode(json.dumps({
+        response = {}
+        if uid and utid:  # Mimic the AAD behavior for "client_info=1" request
+            response["client_info"] = base64.b64encode(json.dumps({
                 "uid": uid, "utid": utid,
-                }).encode()).decode('utf-8'),
-            }
+                }).encode()).decode('utf-8')
         if error:
             response["error"] = error
         if access_token:
@@ -59,7 +58,7 @@ def build_response(  # simulate a response from AAD
     def setUp(self):
         self.cache = TokenCache()
 
-    def testAdd(self):
+    def testAddByAad(self):
         client_id = "my_client_id"
         id_token = self.build_id_token(
             oid="object1234", preferred_username="John Doe", aud=client_id)
@@ -132,6 +131,78 @@ def testAdd(self):
                 "appmetadata-login.example.com-my_client_id")
             )
 
+    def testAddByAdfs(self):
+        client_id = "my_client_id"
+        id_token = self.build_id_token(aud=client_id, upn="JaneDoe@example.com")
+        self.cache.add({
+            "client_id": client_id,
+            "scope": ["s2", "s1", "s3"],  # Not in particular order
+            "token_endpoint": "https://fs.msidlab8.com/adfs/oauth2/token",
+            "response": self.build_response(
+                uid=None, utid=None,  # ADFS will provide no client_info
+                expires_in=3600, access_token="an access token",
+                id_token=id_token, refresh_token="a refresh token"),
+            }, now=1000)
+        self.assertEqual(
+            {
+                'cached_at': "1000",
+                'client_id': 'my_client_id',
+                'credential_type': 'AccessToken',
+                'environment': 'fs.msidlab8.com',
+                'expires_on': "4600",
+                'extended_expires_on': "4600",
+                'home_account_id': "subject",
+                'realm': 'adfs',
+                'secret': 'an access token',
+                'target': 's2 s1 s3',
+            },
+            self.cache._cache["AccessToken"].get(
+                'subject-fs.msidlab8.com-accesstoken-my_client_id-adfs-s2 s1 s3')
+            )
+        self.assertEqual(
+            {
+                'client_id': 'my_client_id',
+                'credential_type': 'RefreshToken',
+                'environment': 'fs.msidlab8.com',
+                'home_account_id': "subject",
+                'secret': 'a refresh token',
+                'target': 's2 s1 s3',
+            },
+            self.cache._cache["RefreshToken"].get(
+                'subject-fs.msidlab8.com-refreshtoken-my_client_id--s2 s1 s3')
+            )
+        self.assertEqual(
+            {
+                'home_account_id': "subject",
+                'environment': 'fs.msidlab8.com',
+                'realm': 'adfs',
+                'local_account_id': "subject",
+                'username': "JaneDoe@example.com",
+                'authority_type': "ADFS",
+            },
+            self.cache._cache["Account"].get('subject-fs.msidlab8.com-adfs')
+            )
+        self.assertEqual(
+            {
+                'credential_type': 'IdToken',
+                'secret': id_token,
+                'home_account_id': "subject",
+                'environment': 'fs.msidlab8.com',
+                'realm': 'adfs',
+                'client_id': 'my_client_id',
+            },
+            self.cache._cache["IdToken"].get(
+                'subject-fs.msidlab8.com-idtoken-my_client_id-adfs-')
+            )
+        self.assertEqual(
+            {
+                "client_id": "my_client_id",
+                'environment': 'fs.msidlab8.com',
+            },
+            self.cache._cache.get("AppMetadata", {}).get(
+                "appmetadata-fs.msidlab8.com-my_client_id")
+            )
+
 
 class SerializableTokenCacheTestCase(TokenCacheTestCase):
     # Run all inherited test methods, and have extra check in tearDown()