Merge pull request #457 from AzureAD/release-1.17.0

MSAL Python 1.17.0

Author: rayluo

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(

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

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,

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

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.")

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={

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

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
             #

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):

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"