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

Should we requrie AZURE_USERNAME being set to use the shared cache from DefaultAzureCredential? #7944

Closed
ellismg opened this issue Oct 16, 2019 · 1 comment · Fixed by #8095
Labels
Azure.Identity blocking-release Blocks release Client This issue points to a problem in the data-plane of the library.

Comments

@ellismg
Copy link
Member

ellismg commented Oct 16, 2019

The justification for requiring AZURE_USERNAME is that the MSAL cache may contain multiple identities, and we need to know which user you want to identify as. However, it seems like the common case is that a single identity will be in the cache and in that case we could just use that identity, even if AZURE_USERNAME was unset.

In Azure/azure-sdk-for-net#7947 we changed the .NET Version of DefaultAzureCredential to behave in a similar manner (by default, if the cache contains exactly one entry, we'll use it, and if there are multiple identities, you set the preferred username on an options bag when constructing the credential.

I am wondering if we should make similar changes here, so things work out of the box in the common case.

@loarabia loarabia added Azure.Identity Client This issue points to a problem in the data-plane of the library. labels Oct 16, 2019
@ellismg
Copy link
Member Author

ellismg commented Oct 16, 2019

FWIW, I prototyped this a little here: https://gist.github.com/ellismg/302cfc8cc6b34b5698642f512df26fa3. I'm no python expert but maybe this a reasonable starting point if we want to do this for GA.

@johanste johanste added the blocking-release Blocks release label Oct 21, 2019
ellismg added a commit to ellismg/azure-sdk-for-python that referenced this issue Oct 24, 2019
Previously, a username was required when using the
SharedTokenCacheCredential, in order to handle the case where multiple
identities were found in the cache. Since it is common to have only a
single account in your user cache (e.g. you have signed in with only a
single identity), we should allow reading from the cache even when an
explicit AZURE_USERNAME is not specified, if there is exactly one
account in the cache.

When username is unset, if we can not find a token in the cache or we
find multiple tokens, a `ClientAuthenticationError` error is raised,
with the text "No cached token found".  This is similar to how other
cache related failures are handled by the API (they raise this error
with similar text but it includes a hint about what username was used.)

As part of this work, `DefaultAzureCredential` now unconditionally uses
the shared cache on supported platforms.

This behavior matches how we handle this case in both the .NET and Java
SDKs.

Fixes Azure#7944
ellismg added a commit that referenced this issue Oct 24, 2019
Previously, a username was required when using the
SharedTokenCacheCredential, in order to handle the case where multiple
identities were found in the cache. Since it is common to have only a
single account in your user cache (e.g. you have signed in with only a
single identity), we should allow reading from the cache even when an
explicit AZURE_USERNAME is not specified, if there is exactly one
account in the cache.

When username is unset, if we can not find a token in the cache or we
find multiple tokens, a `ClientAuthenticationError` error is raised,
with the text "No cached token found".  This is similar to how other
cache related failures are handled by the API (they raise this error
with similar text but it includes a hint about what username was used.)

As part of this work, `DefaultAzureCredential` now unconditionally uses
the shared cache on supported platforms.

This behavior matches how we handle this case in both the .NET and Java
SDKs.

Fixes #7944
fengzhou-msft pushed a commit that referenced this issue Nov 5, 2019
Previously, a username was required when using the
SharedTokenCacheCredential, in order to handle the case where multiple
identities were found in the cache. Since it is common to have only a
single account in your user cache (e.g. you have signed in with only a
single identity), we should allow reading from the cache even when an
explicit AZURE_USERNAME is not specified, if there is exactly one
account in the cache.

When username is unset, if we can not find a token in the cache or we
find multiple tokens, a `ClientAuthenticationError` error is raised,
with the text "No cached token found".  This is similar to how other
cache related failures are handled by the API (they raise this error
with similar text but it includes a hint about what username was used.)

As part of this work, `DefaultAzureCredential` now unconditionally uses
the shared cache on supported platforms.

This behavior matches how we handle this case in both the .NET and Java
SDKs.

Fixes #7944
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity blocking-release Blocks release Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants