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.

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

Use AppHomeTenantId for acquiring app token when TenantId is not tenant #3132

Merged
merged 1 commit into from
Nov 8, 2024
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
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
<MicrosoftGraphVersion>4.36.0</MicrosoftGraphVersion>
<MicrosoftGraphBetaVersion>4.57.0-preview</MicrosoftGraphBetaVersion>
<MicrosoftExtensionsHttpVersion>3.1.3</MicrosoftExtensionsHttpVersion>
<MicrosoftIdentityAbstractionsVersion>7.1.0</MicrosoftIdentityAbstractionsVersion>
<MicrosoftIdentityAbstractionsVersion>7.2.0</MicrosoftIdentityAbstractionsVersion>
<NetNineRuntimeVersion>9.0.0-rc.2.24473.5</NetNineRuntimeVersion>
<AspNetCoreNineRuntimeVersion>9.0.0-rc.2.24474.3</AspNetCoreNineRuntimeVersion>
<!--CVE-2024-43485-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public ConfidentialClientApplicationOptions ConfidentialClientApplicationOptions

// Properties of ConfidentialClientApplication which are not in MicrosoftIdentityOptions
public AadAuthorityAudience AadAuthorityAudience { get; set; }
public string? AppHomeTenantId { get; set; }
public AzureCloudInstance AzureCloudInstance { get; set; }
public string? AzureRegion { get; set; }
public IEnumerable<string>? ClientCapabilities { get; set; }
Expand Down Expand Up @@ -537,6 +538,8 @@ public static void UpdateMergedOptionsFromMicrosoftIdentityApplicationOptions(Mi
mergedOptions.TenantId = microsoftIdentityApplicationOptions.TenantId;
}

mergedOptions.AppHomeTenantId = microsoftIdentityApplicationOptions.AppHomeTenantId;

mergedOptions.WithSpaAuthCode |= microsoftIdentityApplicationOptions.WithSpaAuthCode;

if ((mergedOptions.ClientCredentials == null || !mergedOptions.ClientCredentials.Any()) && microsoftIdentityApplicationOptions.ClientCredentials != null)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?

static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#nullable enable
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.get -> string?
Microsoft.Identity.Web.MergedOptions.AppHomeTenantId.set -> void
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForApp(Microsoft.Identity.Client.AcquireTokenForClientParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions) -> void
readonly Microsoft.Identity.Web.TokenAcquisition.tokenAcquisitionExtensionOptionsMonitor -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Identity.Web.TokenAcquisitionExtensionOptions!>?
static Microsoft.Identity.Web.TokenAcquisition.ResolveTenant(string? tenant, Microsoft.Identity.Web.MergedOptions! mergedOptions) -> string?
38 changes: 28 additions & 10 deletions src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class OAuthConstants
/// <summary>
/// Meta-tenant identifiers which are not allowed in client credentials.
/// </summary>
private readonly HashSet<string> _metaTenantIdentifiers = new HashSet<string>(
private static readonly HashSet<string> _metaTenantIdentifiers = new HashSet<string>(
new[]
{
Constants.Common,
Expand Down Expand Up @@ -397,6 +397,32 @@ private void LogAuthResult(AuthenticationResult? authenticationResult)
}
}

/// <summary>
/// Resolves the tenant based on if the tenant is already set or the TenantId configured
/// in the options or the AppHomeTenantId if the TenantId is a meta tenant.
/// </summary>
/// <param name="tenant">Provided tenant or null if not provided</param>
/// <param name="mergedOptions">Merged configuration from which to retrieve tenant value as necessary</param>
/// <returns>Resolved tenant</returns>
internal static string? ResolveTenant(string? tenant, MergedOptions mergedOptions)
{
if (string.IsNullOrEmpty(tenant))
{
tenant = mergedOptions.TenantId;
if (!string.IsNullOrEmpty(tenant) && _metaTenantIdentifiers.Contains(tenant!) && !string.IsNullOrEmpty(mergedOptions.AppHomeTenantId))
{
tenant = mergedOptions.AppHomeTenantId;
}
}

if (!string.IsNullOrEmpty(tenant) && _metaTenantIdentifiers.Contains(tenant!))
{
throw new ArgumentException(IDWebErrorMessage.ClientCredentialTenantShouldBeTenanted, nameof(tenant));
}

return tenant;
}

/// <summary>
/// Acquires an authentication result from the authority configured in the app, for the confidential client itself (not on behalf of a user)
/// using either a client credentials or managed identity flow. See https://aka.ms/msal-net-client-credentials for client credentials or
Expand Down Expand Up @@ -427,15 +453,7 @@ public async Task<AuthenticationResult> GetAuthenticationResultForAppAsync(

MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authenticationScheme ?? tokenAcquisitionOptions?.AuthenticationOptionsName, out _);

if (string.IsNullOrEmpty(tenant))
{
tenant = mergedOptions.TenantId;
}

if (!string.IsNullOrEmpty(tenant) && _metaTenantIdentifiers.Contains(tenant!))
{
throw new ArgumentException(IDWebErrorMessage.ClientCredentialTenantShouldBeTenanted, nameof(tenant));
}
tenant = ResolveTenant(tenant, mergedOptions);

// If using managed identity
if (tokenAcquisitionOptions != null && tokenAcquisitionOptions.ManagedIdentity != null)
Expand Down
49 changes: 49 additions & 0 deletions tests/Microsoft.Identity.Web.Test/TokenAcquisitionTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Xunit;

namespace Microsoft.Identity.Web.Test
{
public class TokenAcquisitionTests
{
private const string Tenant = "tenant";
private const string TenantId = "tenant-id";
private const string AppHomeTenantId = "app-home-tenant-id";

[Theory]
[InlineData(null, null, null, null)]
[InlineData(null, null, AppHomeTenantId, null)]
[InlineData(Tenant, null, null, Tenant)]
[InlineData(Tenant, TenantId, null, Tenant)]
[InlineData(Tenant, null, AppHomeTenantId, Tenant)]
[InlineData(Tenant, TenantId, AppHomeTenantId, Tenant)]
[InlineData(null, TenantId, null, TenantId)]
[InlineData(null, TenantId, AppHomeTenantId, TenantId)]
[InlineData(null, Constants.Common, AppHomeTenantId, AppHomeTenantId)]
[InlineData(null, Constants.Organizations, AppHomeTenantId, AppHomeTenantId)]
public void TestResolveTenantReturnsCorrectTenant(string? tenant, string? tenantId, string? appHomeTenantId, string? expectedValue)
{
string? resolvedTenant = TokenAcquisition.ResolveTenant(tenant, new MergedOptions { TenantId = tenantId, AppHomeTenantId = appHomeTenantId });
Assert.Equal(expectedValue, resolvedTenant);
}

[Theory]
[InlineData(Constants.Common, null)]
[InlineData(Constants.Organizations, null)]
[InlineData(Constants.Common, TenantId)]
[InlineData(Constants.Organizations, TenantId)]
[InlineData(Constants.Common, Constants.Common)]
[InlineData(Constants.Common, Constants.Organizations)]
[InlineData(Constants.Organizations, Constants.Organizations)]
[InlineData(Constants.Organizations, Constants.Common)]
[InlineData(null, Constants.Common)]
[InlineData(null, Constants.Organizations)]
public void TestResolveTenantThrowsWhenMetaTenant(string? tenant, string? tenantId)
{
var exception = Assert.Throws<ArgumentException>(() => TokenAcquisition.ResolveTenant(tenant, new MergedOptions { TenantId = tenantId }));
Assert.StartsWith(IDWebErrorMessage.ClientCredentialTenantShouldBeTenanted, exception.Message, StringComparison.Ordinal);
}
}
}
Loading