Only call IMDS when input region != None

Author: rayluo

msal/application.py

@@ -232,11 +232,22 @@ def __init__(
         :param str region:
             Added since MSAL Python 1.12.0.
 
-            If enabled, MSAL token requests would remain inside that region.
-            Currently, regional endpoint only supports using
-            ``acquire_token_for_client()`` for some scopes.
+            As of 2021 May, regional service is only available for
+            ``acquire_token_for_client()`` sent by any of the following scenarios::
 
-            The default value is None, which means region support remains turned off.
+            1. An app powered by a capable MSAL
+               (MSAL Python 1.12+ will be provisioned)
+
+            2. An app with managed identity, which is formerly known as MSI.
+               (However MSAL Python does not support managed identity,
+               so this one does not apply.)
+
+            3. An app authenticated by Subject Name/Issuer (SNI).
+
+            4. An app which already onboard to the region's allow-list.
+
+            MSAL's default value is None, which means region behavior remains off.
+            If enabled, some of the MSAL traffic would remain inside that region.
 
             App developer can opt in to regional endpoint,
             by provide a region name, such as "westus", "eastus2".
@@ -261,7 +272,7 @@ def __init__(
             # Requests, does not support session - wide timeout
             # But you can patch that (https://github.com/psf/requests/issues/3341):
             self.http_client.request = functools.partial(
-                self.http_client.request, timeout=timeout or 2)
+                self.http_client.request, timeout=timeout)
 
             # Enable a minimal retry. Better than nothing.
             # https://github.com/psf/requests/blob/v2.25.1/requests/adapters.py#L94-L108
@@ -291,11 +302,15 @@ def _build_telemetry_context(
             correlation_id=correlation_id, refresh_reason=refresh_reason)
 
     def _get_regional_authority(self, central_authority):
-        self._region_detected = self._region_detected or _detect_region(self.http_client)
-        if self._region_configured and self._region_detected != self._region_configured:
+        is_region_specified = bool(self._region_configured
+            and self._region_configured != self.ATTEMPT_REGION_DISCOVERY)
+        self._region_detected = self._region_detected or _detect_region(
+            self.http_client if self._region_configured is not None else None)
+        if (is_region_specified and self._region_configured != self._region_detected):
             logger.warning('Region configured ({}) != region detected ({})'.format(
                 repr(self._region_configured), repr(self._region_detected)))
-        region_to_use = self._region_configured or self._region_detected
+        region_to_use = (
+            self._region_configured if is_region_specified else self._region_detected)
         if region_to_use:
             logger.info('Region to be used: {}'.format(repr(region_to_use)))
             regional_host = ("{}.login.microsoft.com".format(region_to_use)

msal/region.py

@@ -5,8 +5,11 @@
 logger = logging.getLogger(__name__)
 
 
-def _detect_region(http_client):
-    return _detect_region_of_azure_function() or _detect_region_of_azure_vm(http_client)
+def _detect_region(http_client=None):
+    region = _detect_region_of_azure_function()  # It is cheap, so we do it always
+    if http_client and not region:
+        return _detect_region_of_azure_vm(http_client)  # It could hang for minutes
+    return region
 
 
 def _detect_region_of_azure_function():
@@ -15,11 +18,15 @@ def _detect_region_of_azure_function():
 
 def _detect_region_of_azure_vm(http_client):
     url = "http://169.254.169.254/metadata/instance?api-version=2021-01-01"
+    logger.info(
+        "Connecting to IMDS {}. "
+        "You may want to use a shorter timeout on your http_client".format(url))
     try:
         # https://docs.microsoft.com/en-us/azure/virtual-machines/windows/instance-metadata-service?tabs=linux#instance-metadata
         resp = http_client.get(url, headers={"Metadata": "true"})
     except:
-        logger.info("IMDS {} unavailable. Perhaps not running in Azure VM?".format(url))
+        logger.info(
+            "IMDS {} unavailable. Perhaps not running in Azure VM?".format(url))
         return None
     else:
         return json.loads(resp.text)["compute"]["location"]

tests/test_e2e.py

@@ -27,7 +27,7 @@ def _get_app_and_auth_code(
         app = msal.ConfidentialClientApplication(
             client_id,
             client_credential=client_secret,
-            authority=authority, http_client=MinimalHttpClient(timeout=2))
+            authority=authority, http_client=MinimalHttpClient())
     else:
         app = msal.PublicClientApplication(
             client_id, authority=authority, http_client=MinimalHttpClient())
@@ -292,7 +292,7 @@ def test_client_secret(self):
             self.config["client_id"],
             client_credential=self.config.get("client_secret"),
             authority=self.config.get("authority"),
-            http_client=MinimalHttpClient(timeout=2))
+            http_client=MinimalHttpClient())
         scope = self.config.get("scope", [])
         result = self.app.acquire_token_for_client(scope)
         self.assertIn('access_token', result)
@@ -307,7 +307,7 @@ def test_client_certificate(self):
         self.app = msal.ConfidentialClientApplication(
             self.config['client_id'],
             {"private_key": private_key, "thumbprint": client_cert["thumbprint"]},
-            http_client=MinimalHttpClient(timeout=2))
+            http_client=MinimalHttpClient())
         scope = self.config.get("scope", [])
         result = self.app.acquire_token_for_client(scope)
         self.assertIn('access_token', result)
@@ -330,7 +330,7 @@ def test_subject_name_issuer_authentication(self):
                 "thumbprint": self.config["thumbprint"],
                 "public_certificate": public_certificate,
                 },
-            http_client=MinimalHttpClient(timeout=2))
+            http_client=MinimalHttpClient())
         scope = self.config.get("scope", [])
         result = self.app.acquire_token_for_client(scope)
         self.assertIn('access_token', result)
@@ -379,7 +379,7 @@ def get_lab_app(
             client_id,
             client_credential=client_secret,
             authority=authority,
-            http_client=MinimalHttpClient(timeout=2),
+            http_client=MinimalHttpClient(),
             **kwargs)
 
 def get_session(lab_app, scopes):  # BTW, this infrastructure tests the confidential client flow
@@ -528,7 +528,7 @@ def _test_acquire_token_obo(self, config_pca, config_cca):
             config_cca["client_id"],
             client_credential=config_cca["client_secret"],
             authority=config_cca["authority"],
-            http_client=MinimalHttpClient(timeout=2),
+            http_client=MinimalHttpClient(),
             # token_cache= ...,  # Default token cache is all-tokens-store-in-memory.
                 # That's fine if OBO app uses short-lived msal instance per session.
                 # Otherwise, the OBO app need to implement a one-cache-per-user setup.
@@ -556,7 +556,7 @@ def _test_acquire_token_by_client_secret(
         assert client_id and client_secret and authority and scope
         app = msal.ConfidentialClientApplication(
             client_id, client_credential=client_secret, authority=authority,
-            http_client=MinimalHttpClient(timeout=2))
+            http_client=MinimalHttpClient())
         result = app.acquire_token_for_client(scope)
         self.assertIsNotNone(result.get("access_token"), "Got %s instead" % result)
 
@@ -758,7 +758,7 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self):
         scopes = ["https://graph.microsoft.com/.default"]
         result = self.app.acquire_token_for_client(
             scopes,
-            params={"AllowEstsRNonMsi": "true"},  # For testing regional endpoint
+            params={"AllowEstsRNonMsi": "true"},  # For testing regional endpoint. It will be removed once MSAL Python 1.12+ has been onboard to ESTS-R
             )
         self.assertIn('access_token', result)
         self.assertCacheWorksForApp(result, scopes)
@@ -855,7 +855,7 @@ def test_acquire_token_silent_with_an_empty_cache_should_return_none(self):
             usertype="cloud", azureenvironment=self.environment, publicClient="no")
         app = msal.ConfidentialClientApplication(
             config['client_id'], authority=config['authority'],
-            http_client=MinimalHttpClient(timeout=2))
+            http_client=MinimalHttpClient())
         result = app.acquire_token_silent(scopes=config['scope'], account=None)
         self.assertEqual(result, None)
         # Note: An alias in this region is no longer accepting HTTPS traffic.