-
Notifications
You must be signed in to change notification settings - Fork 379
Consolidate fragmented certificate APIs with certificate provider support #5571
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?
Conversation
… bearer/PoP tokens, and claims Co-authored-by: gladjohn <[email protected]>
Co-authored-by: gladjohn <[email protected]>
Co-authored-by: gladjohn <[email protected]>
…e to WithCertificate overload Co-authored-by: gladjohn <[email protected]>
You are also missing a few properties like:
|
|
@copilot question, what if the app was created with a cert but now the request needs to shift between bearer and pop tokens. is this design ready to handle that? |
Good question! The current design applies configuration at app-level, so switching between bearer/PoP per request isn't supported. I'll revise to support request-time control: keep mTLS enablement in CertificateConfiguration, but move the bearer/PoP choice to the request level with
... You're right on all points. I'll address:
Working on these changes now. |
@copilot can we address these concerns |
…e provider, improve claims docs Co-authored-by: gladjohn <[email protected]>
... Yes, all concerns addressed in commit 4697a6f:
CertificateConfiguration now focuses on certificate properties only, with token strategy (PoP/bearer) configurable per-request. |
|
@copilot let's just make sure at the request we are not adding any new APIs for bearer. For POP we will add WithMtlsProofOfPossession but for Bearer MSAL should work the way it does today. Also have you written tests? |
…new API Co-authored-by: gladjohn <[email protected]>
Done in commit 75a6e6d: Removed Only new API: Tests: Yes, comprehensive test suite exists in
|
Fixes #5568
Changes proposed in this request
MSAL.NET has 7+ certificate-related methods scattered across builders with boolean parameters and split concerns. This PR consolidates them into a unified
CertificateConfigurationAPI that focuses on certificate properties only. mTLS PoP tokens are enabled at request-time using the existingWithMtlsProofOfPossession()method, while bearer tokens continue to work exactly as they do today without any new APIs.New unified API:
Request-time mTLS control:
Core implementation:
CertificateConfigurationclass - Focused on certificate properties only (not token strategy)WithCertificate(CertificateConfiguration)- Overload of existing methodFunc<X509Certificate2>support - Certificate providers for rotation scenariosAssociateTokensWithCertificate- Cache tokens by cert thumbprint + claims hashClaimsToSign(JWT) vsClaims(request parameter)Consolidates:
WithCertificate(cert, sendX5C)→SendX5CpropertyWithClientClaims(cert, claims, merge, sendX5C)→ClaimsToSignproperty.WithMtlsProofOfPossession()for PoP tokens (existing API, no changes)Key design decisions based on feedback:
.WithMtlsProofOfPossession()at request-timeFunc<X509Certificate2>enables rotation scenariosAlignment with PR #5399:
Certificate-first scenarios use
CertificateConfiguration. Assertion-first scenarios continue usingWithClientAssertion. Both patterns coexist for different developer mental models.Example:
All existing APIs remain functional for backward compatibility.
Testing
CertificateConfigurationTests.cs(261 lines)CertificateConfigurationwith all property combinationsX509Certificate2andFunc<X509Certificate2>)WithMtlsProofOfPossession()Performance impact
No performance impact:
AssociateTokensWithCertificateis trueDocumentation
ClaimsToSign(JWT claims signed in client assertion) andClaims(Conditional Access request parameter)WithMtlsProofOfPossession()methodOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.