Skip to content

Commit

Permalink
[Suggestion] Fix two issues with dSTS authority, one when using WithT…
Browse files Browse the repository at this point in the history 
…enantId, and … (#4146)

* Fix two issues with dSTS authority, one when using WithTenantId, and adding dSTS as a supported tenant override. Add a test

* Update fix

* fix tests

---------

Co-authored-by: Bogdan Gavril <bogavril@microsoft.com>
Co-authored-by: Gladwin Johnson <90415114+gladjohn@users.noreply.github.com>
  • Loading branch information
@jennyf19 @bgavrilMS @gladjohn
3 people authored May 22, 2023
1 parent f56d121 commit f2498bc
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 28 deletions.
10 changes: 6 additions & 4 deletions src/client/Microsoft.Identity.Client/ApiConfig/AbstractAcquireTokenParameterBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,13 @@ public T WithTenantId(string tenantId)
MsalErrorMessage.TenantOverrideNonAad);
}

AadAuthority aadAuthority = (AadAuthority)ServiceBundle.Config.Authority;
string tenantedAuthority = aadAuthority.GetTenantedAuthority(tenantId, true);
var newAuthorityInfo = AuthorityInfo.FromAadAuthority(
Authority originalAuthority = ServiceBundle.Config.Authority;
string tenantedAuthority = originalAuthority.GetTenantedAuthority(tenantId, true);

var newAuthorityInfo = new AuthorityInfo(
originalAuthority.AuthorityInfo.AuthorityType,
tenantedAuthority,
ServiceBundle.Config.Authority.AuthorityInfo.ValidateAuthority);
originalAuthority.AuthorityInfo.ValidateAuthority);

CommonParameters.AuthorityOverride = newAuthorityInfo;

Expand Down
29 changes: 19 additions & 10 deletions src/client/Microsoft.Identity.Client/AppConfig/AuthorityInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,25 @@ private AuthorityInfo(

internal bool IsInstanceDiscoverySupported => AuthorityType == AuthorityType.Aad;

internal bool IsUserAssertionSupported => AuthorityType != AuthorityType.Adfs && AuthorityType != AuthorityType.B2C;

internal bool IsTenantOverrideSupported => AuthorityType == AuthorityType.Aad;
internal bool IsMultiTenantSupported => AuthorityType != AuthorityType.Adfs;
internal bool IsClientInfoSupported => AuthorityType == AuthorityType.Aad || AuthorityType == AuthorityType.Dsts || AuthorityType == AuthorityType.B2C;
internal bool IsUserAssertionSupported =>
AuthorityType != AuthorityType.Adfs &&
AuthorityType != AuthorityType.B2C;

internal bool IsTenantOverrideSupported =>
AuthorityType == AuthorityType.Aad ||
AuthorityType == AuthorityType.Dsts;

internal bool IsMultiTenantSupported =>
AuthorityType == AuthorityType.Aad ||
AuthorityType == AuthorityType.Dsts ||
AuthorityType == AuthorityType.B2C ||
AuthorityType == AuthorityType.Ciam;

internal bool IsClientInfoSupported =>
AuthorityType == AuthorityType.Aad ||
AuthorityType == AuthorityType.Dsts ||
AuthorityType == AuthorityType.B2C ||
AuthorityType == AuthorityType.Ciam;

#region Builders
internal static AuthorityInfo FromAuthorityUri(string authorityUri, bool validateAuthority)
Expand Down Expand Up @@ -228,11 +242,6 @@ internal static AuthorityInfo FromAadAuthority(AadAuthorityAudience authorityAud
return new AuthorityInfo(AuthorityType.Aad, authorityUri, validateAuthority);
}

internal static AuthorityInfo FromAadAuthority(string authorityUri, bool validateAuthority)
{
return new AuthorityInfo(AuthorityType.Aad, authorityUri, validateAuthority);
}

internal static AuthorityInfo FromAdfsAuthority(string authorityUri, bool validateAuthority)
{
return new AuthorityInfo(AuthorityType.Adfs, authorityUri, validateAuthority);
Expand Down
4 changes: 2 additions & 2 deletions tests/Microsoft.Identity.Test.Common/TestConstants.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ public static HashSet<string> s_scope
public const string ADFSAuthority2 = "https://someAdfs.com/adfs/";

public const string DstsAuthorityTenantless = "https://some.url.dsts.core.azure-test.net/dstsv2/";
public const string DstsAuthorityTenanted = "https://some.url.dsts.core.azure-test.net/dstsv2/" + TenantIdString;
public const string DstsAuthorityCommon = "https://some.url.dsts.core.azure-test.net/dstsv2/" + Common;
public const string DstsAuthorityTenanted = "https://some.url.dsts.core.azure-test.net/dstsv2/" + TenantId + "/";
public const string DstsAuthorityCommon = "https://some.url.dsts.core.azure-test.net/dstsv2/" + Common + "/";

public const string B2CLoginGlobal = ".b2clogin.com";
public const string B2CLoginUSGov = ".b2clogin.us";
Expand Down
2 changes: 1 addition & 1 deletion tests/Microsoft.Identity.Test.Common/TestData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static IEnumerable<object[]> GetAuthorityWithExpectedTenantId()
yield return new AuthorityWithExpectedTenantId { Authority = new Uri(TestConstants.AuthorityTestTenant), ExpectedTenantId = TestConstants.Utid }.ToObjectArray();
yield return new AuthorityWithExpectedTenantId { Authority = new Uri(TestConstants.AadAuthorityWithTestTenantId), ExpectedTenantId = TestConstants.AadTenantId }.ToObjectArray();
yield return new AuthorityWithExpectedTenantId { Authority = new Uri(TestConstants.AuthorityWindowsNet), ExpectedTenantId = TestConstants.Utid }.ToObjectArray();
yield return new AuthorityWithExpectedTenantId { Authority = new Uri(TestConstants.DstsAuthorityTenanted), ExpectedTenantId = TestConstants.TenantIdString }.ToObjectArray();
yield return new AuthorityWithExpectedTenantId { Authority = new Uri(TestConstants.DstsAuthorityTenanted), ExpectedTenantId = TestConstants.TenantId }.ToObjectArray();
}
}
}
2 changes: 1 addition & 1 deletion tests/Microsoft.Identity.Test.Unit/ApiConfigTests/AuthorityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Microsoft.Identity.Test.Common.Core.Helpers;
using Microsoft.Identity.Test.Common.Core.Mocks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute.ReceivedExtensions;

namespace Microsoft.Identity.Test.Unit.ApiConfigTests
{
Expand Down Expand Up @@ -77,6 +76,7 @@ public void WithTenantIdExceptions()
Assert.AreEqual(ex1.ErrorCode, MsalError.TenantOverrideNonAad);
Assert.AreEqual(ex2.ErrorCode, MsalError.TenantOverrideNonAad);
}


[DataTestMethod]
[DynamicData(nameof(TestData.GetAuthorityWithExpectedTenantId), typeof(TestData), DynamicDataSourceType.Method)]
Expand Down
58 changes: 49 additions & 9 deletions tests/Microsoft.Identity.Test.Unit/CoreTests/InstanceTests/DstsAuthorityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.Identity.Test.Common;
using Microsoft.Identity.Test.Common.Core.Mocks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.Identity.Client.Utils;

namespace Microsoft.Identity.Test.Unit.CoreTests.InstanceTests
{
Expand All @@ -36,7 +37,7 @@ private static MockHttpMessageHandler CreateTokenResponseHttpHandler(string auth

return new MockHttpMessageHandler()
{
ExpectedUrl = $"{authority}/oauth2/v2.0/token",
ExpectedUrl = $"{authority}oauth2/v2.0/token",
ExpectedMethod = HttpMethod.Post,
ExpectedPostData = expectedRequestBody,
ResponseMessage = MockHelpers.CreateSuccessfulClientCredentialTokenResponseMessage(MockHelpers.CreateClientInfo(TestConstants.Uid, TestConstants.Utid))
Expand All @@ -57,7 +58,7 @@ public async Task DstsClientCredentialSuccessfulTestAsync(string authority)
.WithClientSecret(TestConstants.ClientSecret)
.Build();

Assert.AreEqual(authority + "/", app.Authority);
Assert.AreEqual(authority, app.Authority);
var confidentailClientApp = (ConfidentialClientApplication)app;
Assert.AreEqual(AuthorityType.Dsts, confidentailClientApp.AuthorityInfo.AuthorityType);

Expand All @@ -83,6 +84,46 @@ public async Task DstsClientCredentialSuccessfulTestAsync(string authority)
}
}

[TestMethod]
public void DstsAuthorityFlags()
{
var app = ConfidentialClientApplicationBuilder
.Create(TestConstants.ClientId)
.WithAuthority(TestConstants.DstsAuthorityTenanted)
.WithClientSecret("secret")
.Build();

Assert.AreEqual(AuthorityType.Dsts, (app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.AuthorityType);

Assert.IsTrue((app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.IsMultiTenantSupported);
Assert.IsTrue((app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.IsClientInfoSupported);
Assert.IsFalse((app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.IsInstanceDiscoverySupported);
Assert.IsTrue((app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.IsTenantOverrideSupported);
Assert.IsTrue((app.AppConfig as ApplicationConfiguration).Authority.AuthorityInfo.IsUserAssertionSupported);
}

[TestMethod]
public void DstsAuthority_WithTenantId_Success()
{
var app = ConfidentialClientApplicationBuilder
.Create(TestConstants.ClientId)
.WithAuthority(TestConstants.DstsAuthorityTenanted)
.WithClientSecret("secret")
.Build();

Assert.AreEqual(TestConstants.DstsAuthorityTenanted, app.Authority);

// change the tenant id
var parameterBuilder = app.AcquireTokenByAuthorizationCode(TestConstants.s_scope, "code")
.WithTenantId(TestConstants.TenantId2);

// Verify Host still matches the original Authority
Assert.AreEqual(new Uri(TestConstants.DstsAuthorityTenanted).Host, parameterBuilder.CommonParameters.AuthorityOverride.Host);

// Verify the Tenant Id matches
Assert.AreEqual(TestConstants.TenantId2, AuthorityHelpers.GetTenantId(parameterBuilder.CommonParameters.AuthorityOverride.CanonicalAuthority));
}

[DataTestMethod]
[DataRow(TestConstants.DstsAuthorityCommon)]
[DataRow(TestConstants.DstsAuthorityTenanted)]
Expand All @@ -94,9 +135,9 @@ public void DstsEndpointsTest(string authority)
_harness.ServiceBundle,
Guid.NewGuid());

Assert.AreEqual($"{authority}/oauth2/v2.0/token", instance.GetTokenEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"{authority}/oauth2/v2.0/authorize", instance.GetAuthorizationEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"{authority}/oauth2/v2.0/devicecode", instance.GetDeviceCodeEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"{authority}oauth2/v2.0/token", instance.GetTokenEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"{authority}oauth2/v2.0/authorize", instance.GetAuthorizationEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"{authority}oauth2/v2.0/devicecode", instance.GetDeviceCodeEndpointAsync(_testRequestContext).Result);
Assert.AreEqual($"https://some.url.dsts.core.azure-test.net/dstsv2/common/userrealm/", instance.AuthorityInfo.UserRealmUriPrefix);
}

Expand All @@ -118,16 +159,15 @@ public void Validate_MinNumberOfSegments()

[TestMethod]
public void CreateAuthorityFromTenantedWithTenantTest()
{

{
Authority authority = AuthorityTestHelper.CreateAuthorityFromUrl(TestConstants.DstsAuthorityTenanted);
Assert.AreEqual("tenantid", authority.TenantId);
Assert.AreEqual(TestConstants.TenantId, authority.TenantId);

string updatedAuthority = authority.GetTenantedAuthority("tenant2");

Assert.AreEqual(
TestConstants.DstsAuthorityTenanted,
updatedAuthority.TrimEnd('/'),
updatedAuthority,
"Not changed, original authority already has tenant id");

string updatedAuthority2 = authority.GetTenantedAuthority("tenant2", true);
Expand Down
2 changes: 1 addition & 1 deletion tests/Microsoft.Identity.Test.Unit/PublicApiTests/TenantIdTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void TestInitialize()
[DataRow(TestConstants.B2CLoginAuthorityMoonCake, TestConstants.SomeTenantId, DisplayName = "B2C MoonCake Tenant Id")]
[DataRow(TestConstants.AuthoritySovereignCNTenant, TestConstants.TenantId, DisplayName = "Sovereign Tenant Id")]
[DataRow(TestConstants.AuthoritySovereignDETenant, TestConstants.TenantId, DisplayName = "Sovereign Tenant Id")]
[DataRow(TestConstants.DstsAuthorityTenanted, "tenantid", DisplayName = "DSTS Tenant Id")]
[DataRow(TestConstants.DstsAuthorityTenanted, TestConstants.TenantId, DisplayName = "DSTS Tenant Id")]
[DataRow(TestConstants.DstsAuthorityCommon, TestConstants.Common, DisplayName = "DSTS Common Tenant Id")]
public void ParseTest_Success(string authorityUrl, string expectedTenantId)
{
Expand Down

0 comments on commit f2498bc

Please sign in to comment.