Skip to content

Commit 3d5729b

Browse files
Avery-DunnAvery-Dunn
authored
Add issuer validation when making call to OIDC endpoint (#5358)
* Add issuer validation when making call to OIDC endpoint * Address PR feedback * PR feedback * PR feedback and minor error message changes
1 parent 237a6b2 commit 3d5729b

File tree

5 files changed

+136
-8
lines changed

5 files changed

+136
-8
lines changed

‎src/client/Microsoft.Identity.Client/Instance/Oidc/OidcMetadata.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ namespace Microsoft.Identity.Client.Instance.Oidc
1919
[Preserve(AllMembers = true)]
2020
internal class OidcMetadata
2121
{
22+
[JsonProperty("issuer")]
23+
public string Issuer { get; set; }
24+
2225
[JsonProperty("token_endpoint")]
2326
public string TokenEndpoint { get; set; }
2427

‎src/client/Microsoft.Identity.Client/Instance/Oidc/OidcRetrieverWithCache.cs‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Concurrent;
6+
using System.Linq;
67
using System.Threading;
78
using System.Threading.Tasks;
89
using Microsoft.Identity.Client.Core;
@@ -27,7 +28,7 @@ public static async Task<OidcMetadata> GetOidcAsync(
2728
}
2829

2930
await s_lockOidcRetrieval.WaitAsync().ConfigureAwait(false);
30-
31+
3132
Uri oidcMetadataEndpoint = null;
3233
try
3334
{
@@ -44,9 +45,13 @@ public static async Task<OidcMetadata> GetOidcAsync(
4445
builder.Path = existingPath.TrimEnd('/') + "/" + Constants.WellKnownOpenIdConfigurationPath;
4546

4647
oidcMetadataEndpoint = builder.Uri;
47-
var client = new OAuth2Client(requestContext.Logger, requestContext.ServiceBundle.HttpManager, null);
48+
var client = new OAuth2Client(requestContext.Logger, requestContext.ServiceBundle.HttpManager, null);
4849
configuration = await client.DiscoverOidcMetadataAsync(oidcMetadataEndpoint, requestContext).ConfigureAwait(false);
4950

51+
// Validate the issuer before caching the configuration
52+
requestContext.Logger.Verbose(() => $"[OIDC Discovery] Validating issuer: {configuration.Issuer} against authority: {authority}");
53+
ValidateIssuer(new Uri(authority), configuration.Issuer);
54+
5055
s_cache[authority] = configuration;
5156
requestContext.Logger.Verbose(() => $"[OIDC Discovery] OIDC discovery retrieved metadata from the network for {authority}");
5257

@@ -70,6 +75,63 @@ public static async Task<OidcMetadata> GetOidcAsync(
7075
}
7176
}
7277

78+
/// <summary>
79+
/// Validates that the issuer in the OIDC metadata matches the authority.
80+
/// </summary>
81+
/// <param name="authority">The authority URL.</param>
82+
/// <param name="issuer">The issuer from the OIDC metadata - the single source of truth.</param>
83+
/// <exception cref="MsalServiceException">Thrown when issuer validation fails.</exception>
84+
private static void ValidateIssuer(Uri authority, string issuer)
85+
{
86+
// Normalize both URLs to handle trailing slash differences
87+
string normalizedAuthority = authority.AbsoluteUri.TrimEnd('/');
88+
string normalizedIssuer = issuer?.TrimEnd('/');
89+
90+
// Primary validation: check if normalized authority starts with normalized issuer (case-insensitive comparison)
91+
if (normalizedAuthority.StartsWith(normalizedIssuer, StringComparison.OrdinalIgnoreCase))
92+
{
93+
return;
94+
}
95+
96+
// Extract tenant for CIAM scenarios. In a CIAM scenario the issuer is expected to have "{tenant}.ciamlogin.com"
97+
// as the host, even when using a custom domain.
98+
string tenant = null;
99+
try
100+
{
101+
tenant = AuthorityInfo.GetFirstPathSegment(authority);
102+
}
103+
catch (InvalidOperationException)
104+
{
105+
// If no path segments exist, try to extract from hostname (first part)
106+
var hostParts = authority.Host.Split('.');
107+
tenant = hostParts.Length > 0 ? hostParts[0] : null;
108+
}
109+
110+
// If tenant extraction failed or returned empty, validation fails
111+
if (!string.IsNullOrEmpty(tenant))
112+
{
113+
// Create a collection of valid CIAM issuer patterns for the tenant
114+
string[] validCiamPatterns =
115+
{
116+
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}",
117+
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}/{tenant}",
118+
$"https://{tenant}{Constants.CiamAuthorityHostSuffix}/{tenant}/v2.0"
119+
};
120+
121+
// Normalize and check if the issuer matches any of the valid patterns
122+
if (validCiamPatterns.Any(pattern =>
123+
string.Equals(normalizedIssuer, pattern.TrimEnd('/'), StringComparison.OrdinalIgnoreCase)))
124+
{
125+
return;
126+
}
127+
}
128+
129+
// Validation failed
130+
throw new MsalServiceException(
131+
MsalError.AuthorityValidationFailed,
132+
string.Format(MsalErrorMessage.IssuerValidationFailed, authority, issuer));
133+
}
134+
73135
// For testing purposes only
74136
public static void ResetCacheForTest()
75137
{

‎src/client/Microsoft.Identity.Client/MsalErrorMessage.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ public static string iOSBrokerKeySaveFailed(string keyChainResult)
149149

150150
public const string AuthorityValidationFailed = "Authority validation failed. ";
151151

152+
public const string IssuerValidationFailed = "Issuer validation failed for authority: {0} . Issuer from OIDC endpoint does not match any expected pattern: {1} . ";
153+
152154
public const string AuthorityUriInsecure = "The authority must use HTTPS scheme. ";
153155

154156
public const string AuthorityUriInvalidPath =

‎tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/CiamIntegrationTests.cs‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,27 +111,26 @@ public async Task ClientCredentialCiam_WithClientCredentials_ReturnsValidTokens(
111111
//Ciam CUD
112112
authority = "https://login.msidlabsciam.com/fe362aec-5d43-45d1-b730-9755e60dc3b9/v2.0/";
113113
string ciamClient = "b244c86f-ed88-45bf-abda-6b37aa482c79";
114-
await RunCiamCCATest(authority, ciamClient).ConfigureAwait(false);
114+
await RunCiamCCATest(authority, ciamClient, true).ConfigureAwait(false);
115115
}
116116

117-
private async Task RunCiamCCATest(string authority, string appId)
117+
private async Task RunCiamCCATest(string authority, string appId, bool useOidcAuthority = false)
118118
{
119119
//Acquire tokens
120120
var msalConfidentialClientBuilder = ConfidentialClientApplicationBuilder
121121
.Create(appId)
122122
.WithCertificate(CertificateHelper.FindCertificateByName(TestConstants.AutomationTestCertName))
123123
.WithExperimentalFeatures();
124124

125-
if (authority.Contains(Constants.CiamAuthorityHostSuffix))
125+
if (useOidcAuthority)
126126
{
127-
msalConfidentialClientBuilder.WithAuthority(authority, false);
127+
msalConfidentialClientBuilder.WithOidcAuthority(authority);
128128
}
129129
else
130130
{
131-
msalConfidentialClientBuilder.WithOidcAuthority(authority);
131+
msalConfidentialClientBuilder.WithAuthority(authority);
132132
}
133133

134-
135134
var msalConfidentialClient = msalConfidentialClientBuilder.Build();
136135

137136
var result = await msalConfidentialClient
@@ -217,6 +216,29 @@ public async Task OBOCiam_CustomDomain_ReturnsValidTokens()
217216
Assert.AreEqual(TokenSource.Cache, resultObo.AuthenticationResultMetadata.TokenSource);
218217
}
219218

219+
[TestMethod]
220+
public async Task WithOidcAuthority_ValidatesIssuerSuccessfully()
221+
{
222+
//Get lab details
223+
var labResponse = await LabUserHelper.GetLabUserDataAsync(new UserQuery()
224+
{
225+
FederationProvider = FederationProvider.CIAMCUD,
226+
SignInAudience = SignInAudience.AzureAdMyOrg
227+
}).ConfigureAwait(false);
228+
229+
//Test with standard and CUD CIAM authorities
230+
string[] authorities =
231+
{
232+
string.Format("https://{0}.ciamlogin.com/{1}/v2.0/", labResponse.Lab.TenantId, labResponse.Lab.TenantId),
233+
string.Format("https://login.msidlabsciam.com/{0}/v2.0/", labResponse.Lab.TenantId)
234+
};
235+
236+
foreach (var authority in authorities)
237+
{
238+
await RunCiamCCATest(authority, labResponse.App.AppId, true).ConfigureAwait(false);
239+
}
240+
}
241+
220242
private string GetCiamSecret()
221243
{
222244
KeyVaultSecretsProvider provider = new KeyVaultSecretsProvider();

‎tests/Microsoft.Identity.Test.Unit/CoreTests/InstanceTests/GenericAuthorityTests.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,45 @@ public async Task BadOidcResponse_ThrowsException_Async(string badOidcResponseTy
333333
}
334334
}
335335

336+
[TestMethod]
337+
public async Task OidcIssuerValidation_ThrowsForNonMatchingIssuer_Async()
338+
{
339+
using (var httpManager = new MockHttpManager())
340+
{
341+
string wrongIssuer = "https://wrong.issuer.com";
342+
343+
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
344+
.Create(TestConstants.ClientId)
345+
.WithHttpManager(httpManager)
346+
.WithOidcAuthority(TestConstants.GenericAuthority)
347+
.WithClientSecret(TestConstants.ClientSecret)
348+
.Build();
349+
350+
// Create OIDC document with non-matching issuer
351+
string validOidcDocumentWithWrongIssuer = TestConstants.GenericOidcResponse.Replace(
352+
$"\"issuer\":\"{TestConstants.GenericAuthority}\"",
353+
$"\"issuer\":\"{wrongIssuer}\"");
354+
355+
// Mock OIDC endpoint response
356+
httpManager.AddMockHandler(new MockHttpMessageHandler
357+
{
358+
ExpectedMethod = HttpMethod.Get,
359+
ExpectedUrl = $"{TestConstants.GenericAuthority}/{Constants.WellKnownOpenIdConfigurationPath}",
360+
ResponseMessage = MockHelpers.CreateSuccessResponseMessage(validOidcDocumentWithWrongIssuer)
361+
});
362+
363+
var ex = await AssertException.TaskThrowsAsync<MsalServiceException>(() =>
364+
app.AcquireTokenForClient(new[] { "api" }).ExecuteAsync()
365+
).ConfigureAwait(false);
366+
367+
string expectedErrorMessage = string.Format(MsalErrorMessage.IssuerValidationFailed, app.Authority, wrongIssuer);
368+
369+
Assert.AreEqual(MsalError.AuthorityValidationFailed, ex.ErrorCode);
370+
Assert.AreEqual(expectedErrorMessage, ex.Message,
371+
"Error message should match the expected error message.");
372+
}
373+
}
374+
336375
private static MockHttpMessageHandler CreateTokenResponseHttpHandler(
337376
string tokenEndpoint,
338377
string scopesInRequest,

0 commit comments

Comments
 (0)