support identitytoken based authentication #842
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This form of authentication is typically used by private registries which use OAuth2 authentication, for example Azure Container Registry (ACR). Note: It is outside the scope of testcontainers to _obtain_ the token.
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
lbruun
changed the title
Comment on lines
+65
to
+83
| if (!JsonValueKind.Object.Equals(authProperty.Value.ValueKind)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| // if the json object has a property named 'identitytoken' then that value is all we need | ||
| // to authenticate. (no need for username or password as it is implied by the token) | ||
|
|
||
| if (authProperty.Value.TryGetProperty("identitytoken", out var identitytoken)) | ||
| { | ||
| if (JsonValueKind.String.Equals(identitytoken.ValueKind)) | ||
| { | ||
| this.logger.DockerRegistryCredentialFound(hostname); | ||
| return new DockerRegistryAuthenticationConfiguration(authProperty.Name, null, null, identitytoken.GetString()); | ||
| } | ||
| } | ||
|
|
||
| // ... otherwise we expect the 'auth' property to contain the username and password | ||
| // in base64 encoded form and separated by a colon char. |
There was a problem hiding this comment.
Suggested change
| if (!JsonValueKind.Object.Equals(authProperty.Value.ValueKind)) | |
| { | |
| return null; | |
| } | |
| // if the json object has a property named 'identitytoken' then that value is all we need | |
| // to authenticate. (no need for username or password as it is implied by the token) | |
| if (authProperty.Value.TryGetProperty("identitytoken", out var identitytoken)) | |
| { | |
| if (JsonValueKind.String.Equals(identitytoken.ValueKind)) | |
| { | |
| this.logger.DockerRegistryCredentialFound(hostname); | |
| return new DockerRegistryAuthenticationConfiguration(authProperty.Name, null, null, identitytoken.GetString()); | |
| } | |
| } | |
| // ... otherwise we expect the 'auth' property to contain the username and password | |
| // in base64 encoded form and separated by a colon char. | |
| if (JsonValueKind.Undefined.Equals(authProperty.Value.ValueKind)) | |
| { | |
| return null; | |
| } | |
| if (authProperty.Value.TryGetProperty("identityToken", out var identityToken) && !string.IsNullOrEmpty(identityToken.GetString())) | |
| { | |
| _logger.DockerRegistryCredentialFound(hostname); | |
| return new DockerRegistryAuthenticationConfiguration(authProperty.Name, null, null, identityToken.GetString()); | |
| } |
Can you please cover the changes with tests.
|
Due to inactivity, I will be closing this pull request. However, please do not hesitate to reopen it again if you decide to resume working on it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This form of authentication is typically used by private registries which use OAuth2 authentication, for example Azure Container Registry (ACR). Specifically this PR fixes the situation where the identity token is given directly in the Docker
config.jsonfile.What does this PR do?
Proposes a fix for issue 841
Note: It is outside the scope of testcontainers library to obtain the token.
Ref : Docker's documentation
Why is it important?
Support for Azure Container Registry from CI pipeline.
Related issues
Relates #841
How to test this PR
az loginaz acr login -name myprivateregistry. This obtains a 3 hour token and uses thedockerCLI to write the identitytoken into your local Docker config file. If you are doing this on a host which has a credstore available then "unfortunately" the Docker CLI is clever enough to use that. This is not good, as then you'll be testing something which testcontainers-dotnet already knows how to handle. The workaround for that is to do as follows:~/.docker/config.jsonand verify that is has an explicitidentitytoken.Implementation notes
This PR simply enhances on the
Base64Providerclass. This is a little bit quick-and-dirty as technically identitytoken-based authentication has nothing to do with base64, so the name is slightly misleading for the identitytoken use-case. It may be "cleaner" to create a new provider class, e.g.IdentityTokenProvider.cs, although it would share 90% of its code with the existingBase64Provider.