Merge pull request #177 from AzureAD/release-1.2.0

Release 1.2.0

docs/conf.py

@@ -12,9 +12,9 @@
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 #
-# import os
-# import sys
-# sys.path.insert(0, os.path.abspath('.'))
+import os
+import sys
+sys.path.insert(0, os.path.abspath('..'))
 
 
 # -- Project information -----------------------------------------------------

msal/application.py

@@ -19,7 +19,7 @@
 
 
 # The __init__.py will import this. Not the other way around.
-__version__ = "1.1.0"
+__version__ = "1.2.0"
 
 logger = logging.getLogger(__name__)
 
@@ -229,6 +229,7 @@ def get_authorization_request_url(
             redirect_uri=None,
             response_type="code",  # Can be "token" if you use Implicit Grant
             prompt=None,
+            nonce=None,
             **kwargs):
         """Constructs a URL for you to start a Authorization Code Grant.
 
@@ -247,6 +248,9 @@ def get_authorization_request_url(
             You will have to specify a value explicitly.
             Its valid values are defined in Open ID Connect specs
             https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+        :param nonce:
+            A cryptographically random value used to mitigate replay attacks. See also
+            `OIDC specs <https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest>`_.
         :return: The authorization url as a string.
         """
         """ # TBD: this would only be meaningful in a new acquire_token_interactive()
@@ -276,6 +280,7 @@ def get_authorization_request_url(
             redirect_uri=redirect_uri, state=state, login_hint=login_hint,
             prompt=prompt,
             scope=decorate_scope(scopes, self.client_id),
+            nonce=nonce,
             )
 
     def acquire_token_by_authorization_code(
@@ -286,6 +291,7 @@ def acquire_token_by_authorization_code(
                 # REQUIRED, if the "redirect_uri" parameter was included in the
                 # authorization request as described in Section 4.1.1, and their
                 # values MUST be identical.
+            nonce=None,
             **kwargs):
         """The second half of the Authorization Code Grant.
 
@@ -306,6 +312,11 @@ def acquire_token_by_authorization_code(
             So the developer need to specify a scope so that we can restrict the
             token to be issued for the corresponding audience.
 
+        :param nonce:
+            If you provided a nonce when calling :func:`get_authorization_request_url`,
+            same nonce should also be provided here, so that we'll validate it.
+            An exception will be raised if the nonce in id token mismatches.
+
         :return: A dict representing the json response from AAD:
 
             - A successful response would contain "access_token" key,
@@ -326,6 +337,7 @@ def acquire_token_by_authorization_code(
                 CLIENT_CURRENT_TELEMETRY: _build_current_telemetry_request_header(
                     self.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE_ID),
                 },
+            nonce=nonce,
             **kwargs)
 
     def get_accounts(self, username=None):
@@ -713,7 +725,7 @@ def acquire_token_by_device_flow(self, flow, **kwargs):
 
     def acquire_token_by_username_password(
             self, username, password, scopes, **kwargs):
-        """Gets a token for a given resource via user credentails.
+        """Gets a token for a given resource via user credentials.
 
         See this page for constraints of Username Password Flow.
         https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki/Username-Password-Authentication

msal/oauth2cli/http.py

@@ -0,0 +1,60 @@
+"""This module documents the minimal http behaviors used by this package.
+
+Its interface is influenced by, and similar to a subset of some popular,
+real-world http libraries, such as requests, aiohttp and httpx.
+"""
+
+
+class HttpClient(object):
+    """This describes a minimal http request interface used by this package."""
+
+    def post(self, url, params=None, data=None, headers=None, **kwargs):
+        """HTTP post.
+
+        params, data and headers MUST accept a dictionary.
+        It returns an :class:`~Response`-like object.
+
+        Note: In its async counterpart, this method would be defined as async.
+        """
+        return Response()
+
+    def get(self, url, params=None, headers=None, **kwargs):
+        """HTTP get.
+
+        params, data and headers MUST accept a dictionary.
+        It returns an :class:`~Response`-like object.
+
+        Note: In its async counterpart, this method would be defined as async.
+        """
+        return Response()
+
+
+class Response(object):
+    """This describes a minimal http response interface used by this package.
+
+    :var int status_code:
+        The status code of this http response.
+
+        Our async code path would also accept an alias as "status".
+
+    :var string text:
+        The body of this http response.
+
+        Our async code path would also accept an awaitable with the same name.
+    """
+    status_code = 200  # Our async code path would also accept a name as "status"
+
+    text = "body as a string"  # Our async code path would also accept an awaitable
+        # We could define a json() method instead of a text property/method,
+        # but a `text` would be more generic,
+        # when downstream packages would potentially access some XML endpoints.
+
+    def raise_for_status(self):
+        """Raise an exception when http response status contains error"""
+        raise NotImplementedError("Your implementation should provide this")
+
+
+def _get_status_code(resp):
+    # RFC defines and some libraries use "status_code", others use "status"
+    return getattr(resp, "status_code", None) or resp.status
+

msal/oauth2cli/oidc.py

@@ -85,3 +85,37 @@ def _obtain_token(self, grant_type, *args, **kwargs):
             ret["id_token_claims"] = self.decode_id_token(ret["id_token"])
         return ret
 
+    def build_auth_request_uri(self, response_type, nonce=None, **kwargs):
+        """Generate an authorization uri to be visited by resource owner.
+
+        Return value and all other parameters are the same as
+        :func:`oauth2.Client.build_auth_request_uri`, plus new parameter(s):
+
+        :param nonce:
+            A hard-to-guess string used to mitigate replay attacks. See also
+            `OIDC specs <https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest>`_.
+        """
+        return super(Client, self).build_auth_request_uri(
+            response_type, nonce=nonce, **kwargs)
+
+    def obtain_token_by_authorization_code(self, code, nonce=None, **kwargs):
+        """Get a token via auhtorization code. a.k.a. Authorization Code Grant.
+
+        Return value and all other parameters are the same as
+        :func:`oauth2.Client.obtain_token_by_authorization_code`,
+        plus new parameter(s):
+
+        :param nonce:
+            If you provided a nonce when calling :func:`build_auth_request_uri`,
+            same nonce should also be provided here, so that we'll validate it.
+            An exception will be raised if the nonce in id token mismatches.
+        """
+        result = super(Client, self).obtain_token_by_authorization_code(
+            code, **kwargs)
+        nonce_in_id_token = result.get("id_token_claims", {}).get("nonce")
+        if "id_token_claims" in result and nonce and nonce != nonce_in_id_token:
+            raise ValueError(
+                'The nonce in id token ("%s") should match your nonce ("%s")' %
+                (nonce_in_id_token, nonce))
+        return result
+

tests/test_client.py

@@ -132,8 +132,11 @@ def test_username_password(self):
     def test_auth_code(self):
         port = CONFIG.get("listen_port", 44331)
         redirect_uri = "http://localhost:%s" % port
+        nonce = "nonce should contain sufficient entropy"
         auth_request_uri = self.client.build_auth_request_uri(
-            "code", redirect_uri=redirect_uri, scope=CONFIG.get("scope"))
+            "code",
+            nonce=nonce,
+            redirect_uri=redirect_uri, scope=CONFIG.get("scope"))
         ac = obtain_auth_code(port, auth_uri=auth_request_uri)
         self.assertNotEqual(ac, None)
         result = self.client.obtain_token_by_authorization_code(
@@ -142,6 +145,7 @@ def test_auth_code(self):
                 "scope": CONFIG.get("scope"),
                 "resource": CONFIG.get("resource"),
                 },  # MSFT AAD only
+            nonce=nonce,
             redirect_uri=redirect_uri)
         self.assertLoosely(result, lambda: self.assertIn('access_token', result))
 

tests/test_e2e.py

@@ -19,12 +19,12 @@ def _get_app_and_auth_code(
         authority="https://login.microsoftonline.com/common",
         port=44331,
         scopes=["https://graph.microsoft.com/.default"],  # Microsoft Graph
-        ):
+        **kwargs):
     from msal.oauth2cli.authcode import obtain_auth_code
     app = msal.ClientApplication(client_id, client_secret, authority=authority)
     redirect_uri = "http://localhost:%d" % port
     ac = obtain_auth_code(port, auth_uri=app.get_authorization_request_url(
-        scopes, redirect_uri=redirect_uri))
+        scopes, redirect_uri=redirect_uri, **kwargs))
     assert ac is not None
     return (app, ac, redirect_uri)
 
@@ -124,20 +124,20 @@ def test_username_password(self):
         self.skipUnlessWithConfig(["client_id", "username", "password", "scope"])
         self._test_username_password(**self.config)
 
-    def _get_app_and_auth_code(self):
+    def _get_app_and_auth_code(self, **kwargs):
         return _get_app_and_auth_code(
             self.config["client_id"],
             client_secret=self.config.get("client_secret"),
             authority=self.config.get("authority"),
             port=self.config.get("listen_port", 44331),
             scopes=self.config["scope"],
-            )
+            **kwargs)
 
-    def test_auth_code(self):
+    def _test_auth_code(self, auth_kwargs, token_kwargs):
         self.skipUnlessWithConfig(["client_id", "scope"])
-        (self.app, ac, redirect_uri) = self._get_app_and_auth_code()
+        (self.app, ac, redirect_uri) = self._get_app_and_auth_code(**auth_kwargs)
         result = self.app.acquire_token_by_authorization_code(
-            ac, self.config["scope"], redirect_uri=redirect_uri)
+            ac, self.config["scope"], redirect_uri=redirect_uri, **token_kwargs)
         logger.debug("%s.cache = %s",
             self.id(), json.dumps(self.app.token_cache._cache, indent=4))
         self.assertIn(
@@ -148,6 +148,18 @@ def test_auth_code(self):
                 error_description=result.get("error_description")))
         self.assertCacheWorksForUser(result, self.config["scope"], username=None)
 
+    def test_auth_code(self):
+        self._test_auth_code({}, {})
+
+    def test_auth_code_with_matching_nonce(self):
+        self._test_auth_code({"nonce": "foo"}, {"nonce": "foo"})
+
+    def test_auth_code_with_mismatching_nonce(self):
+        self.skipUnlessWithConfig(["client_id", "scope"])
+        (self.app, ac, redirect_uri) = self._get_app_and_auth_code(nonce="foo")
+        with self.assertRaises(ValueError):
+            self.app.acquire_token_by_authorization_code(
+                ac, self.config["scope"], redirect_uri=redirect_uri, nonce="bar")
 
     def test_ssh_cert(self):
         self.skipUnlessWithConfig(["client_id", "scope"])
@@ -412,22 +424,22 @@ def test_adfs2019_onprem_acquire_token_by_auth_code(self):
         self.assertCacheWorksForUser(result, scopes, username=None)
 
     @unittest.skipUnless(
-        os.getenv("OBO_CLIENT_SECRET"),
-        "Need OBO_CLIENT_SECRET from https://buildautomation.vault.azure.net/secrets/IdentityDivisionDotNetOBOServiceSecret")
+        os.getenv("LAB_OBO_CLIENT_SECRET"),
+        "Need LAB_OBO_CLIENT SECRET from https://msidlabs.vault.azure.net/secrets/TodoListServiceV2-OBO/c58ba97c34ca4464886943a847d1db56")
     def test_acquire_token_obo(self):
         # Some hardcoded, pre-defined settings
-        obo_client_id = "23c64cd8-21e4-41dd-9756-ab9e2c23f58c"
-        downstream_scopes = ["https://graph.microsoft.com/User.Read"]
+        obo_client_id = "f4aa5217-e87c-42b2-82af-5624dd14ee72"
+        downstream_scopes = ["https://graph.microsoft.com/.default"]
         config = self.get_lab_user(usertype="cloud")
 
         # 1. An app obtains a token representing a user, for our mid-tier service
         pca = msal.PublicClientApplication(
-            "be9b0186-7dfd-448a-a944-f771029105bf", authority=config.get("authority"))
+            "c0485386-1e9a-4663-bc96-7ab30656de7f", authority=config.get("authority"))
         pca_result = pca.acquire_token_by_username_password(
             config["username"],
             self.get_lab_user_secret(config["lab_name"]),
             scopes=[  # The OBO app's scope. Yours might be different.
-                "%s/access_as_user" % obo_client_id],
+                "api://%s/read" % obo_client_id],
             )
         self.assertIsNotNone(
             pca_result.get("access_token"),
@@ -436,7 +448,7 @@ def test_acquire_token_obo(self):
         # 2. Our mid-tier service uses OBO to obtain a token for downstream service
         cca = msal.ConfidentialClientApplication(
             obo_client_id,
-            client_credential=os.getenv("OBO_CLIENT_SECRET"),
+            client_credential=os.getenv("LAB_OBO_CLIENT_SECRET"),
             authority=config.get("authority"),
             # token_cache= ...,  # Default token cache is all-tokens-store-in-memory.
                 # That's fine if OBO app uses short-lived msal instance per session.