From c04e6eacafdfe426f3262471e7a6bbbd6cc732f8 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 6 Oct 2021 11:20:41 -0700 Subject: [PATCH 1/2] Re-enable REGION env var detection --- msal/region.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/msal/region.py b/msal/region.py index dacd49d7..c540dc71 100644 --- a/msal/region.py +++ b/msal/region.py @@ -5,6 +5,9 @@ def _detect_region(http_client=None): + region = os.environ.get("REGION_NAME", "").replace(" ", "").lower() # e.g. westus2 + if region: + return region if http_client: return _detect_region_of_azure_vm(http_client) # It could hang for minutes return None From 56e4b01c8ddf5b04a2040889b1e11be92b977058 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Thu, 21 Oct 2021 19:35:45 -0700 Subject: [PATCH 2/2] Change Regional Endpoint to require opt-in --- msal/application.py | 21 ++++++++++++--------- tests/test_e2e.py | 34 +++++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/msal/application.py b/msal/application.py index 3651d216..c10935dd 100644 --- a/msal/application.py +++ b/msal/application.py @@ -286,7 +286,8 @@ def __init__( which you will later provide via one of the acquire-token request. :param str azure_region: - Added since MSAL Python 1.12.0. + AAD provides regional endpoints for apps to opt in + to keep their traffic remain inside that region. As of 2021 May, regional service is only available for ``acquire_token_for_client()`` sent by any of the following scenarios:: @@ -303,9 +304,7 @@ def __init__( 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, the `acquire_token_for_client()`-relevant traffic - would remain inside that region. + This parameter defaults to None, which means region behavior remains off. App developer can opt in to a regional endpoint, by provide its region name, such as "westus", "eastus2". @@ -331,6 +330,9 @@ def __init__( or provide a custom http_client which has a short timeout. That way, the latency would be under your control, but still less performant than opting out of region feature. + + New in version 1.12.0. + :param list[str] exclude_scopes: (optional) Historically MSAL hardcodes `offline_access` scope, which would allow your app to have prolonged access to user's data. @@ -492,17 +494,18 @@ def _build_telemetry_context( correlation_id=correlation_id, refresh_reason=refresh_reason) def _get_regional_authority(self, central_authority): - 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): + if (self._region_configured != self.ATTEMPT_REGION_DISCOVERY + 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 if is_region_specified else self._region_detected) + self._region_detected + if self._region_configured == self.ATTEMPT_REGION_DISCOVERY + else self._region_configured) # It will retain the None i.e. opted out + logger.debug('Region to be used: {}'.format(repr(region_to_use))) if region_to_use: - logger.info('Region to be used: {}'.format(repr(region_to_use))) regional_host = ("{}.r.login.microsoftonline.com".format(region_to_use) if central_authority.instance in ( # The list came from https://github.com/AzureAD/microsoft-authentication-library-for-python/pull/358/files#r629400328 diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 2defecd6..a23806ed 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -791,7 +791,7 @@ class WorldWideRegionalEndpointTestCase(LabBasedTestCase): region = "westus" timeout = 2 # Short timeout makes this test case responsive on non-VM - def test_acquire_token_for_client_should_hit_regional_endpoint(self): + def _test_acquire_token_for_client(self, configured_region, expected_region): """This is the only grant supported by regional endpoint, for now""" self.app = get_lab_app( # Regional endpoint only supports confidential client @@ -799,8 +799,7 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self): #authority="https://westus.login.microsoft.com/microsoft.onmicrosoft.com", #validate_authority=False, authority="https://login.microsoftonline.com/microsoft.onmicrosoft.com", - azure_region=self.region, # Explicitly use this region, regardless of detection - + azure_region=configured_region, timeout=2, # Short timeout makes this test case responsive on non-VM ) scopes = ["https://graph.microsoft.com/.default"] @@ -809,9 +808,11 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self): self.app.http_client, "post", return_value=MinimalResponse( status_code=400, text='{"error": "mock"}')) as mocked_method: self.app.acquire_token_for_client(scopes) + expected_host = '{}.r.login.microsoftonline.com'.format( + expected_region) if expected_region else 'login.microsoftonline.com' mocked_method.assert_called_with( - 'https://westus.r.login.microsoftonline.com/{}/oauth2/v2.0/token'.format( - self.app.authority.tenant), + 'https://{}/{}/oauth2/v2.0/token'.format( + expected_host, self.app.authority.tenant), params=ANY, data=ANY, headers=ANY) result = self.app.acquire_token_for_client( scopes, @@ -820,6 +821,29 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self): self.assertIn('access_token', result) self.assertCacheWorksForApp(result, scopes) + def test_acquire_token_for_client_should_hit_global_endpoint_by_default(self): + self._test_acquire_token_for_client(None, None) + + def test_acquire_token_for_client_should_ignore_env_var_by_default(self): + os.environ["REGION_NAME"] = "eastus" + self._test_acquire_token_for_client(None, None) + del os.environ["REGION_NAME"] + + def test_acquire_token_for_client_should_use_a_specified_region(self): + self._test_acquire_token_for_client("westus", "westus") + + def test_acquire_token_for_client_should_use_an_env_var_with_short_region_name(self): + os.environ["REGION_NAME"] = "eastus" + self._test_acquire_token_for_client( + msal.ConfidentialClientApplication.ATTEMPT_REGION_DISCOVERY, "eastus") + del os.environ["REGION_NAME"] + + def test_acquire_token_for_client_should_use_an_env_var_with_long_region_name(self): + os.environ["REGION_NAME"] = "East Us 2" + self._test_acquire_token_for_client( + msal.ConfidentialClientApplication.ATTEMPT_REGION_DISCOVERY, "eastus2") + del os.environ["REGION_NAME"] + @unittest.skipUnless( os.getenv("LAB_OBO_CLIENT_SECRET"), "Need LAB_OBO_CLIENT_SECRET from https://aka.ms/GetLabSecret?Secret=TodoListServiceV2-OBO")