Skip to content

Commit

Permalink
Use AppHomeTenantId for acquiring app token when TenantId is not tena…
Browse files Browse the repository at this point in the history
…nt (#3132)
  • Loading branch information
msbw2 authored Nov 8, 2024
1 parent 5ad5095 commit 29761d2
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 12 deletions.
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
3 changes: 3 additions & 0 deletions src/Microsoft.Identity.Web.TokenAcquisition/MergedOptions.cs
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);
}
}
}

0 comments on commit 29761d2

Please sign in to comment.