-
Notifications
You must be signed in to change notification settings - Fork 4k
Support CAE #16852
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
Support CAE #16852
Changes from all commits
a873a98
1c03b4d
6bf4340
ae63ed0
4b524c2
6b023d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,15 @@ | |
using System.Collections.Generic; | ||
using Microsoft.Azure.Commands.Common.Authentication.Abstractions; | ||
using Microsoft.Azure.Commands.Common.Authentication.Abstractions.Core; | ||
using Microsoft.Azure.Commands.Common.Utilities; | ||
using Microsoft.Azure.Commands.Profile.Models; | ||
using System.Globalization; | ||
using Microsoft.Azure.Commands.Common.Authentication; | ||
using Microsoft.Azure.Commands.ResourceManager.Common.ArgumentCompleters; | ||
using System.Linq; | ||
using System.Management.Automation; | ||
using Microsoft.Azure.Commands.Profile.Properties; | ||
using Azure.Identity; | ||
|
||
namespace Microsoft.Azure.Commands.Common | ||
{ | ||
|
@@ -115,8 +117,7 @@ internal void AddAuthorizeRequestHandler( | |
{ | ||
endpointResourceIdKey = endpointResourceIdKey ?? AzureEnvironment.Endpoint.ResourceManager; | ||
var context = GetDefaultContext(_provider, invocationInfo); | ||
await AuthorizeRequest(context, request, cancelToken, endpointResourceIdKey, endpointSuffixKey, tokenAudienceConverter); | ||
return await next(request, cancelToken, cancelAction, signal); | ||
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. As we disscussed in today's meeting, we would like to revisit the part of the design related to autorest -- instead of adding CAE logic to the However, redoing authentication requires information about the access token (such as environment, audience). If the logic is departed from authentication, how does it get those info? A second concern is about calling 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.
Thanks for @isra-fel's conclusions. After the discussion with @dolauli, we find a way to seperate the authentication and reauthentication steps. However, the codes of these 2 parts are coupled, we decide to include them into one step currently. Thanks |
||
return await AuthenticationHelper(context, endpointResourceIdKey, endpointSuffixKey, request, cancelToken, cancelAction, signal, next); | ||
}); | ||
} | ||
|
||
|
@@ -191,6 +192,35 @@ public object GetParameterValue(string resourceId, string moduleName, Invocation | |
return string.Empty; | ||
} | ||
|
||
internal async Task<HttpResponseMessage> AuthenticationHelper(IAzureContext context, string endpointResourceIdKey, string endpointSuffixKey, HttpRequestMessage request, CancellationToken cancelToken, Action cancelAction, SignalDelegate signal, NextDelegate next, TokenAudienceConverterDelegate tokenAudienceConverter = null) | ||
{ | ||
IAccessToken accessToken = await AuthorizeRequest(context, request, cancelToken, endpointResourceIdKey, endpointSuffixKey, tokenAudienceConverter); | ||
var newRequest = await request.CloneWithContentAndDispose(request.RequestUri, request.Method); | ||
var response = await next(request, cancelToken, cancelAction, signal); | ||
|
||
if (response.MatchClaimsChallengePattern()) | ||
{ | ||
//get token again with claims challenge | ||
if (accessToken is IClaimsChallengeProcessor processor) | ||
{ | ||
try | ||
{ | ||
var claimsChallenge = ClaimsChallengeUtilities.GetClaimsChallenge(response); | ||
if (!string.IsNullOrEmpty(claimsChallenge)) | ||
{ | ||
await processor.OnClaimsChallenageAsync(newRequest, claimsChallenge, cancelToken).ConfigureAwait(false); | ||
response = await next(newRequest, cancelToken, cancelAction, signal); | ||
} | ||
} | ||
catch (AuthenticationFailedException e) | ||
{ | ||
throw e.WithAdditionalMessage(response?.GetWwwAuthenticateMessage()); | ||
} | ||
} | ||
} | ||
return response; | ||
} | ||
|
||
/// <summary> | ||
/// | ||
/// </summary> | ||
|
@@ -202,8 +232,7 @@ internal Func<HttpRequestMessage, CancellationToken, Action, SignalDelegate, Nex | |
return async (request, cancelToken, cancelAction, signal, next) => | ||
{ | ||
PatchRequestUri(context, request); | ||
await AuthorizeRequest(context, request, cancelToken, resourceId, resourceId); | ||
return await next(request, cancelToken, cancelAction, signal); | ||
return await AuthenticationHelper(context, resourceId, resourceId, request, cancelToken, cancelAction, signal, next); | ||
}; | ||
} | ||
|
||
|
@@ -213,17 +242,17 @@ internal Func<HttpRequestMessage, CancellationToken, Action, SignalDelegate, Nex | |
/// <param name="context"></param> | ||
/// <param name="endpointResourceIdKey"></param> | ||
/// <param name="request"></param> | ||
/// <param name="outerToken"></param> | ||
/// <param name="cancellationToken"></param> | ||
/// <returns></returns> | ||
internal async Task AuthorizeRequest(IAzureContext context, HttpRequestMessage request, CancellationToken outerToken, string endpointResourceIdKey, | ||
internal async Task<IAccessToken> AuthorizeRequest(IAzureContext context, HttpRequestMessage request, CancellationToken cancellationToken, string endpointResourceIdKey, | ||
string endpointSuffixKey, TokenAudienceConverterDelegate tokenAudienceConverter = null, IDictionary<string, object> extensibleParamters = null) | ||
{ | ||
if (context == null || context.Account == null || context.Environment == null) | ||
{ | ||
throw new InvalidOperationException(Resources.InvalidAzureContext); | ||
} | ||
|
||
await Task.Run(() => | ||
return await Task.Run(() => | ||
{ | ||
if (tokenAudienceConverter != null) | ||
{ | ||
|
@@ -233,7 +262,8 @@ await Task.Run(() => | |
} | ||
var authToken = _authenticator.Authenticate(context.Account, context.Environment, context.Tenant.Id, null, "Never", null, endpointResourceIdKey); | ||
authToken.AuthorizeRequest((type, token) => request.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue(type, token)); | ||
}, outerToken); | ||
return authToken; | ||
}, cancellationToken); | ||
} | ||
|
||
private (string CurEnvEndpointResourceId, string CurEnvEndpointSuffix, string BaseEnvEndpointResourceId, string BaseEnvEndpointSuffix) GetEndpointInfo(IAzureEnvironment environment, string endpointResourceIdKey, string endpointSuffixKey) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
// ---------------------------------------------------------------------------------- | ||
// | ||
// Copyright Microsoft Corporation | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
// ---------------------------------------------------------------------------------- | ||
|
||
using System.Collections.Generic; | ||
using System.Net.Http; | ||
using System.Threading.Tasks; | ||
|
||
namespace Microsoft.Azure.Commands.Common.Utilities | ||
{ | ||
internal static class HttpRequestMessageExtension | ||
{ | ||
internal static HttpRequestMessage CloneAndDispose(this HttpRequestMessage original, System.Uri requestUri = null, System.Net.Http.HttpMethod method = null) | ||
{ | ||
using (original) | ||
{ | ||
return original.Clone(requestUri, method); | ||
} | ||
} | ||
|
||
internal static Task<HttpRequestMessage> CloneWithContentAndDispose(this HttpRequestMessage original, System.Uri requestUri = null, System.Net.Http.HttpMethod method = null) | ||
{ | ||
using (original) | ||
{ | ||
return original.CloneWithContent(requestUri, method); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Clones an HttpRequestMessage (without the content) | ||
/// </summary> | ||
/// <param name="original">Original HttpRequestMessage (Will be diposed before returning)</param> | ||
/// <returns>A clone of the HttpRequestMessage</returns> | ||
internal static HttpRequestMessage Clone(this HttpRequestMessage original, System.Uri requestUri = null, System.Net.Http.HttpMethod method = null) | ||
{ | ||
var clone = new HttpRequestMessage | ||
{ | ||
Method = method ?? original.Method, | ||
RequestUri = requestUri ?? original.RequestUri, | ||
Version = original.Version, | ||
}; | ||
|
||
foreach (KeyValuePair<string, object> prop in original.Properties) | ||
{ | ||
clone.Properties.Add(prop); | ||
} | ||
|
||
foreach (KeyValuePair<string, IEnumerable<string>> header in original.Headers) | ||
{ | ||
/* | ||
**temporarily skip cloning telemetry related headers** | ||
clone.Headers.TryAddWithoutValidation(header.Key, header.Value); | ||
*/ | ||
if (!"x-ms-unique-id".Equals(header.Key) && !"x-ms-client-request-id".Equals(header.Key) && !"CommandName".Equals(header.Key) && !"FullCommandName".Equals(header.Key) && !"ParameterSetName".Equals(header.Key) && !"User-Agent".Equals(header.Key)) | ||
{ | ||
clone.Headers.TryAddWithoutValidation(header.Key, header.Value); | ||
} | ||
} | ||
|
||
return clone; | ||
} | ||
|
||
/// <summary> | ||
/// Clones an HttpRequestMessage (including the content stream and content headers) | ||
/// </summary> | ||
/// <param name="original">Original HttpRequestMessage (Will be diposed before returning)</param> | ||
/// <returns>A clone of the HttpRequestMessage</returns> | ||
internal static async Task<HttpRequestMessage> CloneWithContent(this HttpRequestMessage original, System.Uri requestUri = null, System.Net.Http.HttpMethod method = null) | ||
{ | ||
var clone = original.Clone(requestUri, method); | ||
var stream = new System.IO.MemoryStream(); | ||
if (original.Content != null) | ||
{ | ||
await original.Content.CopyToAsync(stream).ConfigureAwait(false); | ||
stream.Position = 0; | ||
clone.Content = new StreamContent(stream); | ||
if (original.Content.Headers != null) | ||
{ | ||
foreach (var h in original.Content.Headers) | ||
{ | ||
clone.Content.Headers.Add(h.Key, h.Value); | ||
} | ||
} | ||
} | ||
return clone; | ||
} | ||
} | ||
} |
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.
Shall we move the method out of this class because (1) this class is already very long (2) the logic of parsing MsalServiceException is not strongly tied to Connect-AzAccount (3) it's easier to test
We can have a static helper class that handles details of authentication exceptions.
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.
Currently no other place to use the TryParseUnknownAuthenticationException and its logic is very specific for login. We can extract it when we find other place can share the logic.