From 640d88a64a2a194f90ee6dd2621bc3bb3be263b9 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 2 Nov 2021 13:12:51 -0700 Subject: [PATCH 01/15] Bubble up refresh exception when we cannot recover --- msal/application.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/msal/application.py b/msal/application.py index 13015d09..05b77fc3 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1207,7 +1207,9 @@ def _acquire_token_silent_from_cache_and_possibly_refresh_it( if (result and "error" not in result) or (not access_token_from_cache): return result except: # The exact HTTP exception is transportation-layer dependent - logger.exception("Refresh token failed") # Potential AAD outage? + # 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 return access_token_from_cache def _acquire_token_silent_by_finding_rt_belongs_to_me_or_my_family( From 663dd32d77671100375e855f04e35ee59ee24f41 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 4 Nov 2021 23:56:11 -0700 Subject: [PATCH 02/15] Lazy initialization makes partial test faster --- tests/test_application.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/test_application.py b/tests/test_application.py index 5a92c8d4..518042a8 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -331,7 +331,10 @@ class TestApplicationForRefreshInBehaviors(unittest.TestCase): account = {"home_account_id": "{}.{}".format(uid, utid)} rt = "this is a rt" client_id = "my_app" - app = ClientApplication(client_id, authority=authority_url) + + @classmethod + def setUpClass(cls): # Initialization at runtime, not interpret-time + cls.app = ClientApplication(cls.client_id, authority=cls.authority_url) def setUp(self): self.app.token_cache = self.cache = msal.SerializableTokenCache() @@ -485,8 +488,10 @@ def mock_post(url, headers=None, *args, **kwargs): class TestTelemetryOnClientApplication(unittest.TestCase): - app = ClientApplication( - "client_id", authority="https://login.microsoftonline.com/common") + @classmethod + def setUpClass(cls): # Initialization at runtime, not interpret-time + cls.app = ClientApplication( + "client_id", authority="https://login.microsoftonline.com/common") def test_acquire_token_by_auth_code_flow(self): at = "this is an access token" @@ -509,8 +514,10 @@ def mock_post(url, headers=None, *args, **kwargs): class TestTelemetryOnPublicClientApplication(unittest.TestCase): - app = PublicClientApplication( - "client_id", authority="https://login.microsoftonline.com/common") + @classmethod + def setUpClass(cls): # Initialization at runtime, not interpret-time + cls.app = PublicClientApplication( + "client_id", authority="https://login.microsoftonline.com/common") # For now, acquire_token_interactive() is verified by code review. @@ -534,9 +541,11 @@ def mock_post(url, headers=None, *args, **kwargs): class TestTelemetryOnConfidentialClientApplication(unittest.TestCase): - app = ConfidentialClientApplication( - "client_id", client_credential="secret", - authority="https://login.microsoftonline.com/common") + @classmethod + def setUpClass(cls): # Initialization at runtime, not interpret-time + cls.app = ConfidentialClientApplication( + "client_id", client_credential="secret", + authority="https://login.microsoftonline.com/common") def test_acquire_token_for_client(self): at = "this is an access token" From 5ef747f5c66a39478ea200d640d019d4d20f7c3b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 1 Nov 2021 21:10:08 -0700 Subject: [PATCH 03/15] Add cloud instances string constants Reduce duplicated magic strings Add test cases Writing docs --- msal/application.py | 19 +++++++++++++++++-- msal/authority.py | 26 ++++++++++++++++++++++++-- tests/http_client.py | 3 +++ tests/test_authority.py | 35 ++++++++++++++++++++++++++++------- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/msal/application.py b/msal/application.py index 05b77fc3..04ad5fd4 100644 --- a/msal/application.py +++ b/msal/application.py @@ -231,8 +231,23 @@ def __init__( :param str authority: A URL that identifies a token authority. It should be of the format - https://login.microsoftonline.com/your_tenant - By default, we will use https://login.microsoftonline.com/common + ``https://login.microsoftonline.com/your_tenant`` + By default, we will use ``https://login.microsoftonline.com/common`` + + *Changed in version 1.17*: you can also use predefined constant + and a builder like this:: + + from msal.authority import ( + AuthorityBuilder, + AZURE_US_GOVERNMENT, AZURE_CHINA, AZURE_PUBLIC) + my_authority = AuthorityBuilder(AZURE_PUBLIC, "contoso.onmicrosoft.com") + # Now you get an equivalent of + # "https://login.microsoftonline.com/contoso.onmicrosoft.com" + + # You can feed such an authority to msal's ClientApplication + from msal import PublicClientApplication + app = PublicClientApplication("my_client_id", authority=my_authority, ...) + :param bool validate_authority: (optional) Turns authority validation on or off. This parameter default to true. :param TokenCache cache: diff --git a/msal/authority.py b/msal/authority.py index 0656011f..14a6ad1a 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -14,12 +14,19 @@ logger = logging.getLogger(__name__) + +# Endpoints were copied from here +# https://docs.microsoft.com/en-us/azure/active-directory/develop/authentication-national-cloud#azure-ad-authentication-endpoints +AZURE_US_GOVERNMENT = "login.microsoftonline.us" +AZURE_CHINA = "login.chinacloudapi.cn" +AZURE_PUBLIC = "login.microsoftonline.com" + WORLD_WIDE = 'login.microsoftonline.com' # There was an alias login.windows.net WELL_KNOWN_AUTHORITY_HOSTS = set([ WORLD_WIDE, - 'login.chinacloudapi.cn', + AZURE_CHINA, 'login-us.microsoftonline.com', - 'login.microsoftonline.us', + AZURE_US_GOVERNMENT, 'login.microsoftonline.de', ]) WELL_KNOWN_B2C_HOSTS = [ @@ -30,6 +37,19 @@ ] +class AuthorityBuilder(object): + def __init__(self, instance, tenant): + """A helper to save caller from doing string concatenation. + + Usage is documented in :func:`application.ClientApplication.__init__`. + """ + self._instance = instance.rstrip("/") + self._tenant = tenant.strip("/") + + def __str__(self): + return "https://{}/{}".format(self._instance, self._tenant) + + class Authority(object): """This class represents an (already-validated) authority. @@ -53,6 +73,8 @@ def __init__(self, authority_url, http_client, validate_authority=True): performed. """ self._http_client = http_client + if isinstance(authority_url, AuthorityBuilder): + authority_url = str(authority_url) authority, self.instance, tenant = canonicalize(authority_url) parts = authority.path.split('/') is_b2c = any(self.instance.endswith("." + d) for d in WELL_KNOWN_B2C_HOSTS) or ( diff --git a/tests/http_client.py b/tests/http_client.py index a5587b70..5adbbded 100644 --- a/tests/http_client.py +++ b/tests/http_client.py @@ -20,6 +20,9 @@ def get(self, url, params=None, headers=None, **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): diff --git a/tests/test_authority.py b/tests/test_authority.py index cd6db785..9fdc83c5 100644 --- a/tests/test_authority.py +++ b/tests/test_authority.py @@ -8,16 +8,37 @@ @unittest.skipIf(os.getenv("TRAVIS_TAG"), "Skip network io during tagged release") class TestAuthority(unittest.TestCase): + def _test_given_host_and_tenant(self, host, tenant): + c = MinimalHttpClient() + a = Authority('https://{}/{}'.format(host, tenant), c) + self.assertEqual( + a.authorization_endpoint, + 'https://{}/{}/oauth2/v2.0/authorize'.format(host, tenant)) + self.assertEqual( + a.token_endpoint, + 'https://{}/{}/oauth2/v2.0/token'.format(host, tenant)) + c.close() + + def _test_authority_builder(self, host, tenant): + c = MinimalHttpClient() + a = Authority(AuthorityBuilder(host, tenant), c) + self.assertEqual( + a.authorization_endpoint, + 'https://{}/{}/oauth2/v2.0/authorize'.format(host, tenant)) + self.assertEqual( + a.token_endpoint, + 'https://{}/{}/oauth2/v2.0/token'.format(host, tenant)) + c.close() + def test_wellknown_host_and_tenant(self): # Assert all well known authority hosts are using their own "common" tenant for host in WELL_KNOWN_AUTHORITY_HOSTS: - a = Authority( - 'https://{}/common'.format(host), MinimalHttpClient()) - self.assertEqual( - a.authorization_endpoint, - 'https://%s/common/oauth2/v2.0/authorize' % host) - self.assertEqual( - a.token_endpoint, 'https://%s/common/oauth2/v2.0/token' % host) + self._test_given_host_and_tenant(host, "common") + + def test_wellknown_host_and_tenant_using_new_authority_builder(self): + self._test_authority_builder(AZURE_PUBLIC, "consumers") + self._test_authority_builder(AZURE_CHINA, "organizations") + self._test_authority_builder(AZURE_US_GOVERNMENT, "common") @unittest.skip("As of Jan 2017, the server no longer returns V1 endpoint") def test_lessknown_host_will_return_a_set_of_v1_endpoints(self): From b1a7802d4364fe0ee553f835ae725308dde155c0 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 9 Nov 2021 18:51:17 -0800 Subject: [PATCH 04/15] Fine tune http_cache usage pattern --- msal/application.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/msal/application.py b/msal/application.py index 04ad5fd4..5d1406af 100644 --- a/msal/application.py +++ b/msal/application.py @@ -377,10 +377,8 @@ def __init__( with open(http_cache_filename, "rb") as f: persisted_http_cache = pickle.load(f) # Take a snapshot except ( - IOError, # A non-exist http cache file + FileNotFoundError, # Or IOError in Python 2 pickle.UnpicklingError, # A corrupted http cache file - EOFError, # An empty http cache file - AttributeError, ImportError, IndexError, # Other corruption ): persisted_http_cache = {} # Recover by starting afresh atexit.register(lambda: pickle.dump( From e5b2b72c28984cb13d1bab995f903c3782fe6089 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 25 Nov 2021 11:40:19 -0800 Subject: [PATCH 05/15] Descriptive error messages for troubleshooting --- msal/authority.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/msal/authority.py b/msal/authority.py index 14a6ad1a..145ce3d9 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -109,7 +109,8 @@ def __init__(self, authority_url, http_client, validate_authority=True): raise ValueError( "Unable to get authority configuration for {}. " "Authority would typically be in a format of " - "https://login.microsoftonline.com/your_tenant_name".format( + "https://login.microsoftonline.com/your_tenant " + "Also please double check your tenant name or GUID is correct.".format( authority_url)) logger.debug("openid_config = %s", openid_config) self.authorization_endpoint = openid_config['authorization_endpoint'] @@ -170,7 +171,10 @@ def tenant_discovery(tenant_discovery_endpoint, http_client, **kwargs): if 400 <= resp.status_code < 500: # Nonexist tenant would hit this path # e.g. https://login.microsoftonline.com/nonexist_tenant/v2.0/.well-known/openid-configuration - raise ValueError("OIDC Discovery endpoint rejects our request") + raise ValueError( + "OIDC Discovery endpoint rejects our request. Error: {}".format( + resp.text # Expose it as-is b/c OIDC defines no error response format + )) # Transient network error would hit this path resp.raise_for_status() raise RuntimeError( # A fallback here, in case resp.raise_for_status() is no-op From 8bdb1ef8a3ee2ba3da0c50090683c555d7020198 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 10 Dec 2021 21:07:07 -0800 Subject: [PATCH 06/15] Document redirect_uri requirement inside sample --- sample/interactive_sample.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sample/interactive_sample.py b/sample/interactive_sample.py index b5f3950f..530892e5 100644 --- a/sample/interactive_sample.py +++ b/sample/interactive_sample.py @@ -53,7 +53,7 @@ if not result: logging.info("No suitable token exists in cache. Let's get a new one from AAD.") print("A local browser window will be open for you to sign in. CTRL+C to cancel.") - result = app.acquire_token_interactive( + result = app.acquire_token_interactive( # Only works if your app is registered with redirect_uri as http://localhost config["scope"], login_hint=config.get("username"), # Optional. # If you know the username ahead of time, this parameter can pre-fill From af30e44c63f0994933e41bfa98d36f67fbff741e Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 27 Dec 2021 12:46:14 -0800 Subject: [PATCH 07/15] Add actionable suggestion to resolve wrong time --- oauth2cli/oidc.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/oauth2cli/oidc.py b/oauth2cli/oidc.py index 4f1ca2bd..88eee93c 100644 --- a/oauth2cli/oidc.py +++ b/oauth2cli/oidc.py @@ -44,10 +44,11 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) err = None # 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 is correctly synchronized." 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." + err = "0. The ID token is not yet valid. " + TIME_SUGGESTION 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", ' @@ -68,7 +69,7 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) # 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 current time MUST be before the time represented by the exp Claim." + err = "9. The ID token already expires. " + TIME_SUGGESTION 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.") From 3df647045be2fb8a66e04ba585e458ffe4a1f622 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 17 Jan 2022 19:49:53 -0800 Subject: [PATCH 08/15] Also hint to check your time zone --- oauth2cli/oidc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2cli/oidc.py b/oauth2cli/oidc.py index 88eee93c..d4d3a927 100644 --- a/oauth2cli/oidc.py +++ b/oauth2cli/oidc.py @@ -44,7 +44,7 @@ def decode_id_token(id_token, client_id=None, issuer=None, nonce=None, now=None) err = None # 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 is correctly synchronized." + 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 From 304e0692141c089c54a4e00a8c598fa2b855aa78 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 18 Jan 2022 21:27:42 -0800 Subject: [PATCH 09/15] Lazy load dependencies --- msal/application.py | 4 ++-- msal/authority.py | 18 +++++++----------- msal/oauth2cli/assertion.py | 3 +-- msal/oauth2cli/oauth2.py | 4 ++-- tests/test_authority_patch.py | 32 -------------------------------- 5 files changed, 12 insertions(+), 49 deletions(-) delete mode 100644 tests/test_authority_patch.py diff --git a/msal/application.py b/msal/application.py index 5d1406af..a06df303 100644 --- a/msal/application.py +++ b/msal/application.py @@ -11,8 +11,6 @@ from threading import Lock import os -import requests - from .oauth2cli import Client, JwtAssertionCreator from .oauth2cli.oidc import decode_part from .authority import Authority @@ -425,6 +423,8 @@ def __init__( if http_client: self.http_client = http_client else: + import requests # Lazy load + self.http_client = requests.Session() self.http_client.verify = verify self.http_client.proxies = proxies diff --git a/msal/authority.py b/msal/authority.py index 145ce3d9..ecf6b777 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -5,11 +5,6 @@ from urlparse import urlparse import logging -# Historically some customers patched this module-wide requests instance. -# We keep it here for now. They will be removed in next major release. -import requests -import requests as _requests - from .exceptions import MsalServiceError @@ -59,9 +54,10 @@ class Authority(object): _domains_without_user_realm_discovery = set([]) @property - def http_client(self): # Obsolete. We will remove this in next major release. - # A workaround: if module-wide requests is patched, we honor it. - return self._http_client if requests is _requests else requests + def http_client(self): # Obsolete. We will remove this eventually + warnings.warn( + "authority.http_client might be removed in MSAL Python 1.21+", DeprecationWarning) + return self._http_client def __init__(self, authority_url, http_client, validate_authority=True): """Creates an authority instance, and also validates it. @@ -84,7 +80,7 @@ def __init__(self, authority_url, http_client, validate_authority=True): payload = instance_discovery( "https://{}{}/oauth2/v2.0/authorize".format( self.instance, authority.path), - self.http_client) + self._http_client) if payload.get("error") == "invalid_instance": raise ValueError( "invalid_instance: " @@ -104,7 +100,7 @@ def __init__(self, authority_url, http_client, validate_authority=True): try: openid_config = tenant_discovery( tenant_discovery_endpoint, - self.http_client) + self._http_client) except ValueError: raise ValueError( "Unable to get authority configuration for {}. " @@ -124,7 +120,7 @@ def user_realm_discovery(self, username, correlation_id=None, response=None): # "federation_protocol", "cloud_audience_urn", # "federation_metadata_url", "federation_active_auth_url", etc. if self.instance not in self.__class__._domains_without_user_realm_discovery: - resp = response or self.http_client.get( + resp = response or self._http_client.get( "https://{netloc}/common/userrealm/{username}?api-version=1.0".format( netloc=self.instance, username=username), headers={'Accept': 'application/json', diff --git a/msal/oauth2cli/assertion.py b/msal/oauth2cli/assertion.py index 0cf58799..855bd16b 100644 --- a/msal/oauth2cli/assertion.py +++ b/msal/oauth2cli/assertion.py @@ -4,8 +4,6 @@ import uuid import logging -import jwt - logger = logging.getLogger(__name__) @@ -99,6 +97,7 @@ def create_normal_assertion( Parameters are defined in https://tools.ietf.org/html/rfc7523#section-3 Key-value pairs in additional_claims will be added into payload as-is. """ + import jwt # Lazy loading now = time.time() payload = { 'aud': audience, diff --git a/msal/oauth2cli/oauth2.py b/msal/oauth2cli/oauth2.py index e092b3dd..54708004 100644 --- a/msal/oauth2cli/oauth2.py +++ b/msal/oauth2cli/oauth2.py @@ -17,8 +17,6 @@ import string import hashlib -import requests - from .authcode import AuthCodeReceiver as _AuthCodeReceiver try: @@ -159,6 +157,8 @@ def __init__( "when http_client is in use") self._http_client = http_client else: + import requests # Lazy loading + self._http_client = requests.Session() self._http_client.verify = True if verify is None else verify self._http_client.proxies = proxies diff --git a/tests/test_authority_patch.py b/tests/test_authority_patch.py deleted file mode 100644 index 1feca62d..00000000 --- a/tests/test_authority_patch.py +++ /dev/null @@ -1,32 +0,0 @@ -import unittest - -import msal -from tests.http_client import MinimalHttpClient - - -class DummyHttpClient(object): - def get(self, url, **kwargs): - raise RuntimeError("just for testing purpose") - - -class TestAuthorityHonorsPatchedRequests(unittest.TestCase): - """This is only a workaround for an undocumented behavior.""" - def test_authority_honors_a_patched_requests(self): - # First, we test that the original, unmodified authority is working - a = msal.authority.Authority( - "https://login.microsoftonline.com/common", MinimalHttpClient()) - self.assertEqual( - a.authorization_endpoint, - 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize') - - original = msal.authority.requests - try: - # Now we mimic a (discouraged) practice of patching authority.requests - msal.authority.requests = DummyHttpClient() - # msal.authority is expected to honor that patch. - with self.assertRaises(RuntimeError): - a = msal.authority.Authority( - "https://login.microsoftonline.com/common", MinimalHttpClient()) - finally: # Tricky: - # Unpatch is necessary otherwise other test cases would be affected - msal.authority.requests = original From 996a6636e5b746d59d0fdae6a6d06a5b2842948f Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 25 Jan 2022 13:20:12 -0800 Subject: [PATCH 10/15] Document new info on how to detect edge on Linux --- msal/application.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/msal/application.py b/msal/application.py index a06df303..c8d84649 100644 --- a/msal/application.py +++ b/msal/application.py @@ -78,6 +78,10 @@ def _preferred_browser(): if sys.platform != "linux": # On other platforms, we have no browser preference return None browser_path = "/usr/bin/microsoft-edge" # Use a full path owned by sys admin + # Note: /usr/bin/microsoft-edge, /usr/bin/microsoft-edge-stable, etc. + # are symlinks that point to the actual binaries which are found under + # /opt/microsoft/msedge/msedge or /opt/microsoft/msedge-beta/msedge. + # Either method can be used to detect an Edge installation. user_has_no_preference = "BROWSER" not in os.environ user_wont_mind_edge = "microsoft-edge" in os.environ.get("BROWSER", "") # Note: # BROWSER could contain "microsoft-edge" or "/path/to/microsoft-edge". From 59b383b5c306f469076b95190b0cd1517821bf45 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 2 Feb 2022 10:42:31 -0800 Subject: [PATCH 11/15] Remove decommissioned domain to get tests working --- msal/authority.py | 1 - 1 file changed, 1 deletion(-) diff --git a/msal/authority.py b/msal/authority.py index ecf6b777..4fb6e829 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -22,7 +22,6 @@ AZURE_CHINA, 'login-us.microsoftonline.com', AZURE_US_GOVERNMENT, - 'login.microsoftonline.de', ]) WELL_KNOWN_B2C_HOSTS = [ "b2clogin.com", From 57ad7638aae8228763b1b8968a7a43d9630eb5a7 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 2 Feb 2022 22:45:34 -0800 Subject: [PATCH 12/15] Change skip() to skipTest() --- tests/test_e2e.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index a23806ed..65691689 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -175,7 +175,7 @@ def _test_device_flow( assertion=lambda: self.assertIn('access_token', result), skippable_errors=self.app.client.DEVICE_FLOW_RETRIABLE_ERRORS) if "access_token" not in result: - self.skip("End user did not complete Device Flow in time") + self.skipTest("End user did not complete Device Flow in time") self.assertCacheWorksForUser(result, scope, username=None) result["access_token"] = result["refresh_token"] = "************" logger.info( @@ -528,6 +528,8 @@ def _test_acquire_token_by_auth_code_flow(
  • Sign In or Abort
    • """.format(id=self.id(), username_uri=username_uri), ) + if auth_response is None: + self.skipTest("Timed out. Did not have test settings in hand? Prepare and retry.") self.assertIsNotNone( auth_response.get("code"), "Error: {}, Detail: {}".format( auth_response.get("error"), auth_response)) From 1c801c371fe78e52037c0b0e46de1f659d3fad1b Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 8 Feb 2022 03:04:14 -0800 Subject: [PATCH 13/15] Actionable exception from ADFS ROPC --- msal/application.py | 18 ++++++++++++------ msal/wstrust_request.py | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/msal/application.py b/msal/application.py index c8d84649..60e5e2e5 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1417,12 +1417,18 @@ def acquire_token_by_username_password( user_realm_result = self.authority.user_realm_discovery( username, correlation_id=headers[msal.telemetry.CLIENT_REQUEST_ID]) if user_realm_result.get("account_type") == "Federated": - response = _clean_up(self._acquire_token_by_username_password_federated( - user_realm_result, username, password, scopes=scopes, - data=data, - headers=headers, **kwargs)) - telemetry_context.update_telemetry(response) - return response + try: + response = _clean_up(self._acquire_token_by_username_password_federated( + user_realm_result, username, password, scopes=scopes, + data=data, + headers=headers, **kwargs)) + except (ValueError, RuntimeError): + raise RuntimeError( + "ADFS is not configured properly. " + "Consider use acquire_token_interactive() instead.") + else: + telemetry_context.update_telemetry(response) + return response response = _clean_up(self.client.obtain_token_by_username_password( username, password, scope=scopes, headers=headers, diff --git a/msal/wstrust_request.py b/msal/wstrust_request.py index bdfb57ef..570bfc0e 100644 --- a/msal/wstrust_request.py +++ b/msal/wstrust_request.py @@ -44,8 +44,8 @@ def send_request( soap_action = Mex.ACTION_2005 elif '/trust/13/usernamemixed' in endpoint_address: soap_action = Mex.ACTION_13 - assert soap_action in (Mex.ACTION_13, Mex.ACTION_2005), ( # A loose check here - "Unsupported soap action: %s" % soap_action) + if soap_action not in (Mex.ACTION_13, Mex.ACTION_2005): + raise ValueError("Unsupported soap action: %s" % soap_action) data = _build_rst( username, password, cloud_audience_urn, endpoint_address, soap_action) resp = http_client.post(endpoint_address, data=data, headers={ From d590400993e5f0cf4a532a4fe84f81357f4c30e6 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 10 Feb 2022 01:11:56 -0800 Subject: [PATCH 14/15] Removes the middle-layer exception --- msal/application.py | 18 ++++++------------ msal/wstrust_request.py | 3 ++- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/msal/application.py b/msal/application.py index 60e5e2e5..c8d84649 100644 --- a/msal/application.py +++ b/msal/application.py @@ -1417,18 +1417,12 @@ def acquire_token_by_username_password( user_realm_result = self.authority.user_realm_discovery( username, correlation_id=headers[msal.telemetry.CLIENT_REQUEST_ID]) if user_realm_result.get("account_type") == "Federated": - try: - response = _clean_up(self._acquire_token_by_username_password_federated( - user_realm_result, username, password, scopes=scopes, - data=data, - headers=headers, **kwargs)) - except (ValueError, RuntimeError): - raise RuntimeError( - "ADFS is not configured properly. " - "Consider use acquire_token_interactive() instead.") - else: - telemetry_context.update_telemetry(response) - return response + response = _clean_up(self._acquire_token_by_username_password_federated( + user_realm_result, username, password, scopes=scopes, + data=data, + headers=headers, **kwargs)) + telemetry_context.update_telemetry(response) + return response response = _clean_up(self.client.obtain_token_by_username_password( username, password, scope=scopes, headers=headers, diff --git a/msal/wstrust_request.py b/msal/wstrust_request.py index 570bfc0e..43a2804f 100644 --- a/msal/wstrust_request.py +++ b/msal/wstrust_request.py @@ -45,7 +45,8 @@ def send_request( elif '/trust/13/usernamemixed' in endpoint_address: soap_action = Mex.ACTION_13 if soap_action not in (Mex.ACTION_13, Mex.ACTION_2005): - raise ValueError("Unsupported soap action: %s" % soap_action) + raise ValueError("Unsupported soap action: %s. " + "Contact your administrator to check your ADFS's MEX settings." % soap_action) data = _build_rst( username, password, cloud_audience_urn, endpoint_address, soap_action) resp = http_client.post(endpoint_address, data=data, headers={ From 0735871fc7f5ebe846b26c8524ea684f356e1411 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Tue, 8 Feb 2022 10:43:30 -0800 Subject: [PATCH 15/15] MSAL Python 1.17.0 --- 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 c8d84649..e2f20446 100644 --- a/msal/application.py +++ b/msal/application.py @@ -24,7 +24,7 @@ # The __init__.py will import this. Not the other way around. -__version__ = "1.16.0" +__version__ = "1.17.0" # When releasing, also check and bump our dependencies's versions if needed logger = logging.getLogger(__name__) diff --git a/setup.py b/setup.py index c5d89186..bcec8fe7 100644 --- a/setup.py +++ b/setup.py @@ -75,7 +75,7 @@ 'requests>=2.0.0,<3', 'PyJWT[crypto]>=1.0.0,<3', - 'cryptography>=0.6,<38', + 'cryptography>=0.6,<39', # load_pem_private_key() is available since 0.6 # https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst#06---2014-09-29 #