Skip to content
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.

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

Define some Cloud Instance constants and the usage pattern of using them #433

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Nov 2, 2021

The new usage pattern is documented in our doc staging site (please search the keyword "authority (str)") in this page.

You can also try the new syntax by pulling its feature branch:

pip install git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@cloud-instances

This is not a breaking change. The old school, low-level url string input is still supported.

This PR, once agreed upon and merged in, will resolve #221.

@rayluo rayluo force-pushed the cloud-instances branch 2 times, most recently from 6794b76 to f6f5cf5 Compare November 4, 2021 21:42
https://login.microsoftonline.com/your_tenant
By default, we will use https://login.microsoftonline.com/common
``https://login.microsoftonline.com/your_tenant``
By default, we will use ``https://login.microsoftonline.com/common``

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, we will use https://login.microsoftonline.com/common

for CCA it should be tenanted and common should not be allowed?

Copy link
Collaborator Author

@rayluo rayluo Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true that AAD recommend to avoid "common" for confidential client, yet at the same time they can't really block such a usage due to backward compatibility concern.

We are essentially in the same boat here. Historically the input parameter authority is optional and defaults to that ".../common" value. We have, however, long been updated our samples to guide confidential app developers to use their tenant. See here and there. In practice, few people would read our API Reference doc (sad but true :-) ) but they will typically start from our samples. So, I think we are good.

UPDATE: Got some clarification that the "common" is not recommended for acquire_token_for_client(). We will emit a warning for that, in a separate PR #435.

@henrik-me henrik-me requested a review from trwalke November 4, 2021 23:42
Reduce duplicated magic strings

Add test cases

Writing docs
@rayluo rayluo merged commit e642b92 into dev Nov 9, 2021
@rayluo rayluo deleted the cloud-instances branch November 9, 2021 04:07
@rayluo rayluo mentioned this pull request Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add public convenience string constants for endpoints of each cloud (sovereign and public)
2 participants