Skip to content
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.

Sign up for GitHub

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

fix tfp double up in legacy b2c domains #796

Merged
merged 3 commits into from
Dec 1, 2020
Merged

fix tfp double up in legacy b2c domains #796

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1486d1
check for tfp
jennyf19 Dec 1, 2020
c0db97f
add more tests
jennyf19 Dec 1, 2020
418d70c
fix null ref in test
jennyf19 Dec 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/Microsoft.Identity.Web/TokenAcquisition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ public async Task RemoveAccountAsync(RedirectContext context)
/// <summary>
/// Creates an MSAL confidential client application, if needed.
/// </summary>
private async Task<IConfidentialClientApplication> GetOrBuildConfidentialClientApplicationAsync()
internal /* for testing */ async Task<IConfidentialClientApplication> GetOrBuildConfidentialClientApplicationAsync()
{
if (_application == null)
{
Expand All @@ -447,7 +447,7 @@ private async Task<IConfidentialClientApplication> BuildConfidentialClientApplic
_microsoftIdentityOptions.CallbackPath.Value ?? string.Empty);
}

_applicationOptions.Instance = _applicationOptions.Instance.TrimEnd('/') + "/";
PrepareAuthorityInstanceForMsal();

if (!string.IsNullOrEmpty(_microsoftIdentityOptions.ClientSecret))
{
Expand Down Expand Up @@ -505,6 +505,16 @@ private async Task<IConfidentialClientApplication> BuildConfidentialClientApplic
}
}

private void PrepareAuthorityInstanceForMsal()
{
if (_microsoftIdentityOptions.IsB2C && _applicationOptions.Instance.EndsWith("/tfp/"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be checked in the case of the instance ending with /tfp also?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. @jmprieur and i discussed this and we are just focusing on a fix for the template, which has /. developers should not organically use this, it's not even in any documentation. this is a quick fix for an issue we have, and a longer term fix is more involved w/msal .net fixes and asp .net templates. if customer have issues, we can look at this, or move forward faster w/the fixes needed in the other libs.

{
_applicationOptions.Instance = _applicationOptions.Instance.Replace("/tfp/", string.Empty).Trim();
}

_applicationOptions.Instance = _applicationOptions.Instance.TrimEnd('/') + "/";
}

private async Task<AuthenticationResult?> GetAuthenticationResultForWebApiToCallDownstreamApiAsync(
IConfidentialClientApplication application,
string authority,
Expand Down
2 changes: 2 additions & 0 deletions tests/Microsoft.Identity.Web.Test.Common/Microsoft.Identity.Web.Test.Common.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="NSubstitute" Version="4.2.2" />
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.164">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand All @@ -24,6 +25,7 @@

<ItemGroup Condition="'$(TargetFramework)' == 'net472' ">
<Compile Remove="Mocks\LoggerMock.cs" />
<Compile Remove="Mocks\MockHttpContextAccessor.cs" />
<Compile Remove="TestHelpers\HttpContextUtilities.cs" />
<PackageReference Include="Microsoft.Extensions.Caching.Abstractions" Version="3.1.0" />
<PackageReference Include="Microsoft.Extensions.Options" Version="3.1.0" />
Expand Down
22 changes: 22 additions & 0 deletions tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpContextAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Microsoft.AspNetCore.Http;
using NSubstitute;

namespace Microsoft.Identity.Web.Test.Common.Mocks
{
public static class MockHttpContextAccessor
{
public static IHttpContextAccessor CreateMockHttpContextAccessor()
{
var mockHttpContextAccessor = Substitute.For<IHttpContextAccessor>();
mockHttpContextAccessor.HttpContext = new DefaultHttpContext();
mockHttpContextAccessor.HttpContext.Request.Scheme = "https";
mockHttpContextAccessor.HttpContext.Request.Host = new HostString("IdentityDotNetSDKAutomation");
mockHttpContextAccessor.HttpContext.Request.PathBase = "/";

return mockHttpContextAccessor;
}
}
}
1 change: 1 addition & 0 deletions tests/Microsoft.Identity.Web.Test.Common/TestConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public static class TestConstants
public const string B2CInstance = "https://fabrikamb2c.b2clogin.com";
public const string B2CInstance2 = "https://catb2c.b2clogin.com";
public const string B2CCustomDomainInstance = "https://catsAreAmazing.com";
public const string B2CLoginMicrosoft = "https://login.microsoftonline.com";
public const string ClientSecret = "catsarecool";

public const string B2CAuthority = B2CInstance + "/" + B2CTenant + "/" + B2CSignUpSignInUserFlow;
Expand Down
43 changes: 10 additions & 33 deletions tests/Microsoft.Identity.Web.Test.Integration/AcquireTokenForAppIntegrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Client;
using Microsoft.Identity.Web.Test.Common;
using Microsoft.Identity.Web.Test.Common.Mocks;
using Microsoft.Identity.Web.Test.Common.TestHelpers;
using Microsoft.Identity.Web.Test.LabInfrastructure;
using Microsoft.Identity.Web.TokenCacheProviders.InMemory;
using NSubstitute;
using Xunit;
using Xunit.Abstractions;
using IHttpContextAccessor = Microsoft.AspNetCore.Http.IHttpContextAccessor;

namespace Microsoft.Identity.Web.Test.Integration
{
Expand Down Expand Up @@ -106,37 +104,16 @@ async Task result() =>

private void InitializeTokenAcquisitionObjects()
{
IOptions<MicrosoftIdentityOptions> microsoftIdentityOptions = _provider.GetService<IOptions<MicrosoftIdentityOptions>>();
IOptions<MsalMemoryTokenCacheOptions> tokenOptions = _provider.GetService<IOptions<MsalMemoryTokenCacheOptions>>();
IOptions<ConfidentialClientApplicationOptions> ccOptions = _provider.GetService<IOptions<ConfidentialClientApplicationOptions>>();
ILogger<TokenAcquisition> logger = _provider.GetService<ILogger<TokenAcquisition>>();
IHttpClientFactory httpClientFactory = _provider.GetService<IHttpClientFactory>();

IHttpContextAccessor httpContextAccessor = CreateMockHttpContextAccessor();

_msalTestTokenCacheProvider = new MsalTestTokenCacheProvider(
_provider.GetService<IMemoryCache>(),
tokenOptions);

_tokenAcquisition = new TokenAcquisition(
_msalTestTokenCacheProvider,
httpContextAccessor,
microsoftIdentityOptions,
ccOptions,
httpClientFactory,
logger,
_provider);
}

private static IHttpContextAccessor CreateMockHttpContextAccessor()
{
var mockHttpContextAccessor = Substitute.For<IHttpContextAccessor>();
mockHttpContextAccessor.HttpContext = new DefaultHttpContext();
mockHttpContextAccessor.HttpContext.Request.Scheme = "https";
mockHttpContextAccessor.HttpContext.Request.Host = new HostString("IdentityDotNetSDKAutomation");
mockHttpContextAccessor.HttpContext.Request.PathBase = "/";

return mockHttpContextAccessor;
new MsalTestTokenCacheProvider(
_provider.GetService<IMemoryCache>(),
_provider.GetService<IOptions<MsalMemoryTokenCacheOptions>>()),
MockHttpContextAccessor.CreateMockHttpContextAccessor(),
_provider.GetService<IOptions<MicrosoftIdentityOptions>>(),
_provider.GetService<IOptions<ConfidentialClientApplicationOptions>>(),
_provider.GetService<IHttpClientFactory>(),
_provider.GetService<ILogger<TokenAcquisition>>(),
_provider);
}

private void BuildTheRequiredServices()
Expand Down
75 changes: 62 additions & 13 deletions tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@
// Licensed under the MIT License.

using System.Globalization;
using System.Net.Http;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Client;
using Microsoft.Identity.Web.Test.Common;
using Microsoft.Identity.Web.Test.Common.Mocks;
using Microsoft.Identity.Web.Test.Common.TestHelpers;
using Microsoft.Identity.Web.TokenCacheProviders.InMemory;
using Xunit;

namespace Microsoft.Identity.Web.Test
Expand All @@ -15,38 +21,36 @@ public class TokenAcquisitionAuthorityTests
private TokenAcquisition _tokenAcquisition;
private ServiceProvider _provider;
private ConfidentialClientApplicationOptions _applicationOptions;
private MicrosoftIdentityOptions _microsoftIdentityOptions;

private void InitializeTokenAcquisitionObjects()
{
_tokenAcquisition = new TokenAcquisition(
null,
null,
new MsalTestTokenCacheProvider(
_provider.GetService<IMemoryCache>(),
_provider.GetService<IOptions<MsalMemoryTokenCacheOptions>>()),
MockHttpContextAccessor.CreateMockHttpContextAccessor(),
_provider.GetService<IOptions<MicrosoftIdentityOptions>>(),
_provider.GetService<IOptions<ConfidentialClientApplicationOptions>>(),
null,
null,
_provider.GetService<IHttpClientFactory>(),
_provider.GetService<ILogger<TokenAcquisition>>(),
_provider);
}

private void BuildTheRequiredServices()
private void BuildTheRequiredServices(string instance = TestConstants.AadInstance)
{
var services = new ServiceCollection();

_applicationOptions = new ConfidentialClientApplicationOptions
{
Instance = TestConstants.AadInstance,
Instance = instance,
ClientId = TestConstants.ConfidentialClientId,
ClientSecret = "cats",
ClientSecret = TestConstants.ClientSecret,
};

services.AddTokenAcquisition();
services.AddTransient(
provider => Options.Create(new MicrosoftIdentityOptions
{
Authority = TestConstants.AuthorityCommonTenant,
ClientId = TestConstants.ConfidentialClientId,
CallbackPath = string.Empty,
}));
provider => Options.Create(_microsoftIdentityOptions));
services.AddTransient(
provider => Options.Create(_applicationOptions));
_provider = services.BuildServiceProvider();
Expand All @@ -59,6 +63,13 @@ private void BuildTheRequiredServices()
[InlineData("")]
public void VerifyCorrectAuthorityUsedInTokenAcquisitionTests(string tenant)
{
_microsoftIdentityOptions = new MicrosoftIdentityOptions
{
Authority = TestConstants.AuthorityCommonTenant,
ClientId = TestConstants.ConfidentialClientId,
CallbackPath = string.Empty,
};

BuildTheRequiredServices();
InitializeTokenAcquisitionObjects();
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
Expand All @@ -79,5 +90,43 @@ public void VerifyCorrectAuthorityUsedInTokenAcquisitionTests(string tenant)
Assert.Equal(app.Authority, _tokenAcquisition.CreateAuthorityBasedOnTenantIfProvided(app, tenant));
}
}

[Theory]
[InlineData(TestConstants.B2CInstance)]
[InlineData(TestConstants.B2CLoginMicrosoft)]
[InlineData(TestConstants.B2CInstance, true)]
[InlineData(TestConstants.B2CLoginMicrosoft, true)]
public async void VerifyCorrectAuthorityUsedInTokenAcquisition_B2CAuthorityTestsAsync(
string authorityInstance,
bool withTfp = false)
{
_microsoftIdentityOptions = new MicrosoftIdentityOptions
{
SignUpSignInPolicyId = TestConstants.B2CSignUpSignInUserFlow,
Domain = TestConstants.B2CTenant,
};

if (withTfp)
{
BuildTheRequiredServices(authorityInstance + "/tfp/");
}
else
{
BuildTheRequiredServices(authorityInstance);
}

InitializeTokenAcquisitionObjects();

IConfidentialClientApplication app = await _tokenAcquisition.GetOrBuildConfidentialClientApplicationAsync().ConfigureAwait(false);

string expectedAuthority = string.Format(
CultureInfo.InvariantCulture,
"{0}/tfp/{1}/{2}/",
authorityInstance,
TestConstants.B2CTenant,
TestConstants.B2CSignUpSignInUserFlow);

Assert.Equal(expectedAuthority, app.Authority);
}
}
}