Change known_authority_hosts from per-class to per-instance

Author: rayluo

msal/application.py

@@ -159,47 +159,10 @@ class ClientApplication(object):
 
     ATTEMPT_REGION_DISCOVERY = True  # "TryAutoDetect"
 
-    _known_authority_hosts = None
-
-    @classmethod
-    def set_known_authority_hosts(cls, known_authority_hosts):
-        """Declare a list of hosts which you allow MSAL to operate with.
-
-        If your app operates with some authorities that you know and own,
-        such as some ADFS or B2C or private cloud,
-        it is recommended and sometimes required that you declare them here,
-        so that MSAL will use your authorities without discovery,
-        and reject most of the other undefined authorities.
-
-        ``known_authority_hosts`` is meant to be a static and per-deployment setting.
-        This classmethod shall be called at most once,
-        during your entire app's starting-up,
-        before your initializing any ``PublicClientApplication`` or
-        ``ConfidentialClientApplication`` instance(s).
-
-        :param list[str] known_authority_hosts:
-            Authorities that you known, for example::
-
-                [
-                    "contoso.com",  # Your own domain
-                    "login.azs",  # This can be a private cloud
-                ]
-
-        New in version 1.19
-        """
-        new_input = frozenset(known_authority_hosts)
-        if (cls._known_authority_hosts is not None
-                and cls._known_authority_hosts != new_input):
-            raise ValueError(
-                "The known_authority_hosts are considered static. "
-                "Once configured, they should not be changed.")
-        cls._known_authority_hosts = new_input
-        logger.debug('known_authority_hosts is set to %s', known_authority_hosts)
-
-    def _union_known_authority_hosts(cls, url=None, host=None):
+    def _union_known_authority_hosts(self, url=None, host=None):
         host = host if host else urlparse(url).netloc.split(":")[0]
-        return (cls._known_authority_hosts.union([host])
-            if cls._known_authority_hosts else frozenset([host]))
+        return (self._known_authority_hosts.union([host])
+            if self._known_authority_hosts else frozenset([host]))
 
     def __init__(
             self, client_id,
@@ -215,6 +178,7 @@ def __init__(
                 # when we would eventually want to add this feature to PCA in future.
             exclude_scopes=None,
             http_cache=None,
+            known_authority_hosts=None,
             ):
         """Create an instance of application.
 
@@ -450,11 +414,41 @@ def __init__(
             Personally Identifiable Information (PII). Encryption is unnecessary.
 
             New in version 1.16.0.
+
+        :param list[str] known_authority_hosts:
+            Historically, MSAL would try to connect to a central endpoint
+            to acquire some metadata for an unfamiliar authority.
+            This behavior is known as Instance Discovery.
+
+            If you know a list of hosts which you allow MSAL to operate with,
+            you can declare them here, so that MSAL will use them as-is,
+            and also reject most of other authorities such as a call like this:
+            ``ClientApplication("id", authority="https://undefined.com", known_authority_hosts=["contoso.com", "fabricam.com"])``.
+
+            Typically, the ADFS or B2C or private cloud authorities
+            is recommended and sometimes required to be declared here.
+
+            This is meant to be a static and per-deployment setting.
+            The recommended pattern is to load your predefined constant list
+            from a configuration file,
+            and never create new MSAL ``ClientApplication`` instances with
+            unknown and untrusted authorities during runtime.
+
+            Values would look like::
+
+                [
+                    "contoso.com",  # Your own domain
+                    "login.azs",  # This can be a private cloud
+                ]
+
+            New in version 1.19
         """
         self.client_id = client_id
         self.client_credential = client_credential
         self.client_claims = client_claims
         self._client_capabilities = client_capabilities
+        self._known_authority_hosts = frozenset(
+            known_authority_hosts if known_authority_hosts else [])
 
         if exclude_scopes and not isinstance(exclude_scopes, list):
             raise ValueError(
@@ -498,7 +492,7 @@ def __init__(
             self.authority = Authority(
                 authority_to_use,
                 self.http_client, validate_authority=validate_authority,
-                known_authority_hosts=self.__class__._known_authority_hosts,
+                known_authority_hosts=self._known_authority_hosts,
                 )
         except ValueError:  # Those are explicit authority validation errors
             raise
@@ -839,7 +833,7 @@ def get_authorization_request_url(
         the_authority = Authority(
             authority,
             self.http_client,
-            known_authority_hosts=self.__class__._known_authority_hosts,
+            known_authority_hosts=self._known_authority_hosts,
             ) if authority else self.authority
 
         client = _ClientWithCcsRoutingInfo(

tests/test_authority.py

@@ -171,59 +171,52 @@ def test_by_default_b2c_should_skip_validation_and_instance_metadata(
         app.get_accounts()  # This could make an instance metadata call for authority aliases
         instance_metadata.assert_not_called()
 
-    def test_known_authorities_should_allow_multiple_idempotent_calls_but_no_changes(self, *mocks):
-        known_hosts = ["contoso.com", "fabricam.com"]
-        msal.ClientApplication.set_known_authority_hosts(known_hosts)
-        msal.ClientApplication.set_known_authority_hosts(known_hosts)  # Allows idempotent calls
-        msal.ClientApplication.set_known_authority_hosts(
-            # Optional. Here we treat hosts orderless therefore accept equivalent call
-            reversed(known_hosts))
-        with self.assertRaises(ValueError):
-            msal.ClientApplication.set_known_authority_hosts(["hacked.com"])
-
     def test_known_authority_hosts_should_not_coexist_with_validate_authority_boolean(self, *mocks):
-        msal.ClientApplication.set_known_authority_hosts(["private.cloud"])
         with self.assertRaises(ValueError):
             msal.ClientApplication(
                 "id",
-                authority="https://unknown.com/common", validate_authority=False)
+                authority="https://unknown.com/common",
+                validate_authority=False, known_authority_hosts=["private.cloud"])
 
     def test_known_to_developer_authority_should_skip_validation_and_instance_metadata(
             self, instance_metadata, known_to_microsoft_validation, _):
-        msal.ClientApplication.set_known_authority_hosts(["private.cloud"])
-        app = msal.ClientApplication("foo", authority="https://private.cloud/foo")
+        app = msal.ClientApplication(
+            "foo",
+            authority="https://private.cloud/foo",
+            known_authority_hosts=["private.cloud"])
         known_to_microsoft_validation.assert_not_called()
         app.get_accounts()  # This could make an instance metadata call for authority aliases
         instance_metadata.assert_not_called()
 
     def test_known_to_developer_setting_should_still_let_a_known_to_microsoft_authority_skip_validation_and_use_instance_metadata(  # i.e. same as the without-known-to-developer behavior
             self, instance_metadata, known_to_microsoft_validation, _):
-        msal.ClientApplication.set_known_authority_hosts(["private.cloud"])
         app = msal.ClientApplication(
-            "id", authority="https://login.microsoftonline.com/common")
+            "id",
+            authority="https://login.microsoftonline.com/common",
+            known_authority_hosts=["private.cloud"])
         known_to_microsoft_validation.assert_not_called()
         app.get_accounts()  # This could make an instance metadata call for authority aliases
         instance_metadata.assert_called_once_with()
 
     def test_known_authorities_should_make_unknown_adfs_perform_validation_and_instance_metadata(
             self, instance_metadata, known_to_microsoft_validation, _):
-        msal.ClientApplication.set_known_authority_hosts(["private.cloud"])
-        app = msal.ClientApplication("foo", authority="https://unknown.com/adfs")
+        app = msal.ClientApplication(
+            "foo",
+            authority="https://unknown.com/adfs",
+            known_authority_hosts=["private.cloud"])
         known_to_microsoft_validation.assert_called_once_with(ANY, ANY)  # Python 3.5+
         # We effectively mocked a passed validation, so that MSAL will proceed
         app.get_accounts()  # This could make an instance metadata call for authority aliases
         instance_metadata.assert_called_once_with()
 
     def test_known_authorities_should_make_unknown_b2c_perform_validation_and_instance_metadata(
             self, instance_metadata, known_to_microsoft_validation, _):
-        msal.ClientApplication.set_known_authority_hosts(["private.cloud"])
-        app = msal.ClientApplication("foo", authority="https://b2clogin.com/contoso/b2c_policy")
+        app = msal.ClientApplication(
+            "foo",
+            authority="https://b2clogin.com/contoso/b2c_policy",
+            known_authority_hosts=["private.cloud"])
         known_to_microsoft_validation.assert_called_once_with(ANY, ANY)  # Python 3.5+
         # We effectively mocked a passed validation, so that MSAL will proceed
         app.get_accounts()  # This could make an instance metadata call for authority aliases
         instance_metadata.assert_called_once_with()
 
-    def tearDown(self):
-        # This is not part of public API. We do it here only for unit-testing.
-        msal.ClientApplication._known_authority_hosts = None  # Reset
-