Add AcquireTokenForApp(scopes) to ITokenAcquisition
#39
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ jobs: | |
| build: | ||
|
|
||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Setup .NET Core | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,5 +88,16 @@ public interface ITokenAcquisition | |
| void ReplyForbiddenWithWwwAuthenticateHeader( | ||
| IEnumerable<string> scopes, | ||
| MsalUiRequiredException msalSeviceException); | ||
|
|
||
| /// <summary> | ||
|
There was a problem hiding this comment. @jmprieur no idea what you want said here. please review. :) #Resolved |
||
| /// Acquires a token from the authority configured in the app, for the confidential client itself (not on behalf of a user) | ||
| /// using the client credentials flow. See https://aka.ms/msal-net-client-credentials. | ||
| /// </summary> | ||
| /// <param name="scopes">scopes requested to access a protected API. For this flow (client credentials), the scopes | ||
| /// should be of the form "{ResourceIdUri/.default}" for instance <c>https://management.azure.net/.default</c> or, for Microsoft | ||
| /// Graph, <c>https://graph.microsoft.com/.default</c> as the requested scopes are defined statically with the application registration | ||
| /// in the portal, and cannot be overriden in the application.</param> | ||
| /// <returns>An access token for the app itself, based on its scopes</returns> | ||
| Task<string> AcquireTokenForAppAsync(IEnumerable<string> scopes); | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ public class TokenAcquisition : ITokenAcquisition | |
|
|
||
| private readonly IMsalTokenCacheProvider _tokenCacheProvider; | ||
|
|
||
| private IConfidentialClientApplication _application; | ||
| private readonly IHttpContextAccessor _httpContextAccessor; | ||
| private HttpContext CurrentHttpContext => _httpContextAccessor.HttpContext; | ||
| private readonly ILogger _logger; | ||
|
|
@@ -110,7 +110,7 @@ public async Task AddAccountToCacheFromAuthorizationCodeAsync( | |
| { | ||
| throw new ArgumentNullException(nameof(scopes)); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // As AcquireTokenByAuthorizationCodeAsync is asynchronous we want to tell ASP.NET core that we are handing the code | ||
|
|
@@ -125,20 +125,20 @@ public async Task AddAccountToCacheFromAuthorizationCodeAsync( | |
| (context.HttpContext.User.Identity as ClaimsIdentity).AddClaims(context.Principal.Claims); | ||
| } | ||
|
|
||
| _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); | ||
|
|
||
| // Do not share the access token with ASP.NET Core otherwise ASP.NET will cache it and will not send the OAuth 2.0 request in | ||
| // case a further call to AcquireTokenByAuthorizationCodeAsync in the future is required for incremental consent (getting a code requesting more scopes) | ||
| // Share the ID Token though | ||
| var result = await _application | ||
| .AcquireTokenByAuthorizationCode(scopes.Except(_scopesRequestedByMsal), context.ProtocolMessage.Code) | ||
| .ExecuteAsync() | ||
| .ConfigureAwait(false); | ||
| context.HandleCodeRedemption(null, result.IdToken); | ||
| } | ||
| catch (MsalException ex) | ||
| { | ||
| _logger.LogInformation(ex, "Exception occured while adding an account to the cache from the auth code. "); | ||
|
There was a problem hiding this comment. We should discuss putting all error strings into a separate class. |
||
| throw; | ||
| } | ||
| } | ||
|
|
@@ -195,15 +195,14 @@ public async Task<string> GetAccessTokenForUserAsync( | |
| } | ||
|
|
||
| // Use MSAL to get the right token to call the API | ||
| _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); | ||
| string accessToken; | ||
|
|
||
| try | ||
| { | ||
| accessToken = await GetAccessTokenOnBehalfOfUserFromCacheAsync(_application, CurrentHttpContext.User, scopes, tenant) | ||
| .ConfigureAwait(false); | ||
| } | ||
| catch (MsalUiRequiredException ex) | ||
| { | ||
| // GetAccessTokenForUserAsync is an abstraction that can be called from a Web App or a Web API | ||
|
|
@@ -219,7 +218,7 @@ public async Task<string> GetAccessTokenForUserAsync( | |
| // In the case the token is a JWE (encrypted token), we use the decrypted token. | ||
| string tokenUsedToCallTheWebApi = validatedToken.InnerToken == null ? validatedToken.RawData | ||
| : validatedToken.InnerToken.RawData; | ||
| var result = await _application | ||
| .AcquireTokenOnBehalfOf(scopes.Except(_scopesRequestedByMsal), | ||
| new UserAssertion(tokenUsedToCallTheWebApi)) | ||
| .ExecuteAsync() | ||
|
|
@@ -238,6 +237,34 @@ public async Task<string> GetAccessTokenForUserAsync( | |
| return accessToken; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
There was a problem hiding this comment. @jmprieur please review comment #Resolved |
||
| /// Acquires a token from the authority configured in the app, for the confidential client itself (not on behalf of a user) | ||
| /// using the client credentials flow. See https://aka.ms/msal-net-client-credentials. | ||
| /// </summary> | ||
| /// <param name="scopes">scopes requested to access a protected API. For this flow (client credentials), the scopes | ||
| /// should be of the form "{ResourceIdUri/.default}" for instance <c>https://management.azure.net/.default</c> or, for Microsoft | ||
| /// Graph, <c>https://graph.microsoft.com/.default</c> as the requested scopes are defined statically with the application registration | ||
| /// in the portal, and cannot be overriden in the application.</param> | ||
| /// <returns>An access token for the app itself, based on its scopes</returns> | ||
| public async Task<string> AcquireTokenForAppAsync(IEnumerable<string> scopes) | ||
| { | ||
| if (scopes == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(scopes)); | ||
| } | ||
|
|
||
| // Use MSAL to get the right token to call the API | ||
| _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); | ||
|
|
||
| AuthenticationResult result; | ||
| result = await _application | ||
| .AcquireTokenForClient(scopes.Except(_scopesRequestedByMsal)) | ||
| .ExecuteAsync() | ||
| .ConfigureAwait(false); | ||
|
|
||
| return result.AccessToken; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Removes the account associated with context.HttpContext.User from the MSAL.NET cache | ||
| /// </summary> | ||
|
|
@@ -247,7 +274,7 @@ public async Task<string> GetAccessTokenForUserAsync( | |
| public async Task RemoveAccountAsync(RedirectContext context) | ||
| { | ||
| ClaimsPrincipal user = context.HttpContext.User; | ||
| IConfidentialClientApplication app = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); | ||
| IAccount account = null; | ||
|
|
||
| // For B2C, we should remove all accounts of the user regardless the user flow | ||
|
|
@@ -287,61 +314,67 @@ public async Task RemoveAccountAsync(RedirectContext context) | |
| /// </summary> | ||
| /// <param name="claimsPrincipal"></param> | ||
| /// <returns></returns> | ||
| private async Task<IConfidentialClientApplication> GetOrBuildConfidentialClientApplicationAsync() | ||
| { | ||
|
There was a problem hiding this comment. We should discuss renaming this method (and any ones like it). Having 'or' or 'and' in the method name suggests that a method has more than one responsibility. In this case I would probably rename it to just 'GetConfidential...' since the caller only cares about getting an app instance; whether it's built or not is internal/abstracted detail. There was a problem hiding this comment.
|
||
| if (_application == null) | ||
| { | ||
| _application = await BuildConfidentialClientApplicationAsync().ConfigureAwait(false); | ||
| } | ||
| return _application; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates an MSAL Confidential client application | ||
| /// </summary> | ||
| /// <param name="claimsPrincipal"></param> | ||
| /// <returns></returns> | ||
| private async Task<IConfidentialClientApplication> BuildConfidentialClientApplicationAsync() | ||
| { | ||
| var request = CurrentHttpContext.Request; | ||
| string currentUri = UriHelper.BuildAbsolute( | ||
| request.Scheme, | ||
| request.Host, | ||
| request.PathBase, | ||
| _microsoftIdentityOptions.CallbackPath.Value ?? string.Empty); | ||
|
|
||
| if (!_applicationOptions.Instance.EndsWith("/")) | ||
| _applicationOptions.Instance += "/"; | ||
|
|
||
| string authority; | ||
| IConfidentialClientApplication app; | ||
|
|
||
| try | ||
| { | ||
| if (_microsoftIdentityOptions.IsB2C) | ||
| { | ||
| authority = $"{ _applicationOptions.Instance}tfp/{_microsoftIdentityOptions.Domain}/{_microsoftIdentityOptions.DefaultUserFlow}"; | ||
| app = ConfidentialClientApplicationBuilder | ||
| .CreateWithApplicationOptions(_applicationOptions) | ||
| .WithRedirectUri(currentUri) | ||
| .WithB2CAuthority(authority) | ||
| .Build(); | ||
| } | ||
| else | ||
|
There was a problem hiding this comment. what about ADFS? ADFS does not have tenant ID #WontFix There was a problem hiding this comment. |
||
| { | ||
| authority = $"{ _applicationOptions.Instance}{_applicationOptions.TenantId}/"; | ||
|
|
||
| app = ConfidentialClientApplicationBuilder | ||
| .CreateWithApplicationOptions(_applicationOptions) | ||
| .WithRedirectUri(currentUri) | ||
| .WithAuthority(authority) | ||
| .Build(); | ||
| } | ||
|
|
||
| // Initialize token cache providers | ||
| await _tokenCacheProvider.InitializeAsync(app.AppTokenCache).ConfigureAwait(false); | ||
|
|
||
| await _tokenCacheProvider.InitializeAsync(app.UserTokenCache).ConfigureAwait(false); | ||
| return app; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogInformation(ex, "Exception acquiring token for a confidential client. "); | ||
| throw; | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -465,8 +498,8 @@ public void ReplyForbiddenWithWwwAuthenticateHeader(IEnumerable<string> scopes, | |
| } | ||
| } | ||
|
|
||
| string consentUrl = $"{_application.Authority}/oauth2/v2.0/authorize?client_id={_applicationOptions.ClientId}" | ||
| + $"&response_type=code&redirect_uri={_application.AppConfig.RedirectUri}" | ||
| + $"&response_mode=query&scope=offline_access%20{string.Join("%20", scopes)}"; | ||
|
|
||
| IDictionary<string, string> parameters = new Dictionary<string, string>() | ||
|
|
||
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,11 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>netcoreapp3.1</TargetFramework> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\src\Microsoft.Identity.Web\Microsoft.Identity.Web.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
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.
Fancy new C# 8 feature you're
usingthere :) #Resolved