Skip to content

Commit cd5172d

Browse files
ksaafksaaf
authored
For Mtls, do not check for Aad and Dsts Authority type and 'organizations' or 'common' tenant (#5174)
This PR removes from AcquireTokenForClientParameterBuilder.Validate checks for 'common' for Aad and Dsts Authority Types. See conversation in this PR.
1 parent 99e094d commit cd5172d

File tree

8 files changed

+59
-68
lines changed

8 files changed

+59
-68
lines changed

‎src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForClientParameterBuilder.cs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System;
55
using System.Collections.Generic;
66
using System.ComponentModel;
7-
using System.Security.Cryptography.X509Certificates;
87
using System.Threading;
98
using System.Threading.Tasks;
109
using Microsoft.Identity.Client.ApiConfig.Executors;
@@ -170,23 +169,6 @@ protected override void Validate()
170169
{
171170
if (CommonParameters.MtlsCertificate != null)
172171
{
173-
string authorityUri = ServiceBundle.Config.Authority.AuthorityInfo.CanonicalAuthority.AbsoluteUri;
174-
175-
if (ServiceBundle.Config.Authority.AuthorityInfo.AuthorityType != AuthorityType.Aad &&
176-
ServiceBundle.Config.Authority.AuthorityInfo.AuthorityType != AuthorityType.Dsts)
177-
{
178-
throw new MsalClientException(
179-
MsalError.InvalidAuthorityType,
180-
MsalErrorMessage.MtlsInvalidAuthorityTypeMessage);
181-
}
182-
183-
if (authorityUri.Contains("/common", StringComparison.OrdinalIgnoreCase))
184-
{
185-
throw new MsalClientException(
186-
MsalError.MissingTenantedAuthority,
187-
MsalErrorMessage.MtlsNonTenantedAuthorityNotAllowedMessage);
188-
}
189-
190172
// Check for Azure region only if the authority is AAD
191173
// AzureRegion is by default set to null or set to null when the application is created
192174
// with region set to DisableForceRegion (see ConfidentialClientApplicationBuilder.Validate)

‎src/client/Microsoft.Identity.Client/AppConfig/AuthorityInfo.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.Globalization;
65
using System.Linq;
76
using System.Threading.Tasks;
8-
using Microsoft.Identity.Client.AppConfig;
97
using Microsoft.Identity.Client.Instance;
108
using Microsoft.Identity.Client.Instance.Validation;
119
using Microsoft.Identity.Client.Internal;
@@ -65,7 +63,7 @@ public AuthorityInfo(
6563
CanonicalAuthority = new Uri($"https://{authorityUri.Authority}/{pathSegments[0]}/{pathSegments[1]}/");
6664

6765
UserRealmUriPrefix = UriBuilderExtensions.GetHttpsUriWithOptionalPort(
68-
$"https://{authorityUri.Authority}/{pathSegments[0]}/common/userrealm/",
66+
$"https://{authorityUri.Authority}/{pathSegments[0]}/{Constants.Common}/userrealm/",
6967
authorityUri.Port);
7068
break;
7169
default:
@@ -75,7 +73,7 @@ public AuthorityInfo(
7573
authorityUri.Port));
7674

7775
UserRealmUriPrefix = UriBuilderExtensions.GetHttpsUriWithOptionalPort(
78-
$"https://{Host}/common/userrealm/",
76+
$"https://{Host}/{Constants.Common}/userrealm/",
7977
authorityUri.Port);
8078
break;
8179
}
@@ -273,11 +271,11 @@ internal static string GetAadAuthorityAudienceValue(AadAuthorityAudience authori
273271
switch (authorityAudience)
274272
{
275273
case AadAuthorityAudience.AzureAdAndPersonalMicrosoftAccount:
276-
return "common";
274+
return Constants.Common;
277275
case AadAuthorityAudience.AzureAdMultipleOrgs:
278-
return "organizations";
276+
return Constants.Organizations;
279277
case AadAuthorityAudience.PersonalMicrosoftAccount:
280-
return "consumers";
278+
return Constants.Consumers;
281279
case AadAuthorityAudience.AzureAdMyOrg:
282280
if (string.IsNullOrWhiteSpace(tenantId))
283281
{
@@ -451,7 +449,7 @@ private static bool IsCiamAuthority(Uri authorityUri)
451449

452450
private static string[] GetPathSegments(string absolutePath)
453451
{
454-
string[] pathSegments = absolutePath.Substring(1).Split(
452+
string[] pathSegments = absolutePath.Split(
455453
new[]
456454
{
457455
'/'

‎src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ internal class AadAuthority : Authority
2121
private static readonly ISet<string> s_tenantlessTenantNames = new HashSet<string>(
2222
new[]
2323
{
24-
Constants.CommonTenant,
25-
Constants.OrganizationsTenant,
26-
Constants.ConsumerTenant
24+
Constants.Common,
25+
Constants.Organizations,
26+
Constants.Consumers,
2727
},
2828
StringComparer.OrdinalIgnoreCase);
2929

@@ -36,7 +36,7 @@ internal AadAuthority(AuthorityInfo authorityInfo) : base(authorityInfo)
3636

3737
internal bool IsWorkAndSchoolOnly()
3838
{
39-
return !TenantId.Equals(Constants.CommonTenant, StringComparison.OrdinalIgnoreCase) &&
39+
return !TenantId.Equals(Constants.Common, StringComparison.OrdinalIgnoreCase) &&
4040
!IsConsumers(TenantId);
4141
}
4242

@@ -47,7 +47,7 @@ internal bool IsConsumers()
4747

4848
internal static bool IsConsumers(string tenantId)
4949
{
50-
return tenantId.Equals(Constants.ConsumerTenant, StringComparison.OrdinalIgnoreCase) ||
50+
return tenantId.Equals(Constants.Consumers, StringComparison.OrdinalIgnoreCase) ||
5151
tenantId.Equals(Constants.MsaTenantId, StringComparison.OrdinalIgnoreCase);
5252
}
5353

@@ -64,7 +64,7 @@ internal static bool IsCommonOrganizationsOrConsumersTenant(string tenantId)
6464

6565
internal bool IsOrganizationsTenantWithMsaPassthroughEnabled(bool isMsaPassthrough, string accountTenantId)
6666
{
67-
return accountTenantId!= null && isMsaPassthrough && TenantId.Equals(Constants.OrganizationsTenant, StringComparison.OrdinalIgnoreCase) &&
67+
return accountTenantId!= null && isMsaPassthrough && TenantId.Equals(Constants.Organizations, StringComparison.OrdinalIgnoreCase) &&
6868
IsConsumers(accountTenantId);
6969
}
7070

‎src/client/Microsoft.Identity.Client/Internal/Constants.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.Globalization;
65

76
namespace Microsoft.Identity.Client.Internal
87
{
@@ -21,9 +20,9 @@ internal static class Constants
2120
public const string DefaultRealm = "http://schemas.microsoft.com/rel/trusted-realm";
2221

2322
public const string MsaTenantId = "9188040d-6c67-4c5b-b112-36a304b66dad";
24-
public const string ConsumerTenant = "consumers";
25-
public const string OrganizationsTenant = "organizations";
26-
public const string CommonTenant = "common";
23+
public const string Consumers = "consumers";
24+
public const string Organizations = "organizations";
25+
public const string Common = "common";
2726

2827
public const string UserRealmMsaDomainName = "live.com";
2928

‎tests/Microsoft.Identity.Test.Common/TestConstants.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Text.RegularExpressions;
88
using Microsoft.Identity.Client;
99
using Microsoft.Identity.Client.Cache;
10+
using Microsoft.Identity.Client.Internal;
1011
using Microsoft.Identity.Client.OAuth2;
1112
using Microsoft.Identity.Client.Utils;
1213
using Microsoft.Identity.Test.Common.Core.Mocks;
@@ -44,9 +45,9 @@ public static HashSet<string> s_scope
4445
public const string Utid = "my-utid";
4546
public const string Utid2 = "my-utid2";
4647

47-
public const string Common = "common";
48-
public const string Organizations = "organizations";
49-
public const string Consumers = "consumers";
48+
public const string Common = Constants.Common;
49+
public const string Organizations = Constants.Organizations;
50+
public const string Consumers = Constants.Consumers;
5051
public const string Guest = "guest";
5152
public const string Home = "home";
5253
public const string TenantId = "751a212b-4003-416e-b600-e1f48e40db9f";
@@ -112,6 +113,8 @@ public static HashSet<string> s_scope
112113
public const string DstsAuthorityTenantless = "https://some.url.dsts.core.azure-test.net/dstsv2/";
113114
public const string DstsAuthorityTenanted = DstsAuthorityTenantless + TenantId + "/";
114115
public const string DstsAuthorityCommon = DstsAuthorityTenantless + Common + "/";
116+
public const string DstsAuthorityOrganizations = DstsAuthorityTenantless + Organizations + "/";
117+
public const string DstsAuthorityConsumers = DstsAuthorityTenantless + Consumers + "/";
115118

116119
public const string GenericAuthority = "https://demo.duendesoftware.com";
117120

‎tests/Microsoft.Identity.Test.Unit/ApiConfigTests/AuthorityTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.Reflection.Emit;
5+
using System.Collections.Generic;
6+
using System.Linq;
67
using System.Threading.Tasks;
78
using Microsoft.Identity.Client;
89
using Microsoft.Identity.Client.Instance;

‎tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,14 +1041,14 @@ public void GetAuthorizationRequestUrl_WithConsumerInCreate_ReturnsConsumers()
10411041
#pragma warning disable CS0618 // Type or member is obsolete
10421042
Uri authorizationRequestUrl = confidentialClientApplication
10431043
.GetAuthorizationRequestUrl(new List<string> { "" })
1044-
.WithAuthority(AzureCloudInstance.AzurePublic, Constants.ConsumerTenant)
1044+
.WithAuthority(AzureCloudInstance.AzurePublic, Constants.Consumers)
10451045
.ExecuteAsync()
10461046
.ConfigureAwait(false)
10471047
.GetAwaiter()
10481048
.GetResult();
10491049
#pragma warning restore CS0618 // Type or member is obsolete
10501050

1051-
Assert.IsTrue(authorizationRequestUrl.Segments[1].StartsWith(Constants.CommonTenant));
1051+
Assert.IsTrue(authorizationRequestUrl.Segments[1].StartsWith(Constants.Common));
10521052
}
10531053
}
10541054

‎tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public async Task MtlsPop_WithoutRegion_ThrowsException(bool setAzureRegion)
171171
}
172172

173173
[TestMethod]
174-
public async Task MtlsPopWithoutTenantIdAsync()
174+
public async Task MtlsPop_WithUnsupportedNonTenantedAuthorityAsync_ThrowsException()
175175
{
176176
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
177177
.Create(TestConstants.ClientId)
@@ -186,7 +186,8 @@ public async Task MtlsPopWithoutTenantIdAsync()
186186
.ExecuteAsync())
187187
.ConfigureAwait(false);
188188

189-
Assert.AreEqual(MsalError.MissingTenantedAuthority, ex.ErrorCode);
189+
Assert.AreEqual(MsalError.MtlsPopWithoutRegion, ex.ErrorCode);
190+
Assert.AreEqual(MsalErrorMessage.MtlsPopWithoutRegion, ex.Message);
190191
}
191192

192193
[TestMethod]
@@ -449,9 +450,9 @@ await app.AcquireTokenForClient(TestConstants.s_scope)
449450

450451
[TestMethod]
451452
[DataTestMethod]
452-
[DataRow("https://contoso.b2clogin.com/tfp/contoso.onmicrosoft.com/B2C_1_signupsignin", "B2C Authority")]
453-
[DataRow("https://contoso.adfs.contoso.com/adfs", "ADFS Authority")]
454-
public async Task MtlsPop_NonAadAuthorityAsync(string authorityUrl, string authorityType)
453+
[DataRow("https://contoso.b2clogin.com/tfp/contoso.onmicrosoft.com/B2C_1_signupsignin", "B2C Authority", typeof(MsalServiceException))]
454+
[DataRow("https://contoso.adfs.contoso.com/adfs", "ADFS Authority", typeof(HttpRequestException))]
455+
public async Task MtlsPop_NonAadAuthorityAsync(string authorityUrl, string authorityType, Type expectedException)
455456
{
456457
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
457458
.Create(TestConstants.ClientId)
@@ -461,21 +462,28 @@ public async Task MtlsPop_NonAadAuthorityAsync(string authorityUrl, string autho
461462
.Build();
462463

463464
// Set WithMtlsProofOfPossession on the request with a non-AAD authority
464-
MsalClientException ex = await AssertException.TaskThrowsAsync<MsalClientException>(() =>
465-
app.AcquireTokenForClient(TestConstants.s_scope)
466-
.WithMtlsProofOfPossession() // Enables MTLS PoP
467-
.ExecuteAsync())
468-
.ConfigureAwait(false);
469-
470-
Assert.AreEqual(MsalError.InvalidAuthorityType, ex.ErrorCode);
471-
Assert.AreEqual(MsalErrorMessage.MtlsInvalidAuthorityTypeMessage, ex.Message, $"{authorityType} test failed.");
465+
try
466+
{
467+
await app
468+
.AcquireTokenForClient(TestConstants.s_scope)
469+
.WithMtlsProofOfPossession() // Enables MTLS PoP
470+
.ExecuteAsync()
471+
.ConfigureAwait(false);
472+
}
473+
catch (Exception ex)
474+
{
475+
Assert.AreEqual(expectedException, ex.GetType());
476+
}
472477
}
473478

474479
[DataTestMethod]
475-
[DataRow("https://login.microsoftonline.com", "Public Cloud")]
476-
[DataRow("https://login.microsoftonline.us", "Azure Government")]
477-
[DataRow("https://login.partner.microsoftonline.cn", "Azure China")]
478-
public async Task MtlsPop_WithCommonAsync(string authorityUrl, string cloudType)
480+
[DataRow("https://login.microsoftonline.com", TestConstants.Common, "Public Cloud")]
481+
[DataRow("https://login.microsoftonline.com", TestConstants.Organizations, "Public Cloud")]
482+
[DataRow("https://login.microsoftonline.us", TestConstants.Common, "Azure Government")]
483+
[DataRow("https://login.microsoftonline.us", TestConstants.Organizations, "Azure Government")]
484+
[DataRow("https://login.partner.microsoftonline.cn", TestConstants.Common, "Azure China")]
485+
[DataRow("https://login.partner.microsoftonline.cn", TestConstants.Organizations, "Azure China")]
486+
public async Task MtlsPop_WithUnsupportedNonTenantedAuthorityAsync_ThrowsException(string authorityUrl, string nonTenantValue, string cloudType)
479487
{
480488
const string region = "eastus";
481489

@@ -487,13 +495,13 @@ public async Task MtlsPop_WithCommonAsync(string authorityUrl, string cloudType)
487495
{
488496
var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId)
489497
.WithCertificate(s_testCertificate)
490-
.WithAuthority($"{authorityUrl}/common")
498+
.WithAuthority($"{authorityUrl}/{nonTenantValue}")
491499
.WithAzureRegion(ConfidentialClientApplication.AttemptRegionDiscovery)
492500
.WithExperimentalFeatures()
493501
.WithHttpManager(httpManager)
494502
.BuildConcrete();
495503

496-
// Expect an exception due to using /common with MTLS PoP
504+
// Expect an exception due to using /common or /organizations with MTLS PoP
497505
MsalClientException ex = await Assert.ThrowsExceptionAsync<MsalClientException>(async () =>
498506
await app.AcquireTokenForClient(TestConstants.s_scope)
499507
.WithMtlsProofOfPossession()
@@ -733,24 +741,24 @@ public async Task AcquireTokenForClient_WithMtlsPop_Dsts_SuccessAsync()
733741
}
734742
}
735743

736-
[TestMethod]
737-
public async Task MtlsPopDstsCommonAuthorityFailsAsync()
744+
[DataTestMethod]
745+
[DataRow(TestConstants.DstsAuthorityCommon)]
746+
[DataRow(TestConstants.DstsAuthorityOrganizations)]
747+
public async Task MtlsPop_WithUnsupportedNonTenantedAuthorityAsyncForDsts_ThrowsException(string authorityUrl)
738748
{
739749
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
740750
.Create(TestConstants.ClientId)
741-
.WithAuthority(TestConstants.DstsAuthorityCommon)
751+
.WithAuthority(authorityUrl)
742752
.WithCertificate(s_testCertificate)
743753
.WithExperimentalFeatures()
744754
.Build();
745755

746-
// Set WithMtlsProofOfPossession on the request without specifying an authority
747-
MsalClientException ex = await AssertException.TaskThrowsAsync<MsalClientException>(() =>
756+
// Set WithMtlsProofOfPossession on the request specifying an authority
757+
HttpRequestException ex = await AssertException.TaskThrowsAsync<HttpRequestException>(() =>
748758
app.AcquireTokenForClient(TestConstants.s_scope)
749759
.WithMtlsProofOfPossession()
750760
.ExecuteAsync())
751761
.ConfigureAwait(false);
752-
753-
Assert.AreEqual(MsalError.MissingTenantedAuthority, ex.ErrorCode);
754762
}
755763
}
756764
}

0 commit comments

Comments
 (0)