Skip to content

Commit 0366ac2

Browse files
pvaneckpvaneck
authored
[Identity] Fix issue with cache option usage (#27030)
If a user supplies `TokenCachePersistenceOptions` to a `SharedTokenCacheCredential`, these options are not used when the cache is loaded. This can lead to issues when users are trying to use caches with custom names since the default name is used instead. This commit ensures that user-provided cache options are propagated when the cache is loaded. Ref: #26982 Signed-off-by: Paul Van Eck <[email protected]>
1 parent 4c9b464 commit 0366ac2

File tree

5 files changed

+49
-13
lines changed

5 files changed

+49
-13
lines changed

‎sdk/identity/azure-identity/CHANGELOG.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
### Bugs Fixed
1010

11+
- Fixed issue where user-supplied `TokenCachePersistenceOptions` weren't propagated when using `SharedTokenCacheCredential` ([#26982](https://github.com/Azure/azure-sdk-for-python/issues/26982))
12+
1113
### Other Changes
1214

1315
## 1.12.0b2 (2022-10-11)
@@ -133,7 +135,7 @@ Azure-identity is supported on Python 3.7 or later. For more details, please rea
133135
### Bugs Fixed
134136

135137
- Handle injected "tenant_id" and "claims" ([#23138](https://github.com/Azure/azure-sdk-for-python/issues/23138))
136-
138+
137139
"tenant_id" argument in get_token() method is only supported by:
138140

139141
- `AuthorizationCodeCredential`
@@ -163,7 +165,7 @@ Azure-identity is supported on Python 3.7 or later. For more details, please rea
163165
> Only code written against a beta version such as 1.7.0b1 may be affected.
164166
165167
- The `allow_multitenant_authentication` argument has been removed and the default behavior is now as if it were true.
166-
The multitenant authentication feature can be totally disabled by setting the environment variable
168+
The multitenant authentication feature can be totally disabled by setting the environment variable
167169
`AZURE_IDENTITY_DISABLE_MULTITENANTAUTH` to `True`.
168170
- `azure.identity.RegionalAuthority` is removed.
169171
- `regional_authority` argument is removed for `CertificateCredential` and `ClientSecretCredential`.

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def __init__(self, authentication_record, **kwargs):
3636
self._tenant_id = kwargs.pop("tenant_id", None) or self._auth_record.tenant_id
3737
validate_tenant_id(self._tenant_id)
3838
self._cache = kwargs.pop("_cache", None)
39+
self._cache_persistence_options = kwargs.pop("cache_persistence_options", None)
3940
self._client_applications = {} # type: Dict[str, PublicClientApplication]
4041
self._additionally_allowed_tenants = kwargs.pop("additionally_allowed_tenants", [])
4142
self._client = MsalClient(**kwargs)
@@ -64,10 +65,13 @@ def get_token(self, *scopes, **kwargs): # pylint:disable=unused-argument
6465
def _initialize(self):
6566
if not self._cache and platform.system() in {"Darwin", "Linux", "Windows"}:
6667
try:
67-
# This credential accepts the user's default cache regardless of whether it's encrypted. It doesn't
68-
# create a new cache. If the default cache exists, the user must have created it earlier. If it's
69-
# unencrypted, the user must have allowed that.
70-
self._cache = _load_persistent_cache(TokenCachePersistenceOptions(allow_unencrypted_storage=True))
68+
# If no cache options were provided, the default cache will be used. This credential accepts the
69+
# user's default cache regardless of whether it's encrypted. It doesn't create a new cache. If the
70+
# default cache exists, the user must have created it earlier. If it's unencrypted, the user must
71+
# have allowed that.
72+
options = self._cache_persistence_options or \
73+
TokenCachePersistenceOptions(allow_unencrypted_storage=True)
74+
self._cache = _load_persistent_cache(options)
7175
except Exception: # pylint:disable=broad-except
7276
pass
7377

‎sdk/identity/azure-identity/azure/identity/_internal/shared_token_cache.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ def __init__(self, username=None, **kwargs): # pylint:disable=unused-argument
9595
self._username = username
9696
self._tenant_id = kwargs.pop("tenant_id", None)
9797
self._cache = kwargs.pop("_cache", None)
98+
self._cache_persistence_options = kwargs.pop("cache_persistence_options", None)
9899
self._client = None # type: Optional[AadClientBase]
99100
self._client_kwargs = kwargs
100101
self._client_kwargs["tenant_id"] = "organizations"
@@ -116,10 +117,13 @@ def _initialize(self):
116117
def _load_cache(self):
117118
if not self._cache and self.supported():
118119
try:
119-
# This credential accepts the user's default cache regardless of whether it's encrypted. It doesn't
120-
# create a new cache. If the default cache exists, the user must have created it earlier. If it's
121-
# unencrypted, the user must have allowed that.
122-
self._cache = _load_persistent_cache(TokenCachePersistenceOptions(allow_unencrypted_storage=True))
120+
# If no cache options were provided, the default cache will be used. This credential accepts the
121+
# user's default cache regardless of whether it's encrypted. It doesn't create a new cache. If the
122+
# default cache exists, the user must have created it earlier. If it's unencrypted, the user must
123+
# have allowed that.
124+
options = self._cache_persistence_options or \
125+
TokenCachePersistenceOptions(allow_unencrypted_storage=True)
126+
self._cache = _load_persistent_cache(options)
123127
except Exception: # pylint:disable=broad-except
124128
pass
125129

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
AzureAuthorityHosts,
1010
CredentialUnavailableError,
1111
SharedTokenCacheCredential,
12+
TokenCachePersistenceOptions,
1213
)
1314
from azure.identity._constants import DEVELOPER_SIGN_ON_CLIENT_ID, EnvironmentVariables
1415
from azure.identity._internal.shared_token_cache import (
@@ -764,6 +765,18 @@ def test_initialization():
764765
assert mock_cache_loader.call_count == 1
765766

766767

768+
def test_initialization_with_cache_options():
769+
"""the credential should use user-supplied persistence options"""
770+
771+
with patch("azure.identity._internal.shared_token_cache._load_persistent_cache") as mock_cache_loader:
772+
options = TokenCachePersistenceOptions(name="foo.cache")
773+
credential = SharedTokenCacheCredential(cache_persistence_options=options)
774+
775+
with pytest.raises(CredentialUnavailableError):
776+
credential.get_token("scope")
777+
mock_cache_loader.assert_called_once_with(options)
778+
779+
767780
def test_authentication_record_authenticating_tenant():
768781
"""when given a record and 'tenant_id', the credential should authenticate in the latter"""
769782

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from azure.core.exceptions import ClientAuthenticationError
99
from azure.core.pipeline.policies import SansIOHTTPPolicy
10-
from azure.identity import CredentialUnavailableError
10+
from azure.identity import CredentialUnavailableError, TokenCachePersistenceOptions
1111
from azure.identity.aio import SharedTokenCacheCredential
1212
from azure.identity._constants import EnvironmentVariables
1313
from azure.identity._internal.shared_token_cache import (
@@ -68,14 +68,14 @@ async def send(*_, **__):
6868
_cache=populated_cache(get_account_event("test@user", "uid", "utid")), transport=transport
6969
)
7070

71-
# async with before initialization: credential should call aexit but not aenter
71+
# async with before initialization: credential should call __aexit__ but not __aenter__
7272
async with credential:
7373
await credential.get_token("scope")
7474

7575
assert transport.__aenter__.call_count == 0
7676
assert transport.__aexit__.call_count == 1
7777

78-
# async with after initialization: credential should call aenter and aexit
78+
# async with after initialization: credential should call __aenter__ and __aexit__
7979
async with credential:
8080
await credential.get_token("scope")
8181
assert transport.__aenter__.call_count == 1
@@ -621,6 +621,19 @@ async def test_initialization():
621621
assert mock_cache_loader.call_count == 1
622622

623623

624+
@pytest.mark.asyncio
625+
async def test_initialization_with_cache_options():
626+
"""the credential should use user-supplied persistence options"""
627+
628+
with patch("azure.identity._internal.shared_token_cache._load_persistent_cache") as mock_cache_loader:
629+
options = TokenCachePersistenceOptions(name="foo.cache")
630+
credential = SharedTokenCacheCredential(cache_persistence_options=options)
631+
632+
with pytest.raises(CredentialUnavailableError):
633+
await credential.get_token("scope")
634+
mock_cache_loader.assert_called_once_with(options)
635+
636+
624637
@pytest.mark.asyncio
625638
async def test_multitenant_authentication():
626639
first_token = "***"

0 commit comments

Comments
 (0)