diff --git a/msal/authority.py b/msal/authority.py index faf11603..bfb73e15 100644 --- a/msal/authority.py +++ b/msal/authority.py @@ -178,7 +178,7 @@ def user_realm_discovery(self, username, correlation_id=None, response=None): def canonicalize(authority_or_auth_endpoint): # Returns (url_parsed_result, hostname_in_lowercase, tenant) authority = urlparse(authority_or_auth_endpoint) - if authority.scheme == "https": + if authority.scheme == "https" and authority.hostname: parts = authority.path.split("/") first_part = parts[1] if len(parts) >= 2 and parts[1] else None if authority.hostname.endswith(_CIAM_DOMAIN_SUFFIX): # CIAM @@ -192,7 +192,7 @@ def canonicalize(authority_or_auth_endpoint): return authority, authority.hostname, parts[1] raise ValueError( "Your given address (%s) should consist of " - "an https url with a minimum of one segment in a path: e.g. " + "an https url with hostname and a minimum of one segment in a path: e.g. " "https://login.microsoftonline.com/{tenant} " "or https://{tenant_name}.ciamlogin.com/{tenant} " "or https://{tenant_name}.b2clogin.com/{tenant_name}.onmicrosoft.com/policy" diff --git a/msal/managed_identity.py b/msal/managed_identity.py index fe76691f..422b76e3 100644 --- a/msal/managed_identity.py +++ b/msal/managed_identity.py @@ -6,7 +6,6 @@ import json import logging import os -import socket import sys import time from urllib.parse import urlparse # Python 3+ @@ -146,7 +145,10 @@ class ManagedIdentityClient(object): (like what a ``PublicClientApplication`` does), not a token with application permissions for an app. """ - __instance, _tenant = None, "managed_identity" # Placeholders + __instance = "localhost" # We used to get this value from socket.getfqdn() + # but it is unreliable because getfqdn() either hangs or returns empty value + # on some misconfigured machines + _tenant = "managed_identity" _TOKEN_SOURCE = "token_source" _TOKEN_SOURCE_IDP = "identity_provider" _TOKEN_SOURCE_CACHE = "cache" @@ -252,11 +254,6 @@ def __init__( self._token_cache = token_cache or TokenCache() self._client_capabilities = client_capabilities - def _get_instance(self): - if self.__instance is None: - self.__instance = socket.getfqdn() # Moved from class definition to here - return self.__instance - def acquire_token_for_client( self, *, @@ -302,7 +299,7 @@ def acquire_token_for_client( target=[resource], query=dict( client_id=client_id_in_cache, - environment=self._get_instance(), + environment=self.__instance, realm=self._tenant, home_account_id=None, ), @@ -344,7 +341,7 @@ def acquire_token_for_client( client_id=client_id_in_cache, scope=[resource], token_endpoint="https://{}/{}".format( - self._get_instance(), self._tenant), + self.__instance, self._tenant), response=result, params={}, data={}, diff --git a/tests/test_authority.py b/tests/test_authority.py index 3fd1fce1..312037ff 100644 --- a/tests/test_authority.py +++ b/tests/test_authority.py @@ -190,6 +190,10 @@ def test_canonicalize_rejects_tenantless_host_with_trailing_slash(self): with self.assertRaises(ValueError): canonicalize("https://no.tenant.example.com/") + def test_canonicalize_rejects_empty_host(self): + with self.assertRaises(ValueError): + canonicalize("https:///tenant") + @unittest.skipIf(os.getenv("TRAVIS_TAG"), "Skip network io during tagged release") class TestAuthorityInternalHelperUserRealmDiscovery(unittest.TestCase): diff --git a/tests/test_mi.py b/tests/test_mi.py index 864d258c..bf1faedd 100644 --- a/tests/test_mi.py +++ b/tests/test_mi.py @@ -190,8 +190,12 @@ def test_happy_path_of_vm(self): headers={'Metadata': 'true'}, ) - @patch("msal.managed_identity.socket.getfqdn", new=lambda: "MixedCaseHostName") - def test_happy_path_of_windows_vm(self): + @patch.object(ManagedIdentityClient, "_ManagedIdentityClient__instance", "MixedCaseHostName") + def test_happy_path_of_theoretical_mixed_case_hostname(self): + """Historically, we used to get the host name from socket.getfqdn(), + which could return a mixed-case host name on Windows. + Although we no longer use getfqdn(), we still keep this test case to ensure we tolerate it. + """ self.test_happy_path_of_vm() @patch.dict(os.environ, {"AZURE_POD_IDENTITY_AUTHORITY_HOST": "http://localhost:1234//"})