Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private static void InitMtlsPopParameters(
if (serviceBundle.Config.Authority.AuthorityInfo.AuthorityType == AuthorityType.Aad)
{
string tenant = AuthorityInfo.GetFirstPathSegment(serviceBundle.Config.Authority.AuthorityInfo.CanonicalAuthority);
if (AadAuthority.IsCommonOrganizationsOrConsumersTenant(tenant))
if (AadAuthority.IsCommonOrOrganizationsTenant(tenant))
Comment thread
bgavrilMS marked this conversation as resolved.
{
throw new MsalClientException(
MsalError.MissingTenantedAuthority,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,16 +491,16 @@ public static IAuthorityValidator CreateAuthorityValidator(AuthorityInfo authori
/// 1. If there is no request authority (i.e. no authority override), use the config authority.
/// 1.1. For AAD, if the config authority is "common" etc, try to use the tenanted version with the home account tenant ID
/// 2. If there is a request authority, try to use it.
/// 2.1. If the request authority is not "common", then use it
/// 2.2 If the request authority is "common", ignore it, and use 1.1
/// 2.1. If the request authority is not "common" or "organizations", use it (full relaxation)
/// 2.2 If the request authority is "common" or "organizations", ignore it, and use 1.1
///
/// Special cases:
///
/// - if the authority is not defined at the application level and the request level is not AAD, use the request authority
/// - if the authority is defined at app level, and the request level authority is of different type, throw an exception
///
/// - if the intended authority is consumers, please define it at the app level and not at the request level.
/// known issue: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/2929
///
/// Note: "consumers" and the MSA tenant GUID (9188040d-6c67-4c5b-b112-36a304b66dad) are both real tenant
/// identifiers — they are honored when specified at the request level, unlike "common" and "organizations".
/// </summary>
public static async Task<Authority> CreateAuthorityForRequestAsync(RequestContext requestContext,
AuthorityInfo requestAuthorityInfo,
Expand Down Expand Up @@ -565,7 +565,7 @@ public static async Task<Authority> CreateAuthorityForRequestAsync(RequestContex
var requestAuthority = updateEnvironment ?
new AadAuthority(CreateAuthorityWithEnvironment(requestAuthorityInfo, account?.Environment).AuthorityInfo) :
new AadAuthority(requestAuthorityInfo);
if (!requestAuthority.IsCommonOrganizationsOrConsumersTenant() ||
if (!requestAuthority.IsCommonOrOrganizationsTenant() ||
requestAuthority.IsOrganizationsTenantWithMsaPassthroughEnabled(requestContext.ServiceBundle.Config.IsBrokerEnabled && requestContext.ServiceBundle.Config.BrokerOptions != null && requestContext.ServiceBundle.Config.BrokerOptions.MsaPassthrough, account?.HomeAccountId?.TenantId))
{
return requestAuthority;
Expand Down
14 changes: 1 addition & 13 deletions src/client/Microsoft.Identity.Client/Instance/AadAuthority.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ internal class AadAuthority : Authority
{
Constants.Common,
Constants.Organizations,
Constants.Consumers,
},
StringComparer.OrdinalIgnoreCase);

Expand Down Expand Up @@ -51,17 +50,6 @@ internal static bool IsConsumers(string tenantId)
tenantId.Equals(Constants.MsaTenantId, StringComparison.OrdinalIgnoreCase);
}

internal bool IsCommonOrganizationsOrConsumersTenant()
Comment thread
Robbie-Microsoft marked this conversation as resolved.
{
return IsCommonOrganizationsOrConsumersTenant(TenantId);
}

internal static bool IsCommonOrganizationsOrConsumersTenant(string tenantId)
{
return !string.IsNullOrEmpty(tenantId) &&
(IsCommonOrOrganizationsTenant(tenantId) || IsConsumers(tenantId));
}

internal bool IsOrganizationsTenantWithMsaPassthroughEnabled(bool isMsaPassthrough, string accountTenantId)
{
return accountTenantId!= null && isMsaPassthrough && TenantId.Equals(Constants.Organizations, StringComparison.OrdinalIgnoreCase) &&
Expand All @@ -83,7 +71,7 @@ internal static bool IsCommonOrOrganizationsTenant(string tenantId)
internal override string GetTenantedAuthority(string tenantId, bool forceSpecifiedTenant = false)
{
if (!string.IsNullOrEmpty(tenantId) &&
(forceSpecifiedTenant || IsCommonOrganizationsOrConsumersTenant()))
(forceSpecifiedTenant || IsCommonOrOrganizationsTenant()))
{
var authorityUri = AuthorityInfo.CanonicalAuthority;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ internal CiamAuthority(AuthorityInfo authorityInfo) :
internal override string GetTenantedAuthority(string tenantId, bool forceSpecifiedTenant = false)
{
if (!string.IsNullOrEmpty(tenantId) &&
(forceSpecifiedTenant || IsCommonOrganizationsOrConsumersTenant()))
(forceSpecifiedTenant || IsCommonOrOrganizationsTenant()))
Comment thread
Robbie-Microsoft marked this conversation as resolved.
{
var authorityUri = AuthorityInfo.CanonicalAuthority;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public DstsAuthority(AuthorityInfo authorityInfo)
internal override string GetTenantedAuthority(string tenantId, bool forceSpecifiedTenant = false)
{
if (!string.IsNullOrEmpty(tenantId) &&
(forceSpecifiedTenant || AadAuthority.IsCommonOrganizationsOrConsumersTenant(TenantId)))
(forceSpecifiedTenant || AadAuthority.IsCommonOrOrganizationsTenant(TenantId)))
{
var authorityUri = AuthorityInfo.CanonicalAuthority;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ private static void FilterTokensByHomeAccountTenantOrAssertion(
// then we cannot filter by tenant and will use whatever is in the cache.
filterByTenantId =
!string.IsNullOrEmpty(requestTenantId) &&
!AadAuthority.IsCommonOrganizationsOrConsumersTenant(requestTenantId);
!AadAuthority.IsCommonOrOrganizationsTenant(requestTenantId);
}

if (filterByTenantId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ public class AuthorityTests : TestBase
private static readonly Authority s_b2cAuthority = Authority.CreateAuthority(TestConstants.B2CAuthority, true);
private static readonly Authority s_commonNetAuthority = Authority.CreateAuthority(TestConstants.PrefCacheAuthorityCommonTenant, true);

private static readonly Authority s_consumersTenantAuthority =
Authority.CreateAuthority(TestConstants.AuthorityConsumersTenant, true);
private static readonly Authority s_consumerTidAuthority =
Authority.CreateAuthority(TestConstants.AuthorityConsumerTidTenant, true);

private MockHttpAndServiceBundle _harness;
private RequestContext _testRequestContext;

Expand Down Expand Up @@ -453,6 +458,41 @@ public void VerifyConfigAuthorityType(string authorityHost, Type authorityTypeIn
Assert.AreEqual(app.AuthorityInfo.AuthorityType.ToString(), authorityType);
}

/// <summary>
/// Regression test for https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/5951
/// When WithTenantId is called with a real tenant (MSA GUID or "consumers" alias) at request level,
/// it should be honored regardless of the app-level authority. Only "common" and "organizations"
/// are truly tenantless and are ignored at request level.
/// </summary>
[TestMethod]
[DataRow(TestConstants.AuthorityUtidTenant, TestConstants.AuthorityConsumerTidTenant, TestConstants.MsaTenantId,
DisplayName = "AppSpecificTenant_RequestMsaGuid_MsaGuidWins")]
[DataRow(TestConstants.AuthorityCommonTenant, TestConstants.AuthorityConsumerTidTenant, TestConstants.MsaTenantId,
DisplayName = "AppCommon_RequestMsaGuid_MsaGuidWins")]
[DataRow(TestConstants.AuthorityUtidTenant, TestConstants.AuthorityConsumersTenant, TestConstants.Consumers,
DisplayName = "AppSpecificTenant_RequestConsumersAlias_ConsumersWins")]
[DataRow(TestConstants.AuthorityCommonTenant, TestConstants.AuthorityConsumersTenant, TestConstants.Consumers,
DisplayName = "AppCommon_RequestConsumersAlias_ConsumersWins")]
[DataRow(TestConstants.AuthorityConsumerTidTenant, null, TestConstants.MsaTenantId,
DisplayName = "AppMsaGuid_NoRequestOverride_MsaGuidUsed")]
public void WithTenantId_ConsumerGuid_IsHonoredAtRequestLevel(
string configAuthorityUrl,
string requestAuthorityUrl,
string expectedTenantId)
{
var configAuthority = Authority.CreateAuthority(configAuthorityUrl, true);
Authority requestAuthority = requestAuthorityUrl == null
? null
: Authority.CreateAuthority(requestAuthorityUrl, true);

VerifyAuthority(
configAuthority: configAuthority,
requestAuthority: requestAuthority,
account: null,
expectedTenantId: expectedTenantId,
_testRequestContext);
}

private static void VerifyAuthority(
Authority configAuthority,
Authority requestAuthority,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,25 @@ public void CreateAuthorityFromTenantedWithTenantTest()
Assert.AreEqual("https://login.microsoft.com/other_tenant_id/", updatedAuthority2, "Changed with forced flag");
}

[TestMethod]
public void GetTenantedAuthority_MsaGuid_IsNotReplaced()
{
// Regression test for https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/5951
// The MSA GUID is a real tenant ID — GetTenantedAuthority should NOT replace it
// unless forceSpecifiedTenant=true.
Authority authority = Authority.CreateAuthority(TestConstants.AuthorityConsumerTidTenant);

// Without force: MSA GUID is preserved (it's a real tenant)
string updatedAuthority = authority.GetTenantedAuthority("other_tenant_id", false);
Assert.AreEqual(TestConstants.AuthorityConsumerTidTenant, updatedAuthority,
"MSA GUID authority should not be replaced when forceSpecifiedTenant=false");

// With force: replacement is honored
string updatedAuthority2 = authority.GetTenantedAuthority("other_tenant_id", true);
StringAssert.Contains(updatedAuthority2, "other_tenant_id",
"MSA GUID authority should be replaced when forceSpecifiedTenant=true");
}

[TestMethod]
public void CreateAuthorityFromCommonWithTenantTest()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,49 @@ public async Task ClientCreds_MustFilterByTenantId_Async()
}
}

[TestMethod]
[TestCategory(TestCategories.Regression)]
[WorkItem(5951)] // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/5951
public async Task ClientCreds_WithTenantId_MsaGuid_UsesCorrectTokenEndpoint_Async()
Comment thread
Robbie-Microsoft marked this conversation as resolved.
{
// Regression test: the MSA tenant GUID (9188040d-...) is a real tenant.
// App is configured with the MSA GUID authority at app level. A request-level
// .WithTenantId() override with a different tenant must be honored — the token
// request must target the override tenant endpoint, not silently use the MSA GUID.
string expectedTokenEndpoint =
$"https://login.microsoftonline.com/{TestConstants.Utid}/oauth2/v2.0/token";

using (var httpManager = new MockHttpManager())
{
httpManager.AddInstanceDiscoveryMockHandler();

var tokenHandler = new MockHttpMessageHandler
{
ExpectedUrl = expectedTokenEndpoint,
ExpectedMethod = HttpMethod.Post,
ResponseMessage = MockHelpers.CreateSuccessfulClientCredentialTokenResponseMessage()
};
httpManager.AddMockHandler(tokenHandler);

ConfidentialClientApplication app =
ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId)
.WithClientSecret(TestConstants.ClientSecret)
.WithAuthority(TestConstants.AuthorityConsumerTidTenant)
.WithHttpManager(httpManager)
.BuildConcrete();

var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray())
.WithTenantId(TestConstants.Utid)
.ExecuteAsync(CancellationToken.None).ConfigureAwait(false);

Assert.IsNotNull(result.AccessToken);
// The cache entry should be keyed on the override tenant, not the MSA GUID
Assert.AreEqual(TestConstants.Utid,
app.AppTokenCacheInternal.Accessor.GetAllAccessTokens().Single().TenantId,
StringComparer.OrdinalIgnoreCase);
}
}

[TestMethod]
public async Task ClientCreds_UsesDefaultPartitionedCacheCorrectly_Async()
{
Expand Down Expand Up @@ -1049,7 +1092,7 @@ public void GetAuthorizationRequestUrl_WithConsumerInCreate_ReturnsConsumers()
.GetResult();
#pragma warning restore CS0618 // Type or member is obsolete

Assert.StartsWith(Constants.Common, authorizationRequestUrl.Segments[1]);
Assert.StartsWith(Constants.Consumers, authorizationRequestUrl.Segments[1]);
Comment thread
Robbie-Microsoft marked this conversation as resolved.
}
}

Expand Down
Loading