Merge pull request #581 from AzureAD/release-1.23.0

rayluo · web-flow · commit 131802556e8c · 2023-07-22T10:08:32.000-07:00
MSAL Python 1.23.0
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
@@ -26,7 +26,7 @@ jobs:
     runs-on: ubuntu-latest  # It switched to 22.04 shortly after 2022-Nov-8
     strategy:
       matrix:
-        python-version: [2.7, 3.7, 3.8, 3.9, "3.10", "3.11", "3.12-dev"]
+        python-version: [3.7, 3.8, 3.9, "3.10", "3.11", "3.12-dev"]
 
     steps:
     - uses: actions/checkout@v2
diff --git a/docs/index.rst b/docs/index.rst
@@ -1,13 +1,16 @@
-MSAL Python documentation
+MSAL Python Documentation
 =========================
 
 .. toctree::
    :maxdepth: 2
    :caption: Contents:
    :hidden:
 
-   MSAL Documentation <https://docs.microsoft.com/en-au/azure/active-directory/develop/msal-authentication-flows>
-   GitHub Repository <https://github.com/AzureAD/microsoft-authentication-library-for-python>
+   index
+
+..
+    Comment: Perhaps because of the theme, only the first level sections will show in TOC,
+    regardless of maxdepth setting.
 
 You can find high level conceptual documentations in the project
 `README <https://github.com/AzureAD/microsoft-authentication-library-for-python>`_.
@@ -58,8 +61,8 @@ MSAL Python supports some of them.
   <https://github.com/AzureAD/microsoft-authentication-library-for-python/tree/dev/sample>`_.
 
 
-API
-===
+API Reference
+=============
 
 The following section is the API Reference of MSAL Python.
 The API Reference is like a dictionary. You **read this API section when and only when**:
@@ -88,26 +91,32 @@ MSAL proposes a clean separation between
 They are implemented as two separated classes,
 with different methods for different authentication scenarios.
 
+ClientApplication
+=================
+
+.. autoclass:: msal.ClientApplication
+   :members:
+   :inherited-members:
+
+   .. automethod:: __init__
+
 PublicClientApplication
------------------------
+=======================
 
 .. autoclass:: msal.PublicClientApplication
    :members:
-   :inherited-members:
 
    .. automethod:: __init__
 
 ConfidentialClientApplication
------------------------------
+=============================
 
 .. autoclass:: msal.ConfidentialClientApplication
    :members:
-   :inherited-members:
 
-   .. automethod:: __init__
 
 TokenCache
-----------
+==========
 
 One of the parameters accepted by
 both `PublicClientApplication` and `ConfidentialClientApplication`
diff --git a/msal/application.py b/msal/application.py
diff --git a/msal/oauth2cli/http.py b/msal/oauth2cli/http.py
@@ -58,6 +58,11 @@ class Response(object):
         # but a `text` would be more generic,
         # when downstream packages would potentially access some XML endpoints.
 
+    headers = {}  # Duplicated headers are expected to be combined into one header
+                  # with its value as a comma-separated string.
+                  # https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.2
+                  # Popular HTTP libraries model it as a case-insensitive dict.
+
     def raise_for_status(self):
         """Raise an exception when http response status contains error"""
         raise NotImplementedError("Your implementation should provide this")
diff --git a/msal/token_cache.py b/msal/token_cache.py
@@ -102,7 +102,6 @@ def find(self, credential_type, target=None, query=None):
                 ]
 
     def add(self, event, now=None):
-        # type: (dict) -> None
         """Handle a token obtaining event, and add tokens into cache."""
         def make_clean_copy(dictionary, sensitive_fields):  # Masks sensitive info
             return {
diff --git a/sample/confidential_client_certificate_sample.py b/sample/confidential_client_certificate_sample.py
@@ -51,17 +51,9 @@
                        # https://msal-python.readthedocs.io/en/latest/#msal.SerializableTokenCache
     )
 
-# The pattern to acquire a token looks like this.
-result = None
-
-# Firstly, looks up a token from cache
-# Since we are looking for token for the current app, NOT for an end user,
-# notice we give account parameter as None.
-result = app.acquire_token_silent(config["scope"], account=None)
-
-if not result:
-    logging.info("No suitable token exists in cache. Let's get a new one from AAD.")
-    result = app.acquire_token_for_client(scopes=config["scope"])
+# Since MSAL 1.23, acquire_token_for_client(...) will automatically look up
+# a token from cache, and fall back to acquire a fresh token when needed.
+result = app.acquire_token_for_client(scopes=config["scope"])
 
 if "access_token" in result:
     # Calling graph using the access token
diff --git a/sample/confidential_client_secret_sample.py b/sample/confidential_client_secret_sample.py
@@ -50,17 +50,9 @@
                        # https://msal-python.readthedocs.io/en/latest/#msal.SerializableTokenCache
     )
 
-# The pattern to acquire a token looks like this.
-result = None
-
-# Firstly, looks up a token from cache
-# Since we are looking for token for the current app, NOT for an end user,
-# notice we give account parameter as None.
-result = app.acquire_token_silent(config["scope"], account=None)
-
-if not result:
-    logging.info("No suitable token exists in cache. Let's get a new one from AAD.")
-    result = app.acquire_token_for_client(scopes=config["scope"])
+# Since MSAL 1.23, acquire_token_for_client(...) will automatically look up
+# a token from cache, and fall back to acquire a fresh token when needed.
+result = app.acquire_token_for_client(scopes=config["scope"])
 
 if "access_token" in result:
     # Calling graph using the access token
diff --git a/setup.cfg b/setup.cfg
@@ -5,5 +5,5 @@ universal=1
 project_urls =
     Changelog = https://github.com/AzureAD/microsoft-authentication-library-for-python/releases
     Documentation = https://msal-python.readthedocs.io/
-    Questions = https://stackoverflow.com/questions/tagged/msal+python
+    Questions = https://stackoverflow.com/questions/tagged/azure-ad-msal+python
     Feature/Bug Tracker = https://github.com/AzureAD/microsoft-authentication-library-for-python/issues
diff --git a/setup.py b/setup.py
@@ -77,7 +77,7 @@
         'requests>=2.0.0,<3',
         'PyJWT[crypto]>=1.0.0,<3',  # MSAL does not use jwt.decode(), therefore is insusceptible to CVE-2022-29217 so no need to bump to PyJWT 2.4+
 
-        'cryptography>=0.6,<43',
+        'cryptography>=0.6,<44',
             # load_pem_private_key() is available since 0.6
             # https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst#06---2014-09-29
             #
diff --git a/tests/http_client.py b/tests/http_client.py
@@ -25,9 +25,10 @@ def close(self):  # Not required, but we use it to avoid a warning in unit test
 
 
 class MinimalResponse(object):  # Not for production use
-    def __init__(self, requests_resp=None, status_code=None, text=None):
+    def __init__(self, requests_resp=None, status_code=None, text=None, headers=None):
         self.status_code = status_code or requests_resp.status_code
-        self.text = text or requests_resp.text
+        self.text = text if text is not None else requests_resp.text
+        self.headers = {} if headers is None else headers
         self._raw_resp = requests_resp
 
     def raise_for_status(self):
diff --git a/tests/test_application.py b/tests/test_application.py
@@ -382,8 +382,8 @@ def test_aging_token_and_unavailable_aad_should_return_old_token(self):
         old_at = "old AT"
         self.populate_cache(access_token=old_at, expires_in=3599, refresh_in=-1)
         def mock_post(url, headers=None, *args, **kwargs):
-            self.assertEqual("4|84,2|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
-            return MinimalResponse(status_code=400, text=json.dumps({"error": error}))
+            self.assertEqual("4|84,4|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
+            return MinimalResponse(status_code=400, text=json.dumps({"error": "foo"}))
         result = self.app.acquire_token_silent(['s1'], self.account, post=mock_post)
         self.assertEqual(old_at, result.get("access_token"))
 
@@ -549,12 +549,31 @@ def setUpClass(cls):  # Initialization at runtime, not interpret-time
             authority="https://login.microsoftonline.com/common")
 
     def test_acquire_token_for_client(self):
-        at = "this is an access token"
         def mock_post(url, headers=None, *args, **kwargs):
-            self.assertEqual("4|730,0|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
-            return MinimalResponse(status_code=200, text=json.dumps({"access_token": at}))
+            self.assertEqual("4|730,2|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
+            return MinimalResponse(status_code=200, text=json.dumps({
+                "access_token": "AT 1",
+                "expires_in": 0,
+                }))
         result = self.app.acquire_token_for_client(["scope"], post=mock_post)
-        self.assertEqual(at, result.get("access_token"))
+        self.assertEqual("AT 1", result.get("access_token"), "Shall get a new token")
+
+        def mock_post(url, headers=None, *args, **kwargs):
+            self.assertEqual("4|730,3|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
+            return MinimalResponse(status_code=200, text=json.dumps({
+                "access_token": "AT 2",
+                "expires_in": 3600,
+                "refresh_in": -100,  # A hack to make sure it will attempt refresh
+                }))
+        result = self.app.acquire_token_for_client(["scope"], post=mock_post)
+        self.assertEqual("AT 2", result.get("access_token"), "Shall get a new token")
+
+        def mock_post(url, headers=None, *args, **kwargs):
+            # 1/0  # TODO: Make sure this was called
+            self.assertEqual("4|730,4|", (headers or {}).get(CLIENT_CURRENT_TELEMETRY))
+            return MinimalResponse(status_code=400, text=json.dumps({"error": "foo"}))
+        result = self.app.acquire_token_for_client(["scope"], post=mock_post)
+        self.assertEqual("AT 2", result.get("access_token"), "Shall get aging token")
 
     def test_acquire_token_on_behalf_of(self):
         at = "this is an access token"
diff --git a/tests/test_e2e.py b/tests/test_e2e.py
@@ -146,17 +146,15 @@ def assertCacheWorksForApp(self, result_from_wire, scope):
             json.dumps(self.app.token_cache._cache, indent=4),
             json.dumps(result_from_wire.get("id_token_claims"), indent=4),
             )
-        # Going to test acquire_token_silent(...) to locate an AT from cache
-        result_from_cache = self.app.acquire_token_silent(scope, account=None)
+        self.assertIsNone(
+            self.app.acquire_token_silent(scope, account=None),
+            "acquire_token_silent(..., account=None) shall always return None")
+        # Going to test acquire_token_for_client(...) to locate an AT from cache
+        result_from_cache = self.app.acquire_token_for_client(scope)
         self.assertIsNotNone(result_from_cache)
         self.assertEqual(
             result_from_wire['access_token'], result_from_cache['access_token'],
             "We should get a cached AT")
-        self.app.acquire_token_silent(
-            # Result will typically be None, because client credential grant returns no RT.
-            # But we care more on this call should succeed without exception.
-            scope, account=None,
-            force_refresh=True)  # Mimic the AT already expires
 
     @classmethod
     def _build_app(cls,
@@ -925,10 +923,16 @@ def test_ciam_acquire_token_for_client(self):
             client_secret=self.get_lab_user_secret(
                 self.app_config["clientSecret"].split("=")[-1]),
             authority=self.app_config["authority"],
-            scope=["{}/.default".format(self.app_config["appId"])],  # App permission
+            #scope=["{}/.default".format(self.app_config["appId"])],  # AADSTS500207: The account type can't be used for the resource you're trying to access.
+            #scope=["api://{}/.default".format(self.app_config["appId"])],  # AADSTS500011: The resource principal named api://ced781e7-bdb0-4c99-855c-d3bacddea88a was not found in the tenant named MSIDLABCIAM2. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You might have sent your authentication request to the wrong tenant.
+            scope=self.app_config["scopes"],  # It shall ends with "/.default"
             )
 
     def test_ciam_acquire_token_by_ropc(self):
+        """CIAM does not officially support ROPC, especially not for external emails.
+
+        We keep this test case for now, because the test data will use a local email.
+        """
         # Somehow, this would only work after creating a secret for the test app
         # and enabling "Allow public client flows".
         # Otherwise it would hit AADSTS7000218.
diff --git a/tests/test_throttled_http_client.py b/tests/test_throttled_http_client.py
@@ -11,19 +11,13 @@
 logging.basicConfig(level=logging.DEBUG)
 
 
-class DummyHttpResponse(MinimalResponse):
-    def __init__(self, headers=None, **kwargs):
-        self.headers = {} if headers is None else headers
-        super(DummyHttpResponse, self).__init__(**kwargs)
-
-
 class DummyHttpClient(object):
     def __init__(self, status_code=None, response_headers=None):
         self._status_code = status_code
         self._response_headers = response_headers
 
     def _build_dummy_response(self):
-        return DummyHttpResponse(
+        return MinimalResponse(
             status_code=self._status_code,
             headers=self._response_headers,
             text=random(),  # So that we'd know whether a new response is received

Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,6 @@ def find(self, credential_type, target=None, query=None):`
`102`	`102`	`]`
`103`	`103`
`104`	`104`	`def add(self, event, now=None):`
`105`		`- # type: (dict) -> None`
`106`	`105`	`"""Handle a token obtaining event, and add tokens into cache."""`
`107`	`106`	`def make_clean_copy(dictionary, sensitive_fields): # Masks sensitive info`
`108`	`107`	`return {`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@`
`77`	`77`	`'requests>=2.0.0,<3',`
`78`	`78`	`'PyJWT[crypto]>=1.0.0,<3', # MSAL does not use jwt.decode(), therefore is insusceptible to CVE-2022-29217 so no need to bump to PyJWT 2.4+`
`79`	`79`
`80`		`- 'cryptography>=0.6,<43',`
	`80`	`+ 'cryptography>=0.6,<44',`
`81`	`81`	`# load_pem_private_key() is available since 0.6`
`82`	`82`	`# https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst#06---2014-09-29`
`83`	`83`	`#`