Remove usage of com.nimbusds.oauth2's HTTP classes #927
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
Avery-Dunn
changed the title
billwert
reviewed
Apr 1, 2025
| @@ -29,7 +27,6 @@ public abstract class AbstractClientApplicationBase extends AbstractApplicationB | |||
| private String applicationName; | |||
| private String applicationVersion; | |||
| private AadInstanceDiscoveryResponse aadAadInstanceDiscoveryResponse; | |||
| protected abstract ClientAuthentication clientAuthentication(); | |||
There was a problem hiding this comment.
Long story short, this was mainly used as a parameter or return type of various nimbus methods. It's utility behavior was fully delegated to other classes:
- It represented a client assertion/secret/cert, which some existing MSAL classes did as well
- It was used to add query parameters to an HTTP request, but
TokenRequestExecutor.addJWTBearerAssertionParams()now handles that - It was passed into some nimbus APIs that are no longer used
billwert
reviewed
Apr 1, 2025
|
|
||
| final String location = HttpUtils.headerValue(httpResponse.headers(), "Location"); | ||
| if (!StringHelper.isBlank(location)) { | ||
| try { | ||
| response.setLocation(new URI(location)); | ||
| response.addHeader("Location", new URI(location).toString()); |
There was a problem hiding this comment.
Maybe worth it to have an enum/constants for well known headers.
There was a problem hiding this comment.
I was planning to do this in a follow up PR that removes various nimbus utility methods, since we'll then have the full picture of what constants and enums would be useful
billwert
approved these changes
Apr 1, 2025
bgavrilMS
approved these changes
Apr 2, 2025
Remove usage of com.nimbusds.oauth2's Token classes
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.
This PR removes usage of several com.nimbusds.oauth2 classes, as per #909
Mainly HTTPRequest, HTTPResponse, and ClientAuthentication were removed, along with some related minor utils. These were essentially just used as a helper for making HTTP requests: managing query parameters, creating assertions for different credentials, etc.
To replace them, this PR does the following:
ConfidentialClientApplicationrelated to credentials and assertions, it now mainly deals with assertion/secret Strings, some logic was moved to other classes, and some methods that now had duplicate or unused codeTokenRequestExecutorrelated to credentials and assertions, simplifying and clarifying the behavior around query parameters and assertions/secrets that are needed in each scenario