Refactor SharedTokenCacheCredential exception handling (Azure#11962)

Author: chlowell

sdk/identity/azure-identity/CHANGELOG.md

@@ -1,7 +1,9 @@
 # Release History
 
 ## 1.4.0b5 (Unreleased)
-
+- `SharedTokenCacheCredential.get_token` raises `ValueError` instead of
+  `ClientAuthenticationError` when called with no scopes.
+  ([#11553](https://github.com/Azure/azure-sdk-for-python/issues/11553))
 
 ## 1.4.0b4 (2020-06-09)
 - `ManagedIdentityCredential` can configure a user-assigned identity using any

sdk/identity/azure-identity/azure/identity/_credentials/shared_cache.py

@@ -4,7 +4,7 @@
 # ------------------------------------
 from .. import CredentialUnavailableError
 from .._constants import AZURE_CLI_CLIENT_ID
-from .._internal import AadClient, wrap_exceptions
+from .._internal import AadClient
 from .._internal.shared_token_cache import NO_TOKEN, SharedTokenCacheBase
 
 try:
@@ -36,7 +36,6 @@ class SharedTokenCacheCredential(SharedTokenCacheBase):
         is unavailable. Defaults to False.
     """
 
-    @wrap_exceptions
     def get_token(self, *scopes, **kwargs):  # pylint:disable=unused-argument
         # type (*str, **Any) -> AccessToken
         """Get an access token for `scopes` from the shared cache.

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

@@ -6,12 +6,13 @@
 import time
 
 from msal import TokenCache
+import six
 from six.moves.urllib_parse import urlparse
 
 from azure.core.credentials import AccessToken
 from .. import CredentialUnavailableError
 from .._constants import KnownAuthorities
-from .._internal import get_default_authority, normalize_authority
+from .._internal import get_default_authority, normalize_authority, wrap_exceptions
 from .._internal.persistent_cache import load_user_cache
 
 try:
@@ -158,6 +159,7 @@ def _get_accounts_having_matching_refresh_tokens(self):
                     accounts[account["home_account_id"]] = account
         return accounts.values()
 
+    @wrap_exceptions
     def _get_account(self, username=None, tenant_id=None):
         # type: (Optional[str], Optional[str]) -> CacheItem
         """returns exactly one account which has a refresh token and matches username and/or tenant_id"""
@@ -198,26 +200,34 @@ def _get_cached_access_token(self, scopes, account):
         if "home_account_id" not in account:
             return None
 
-        cache_entries = self._cache.find(
-            TokenCache.CredentialType.ACCESS_TOKEN,
-            target=list(scopes),
-            query={"home_account_id": account["home_account_id"]},
-        )
+        try:
+            cache_entries = self._cache.find(
+                TokenCache.CredentialType.ACCESS_TOKEN,
+                target=list(scopes),
+                query={"home_account_id": account["home_account_id"]},
+            )
+            for token in cache_entries:
+                expires_on = int(token["expires_on"])
+                if expires_on - 300 > int(time.time()):
+                    return AccessToken(token["secret"], expires_on)
+        except Exception as ex:  # pylint:disable=broad-except
+            message = "Error accessing cached data: {}".format(ex)
+            six.raise_from(CredentialUnavailableError(message=message), ex)
 
-        for token in cache_entries:
-            expires_on = int(token["expires_on"])
-            if expires_on - 300 > int(time.time()):
-                return AccessToken(token["secret"], expires_on)
         return None
 
     def _get_refresh_tokens(self, account):
         if "home_account_id" not in account:
             return None
 
-        cache_entries = self._cache.find(
-            TokenCache.CredentialType.REFRESH_TOKEN, query={"home_account_id": account["home_account_id"]}
-        )
-        return (token["secret"] for token in cache_entries if "secret" in token)
+        try:
+            cache_entries = self._cache.find(
+                TokenCache.CredentialType.REFRESH_TOKEN, query={"home_account_id": account["home_account_id"]}
+            )
+            return [token["secret"] for token in cache_entries if "secret" in token]
+        except Exception as ex:  # pylint:disable=broad-except
+            message = "Error accessing cached data: {}".format(ex)
+            six.raise_from(CredentialUnavailableError(message=message), ex)
 
     @staticmethod
     def supported():

sdk/identity/azure-identity/azure/identity/aio/_credentials/shared_cache.py

@@ -8,7 +8,6 @@
 from ..._constants import AZURE_CLI_CLIENT_ID
 from ..._internal.shared_token_cache import NO_TOKEN, SharedTokenCacheBase
 from .._internal.aad_client import AadClient
-from .._internal.exception_wrapper import wrap_exceptions
 from .base import AsyncCredentialBase
 
 if TYPE_CHECKING:
@@ -46,7 +45,6 @@ async def close(self):
         if self._client:
             await self._client.__aexit__()
 
-    @wrap_exceptions
     async def get_token(self, *scopes: str, **kwargs: "Any") -> "AccessToken":  # pylint:disable=unused-argument
         """Get an access token for `scopes` from the shared cache.
 

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

@@ -35,7 +35,7 @@ def test_no_scopes():
     """The credential should raise when get_token is called with no scopes"""
 
     credential = SharedTokenCacheCredential(_cache=TokenCache())
-    with pytest.raises(ClientAuthenticationError):
+    with pytest.raises(ValueError):
         credential.get_token()
 
 

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

@@ -31,7 +31,7 @@ async def test_no_scopes():
     """The credential should raise when get_token is called with no scopes"""
 
     credential = SharedTokenCacheCredential(_cache=TokenCache())
-    with pytest.raises(ClientAuthenticationError):
+    with pytest.raises(ValueError):
         await credential.get_token()