Skip to content

Commit

Permalink
address arch board review feedback (#23539)
Browse files Browse the repository at this point in the history
  • Loading branch information
xiangyan99 authored Mar 22, 2022
1 parent 005b707 commit 95139f4
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 26 deletions.
3 changes: 3 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

### Other Changes

- Removed `resource_id`, please use `identity_config` instead.
- Renamed argument name `get_assertion` to `func` for `ClientAssertionCredential`.

## 1.9.0b1 (2022-03-08)

### Features Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ def _get_client_args(**kwargs):
# App Service managed identity isn't available in this environment
return None

if kwargs.get("resource_id"):
identity_config["mi_res_id"] = kwargs.pop("resource_id")

return dict(
kwargs,
identity_config=identity_config,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ def _get_client_args(**kwargs):

if kwargs.get("client_id"):
identity_config["clientid"] = kwargs.pop("client_id")
if kwargs.get("resource_id"):
identity_config["mi_res_id"] = kwargs.pop("resource_id")

return dict(
kwargs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@


class ClientAssertionCredential(GetTokenMixin):
def __init__(self, tenant_id, client_id, get_assertion, **kwargs):
def __init__(self, tenant_id, client_id, func, **kwargs):
# type: (str, str, Callable[[], str], **Any) -> None
"""Authenticates a service principal with a JWT assertion.
Expand All @@ -22,15 +22,15 @@ def __init__(self, tenant_id, client_id, get_assertion, **kwargs):
:param str tenant_id: ID of the principal's tenant. Also called its "directory" ID.
:param str client_id: the principal's client ID
:param get_assertion: a callable that returns a string assertion. The credential will call this every time it
:param func: a callable that returns a string assertion. The credential will call this every time it
acquires a new token.
:paramtype get_assertion: Callable[[], str]
:paramtype func: Callable[[], str]
:keyword str authority: authority of an Azure Active Directory endpoint, for example
"login.microsoftonline.com", the authority for Azure Public Cloud (which is the default).
:class:`~azure.identity.AzureAuthorityHosts` defines authorities for other clouds.
"""
self._get_assertion = get_assertion
self._func = func
self._client = AadClient(tenant_id, client_id, **kwargs)
super(ClientAssertionCredential, self).__init__(**kwargs)

Expand All @@ -51,6 +51,6 @@ def _acquire_token_silently(self, *scopes, **kwargs):

def _request_token(self, *scopes, **kwargs):
# type: (*str, **Any) -> AccessToken
assertion = self._get_assertion()
assertion = self._func()
token = self._client.obtain_token_by_jwt_assertion(scopes, assertion, **kwargs)
return token
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def __init__(self, **kwargs):
else:
self._endpoint_available = None
self._error_message = None # type: Optional[str]
self._user_assigned_identity = "client_id" in kwargs or "resource_id" in kwargs or "identity_config" in kwargs
self._user_assigned_identity = "client_id" in kwargs or "identity_config" in kwargs

def __enter__(self):
self._client.__enter__()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ class ManagedIdentityCredential(object):
:keyword str client_id: a user-assigned identity's client ID or, when using Pod Identity, the client ID of an Azure
AD app registration. This argument is supported in all hosting environments.
:keyword str resource_id: The resource ID to authenticate for a user-assigned managed identity.
See `Managed identity types
<https://docs.microsoft.com/azure/active-directory/managed-identities-azure-resources/overview#managed-identity-types>`_
for more information about user-assigned managed identities.
:keyword identity_config: a mapping ``{parameter_name: value}`` specifying a user-assigned identity by its object
or resource ID, for example ``{"object_id": "..."}``. Check the documentation for your hosting environment to
learn what values it expects.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def __init__(self, tenant_id, client_id, token_file_path, **kwargs):
super(TokenExchangeCredential, self).__init__(
tenant_id=tenant_id,
client_id=client_id,
get_assertion=self.get_service_account_token,
func=self.get_service_account_token,
token_file_path=token_file_path,
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@

class ManagedIdentityClientBase(ABC):
# pylint:disable=missing-client-constructor-parameter-credential
def __init__(self, request_factory, client_id=None, resource_id=None, identity_config=None, **kwargs):
# type: (Callable[[str, dict], HttpRequest], Optional[str], Optional[str], Optional[Dict], **Any) -> None
def __init__(self, request_factory, client_id=None, identity_config=None, **kwargs):
# type: (Callable[[str, dict], HttpRequest], Optional[str], Optional[Dict], **Any) -> None
self._cache = kwargs.pop("_cache", None) or TokenCache()
self._content_callback = kwargs.pop("_content_callback", None)
self._identity_config = identity_config or {}
if client_id:
self._identity_config["client_id"] = client_id
if resource_id:
self._identity_config["mi_res_id"] = resource_id
self._pipeline = self._build_pipeline(**kwargs)
self._request_factory = request_factory

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,23 @@


class ClientAssertionCredential(AsyncContextManager, GetTokenMixin):
def __init__(self, tenant_id: str, client_id: str, get_assertion: "Callable[[], str]", **kwargs: "Any") -> None:
def __init__(self, tenant_id: str, client_id: str, func: "Callable[[], str]", **kwargs: "Any") -> None:
"""Authenticates a service principal with a JWT assertion.
This credential is for advanced scenarios. :class:`~azure.identity.ClientCertificateCredential` has a more
convenient API for the most common assertion scenario, authenticating a service principal with a certificate.
:param str tenant_id: ID of the principal's tenant. Also called its "directory" ID.
:param str client_id: the principal's client ID
:param get_assertion: a callable that returns a string assertion. The credential will call this every time it
:param func: a callable that returns a string assertion. The credential will call this every time it
acquires a new token.
:paramtype get_assertion: Callable[[], str]
:paramtype func: Callable[[], str]
:keyword str authority: authority of an Azure Active Directory endpoint, for example
"login.microsoftonline.com", the authority for Azure Public Cloud (which is the default).
:class:`~azure.identity.AzureAuthorityHosts` defines authorities for other clouds.
"""
self._get_assertion = get_assertion
self._func = func
self._client = AadClient(tenant_id, client_id, **kwargs)
super().__init__(**kwargs)

Expand All @@ -45,6 +45,6 @@ async def _acquire_token_silently(self, *scopes: str, **kwargs: "Any") -> "Optio
return self._client.get_cached_access_token(scopes, **kwargs)

async def _request_token(self, *scopes: str, **kwargs: "Any") -> "AccessToken":
assertion = self._get_assertion()
assertion = self._func()
token = await self._client.obtain_token_by_jwt_assertion(scopes, assertion, **kwargs)
return token
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def __init__(self, tenant_id: str, client_id: str, token_file_path: str, **kwarg
super().__init__(
tenant_id=tenant_id,
client_id=client_id,
get_assertion=self.get_service_account_token,
func=self.get_service_account_token,
token_file_path=token_file_path,
**kwargs
)

0 comments on commit 95139f4

Please sign in to comment.