From 9fefbb3a8feb1a9646185a0b5440938020c7114f Mon Sep 17 00:00:00 2001 From: jennyf19 Date: Tue, 4 Aug 2020 11:03:29 -0700 Subject: [PATCH] move obo call to earlier (#383) * initial commit to improve obo * add test & experiment w/renaming a few private methods * Jennyf/merge from master (#390) * Fix nullable warnings. (#362) * fix userflow claims in addAccounToCacheFromAuthCode (#388) * make aadIssuerValidator public (#385) * Fixes Azure DevOps issue 1097354 (#387) Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com> Co-authored-by: jennyf19 * pr feedback Co-authored-by: Jean-Marc Prieur Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com> --- .../Constants/LogMessages.cs | 2 + .../TokenAcquisition.cs | 165 +++++++++++------- .../TestConstants.cs | 3 +- .../ClaimsPrincipalExtensionsTests.cs | 6 +- .../MicrosoftIdentityOptionsTests.cs | 10 +- .../TokenAcquisitionAuthorityTests.cs | 83 +++++++++ 6 files changed, 197 insertions(+), 72 deletions(-) create mode 100644 tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs diff --git a/src/Microsoft.Identity.Web/Constants/LogMessages.cs b/src/Microsoft.Identity.Web/Constants/LogMessages.cs index e15abe6be..c361f5ee1 100644 --- a/src/Microsoft.Identity.Web/Constants/LogMessages.cs +++ b/src/Microsoft.Identity.Web/Constants/LogMessages.cs @@ -21,5 +21,7 @@ internal static class LogMessages public const string SessionCacheKeyNotFound = "Cache key {0} not found in session {1}. "; public const string SerializingSessionCache = "Serializing session {0}, cache key {1}. "; public const string ClearingSessionCache = "Clearing session {0}, cache key {1}. "; + + public const string ErrorAcquiringTokenForDownstreamWebApi = "Error acquiring a token for a downstream web API - MsalUiRequiredException message is: {0} ."; } } diff --git a/src/Microsoft.Identity.Web/TokenAcquisition.cs b/src/Microsoft.Identity.Web/TokenAcquisition.cs index 70aa41acc..487ce60c5 100644 --- a/src/Microsoft.Identity.Web/TokenAcquisition.cs +++ b/src/Microsoft.Identity.Web/TokenAcquisition.cs @@ -211,93 +211,82 @@ public async Task GetAccessTokenForUserAsync( string? userFlow = null, ClaimsPrincipal? user = null) { - if (user == null && _httpContextAccessor.HttpContext != null) - { - user = _httpContextAccessor.HttpContext.User; - } - - if (user == null) - { - try - { - AuthenticationStateProvider? authenticationStateProvider = - _serviceProvider.GetService(typeof(AuthenticationStateProvider)) - as AuthenticationStateProvider; - - if (authenticationStateProvider != null) - { - // AuthenticationState provider is only available in Blazor - AuthenticationState state = await authenticationStateProvider.GetAuthenticationStateAsync().ConfigureAwait(false); - user = state.User; - } - } - catch - { - } - } - if (scopes == null) { throw new ArgumentNullException(nameof(scopes)); } - // Use MSAL to get the right token to call the API - _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); - string accessToken; - string authority; + user = await GetAuthenticatedUserAsync(user).ConfigureAwait(false); - if (!string.IsNullOrEmpty(tenant)) - { - authority = _application.Authority.Replace(new Uri(_application.Authority).PathAndQuery, $"/{tenant}/"); - } - else - { - authority = _application.Authority; - } + _application = await GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false); + string authority = CreateAuthorityBasedOnTenantIfProvided(_application, tenant); + string? accessToken; try { - accessToken = await GetAccessTokenOnBehalfOfUserFromCacheAsync( + // Access token will return if call is from a web API + accessToken = await GetTokenForWebApiToCallDownstreamApiAsync( _application, - user, - scopes, authority, - userFlow) - .ConfigureAwait(false); + scopes).ConfigureAwait(false); + + if (!string.IsNullOrEmpty(accessToken)) + { + return accessToken; + } + + // If access token is null, this is a web app + return await GetAccessTokenForWebAppWithAccountFromCacheAsync( + _application, + user, + scopes, + authority, + userFlow) + .ConfigureAwait(false); } catch (MsalUiRequiredException ex) { - // GetAccessTokenForUserAsync is an abstraction that can be called from a web app or a web API + // GetAccessTokenForUserAsync is an abstraction that can be called from a Web App or a Web API _logger.LogInformation(ex.Message); - // to get a token for a Web API on behalf of the user, but not necessarily with the on behalf of OAuth2.0 - // flow as this one only applies to Web APIs. + // Case of the Web App: we let the MsalUiRequiredException be caught by the + // AuthorizeForScopesAttribute exception filter so that the user can consent, do 2FA, etc ... + throw new MicrosoftIdentityWebChallengeUserException(ex, scopes.ToArray()); + } + } + + private async Task GetTokenForWebApiToCallDownstreamApiAsync( + IConfidentialClientApplication application, + string authority, + IEnumerable scopes) + { + try + { + // In web API, validatedToken will not be null JwtSecurityToken? validatedToken = CurrentHttpContext?.GetTokenUsedToCallWebAPI(); - // Case of web APIs: we need to do an on-behalf-of flow + // Case of web APIs: we need to do an on-behalf-of flow, with the token used to call the API if (validatedToken != null) { // 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 + var result = await application .AcquireTokenOnBehalfOf(scopes.Except(_scopesRequestedByMsal), new UserAssertion(tokenUsedToCallTheWebApi)) .WithSendX5C(_microsoftIdentityOptions.SendX5C) .WithAuthority(authority) .ExecuteAsync() .ConfigureAwait(false); - accessToken = result.AccessToken; + return result.AccessToken; } - // Case of the Web App: we let the MsalUiRequiredException be caught by the - // AuthorizeForScopesAttribute exception filter so that the user can consent, do 2FA, etc ... - else - { - throw new MicrosoftIdentityWebChallengeUserException(ex, scopes.ToArray()); - } + return null; + } + catch (MsalUiRequiredException ex) + { + _logger.LogInformation(string.Format(CultureInfo.InvariantCulture, LogMessages.ErrorAcquiringTokenForDownstreamWebApi, ex.Message)); + throw ex; } - - return accessToken; } /// @@ -456,7 +445,7 @@ private async Task BuildConfidentialClientApplic /// (optional) Authority based on a specific tenant for which to acquire a token to access the scopes /// on behalf of the user described in the claimsPrincipal. /// Azure AD B2C user flow to target. - private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( + private async Task GetAccessTokenForWebAppWithAccountFromCacheAsync( IConfidentialClientApplication application, ClaimsPrincipal? claimsPrincipal, IEnumerable scopes, @@ -481,7 +470,7 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( } } - return await GetAccessTokenOnBehalfOfUserFromCacheAsync( + return await GetAccessTokenForWebAppWithAccountFromCacheAsync( application, account, scopes, @@ -498,7 +487,7 @@ private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( /// Authority based on a specific tenant for which to acquire a token to access the scopes /// on behalf of the user. /// Azure AD B2C user flow. - private async Task GetAccessTokenOnBehalfOfUserFromCacheAsync( + private async Task GetAccessTokenForWebAppWithAccountFromCacheAsync( IConfidentialClientApplication application, IAccount? account, IEnumerable scopes, @@ -573,11 +562,14 @@ public async Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(IEnumerable $"{p.Key}=\"{p.Value}\"")); - var httpResponse = CurrentHttpContext.Response; - var headers = httpResponse.Headers; - httpResponse.StatusCode = (int)HttpStatusCode.Forbidden; + if (CurrentHttpContext != null) + { + var httpResponse = CurrentHttpContext.Response; + var headers = httpResponse.Headers; + httpResponse.StatusCode = (int)HttpStatusCode.Forbidden; - headers[HeaderNames.WWWAuthenticate] = new StringValues($"{Constants.Bearer} {parameterString}"); + headers[HeaderNames.WWWAuthenticate] = new StringValues($"{Constants.Bearer} {parameterString}"); + } } private static bool AcceptedTokenVersionMismatch(MsalUiRequiredException msalSeviceException) @@ -588,5 +580,52 @@ private static bool AcceptedTokenVersionMismatch(MsalUiRequiredException msalSev // This is subject to change in the future return msalSeviceException.Message.Contains(ErrorCodes.B2CPasswordResetErrorCode, StringComparison.InvariantCulture); } + + private async Task GetAuthenticatedUserAsync(ClaimsPrincipal? user) + { + if (user == null && _httpContextAccessor.HttpContext?.User != null) + { + user = _httpContextAccessor.HttpContext.User; + } + + if (user == null) + { + try + { + AuthenticationStateProvider? authenticationStateProvider = + _serviceProvider.GetService(typeof(AuthenticationStateProvider)) + as AuthenticationStateProvider; + + if (authenticationStateProvider != null) + { + // AuthenticationState provider is only available in Blazor + AuthenticationState state = await authenticationStateProvider.GetAuthenticationStateAsync().ConfigureAwait(false); + user = state.User; + } + } + catch + { + } + } + + return user; + } + + internal /*for tests*/ string CreateAuthorityBasedOnTenantIfProvided( + IConfidentialClientApplication application, + string? tenant) + { + string authority; + if (!string.IsNullOrEmpty(tenant)) + { + authority = application.Authority.Replace(new Uri(application.Authority).PathAndQuery, $"/{tenant}/"); + } + else + { + authority = application.Authority; + } + + return authority; + } } } diff --git a/tests/Microsoft.Identity.Web.Test.Common/TestConstants.cs b/tests/Microsoft.Identity.Web.Test.Common/TestConstants.cs index 52c8741b1..c4b0d9aeb 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/TestConstants.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/TestConstants.cs @@ -20,7 +20,8 @@ public static class TestConstants public const string UserTwo = "User Two"; public const string ClientId = "87f0ee88-8251-48b3-8825-e0c9563f5234"; - public const string TenantId = "guest-tenant-id"; + public const string GuestTenantId = "guest-tenant-id"; + public const string HomeTenantId = "home-tenant-id"; public const string TenantIdAsGuid = "da41245a5-11b3-996c-00a8-4d99re19f292"; public const string ObjectIdAsGuid = "6364bb70-9521-3fa8-989d-c2c19ff90223"; public const string Domain = "contoso.onmicrosoft.com"; diff --git a/tests/Microsoft.Identity.Web.Test/ClaimsPrincipalExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/ClaimsPrincipalExtensionsTests.cs index 5c40d5c38..5669b6e41 100644 --- a/tests/Microsoft.Identity.Web.Test/ClaimsPrincipalExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/ClaimsPrincipalExtensionsTests.cs @@ -129,17 +129,17 @@ public void GetTenantId_WithTidOrTenantIdClaims_ReturnsTenantId() var claimsPrincipalWithTenantId = new ClaimsPrincipal( new ClaimsIdentity(new Claim[] { - new Claim(ClaimConstants.TenantId, TestConstants.TenantId), + new Claim(ClaimConstants.TenantId, TestConstants.GuestTenantId), })); var claimsPrincipalWithTidAndTenantId = new ClaimsPrincipal( new ClaimsIdentity(new Claim[] { new Claim(ClaimConstants.Tid, TestConstants.TenantIdAsGuid), - new Claim(ClaimConstants.TenantId, TestConstants.TenantId), + new Claim(ClaimConstants.TenantId, TestConstants.GuestTenantId), })); Assert.Equal(TestConstants.TenantIdAsGuid, claimsPrincipalWithTid.GetTenantId()); - Assert.Equal(TestConstants.TenantId, claimsPrincipalWithTenantId.GetTenantId()); + Assert.Equal(TestConstants.GuestTenantId, claimsPrincipalWithTenantId.GetTenantId()); Assert.Equal(TestConstants.TenantIdAsGuid, claimsPrincipalWithTidAndTenantId.GetTenantId()); } diff --git a/tests/Microsoft.Identity.Web.Test/MicrosoftIdentityOptionsTests.cs b/tests/Microsoft.Identity.Web.Test/MicrosoftIdentityOptionsTests.cs index 35fc1889b..83a9274c5 100644 --- a/tests/Microsoft.Identity.Web.Test/MicrosoftIdentityOptionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/MicrosoftIdentityOptionsTests.cs @@ -38,11 +38,11 @@ public void IsB2C_NullOrEmptyUserFlow_ReturnsFalse() } [Theory] - [InlineData(TestConstants.ClientId, TestConstants.AadInstance, TestConstants.TenantId, null, AzureAd, null)] - [InlineData(null, TestConstants.AadInstance, TestConstants.TenantId, null, null, AzureAd, MissingParam.ClientId)] - [InlineData("", TestConstants.AadInstance, TestConstants.TenantId, null, null, AzureAd, MissingParam.ClientId)] - [InlineData(TestConstants.ClientId, null, TestConstants.TenantId, null, null, AzureAd, MissingParam.Instance)] - [InlineData(TestConstants.ClientId, "", TestConstants.TenantId, null, null, AzureAd, MissingParam.Instance)] + [InlineData(TestConstants.ClientId, TestConstants.AadInstance, TestConstants.GuestTenantId, null, AzureAd, null)] + [InlineData(null, TestConstants.AadInstance, TestConstants.GuestTenantId, null, null, AzureAd, MissingParam.ClientId)] + [InlineData("", TestConstants.AadInstance, TestConstants.GuestTenantId, null, null, AzureAd, MissingParam.ClientId)] + [InlineData(TestConstants.ClientId, null, TestConstants.GuestTenantId, null, null, AzureAd, MissingParam.Instance)] + [InlineData(TestConstants.ClientId, "", TestConstants.GuestTenantId, null, null, AzureAd, MissingParam.Instance)] [InlineData(TestConstants.ClientId, TestConstants.AadInstance, null, null, null, AzureAd, MissingParam.TenantId)] [InlineData(TestConstants.ClientId, TestConstants.AadInstance, "", null, null, AzureAd, MissingParam.TenantId)] [InlineData(TestConstants.ClientId, TestConstants.B2CInstance, null, TestConstants.B2CSignUpSignInUserFlow, TestConstants.B2CTenant, AzureAdB2C)] diff --git a/tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs b/tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs new file mode 100644 index 000000000..763921f7b --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Globalization; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Microsoft.Identity.Client; +using Microsoft.Identity.Web.Test.Common; +using Xunit; + +namespace Microsoft.Identity.Web.Test +{ + public class TokenAcquisitionAuthorityTests + { + private TokenAcquisition _tokenAcquisition; + private ServiceProvider _provider; + private ConfidentialClientApplicationOptions _applicationOptions; + + private void InitializeTokenAcquisitionObjects() + { + _tokenAcquisition = new TokenAcquisition( + null, + null, + _provider.GetService>(), + _provider.GetService>(), + null, + null, + _provider); + } + + private void BuildTheRequiredServices() + { + var services = new ServiceCollection(); + + _applicationOptions = new ConfidentialClientApplicationOptions + { + Instance = TestConstants.AadInstance, + ClientId = TestConstants.ConfidentialClientId, + ClientSecret = "cats", + }; + + services.AddTokenAcquisition(); + services.AddTransient( + provider => Options.Create(new MicrosoftIdentityOptions + { + Authority = TestConstants.AuthorityCommonTenant, + ClientId = TestConstants.ConfidentialClientId, + CallbackPath = string.Empty, + })); + services.AddTransient( + provider => Options.Create(_applicationOptions)); + _provider = services.BuildServiceProvider(); + } + + [Theory] + [InlineData(TestConstants.GuestTenantId)] + [InlineData(TestConstants.HomeTenantId)] + [InlineData(null)] + [InlineData("")] + public void VerifyCorrectAuthorityUsedInTokenAcquisitionTests(string tenant) + { + BuildTheRequiredServices(); + InitializeTokenAcquisitionObjects(); + IConfidentialClientApplication app = ConfidentialClientApplicationBuilder + .CreateWithApplicationOptions(_applicationOptions) + .WithAuthority(TestConstants.AuthorityCommonTenant).Build(); + + if (!string.IsNullOrEmpty(tenant)) + { + Assert.Equal( + string.Format( + CultureInfo.InvariantCulture, "{0}/{1}/", TestConstants.AadInstance, tenant), + _tokenAcquisition.CreateAuthorityBasedOnTenantIfProvided( + app, + tenant)); + } + else + { + Assert.Equal(app.Authority, _tokenAcquisition.CreateAuthorityBasedOnTenantIfProvided(app, tenant)); + } + } + } +}