-
Notifications
You must be signed in to change notification settings - Fork 400
Refactor client credential material resolution #5835
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
7052fed
22cb7d2
7ef8d96
1503f8e
bb40a47
dc8e80f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -259,3 +259,4 @@ package-lock.json | |
|
|
||
| # ignore JetBrains Rider files | ||
| .idea/ | ||
| /git | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,19 +2,18 @@ | |
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Identity.Client.Core; | ||
| using Microsoft.Identity.Client.Internal.Requests; | ||
| using Microsoft.Identity.Client.OAuth2; | ||
| using Microsoft.Identity.Client.PlatformsCommon.Interfaces; | ||
| using Microsoft.Identity.Client.TelemetryCore; | ||
|
|
||
| namespace Microsoft.Identity.Client.Internal.ClientCredential | ||
| { | ||
| /// <summary> | ||
| /// Handles client assertions supplied via a delegate that returns an | ||
| /// <see cref="ClientSignedAssertion"/> (JWT + optional certificate bound for mTLS‑PoP). | ||
| /// Handles client assertions supplied via a delegate that returns a | ||
| /// <see cref="ClientSignedAssertion"/> (JWT + optional certificate bound for mTLS-PoP). | ||
| /// </summary> | ||
| internal sealed class ClientAssertionDelegateCredential : IClientCredential, IClientSignedAssertionProvider | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add tests or a debug assertion to ensure that each credential, and especially this one, is called once and only once per token request?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added test coverage to verify the callback-backed credential path is invoked exactly once per token request. |
||
| { | ||
|
|
@@ -26,41 +25,34 @@ internal ClientAssertionDelegateCredential( | |
| _provider = provider ?? throw new ArgumentNullException(nameof(provider)); | ||
| } | ||
|
|
||
| // Private helper for internal readability | ||
| private Task<ClientSignedAssertion> GetAssertionAsync( | ||
| AssertionRequestOptions options, | ||
| CancellationToken cancellationToken) => | ||
| _provider(options, cancellationToken); | ||
|
|
||
| // Capability interface (only used where we intentionally cast to check the capability) | ||
| Task<ClientSignedAssertion> IClientSignedAssertionProvider.GetAssertionAsync( | ||
| AssertionRequestOptions options, | ||
| CancellationToken cancellationToken) => | ||
| GetAssertionAsync(options, cancellationToken); | ||
| _provider(options, cancellationToken); | ||
|
|
||
| public AssertionType AssertionType => AssertionType.ClientAssertion; | ||
|
|
||
| // ────────────────────────────────── | ||
| // Main hook for token requests | ||
| // ────────────────────────────────── | ||
| public async Task<ClientCredentialApplicationResult> AddConfidentialClientParametersAsync( | ||
| OAuth2Client oAuth2Client, | ||
| AuthenticationRequestParameters p, | ||
| ICryptographyManager _, | ||
| string tokenEndpoint, | ||
| CancellationToken ct) | ||
| public async Task<CredentialMaterial> GetCredentialMaterialAsync( | ||
| CredentialContext context, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| context.Logger.Verbose(() => $"[ClientAssertionDelegateCredential] Resolving client assertion material. " + | ||
| $"Mode={context.Mode}, TokenEndpoint={context.TokenEndpoint}"); | ||
|
|
||
| var opts = new AssertionRequestOptions | ||
| { | ||
| CancellationToken = ct, | ||
| ClientID = p.AppConfig.ClientId, | ||
| TokenEndpoint = tokenEndpoint, | ||
| ClientCapabilities = p.RequestContext.ServiceBundle.Config.ClientCapabilities, | ||
| Claims = p.Claims, | ||
| ClientAssertionFmiPath = p.ClientAssertionFmiPath | ||
| CancellationToken = cancellationToken, | ||
| ClientID = context.ClientId, | ||
| TokenEndpoint = context.TokenEndpoint, | ||
| ClientCapabilities = context.ClientCapabilities, | ||
| Claims = context.Claims, | ||
| ClientAssertionFmiPath = context.ClientAssertionFmiPath | ||
| }; | ||
|
|
||
| ClientSignedAssertion resp = await GetAssertionAsync(opts, ct).ConfigureAwait(false); | ||
| context.Logger.Verbose(() => "[ClientAssertionDelegateCredential] Invoking client assertion provider delegate."); | ||
|
|
||
| ClientSignedAssertion resp = await _provider(opts, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(resp?.Assertion)) | ||
| { | ||
|
|
@@ -71,28 +63,35 @@ public async Task<ClientCredentialApplicationResult> AddConfidentialClientParame | |
|
|
||
| bool hasCert = resp.TokenBindingCertificate != null; | ||
|
|
||
| // If PoP was explicitly requested, we must have a certificate. | ||
| // (Preflight should enforce this too, but keep this defensive.) | ||
| if (p.IsMtlsPopRequested && !hasCert) | ||
| context.Logger.Verbose(() => $"[ClientAssertionDelegateCredential] Provider returned assertion. " + | ||
| $"TokenBindingCertificatePresent={hasCert}"); | ||
|
|
||
| if (context.Mode == OAuthMode.MtlsMode && !hasCert) | ||
| { | ||
| throw new MsalClientException( | ||
| MsalError.MtlsCertificateNotProvided, | ||
| MsalErrorMessage.MtlsCertificateNotProvidedMessage); | ||
| } | ||
|
|
||
| // JWT-PoP if explicit PoP was requested OR delegate returned a cert (implicit bearer-over-mTLS) | ||
| bool useJwtPop = p.IsMtlsPopRequested || hasCert; | ||
| // Select the appropriate assertion type based on the presence of a certificate and the OAuth mode. | ||
| string assertionType = | ||
| (context.Mode == OAuthMode.MtlsMode || hasCert) | ||
| ? OAuth2AssertionType.JwtPop | ||
| : OAuth2AssertionType.JwtBearer; | ||
|
|
||
| context.Logger.Verbose(() => $"[ClientAssertionDelegateCredential] Selected client assertion type: {assertionType}"); | ||
|
|
||
| oAuth2Client.AddBodyParameter( | ||
| OAuth2Parameter.ClientAssertionType, | ||
| useJwtPop ? OAuth2AssertionType.JwtPop : OAuth2AssertionType.JwtBearer); | ||
| var parameters = new Dictionary<string, string> | ||
| { | ||
| { OAuth2Parameter.ClientAssertionType, assertionType }, | ||
| { OAuth2Parameter.ClientAssertion, resp.Assertion } | ||
| }; | ||
|
|
||
| oAuth2Client.AddBodyParameter(OAuth2Parameter.ClientAssertion, resp.Assertion); | ||
| context.Logger.Verbose(() => "[ClientAssertionDelegateCredential] Client assertion material created successfully."); | ||
|
|
||
| // Only return a cert if we actually have one. | ||
| return hasCert | ||
| ? new ClientCredentialApplicationResult(useJwtPopClientAssertion: useJwtPop, mtlsCertificate: resp.TokenBindingCertificate) | ||
| : ClientCredentialApplicationResult.None; | ||
| return new CredentialMaterial( | ||
| parameters, | ||
| hasCert ? resp.TokenBindingCertificate : null); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.