Skip to content

Add unit tests with HTTP mocking for vanilla dSTS + fix Authority par… - #3805

Merged
XiaoxinMS2 merged 7 commits into
masterfrom
zhanli1/dsts-unit-tests-with-http-mocking
May 21, 2026
Merged

Add unit tests with HTTP mocking for vanilla dSTS + fix Authority par…#3805
XiaoxinMS2 merged 7 commits into
masterfrom
zhanli1/dsts-unit-tests-with-http-mocking

Conversation

@XiaoxinMS2

@XiaoxinMS2 XiaoxinMS2 commented May 5, 2026

Copy link
Copy Markdown
Contributor

…sing

Adds 6 HTTP-mocked unit tests for the vanilla dSTS (Dedicated Security Token Service) token-acquisition path in Microsoft.Identity.Web, and fixes MergedOptions.ParseAuthorityIfNecessary so that the natural / documented dSTS configuration form works end-to-end:

options.Authority = "https://{host}/dstsv2/{tenantGuid}";

Without the fix, the AAD-style parser took the literal "dstsv2" as the tenant and dropped the actual tenant GUID, producing an authority MSAL rejected with "The DSTS authority URI should have at least 2 segments...".

Tests cover: token endpoint URI; client_credentials grant body; second-call cache hit; OAuth2 error -> MsalServiceException mapping; SendX5C=true includes x5c JWT header; SendX5C=false omits it. All tests use the existing MockHttpClientFactory infrastructure (no real network / Key Vault / cert) and run in any CI environment.

No public API changes.

Add unit tests with HTTP mocking for vanilla dSTS scenarios + fix Authority-only configuration

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Description

Adds 6 HTTP-mocked unit tests covering the vanilla dSTS (Dedicated Security Token Service) token-acquisition path in Microsoft.Identity.Web, and fixes a parser bug in MergedOptions.ParseAuthorityIfNecessary that prevented the natural / documented dSTS configuration form (options.Authority = "https://{host}/dstsv2/{tenantGuid}") from working.

Both changes ship together because the new tests use the natural configuration form, which is the form the parser fix unblocks.

Vanilla dSTS support in Id.Web previously had zero unit-test coverage — the only assertion lived in integration tests that required a real dSTS deployment + Key Vault certificate, so they couldn't run in CI. Anyone refactoring MergedOptions / TokenAcquirerFactory could silently break dSTS token acquisition without any signal.

Fixes #{bug number} (in this specific format)

@XiaoxinMS2
XiaoxinMS2 requested a review from a team as a code owner May 5, 2026 14:57
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.
@XiaoxinMS2
XiaoxinMS2 force-pushed the zhanli1/dsts-unit-tests-with-http-mocking branch from eedbd53 to c625531 Compare May 5, 2026 21:54
Comment thread src/Microsoft.Identity.Web.TokenAcquisition/MergedOptions.cs Outdated

Copilot AI commented May 21, 2026

Copy link
Copy Markdown
Contributor

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • login.microsoftonline.com
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-identity-web/microsoft-identity-web/tests/Microsoft.Identity.Web.Test/bin/Release/net8.0/Microsoft.Identity.Web.Test.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-identity-web/microsoft-identity-web/tests/Microsoft.Identity.Web.Test/bin/Release/net8.0/Microsoft.Identity.Web.Test.deps.json /home/REDACTED/work/microsoft-identity-web/microsoft-identity-web/tests/Microsoft.Identity.Web.Test/bin/Release/net8.0/testhost.dll --port 40265 --endpoint 127.0.0.1:040265 --role client --parentprocessid 5124 --telemetryoptedin false (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@XiaoxinMS2

XiaoxinMS2 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants