From f7fb996cbf49c8327bed46e3ca5c43585e4b10ce Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Wed, 4 Aug 2021 14:02:57 -0700 Subject: [PATCH 1/4] fix error message formatting --- .../azure-identity/azure/identity/_persistent_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_persistent_cache.py b/sdk/identity/azure-identity/azure/identity/_persistent_cache.py index f863b328740a..1969be9b923b 100644 --- a/sdk/identity/azure-identity/azure/identity/_persistent_cache.py +++ b/sdk/identity/azure-identity/azure/identity/_persistent_cache.py @@ -87,8 +87,8 @@ def _get_persistence(allow_unencrypted, account_name, cache_name): except ImportError: if not allow_unencrypted: raise ValueError( - "PyGObject is required to encrypt the persistent cache. Please install that library or ", - "specify 'allow_unencrypted_cache=True' to store the cache without encryption.", + "PyGObject is required to encrypt the persistent cache. Please install that library or " + + 'specify "allow_unencrypted_cache=True" to store the cache without encryption.' ) return msal_extensions.FilePersistence(file_path) From d0fd0809eac06d4c43c9390e26c5d1335b8b7052 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Wed, 4 Aug 2021 14:33:02 -0700 Subject: [PATCH 2/4] import msal_extensions only when necessary --- .../azure-identity/azure/identity/_persistent_cache.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sdk/identity/azure-identity/azure/identity/_persistent_cache.py b/sdk/identity/azure-identity/azure/identity/_persistent_cache.py index 1969be9b923b..ed7bb929afda 100644 --- a/sdk/identity/azure-identity/azure/identity/_persistent_cache.py +++ b/sdk/identity/azure-identity/azure/identity/_persistent_cache.py @@ -6,10 +6,9 @@ import sys from typing import TYPE_CHECKING -import msal_extensions - if TYPE_CHECKING: from typing import Any + import msal_extensions class TokenCachePersistenceOptions(object): @@ -49,8 +48,10 @@ def __init__(self, **kwargs): def _load_persistent_cache(options): # type: (TokenCachePersistenceOptions) -> msal_extensions.PersistedTokenCache + import msal_extensions + persistence = _get_persistence( - allow_unencrypted=options.allow_unencrypted_storage, account_name="MSALCache", cache_name=options.name, + allow_unencrypted=options.allow_unencrypted_storage, account_name="MSALCache", cache_name=options.name ) return msal_extensions.PersistedTokenCache(persistence) @@ -66,6 +67,7 @@ def _get_persistence(allow_unencrypted, account_name, cache_name): :param bool allow_unencrypted: when True, the cache will be kept in plaintext should encryption be impossible in the current environment """ + import msal_extensions if sys.platform.startswith("win") and "LOCALAPPDATA" in os.environ: cache_location = os.path.join(os.environ["LOCALAPPDATA"], ".IdentityService", cache_name) From cf3ab56658e7cb62772dc5d1ff3254507a801d09 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Wed, 4 Aug 2021 14:38:28 -0700 Subject: [PATCH 3/4] update test patches --- .../tests/test_certificate_credential.py | 6 +++--- .../tests/test_certificate_credential_async.py | 6 +++--- .../tests/test_client_secret_credential.py | 6 +++--- .../test_client_secret_credential_async.py | 6 +++--- .../tests/test_interactive_credential.py | 6 +++--- .../tests/test_persistent_cache.py | 17 ++++++++++------- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/sdk/identity/azure-identity/tests/test_certificate_credential.py b/sdk/identity/azure-identity/tests/test_certificate_credential.py index 38272fdb47ec..56c3c685ff3c 100644 --- a/sdk/identity/azure-identity/tests/test_certificate_credential.py +++ b/sdk/identity/azure-identity/tests/test_certificate_credential.py @@ -272,9 +272,9 @@ def validate_jwt(request, client_id, pem_bytes, expect_x5c=False): def test_token_cache(cert_path, cert_password): """the credential should optionally use a persistent cache, and default to an in memory cache""" - with patch("azure.identity._persistent_cache.msal_extensions") as mock_msal_extensions: + with patch("azure.identity._internal.msal_credentials._load_persistent_cache") as load_persistent_cache: credential = CertificateCredential("tenant", "client-id", cert_path, password=cert_password) - assert not mock_msal_extensions.PersistedTokenCache.called + assert not load_persistent_cache.called assert isinstance(credential._cache, TokenCache) CertificateCredential( @@ -284,7 +284,7 @@ def test_token_cache(cert_path, cert_password): password=cert_password, cache_persistence_options=TokenCachePersistenceOptions(), ) - assert mock_msal_extensions.PersistedTokenCache.call_count == 1 + assert load_persistent_cache.call_count == 1 @pytest.mark.parametrize("cert_path,cert_password", BOTH_CERTS) diff --git a/sdk/identity/azure-identity/tests/test_certificate_credential_async.py b/sdk/identity/azure-identity/tests/test_certificate_credential_async.py index fbfaa562f157..2608c6748260 100644 --- a/sdk/identity/azure-identity/tests/test_certificate_credential_async.py +++ b/sdk/identity/azure-identity/tests/test_certificate_credential_async.py @@ -192,11 +192,11 @@ async def mock_send(request, **kwargs): def test_token_cache(cert_path, cert_password): """the credential should optionally use a persistent cache, and default to an in memory cache""" - with patch("azure.identity._persistent_cache.msal_extensions") as mock_msal_extensions: + with patch(CertificateCredential.__module__ + "._load_persistent_cache") as load_persistent_cache: with patch(CertificateCredential.__module__ + ".msal") as mock_msal: CertificateCredential("tenant", "client-id", cert_path, password=cert_password) assert mock_msal.TokenCache.call_count == 1 - assert not mock_msal_extensions.PersistedTokenCache.called + assert not load_persistent_cache.called CertificateCredential( "tenant", @@ -205,7 +205,7 @@ def test_token_cache(cert_path, cert_password): password=cert_password, cache_persistence_options=TokenCachePersistenceOptions(), ) - assert mock_msal_extensions.PersistedTokenCache.call_count == 1 + assert load_persistent_cache.call_count == 1 @pytest.mark.asyncio diff --git a/sdk/identity/azure-identity/tests/test_client_secret_credential.py b/sdk/identity/azure-identity/tests/test_client_secret_credential.py index 854990f232b6..7c694c617c60 100644 --- a/sdk/identity/azure-identity/tests/test_client_secret_credential.py +++ b/sdk/identity/azure-identity/tests/test_client_secret_credential.py @@ -153,15 +153,15 @@ def test_regional_authority(): def test_token_cache(): """the credential should default to an in memory cache, and optionally use a persistent cache""" - with patch("azure.identity._persistent_cache.msal_extensions") as mock_msal_extensions: + with patch("azure.identity._internal.msal_credentials._load_persistent_cache") as load_persistent_cache: credential = ClientSecretCredential("tenant", "client-id", "secret") - assert not mock_msal_extensions.PersistedTokenCache.called + assert not load_persistent_cache.called assert isinstance(credential._cache, TokenCache) ClientSecretCredential( "tenant", "client-id", "secret", cache_persistence_options=TokenCachePersistenceOptions() ) - assert mock_msal_extensions.PersistedTokenCache.call_count == 1 + assert load_persistent_cache.call_count == 1 def test_cache_multiple_clients(): diff --git a/sdk/identity/azure-identity/tests/test_client_secret_credential_async.py b/sdk/identity/azure-identity/tests/test_client_secret_credential_async.py index e20ec1317342..60554ce9c90b 100644 --- a/sdk/identity/azure-identity/tests/test_client_secret_credential_async.py +++ b/sdk/identity/azure-identity/tests/test_client_secret_credential_async.py @@ -190,16 +190,16 @@ async def test_cache(): def test_token_cache(): """the credential should default to an in memory cache, and optionally use a persistent cache""" - with patch("azure.identity._persistent_cache.msal_extensions") as mock_msal_extensions: + with patch(ClientSecretCredential.__module__ + "._load_persistent_cache") as load_persistent_cache: with patch(ClientSecretCredential.__module__ + ".msal") as mock_msal: ClientSecretCredential("tenant", "client-id", "secret") assert mock_msal.TokenCache.call_count == 1 - assert not mock_msal_extensions.PersistedTokenCache.called + assert not load_persistent_cache.called ClientSecretCredential( "tenant", "client-id", "secret", cache_persistence_options=TokenCachePersistenceOptions() ) - assert mock_msal_extensions.PersistedTokenCache.call_count == 1 + assert load_persistent_cache.call_count == 1 @pytest.mark.asyncio diff --git a/sdk/identity/azure-identity/tests/test_interactive_credential.py b/sdk/identity/azure-identity/tests/test_interactive_credential.py index 6947dfe20d8b..e82f76f6c5ba 100644 --- a/sdk/identity/azure-identity/tests/test_interactive_credential.py +++ b/sdk/identity/azure-identity/tests/test_interactive_credential.py @@ -215,14 +215,14 @@ def __init__(self, **kwargs): def _request_token(self, *_, **__): pass - with patch("azure.identity._persistent_cache.msal_extensions") as mock_msal_extensions: + with patch("azure.identity._internal.msal_credentials._load_persistent_cache") as load_persistent_cache: with patch("azure.identity._internal.msal_credentials.msal") as mock_msal: TestCredential() - assert not mock_msal_extensions.PersistedTokenCache.called + assert not load_persistent_cache.called assert mock_msal.TokenCache.call_count == 1 TestCredential(cache_persistence_options=TokenCachePersistenceOptions()) - assert mock_msal_extensions.PersistedTokenCache.call_count == 1 + assert load_persistent_cache.call_count == 1 def test_home_account_id_client_info(): diff --git a/sdk/identity/azure-identity/tests/test_persistent_cache.py b/sdk/identity/azure-identity/tests/test_persistent_cache.py index 93b4cbacb809..ca2d9ee9309a 100644 --- a/sdk/identity/azure-identity/tests/test_persistent_cache.py +++ b/sdk/identity/azure-identity/tests/test_persistent_cache.py @@ -4,12 +4,13 @@ # ------------------------------------ from azure.identity import InteractiveBrowserCredential, TokenCachePersistenceOptions import pytest +import msal_extensions from helpers import mock def test_token_cache_persistence_options(): - with mock.patch("azure.identity._persistent_cache.msal_extensions"): + with mock.patch("azure.identity._internal.msal_credentials._load_persistent_cache"): # [START snippet] cache_options = TokenCachePersistenceOptions() credential = InteractiveBrowserCredential(cache_persistence_options=cache_options) @@ -23,20 +24,22 @@ def test_token_cache_persistence_options(): @mock.patch("azure.identity._persistent_cache.sys.platform", "linux2") -@mock.patch("azure.identity._persistent_cache.msal_extensions") -def test_persistent_cache_linux(mock_extensions): +def test_persistent_cache_linux(monkeypatch): """Credentials should use an unencrypted cache when encryption is unavailable and the user explicitly opts in. This test was written when Linux was the only platform on which encryption may not be available. """ from azure.identity._persistent_cache import _load_persistent_cache + for cls in ("FilePersistence", "LibsecretPersistence", "PersistedTokenCache"): + monkeypatch.setattr(msal_extensions, cls, mock.Mock()) + _load_persistent_cache(TokenCachePersistenceOptions()) - assert mock_extensions.PersistedTokenCache.called_with(mock_extensions.LibsecretPersistence) - mock_extensions.PersistedTokenCache.reset_mock() + assert msal_extensions.PersistedTokenCache.called_with(msal_extensions.LibsecretPersistence) + msal_extensions.PersistedTokenCache.reset_mock() # when LibsecretPersistence's dependencies aren't available, constructing it raises ImportError - mock_extensions.LibsecretPersistence = mock.Mock(side_effect=ImportError) + msal_extensions.LibsecretPersistence = mock.Mock(side_effect=ImportError) # encryption unavailable, no unencrypted storage not allowed with pytest.raises(ValueError): @@ -44,4 +47,4 @@ def test_persistent_cache_linux(mock_extensions): # encryption unavailable, unencrypted storage allowed _load_persistent_cache(TokenCachePersistenceOptions(allow_unencrypted_storage=True)) - mock_extensions.PersistedTokenCache.called_with(mock_extensions.FilePersistence) + msal_extensions.PersistedTokenCache.called_with(msal_extensions.FilePersistence) From a13bce29f1b235e2753980c286bdece2a47b0596 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Wed, 4 Aug 2021 14:41:37 -0700 Subject: [PATCH 4/4] changelog --- sdk/identity/azure-identity/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index d32cffe770d4..b30b8eab7afb 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -20,6 +20,10 @@ logging. On Python 3.7+, credentials invoked by these classes now log debug rather than info messages. ([#18972](https://github.com/Azure/azure-sdk-for-python/issues/18972)) +- Persistent cache implementations are now loaded on demand, enabling + workarounds when importing transitive dependencies such as pywin32 + fails + ([#19989](https://github.com/Azure/azure-sdk-for-python/issues/19989)) ## 1.7.0b2 (2021-07-08) ### Features Added