From f788a3bf83187d77b7179ab44e2330dbcff97593 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 30 Mar 2023 10:12:13 -0700 Subject: [PATCH 01/12] Turns out they changed to a new tag for MSAL. Fix #539 --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 013719f3..3ec1c6ab 100644 --- 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 From 2168027c4694b243ddeff08592396557d9848de0 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 17 Mar 2023 21:02:33 -0700 Subject: [PATCH 02/12] Clarify that allow_broker is not applicable to ConfidentialClientApplication It is applicable to PublicClientApplication and base class ClientApplication --- msal/application.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/msal/application.py b/msal/application.py index b3e2c209..a3295cc2 100644 --- a/msal/application.py +++ b/msal/application.py @@ -444,6 +444,8 @@ def __init__( New in version 1.19.0. :param boolean allow_broker: + This parameter is NOT applicable to :class:`ConfidentialClientApplication`. + A broker is a component installed on your device. Broker implicitly gives your device an identity. By using a broker, your device becomes a factor that can satisfy MFA (Multi-factor authentication). From 79efb89b195fbd5f317be3614f1cfd58c05a261f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sun, 26 Feb 2023 15:29:15 -0800 Subject: [PATCH 03/12] Backport test improvements --- tests/http_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/http_client.py b/tests/http_client.py index 4bff9b45..c532b94b 100644 --- a/tests/http_client.py +++ b/tests/http_client.py @@ -10,21 +10,27 @@ def __init__(self, verify=True, proxies=None, timeout=None): self.timeout = timeout def post(self, url, params=None, data=None, headers=None, **kwargs): + assert not kwargs, "Our stack shouldn't leak extra kwargs: %s" % kwargs return MinimalResponse(requests_resp=self.session.post( url, params=params, data=data, headers=headers, timeout=self.timeout)) def get(self, url, params=None, headers=None, **kwargs): + assert not kwargs, "Our stack shouldn't leak extra kwargs: %s" % kwargs return MinimalResponse(requests_resp=self.session.get( url, params=params, headers=headers, timeout=self.timeout)) + def close(self): # Not required, but we use it to avoid a warning in unit test + self.session.close() + class MinimalResponse(object): # Not for production use def __init__(self, requests_resp=None, status_code=None, text=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._raw_resp = requests_resp def raise_for_status(self): - if self._raw_resp: + if self._raw_resp is not None: # Turns out `if requests.response` won't work + # cause it would be True when 200<=status<400 self._raw_resp.raise_for_status() From a07c3ef99d0fec923560eb4994573e2e05bb0394 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sat, 4 Mar 2023 15:35:16 -0800 Subject: [PATCH 04/12] Expand http interface to include response.headers --- oauth2cli/http.py | 5 +++++ tests/http_client.py | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/oauth2cli/http.py b/oauth2cli/http.py index 12e2dac6..668b98ff 100644 --- a/oauth2cli/http.py +++ b/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/tests/http_client.py b/tests/http_client.py index c532b94b..f6f24739 100644 --- 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 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): From 21755029cbe5e9f520c84ab5897906603d864416 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 15 Mar 2023 17:31:16 -0700 Subject: [PATCH 05/12] No need for DummyHttpResponse --- tests/test_throttled_http_client.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/test_throttled_http_client.py b/tests/test_throttled_http_client.py index 93820505..aa20060d 100644 --- 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 From 49090cb692707eadf2f2248b3ea5b8d082d1e84c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 28 Apr 2023 23:56:28 -0700 Subject: [PATCH 06/12] Adjustment for new CIAM partition --- tests/test_e2e.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 44c1d5f2..657e777e 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -925,10 +925,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. From 6b2f3375064ab6d302ee788cdeeb08c30f3c219e Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 26 May 2023 11:34:27 -0700 Subject: [PATCH 07/12] Improve logs --- msal/application.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index a3295cc2..659a7409 100644 --- a/msal/application.py +++ b/msal/application.py @@ -65,6 +65,12 @@ def _str2bytes(raw): return raw +def _pii_less_home_account_id(home_account_id): + parts = home_account_id.split(".") # It could contain one or two parts + parts[0] = "********" + return ".".join(parts) + + def _clean_up(result): if isinstance(result, dict): return { @@ -1460,7 +1466,10 @@ def _acquire_token_silent_by_finding_specific_refresh_token( self.token_cache.CredentialType.REFRESH_TOKEN, # target=scopes, # AAD RTs are scope-independent query=query) - logger.debug("Found %d RTs matching %s", len(matches), query) + logger.debug("Found %d RTs matching %s", len(matches), { + k: _pii_less_home_account_id(v) if k == "home_account_id" and v else v + for k, v in query.items() + }) response = None # A distinguishable value to mean cache is empty if not matches: # Then exit early to avoid expensive operations From 1b7db8db56e27e9b57542ae5eb48d348a94e9d60 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 31 May 2023 13:58:32 -0700 Subject: [PATCH 08/12] Add more sections into TOC for the now long doc --- docs/index.rst | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 8f24a58d..5c49a7ba 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -1,4 +1,4 @@ -MSAL Python documentation +MSAL Python Documentation ========================= .. toctree:: @@ -6,8 +6,11 @@ MSAL Python documentation :caption: Contents: :hidden: - MSAL Documentation - GitHub Repository + 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 `_. @@ -58,8 +61,8 @@ MSAL Python supports some of them. `_. -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,8 +91,10 @@ MSAL proposes a clean separation between They are implemented as two separated classes, with different methods for different authentication scenarios. + + PublicClientApplication ------------------------ +======================= .. autoclass:: msal.PublicClientApplication :members: @@ -98,7 +103,7 @@ PublicClientApplication .. automethod:: __init__ ConfidentialClientApplication ------------------------------ +============================= .. autoclass:: msal.ConfidentialClientApplication :members: @@ -107,7 +112,7 @@ ConfidentialClientApplication .. automethod:: __init__ TokenCache ----------- +========== One of the parameters accepted by both `PublicClientApplication` and `ConfidentialClientApplication` From 10c8dd50bbe25bd51371c85f4121b5b396494815 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 2 Jun 2023 14:43:40 -0700 Subject: [PATCH 09/12] Remove many Sphinx warnings --- docs/index.rst | 10 +++++++--- msal/application.py | 31 ++++++++++++++++++++----------- msal/token_cache.py | 1 - 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 5c49a7ba..e608fe6b 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -91,14 +91,20 @@ 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__ @@ -107,9 +113,7 @@ ConfidentialClientApplication .. autoclass:: msal.ConfidentialClientApplication :members: - :inherited-members: - .. automethod:: __init__ TokenCache ========== diff --git a/msal/application.py b/msal/application.py index 659a7409..48b6575b 100644 --- a/msal/application.py +++ b/msal/application.py @@ -156,6 +156,9 @@ def obtain_token_by_username_password(self, username, password, **kwargs): class ClientApplication(object): + """You do not usually directly use this class. Use its subclasses instead: + :class:`PublicClientApplication` and :class:`ConfidentialClientApplication`. + """ ACQUIRE_TOKEN_SILENT_ID = "84" ACQUIRE_TOKEN_BY_REFRESH_TOKEN = "85" ACQUIRE_TOKEN_BY_USERNAME_PASSWORD_ID = "301" @@ -319,7 +322,7 @@ def __init__( to keep their traffic remain inside that region. As of 2021 May, regional service is only available for - ``acquire_token_for_client()`` sent by any of the following scenarios:: + ``acquire_token_for_client()`` sent by any of the following scenarios: 1. An app powered by a capable MSAL (MSAL Python 1.12+ will be provisioned) @@ -764,9 +767,9 @@ def initiate_auth_code_flow( Can be one of "consumers" or "organizations" or your tenant domain "contoso.com". If included, it will skip the email-based discovery process that user goes through on the sign-in page, leading to a slightly more streamlined user experience. - More information on possible values - `here `_ and - `here `_. + More information on possible values available in + `Auth Code Flow doc `_ and + `domain_hint doc `_. :param int max_age: OPTIONAL. Maximum Authentication Age. @@ -804,7 +807,7 @@ def initiate_auth_code_flow( "...": "...", // Everything else are reserved and internal } - The caller is expected to:: + The caller is expected to: 1. somehow store this content, typically inside the current session, 2. guide the end user (i.e. resource owner) to visit that auth_uri, @@ -868,9 +871,9 @@ def get_authorization_request_url( Can be one of "consumers" or "organizations" or your tenant domain "contoso.com". If included, it will skip the email-based discovery process that user goes through on the sign-in page, leading to a slightly more streamlined user experience. - More information on possible values - `here `_ and - `here `_. + More information on possible values available in + `Auth Code Flow doc `_ and + `domain_hint doc `_. :param claims_challenge: The claims_challenge parameter requests specific claims requested by the resource provider in the form of a claims_challenge directive in the www-authenticate header to be @@ -1682,6 +1685,9 @@ class PublicClientApplication(ClientApplication): # browser app or mobile app CONSOLE_WINDOW_HANDLE = object() def __init__(self, client_id, client_credential=None, **kwargs): + """Same as :func:`ClientApplication.__init__`, + except that ``client_credential`` parameter shall remain ``None``. + """ if client_credential is not None: raise ValueError("Public Client should not possess credentials") super(PublicClientApplication, self).__init__( @@ -1722,9 +1728,9 @@ def acquire_token_interactive( Can be one of "consumers" or "organizations" or your tenant domain "contoso.com". If included, it will skip the email-based discovery process that user goes through on the sign-in page, leading to a slightly more streamlined user experience. - More information on possible values - `here `_ and - `here `_. + More information on possible values available in + `Auth Code Flow doc `_ and + `domain_hint doc `_. :param claims_challenge: The claims_challenge parameter requests specific claims requested by the resource provider @@ -1994,6 +2000,9 @@ def acquire_token_by_device_flow(self, flow, claims_challenge=None, **kwargs): class ConfidentialClientApplication(ClientApplication): # server-side web app + """Same as :func:`ClientApplication.__init__`, + except that ``allow_broker`` parameter shall remain ``None``. + """ def acquire_token_for_client(self, scopes, claims_challenge=None, **kwargs): """Acquires token for the current confidential client, not for an end user. diff --git a/msal/token_cache.py b/msal/token_cache.py index 4f6d225c..49262069 100644 --- 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 { From 3e3b97a6d8dfc218dd0a7b290f7d08bad0fbe817 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 7 Jun 2023 17:44:59 -0700 Subject: [PATCH 10/12] Github removes Python 2.7 support on 2023-6-19 See also https://github.com/actions/setup-python/issues/672 MSAL downloads from Python 2.7 is less than 0.2% according to https://pypistats.org/packages/msal --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index cf56cb2a..9d24904a 100644 --- 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 From 2288b7792148892769b8753cb9df22c946fa5c40 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 28 Jun 2023 23:52:47 -0700 Subject: [PATCH 11/12] Remove acquire_token_silent(..., account=None) usage in a backward-compatible way Now acquire_token_for_client()'s cache behavior will have corresponding api id Continue to disallow acquire_token_for_client(..., force_refresh=True) --- msal/application.py | 90 +++++++++++++------ .../confidential_client_certificate_sample.py | 14 +-- sample/confidential_client_secret_sample.py | 14 +-- tests/test_application.py | 31 +++++-- tests/test_e2e.py | 12 ++- 5 files changed, 99 insertions(+), 62 deletions(-) diff --git a/msal/application.py b/msal/application.py index 48b6575b..29e3cb28 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1209,32 +1209,24 @@ def acquire_token_silent( **kwargs): """Acquire an access token for given account, without user interaction. - It is done either by finding a valid access token from cache, - or by finding a valid refresh token from cache and then automatically - use it to redeem a new access token. - + It has same parameters as the :func:`~acquire_token_silent_with_error`. + The difference is the behavior of the return value. This method will combine the cache empty and refresh error into one return value, `None`. If your app does not care about the exact token refresh error during token cache look-up, then this method is easier and recommended. - Internally, this method calls :func:`~acquire_token_silent_with_error`. - - :param claims_challenge: - The claims_challenge parameter requests specific claims requested by the resource provider - in the form of a claims_challenge directive in the www-authenticate header to be - returned from the UserInfo Endpoint and/or in the ID Token and/or Access Token. - It is a string of a JSON object which contains lists of claims being requested from these locations. - :return: - A dict containing no "error" key, and typically contains an "access_token" key, if cache lookup succeeded. - None when cache lookup does not yield a token. """ - result = self.acquire_token_silent_with_error( + if not account: + return None # A backward-compatible NO-OP to drop the account=None usage + result = _clean_up(self._acquire_token_silent_with_error( scopes, account, authority=authority, force_refresh=force_refresh, - claims_challenge=claims_challenge, **kwargs) + claims_challenge=claims_challenge, **kwargs)) return result if result and "error" not in result else None def acquire_token_silent_with_error( @@ -1258,9 +1250,10 @@ def acquire_token_silent_with_error( :param list[str] scopes: (Required) Scopes requested to access a protected API (a resource). - :param account: - one of the account object returned by :func:`~get_accounts`, - or use None when you want to find an access token for this client. + :param account: (Required) + One of the account object returned by :func:`~get_accounts`. + Starting from MSAL Python 1.23, + a ``None`` input will become a NO-OP and always return ``None``. :param force_refresh: If True, it will skip Access Token look-up, and try to find a Refresh Token to obtain a new Access Token. @@ -1276,6 +1269,20 @@ def acquire_token_silent_with_error( - None when there is simply no token in the cache. - A dict containing an "error" key, when token refresh failed. """ + if not account: + return None # A backward-compatible NO-OP to drop the account=None usage + return _clean_up(self._acquire_token_silent_with_error( + scopes, account, authority=authority, force_refresh=force_refresh, + claims_challenge=claims_challenge, **kwargs)) + + def _acquire_token_silent_with_error( + self, + scopes, # type: List[str] + account, # type: Optional[Account] + authority=None, # See get_authorization_request_url() + force_refresh=False, # type: Optional[boolean] + claims_challenge=None, + **kwargs): assert isinstance(scopes, list), "Invalid parameter type" self._validate_ssh_cert_input_data(kwargs.get("data", {})) correlation_id = msal.telemetry._get_new_correlation_id() @@ -1335,7 +1342,11 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( force_refresh=False, # type: Optional[boolean] claims_challenge=None, correlation_id=None, + http_exceptions=None, **kwargs): + # This internal method has two calling patterns: + # it accepts a non-empty account to find token for a user, + # and accepts account=None to find a token for the current app. access_token_from_cache = None if not (force_refresh or claims_challenge): # Bypass AT when desired or using claims query={ @@ -1372,6 +1383,10 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( else: refresh_reason = msal.telemetry.FORCE_REFRESH # TODO: It could also mean claims_challenge assert refresh_reason, "It should have been established at this point" + if not http_exceptions: # It can be a tuple of exceptions + # The exact HTTP exceptions are transportation-layer dependent + from requests.exceptions import RequestException # Lazy load + http_exceptions = (RequestException,) try: data = kwargs.get("data", {}) if account and account.get("authority_type") == _AUTHORITY_TYPE_CLOUDSHELL: @@ -1391,14 +1406,19 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( if response: # The broker provided a decisive outcome, so we use it return self._process_broker_response(response, scopes, data) - result = _clean_up(self._acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( - authority, self._decorate_scope(scopes), account, - refresh_reason=refresh_reason, claims_challenge=claims_challenge, - correlation_id=correlation_id, - **kwargs)) + if account: + result = self._acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( + authority, self._decorate_scope(scopes), account, + refresh_reason=refresh_reason, claims_challenge=claims_challenge, + correlation_id=correlation_id, + **kwargs) + else: # The caller is acquire_token_for_client() + result = self._acquire_token_for_client( + scopes, refresh_reason, claims_challenge=claims_challenge, + **kwargs) if (result and "error" not in result) or (not access_token_from_cache): return result - except: # The exact HTTP exception is transportation-layer dependent + except http_exceptions: # Typically network error. Potential AAD outage? if not access_token_from_cache: # It means there is no fall back option raise # We choose to bubble up the exception @@ -2007,6 +2027,9 @@ class ConfidentialClientApplication(ClientApplication): # server-side web app def acquire_token_for_client(self, scopes, claims_challenge=None, **kwargs): """Acquires token for the current confidential client, not for an end user. + Since MSAL Python 1.23, it will automatically look for token from cache, + and only send request to Identity Provider when cache misses. + :param list[str] scopes: (Required) Scopes requested to access a protected API (a resource). :param claims_challenge: @@ -2020,7 +2043,20 @@ def acquire_token_for_client(self, scopes, claims_challenge=None, **kwargs): - A successful response would contain "access_token" key, - an error response would contain "error" and usually "error_description". """ - # TBD: force_refresh behavior + if kwargs.get("force_refresh"): + raise ValueError( # We choose to disallow force_refresh + "Historically, this method does not support force_refresh behavior. " + ) + return _clean_up(self._acquire_token_silent_with_error( + scopes, None, claims_challenge=claims_challenge, **kwargs)) + + def _acquire_token_for_client( + self, + scopes, + refresh_reason, + claims_challenge=None, + **kwargs + ): if self.authority.tenant.lower() in ["common", "organizations"]: warnings.warn( "Using /common or /organizations authority " @@ -2028,16 +2064,16 @@ def acquire_token_for_client(self, scopes, claims_challenge=None, **kwargs): "Please use a specific tenant instead.", DeprecationWarning) self._validate_ssh_cert_input_data(kwargs.get("data", {})) telemetry_context = self._build_telemetry_context( - self.ACQUIRE_TOKEN_FOR_CLIENT_ID) + self.ACQUIRE_TOKEN_FOR_CLIENT_ID, refresh_reason=refresh_reason) client = self._regional_client or self.client - response = _clean_up(client.obtain_token_for_client( + response = client.obtain_token_for_client( scope=scopes, # This grant flow requires no scope decoration headers=telemetry_context.generate_headers(), data=dict( kwargs.pop("data", {}), claims=_merge_claims_challenge_and_capabilities( self._client_capabilities, claims_challenge)), - **kwargs)) + **kwargs) telemetry_context.update_telemetry(response) return response diff --git a/sample/confidential_client_certificate_sample.py b/sample/confidential_client_certificate_sample.py index 7e5d8069..6cd22a86 100644 --- 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 index d4c06e20..61fd1db7 100644 --- 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/tests/test_application.py b/tests/test_application.py index b62f41d5..0d93737e 100644 --- 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 index 657e777e..d1fc50dd 100644 --- 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, From 44c3bfbc6edaf7927c4372f6a85b6dd3ef3ea50c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 11 Jul 2023 17:58:19 -0700 Subject: [PATCH 12/12] Bumping up version numbers --- msal/application.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/msal/application.py b/msal/application.py index 29e3cb28..16fbac28 100644 --- a/msal/application.py +++ b/msal/application.py @@ -25,7 +25,7 @@ # The __init__.py will import this. Not the other way around. -__version__ = "1.22.0" # When releasing, also check and bump our dependencies's versions if needed +__version__ = "1.23.0" # When releasing, also check and bump our dependencies's versions if needed logger = logging.getLogger(__name__) _AUTHORITY_TYPE_CLOUDSHELL = "CLOUDSHELL" diff --git a/setup.py b/setup.py index 721baa6d..f7a2a4a1 100644 --- 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 #