Skip to content

[Identity] Explicitly implement TokenCredential protocol #25788

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.

Sign up for GitHub

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

Closed
Laia Patiño (laiapat) wants to merge 5 commits into from
Closed

[Identity] Explicitly implement TokenCredential protocol #25788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7e196a
Explicit TokenCredentials where necessary
laiapat Aug 20, 2022
2d73a74
Ignore protocol init warnings
laiapat Aug 31, 2022
4fcc804
Remove GetTokenMixin protocol inheritance
laiapat Aug 31, 2022
a9682a6
Also fix async GetTokenMixin/imports
laiapat Aug 31, 2022
d5c53f0
Bump minimum core version
laiapat Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bugs Fixed

### Other Changes
- Upgraded minimum `azure-core` version to 1.23.1

## 1.11.0b3 (2022-08-09)

Expand Down
8 changes: 3 additions & 5 deletions sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import six

from azure.core.credentials import AccessToken
from azure.core.credentials import AccessToken, TokenCredential
from azure.core.exceptions import ClientAuthenticationError

from .. import CredentialUnavailableError
Expand All @@ -31,16 +31,14 @@
NOT_LOGGED_IN = "Please run 'az login' to set up an account"


class AzureCliCredential(object):
class AzureCliCredential(TokenCredential):
"""Authenticates by requesting a token from the Azure CLI.

This requires previously logging in to Azure via "az login", and will use the CLI's currently logged in identity.

:keyword str tenant_id: optional tenant to include in the token request.
"""
def __init__(self, *, tenant_id: str = ""):
object.__init__(self)

def __init__(self, *, tenant_id: str = ""): # pylint:disable=super-init-not-called
self.tenant_id = tenant_id

def __enter__(self):
Expand Down
4 changes: 2 additions & 2 deletions sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import six

from azure.core.credentials import AccessToken
from azure.core.credentials import AccessToken, TokenCredential
from azure.core.exceptions import ClientAuthenticationError

from .azure_cli import get_safe_working_dir
Expand Down Expand Up @@ -47,7 +47,7 @@
"""


class AzurePowerShellCredential(object):
class AzurePowerShellCredential(TokenCredential):
"""Authenticates by requesting a token from Azure PowerShell.

This requires previously logging in to Azure via "Connect-AzAccount", and will use the currently logged in identity.
Expand Down
7 changes: 4 additions & 3 deletions sdk/identity/azure-identity/azure/identity/_credentials/chained.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# ------------------------------------
import logging

from azure.core.credentials import TokenCredential
from azure.core.exceptions import ClientAuthenticationError

from .. import CredentialUnavailableError
Expand All @@ -17,7 +18,7 @@
if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from typing import Any, Optional
from azure.core.credentials import AccessToken, TokenCredential
from azure.core.credentials import AccessToken

_LOGGER = logging.getLogger(__name__)

Expand All @@ -35,7 +36,7 @@ def _get_error_message(history):
)


class ChainedTokenCredential(object):
class ChainedTokenCredential(TokenCredential):
"""A sequence of credentials that is itself a credential.

Its :func:`get_token` method calls ``get_token`` on each credential in the sequence, in order, returning the first
Expand All @@ -45,7 +46,7 @@ class ChainedTokenCredential(object):
:type credentials: :class:`azure.core.credentials.TokenCredential`
"""

def __init__(self, *credentials):
def __init__(self, *credentials): # pylint:disable=super-init-not-called
# type: (*TokenCredential) -> None
if not credentials:
raise ValueError("at least one credential is required")
Expand Down
5 changes: 3 additions & 2 deletions sdk/identity/azure-identity/azure/identity/_credentials/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os

from azure.core.credentials import TokenCredential

from .. import CredentialUnavailableError
from .._constants import EnvironmentVariables
Expand All @@ -28,7 +29,7 @@
_LOGGER = logging.getLogger(__name__)


class EnvironmentCredential(object):
class EnvironmentCredential(TokenCredential):
"""A credential configured by environment variables.

This credential is capable of authenticating as a service principal using a client secret or a certificate, or as
Expand Down Expand Up @@ -63,7 +64,7 @@ class EnvironmentCredential(object):
when no value is given.
"""

def __init__(self, **kwargs):
def __init__(self, **kwargs): # pylint:disable=super-init-not-called
# type: (Mapping[str, Any]) -> None
self._credential = None # type: Optional[EnvironmentCredentialTypes]

Expand Down
8 changes: 5 additions & 3 deletions sdk/identity/azure-identity/azure/identity/_credentials/managed_identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import logging
import os

from azure.core.credentials import TokenCredential

from .. import CredentialUnavailableError
from .._constants import EnvironmentVariables
from .._internal.decorators import log_get_token
Expand All @@ -17,12 +19,12 @@
if TYPE_CHECKING:
# pylint:disable=unused-import
from typing import Any, Optional
from azure.core.credentials import AccessToken, TokenCredential
from azure.core.credentials import AccessToken

_LOGGER = logging.getLogger(__name__)


class ManagedIdentityCredential(object):
class ManagedIdentityCredential(TokenCredential):
"""Authenticates with an Azure managed identity in any hosting environment which supports managed identities.

This credential defaults to using a system-assigned identity. To configure a user-assigned identity, use one of
Expand All @@ -38,7 +40,7 @@ class ManagedIdentityCredential(object):
:paramtype identity_config: Mapping[str, str]
"""

def __init__(self, **kwargs):
def __init__(self, **kwargs): # pylint:disable=super-init-not-called
# type: (**Any) -> None
self._credential = None # type: Optional[TokenCredential]
if os.environ.get(EnvironmentVariables.IDENTITY_ENDPOINT):
Expand Down
7 changes: 4 additions & 3 deletions sdk/identity/azure-identity/azure/identity/_credentials/shared_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
# ------------------------------------
from typing import TYPE_CHECKING

from azure.core.credentials import TokenCredential

from .silent import SilentAuthenticationCredential
from .. import CredentialUnavailableError
from .._constants import DEVELOPER_SIGN_ON_CLIENT_ID
Expand All @@ -14,11 +16,10 @@
if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from typing import Any, Optional
from azure.core.credentials import TokenCredential
from .._internal import AadClientBase


class SharedTokenCacheCredential(object):
class SharedTokenCacheCredential(TokenCredential):
"""Authenticates using tokens in the local cache shared between Microsoft applications.

:param str username: Username (typically an email address) of the user to authenticate as. This is used when the
Expand All @@ -36,7 +37,7 @@ class SharedTokenCacheCredential(object):
:paramtype cache_persistence_options: ~azure.identity.TokenCachePersistenceOptions
"""

def __init__(self, username=None, **kwargs):
def __init__(self, username=None, **kwargs): # pylint:disable=super-init-not-called
# type: (Optional[str], **Any) -> None

if "authentication_record" in kwargs:
Expand Down
6 changes: 3 additions & 3 deletions sdk/identity/azure-identity/azure/identity/_credentials/silent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from msal import PublicClientApplication

from azure.core.credentials import AccessToken
from azure.core.credentials import AccessToken, TokenCredential
from azure.core.exceptions import ClientAuthenticationError

from .. import CredentialUnavailableError
Expand All @@ -25,10 +25,10 @@
from .. import AuthenticationRecord


class SilentAuthenticationCredential(object):
class SilentAuthenticationCredential(TokenCredential):
"""Internal class for authenticating from the default shared cache given an AuthenticationRecord"""

def __init__(self, authentication_record, **kwargs):
def __init__(self, authentication_record, **kwargs): # pylint:disable=super-init-not-called
# type: (AuthenticationRecord, **Any) -> None
self._auth_record = authentication_record

Expand Down
6 changes: 4 additions & 2 deletions sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import os
from typing import TYPE_CHECKING

from azure.core.credentials_async import AsyncTokenCredential
from azure.core.exceptions import ClientAuthenticationError

from .._internal import AsyncContextManager
from .._internal.decorators import log_get_token_async
from ... import CredentialUnavailableError
Expand All @@ -27,14 +29,14 @@
from azure.core.credentials import AccessToken


class AzureCliCredential(AsyncContextManager):
class AzureCliCredential(AsyncContextManager, AsyncTokenCredential):
"""Authenticates by requesting a token from the Azure CLI.

This requires previously logging in to Azure via "az login", and will use the CLI's currently logged in identity.

:keyword str tenant_id: optional tenant to include in the token request.
"""
def __init__(self, *, tenant_id: str = ""):
def __init__(self, *, tenant_id: str = ""): # pylint:disable=super-init-not-called
AsyncContextManager.__init__(self)

self.tenant_id = tenant_id
Expand Down
4 changes: 3 additions & 1 deletion sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import sys
from typing import cast, TYPE_CHECKING

from azure.core.credentials_async import AsyncTokenCredential

from .._internal import AsyncContextManager
from .._internal.decorators import log_get_token_async
from ... import CredentialUnavailableError
Expand All @@ -24,7 +26,7 @@
from azure.core.credentials import AccessToken


class AzurePowerShellCredential(AsyncContextManager):
class AzurePowerShellCredential(AsyncContextManager, AsyncTokenCredential):
"""Authenticates by requesting a token from Azure PowerShell.

This requires previously logging in to Azure via "Connect-AzAccount", and will use the currently logged in identity.
Expand Down
6 changes: 3 additions & 3 deletions sdk/identity/azure-identity/azure/identity/aio/_credentials/chained.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import logging
from typing import TYPE_CHECKING

from azure.core.credentials_async import AsyncTokenCredential
from azure.core.exceptions import ClientAuthenticationError
from .._internal import AsyncContextManager
from ... import CredentialUnavailableError
Expand All @@ -15,12 +16,11 @@
if TYPE_CHECKING:
from typing import Any, Optional
from azure.core.credentials import AccessToken
from azure.core.credentials_async import AsyncTokenCredential

_LOGGER = logging.getLogger(__name__)


class ChainedTokenCredential(AsyncContextManager):
class ChainedTokenCredential(AsyncContextManager, AsyncTokenCredential):
"""A sequence of credentials that is itself a credential.

Its :func:`get_token` method calls ``get_token`` on each credential in the sequence, in order, returning the first
Expand All @@ -30,7 +30,7 @@ class ChainedTokenCredential(AsyncContextManager):
:type credentials: :class:`azure.core.credentials.AsyncTokenCredential`
"""

def __init__(self, *credentials: "AsyncTokenCredential") -> None:
def __init__(self, *credentials: "AsyncTokenCredential") -> None: # pylint:disable=super-init-not-called
if not credentials:
raise ValueError("at least one credential is required")

Expand Down
7 changes: 4 additions & 3 deletions sdk/identity/azure-identity/azure/identity/aio/_credentials/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import os
from typing import TYPE_CHECKING

from .._internal.decorators import log_get_token_async
from azure.core.credentials_async import AsyncTokenCredential

from .._internal.decorators import log_get_token_async
from ... import CredentialUnavailableError
from ..._constants import EnvironmentVariables
from .._internal import AsyncContextManager
Expand All @@ -21,7 +22,7 @@
_LOGGER = logging.getLogger(__name__)


class EnvironmentCredential(AsyncContextManager):
class EnvironmentCredential(AsyncContextManager, AsyncTokenCredential):
"""A credential configured by environment variables.

This credential is capable of authenticating as a service principal using a client secret or a certificate, or as
Expand All @@ -45,7 +46,7 @@ class EnvironmentCredential(AsyncContextManager):
when no value is given.
"""

def __init__(self, **kwargs: "Any") -> None:
def __init__(self, **kwargs: "Any") -> None: # pylint:disable=super-init-not-called
self._credential = None # type: Optional[Union[CertificateCredential, ClientSecretCredential]]

if all(os.environ.get(v) is not None for v in EnvironmentVariables.CLIENT_SECRET_VARS):
Expand Down
7 changes: 4 additions & 3 deletions sdk/identity/azure-identity/azure/identity/aio/_credentials/managed_identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import os
from typing import TYPE_CHECKING

from azure.core.credentials_async import AsyncTokenCredential

from .._internal import AsyncContextManager
from .._internal.decorators import log_get_token_async
from ... import CredentialUnavailableError
Expand All @@ -14,12 +16,11 @@
if TYPE_CHECKING:
from typing import Any, Optional
from azure.core.credentials import AccessToken
from azure.core.credentials_async import AsyncTokenCredential

_LOGGER = logging.getLogger(__name__)


class ManagedIdentityCredential(AsyncContextManager):
class ManagedIdentityCredential(AsyncContextManager, AsyncTokenCredential):
"""Authenticates with an Azure managed identity in any hosting environment which supports managed identities.

This credential defaults to using a system-assigned identity. To configure a user-assigned identity, use one of
Expand All @@ -35,7 +36,7 @@ class ManagedIdentityCredential(AsyncContextManager):
:paramtype identity_config: Mapping[str, str]
"""

def __init__(self, **kwargs: "Any") -> None:
def __init__(self, **kwargs: "Any") -> None: # pylint:disable=super-init-not-called
self._credential = None # type: Optional[AsyncTokenCredential]

if os.environ.get(EnvironmentVariables.IDENTITY_ENDPOINT):
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/azure-identity/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
),
python_requires=">=3.7",
install_requires=[
"azure-core<2.0.0,>=1.11.0",
"azure-core<2.0.0,>=1.23.1",

@xiangyan99 Xiang Yan (xiangyan99) Feb 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not force users to upgrade azure-core. We can use try import catch instead?

@laiapat Laia Patiño (laiapat) Feb 2, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was motivated by the TokenCredential import, but that may not actually be necessary. I talked this over with Laurent, and he feels that the best solution would be to actually match the method signatures of the protocol; i.e. document kwargs like claims even if they wouldn't have an effect for a particular credential. I think this does make sense, since it makes the capabilities and limitations of each credential more explicit and more correctly implements the protocol.

I want to first check what pyright thinks of this PR's approach. If explicit subtyping without full implementation raises errors with pyright anyway, then this approach won't be viable regardless and would make the choice easier to make (unless you agree that we should go for the full implementation route regardless).

"cryptography>=2.5",
"msal<2.0.0,>=1.12.0",
"msal-extensions<2.0.0,>=0.3.0",
Expand Down
2 changes: 1 addition & 1 deletion shared_requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ chardet<5,>=3.0.2
#override azure-digitaltwins-core azure-core<2.0.0,>=1.20.0
#override azure-eventhub azure-core<2.0.0,>=1.14.0
#override azure-eventhub typing-extensions>=4.0.1
#override azure-identity azure-core<2.0.0,>=1.11.0
#override azure-identity azure-core<2.0.0,>=1.23.1
#override azure-keyvault-administration azure-core<2.0.0,>=1.24.0
#override azure-keyvault-certificates azure-core<2.0.0,>=1.20.0
#override azure-keyvault-keys azure-core<2.0.0,>=1.24.0
Expand Down