-
Notifications
You must be signed in to change notification settings - Fork 378
Expose cert used in mtls flows in the auth result #5371
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
Conversation
|
@neha-bhargava @trwalke @bgavrilMS , need suggestion on how to fix the failing tests - PublicTestConstructorCoversAllProperties test fails with, Assert.AreEqual failed. Expected:<15>. Actual:<16>. The constructor should include all properties of AuthenticationObject but if I add the property to the test ctor, and update the public API, then I get this, Symbol 'AuthenticationResult' violates the backcompat requirement: 'Do not add multiple overloads with optional parameters'. See 'https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md' for details. |
i have a fix for this, hopefully not breaking any public APIs, please review carefully. |
...s/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/ClientCredentialsMtlsPopTests.cs
Outdated
Show resolved
Hide resolved
...s/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/ClientCredentialsMtlsPopTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for new integration test
4c9e84d to
fe22302
Compare
d797007 to
958ee16
Compare
Fixes #5370
Changes proposed in this request
This pull request introduces several updates to the
Microsoft.Identity.Clientlibrary, focusing on adding support for binding certificates in mTLS-PoP scenarios, improving theAuthenticationResultAPI, and enhancing test coverage. Key changes include the addition of aBindingCertificateproperty, modifications to property access levels, and updates to unit and integration tests to validate the new functionality.Updates to
AuthenticationResultAPI:BindingCertificateproperty toAuthenticationResultfor mTLS-PoP token scenarios. This property allows binding an X509 certificate to access tokens. (src/client/Microsoft.Identity.Client/AuthenticationResult.cs- [1] [2]AuthenticationResult(e.g.,UniqueId,ExpiresOn,TenantId,Account, etc.) tointernal setto enable modification within the library while maintaining encapsulation. (src/client/Microsoft.Identity.Client/AuthenticationResult.cs- [1] [2] [3]AuthenticationResultas obsolete to discourage direct use and encourage usage of factory methods or builders. (src/client/Microsoft.Identity.Client/AuthenticationResult.cs- [1] [2]Integration of mTLS-PoP functionality:
MtlsPopAuthenticationOperationto set theBindingCertificateproperty inAuthenticationResultduring the token acquisition process. (src/client/Microsoft.Identity.Client/AuthScheme/PoP/MtlsPopAuthenticationOperation.cs- src/client/Microsoft.Identity.Client/AuthScheme/PoP/MtlsPopAuthenticationOperation.csL41-R41)BindingCertificateproperty across various platform-specificPublicAPI.Unshipped.txtfiles. (src/client/Microsoft.Identity.Client/PublicApi/*/PublicAPI.Unshipped.txt- src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txtR3)Test enhancements:
BindingCertificatefunctionality in mTLS-PoP scenarios, ensuring the certificate is correctly set and retrieved from cache during token acquisition. (tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/ClientCredentialsMtlsPopTests.cs- [1] [2]BindingCertificateproperty and deprecated constructor usage warnings. (tests/Microsoft.Identity.Test.Unit/PublicApiTests/AuthenticationResultTests.cs- [1] [2] [3]BindingCertificatein cached and non-cached scenarios. (tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs- [1] [2]These changes collectively improve the library's support for mTLS-PoP scenarios, enhance API usability, and ensure robust test coverage for the new functionality.
Testing
unit and integration
Performance impact
none
Documentation