Refactor: instance_discovery() emits actionable msg

It will guide app developer to use validate_authority=False when needed.

Author: rayluo

msal/authority.py

@@ -46,9 +46,18 @@ def __init__(self, authority_url, validate_authority=True,
                 ))
         if (tenant != "adfs" and validate_authority
                 and self.instance not in WELL_KNOWN_AUTHORITY_HOSTS):
-            tenant_discovery_endpoint = instance_discovery(
+            payload = instance_discovery(
                 canonicalized + "/oauth2/v2.0/authorize",
                 verify=verify, proxies=proxies, timeout=timeout)
+            if payload.get("error") == "invalid_instance":
+                raise ValueError(
+                    "invalid_instance: "
+                    "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"
+                    % authority_url)
+            tenant_discovery_endpoint = payload['tenant_discovery_endpoint']
         openid_config = tenant_discovery(
             tenant_discovery_endpoint,
             verify=verify, proxies=proxies, timeout=timeout)
@@ -80,20 +89,15 @@ def canonicalize(url):
             "https://login.microsoftonline.com/<tenant_name>" % url)
     return match_object.group(0), match_object.group(1), match_object.group(2)
 
-def instance_discovery(url, response=None, **kwargs):
-    # Returns tenant discovery endpoint
-    resp = requests.get(  # Note: This URL seemingly returns V1 endpoint only
+def instance_discovery(url, **kwargs):
+    return requests.get(  # Note: This URL seemingly returns V1 endpoint only
         'https://{}/common/discovery/instance'.format(
             WORLD_WIDE  # Historically using WORLD_WIDE. Could use self.instance too
                 # See https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/4.0.0/src/Microsoft.Identity.Client/Instance/AadInstanceDiscovery.cs#L101-L103
                 # and https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/4.0.0/src/Microsoft.Identity.Client/Instance/AadAuthority.cs#L19-L33
             ),
         params={'authorization_endpoint': url, 'api-version': '1.0'},
-        **kwargs)
-    payload = response or resp.json()
-    if 'tenant_discovery_endpoint' not in payload:
-        raise MsalServiceError(status_code=resp.status_code, **payload)
-    return payload['tenant_discovery_endpoint']
+        **kwargs).json()
 
 def tenant_discovery(tenant_discovery_endpoint, **kwargs):
     # Returns Openid Configuration

tests/test_authority.py

@@ -1,8 +1,11 @@
+import os
+
 from msal.authority import *
 from msal.exceptions import MsalServiceError
 from tests import unittest
 
 
+@unittest.skipIf(os.getenv("TRAVIS_TAG"), "Skip network io during tagged release")
 class TestAuthority(unittest.TestCase):
 
     def test_wellknown_host_and_tenant(self):
@@ -26,7 +29,7 @@ def test_lessknown_host_will_return_a_set_of_v1_endpoints(self):
         self.assertNotIn('v2.0', a.token_endpoint)
 
     def test_unknown_host_wont_pass_instance_discovery(self):
-        with self.assertRaisesRegexp(MsalServiceError, "invalid_instance"):
+        with self.assertRaisesRegexp(ValueError, "invalid_instance"):
             Authority('https://unknown.host/tenant_doesnt_matter_in_this_case')
 
     def test_invalid_host_skipping_validation_meets_connection_error_down_the_road(self):
@@ -63,21 +66,3 @@ def test_canonicalize_rejects_tenantless_host_with_trailing_slash(self):
         with self.assertRaises(ValueError):
             canonicalize("https://no.tenant.example.com/")
 
-
-class TestAuthorityInternalHelperInstanceDiscovery(unittest.TestCase):
-
-    def test_instance_discovery_happy_case(self):
-        self.assertEqual(
-            instance_discovery("https://login.windows.net/tenant"),
-            "https://login.windows.net/tenant/.well-known/openid-configuration")
-
-    def test_instance_discovery_with_unknown_instance(self):
-        with self.assertRaisesRegexp(MsalServiceError, "invalid_instance"):
-            instance_discovery('https://unknown.host/tenant_doesnt_matter_here')
-
-    def test_instance_discovery_with_mocked_response(self):
-        mock_response = {'tenant_discovery_endpoint': 'http://a.com/t/openid'}
-        endpoint = instance_discovery(
-            "https://login.microsoftonline.in/tenant.com", response=mock_response)
-        self.assertEqual(endpoint, mock_response['tenant_discovery_endpoint'])
-