Skip to content

Changing region endpoint, and remove usage of REGION_NAME env var #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 10, 2021
Merged

Changing region endpoint, and remove usage of REGION_NAME env var #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71802d0
Change regional endpoint doname name
rayluo Aug 10, 2021
96140b0
Regional endpoint test cases do not rely on env var REGION_NAME
rayluo Aug 10, 2021
5fdae2d
REGION_NAME has no unified format across services
rayluo Aug 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion msal/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ def _get_regional_authority(self, central_authority):
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)
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
"login.microsoftonline.com",
Expand Down
9 changes: 2 additions & 7 deletions msal/region.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,9 @@


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:
if http_client:
return _detect_region_of_azure_vm(http_client) # It could hang for minutes
return region


def _detect_region_of_azure_function():
return os.environ.get("REGION_NAME")
return None


def _detect_region_of_azure_vm(http_client):
Expand Down
42 changes: 26 additions & 16 deletions tests/test_e2e.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,15 @@ def assertCacheWorksForApp(self, result_from_wire, scope):
def _test_username_password(self,
authority=None, client_id=None, username=None, password=None, scope=None,
client_secret=None, # Since MSAL 1.11, confidential client has ROPC too
azure_region=None,
http_client=None,
**ignored):
assert authority and client_id and username and password and scope
self.app = msal.ClientApplication(
client_id, authority=authority, http_client=MinimalHttpClient(),
client_id, authority=authority,
http_client=http_client or MinimalHttpClient(),
azure_region=azure_region, # Regional endpoint does not support ROPC.
# Here we just use it to test a regional app won't break ROPC.
client_credential=client_secret)
result = self.app.acquire_token_by_username_password(
username, password, scopes=scope)
Expand Down Expand Up @@ -541,11 +546,16 @@ def _test_acquire_token_by_auth_code_flow(
error_description=result.get("error_description")))
self.assertCacheWorksForUser(result, scope, username=None)

def _test_acquire_token_obo(self, config_pca, config_cca):
def _test_acquire_token_obo(self, config_pca, config_cca,
azure_region=None, # Regional endpoint does not really support OBO.
# Here we just test regional apps won't adversely break OBO
http_client=None,
):
# 1. An app obtains a token representing a user, for our mid-tier service
pca = msal.PublicClientApplication(
config_pca["client_id"], authority=config_pca["authority"],
http_client=MinimalHttpClient())
azure_region=azure_region,
http_client=http_client or MinimalHttpClient())
pca_result = pca.acquire_token_by_username_password(
config_pca["username"],
config_pca["password"],
Expand All @@ -560,7 +570,8 @@ 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(),
azure_region=azure_region,
http_client=http_client or 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.
Expand Down Expand Up @@ -778,6 +789,7 @@ def test_b2c_acquire_token_by_ropc(self):

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):
"""This is the only grant supported by regional endpoint, for now"""
Expand All @@ -798,7 +810,7 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self):
status_code=400, text='{"error": "mock"}')) as mocked_method:
self.app.acquire_token_for_client(scopes)
mocked_method.assert_called_with(
'https://westus.login.microsoft.com/{}/oauth2/v2.0/token'.format(
'https://westus.r.login.microsoftonline.com/{}/oauth2/v2.0/token'.format(
self.app.authority.tenant),
params=ANY, data=ANY, headers=ANY)
result = self.app.acquire_token_for_client(
Expand All @@ -808,15 +820,6 @@ def test_acquire_token_for_client_should_hit_regional_endpoint(self):
self.assertIn('access_token', result)
self.assertCacheWorksForApp(result, scopes)


class RegionalEndpointViaEnvVarTestCase(WorldWideRegionalEndpointTestCase):

def setUp(self):
os.environ["REGION_NAME"] = "eastus"

def tearDown(self):
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")
Expand All @@ -842,7 +845,11 @@ def test_cca_obo_should_bypass_regional_endpoint_therefore_still_work(self):
config_pca["password"] = self.get_lab_user_secret(config_pca["lab_name"])
config_pca["scope"] = ["api://%s/read" % config_cca["client_id"]]

self._test_acquire_token_obo(config_pca, config_cca)
self._test_acquire_token_obo(
config_pca, config_cca,
azure_region=self.region,
http_client=MinimalHttpClient(timeout=self.timeout),
)

@unittest.skipUnless(
os.getenv("LAB_OBO_CLIENT_SECRET"),
Expand All @@ -859,7 +866,10 @@ def test_cca_ropc_should_bypass_regional_endpoint_therefore_still_work(self):
config["client_id"] = os.getenv("LAB_OBO_CONFIDENTIAL_CLIENT_ID")
config["scope"] = ["https://graph.microsoft.com/.default"]
config["client_secret"] = os.getenv("LAB_OBO_CLIENT_SECRET")
self._test_username_password(**config)
self._test_username_password(
azure_region=self.region,
http_client=MinimalHttpClient(timeout=self.timeout),
**config)


class ArlingtonCloudTestCase(LabBasedTestCase):
Expand Down