AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
757 stars 192 forks source link

Add support to use "TryAutoDetect" to enable auto detect #526

Closed yingxumsft closed 1 year ago

yingxumsft commented 1 year ago

Describe the bug Looks like we can only use True (boolean) to enable auto detect regional endpoint, rather than using "TryAutoDetect": ATTEMPT_REGION_DISCOVERY = True # "TryAutoDetect"

However, Azure.Identity library is using environment variable AZURE_REGIONAL_AUTHORITY_NAME ("TryAutoDetect"), which can not be set to true. https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/identity/azure-identity/azure/identity/_internal/msal_credentials.py

Thus we cannot use auto detect with Azure.Identity library.

To Reproduce Steps to reproduce the behavior - run following code in local machine, which trying to auto detect but it will fail as local machine is not AzureVM.

region = os.getenv("AZURE_REGIONAL_AUTHORITY_NAME") if region is None or len(region) == 0: print("set region to auto detect") os.environ["AZURE_REGIONAL_AUTHORITY_NAME"] = "TryAutoDetect"

certificate_path = "your_local_cert.pfx" certificate_credential = CertificateCredential(tenant_id='your tenant id', client_id='your client id', certificate_path=certificate_path, password="your password", send_certificate_chain=True) token = certificate_credential.get_token('https://vault.azure.net/.default') print(token)

Expected behavior token should be acquired successfully.

What you see instead CertificateCredential.get_token failed: Authentication failed: <urllib3.connection.HTTPSConnection object at 0x000001CAF0AE0D88>: Failed to establish a new connection: [Errno 11001] getaddrinfo failed

The MSAL Python version you are using 1.20.0

Additional context Add any other context about the problem here.

rayluo commented 1 year ago
os.environ["AZURE_REGIONAL_AUTHORITY_NAME"] = "TryAutoDetect"
...
certificate_credential = CertificateCredential(...)

So, you were using the Azure Identity library (which is part of the Azure SDK for Python). The Azure Identity is built on top of MSAL Python, but their calling patterns are different.

xiangyan99 commented 1 year ago

Ray is right. You can use RegionalAuthority.AUTO_DISCOVER_REGION https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/identity/azure-identity/azure/identity/_enums.py#L15 in AZURE_REGIONAL_AUTHORITY_NAME to opt in.

yingxumsft commented 1 year ago

hi @xiangyan99 , thanks for the info. I tried to set that with following code: os.environ[EnvironmentVariables.AZURE_REGIONAL_AUTHORITY_NAME] = RegionalAuthority.AUTO_DISCOVER_REGION.value

On my local machine, the auto discover will return None region as expected, but the problem is MSAL think the configured region is 'True', thus it will use the 'True.r.login.microsoftonline.com' as the regional host in this code: https://github.com/AzureAD/microsoft-authentication-library-for-python/blob/dev/msal/application.py#:~:text=region_to_use%20%3D%20(,e.%20opted%20out

The reason I think is because Identiy library is passing 'True' as string to for azure_region in MSAL application, while MSAL application expect azure_region is boolean True to opt in auto discovery. For string 'True', MSAL application think it is same as any other configured region like 'westus2'.

image

xiangyan99 commented 1 year ago

Looks like it is by design that ATTEMPT_REGION_DISCOVERY = True # "TryAutoDetect"?

https://github.com/AzureAD/microsoft-authentication-library-for-python/blob/dev/msal/application.py#L164

@rayluo ?

rayluo commented 1 year ago

Looks like it is by design that ATTEMPT_REGION_DISCOVERY = True # "TryAutoDetect"?

There is some history behind that line. MSALs accept either a string region name, or a special flag to represent auto-detect. MSAL .Net chose to use a magic string "TryAutoDetect" for the latter. MSAL Python tried that during prototyping, but eventually switched to a non-string value approach (so that all strings are considered a valid region name).

But if we are talking about the design, then, by design, none of implementation details above really matters in MSAL Python, because MSAL Python only documents its auto-detect API as "use a special keyword ClientApplication.ATTEMPT_REGION_DISCOVERY", even without disclosing what value that constant actually is. And MSAL Python may even change that constant's value in the future, and that shall not be considered as a breaking change. (Now that the fact that we are having this conversation, makes me seriously considering to change MSAL Python to use "sentinel object pattern" ClientApplication.ATTEMPT_REGION_DISCOVERY = object() in upcoming version 1.21, so that there will be zero chance for other downstream apps to run into this same issue again. Are you OK with that, @xiangyan99 ?)

rayluo commented 1 year ago

Conclusion: The issue reported here is actually for MSAL's downstream library, the Azure Identity package, and the fix is provided there.

MSAL itself does not have to change the magic value, as long as the caller will stick with MSAL's documented constant's name ConfidentialClientApplication.ATTEMPT_REGION_DISCOVERY.