-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 ACR challenge-based authentication and remove basic auth policy #19696
Merged
Merged
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
a5bf384
add challenge-based auth policy
annelo-msft f042182
retrieve aad access token from request header.
annelo-msft ba63a01
use two http pipelines
annelo-msft 9558c37
test cleanup and re-record
annelo-msft 2f371f1
update api
annelo-msft f6d2e08
rename policy and begin work on test.
annelo-msft 5676111
upgrade to latest swagger to reduce number of auth clients
annelo-msft 61a120d
add policy test
annelo-msft 9ac249a
update api for changes from generated code
annelo-msft d9a1f57
parameter refactor
annelo-msft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
131 changes: 131 additions & 0 deletions
131
...ure.Containers.ContainerRegistry/src/Authentication/ContainerRegistryCredentialsPolicy.cs
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Azure.Core; | ||
using Azure.Core.Pipeline; | ||
|
||
namespace Azure.Containers.ContainerRegistry | ||
{ | ||
/// <summary> | ||
/// Challenge-based authentication policy for Container Registry Service. | ||
/// | ||
/// The challenge-based authorization flow for ACR is illustrated in the following steps. | ||
/// For example, GET /api/v1/acr/repositories translates into the following calls. | ||
/// | ||
/// Step 1: GET /api/v1/acr/repositories | ||
/// Return Header: 401: www-authenticate header - Bearer realm="{url}",service="{serviceName}",scope="{scope}",error="invalid_token" | ||
/// | ||
/// Step 2: Retrieve the serviceName, scope from the WWW-Authenticate header. (Parse the string.) | ||
/// | ||
/// Step 3: POST /api/oauth2/exchange | ||
/// Request Body : { service, scope, grant-type, aadToken with ARM scope } | ||
/// Response Body: { acrRefreshToken } | ||
/// | ||
/// Step 4: POST /api/oauth2/token | ||
/// Request Body: { acrRefreshToken, scope, grant-type } | ||
/// Response Body: { acrAccessToken } | ||
/// | ||
/// Step 5: GET /api/v1/acr/repositories | ||
/// Request Header: { Bearer acrTokenAccess } | ||
/// </summary> | ||
internal class ContainerRegistryCredentialsPolicy : BearerTokenChallengeAuthenticationPolicy | ||
{ | ||
private readonly RefreshTokensRestClient _exchangeRestClient; | ||
private readonly AccessTokensRestClient _tokenRestClient; | ||
|
||
public ContainerRegistryCredentialsPolicy(TokenCredential credential, string aadScope, RefreshTokensRestClient exchangeRestClient, AccessTokensRestClient tokenRestClient) | ||
: base(credential, aadScope) | ||
{ | ||
Argument.AssertNotNull(credential, nameof(credential)); | ||
Argument.AssertNotNull(aadScope, nameof(aadScope)); | ||
|
||
_exchangeRestClient = exchangeRestClient; | ||
_tokenRestClient = tokenRestClient; | ||
} | ||
|
||
protected override async ValueTask<bool> AuthenticateRequestOnChallengeAsync(HttpMessage message, bool async) | ||
{ | ||
// Once we're here, we've completed Step 1. | ||
|
||
// Step 2: Parse challenge string to retrieve serviceName and scope, where scope is the ACR Scope | ||
var service = AuthorizationChallengeParser.GetChallengeParameterFromResponse(message.Response, "Bearer", "service"); | ||
var scope = AuthorizationChallengeParser.GetChallengeParameterFromResponse(message.Response, "Bearer", "scope"); | ||
|
||
string acrAccessToken = string.Empty; | ||
if (async) | ||
{ | ||
// Step 3: Exchange AAD Access Token for ACR Refresh Token | ||
string acrRefreshToken = await ExchangeAadAccessTokenForAcrRefreshTokenAsync(message, service, true).ConfigureAwait(false); | ||
|
||
// Step 4: Send in acrRefreshToken and get back acrAccessToken | ||
acrAccessToken = await ExchangeAcrRefreshTokenForAcrAccessTokenAsync(acrRefreshToken, service, scope, true).ConfigureAwait(false); | ||
} | ||
else | ||
{ | ||
// Step 3: Exchange AAD Access Token for ACR Refresh Token | ||
string acrRefreshToken = ExchangeAadAccessTokenForAcrRefreshTokenAsync(message, service, false).EnsureCompleted(); | ||
|
||
// Step 4: Send in acrRefreshToken and get back acrAccessToken | ||
acrAccessToken = ExchangeAcrRefreshTokenForAcrAccessTokenAsync(acrRefreshToken, service, scope, false).EnsureCompleted(); | ||
} | ||
|
||
// Step 5 - Authorize Request. Note, we don't use SetAuthorizationHeader here, because it | ||
// sets an AAD access token header, and at this point we're done with AAD and using an ACR access token. | ||
message.Request.Headers.SetValue(HttpHeader.Names.Authorization, $"Bearer {acrAccessToken}"); | ||
|
||
return true; | ||
annelo-msft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private async Task<string> ExchangeAadAccessTokenForAcrRefreshTokenAsync(HttpMessage message, string service, bool async) | ||
{ | ||
string aadAccessToken = GetAuthorizationHeader(message); | ||
|
||
Response<RefreshToken> acrRefreshToken = null; | ||
if (async) | ||
{ | ||
acrRefreshToken = await _exchangeRestClient.GetFromExchangeAsync( | ||
PostContentSchemaGrantType.AccessToken, | ||
service, | ||
accessToken: aadAccessToken).ConfigureAwait(false); | ||
} | ||
else | ||
{ | ||
acrRefreshToken = _exchangeRestClient.GetFromExchange( | ||
PostContentSchemaGrantType.AccessToken, | ||
service, | ||
accessToken: aadAccessToken); | ||
} | ||
|
||
return acrRefreshToken.Value.RefreshTokenValue; | ||
} | ||
|
||
private async Task<string> ExchangeAcrRefreshTokenForAcrAccessTokenAsync(string acrRefreshToken, string service, string scope, bool async) | ||
{ | ||
Response<AccessToken> acrAccessToken = null; | ||
if (async) | ||
{ | ||
acrAccessToken = await _tokenRestClient.GetAsync(service, scope, acrRefreshToken).ConfigureAwait(false); | ||
} | ||
else | ||
{ | ||
acrAccessToken = _tokenRestClient.Get(service, scope, acrRefreshToken); | ||
} | ||
|
||
return acrAccessToken.Value.AccessTokenValue; | ||
} | ||
|
||
private static string GetAuthorizationHeader(HttpMessage message) | ||
{ | ||
string aadAuthHeader; | ||
if (!message.Request.Headers.TryGetValue(HttpHeader.Names.Authorization, out aadAuthHeader)) | ||
{ | ||
throw new InvalidOperationException("Failed to retrieve Authentication header from message request."); | ||
} | ||
|
||
return aadAuthHeader.Remove(0, "Bearer ".Length); | ||
} | ||
} | ||
} |
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 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 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
53 changes: 0 additions & 53 deletions
53
...nerregistry/Azure.Containers.ContainerRegistry/src/Temporary/BasicAuthenticationPolicy.cs
This file was deleted.
Oops, something went wrong.
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 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
Oops, something went wrong.
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.
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.
Do we have tests for this policy?
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.
Yes! see: sdk/containerregistry/Azure.Containers.ContainerRegistry/tests/Authentication/ContainerRegistryChallengeAuthenticationPolicyTest.cs