-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[Container Registry] Anonymous Access Client #18550
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.
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
Changes from all commits
d7bcfe6
7a0b598
15bbaaf
b91302c
6f7c55c
22ecb18
d5986dd
1287136
145949b
09f3636
6e38bf1
d9659c0
941473a
44ceace
4e5dbc8
f7caebd
6ced986
5417f1d
83c0d90
c0ed9ff
ed3f937
ccec23b
afa0c1b
0e2e335
24cb3a0
0dcd6cd
62964e8
64937b8
315c9f8
abd4e4e
119575e
7c31693
0c9e77a
7f57d03
d260ff9
8fbf308
dc033e9
9a93a9c
c06fecd
2b78a40
77aed4b
67887d6
4e7c9b6
86f86aa
87b1928
3255a2d
68f7bfd
8a254ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| # coding=utf-8 | ||
| # ------------------------------------ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
| # ------------------------------------ | ||
| from typing import TYPE_CHECKING, Dict, Any | ||
|
|
||
| from ._exchange_client import ExchangeClientAuthenticationPolicy | ||
| from ._generated import ContainerRegistry | ||
| from ._generated.models._container_registry_enums import TokenGrantType | ||
| from ._helpers import _parse_challenge | ||
| from ._user_agent import USER_AGENT | ||
|
|
||
| if TYPE_CHECKING: | ||
| from azure.core.credentials import TokenCredential | ||
|
|
||
|
|
||
| class AnonymousACRExchangeClient(object): | ||
| """Class for handling oauth authentication requests | ||
|
|
||
| :param endpoint: Azure Container Registry endpoint | ||
| :type endpoint: str | ||
| :param credential: Credential which provides tokens to authenticate requests | ||
| :type credential: :class:`~azure.core.credentials.TokenCredential` | ||
| """ | ||
|
|
||
| def __init__(self, endpoint, **kwargs): # pylint: disable=missing-client-constructor-parameter-credential | ||
| # type: (str, Dict[str, Any]) -> None | ||
| if not endpoint.startswith("https://") and not endpoint.startswith("http://"): | ||
| endpoint = "https://" + endpoint | ||
| self._endpoint = endpoint | ||
| self.credential_scope = "https://management.core.windows.net/.default" | ||
| self._client = ContainerRegistry( | ||
| credential=None, | ||
| url=endpoint, | ||
| sdk_moniker=USER_AGENT, | ||
| authentication_policy=ExchangeClientAuthenticationPolicy(), | ||
| credential_scopes=kwargs.pop("credential_scopes", self.credential_scope), | ||
| **kwargs | ||
| ) | ||
|
|
||
| def get_acr_access_token(self, challenge, **kwargs): | ||
| # type: (str, Dict[str, Any]) -> str | ||
| parsed_challenge = _parse_challenge(challenge) | ||
| parsed_challenge["grant_type"] = TokenGrantType.PASSWORD | ||
| return self.exchange_refresh_token_for_access_token( | ||
| None, | ||
| service=parsed_challenge["service"], | ||
| scope=parsed_challenge["scope"], | ||
| grant_type=TokenGrantType.PASSWORD, | ||
| **kwargs | ||
| ) | ||
|
|
||
| def exchange_refresh_token_for_access_token( | ||
| self, refresh_token=None, service=None, scope=None, grant_type=TokenGrantType.PASSWORD, **kwargs | ||
| ): | ||
| # type: (str, str, str, str, Dict[str, Any]) -> str | ||
| access_token = self._client.authentication.exchange_acr_refresh_token_for_acr_access_token( | ||
| service=service, scope=scope, refresh_token=refresh_token, grant_type=grant_type, **kwargs | ||
| ) | ||
| return access_token.access_token | ||
|
|
||
| def __enter__(self): | ||
| self._client.__enter__() | ||
| return self | ||
|
|
||
| def __exit__(self, *args): | ||
| self._client.__exit__(*args) | ||
|
|
||
| def close(self): | ||
| # type: () -> None | ||
| """Close sockets opened by the client. | ||
| Calling this method is unnecessary when using the client as a context manager. | ||
| """ | ||
| self._client.close() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
|
|
||
| from azure.core.pipeline.policies import HTTPPolicy | ||
|
|
||
| from ._anonymous_exchange_client import AnonymousACRExchangeClient | ||
| from ._exchange_client import ACRExchangeClient | ||
| from ._helpers import _enforce_https | ||
|
|
||
|
|
@@ -24,7 +25,10 @@ def __init__(self, credential, endpoint): | |
| # type: (TokenCredential, str) -> None | ||
| super(ContainerRegistryChallengePolicy, self).__init__() | ||
| self._credential = credential | ||
| self._exchange_client = ACRExchangeClient(endpoint, self._credential) | ||
| if self._credential is None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this check move before line 28? Otherwise we are building the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I can move it, I originally didn't like the |
||
| self._exchange_client = AnonymousACRExchangeClient(endpoint) | ||
| else: | ||
| self._exchange_client = ACRExchangeClient(endpoint, self._credential) | ||
|
|
||
| def on_request(self, request): | ||
| # type: (PipelineRequest) -> None | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| # Licensed under the MIT License. | ||
| # ------------------------------------ | ||
| from enum import Enum | ||
| from typing import TYPE_CHECKING | ||
| from typing import TYPE_CHECKING, Dict, Any | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: it looks like Dict and Any can just be imported if type checking |
||
|
|
||
| from azure.core.pipeline.transport import HttpTransport | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| # coding=utf-8 | ||
| # ------------------------------------ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
| # ------------------------------------ | ||
| from typing import TYPE_CHECKING, Dict, List, Any | ||
|
|
||
| from ._async_exchange_client import ExchangeClientAuthenticationPolicy | ||
| from .._generated.aio import ContainerRegistry | ||
| from .._generated.models._container_registry_enums import TokenGrantType | ||
| from .._helpers import _parse_challenge | ||
| from .._user_agent import USER_AGENT | ||
|
|
||
| if TYPE_CHECKING: | ||
| from azure.core.credentials_async import AsyncTokenCredential | ||
|
|
||
|
|
||
| class AnonymousACRExchangeClient(object): | ||
| """Class for handling oauth authentication requests | ||
|
|
||
| :param endpoint: Azure Container Registry endpoint | ||
| :type endpoint: str | ||
| """ | ||
|
|
||
| def __init__(self, endpoint: str, **kwargs: Dict[str, Any]) -> None: # pylint: disable=missing-client-constructor-parameter-credential | ||
| if not endpoint.startswith("https://") and not endpoint.startswith("http://"): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about parameter validation, but I realize that this may be kind of ACR-specific |
||
| endpoint = "https://" + endpoint | ||
| self._endpoint = endpoint | ||
| self._credential_scope = "https://management.core.windows.net/.default" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be a constant somewhere to be shared between the ExchangeClients
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The credential scope needs to be fixed in the next release to handle foreign clouds as well. I'm going to address this in our next beta. |
||
| self._client = ContainerRegistry( | ||
| credential=None, | ||
| url=endpoint, | ||
| sdk_moniker=USER_AGENT, | ||
| authentication_policy=ExchangeClientAuthenticationPolicy(), | ||
| credential_scopes=kwargs.pop("credential_scopes", self._credential_scope), | ||
| **kwargs | ||
| ) | ||
|
|
||
| async def get_acr_access_token(self, challenge: str, **kwargs: Dict[str, Any]) -> str: | ||
| parsed_challenge = _parse_challenge(challenge) | ||
| parsed_challenge["grant_type"] = TokenGrantType.PASSWORD | ||
| return await self.exchange_refresh_token_for_access_token( | ||
| None, | ||
| service=parsed_challenge["service"], | ||
| scope=parsed_challenge["scope"], | ||
| grant_type=TokenGrantType.PASSWORD, | ||
| **kwargs | ||
| ) | ||
|
|
||
| async def exchange_refresh_token_for_access_token( | ||
| self, | ||
| refresh_token: str = None, | ||
| service: str = None, | ||
| scope: str = None, | ||
| grant_type: str = TokenGrantType.PASSWORD, | ||
| **kwargs: Any | ||
| ) -> str: | ||
| access_token = await self._client.authentication.exchange_acr_refresh_token_for_acr_access_token( | ||
| service=service, scope=scope, refresh_token=refresh_token, grant_type=grant_type, **kwargs | ||
| ) | ||
| return access_token.access_token | ||
|
|
||
| async def __aenter__(self): | ||
| self._client.__aenter__() | ||
| return self | ||
|
|
||
| async def __aexit__(self, *args): | ||
| self._client.__aexit__(*args) | ||
|
|
||
| async def close(self) -> None: | ||
| """Close sockets opened by the client. | ||
| Calling this method is unnecessary when using the client as a context manager. | ||
| """ | ||
| await self._client.close() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like it could be veering into client-side validation territory, but if there are other clients that do this then it's probably okay for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we do this in all four tier-1 languages, and the biggest reason being the value given in the portal when you create an account does not prefix the endpoint with
https://orhttp://