From 1ae2d19c6e5f8544531566ac74d0422baa0a4a8a Mon Sep 17 00:00:00 2001 From: Bogdan Gavril Date: Wed, 20 Dec 2023 15:40:14 +0000 Subject: [PATCH 01/25] Update issue templates (#642) * Update issue templates * Update feature_request.md * Update feature_request.md * Remove excess spaces, and rename .md to .yaml --------- Co-authored-by: Ray Luo --- .github/ISSUE_TEMPLATE/bug_report.md | 4 +-- .github/ISSUE_TEMPLATE/feature_request.yaml | 40 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/feature_request.yaml diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 8d823e8..cbd8381 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,8 +1,8 @@ --- name: Bug report about: Create a report to help us improve -title: '[Bug] ' -labels: ["untriaged", "needs attention"] +title: "[Bug] " +labels: needs attention, untriaged assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/feature_request.yaml b/.github/ISSUE_TEMPLATE/feature_request.yaml new file mode 100644 index 0000000..ddc73b5 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yaml @@ -0,0 +1,40 @@ +name: Feature request +description: Suggest a new feature for MSAL Python. +labels: ["feature request", "untriaged", "needs attention"] +title : '[Feature Request] ' +body: +- type: markdown + attributes: + value: | + ## Before submitting your feature request + Please make sure that your question or issue is not already covered in [MSAL documentation](https://learn.microsoft.com/entra/msal/python/) or [samples](https://learn.microsoft.com/azure/active-directory/develop/sample-v2-code?tabs=apptype). + +- type: markdown + attributes: + value: | + ## Feature request for MSAL Python + +- type: dropdown + attributes: + label: MSAL client type + description: Are you using Public Client (desktop apps, CLI apps) or Confidential Client (web apps, web APIs, service-to-service, managed identity)? + multiple: true + options: + - "Public" + - "Confidential" + validations: + required: true + +- type: textarea + attributes: + label: Problem Statement + description: "Describe the problem or context for this feature request." + validations: + required: true + +- type: textarea + attributes: + label: Proposed solution + description: "Describe the solution you'd like." + validations: + required: false From 72b853d91e1b70844fde98eb1135b0dac93ff52f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sun, 3 Dec 2023 15:40:27 -0800 Subject: [PATCH 02/25] No more gibberish log from https request to mess up the current terminal --- oauth2cli/authcode.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/oauth2cli/authcode.py b/oauth2cli/authcode.py index 5d46528..ba26622 100644 --- a/oauth2cli/authcode.py +++ b/oauth2cli/authcode.py @@ -102,6 +102,11 @@ def _escape(key_value_pairs): return {k: escape(v) for k, v in key_value_pairs.items()} +def _printify(text): + # If an https request is sent to an http server, the text needs to be repr-ed + return repr(text) if isinstance(text, str) and not text.isprintable() else text + + class _AuthCodeHandler(BaseHTTPRequestHandler): def do_GET(self): # For flexibility, we choose to not check self.path matching redirect_uri @@ -135,7 +140,8 @@ def _send_full_response(self, body, is_ok=True): self.wfile.write(body.encode("utf-8")) def log_message(self, format, *args): - logger.debug(format, *args) # To override the default log-to-stderr behavior + # To override the default log-to-stderr behavior + logger.debug(format, *map(_printify, args)) class _AuthCodeHttpServer(HTTPServer, object): From 866ba2b725bcbd49add91d777d0469d7019dceb0 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 28 Dec 2023 11:45:39 -0800 Subject: [PATCH 03/25] AT POP with SHR is tested with Graph end-to-end --- tests/test_e2e.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index ace95a5..33c8cf5 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -1241,9 +1241,6 @@ def test_at_pop_calling_pattern(self): resp = requests.get(api_endpoint, verify=False, headers={ "Authorization": "pop {}".format(result["access_token"]), }) - if resp.status_code != 200: - # TODO https://teams.microsoft.com/l/message/19:b1697a70b1de43ddaea281d98ff2e985@thread.v2/1700184847801?context=%7B%22contextType%22%3A%22chat%22%7D - self.skipTest("We haven't got this end-to-end test case working") self.assertEqual(resp.status_code, 200, "POP resource should be accessible") def _extract_pop_nonce(self, www_authenticate): From c1a0ce19f6c1d3252db2e0b38e7e6381a17568cd Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 1 Jan 2024 21:50:36 -0800 Subject: [PATCH 04/25] Sort scopes before writing to token cache --- msal/token_cache.py | 2 +- tests/test_token_cache.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/msal/token_cache.py b/msal/token_cache.py index bd6d8a6..da67e07 100644 --- a/msal/token_cache.py +++ b/msal/token_cache.py @@ -160,7 +160,7 @@ def __add(self, event, now=None): decode_id_token(id_token, client_id=event["client_id"]) if id_token else {}) client_info, home_account_id = self.__parse_account(response, id_token_claims) - target = ' '.join(event.get("scope") or []) # Per schema, we don't sort it + target = ' '.join(sorted(event.get("scope") or [])) # Schema should have required sorting with self._lock: now = int(time.time() if now is None else now) diff --git a/tests/test_token_cache.py b/tests/test_token_cache.py index 2fe486c..94bf496 100644 --- a/tests/test_token_cache.py +++ b/tests/test_token_cache.py @@ -76,11 +76,11 @@ def testAddByAad(self): 'home_account_id': "uid.utid", 'realm': 'contoso', 'secret': 'an access token', - 'target': 's2 s1 s3', + 'target': 's1 s2 s3', # Sorted 'token_type': 'some type', }, self.cache._cache["AccessToken"].get( - 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s2 s1 s3') + 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s1 s2 s3') ) self.assertEqual( { @@ -90,10 +90,10 @@ def testAddByAad(self): 'home_account_id': "uid.utid", 'last_modification_time': '1000', 'secret': 'a refresh token', - 'target': 's2 s1 s3', + 'target': 's1 s2 s3', # Sorted }, self.cache._cache["RefreshToken"].get( - 'uid.utid-login.example.com-refreshtoken-my_client_id--s2 s1 s3') + 'uid.utid-login.example.com-refreshtoken-my_client_id--s1 s2 s3') ) self.assertEqual( { @@ -150,11 +150,11 @@ def testAddByAdfs(self): 'home_account_id': "subject", 'realm': 'adfs', 'secret': 'an access token', - 'target': 's2 s1 s3', + 'target': 's1 s2 s3', # Sorted 'token_type': 'some type', }, self.cache._cache["AccessToken"].get( - 'subject-fs.msidlab8.com-accesstoken-my_client_id-adfs-s2 s1 s3') + 'subject-fs.msidlab8.com-accesstoken-my_client_id-adfs-s1 s2 s3') ) self.assertEqual( { @@ -164,10 +164,10 @@ def testAddByAdfs(self): 'home_account_id': "subject", 'last_modification_time': "1000", 'secret': 'a refresh token', - 'target': 's2 s1 s3', + 'target': 's1 s2 s3', # Sorted }, self.cache._cache["RefreshToken"].get( - 'subject-fs.msidlab8.com-refreshtoken-my_client_id--s2 s1 s3') + 'subject-fs.msidlab8.com-refreshtoken-my_client_id--s1 s2 s3') ) self.assertEqual( { @@ -214,7 +214,7 @@ def test_key_id_is_also_recorded(self): refresh_token="a refresh token"), }, now=1000) cached_key_id = self.cache._cache["AccessToken"].get( - 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s2 s1 s3', + 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s1 s2 s3', {}).get("key_id") self.assertEqual(my_key_id, cached_key_id, "AT should be bound to the key") @@ -229,7 +229,7 @@ def test_refresh_in_should_be_recorded_as_refresh_on(self): # Sounds weird. Yep ), #refresh_token="a refresh token"), }, now=1000) refresh_on = self.cache._cache["AccessToken"].get( - 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s2 s1 s3', + 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s1 s2 s3', {}).get("refresh_on") self.assertEqual("2800", refresh_on, "Should save refresh_on") From 313d7219c9b098d727ce3b66cdf0019c3fc74c5f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 4 Jan 2024 23:26:33 -0800 Subject: [PATCH 05/25] O(1) happy path for access token hits --- msal/application.py | 11 +++++---- msal/token_cache.py | 56 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/msal/application.py b/msal/application.py index 49de7cd..89d0aec 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1357,13 +1357,14 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( key_id = kwargs.get("data", {}).get("key_id") if key_id: # Some token types (SSH-certs, POP) are bound to a key query["key_id"] = key_id - matches = self.token_cache.find( - self.token_cache.CredentialType.ACCESS_TOKEN, - target=scopes, - query=query) now = time.time() refresh_reason = msal.telemetry.AT_ABSENT - for entry in matches: + for entry in self.token_cache._find( # It returns a generator + self.token_cache.CredentialType.ACCESS_TOKEN, + target=scopes, + query=query, + ): # Note that _find() holds a lock during this for loop; + # that is fine because this loop is fast expires_in = int(entry["expires_on"]) - now if expires_in < 5*60: # Then consider it expired refresh_reason = msal.telemetry.AT_EXPIRED diff --git a/msal/token_cache.py b/msal/token_cache.py index da67e07..ae408a9 100644 --- a/msal/token_cache.py +++ b/msal/token_cache.py @@ -88,20 +88,60 @@ def __init__(self): "appmetadata-{}-{}".format(environment or "", client_id or ""), } - def find(self, credential_type, target=None, query=None): - target = target or [] + def _get_access_token( + self, + home_account_id, environment, client_id, realm, target, # Together they form a compound key + default=None, + ): # O(1) + return self._get( + self.CredentialType.ACCESS_TOKEN, + self.key_makers[TokenCache.CredentialType.ACCESS_TOKEN]( + home_account_id=home_account_id, + environment=environment, + client_id=client_id, + realm=realm, + target=" ".join(target), + ), + default=default) + + def _get(self, credential_type, key, default=None): # O(1) + with self._lock: + return self._cache.get(credential_type, {}).get(key, default) + + def _find(self, credential_type, target=None, query=None): # O(n) generator + """Returns a generator of matching entries. + + It is O(1) for AT hits, and O(n) for other types. + Note that it holds a lock during the entire search. + """ + target = sorted(target or []) # Match the order sorted by add() assert isinstance(target, list), "Invalid parameter type" + + preferred_result = None + if (credential_type == self.CredentialType.ACCESS_TOKEN + and "home_account_id" in query and "environment" in query + and "client_id" in query and "realm" in query and target + ): # Special case for O(1) AT lookup + preferred_result = self._get_access_token( + query["home_account_id"], query["environment"], + query["client_id"], query["realm"], target) + if preferred_result: + yield preferred_result + target_set = set(target) with self._lock: # Since the target inside token cache key is (per schema) unsorted, # there is no point to attempt an O(1) key-value search here. # So we always do an O(n) in-memory search. - return [entry - for entry in self._cache.get(credential_type, {}).values() - if is_subdict_of(query or {}, entry) - and (target_set <= set(entry.get("target", "").split()) - if target else True) - ] + for entry in self._cache.get(credential_type, {}).values(): + if is_subdict_of(query or {}, entry) and ( + target_set <= set(entry.get("target", "").split()) + if target else True): + if entry != preferred_result: # Avoid yielding the same entry twice + yield entry + + def find(self, credential_type, target=None, query=None): # Obsolete. Use _find() instead. + return list(self._find(credential_type, target=target, query=query)) def add(self, event, now=None): """Handle a token obtaining event, and add tokens into cache.""" From 5272fbd8a86ca635f8af2662c8a3a0ce67b39f31 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 5 Jan 2024 23:26:01 -0800 Subject: [PATCH 06/25] Might as well refactor a _get_app_metadata() --- msal/application.py | 6 ++---- msal/token_cache.py | 9 +++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/msal/application.py b/msal/application.py index 89d0aec..afa08f5 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1493,10 +1493,8 @@ def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( **kwargs) or last_resp def _get_app_metadata(self, environment): - apps = self.token_cache.find( # Use find(), rather than token_cache.get(...) - TokenCache.CredentialType.APP_METADATA, query={ - "environment": environment, "client_id": self.client_id}) - return apps[0] if apps else {} + return self.token_cache._get_app_metadata( + environment=environment, client_id=self.client_id, default={}) def _acquire_token_silent_by_finding_specific_refresh_token( self, authority, scopes, query, diff --git a/msal/token_cache.py b/msal/token_cache.py index ae408a9..d19a1db 100644 --- a/msal/token_cache.py +++ b/msal/token_cache.py @@ -104,6 +104,15 @@ def _get_access_token( ), default=default) + def _get_app_metadata(self, environment, client_id, default=None): # O(1) + return self._get( + self.CredentialType.APP_METADATA, + self.key_makers[TokenCache.CredentialType.APP_METADATA]( + environment=environment, + client_id=client_id, + ), + default=default) + def _get(self, credential_type, key, default=None): # O(1) with self._lock: return self._cache.get(credential_type, {}).get(key, default) From 84bdfabfc3cbd73bca485b2420fcb7ccc01191d0 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 9 Jan 2024 13:52:18 -0800 Subject: [PATCH 07/25] Prevent crash on token_cache.find(..., query=None) --- msal/token_cache.py | 1 + tests/test_token_cache.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/msal/token_cache.py b/msal/token_cache.py index d19a1db..f9a5580 100644 --- a/msal/token_cache.py +++ b/msal/token_cache.py @@ -128,6 +128,7 @@ def _find(self, credential_type, target=None, query=None): # O(n) generator preferred_result = None if (credential_type == self.CredentialType.ACCESS_TOKEN + and isinstance(query, dict) and "home_account_id" in query and "environment" in query and "client_id" in query and "realm" in query and target ): # Special case for O(1) AT lookup diff --git a/tests/test_token_cache.py b/tests/test_token_cache.py index 94bf496..4e301fa 100644 --- a/tests/test_token_cache.py +++ b/tests/test_token_cache.py @@ -65,8 +65,7 @@ def testAddByAad(self): expires_in=3600, access_token="an access token", id_token=id_token, refresh_token="a refresh token"), }, now=1000) - self.assertEqual( - { + access_token_entry = { 'cached_at': "1000", 'client_id': 'my_client_id', 'credential_type': 'AccessToken', @@ -78,10 +77,16 @@ def testAddByAad(self): 'secret': 'an access token', 'target': 's1 s2 s3', # Sorted 'token_type': 'some type', - }, + } + self.assertEqual( + access_token_entry, self.cache._cache["AccessToken"].get( 'uid.utid-login.example.com-accesstoken-my_client_id-contoso-s1 s2 s3') ) + self.assertIn( + access_token_entry, + self.cache.find(self.cache.CredentialType.ACCESS_TOKEN), + "find(..., query=None) should not crash, even though MSAL does not use it") self.assertEqual( { 'client_id': 'my_client_id', From 49a919827ca8c799e6019039d1af39eb42e69d14 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 17 Jan 2024 22:08:53 -0800 Subject: [PATCH 08/25] Attempts account removal from broker first --- msal/application.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index afa08f5..96277e4 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1123,12 +1123,13 @@ def _get_authority_aliases(self, instance): def remove_account(self, account): """Sign me out and forget me from token cache""" - self._forget_me(account) if self._enable_broker: from .broker import _signout_silently error = _signout_silently(self.client_id, account["local_account_id"]) if error: logger.debug("_signout_silently() returns error: %s", error) + # Broker sign-out has been attempted, even if the _forget_me() below throws. + self._forget_me(account) def _sign_out(self, home_account): # Remove all relevant RTs and ATs from token cache From c131b9b57770de03cb82a32c871cca84a9f1162f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 19 Jan 2024 13:14:10 -0800 Subject: [PATCH 09/25] Adding docs for PopAuthScheme --- docs/index.rst | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index e608fe6..2129e10 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -1,3 +1,4 @@ +========================= MSAL Python Documentation ========================= @@ -11,6 +12,8 @@ MSAL Python Documentation .. Comment: Perhaps because of the theme, only the first level sections will show in TOC, regardless of maxdepth setting. + UPDATE: And now (early 2024) suddenly a function-level, long TOC is generated, + even though maxdepth is set to 2. You can find high level conceptual documentations in the project `README `_. @@ -92,7 +95,7 @@ They are implemented as two separated classes, with different methods for different authentication scenarios. ClientApplication -================= +----------------- .. autoclass:: msal.ClientApplication :members: @@ -101,7 +104,7 @@ ClientApplication .. automethod:: __init__ PublicClientApplication -======================= +----------------------- .. autoclass:: msal.PublicClientApplication :members: @@ -109,14 +112,14 @@ PublicClientApplication .. automethod:: __init__ ConfidentialClientApplication -============================= +----------------------------- .. autoclass:: msal.ConfidentialClientApplication :members: TokenCache -========== +---------- One of the parameters accepted by both `PublicClientApplication` and `ConfidentialClientApplication` @@ -130,3 +133,18 @@ See `SerializableTokenCache` for example. .. autoclass:: msal.SerializableTokenCache :members: + + +PopAuthScheme +------------- + +This is used as the `auth_scheme` parameter in many of the acquire token methods +to support for Proof of Possession (PoP) tokens. + +New in MSAL Python 1.26 + +.. autoclass:: msal.PopAuthScheme + :members: + + .. automethod:: __init__ + From d7331f26c634240c3b1d1eeabeebc2b963c2597c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 22 Jan 2024 01:14:34 -0800 Subject: [PATCH 10/25] Tested with latest cryptography 42.x --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 75df4f9..d3b7e40 100644 --- a/setup.cfg +++ b/setup.cfg @@ -54,7 +54,7 @@ install_requires = # And we will use the cryptography (X+3).0.0 as the upper bound, # based on their latest deprecation policy # https://cryptography.io/en/latest/api-stability/#deprecation - cryptography>=0.6,<44 + cryptography>=0.6,<45 mock; python_version<'3.3' From 3e68838f3aaffb658bf61f1f403f09d79eb4dbce Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 21 Feb 2023 17:54:00 -0800 Subject: [PATCH 11/25] Mention instance_discovery instead of validate_authority in an error message --- msal/__main__.py | 26 ++++++++++++++++---------- msal/authority.py | 2 +- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/msal/__main__.py b/msal/__main__.py index aeb123b..79776b8 100644 --- a/msal/__main__.py +++ b/msal/__main__.py @@ -242,18 +242,24 @@ def _main(): enable_broker = _input_boolean("Enable broker? It will error out later if your app has not registered some redirect URI") enable_debug_log = _input_boolean("Enable MSAL Python's DEBUG log?") enable_pii_log = _input_boolean("Enable PII in broker's log?") if enable_broker and enable_debug_log else False + authority = _select_options([ + "https://login.microsoftonline.com/common", + "https://login.microsoftonline.com/organizations", + "https://login.microsoftonline.com/microsoft.onmicrosoft.com", + "https://login.microsoftonline.com/msidlab4.onmicrosoft.com", + "https://login.microsoftonline.com/consumers", + ], + header="Input authority (Note that MSA-PT apps would NOT use the /common authority)", + accept_nonempty_string=True, + ) + instance_discovery = _input_boolean( + "You input an unusual authority which might fail the Instance Discovery. " + "Now, do you want to perform Instance Discovery on your input authority?" + ) if not authority.startswith("https://login.microsoftonline.com") else None app = msal.PublicClientApplication( chosen_app["client_id"] if isinstance(chosen_app, dict) else chosen_app, - authority=_select_options([ - "https://login.microsoftonline.com/common", - "https://login.microsoftonline.com/organizations", - "https://login.microsoftonline.com/microsoft.onmicrosoft.com", - "https://login.microsoftonline.com/msidlab4.onmicrosoft.com", - "https://login.microsoftonline.com/consumers", - ], - header="Input authority (Note that MSA-PT apps would NOT use the /common authority)", - accept_nonempty_string=True, - ), + authority=authority, + instance_discovery=instance_discovery, enable_broker_on_windows=enable_broker, enable_pii_log=enable_pii_log, token_cache=global_cache, diff --git a/msal/authority.py b/msal/authority.py index ae3ebf7..923f3ff 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -100,7 +100,7 @@ def __init__( "The authority you provided, %s, is not whitelisted. " "If it is indeed your legit customized domain name, " "you can turn off this check by passing in " - "validate_authority=False" + "instance_discovery=False" % authority_url) tenant_discovery_endpoint = payload['tenant_discovery_endpoint'] else: From d52459563defadffdb5fa25060d7c98593abaa87 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sun, 28 Jan 2024 15:30:22 -0800 Subject: [PATCH 12/25] Tolerate ID token time errors --- oauth2cli/__init__.py | 2 +- oauth2cli/oidc.py | 80 ++++++++++++++++++++++++++++++++++++------- tests/test_oidc.py | 21 ++++++++++++ 3 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 tests/test_oidc.py diff --git a/oauth2cli/__init__.py b/oauth2cli/__init__.py index 60bf259..d997872 100644 --- a/oauth2cli/__init__.py +++ b/oauth2cli/__init__.py @@ -1,6 +1,6 @@ __version__ = "0.4.0" -from .oidc import Client +from .oidc import Client, IdTokenError from .assertion import JwtAssertionCreator from .assertion import JwtSigner # Obsolete. For backward compatibility. from .authcode import AuthCodeReceiver diff --git a/oauth2cli/oidc.py b/oauth2cli/oidc.py index d4d3a92..01ee789 100644 --- a/oauth2cli/oidc.py +++ b/oauth2cli/oidc.py @@ -5,9 +5,13 @@ import string import warnings import hashlib +import logging from . import oauth2 + +logger = logging.getLogger(__name__) + def decode_part(raw, encoding="utf-8"): """Decode a part of the JWT. @@ -32,6 +36,45 @@ def decode_part(raw, encoding="utf-8"): base64decode = decode_part # Obsolete. For backward compatibility only. +def _epoch_to_local(epoch): + return time.strftime("%Y-%m-%d %H:%M:%S", time.localtime(epoch)) + +class IdTokenError(RuntimeError): # We waised RuntimeError before, so keep it + """In unlikely event of an ID token is malformed, this exception will be raised.""" + def __init__(self, reason, now, claims): + super(IdTokenError, self).__init__( + "%s Current epoch = %s. The id_token was approximately: %s" % ( + reason, _epoch_to_local(now), json.dumps(dict( + claims, + iat=_epoch_to_local(claims["iat"]) if claims.get("iat") else None, + exp=_epoch_to_local(claims["exp"]) if claims.get("exp") else None, + ), indent=2))) + +class _IdTokenTimeError(IdTokenError): # This is not intended to be raised and caught + _SUGGESTION = "Make sure your computer's time and time zone are both correct." + def __init__(self, reason, now, claims): + super(_IdTokenTimeError, self).__init__(reason+ " " + self._SUGGESTION, now, claims) + def log(self): + # Influenced by JWT specs https://tools.ietf.org/html/rfc7519#section-4.1.5 + # and OIDC specs https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation + # We used to raise this error, but now we just log it as warning, because: + # 1. If it is caused by incorrect local machine time, + # then the token(s) are still correct and probably functioning, + # so, there is no point to error out. + # 2. If it is caused by incorrect IdP time, then it is IdP's fault, + # There is not much a client can do, so, we might as well return the token(s) + # and let downstream components to decide what to do. + logger.warning(str(self)) + +class IdTokenIssuerError(IdTokenError): + pass + +class IdTokenAudienceError(IdTokenError): + pass + +class IdTokenNonceError(IdTokenError): + pass + def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None): """Decodes and validates an id_token and returns its claims as a dictionary. @@ -41,41 +84,52 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) `maybe more `_ """ decoded = json.loads(decode_part(id_token.split('.')[1])) - err = None # https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation + # Based on https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation _now = int(now or time.time()) skew = 120 # 2 minutes - TIME_SUGGESTION = "Make sure your computer's time and time zone are both correct." + if _now + skew < decoded.get("nbf", _now - 1): # nbf is optional per JWT specs # This is not an ID token validation, but a JWT validation # https://tools.ietf.org/html/rfc7519#section-4.1.5 - err = "0. The ID token is not yet valid. " + TIME_SUGGESTION + _IdTokenTimeError("0. The ID token is not yet valid.", _now, decoded).log() + if issuer and issuer != decoded["iss"]: # https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse - err = ('2. The Issuer Identifier for the OpenID Provider, "%s", ' + raise IdTokenIssuerError( + '2. The Issuer Identifier for the OpenID Provider, "%s", ' "(which is typically obtained during Discovery), " - "MUST exactly match the value of the iss (issuer) Claim.") % issuer + "MUST exactly match the value of the iss (issuer) Claim." % issuer, + _now, + decoded) + if client_id: valid_aud = client_id in decoded["aud"] if isinstance( decoded["aud"], list) else client_id == decoded["aud"] if not valid_aud: - err = ( + raise IdTokenAudienceError( "3. The aud (audience) claim must contain this client's client_id " '"%s", case-sensitively. Was your client_id in wrong casing?' # Some IdP accepts wrong casing request but issues right casing IDT - ) % client_id + % client_id, + _now, + decoded) + # Per specs: # 6. If the ID Token is received via direct communication between # the Client and the Token Endpoint (which it is during _obtain_token()), # the TLS server validation MAY be used to validate the issuer # in place of checking the token signature. + if _now - skew > decoded["exp"]: - err = "9. The ID token already expires. " + TIME_SUGGESTION + _IdTokenTimeError("9. The ID token already expires.", _now, decoded).log() + if nonce and nonce != decoded.get("nonce"): - err = ("11. Nonce must be the same value " - "as the one that was sent in the Authentication Request.") - if err: - raise RuntimeError("%s Current epoch = %s. The id_token was: %s" % ( - err, _now, json.dumps(decoded, indent=2))) + raise IdTokenNonceError( + "11. Nonce must be the same value " + "as the one that was sent in the Authentication Request.", + _now, + decoded) + return decoded diff --git a/tests/test_oidc.py b/tests/test_oidc.py new file mode 100644 index 0000000..d6a929b --- /dev/null +++ b/tests/test_oidc.py @@ -0,0 +1,21 @@ +from tests import unittest + +import oauth2cli + + +class TestIdToken(unittest.TestCase): + EXPIRED_ID_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJpc3N1ZXIiLCJpYXQiOjE3MDY1NzA3MzIsImV4cCI6MTY3NDk0ODMzMiwiYXVkIjoiZm9vIiwic3ViIjoic3ViamVjdCJ9.wyWNFxnE35SMP6FpxnWZmWQAy4KD0No_Q1rUy5bNnLs" + + def test_id_token_should_tolerate_time_error(self): + self.assertEqual(oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN), { + "iss": "issuer", + "iat": 1706570732, + "exp": 1674948332, # 2023-1-28 + "aud": "foo", + "sub": "subject", + }, "id_token is decoded correctly, without raising exception") + + def test_id_token_should_error_out_on_client_id_error(self): + with self.assertRaises(oauth2cli.IdTokenError): + oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN, client_id="not foo") + From 386ea2e02a533373ab2d557da6d5aa55a748d7d3 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 25 Jan 2024 23:48:09 -0800 Subject: [PATCH 13/25] Tolerate ID token time errors --- docs/index.rst | 9 +++++++++ msal/__init__.py | 3 +-- tests/test_oidc.py | 5 +++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index 2129e10..11dd9b0 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -148,3 +148,12 @@ New in MSAL Python 1.26 .. automethod:: __init__ + +Exceptions +---------- +These are exceptions that MSAL Python may raise. +You should not need to create them directly. +You may want to catch them to provide a better error message to your end users. + +.. autoclass:: msal.IdTokenError + diff --git a/msal/__init__.py b/msal/__init__.py index 09b7a50..5c5292f 100644 --- a/msal/__init__.py +++ b/msal/__init__.py @@ -31,7 +31,6 @@ ConfidentialClientApplication, PublicClientApplication, ) -from .oauth2cli.oidc import Prompt +from .oauth2cli.oidc import Prompt, IdTokenError from .token_cache import TokenCache, SerializableTokenCache from .auth_scheme import PopAuthScheme - diff --git a/tests/test_oidc.py b/tests/test_oidc.py index d6a929b..297dfeb 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -1,6 +1,7 @@ from tests import unittest -import oauth2cli +import msal +from msal import oauth2cli class TestIdToken(unittest.TestCase): @@ -16,6 +17,6 @@ def test_id_token_should_tolerate_time_error(self): }, "id_token is decoded correctly, without raising exception") def test_id_token_should_error_out_on_client_id_error(self): - with self.assertRaises(oauth2cli.IdTokenError): + with self.assertRaises(msal.IdTokenError): oauth2cli.oidc.decode_id_token(self.EXPIRED_ID_TOKEN, client_id="not foo") From 1a19c4b4ffa44f951fa63e84ded49f6558a35512 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 15 May 2023 12:43:56 -0700 Subject: [PATCH 14/25] Provide examples for B2C and CIAM --- msal/authority.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/msal/authority.py b/msal/authority.py index 923f3ff..5e0131f 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -120,6 +120,8 @@ def __init__( "Unable to get authority configuration for {}. " "Authority would typically be in a format of " "https://login.microsoftonline.com/your_tenant " + "or https://tenant_name.ciamlogin.com " + "or https://tenant_name.b2clogin.com/tenant.onmicrosoft.com/policy. " "Also please double check your tenant name or GUID is correct.".format( authority_url)) logger.debug("openid_config = %s", openid_config) From 5d9b2211a5bd9e18e05b6ce77f139bac9b7c17da Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 2 Feb 2024 16:39:36 -0800 Subject: [PATCH 15/25] Give a hint on where the client_id came from --- sample/confidential_client_certificate_sample.py | 2 +- sample/confidential_client_secret_sample.py | 2 +- sample/device_flow_sample.py | 2 +- sample/interactive_sample.py | 2 +- sample/migrate_rt.py | 2 +- sample/username_password_sample.py | 2 +- sample/vault_jwt_sample.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sample/confidential_client_certificate_sample.py b/sample/confidential_client_certificate_sample.py index 93c72ee..c8c5f3c 100644 --- a/sample/confidential_client_certificate_sample.py +++ b/sample/confidential_client_certificate_sample.py @@ -3,7 +3,7 @@ { "authority": "https://login.microsoftonline.com/Enter_the_Tenant_Name_Here", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "scope": ["https://graph.microsoft.com/.default"], // Specific to Client Credentials Grant i.e. acquire_token_for_client(), // you don't specify, in the code, the individual scopes you want to access. diff --git a/sample/confidential_client_secret_sample.py b/sample/confidential_client_secret_sample.py index 9c616d5..48948ff 100644 --- a/sample/confidential_client_secret_sample.py +++ b/sample/confidential_client_secret_sample.py @@ -3,7 +3,7 @@ { "authority": "https://login.microsoftonline.com/Enter_the_Tenant_Name_Here", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "scope": ["https://graph.microsoft.com/.default"], // Specific to Client Credentials Grant i.e. acquire_token_for_client(), // you don't specify, in the code, the individual scopes you want to access. diff --git a/sample/device_flow_sample.py b/sample/device_flow_sample.py index 89dccd1..816bbb1 100644 --- a/sample/device_flow_sample.py +++ b/sample/device_flow_sample.py @@ -3,7 +3,7 @@ { "authority": "https://login.microsoftonline.com/common", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "scope": ["User.ReadBasic.All"], // You can find the other permission names from this document // https://docs.microsoft.com/en-us/graph/permissions-reference diff --git a/sample/interactive_sample.py b/sample/interactive_sample.py index f283ed2..f4feb6e 100644 --- a/sample/interactive_sample.py +++ b/sample/interactive_sample.py @@ -6,7 +6,7 @@ { "authority": "https://login.microsoftonline.com/organizations", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "scope": ["User.ReadBasic.All"], // You can find the other permission names from this document // https://docs.microsoft.com/en-us/graph/permissions-reference diff --git a/sample/migrate_rt.py b/sample/migrate_rt.py index ed0011e..e854866 100644 --- a/sample/migrate_rt.py +++ b/sample/migrate_rt.py @@ -3,7 +3,7 @@ { "authority": "https://login.microsoftonline.com/organizations", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "scope": ["User.ReadBasic.All"], // You can find the other permission names from this document // https://docs.microsoft.com/en-us/graph/permissions-reference diff --git a/sample/username_password_sample.py b/sample/username_password_sample.py index a25407d..25e49ff 100644 --- a/sample/username_password_sample.py +++ b/sample/username_password_sample.py @@ -3,7 +3,7 @@ { "authority": "https://login.microsoftonline.com/organizations", - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "username": "your_username@your_tenant.com", "password": "This is a sample only. You better NOT persist your password.", "scope": ["User.ReadBasic.All"], diff --git a/sample/vault_jwt_sample.py b/sample/vault_jwt_sample.py index 9410039..e2448fc 100644 --- a/sample/vault_jwt_sample.py +++ b/sample/vault_jwt_sample.py @@ -3,7 +3,7 @@ { "tenant": "your_tenant_name", // Your target tenant, DNS name - "client_id": "your_client_id", + "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", // Target app ID in Azure AD "scope": ["https://graph.microsoft.com/.default"], // Specific to Client Credentials Grant i.e. acquire_token_for_client(), From 4b34dd6ea0b5a511a5fbc76dfefa9cd9fff7e004 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 1 Feb 2024 12:12:46 -0800 Subject: [PATCH 16/25] Allow github action to write perf result into repo This is needed because our org has transitioned to a read-only GITHUB_TOKEN for GitHub Action workflows. This change fixes #653 --- .github/workflows/python-package.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 461eb95..0ef8fe3 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -56,6 +56,8 @@ jobs: # and then run benchmark only once (sampling with only one Python version). needs: ci runs-on: ubuntu-latest + permissions: + contents: write steps: - uses: actions/checkout@v4 - name: Set up Python 3.9 From b28654038fbaf684bb633b456db618a32372e1f7 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 27 Feb 2023 11:25:39 -0800 Subject: [PATCH 17/25] Adding attributes that were not auto documented --- docs/conf.py | 4 ++-- docs/index.rst | 47 ++++++++++++++++++++++++++++----------------- msal/application.py | 27 +++++++++++++++----------- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 024451d..f9762ca 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -62,7 +62,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = None +language = "en" # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. @@ -95,7 +95,7 @@ # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". -html_static_path = ['_static'] +#html_static_path = ['_static'] # Custom sidebar templates, must be a dictionary that maps document names # to template names. diff --git a/docs/index.rst b/docs/index.rst index 11dd9b0..15ee4a0 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -7,8 +7,6 @@ MSAL Python Documentation :caption: Contents: :hidden: - index - .. Comment: Perhaps because of the theme, only the first level sections will show in TOC, regardless of maxdepth setting. @@ -26,7 +24,7 @@ MSAL Python supports some of them. **The following diagram serves as a map. Locate your application scenario on the map.** **If the corresponding icon is clickable, it will bring you to an MSAL Python sample for that scenario.** -* Most authentication scenarios acquire tokens on behalf of signed-in users. +* Most authentication scenarios acquire tokens representing the signed-in user. .. raw:: html @@ -46,7 +44,7 @@ MSAL Python supports some of them. alt="Browserless app" title="Browserless app" href="https://github.com/Azure-Samples/ms-identity-python-devicecodeflow"> -* There are also daemon apps. In these scenarios, applications acquire tokens on behalf of themselves with no user. +* There are also daemon apps, who acquire tokens representing themselves, not a user. .. raw:: html @@ -66,26 +64,24 @@ MSAL Python supports some of them. API Reference ============= +.. note:: + + Only the contents inside + `this source file `_ + and their documented methods (unless otherwise marked as deprecated) + are MSAL Python public API, + which are guaranteed to be backward-compatible until the next major version. + + Everything else, regardless of their naming, are all internal helpers, + which could change at anytime in the future, without prior notice. 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**: +The API Reference is like a dictionary, which is useful when: * You already followed our sample(s) above and have your app up and running, but want to know more on how you could tweak the authentication experience by using other optional parameters (there are plenty of them!) -* You read the MSAL Python source code and found a helper function that is useful to you, - then you would want to double check whether that helper is documented below. - Only documented APIs are considered part of the MSAL Python public API, - which are guaranteed to be backward-compatible in MSAL Python 1.x series. - Undocumented internal helpers are subject to change anytime, without prior notice. - -.. note:: - - Only APIs and their parameters documented in this section are part of public API, - with guaranteed backward compatibility for the entire 1.x series. - - Other modules in the source code are all considered as internal helpers, - which could change at anytime in the future, without prior notice. +* Some important features have their in-depth documentations in the API Reference. MSAL proposes a clean separation between `public client applications and confidential client applications @@ -109,6 +105,7 @@ PublicClientApplication .. autoclass:: msal.PublicClientApplication :members: + .. autoattribute:: msal.PublicClientApplication.CONSOLE_WINDOW_HANDLE .. automethod:: __init__ ConfidentialClientApplication @@ -134,6 +131,15 @@ See `SerializableTokenCache` for example. .. autoclass:: msal.SerializableTokenCache :members: +Prompt +------ +.. autoclass:: msal.Prompt + :members: + + .. autoattribute:: msal.Prompt.SELECT_ACCOUNT + .. autoattribute:: msal.Prompt.NONE + .. autoattribute:: msal.Prompt.CONSENT + .. autoattribute:: msal.Prompt.LOGIN PopAuthScheme ------------- @@ -146,6 +152,11 @@ New in MSAL Python 1.26 .. autoclass:: msal.PopAuthScheme :members: + .. autoattribute:: msal.PopAuthScheme.HTTP_GET + .. autoattribute:: msal.PopAuthScheme.HTTP_POST + .. autoattribute:: msal.PopAuthScheme.HTTP_PUT + .. autoattribute:: msal.PopAuthScheme.HTTP_DELETE + .. autoattribute:: msal.PopAuthScheme.HTTP_PATCH .. automethod:: __init__ diff --git a/msal/application.py b/msal/application.py index 96277e4..f82ea2e 100644 --- a/msal/application.py +++ b/msal/application.py @@ -737,10 +737,11 @@ def initiate_auth_code_flow( maintain state between the request and callback. If absent, this library will automatically generate one internally. :param str prompt: - By default, no prompt value will be sent, not even "none". + By default, no prompt value will be sent, not even string ``"none"``. 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 + Its valid values are the constants defined in + :class:`Prompt `. + :param str login_hint: Optional. Identifier of the user. Generally a User Principal Name (UPN). :param domain_hint: @@ -840,10 +841,10 @@ def get_authorization_request_url( `not recommended `_. :param str prompt: - By default, no prompt value will be sent, not even "none". + By default, no prompt value will be sent, not even string ``"none"``. 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 + Its valid values are the constants defined in + :class:`Prompt `. :param nonce: A cryptographically random value used to mitigate replay attacks. See also `OIDC specs `_. @@ -1819,10 +1820,10 @@ def acquire_token_interactive( :param list scopes: It is a list of case-sensitive strings. :param str prompt: - By default, no prompt value will be sent, not even "none". + By default, no prompt value will be sent, not even string ``"none"``. 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 + Its valid values are the constants defined in + :class:`Prompt `. :param str login_hint: Optional. Identifier of the user. Generally a User Principal Name (UPN). :param domain_hint: @@ -1867,11 +1868,15 @@ def acquire_token_interactive( New in version 1.15. :param int parent_window_handle: - OPTIONAL. If your app is a GUI app running on modern Windows system, - and your app opts in to use broker, + Required if your app is running on Windows and opted in to use broker. + + If your app is a GUI app, you are recommended to also provide its window handle, so that the sign in UI window will properly pop up on top of your window. + If your app is a console app (most Python scripts are console apps), + you can use a placeholder value ``msal.PublicClientApplication.CONSOLE_WINDOW_HANDLE``. + New in version 1.20.0. :param function on_before_launching_ui: From 3b96de605ff4898aaf1f19c919c9574616b3dbed Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 7 Feb 2024 13:11:14 -0800 Subject: [PATCH 18/25] Implement remove_tokens_for_client() --- msal/application.py | 13 +++++++++++++ tests/test_application.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/msal/application.py b/msal/application.py index f82ea2e..0f86be5 100644 --- a/msal/application.py +++ b/msal/application.py @@ -2178,6 +2178,19 @@ def _acquire_token_for_client( telemetry_context.update_telemetry(response) return response + def remove_tokens_for_client(self): + """Remove all tokens that were previously acquired via + :func:`~acquire_token_for_client()` for the current client.""" + for env in [self.authority.instance] + self._get_authority_aliases( + self.authority.instance): + for at in self.token_cache.find(TokenCache.CredentialType.ACCESS_TOKEN, query={ + "client_id": self.client_id, + "environment": env, + "home_account_id": None, # These are mostly app-only tokens + }): + self.token_cache.remove_at(at) + # acquire_token_for_client() obtains no RTs, so we have no RT to remove + def acquire_token_on_behalf_of(self, user_assertion, scopes, claims_challenge=None, **kwargs): """Acquires token using on-behalf-of (OBO) flow. diff --git a/tests/test_application.py b/tests/test_application.py index fc529f0..cebc722 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -662,6 +662,35 @@ def test_organizations_authority_should_emit_warning(self): authority="https://login.microsoftonline.com/organizations") +class TestRemoveTokensForClient(unittest.TestCase): + def test_remove_tokens_for_client_should_remove_client_tokens_only(self): + at_for_user = "AT for user" + cca = msal.ConfidentialClientApplication( + "client_id", client_credential="secret", + authority="https://login.microsoftonline.com/microsoft.onmicrosoft.com") + self.assertEqual( + 0, len(cca.token_cache.find(msal.TokenCache.CredentialType.ACCESS_TOKEN))) + cca.acquire_token_for_client( + ["scope"], + post=lambda url, **kwargs: MinimalResponse( + status_code=200, text=json.dumps({"access_token": "AT for client"}))) + self.assertEqual( + 1, len(cca.token_cache.find(msal.TokenCache.CredentialType.ACCESS_TOKEN))) + cca.acquire_token_by_username_password( + "johndoe", "password", ["scope"], + post=lambda url, **kwargs: MinimalResponse( + status_code=200, text=json.dumps(build_response( + access_token=at_for_user, expires_in=3600, + uid="uid", utid="utid", # This populates home_account_id + )))) + self.assertEqual( + 2, len(cca.token_cache.find(msal.TokenCache.CredentialType.ACCESS_TOKEN))) + cca.remove_tokens_for_client() + remaining_tokens = cca.token_cache.find(msal.TokenCache.CredentialType.ACCESS_TOKEN) + self.assertEqual(1, len(remaining_tokens)) + self.assertEqual(at_for_user, remaining_tokens[0].get("secret")) + + class TestScopeDecoration(unittest.TestCase): def _test_client_id_should_be_a_valid_scope(self, client_id, other_scopes): # B2C needs this https://learn.microsoft.com/en-us/azure/active-directory-b2c/access-tokens#openid-connect-scopes From bb0e24a2e13571d204799820d18089c0974e3709 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 10 Mar 2023 12:14:51 -0800 Subject: [PATCH 19/25] Remove premature int(...) --- msal/throttled_http_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal/throttled_http_client.py b/msal/throttled_http_client.py index 378cd3d..1e285ff 100644 --- a/msal/throttled_http_client.py +++ b/msal/throttled_http_client.py @@ -30,7 +30,7 @@ def _parse_http_429_5xx_retry_after(result=None, **ignored): return 0 # Quick exit default = 60 # Recommended at the end of # https://identitydivision.visualstudio.com/devex/_git/AuthLibrariesApiReview?version=GBdev&path=%2FService%20protection%2FIntial%20set%20of%20protection%20measures.md&_a=preview - retry_after = int(lowercase_headers.get("retry-after", default)) + retry_after = lowercase_headers.get("retry-after", default) try: # AAD's retry_after uses integer format only # https://stackoverflow.microsoft.com/questions/264931/264932 From 0d8b2c201f3d09efd61f23202d051c9433ae0e9d Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 5 Jun 2023 12:22:47 -0700 Subject: [PATCH 20/25] MSAL's fallback-from-broker behavior remains a FAQ --- msal/application.py | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/msal/application.py b/msal/application.py index 0f86be5..464714d 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1747,7 +1747,7 @@ def __init__(self, client_id, client_credential=None, **kwargs): You may set enable_broker_on_windows to True. - What is a broker, and why use it? + **What is a broker, and why use it?** A broker is a component installed on your device. Broker implicitly gives your device an identity. By using a broker, @@ -1764,10 +1764,7 @@ def __init__(self, client_id, client_credential=None, **kwargs): so that your broker-enabled apps (even a CLI) could automatically SSO from a previously established signed-in session. - ADFS and B2C do not support broker. - MSAL will automatically fallback to use browser. - - You shall only enable broker when your app: + **You shall only enable broker when your app:** 1. is running on supported platforms, and already registered their corresponding redirect_uri @@ -1780,6 +1777,29 @@ def __init__(self, client_id, client_credential=None, **kwargs): 3. tested with ``acquire_token_interactive()`` and ``acquire_token_silent()``. + **The fallback behaviors of MSAL Python's broker support** + + MSAL will either error out, or silently fallback to non-broker flows. + + 1. MSAL will ignore the `enable_broker_...` and bypass broker + on those auth flows that are known to be NOT supported by broker. + This includes ADFS, B2C, etc.. + For other "could-use-broker" scenarios, please see below. + 2. MSAL errors out when app developer opted-in to use broker + but a direct dependency "mid-tier" package is not installed. + Error message guides app developer to declare the correct dependency + ``msal[broker]``. + We error out here because the error is actionable to app developers. + 3. MSAL silently "deactivates" the broker and fallback to non-broker, + when opted-in, dependency installed yet failed to initialize. + We anticipate this would happen on a device whose OS is too old + or the underlying broker component is somehow unavailable. + There is not much an app developer or the end user can do here. + Eventually, the conditional access policy shall + force the user to switch to a different device. + 4. MSAL errors out when broker is opted in, installed, initialized, + but subsequent token request(s) failed. + :param boolean enable_broker_on_windows: This setting is only effective if your app is running on Windows 10+. This parameter defaults to None, which means MSAL will not utilize a broker. From bf87155e158c360a9047205e5fbe7717345cdf94 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 31 Oct 2023 12:30:35 -0700 Subject: [PATCH 21/25] Change back to use print(result) in error path --- sample/confidential_client_certificate_sample.py | 2 +- sample/confidential_client_secret_sample.py | 2 +- sample/device_flow_sample.py | 2 +- sample/interactive_sample.py | 2 +- sample/username_password_sample.py | 11 ++++++----- sample/vault_jwt_sample.py | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/sample/confidential_client_certificate_sample.py b/sample/confidential_client_certificate_sample.py index c8c5f3c..2c4118a 100644 --- a/sample/confidential_client_certificate_sample.py +++ b/sample/confidential_client_certificate_sample.py @@ -70,7 +70,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},).json() print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error while True: # Here we mimic a long-lived daemon diff --git a/sample/confidential_client_secret_sample.py b/sample/confidential_client_secret_sample.py index 48948ff..9ff6a81 100644 --- a/sample/confidential_client_secret_sample.py +++ b/sample/confidential_client_secret_sample.py @@ -69,7 +69,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},).json() print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error while True: # Here we mimic a long-lived daemon diff --git a/sample/device_flow_sample.py b/sample/device_flow_sample.py index 816bbb1..7d99847 100644 --- a/sample/device_flow_sample.py +++ b/sample/device_flow_sample.py @@ -91,7 +91,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},).json() print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error while True: # Here we mimic a long-lived daemon diff --git a/sample/interactive_sample.py b/sample/interactive_sample.py index f4feb6e..58d8c9a 100644 --- a/sample/interactive_sample.py +++ b/sample/interactive_sample.py @@ -86,7 +86,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},) print("Graph API call result: %s ..." % graph_response.text[:100]) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error while True: # Here we mimic a long-lived daemon diff --git a/sample/username_password_sample.py b/sample/username_password_sample.py index 25e49ff..3578e5e 100644 --- a/sample/username_password_sample.py +++ b/sample/username_password_sample.py @@ -5,7 +5,6 @@ "authority": "https://login.microsoftonline.com/organizations", "client_id": "your_client_id came from https://learn.microsoft.com/entra/identity-platform/quickstart-register-app", "username": "your_username@your_tenant.com", - "password": "This is a sample only. You better NOT persist your password.", "scope": ["User.ReadBasic.All"], // You can find the other permission names from this document // https://docs.microsoft.com/en-us/graph/permissions-reference @@ -20,6 +19,7 @@ """ import sys # For simplicity, we'll read config file from 1st CLI param sys.argv[1] +import getpass import json import logging import time @@ -33,6 +33,7 @@ # logging.getLogger("msal").setLevel(logging.INFO) # Optionally disable MSAL DEBUG logs config = json.load(open(sys.argv[1])) +config["password"] = getpass.getpass() # If for whatever reason you plan to recreate same ClientApplication periodically, # you shall create one global token cache and reuse it by each ClientApplication @@ -40,9 +41,10 @@ # See more options in https://msal-python.readthedocs.io/en/latest/#tokencache # Create a preferably long-lived app instance, to avoid the overhead of app creation -global_app = msal.PublicClientApplication( - config["client_id"], authority=config["authority"], +global_app = msal.ClientApplication( + config["client_id"], client_credential=config.get("client_secret"), + authority=config["authority"], token_cache=global_token_cache, # Let this app (re)use an existing token cache. # If absent, ClientApplication will create its own empty token cache ) @@ -73,8 +75,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},).json() print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error - print(result) + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error if 65001 in result.get("error_codes", []): # Not mean to be coded programatically, but... raise RuntimeError( "AAD requires user consent for U/P flow to succeed. " diff --git a/sample/vault_jwt_sample.py b/sample/vault_jwt_sample.py index e2448fc..b9c5191 100644 --- a/sample/vault_jwt_sample.py +++ b/sample/vault_jwt_sample.py @@ -132,7 +132,7 @@ def acquire_and_use_token(): headers={'Authorization': 'Bearer ' + result['access_token']},).json() print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) else: - print("Token acquisition failed") # Examine result["error_description"] etc. to diagnose error + print("Token acquisition failed", result) # Examine result["error_description"] etc. to diagnose error while True: # Here we mimic a long-lived daemon From 4f0e03d5141d3fe405bc6ad52a2db08ed8287951 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 8 Feb 2024 14:49:15 -0800 Subject: [PATCH 22/25] CCA can be tested by: python -m msal --- msal/__main__.py | 74 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/msal/__main__.py b/msal/__main__.py index 79776b8..1c09f86 100644 --- a/msal/__main__.py +++ b/msal/__main__.py @@ -60,6 +60,17 @@ def _select_options( if raw_data and accept_nonempty_string: return raw_data +enable_debug_log = _input_boolean("Enable MSAL Python's DEBUG log?") +logging.basicConfig(level=logging.DEBUG if enable_debug_log else logging.INFO) +try: + from dotenv import load_dotenv + load_dotenv() + logging.info("Loaded environment variables from .env file") +except ImportError: + logging.warning( + "python-dotenv is not installed. " + "You may need to set environment variables manually.") + def _input_scopes(): scopes = _select_options([ "https://graph.microsoft.com/.default", @@ -100,6 +111,7 @@ def _acquire_token_silent(app): def _acquire_token_interactive(app, scopes=None, data=None): """acquire_token_interactive() - User will be prompted if app opts to do select_account.""" + assert isinstance(app, msal.PublicClientApplication) scopes = scopes or _input_scopes() # Let user input scope param before less important prompt and login_hint prompt = _select_options([ {"value": None, "description": "Unspecified. Proceed silently with a default account (if any), fallback to prompt."}, @@ -143,6 +155,7 @@ def _acquire_token_by_username_password(app): def _acquire_token_by_device_flow(app): """acquire_token_by_device_flow() - Note that this one does not go through broker""" + assert isinstance(app, msal.PublicClientApplication) flow = app.initiate_device_flow(scopes=_input_scopes()) print(flow["message"]) sys.stdout.flush() # Some terminal needs this to ensure the message is shown @@ -156,6 +169,7 @@ def _acquire_token_by_device_flow(app): def _acquire_ssh_cert_silently(app): """Acquire an SSH Cert silently- This typically only works with Azure CLI""" + assert isinstance(app, msal.PublicClientApplication) account = _select_account(app) if account: result = app.acquire_token_silent( @@ -170,6 +184,7 @@ def _acquire_ssh_cert_silently(app): def _acquire_ssh_cert_interactive(app): """Acquire an SSH Cert interactively - This typically only works with Azure CLI""" + assert isinstance(app, msal.PublicClientApplication) result = _acquire_token_interactive(app, scopes=_SSH_CERT_SCOPE, data=_SSH_CERT_DATA) if result.get("token_type") != "ssh-cert": logging.error("Unable to acquire an ssh-cert") @@ -185,6 +200,7 @@ def _acquire_ssh_cert_interactive(app): def _acquire_pop_token_interactive(app): """Acquire a POP token interactively - This typically only works with Azure CLI""" + assert isinstance(app, msal.PublicClientApplication) POP_SCOPE = ['6256c85f-0aad-4d50-b960-e6e9b21efe35/.default'] # KAP 1P Server App Scope, obtained from https://github.com/Azure/azure-cli-extensions/pull/4468/files#diff-a47efa3186c7eb4f1176e07d0b858ead0bf4a58bfd51e448ee3607a5b4ef47f6R116 result = _acquire_token_interactive(app, scopes=POP_SCOPE, data=_POP_DATA) print_json(result) @@ -198,6 +214,16 @@ def _remove_account(app): app.remove_account(account) print('Account "{}" and/or its token(s) are signed out from MSAL Python'.format(account["username"])) +def _acquire_token_for_client(app): + """CCA.acquire_token_for_client() - Rerun this will get same token from cache.""" + assert isinstance(app, msal.ConfidentialClientApplication) + print_json(app.acquire_token_for_client(scopes=_input_scopes())) + +def _remove_tokens_for_client(app): + """CCA.remove_tokens_for_client() - Run this to evict tokens from cache.""" + assert isinstance(app, msal.ConfidentialClientApplication) + app.remove_tokens_for_client() + def _exit(app): """Exit""" bug_link = ( @@ -235,12 +261,23 @@ def _main(): {"client_id": _AZURE_CLI, "name": "Azure CLI (Correctly configured for MSA-PT)"}, {"client_id": _VISUAL_STUDIO, "name": "Visual Studio (Correctly configured for MSA-PT)"}, {"client_id": "95de633a-083e-42f5-b444-a4295d8e9314", "name": "Whiteboard Services (Non MSA-PT app. Accepts AAD & MSA accounts.)"}, + { + "client_id": os.getenv("CLIENT_ID"), + "client_secret": os.getenv("CLIENT_SECRET"), + "name": "A confidential client app (CCA) whose settings are defined " + "in environment variables CLIENT_ID and CLIENT_SECRET", + }, ], option_renderer=lambda a: a["name"], - header="Impersonate this app (or you can type in the client_id of your own app)", + header="Impersonate this app " + "(or you can type in the client_id of your own public client app)", accept_nonempty_string=True) - enable_broker = _input_boolean("Enable broker? It will error out later if your app has not registered some redirect URI") - enable_debug_log = _input_boolean("Enable MSAL Python's DEBUG log?") + is_cca = isinstance(chosen_app, dict) and "client_secret" in chosen_app + if is_cca and not (chosen_app["client_id"] and chosen_app["client_secret"]): + raise ValueError("You need to set environment variables CLIENT_ID and CLIENT_SECRET") + enable_broker = (not is_cca) and _input_boolean("Enable broker? " + "(It will error out later if your app has not registered some redirect URI)" + ) enable_pii_log = _input_boolean("Enable PII in broker's log?") if enable_broker and enable_debug_log else False authority = _select_options([ "https://login.microsoftonline.com/common", @@ -255,7 +292,8 @@ def _main(): instance_discovery = _input_boolean( "You input an unusual authority which might fail the Instance Discovery. " "Now, do you want to perform Instance Discovery on your input authority?" - ) if not authority.startswith("https://login.microsoftonline.com") else None + ) if authority and not authority.startswith( + "https://login.microsoftonline.com") else None app = msal.PublicClientApplication( chosen_app["client_id"] if isinstance(chosen_app, dict) else chosen_app, authority=authority, @@ -263,21 +301,35 @@ def _main(): enable_broker_on_windows=enable_broker, enable_pii_log=enable_pii_log, token_cache=global_cache, + ) if not is_cca else msal.ConfidentialClientApplication( + chosen_app["client_id"], + client_credential=chosen_app["client_secret"], + authority=authority, + instance_discovery=instance_discovery, + enable_pii_log=enable_pii_log, + token_cache=global_cache, ) - if enable_debug_log: - logging.basicConfig(level=logging.DEBUG) - while True: - func = _select_options([ + methods_to_be_tested = [ _acquire_token_silent, + ] + ([ _acquire_token_interactive, - _acquire_token_by_username_password, _acquire_token_by_device_flow, _acquire_ssh_cert_silently, _acquire_ssh_cert_interactive, _acquire_pop_token_interactive, + ] if isinstance(app, msal.PublicClientApplication) else [] + ) + [ + _acquire_token_by_username_password, _remove_account, - _exit, - ], option_renderer=lambda f: f.__doc__, header="MSAL Python APIs:") + ] + ([ + _acquire_token_for_client, + _remove_tokens_for_client, + ] if isinstance(app, msal.ConfidentialClientApplication) else [] + ) + while True: + func = _select_options( + methods_to_be_tested + [_exit], + option_renderer=lambda f: f.__doc__, header="MSAL Python APIs:") try: func(app) except ValueError as e: From 59c3000192e92a49f483045071b97aa79929d19f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 8 Feb 2024 21:31:13 -0800 Subject: [PATCH 23/25] Pick up latest PyMsalRuntime 0.14.x --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index d3b7e40..2177d2c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -65,7 +65,7 @@ broker = # most existing MSAL Python apps do not have the redirect_uri needed by broker. # MSAL Python uses a subset of API from PyMsalRuntime 0.13.0+, # but we still bump the lower bound to 0.13.2+ for its important bugfix (https://github.com/AzureAD/microsoft-authentication-library-for-cpp/pull/3244) - pymsalruntime>=0.13.2,<0.14; python_version>='3.6' and platform_system=='Windows' + pymsalruntime>=0.13.2,<0.15; python_version>='3.6' and platform_system=='Windows' [options.packages.find] exclude = From 9a866ca6c2c960c3412ba613cfb89033bbfa7ca0 Mon Sep 17 00:00:00 2001 From: Ed Singleton Date: Thu, 22 Feb 2024 17:35:54 +0000 Subject: [PATCH 24/25] Don't use bare except when importing (#667) Using a bare except statement when importing hides other errors, which then get lost when the next import fails. Co-authored-by: Ed Singleton --- msal/mex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal/mex.py b/msal/mex.py index edecba3..e6f3ed0 100644 --- a/msal/mex.py +++ b/msal/mex.py @@ -27,7 +27,7 @@ try: from urllib.parse import urlparse -except: +except ImportError: from urlparse import urlparse try: from xml.etree import cElementTree as ET From 7e045199798adbe5309034de12c64b1816d23489 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 21 Dec 2023 11:55:15 -0800 Subject: [PATCH 25/25] Releasing 1.27 --- msal/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index 464714d..7b08c5e 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.26.0" # When releasing, also check and bump our dependencies's versions if needed +__version__ = "1.27.0" # When releasing, also check and bump our dependencies's versions if needed logger = logging.getLogger(__name__) _AUTHORITY_TYPE_CLOUDSHELL = "CLOUDSHELL"