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 oidc endpoint for issuer validation. #1241

Merged
merged 2 commits into from
Jun 7, 2021
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
1 change: 1 addition & 0 deletions src/Microsoft.Identity.Web/Constants/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public static class Constants
internal const string AzureADIssuerMetadataUrl = "https://login.microsoftonline.com/common/discovery/instance?authorization_endpoint=https://login.microsoftonline.com/common/oauth2/v2.0/authorize&api-version=1.1";
#pragma warning restore S1075 // URIs should not be hardcoded
internal const string FallbackAuthority = "https://login.microsoftonline.com/";
internal const string OidcEndpoint = "/.well-known/openid-configuration";

// RegisterValidAudience
internal const string Version = "ver";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,6 @@ internal static class IDWebErrorMessage
public const string InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. ";
public const string ReplyForbiddenWithWwwAuthenticateHeaderAsyncIsObsolete = "IDW10802: Use ReplyForbiddenWithWwwAuthenticateHeader instead. See https://aka.ms/ms-id-web/1.9.0. ";
public const string FromStoreWithThumprintIsObsolete = "IDW10803: Use FromStoreWithThumbprint instead, due to spelling error. ";
public const string AadIssuerValidatorIsObsolete = "IDW10804: Use MicrosoftIdentityIssuerValidator. ";
}
}
19 changes: 3 additions & 16 deletions src/Microsoft.Identity.Web/InstanceDiscovery/IssuerMetadata.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Text.Json.Serialization;

namespace Microsoft.Identity.Web.InstanceDiscovery
Expand All @@ -12,21 +11,9 @@ namespace Microsoft.Identity.Web.InstanceDiscovery
internal class IssuerMetadata
{
/// <summary>
/// Tenant discovery endpoint.
/// Issuer associated with the OIDC endpoint.
/// </summary>
[JsonPropertyName(Constants.TenantDiscoveryEndpoint)]
public string? TenantDiscoveryEndpoint { get; set; }

/// <summary>
/// API Version.
/// </summary>
[JsonPropertyName(Constants.ApiVersion)]
public string? ApiVersion { get; set; }

/// <summary>
/// List of metadata associated with the endpoint.
/// </summary>
[JsonPropertyName(Constants.Metadata)]
public List<Metadata> Metadata { get; set; } = new List<Metadata>();
[JsonPropertyName("issuer")]
public string? Issuer { get; set; }
}
}
33 changes: 0 additions & 33 deletions src/Microsoft.Identity.Web/InstanceDiscovery/Metadata.cs

This file was deleted.

50 changes: 3 additions & 47 deletions src/Microsoft.Identity.Web/Microsoft.Identity.Web.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

118 changes: 80 additions & 38 deletions src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IdentityModel.Tokens.Jwt;
using System.Net.Http;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Web.InstanceDiscovery;
using Microsoft.IdentityModel.JsonWebTokens;
using Microsoft.IdentityModel.Protocols;
using Microsoft.IdentityModel.Tokens;

namespace Microsoft.Identity.Web.Resource
Expand All @@ -15,16 +18,22 @@ namespace Microsoft.Identity.Web.Resource
/// </summary>
public class AadIssuerValidator
{
/// <summary>
/// A list of all Issuers across the various Azure AD instances.
/// </summary>
private readonly ISet<string> _issuerAliases;

internal /*internal for tests*/ AadIssuerValidator(IEnumerable<string> aliases)
internal AadIssuerValidator(
IOptions<AadIssuerValidatorOptions> aadIssuerValidatorOptions,
IHttpClientFactory httpClientFactory,
string aadAuthority)
{
_issuerAliases = new HashSet<string>(aliases, StringComparer.OrdinalIgnoreCase);
AadIssuerValidatorOptions = aadIssuerValidatorOptions;
HttpClientFactory = httpClientFactory;
AadAuthority = aadAuthority.TrimEnd('/');
}

private IOptions<AadIssuerValidatorOptions> AadIssuerValidatorOptions { get; }
private IHttpClientFactory HttpClientFactory { get; }
internal string? AadIssuerV1 { get; set; }
internal string? AadIssuerV2 { get; set; }
internal string AadAuthority { get; set; }

/// <summary>
/// Validate the issuer for multi-tenant applications of various audiences (Work and School accounts, or Work and School accounts +
/// Personal accounts).
Expand All @@ -34,13 +43,15 @@ public class AadIssuerValidator
/// <param name="validationParameters">Token validation parameters.</param>
/// <remarks>The issuer is considered as valid if it has the same HTTP scheme and authority as the
/// authority from the configuration file, has a tenant ID, and optionally v2.0 (this web API
/// accepts both V1 and V2 tokens).
/// Authority aliasing is also taken into account.</remarks>
/// accepts both V1 and V2 tokens).</remarks>
/// <returns>The <c>issuer</c> if it's valid, or otherwise <c>SecurityTokenInvalidIssuerException</c> is thrown.</returns>
/// <exception cref="ArgumentNullException"> if <paramref name="securityToken"/> is null.</exception>
/// <exception cref="ArgumentNullException"> if <paramref name="validationParameters"/> is null.</exception>
/// <exception cref="SecurityTokenInvalidIssuerException">if the issuer is invalid. </exception>
public string Validate(string actualIssuer, SecurityToken securityToken, TokenValidationParameters validationParameters)
public string Validate(
string actualIssuer,
SecurityToken securityToken,
TokenValidationParameters validationParameters)
{
if (string.IsNullOrEmpty(actualIssuer))
{
Expand All @@ -63,6 +74,38 @@ public string Validate(string actualIssuer, SecurityToken securityToken, TokenVa
throw new SecurityTokenInvalidIssuerException(IDWebErrorMessage.TenantIdClaimNotPresentInToken);
}

if (validationParameters.ValidIssuers == null && validationParameters.ValidIssuer == null)
{
if (securityToken.Issuer.EndsWith("v2.0"))
{
if (AadIssuerV2 == null)
{
IssuerMetadata issuerMetadata =
CreateConfigManager(AadAuthority).GetConfigurationAsync().ConfigureAwait(false).GetAwaiter().GetResult();
AadIssuerV2 = issuerMetadata.Issuer!;
}

if (IsValidIssuer(AadIssuerV2, tenantId, actualIssuer))
{
return actualIssuer;
}
}
else
{
if (AadIssuerV1 == null)
{
IssuerMetadata issuerMetadata =
CreateConfigManager(AadAuthority.Replace("/v2.0", string.Empty)).GetConfigurationAsync().ConfigureAwait(false).GetAwaiter().GetResult();
AadIssuerV1 = issuerMetadata.Issuer!;
}

if (IsValidIssuer(AadIssuerV1, tenantId, actualIssuer))
{
return actualIssuer;
}
}
}

if (validationParameters.ValidIssuers != null)
{
foreach (var validIssuerTemplate in validationParameters.ValidIssuers)
Expand All @@ -74,9 +117,12 @@ public string Validate(string actualIssuer, SecurityToken securityToken, TokenVa
}
}

if (IsValidIssuer(validationParameters.ValidIssuer, tenantId, actualIssuer))
if (validationParameters.ValidIssuer != null)
{
return actualIssuer;
if (IsValidIssuer(validationParameters.ValidIssuer, tenantId, actualIssuer))
{
return actualIssuer;
}
}

// If a valid issuer is not found, throw
Expand All @@ -87,6 +133,26 @@ public string Validate(string actualIssuer, SecurityToken securityToken, TokenVa
actualIssuer));
}

private ConfigurationManager<IssuerMetadata> CreateConfigManager(
string aadAuthority)
{
if (AadIssuerValidatorOptions?.Value?.HttpClientName != null && HttpClientFactory != null)
{
return
new ConfigurationManager<IssuerMetadata>(
$"{aadAuthority}{Constants.OidcEndpoint}",
new IssuerConfigurationRetriever(),
HttpClientFactory.CreateClient(AadIssuerValidatorOptions.Value.HttpClientName));
}
else
{
return
new ConfigurationManager<IssuerMetadata>(
$"{aadAuthority}{Constants.OidcEndpoint}",
new IssuerConfigurationRetriever());
}
}

private bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
{
if (string.IsNullOrEmpty(validIssuerTemplate))
Expand All @@ -99,14 +165,7 @@ private bool IsValidIssuer(string validIssuerTemplate, string tenantId, string a
Uri issuerFromTemplateUri = new Uri(validIssuerTemplate.Replace("{tenantid}", tenantId));
Uri actualIssuerUri = new Uri(actualIssuer);

// Template authority is in the aliases
return _issuerAliases.Contains(issuerFromTemplateUri.Authority) &&
// "iss" authority is in the aliases
_issuerAliases.Contains(actualIssuerUri.Authority) &&
// Template authority ends in the tenant ID
IsValidTidInLocalPath(tenantId, issuerFromTemplateUri) &&
// "iss" ends in the tenant ID
IsValidTidInLocalPath(tenantId, actualIssuerUri);
return issuerFromTemplateUri.AbsoluteUri == actualIssuerUri.AbsoluteUri;
}
catch
{
Expand All @@ -116,12 +175,6 @@ private bool IsValidIssuer(string validIssuerTemplate, string tenantId, string a
return false;
}

private static bool IsValidTidInLocalPath(string tenantId, Uri uri)
{
string trimmedLocalPath = uri.LocalPath.Trim('/');
return trimmedLocalPath == tenantId || trimmedLocalPath == $"{tenantId}/v2.0";
}

/// <summary>Gets the tenant ID from a token.</summary>
/// <param name="securityToken">A JWT token.</param>
/// <returns>A string containing the tenant ID, if found or <see cref="string.Empty"/>.</returns>
Expand Down Expand Up @@ -192,16 +245,5 @@ private static string GetTenantIdFromIss(string iss)

return string.Empty;
}

/// <summary>
/// This method is now Obsolete.
/// </summary>
/// <param name="aadAuthority">Aad authority.</param>
/// <returns>NotImplementedException.</returns>
[Obsolete(IDWebErrorMessage.AadIssuerValidatorGetIssuerValidatorIsObsolete, true)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's ok to remove this obsolete method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can add it back if you want.

public static AadIssuerValidator GetIssuerValidator(string aadAuthority)
{
throw new NotImplementedException(IDWebErrorMessage.AadIssuerValidatorGetIssuerValidatorIsObsolete);
}
}
}
Loading