diff --git a/msal/authority.py b/msal/authority.py index 689ea8cb..51289d2c 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -1,10 +1,12 @@ import re +import logging import requests from .exceptions import MsalServiceError +logger = logging.getLogger(__name__) WORLD_WIDE = 'login.microsoftonline.com' # There was an alias login.windows.net WELL_KNOWN_AUTHORITY_HOSTS = set([ WORLD_WIDE, @@ -38,7 +40,7 @@ def __init__(self, authority_url, validate_authority=True, canonicalized, self.instance, tenant = canonicalize(authority_url) tenant_discovery_endpoint = ( # Hard code a V2 pattern as default value 'https://{}/{}/v2.0/.well-known/openid-configuration' - .format(WORLD_WIDE, tenant)) + .format(self.instance, tenant)) if validate_authority and self.instance not in WELL_KNOWN_AUTHORITY_HOSTS: tenant_discovery_endpoint = instance_discovery( canonicalized + "/oauth2/v2.0/authorize", @@ -46,6 +48,7 @@ def __init__(self, authority_url, validate_authority=True, openid_config = tenant_discovery( tenant_discovery_endpoint, verify=verify, proxies=proxies, timeout=timeout) + logger.debug("openid_config = %s", openid_config) self.authorization_endpoint = openid_config['authorization_endpoint'] self.token_endpoint = openid_config['token_endpoint'] _, _, self.tenant = canonicalize(self.token_endpoint) # Usually a GUID @@ -76,7 +79,11 @@ def canonicalize(url): def instance_discovery(url, response=None, **kwargs): # Returns tenant discovery endpoint resp = requests.get( # Note: This URL seemingly returns V1 endpoint only - 'https://{}/common/discovery/instance'.format(WORLD_WIDE), + '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() diff --git a/tests/test_authority.py b/tests/test_authority.py index 41714552..d7fc5cac 100644 --- a/tests/test_authority.py +++ b/tests/test_authority.py @@ -4,24 +4,16 @@ class TestAuthority(unittest.TestCase): - COMMON_AUTH_ENDPOINT = \ - 'https://login.microsoftonline.com/common/oauth2/v2.0/authorize' - COMMON_TOKEN_ENDPOINT = \ - 'https://login.microsoftonline.com/common/oauth2/v2.0/token' def test_wellknown_host_and_tenant(self): - # Test one specific sample in straightforward way, for readability - a = Authority('https://login.microsoftonline.com/common') - self.assertEqual(a.authorization_endpoint, self.COMMON_AUTH_ENDPOINT) - self.assertEqual(a.token_endpoint, self.COMMON_TOKEN_ENDPOINT) - - # Test all well known authority hosts, using same real "common" tenant + # 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)) - # Note: this "common" tenant endpoints always point to its real host self.assertEqual( - a.authorization_endpoint, self.COMMON_AUTH_ENDPOINT) - self.assertEqual(a.token_endpoint, self.COMMON_TOKEN_ENDPOINT) + a.authorization_endpoint, + 'https://%s/common/oauth2/v2.0/authorize' % host) + self.assertEqual( + a.token_endpoint, 'https://%s/common/oauth2/v2.0/token' % host) @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): @@ -33,20 +25,12 @@ def test_lessknown_host_will_return_a_set_of_v1_endpoints(self): self.assertEqual(a.token_endpoint, v1_token_endpoint) self.assertNotIn('v2.0', a.token_endpoint) - def test_unknown_host(self): + def test_unknown_host_wont_pass_instance_discovery(self): with self.assertRaisesRegexp(MsalServiceError, "invalid_instance"): Authority('https://unknown.host/tenant_doesnt_matter_in_this_case') - def test_unknown_host_valid_tenant_and_skip_host_validation(self): - # When skipping host (a.k.a. instance) validation, - # the Tenant Discovery will always use WORLD_WIDE service as instance, - # so, if the tenant happens to exist there, it will find some endpoints. - a = Authority('https://incorrect.host/common', validate_authority=False) - self.assertEqual(a.authorization_endpoint, self.COMMON_AUTH_ENDPOINT) - self.assertEqual(a.token_endpoint, self.COMMON_TOKEN_ENDPOINT) - - def test_unknown_host_unknown_tenant_and_skip_host_validation(self): - with self.assertRaisesRegexp(MsalServiceError, "invalid_tenant"): + def test_invalid_host_skipping_validation_meets_connection_error_down_the_road(self): + with self.assertRaises(requests.exceptions.RequestException): Authority('https://unknown.host/invalid', validate_authority=False)