Skip to content
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

Add support for authenticating against Azure Container Registry #263

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ThorstenHans
Copy link

As described in #262 Azure Container Registry returns the access token using the access_token field instead of the token field. With this commit, the token could be fetched from both fields

Azure Container Registry returns the access token using the `access_token` field instead of the `token` field. With this commit, the token could be fetched from both fields

Signed-off-by: Thorsten Hans <thorsten.hans@fermyon.com>
In contrast to ACR, Docker Hub responds with `token` and `access_token` which is not supported by the alias attribute provided by serde. That is why I came up with a dedicated struct deserializing both fields into an Option<String> the TryFrom<> implementation first tries to use the token property (as it was before this PR). If that is not present, it tries to take the access_token property. If both are missing, a corresponding error is thrown

Signed-off-by: Thorsten Hans <thorsten.hans@fermyon.com>
@ThorstenHans
Copy link
Author

Upon further testing I discovered that Docker Hub returns both fields token and access_token in the response. I know that the name of the added struct (AzureBearerAuth) is not optimal. But using an intermediate struct allowed me to successfully authenticate against both Docker Hub and ACR.

Signed-off-by: Thorsten Hans <thorsten.hans@fermyon.com>
@vrutkovs
Copy link
Contributor

cc @PratikMahajan

@ThorstenHans
Copy link
Author

Renamed the struct from AzureBearerAuth to MultiTokenBearerAuth which is a bit more meaningful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants