Skip to content

Commit de802a5

Browse files
ellismgellismg
authored
Do not require AZURE_USERNAME for shared cache (#8095)
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
1 parent 3434e38 commit de802a5

8 files changed

Lines changed: 103 additions & 112 deletions

File tree

‎sdk/identity/azure-identity/azure/identity/_authn_client.py‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818
NetworkTraceLoggingPolicy,
1919
ProxyPolicy,
2020
RetryPolicy,
21-
DistributedTracingPolicy
21+
DistributedTracingPolicy,
2222
)
2323
from azure.core.pipeline.transport import RequestsTransport, HttpRequest
24-
from azure.identity._constants import AZURE_CLI_CLIENT_ID, KnownAuthorities
24+
from ._constants import AZURE_CLI_CLIENT_ID, KnownAuthorities
2525

2626
try:
2727
ABC = abc.ABC
@@ -98,6 +98,7 @@ def request_token(self, scopes, method, headers, form_data, params, **kwargs):
9898

9999
@abc.abstractmethod
100100
def obtain_token_by_refresh_token(self, scopes, username):
101+
# type: (Iterable[str], Optional[str]) -> AccessToken
101102
pass
102103

103104
def _deserialize_and_cache_token(self, response, scopes, request_time):
@@ -214,22 +215,45 @@ def request_token(
214215
token = self._deserialize_and_cache_token(response=response, scopes=scopes, request_time=request_time)
215216
return token
216217

217-
def obtain_token_by_refresh_token(self, scopes, username):
218-
# type: (Iterable[str], str) -> Optional[AccessToken]
219-
"""Acquire an access token using a cached refresh token. Returns ``None`` when that fails, or the cache has no
220-
refresh token. This is only used by SharedTokenCacheCredential and isn't robust enough for anything else."""
218+
def obtain_token_by_refresh_token(self, scopes, username=None):
219+
# type: (Iterable[str], Optional[str]) -> AccessToken
220+
"""Acquire an access token using a cached refresh token. Raises ClientAuthenticationError if that fails.
221+
This is only used by SharedTokenCacheCredential and isn't robust enough for anything else."""
222+
223+
# if an username is provided, restrict our search to accounts that have that username
224+
query = {"username": username} if username else {}
225+
accounts = self._cache.find(TokenCache.CredentialType.ACCOUNT, query=query)
226+
227+
# if more than one account was returned, ensure that that they all have the same home_account_id. If so,
228+
# we'll treat them as equal, otherwise we can't know which one to pick, so we'll raise an error.
229+
if len(accounts) > 1 and len({account.get("home_account_id") for account in accounts}) != 1:
230+
if username:
231+
message = (
232+
"Multiple entries found for user '{}' were found in the shared token cache. "
233+
"This is not currently supported by SharedTokenCacheCredential."
234+
).format(username)
235+
else:
236+
# TODO: we could identify usernames associated with exactly one home account id
237+
message = (
238+
"Multiple users were discovered in the shared token cache. If using DefaultAzureCredential, set "
239+
"the AZURE_USERNAME environment variable to the preferred username. Otherwise, specify it when "
240+
"constructing SharedTokenCacheCredential."
241+
"\nDiscovered accounts: {}"
242+
).format(", ".join({account.get("username") for account in accounts}))
243+
raise ClientAuthenticationError(message=message)
221244

222-
# find account matching username
223-
accounts = self._cache.find(TokenCache.CredentialType.ACCOUNT, query={"username": username})
224245
for account in accounts:
225-
# try each refresh token that might work, return the first access token acquired
226-
for token in self.get_refresh_tokens(scopes, account):
227-
# currently we only support login.microsoftonline.com, which has an alias login.windows.net
228-
# TODO: this must change to support sovereign clouds
229-
environment = account.get("environment")
230-
if not environment or (environment not in self._auth_url and environment != "login.windows.net"):
246+
# ensure the account is associated with the token authority we expect to use
247+
# ('environment' is an authority e.g. 'login.microsoftonline.com')
248+
environment = account.get("environment")
249+
if not environment or environment not in self._auth_url:
250+
# doubtful this account can get the access token we want but public cloud's a special case
251+
# because its authority has an alias: for our purposes login.windows.net = login.microsoftonline.com
252+
if not (environment == "login.windows.net" and KnownAuthorities.AZURE_PUBLIC_CLOUD in self._auth_url):
231253
continue
232254

255+
# try each refresh token, returning the first access token acquired
256+
for token in self.get_refresh_tokens(scopes, account):
233257
request = self.get_refresh_token_grant_request(token, scopes)
234258
request_time = int(time.time())
235259
response = self._pipeline.run(request, stream=False)
@@ -240,7 +264,11 @@ def obtain_token_by_refresh_token(self, scopes, username):
240264
except ClientAuthenticationError:
241265
continue
242266

243-
return None
267+
message = "No cached token found"
268+
if username:
269+
message += " for '{}'".format(username)
270+
271+
raise ClientAuthenticationError(message=message)
244272

245273
@staticmethod
246274
def _create_config(**kwargs):

‎sdk/identity/azure-identity/azure/identity/_credentials/default.py‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class DefaultAzureCredential(ChainedTokenCredential):
2020
1. A service principal configured by environment variables. See :class:`~azure.identity.EnvironmentCredential` for
2121
more details.
2222
2. An Azure managed identity. See :class:`~azure.identity.ManagedIdentityCredential` for more details.
23-
3. On Windows only: a user who has signed in with a Microsoft application, such as Visual Studio. This requires a
24-
value for the environment variable ``AZURE_USERNAME``. See :class:`~azure.identity.SharedTokenCacheCredential`
25-
for more details.
23+
3. On Windows only: a user who has signed in with a Microsoft application, such as Visual Studio. If multiple
24+
identities are in the cache, then the value of the environment variable ``AZURE_USERNAME`` is used to select
25+
which identity to use. See :class:`~azure.identity.SharedTokenCacheCredential` for more details.
2626
2727
Keyword arguments
2828
- **authority** (str): Authority of an Azure Active Directory endpoint, for example 'login.microsoftonline.com',
@@ -34,15 +34,8 @@ def __init__(self, **kwargs):
3434
authority = kwargs.pop("authority", None)
3535
credentials = [EnvironmentCredential(authority=authority, **kwargs), ManagedIdentityCredential(**kwargs)]
3636

37-
# SharedTokenCacheCredential is part of the default only on supported platforms, when $AZURE_USERNAME has a
38-
# value (because the cache may contain tokens for multiple identities and we can only choose one arbitrarily
39-
# without more information from the user), and when $AZURE_PASSWORD has no value (because when $AZURE_USERNAME
40-
# and $AZURE_PASSWORD are set, EnvironmentCredential will be used instead)
41-
if (
42-
SharedTokenCacheCredential.supported()
43-
and EnvironmentVariables.AZURE_USERNAME in os.environ
44-
and EnvironmentVariables.AZURE_PASSWORD not in os.environ
45-
):
37+
# SharedTokenCacheCredential is part of the default only on supported platforms.
38+
if SharedTokenCacheCredential.supported():
4639
credentials.append(
4740
SharedTokenCacheCredential(
4841
username=os.environ.get(EnvironmentVariables.AZURE_USERNAME), authority=authority, **kwargs

‎sdk/identity/azure-identity/azure/identity/_credentials/user.py‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ class SharedTokenCacheCredential(object):
123123
defines authorities for other clouds.
124124
"""
125125

126-
def __init__(self, username, **kwargs): # pylint:disable=unused-argument
127-
# type: (str, **Any) -> None
126+
def __init__(self, username=None, **kwargs): # pylint:disable=unused-argument
127+
# type: (Optional[str], **Any) -> None
128128

129129
self._username = username
130130

@@ -161,11 +161,7 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument
161161
if not self._client:
162162
raise ClientAuthenticationError(message="Shared token cache unavailable")
163163

164-
token = self._client.obtain_token_by_refresh_token(scopes, self._username)
165-
if not token:
166-
raise ClientAuthenticationError(message="No cached token found for '{}'".format(self._username))
167-
168-
return token
164+
return self._client.obtain_token_by_refresh_token(scopes, self._username)
169165

170166
@staticmethod
171167
def supported():

‎sdk/identity/azure-identity/azure/identity/aio/_authn_client.py‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from azure.core.pipeline.transport import AioHttpTransport
2222

2323
from .._authn_client import AuthnClientBase
24+
from .._constants import KnownAuthorities
2425

2526
if TYPE_CHECKING:
2627
from typing import Any, Dict, Iterable, Mapping, Optional
@@ -67,21 +68,46 @@ async def request_token(
6768
token = self._deserialize_and_cache_token(response=response, scopes=scopes, request_time=request_time)
6869
return token
6970

70-
async def obtain_token_by_refresh_token(self, scopes: "Iterable[str]", username: str) -> "Optional[AccessToken]":
71-
"""Acquire an access token using a cached refresh token. Returns ``None`` when that fails, or the cache has no
72-
refresh token. This is only used by SharedTokenCacheCredential and isn't robust enough for anything else."""
71+
async def obtain_token_by_refresh_token(
72+
self, scopes: "Iterable[str]", username: "Optional[str]" = None
73+
) -> "AccessToken":
74+
"""Acquire an access token using a cached refresh token. Raises ClientAuthenticationError if that fails.
75+
This is only used by SharedTokenCacheCredential and isn't robust enough for anything else."""
76+
77+
# if an username is provided, restrict our search to accounts that have that username
78+
query = {"username": username} if username else {}
79+
accounts = self._cache.find(TokenCache.CredentialType.ACCOUNT, query=query)
80+
81+
# if more than one account was returned, ensure that that they all have the same home_account_id. If so,
82+
# we'll treat them as equal, otherwise we can't know which one to pick, so we'll raise an error.
83+
if len(accounts) > 1 and len({account.get("home_account_id") for account in accounts}) != 1:
84+
if username:
85+
message = (
86+
"Multiple entries found for user '{}' were found in the shared token cache. "
87+
"This is not currently supported by SharedTokenCacheCredential"
88+
).format(username)
89+
else:
90+
# TODO: we could identify usernames associated with exactly one home account id
91+
message = (
92+
"Multiple users were discovered in the shared token cache. If using DefaultAzureCredential, set "
93+
"the AZURE_USERNAME environment variable to the preferred username. Otherwise, specify it when "
94+
"constructing SharedTokenCacheCredential."
95+
"\nDiscovered accounts: {}"
96+
).format(", ".join({account.get("username") for account in accounts}))
97+
raise ClientAuthenticationError(message=message)
7398

74-
# find account matching username
75-
accounts = self._cache.find(TokenCache.CredentialType.ACCOUNT, query={"username": username})
7699
for account in accounts:
77-
# try each refresh token that might work, return the first access token acquired
78-
for token in self.get_refresh_tokens(scopes, account):
79-
# currently we only support login.microsoftonline.com, which has an alias login.windows.net
80-
# TODO: this must change to support sovereign clouds
81-
environment = account.get("environment")
82-
if not environment or (environment not in self._auth_url and environment != "login.windows.net"):
100+
# ensure the account is associated with the token authority we expect to use
101+
# ('environment' is an authority e.g. 'login.microsoftonline.com')
102+
environment = account.get("environment")
103+
if not environment or environment not in self._auth_url:
104+
# doubtful this account can get the access token we want but public cloud's a special case
105+
# because its authority has an alias: for our purposes login.windows.net = login.microsoftonline.com
106+
if not (environment == "login.windows.net" and KnownAuthorities.AZURE_PUBLIC_CLOUD in self._auth_url):
83107
continue
84108

109+
# try each refresh token, returning the first access token acquired
110+
for token in self.get_refresh_tokens(scopes, account):
85111
request = self.get_refresh_token_grant_request(token, scopes)
86112
request_time = int(time.time())
87113
response = await self._pipeline.run(request, stream=False)
@@ -92,7 +118,11 @@ async def obtain_token_by_refresh_token(self, scopes: "Iterable[str]", username:
92118
except ClientAuthenticationError:
93119
continue
94120

95-
return None
121+
message = "No cached token found"
122+
if username:
123+
message += " for '{}'".format(username)
124+
125+
raise ClientAuthenticationError(message=message)
96126

97127
@staticmethod
98128
def _create_config(**kwargs: "Any") -> Configuration:

‎sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class DefaultAzureCredential(ChainedTokenCredential):
2020
1. A service principal configured by environment variables. See :class:`~azure.identity.aio.EnvironmentCredential`
2121
for more details.
2222
2. An Azure managed identity. See :class:`~azure.identity.aio.ManagedIdentityCredential` for more details.
23-
3. On Windows only: a user who has signed in with a Microsoft application, such as Visual Studio. This requires a
24-
value for the environment variable ``AZURE_USERNAME``. See
25-
:class:`~azure.identity.aio.SharedTokenCacheCredential` for more details.
23+
3. On Windows only: a user who has signed in with a Microsoft application, such as Visual Studio. If multiple
24+
identities are in the cache, then the value of the environment variable ``AZURE_USERNAME`` is used to select
25+
which identity to use. See :class:`~azure.identity.aio.SharedTokenCacheCredential` for more details.
2626
2727
Keyword arguments
2828
- **authority** (str): Authority of an Azure Active Directory endpoint, for example 'login.microsoftonline.com',
@@ -34,15 +34,8 @@ def __init__(self, **kwargs):
3434
authority = kwargs.pop("authority", None)
3535
credentials = [EnvironmentCredential(authority=authority, **kwargs), ManagedIdentityCredential(**kwargs)]
3636

37-
# SharedTokenCacheCredential is part of the default only on supported platforms, when $AZURE_USERNAME has a
38-
# value (because the cache may contain tokens for multiple identities and we can only choose one arbitrarily
39-
# without more information from the user), and when $AZURE_PASSWORD has no value (because when $AZURE_USERNAME
40-
# and $AZURE_PASSWORD are set, EnvironmentCredential will be used instead)
41-
if (
42-
SharedTokenCacheCredential.supported()
43-
and EnvironmentVariables.AZURE_USERNAME in os.environ
44-
and EnvironmentVariables.AZURE_PASSWORD not in os.environ
45-
):
37+
# SharedTokenCacheCredential is part of the default only on supported platforms.
38+
if SharedTokenCacheCredential.supported():
4639
credentials.append(
4740
SharedTokenCacheCredential(
4841
username=os.environ.get(EnvironmentVariables.AZURE_USERNAME), authority=authority, **kwargs

‎sdk/identity/azure-identity/azure/identity/aio/_credentials/user.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,7 @@ async def get_token(self, *scopes: str, **kwargs: "Any") -> "AccessToken": # py
4545
if not self._client:
4646
raise ClientAuthenticationError(message="Shared token cache unavailable")
4747

48-
token = await self._client.obtain_token_by_refresh_token(scopes, self._username)
49-
if not token:
50-
raise ClientAuthenticationError(message="No cached token found for '{}'".format(self._username))
51-
52-
return token
48+
return await self._client.obtain_token_by_refresh_token(scopes, self._username)
5349

5450
@staticmethod
5551
def _get_auth_client(cache: "msal_extensions.FileTokenCache") -> "AuthnClientBase":

‎sdk/identity/azure-identity/tests/test_identity.py‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -247,37 +247,14 @@ def test_default_credential_shared_cache_use(mock_credential):
247247
assert mock_credential.supported.call_count == 1
248248
mock_credential.supported.reset_mock()
249249

250-
# unsupported platform, $AZURE_USERNAME set, $AZURE_PASSWORD not set -> default credential shouldn't use shared cache
251-
credential = DefaultAzureCredential()
252-
assert mock_credential.call_count == 0
253-
assert mock_credential.supported.call_count == 1
254-
255250
mock_credential.supported = Mock(return_value=True)
256251

257-
# supported platform, $AZURE_USERNAME not set -> default credential shouldn't use shared cache
252+
# supported platform -> default credential should use shared cache
258253
credential = DefaultAzureCredential()
259-
assert mock_credential.call_count == 0
254+
assert mock_credential.call_count == 1
260255
assert mock_credential.supported.call_count == 1
261256
mock_credential.supported.reset_mock()
262257

263-
# supported platform, $AZURE_USERNAME and $AZURE_PASSWORD set -> default credential shouldn't use shared cache
264-
# (EnvironmentCredential should be used when both variables are set)
265-
with patch.dict("os.environ", {"AZURE_USERNAME": "foo@bar.com", "AZURE_PASSWORD": "***"}):
266-
credential = DefaultAzureCredential()
267-
assert mock_credential.call_count == 0
268-
269-
# supported platform, $AZURE_USERNAME set, $AZURE_PASSWORD not set -> default credential should use shared cache
270-
with patch.dict("os.environ", {"AZURE_USERNAME": "foo@bar.com"}):
271-
expected_token = AccessToken("***", 42)
272-
mock_credential.return_value = Mock(get_token=lambda *_: expected_token)
273-
274-
credential = DefaultAzureCredential()
275-
assert mock_credential.call_count == 1
276-
277-
token = credential.get_token("scope")
278-
assert token == expected_token
279-
280-
281258
def test_device_code_credential():
282259
expected_token = "access-token"
283260
user_code = "user-code"

‎sdk/identity/azure-identity/tests/test_identity_async.py‎

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -296,32 +296,10 @@ async def test_default_credential_shared_cache_use():
296296
assert mock_credential.supported.call_count == 1
297297
mock_credential.supported.reset_mock()
298298

299-
# unsupported platform, $AZURE_USERNAME set, $AZURE_PASSWORD not set -> default credential shouldn't use shared cache
300-
credential = DefaultAzureCredential()
301-
assert mock_credential.call_count == 0
302-
assert mock_credential.supported.call_count == 1
303-
304299
mock_credential.supported = Mock(return_value=True)
305300

306-
# supported platform, $AZURE_USERNAME not set -> default credential shouldn't use shared cache
301+
# supported platform -> default credential should use shared cache
307302
credential = DefaultAzureCredential()
308-
assert mock_credential.call_count == 0
303+
assert mock_credential.call_count == 1
309304
assert mock_credential.supported.call_count == 1
310305
mock_credential.supported.reset_mock()
311-
312-
# supported platform, $AZURE_USERNAME and $AZURE_PASSWORD set -> default credential shouldn't use shared cache
313-
# (EnvironmentCredential should be used when both variables are set)
314-
with patch.dict("os.environ", {"AZURE_USERNAME": "foo@bar.com", "AZURE_PASSWORD": "***"}):
315-
credential = DefaultAzureCredential()
316-
assert mock_credential.call_count == 0
317-
318-
# supported platform, $AZURE_USERNAME set, $AZURE_PASSWORD not set -> default credential should use shared cache
319-
with patch.dict("os.environ", {"AZURE_USERNAME": "foo@bar.com"}):
320-
expected_token = AccessToken("***", 42)
321-
mock_credential.return_value = Mock(get_token=asyncio.coroutine(lambda *_: expected_token))
322-
323-
credential = DefaultAzureCredential()
324-
assert mock_credential.call_count == 1
325-
326-
token = await credential.get_token("scope")
327-
assert token == expected_token

0 commit comments

Comments
 (0)
⚡