Skip to content

Add issuer validation when making call to OIDC endpoint #5358

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ namespace Microsoft.Identity.Client.Instance.Oidc
[Preserve(AllMembers = true)]
internal class OidcMetadata
{
[JsonProperty("issuer")]
public string Issuer { get; set; }

[JsonProperty("token_endpoint")]
public string TokenEndpoint { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Concurrent;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Identity.Client.Core;
Expand All @@ -27,7 +28,7 @@ public static async Task<OidcMetadata> GetOidcAsync(
}

await s_lockOidcRetrieval.WaitAsync().ConfigureAwait(false);

Uri oidcMetadataEndpoint = null;
try
{
Expand All @@ -44,9 +45,13 @@ public static async Task<OidcMetadata> GetOidcAsync(
builder.Path = existingPath.TrimEnd('/') + "/" + Constants.WellKnownOpenIdConfigurationPath;

oidcMetadataEndpoint = builder.Uri;
var client = new OAuth2Client(requestContext.Logger, requestContext.ServiceBundle.HttpManager, null);
var client = new OAuth2Client(requestContext.Logger, requestContext.ServiceBundle.HttpManager, null);
configuration = await client.DiscoverOidcMetadataAsync(oidcMetadataEndpoint, requestContext).ConfigureAwait(false);

// Validate the issuer before caching the configuration
requestContext.Logger.Verbose(() => $"[OIDC Discovery] Validating issuer: {configuration.Issuer} against authority: {authority}");
ValidateIssuer(new Uri(authority), configuration.Issuer);

s_cache[authority] = configuration;
requestContext.Logger.Verbose(() => $"[OIDC Discovery] OIDC discovery retrieved metadata from the network for {authority}");

Expand All @@ -70,6 +75,58 @@ public static async Task<OidcMetadata> GetOidcAsync(
}
}

/// <summary>
/// Validates that the issuer in the OIDC metadata matches the authority.
/// </summary>
/// <param name="authority">The authority URL.</param>
/// <param name="issuer">The issuer from the OIDC metadata - the single source of truth.</param>
/// <exception cref="MsalServiceException">Thrown when issuer validation fails.</exception>
private static void ValidateIssuer(Uri authority, string issuer)
{

// Primary validation: check if authority begins with the issuer string (exact string comparison)
if (authority.AbsoluteUri.StartsWith(issuer, StringComparison.OrdinalIgnoreCase))
{
return;
}

// Extract tenant for CIAM-like scenarios
string tenant = null;
try
{
tenant = AuthorityInfo.GetFirstPathSegment(authority);
}
catch (InvalidOperationException)
{
// If no path segments exist, try to extract from hostname (first part)
var hostParts = authority.Host.Split('.');
tenant = hostParts.Length > 0 ? hostParts[0] : null;
}

// If tenant extraction failed or returned empty, validation fails
if (!string.IsNullOrEmpty(tenant))
{
// Create a collection of valid CIAM issuer patterns for the tenant
string[] validCiamPatterns =
{
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}",
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}/{tenant}",
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}/{tenant}/v2.0"
};

// Check if the issuer matches any of the valid patterns
if (validCiamPatterns.Any(pattern => string.Equals(issuer, pattern, StringComparison.OrdinalIgnoreCase)))
{
return;
}
}

// Validation failed
throw new MsalServiceException(
"authority_validation_failed",
$"Issuer validation failed for {authority}. Issuer from OIDC endpoint does not match any expected pattern: {issuer}");
}

// For testing purposes only
public static void ResetCacheForTest()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,31 @@ public async Task OBOCiam_CustomDomain_ReturnsValidTokens()
Assert.AreEqual(TokenSource.Cache, resultObo.AuthenticationResultMetadata.TokenSource);
}

[TestMethod]
public async Task WithOidcAuthority_ValidatesIssuerSuccessfully()
{
//Get lab details
var labResponse = await LabUserHelper.GetLabUserDataAsync(new UserQuery()
{
FederationProvider = FederationProvider.CIAMCUD,
SignInAudience = SignInAudience.AzureAdMyOrg
}).ConfigureAwait(false);

//Test with different CIAM authority formats
string[] authorities =
{
string.Format("https://{0}.ciamlogin.com/", labResponse.User.LabName),
string.Format("https://{0}.ciamlogin.com/{1}.onmicrosoft.com", labResponse.User.LabName, labResponse.User.LabName),
string.Format("https://{0}.ciamlogin.com/{1}", labResponse.User.LabName, labResponse.Lab.TenantId),
string.Format("https://login.msidlabsciam.com/{0}/v2.0/", labResponse.Lab.TenantId)
};

foreach (var authority in authorities)
{
await RunCiamCCATest(authority, labResponse.App.AppId).ConfigureAwait(false);
}
}

private string GetCiamSecret()
{
KeyVaultSecretsProvider provider = new KeyVaultSecretsProvider();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,44 @@ public async Task BadOidcResponse_ThrowsException_Async(string badOidcResponseTy
}
}

[TestMethod]
public async Task OidcIssuerValidation_ThrowsForNonMatchingIssuer_Async()
{
using (var httpManager = new MockHttpManager())
{
string authority = "https://demo.duendesoftware.com";
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
.Create(TestConstants.ClientId)
.WithHttpManager(httpManager)
.WithOidcAuthority(authority)
.WithClientSecret(TestConstants.ClientSecret)
.Build();

// Create OIDC document with non-matching issuer
string validOidcDocumentWithWrongIssuer = TestConstants.GenericOidcResponse.Replace(
"\"issuer\":\"https://demo.duendesoftware.com\"",
"\"issuer\":\"https://wrong.issuer.com\"");

// Mock OIDC endpoint response
httpManager.AddMockHandler(new MockHttpMessageHandler
{
ExpectedMethod = HttpMethod.Get,
ExpectedUrl = authority + "/.well-known/openid-configuration",
ResponseMessage = MockHelpers.CreateSuccessResponseMessage(validOidcDocumentWithWrongIssuer)
});

var ex = await AssertException.TaskThrowsAsync<MsalServiceException>(() =>
app.AcquireTokenForClient(new[] { "api" }).ExecuteAsync()
).ConfigureAwait(false);

Assert.AreEqual("authority_validation_failed", ex.ErrorCode);
Assert.IsTrue(ex.Message.Contains("Issuer validation failed"),
$"Expected error message to contain 'Issuer validation failed', but was: {ex.Message}");
Assert.IsTrue(ex.Message.Contains("https://wrong.issuer.com"),
"Error message should include the non-matching issuer value");
}
}

private static MockHttpMessageHandler CreateTokenResponseHttpHandler(
string tokenEndpoint,
string scopesInRequest,
Expand Down