Remove usage of com.nimbusds.oauth2 from grant-related classes #926

Avery-Dunn · 2025-03-27T00:17:47Z

This PR removes the com.nimbusds.oauth2 dependency from various classes that deal with different grant types, as per #909

The dependency was mainly used by these classes to create maps of different parameters needed for each OAuth grant type, so the changes are mostly the same for each file:

Remove com.nimbusds.oauth2.sdk.AuthorizationGrant and various similar imports
Replace usages of Nimbus's AuthorizationGrant subclasses with our existing OAuthAuthorizationGrant class
Replace Nimbus's AuthorizationGrant.toParameters() method with our own Map<String, List<String>> creation
Add a new GrantConstants file for standardized OAuth parameter names and values
Refactor older code and remove unused code

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OAuthAuthorizationGrant.java

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByAuthorizationGrantSupplier.java

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfRequest.java

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/RefreshTokenRequest.java

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OAuthAuthorizationGrant.java

bgavrilMS

See comments marked with issue

…tion-library-for-java into avdunn/nimbus-grants # Conflicts: # msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfRequest.java

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OAuthAuthorizationGrant.java

billwert · 2025-04-02T19:41:34Z

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByAuthorizationGrantSupplier.java

+            params.putAll(authGrant.toParameters());

-            authGrant = new OAuthAuthorizationGrant(updatedGrant, authGrant.getParameters());
+            authGrant = new OAuthAuthorizationGrant(params, null, null);


Maybe some constructor overloads if null, null is a commonish case.

This spot and one other in a test were the only places that used a double null, however another also had a null for the third parameter (claims) but not the second (scopes).

In the latest commit I added a constructor that omits the claims parameter, as that's the one that's most likely to be null in a flow.

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByAuthorizationGrantSupplier.java

billwert

High level looks good. Two themes:

Replace Map<String, List<String>> as the representation of params with some type with good helpers.
OAuthAuthorizationGrant seems immutable, so it doesn't need to recreate the params and add claims etc every time. Just do it at construction.

bgavrilMS · 2025-04-04T16:29:21Z

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeRequest.java

-        } else {
-            authorizationGrant = new AuthorizationCodeGrant(
-                    new AuthorizationCode(parameters.authorizationCode()), parameters.redirectUri());
+            params.put("code_verifier", Collections.singletonList(parameters.codeVerifier()));


I am not sure how much of PKCE was handled by nimbus, so please make sure we have enough tests that validate that auth_code uses PKCE. Both desktop and web app scenarios.

…tion-library-for-java into avdunn/nimbus-grants

changelog.txt

…crosoft-authentication-library-for-java into avdunn/nimbus-grants

Ensure Base64URL decoding is used

Update version and changelog for release 1.20.1

Address codeql alerts

…d scenarios

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByAuthorizationGrantSupplier.java

bgavrilMS · 2025-04-25T10:42:19Z

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByAuthorizationGrantSupplier.java


-        if (msalRequest.application().authenticationAuthority.authorityType != AuthorityType.AAD) {
-            return authGrant;
+        //Additional processing is only needed if it's a password grant with a non-AAD authority


User Real discovery needs to be done only for AAD authority. For non-AAD authority, perform the basic grant only.

I think you got the condition correct, but the comment wrong?

Yep, the if statement correctly exits the method if it's not-AAD, however when writing the comment I just said "non-AAD" because of the != AuthorityType.AAD part

Comment fixed in the latest commit

…tion-library-for-java into avdunn/nimbus-grants # Conflicts: # msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java

Remove usage of com.nimbusds.oauth2 from grant-related classes

29add9f

Avery-Dunn requested a review from a team as a code owner March 27, 2025 00:17

Avery-Dunn changed the base branch from dev to avdunn/nimbus-removal March 27, 2025 00:19