-
Notifications
You must be signed in to change notification settings - Fork 156
Refactor client credential behavior to be per-request #986
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 2, 2025
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java
Outdated
Show resolved
Hide resolved
sudoapt-getclean
approved these changes
Sep 3, 2025
bgavrilMS
approved these changes
Sep 3, 2025
bgavrilMS
reviewed
Sep 3, 2025
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java
Outdated
Show resolved
Hide resolved
bgavrilMS
reviewed
Sep 3, 2025
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java
Outdated
Show resolved
Hide resolved
bgavrilMS
reviewed
Sep 3, 2025
msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientCertificateTest.java
Show resolved
Hide resolved
bgavrilMS
previously requested changes
Sep 3, 2025
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.
assertion should be at request level?
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java
Outdated
Show resolved
Hide resolved
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Neha Bhargava <[email protected]>
…ntials # Conflicts: # msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientCredentialTest.java
… avdunn/improve-credentials
Improve behavior related to assertions
neha-bhargava
approved these changes
Sep 11, 2025
gladjohn
reviewed
Sep 11, 2025
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/Authority.java
Outdated
Show resolved
Hide resolved
gladjohn
reviewed
Sep 11, 2025
msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java
Show resolved
Hide resolved
gladjohn
approved these changes
Sep 11, 2025
trwalke
reviewed
Sep 11, 2025
trwalke
approved these changes
Sep 11, 2025
gladjohn
approved these changes
Sep 12, 2025
Requested changes were implemented.
bgavrilMS
approved these changes
Sep 12, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactors behavior related to client credentials and also fixes the regression described in #984
Version 1.23.0 introduced a large refactor to remove usage of third-party dependencies, in particular those from
com.nimbusds
. Prior to 1.23.0 assertions created forClientCertificate
were given 10 minute expiry time and were only recreated if the old one expired, however the refresh behavior was lost during a refactor in PR #927In addition, this PR fixes/improves assertion behavior to resolve the issues described in #879 and #977: when using the callback option for assertions the original design only called the callback when the
ClientAssertion
instance was first created, meaning an app-level assertion was only created onceThis PR fixes that regression and refactors the behavior around client credentials to be more in line with other MSALs:
ConfidentialClientApplication
, and move logic for creating assertion fromConfidentialClientApplication
toClientCertificate
ClientAssertion
andClientCredentialFactory
to properly call callbacks set at the app-levelTokenRequestExecutor
to use credential fields/methods directly, properly creates assertions when tenants are set at the request-level, and generally improve some complicated logicClientAssertion
andClientCredentialFactory
to properly call the app-level callback whenever the assertion is neededClientCertificateTest
andClientCredentialTest
to cover the new behavior