Skip to content

Credential system refactor: unified context, canonical matrix, eliminate dead code#5831

Closed
Copilot wants to merge 3 commits into
mainfrom
copilot/refactor-credential-system-again
Closed

Credential system refactor: unified context, canonical matrix, eliminate dead code#5831
Copilot wants to merge 3 commits into
mainfrom
copilot/refactor-credential-system-again

Conversation

Copilot AI commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

Rewrites MSAL.NET's internal credential resolution architecture to replace the AddConfidentialClientParametersAsync / ClientCredentialApplicationResult pattern with a clean, decoupled pipeline.

Core design changes

  • IClientCredential interface — replaces AddConfidentialClientParametersAsync(OAuth2Client, AuthenticationRequestParameters, ...) with GetCredentialMaterialAsync(CredentialContext, CancellationToken). Credentials now return data; they no longer write directly to OAuth2Client.

  • CredentialContext (immutable struct) — single input carrying all 12 resolution parameters (ClientId, TokenEndpoint, Mode, Claims, CryptographyManager, SendX5C, UseSha2, AzureRegion, …). Replaces scattered parameter passing.

  • CredentialMaterial (output class) — replaces ClientCredentialApplicationResult. Fields: TokenRequestParameters (never null), Source (Static/Callback), ResolvedCertificate (optional).

  • ClientAuthMode enum (Regular / MtlsMode) — replaces the IsMtlsPopRequested + UseJwtPopClientAssertion boolean pair.

  • CredentialMaterialResolver — central static coordinator invoked exactly once per request by TokenClient. Builds CredentialContext, calls the credential, validates invariants (InvalidOperationException for null returns; MsalClientException for unsupported mode combinations).

  • CREDENTIAL_MATRIX.cs — documentation-only file enumerating all 10 supported (credential-type × mode) combinations and their TokenRequestParameters / ResolvedCertificate outputs.

Credential implementations rewritten (CertificateAndClaimsClientCredential, SecretStringClientCredential, SignedAssertionClientCredential, ClientAssertionDelegateCredential, ClientAssertionStringDelegateCredential) — each now returns CredentialMaterial; unsupported mTLS combinations throw MsalClientException(InvalidCredentialMaterial).

Dead code removedClientCredentialApplicationResult gutted; Stopwatch tracking, hash-prefix helpers, and defensive parameter guards deleted from credential implementations.

MsalError.InvalidCredentialMaterial added (public constant, all platform PublicAPI.Unshipped.txt files updated).

Testing

18 new unit tests in CredentialMatrixTests.cs cover every matrix row (X509Cert × Regular/Mtls, Secret × Regular/Mtls, static JWT × Regular/Mtls, JWT+cert callback × Regular/Mtls) plus edge cases: null certificate from callback, empty assertion, null TokenRequestParameters at construction. All 18 pass; existing failures are pre-existing (missing test resource files, Windows-only CNG on Linux CI).

Performance impact

No measurable impact. One fewer async call per token request (resolver path eliminates the double-call to assertion providers that existed in MtlsPopParametersInitializer).

Documentation

  • All relevant documentation is updated.

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:

  • contoso.adfs.contoso.com
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 44807 --endpoint 127.0.0.1:044807 --role client --parentprocessid 7180 --telemetryoptedin false "$k=".$v["local--norc (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 42683 --endpoint 127.0.0.1:042683 --role client --parentprocessid 7424 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 33073 --endpoint 127.0.0.1:033073 --role client --parentprocessid 9036 --telemetryoptedin false (dns block)
  • contoso.b2clogin.com
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 44807 --endpoint 127.0.0.1:044807 --role client --parentprocessid 7180 --telemetryoptedin false "$k=".$v["local--norc (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 42683 --endpoint 127.0.0.1:042683 --role client --parentprocessid 7424 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 33073 --endpoint 127.0.0.1:033073 --role client --parentprocessid 9036 --telemetryoptedin false (dns block)
  • login.microsoftonline.com
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 44807 --endpoint 127.0.0.1:044807 --role client --parentprocessid 7180 --telemetryoptedin false "$k=".$v["local--norc (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 42683 --endpoint 127.0.0.1:042683 --role client --parentprocessid 7424 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 37453 --endpoint 127.0.0.1:037453 --role client --parentprocessid 7669 --telemetryoptedin false (dns block)
  • some.url.dsts.core.azure-test.net
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 44807 --endpoint 127.0.0.1:044807 --role client --parentprocessid 7180 --telemetryoptedin false "$k=".$v["local--norc (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 42683 --endpoint 127.0.0.1:042683 --role client --parentprocessid 7424 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.runtimeconfig.json --depsfile /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/Microsoft.Identity.Test.Unit.deps.json /home/REDACTED/work/microsoft-authentication-library-for-dotnet/microsoft-authentication-library-for-dotnet/tests/Microsoft.Identity.Test.Unit/bin/Debug/net8.0/testhost.dll --port 33073 --endpoint 127.0.0.1:033073 --role client --parentprocessid 9036 --telemetryoptedin false (dns block)

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

Original prompt

Credential System Refactor - Complete Rewrite

Overview

This PR comprehensively rewrites the credential system to address all 32 code review comments from PR #5748. The refactor introduces a clean, testable architecture for credential resolution.

Key Changes

1. Single ClientAuthMode Enum (Addresses comments #2795377908, #2799230854)

  • Replaces confusing boolean flags (IsMtlsPopRequested, UseJwtPopClientAssertion)
  • Two modes: Regular (JWT/secret) and MtlsMode (certificate-only)
  • Makes intent explicit and mode selection deterministic

2. Unified CredentialContext (Addresses comments #2795282172, #2795298829, #2798482280)

  • Single immutable struct containing all credential resolution inputs
  • Merged from CredentialRequestContext and MtlsValidationContext
  • Properties: ClientId, TokenEndpoint, Mode, Claims, ClientCapabilities, CryptographyManager, SendX5C, UseSha2, ExtraClientAssertionClaims, ClientAssertionFmiPath, AuthorityType, AzureRegion

3. CredentialMaterial Output (Addresses comments #2798443132, #2798439218, #2798551197)

  • Normalized credential resolution output
  • Replaces ClientCredentialApplicationResult
  • Properties: TokenRequestParameters (never null), Source (Static/Callback), ResolvedCertificate (optional)
  • Decouples "what credentials produce" from "how they're used"

4. CredentialMaterialResolver (Addresses comments #2798553940, #2798559671, #2798550191)

  • Central authority for credential invocation
  • Exactly once per logical request
  • Validates structure invariants using InvalidOperationException
  • Enforces mTLS constraints using MsalClientException

5. Canonical Credential Matrix (Addresses comment #2798409064)

  • Documents all supported input/mode/output combinations in CREDENTIAL_MATRIX.cs
  • 10 rows: X509Cert, secret, JWT, JWT+cert × Regular/MtlsMode
  • Unsupported combinations throw MsalClientException

6. Removed Dead Code (Addresses comments #2795673346, #2795667204, #2798451217, #2798444768, #2798448380, #2798413416)

  • Deleted ClientCredentialApplicationResult class
  • Deleted CredentialMaterialMetadata
  • Deleted Stopwatch tracking
  • Deleted hash prefix helper
  • Deleted parameter guard assertions
  • Removed unnecessary logging

7. Proper Enum Usage (Addresses comment #2795347697)

  • CredentialSource enum (Static vs Callback) instead of string
  • Type-safe credential source tracking

8. Exception Taxonomy (Addresses comments #2798550191, #2798551197, #2798417759)

  • InvalidOperationException for invariant violations (credential returned null, TokenRequestParameters is null)
  • MsalClientException for configuration errors (unsupported mode combinations)
  • Removed defensive code guards (use tests instead)

9. Explicit CancellationToken (Addresses comment #2798464735)

  • Removed CancellationToken from CredentialContext
  • Explicit parameter in GetCredentialMaterialAsync
  • Follows .NET conventions (CancellationToken as last parameter)

10. Comprehensive Test Coverage (16 tests)

  • All matrix rows: X509Cert (Regular/MtlsMode), Secret (Regular/MtlsMode), JWT (Regular/MtlsMode), JWT+Cert (Regular/MtlsMode)
  • Callback vs Static sources
  • Edge cases: null certificates, empty assertions
  • Token parameter validation

Interface Changes

IClientCredential (Before)

Task<ClientCredentialApplicationResult> AddConfidentialClientParametersAsync(
    OAuth2Client oAuth2Client,
    AuthenticationRequestParameters authenticationRequestParameters,
    ICryptographyManager cryptographyManager,
    string tokenEndpoint,
    CancellationToken cancellationToken);

IClientCredential (After)

Task<CredentialMaterial> GetCredentialMaterialAsync(
    CredentialContext context,
    CancellationToken cancellationToken);

Benefits

Cleaner Architecture - Credentials focus on "what to produce", not "how to apply"
Type Safety - Enums instead of strings/booleans
Single Source of Truth - Canonical matrix documents all supported combinations
Better Testability - No OAuth2Client coupling, immutable context
Explicit Error Handling - Clear distinction between invariant violations and config errors
Reduced Complexity - No dead code, no defensive guards in product code
Standards Compliance - Follows .NET conventions

Files Modified

  • ClientAuthMode.cs (NEW)
  • CredentialContext.cs (NEW)
  • CredentialMaterial.cs (NEW)
  • CredentialMaterialResolver.cs (NEW)
  • CREDENTIAL_MATRIX.cs (NEW)
  • IClientCredential.cs (MODIFIED - new method signature)
  • CertificateAndClaimsClientCredential.cs (REWRITTEN)
  • SecretStringClientCredential.cs (REWRITTEN)
  • SignedAssertionClientCredential.cs (REWRITTEN)
  • ClientAssertionDelegateCredential.cs (REWRITTEN)
  • ClientAssertionStringDelegateCredential.cs (REWRITTEN)
  • ClientCredentialApplicationResult.cs (DELETED)
  • `AuthenticationRequestPa...

This pull request was created from Copilot chat.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits March 10, 2026 17:05
…edentialMaterial, and CredentialMaterialResolver

Co-authored-by: gladjohn <90415114+gladjohn@users.noreply.github.com>
…nes and convert synchronous tests to non-async

Co-authored-by: gladjohn <90415114+gladjohn@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor credential system to improve architecture Credential system refactor: unified context, canonical matrix, eliminate dead code Mar 10, 2026
@gladjohn gladjohn closed this Mar 10, 2026
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.

2 participants