Skip to content

Commit c625531

Browse files
author
Zhan Li
committed
Add unit tests with HTTP mocking for vanilla dSTS scenarios
Adds 7 HTTP-mocked unit tests for the vanilla dSTS (Dedicated Security Token Service) token-acquisition path in Microsoft.Identity.Web. Per PR review feedback, dSTS users MUST configure 'Instance' (e.g. "https://{host}/dstsv2") and 'TenantId' separately. The single-string 'Authority' option is reserved for vanilla OIDC / CIAM scenarios and routes through MSAL.WithOidcAuthority(), which is incompatible with dSTS; configuring a dSTS-style URL there now throws an InvalidOperationException with a clear, actionable error message instead of letting MSAL surface its opaque "DSTS authority URI should have at least 2 segments..." error later. Tests cover (canonical Instance + TenantId shape): 1. Token endpoint URI lock (POST to https://{host}/dstsv2/{tenant}/oauth2/v2.0/token) 2. Client_credentials grant body (grant_type, scope, client_id, client_secret) 3. Second-call cache hit (only one mock handler registered) 4. OAuth2 token-endpoint error -> MsalServiceException mapping 5. SendX5C=true -> client_assertion JWT header includes x5c 6. SendX5C=false -> x5c omitted Plus 1 negative test: 7. Configuring dSTS URL via 'Authority' option -> InvalidOperationException with clear guidance to use Instance + TenantId All tests use the existing MockHttpClientFactory infrastructure (no real network / Key Vault / cert) and run in any CI environment. Implementation change in MergedOptions.ParseAuthorityIfNecessary: detects dSTS-shaped Authority (path segment "dstsv2") and throws an InvalidOperationException with a message that points users to the canonical Instance+TenantId shape. No public API changes.
1 parent ef54b6e commit c625531

2 files changed

Lines changed: 484 additions & 25 deletions

File tree

src/Microsoft.Identity.Web.TokenAcquisition/MergedOptions.cs

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ namespace Microsoft.Identity.Web
2020
/// Options for configuring authentication using Azure Active Directory. It has both AAD and B2C configuration attributes.
2121
/// Merges the MicrosoftIdentityWebOptions and the ConfidentialClientApplicationOptions.
2222
/// </summary>
23-
/*
24-
* Used by Microsoft.Identity.Web, Microsoft.Identity.Web.OWIN
25-
* Any changes to this member (including removal) can cause runtime failures.
26-
* Treat as a public member.
27-
*/
23+
/*
24+
* Used by Microsoft.Identity.Web, Microsoft.Identity.Web.OWIN
25+
* Any changes to this member (including removal) can cause runtime failures.
26+
* Treat as a public member.
27+
*/
2828
internal sealed class MergedOptions : MicrosoftIdentityOptions
2929
{
3030
private ConfidentialClientApplicationOptions? _confidentialClientApplicationOptions;
@@ -63,18 +63,18 @@ public ConfidentialClientApplicationOptions ConfidentialClientApplicationOptions
6363
public LogLevel LogLevel { get; set; }
6464
public string? RedirectUri { get; set; }
6565
public bool EnableCacheSynchronization { get; set; }
66-
/*
67-
* Used by Microsoft.Identity.Web.OWIN
68-
* Any changes to this member (including removal) can cause runtime failures.
69-
* Treat as a public member.
70-
*/
66+
/*
67+
* Used by Microsoft.Identity.Web.OWIN
68+
* Any changes to this member (including removal) can cause runtime failures.
69+
* Treat as a public member.
70+
*/
7171
internal bool MergedWithCca { get; set; }
7272
// This is for supporting for CIAM authorities including custom url domains, see https://github.com/AzureAD/microsoft-identity-web/issues/2690
73-
/*
74-
* Used by Microsoft.Identity.Web
75-
* Any changes to this member (including removal) can cause runtime failures.
76-
* Treat as a public member.
77-
*/
73+
/*
74+
* Used by Microsoft.Identity.Web
75+
* Any changes to this member (including removal) can cause runtime failures.
76+
* Treat as a public member.
77+
*/
7878
internal bool PreserveAuthority { get; set; }
7979

8080
/// <summary>
@@ -353,11 +353,11 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId
353353
}
354354
}
355355

356-
/*
357-
* Used by Microsoft.Identity.Web
358-
* Any changes to this member (including removal) can cause runtime failures.
359-
* Treat as a public member.
360-
*/
356+
/*
357+
* Used by Microsoft.Identity.Web
358+
* Any changes to this member (including removal) can cause runtime failures.
359+
* Treat as a public member.
360+
*/
361361
internal static void UpdateMergedOptionsFromConfidentialClientApplicationOptions(ConfidentialClientApplicationOptions confidentialClientApplicationOptions, MergedOptions mergedOptions)
362362
{
363363
mergedOptions.MergedWithCca = true;
@@ -466,11 +466,11 @@ internal static void UpdateConfidentialClientApplicationOptionsFromMergedOptions
466466
}
467467
}
468468

469-
/*
470-
* Used by Microsoft.Identity.Web.OWIN
471-
* Any changes to this member (including removal) can cause runtime failures.
472-
* Treat as a public member.
473-
*/
469+
/*
470+
* Used by Microsoft.Identity.Web.OWIN
471+
* Any changes to this member (including removal) can cause runtime failures.
472+
* Treat as a public member.
473+
*/
474474
internal static void ParseAuthorityIfNecessary(MergedOptions mergedOptions, IdWebLogger.ILogger? logger = null)
475475
{
476476
// Check if Authority is configured but being ignored due to Instance/TenantId taking precedence
@@ -504,6 +504,28 @@ internal static void ParseAuthorityIfNecessary(MergedOptions mergedOptions, IdWe
504504
int indexVersion = authoritySpan.Slice(indexTenant + 1).IndexOf('/');
505505
int indexEndOfTenant = indexVersion == -1 ? authoritySpan.Length : indexVersion + indexTenant + 1;
506506

507+
// dSTS authorities have the shape https://{host}/dstsv2/{tenantGuid}, i.e. TWO path
508+
// segments instead of the AAD-style single segment. By design (cf. PR review),
509+
// the single 'Authority' string is reserved for vanilla OIDC / CIAM scenarios,
510+
// which route through MSAL.WithOidcAuthority() — a path that is incompatible with
511+
// dSTS. dSTS users MUST configure 'Instance' and 'TenantId' separately so that
512+
// the request flows through MSAL.WithAuthority() instead.
513+
//
514+
// Detecting the "dstsv2" path segment here lets us bail with a clear, actionable
515+
// error message instead of letting the generic AAD parser silently drop the
516+
// tenant GUID, which would surface later as MSAL's opaque
517+
// "The DSTS authority URI should have at least 2 segments..."
518+
ReadOnlySpan<char> firstPathSegment = authoritySpan.Slice(indexTenant + 1, indexEndOfTenant - indexTenant - 1);
519+
if (firstPathSegment.Equals("dstsv2".AsSpan(), StringComparison.OrdinalIgnoreCase))
520+
{
521+
throw new InvalidOperationException(
522+
"Configuring a dSTS authority via the single 'Authority' option is not supported. " +
523+
"The 'Authority' option targets vanilla OIDC / CIAM scenarios and routes through " +
524+
"MSAL.WithOidcAuthority(), which is incompatible with dSTS. " +
525+
"For dSTS, configure 'Instance' (e.g. \"https://{host}/dstsv2\") and 'TenantId' " +
526+
"(the dSTS tenant GUID) separately so the request flows through MSAL.WithAuthority().");
527+
}
528+
507529
// In CIAM and B2C, customers will use "authority", not Instance and TenantId
508530
mergedOptions.Instance = mergedOptions.PreserveAuthority ? mergedOptions.Authority! : authoritySpan.Slice(0, indexTenant).ToString();
509531
mergedOptions.TenantId = mergedOptions.PreserveAuthority ? null : authoritySpan.Slice(indexTenant + 1, indexEndOfTenant - indexTenant - 1).ToString();

0 commit comments

Comments
 (0)