diff --git a/msal/application.py b/msal/application.py index 13015d09..e2f20446 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 @@ -26,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__) @@ -80,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". @@ -231,8 +233,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: @@ -362,10 +379,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( @@ -412,6 +427,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 @@ -1207,7 +1224,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( diff --git a/msal/authority.py b/msal/authority.py index 0656011f..4fb6e829 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -5,22 +5,23 @@ 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 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', - 'login.microsoftonline.de', + AZURE_US_GOVERNMENT, ]) WELL_KNOWN_B2C_HOSTS = [ "b2clogin.com", @@ -30,6 +31,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. @@ -39,9 +53,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. @@ -53,6 +68,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 ( @@ -62,7 +79,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: " @@ -82,12 +99,13 @@ 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 {}. " "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'] @@ -101,7 +119,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', @@ -148,7 +166,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 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/msal/oauth2cli/oidc.py b/msal/oauth2cli/oidc.py index 4f1ca2bd..d4d3a927 100644 --- a/msal/oauth2cli/oidc.py +++ b/msal/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 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." + 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.") diff --git a/msal/wstrust_request.py b/msal/wstrust_request.py index bdfb57ef..43a2804f 100644 --- a/msal/wstrust_request.py +++ b/msal/wstrust_request.py @@ -44,8 +44,9 @@ 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. " + "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={ 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 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 # 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_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" 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): 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 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))