NimbusJwtEncoder should simplify constructing with javax.security Keys #17033

surajbh123 · 2025-05-02T20:49:55Z

No description provided.

surajbh123 · 2025-05-03T05:25:45Z

Working on test cases and code refactoring

surajbh123 · 2025-05-04T18:52:34Z

Helper Methods for Key Generation

        // 1. Using SecretKey with defaults
        SecretKey defaultSecretKey = createSecretKey("HmacSHA256", 256); // Key for default HS256
        NimbusJwtEncoder encoderWithSecretDefaults = NimbusJwtEncoder.withSecretKey(defaultSecretKey)
                // .macAlgorithm(MacAlgorithm.HS256) // Default is HS256
                // .keyId(null)                     // Default keyId is null
                // .jwkSelector(...)               // Default selector throws if multiple keys match
                .build();
                
        // 2. Using SecretKey with specific algorithm and keyId
        SecretKey hs512SecretKey = createSecretKey("HmacSHA512", 512);
        String secretKeyId = "my-hmac-key-01";
        NimbusJwtEncoder encoderWithSecretCustom = NimbusJwtEncoder.withSecretKey(hs512SecretKey)
                .macAlgorithm(MacAlgorithm.HS512) // Specify HS512
                .keyId(secretKeyId)               // Specify a key ID
                .build();

        // 3. Using RSA KeyPair with defaults
        KeyPair rsaKeyPair = createRsaKeyPair(2048);
        NimbusJwtEncoder encoderWithRsaDefaults = NimbusJwtEncoder.withKeyPair(rsaKeyPair)
                // .signatureAlgorithm(SignatureAlgorithm.RS256) // Default for RSA is RS256
                // .keyId(UUID generated)                      // Default keyId is a random UUID
                // .jwkSelector(...)                          // Default selector picks first if multiple
                .build();
                
        // 4. Using RSA KeyPair with specific algorithm (PSS) and keyId
        String rsaKeyId = "my-rsa-key-01";
        NimbusJwtEncoder encoderWithRsaCustom = NimbusJwtEncoder.withKeyPair(rsaKeyPair)
                .signatureAlgorithm(SignatureAlgorithm.PS256) // Specify PS256
                .keyId(rsaKeyId)                              // Specify a key ID
                .build();

        // 5. Using EC KeyPair with defaults
        KeyPair ecP256KeyPair = createEcKeyPair(Curve.P_256); // Key for default ES256
        NimbusJwtEncoder encoderWithEcDefaults = NimbusJwtEncoder.withKeyPair(ecP256KeyPair)
                // .signatureAlgorithm(SignatureAlgorithm.ES256) // Default for EC is ES256
                // .keyId(UUID generated)                      // Default keyId is a random UUID
                // .jwkSelector(...)                          // Default selector picks first if multiple
                .build();

        // 6. Using EC KeyPair with specific algorithm and keyId
        KeyPair ecP521KeyPair = createEcKeyPair(Curve.P_521); // Key for ES512
        String ecKeyId = "my-ec-key-01";
        NimbusJwtEncoder encoderWithEcCustom = NimbusJwtEncoder.withKeyPair(ecP521KeyPair)
                .signatureAlgorithm(SignatureAlgorithm.ES512) // Specify ES512
                .keyId(ecKeyId)                               // Specify a key ID
                .build();

jzheaux

Thanks, @surajbh123! This PR will make using NimbusJwtEncoder much simpler.

I've left my feedback inline. In addition to that, please ensure that your commit only has the signoff in the description and not in the title. Please also ensure that the description has the issue that this PR resolves:

Add NimbusJwtEncoder Builders

Closes gh-16267

Signed-off-by ...

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

jzheaux · 2025-05-07T21:50:24Z

Just one more thing, @surajbh123, when you submit, it will save you time in the review process if you run the following before submitting:

./gradlew format && ./gradlew :spring-security-oauth2-jose:check

surajbh123 · 2025-05-08T20:16:47Z

Working on reviews comments
Not all resolved yet

Closes spring-projectsgh-16267 Signed-off-by: Suraj Bhadrike <[email protected]>

jzheaux

Thanks for the quick turnaround, @surajbh123! This is coming together nicely. I've added some additional inline feedback.

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

jzheaux · 2025-05-09T18:30:23Z

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

+		 * with a {@link SecretKey}.
+		 */
+		public NimbusJwtEncoder build() {
+			this.jwsAlgorithm = (this.jwsAlgorithm != null) ? this.jwsAlgorithm : JWSAlgorithm.HS256;


Since you are giving this a default value and there isn't a way for the application to set it back to null, I believe you can remove this line.

In the following case, it may be null.

SecretKey defaultSecretKey = createSecretKey("HmacSHA256", 256); // Key for default HS256 NimbusJwtEncoder encoderWithSecretDefaults = NimbusJwtEncoder.withSecretKey(defaultSecretKey) // .macAlgorithm(MacAlgorithm.HS256) // Default is HS256 // .keyId(null) // Default keyId is null // .jwkSelector(...) // Default selector throws if multiple keys match .build();

Please take a look at my suggestions for the constructor so that this value need not be checked in this way.

By setting reasonable defaults in the constructor, readying the JWK is as simple as:

OctetSequenceKey jwk = this.builder.build()

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

jzheaux

Hi, @surajbh123, thanks for the updates and for your feedback.

Given some of the confusion I caused with some of my feedback, I wanted to take some more time with the PR before replying. You'll see that there are many more comments now with more specifics about what and why; hopefully that's clearer this time around.

To help with processing the comments, I also added a commit to my fork for you to review which applies my comments. I feel these changes make the builder classes easier to use and more maintainable by our team. You are welcome to review this commit to get a holistic view of the changes I'm proposing:

jzheaux@7e1847d

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

jzheaux · 2025-05-19T17:46:35Z

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

+		 * with a {@link SecretKey}.
+		 */
+		public NimbusJwtEncoder build() {
+			this.jwsAlgorithm = (this.jwsAlgorithm != null) ? this.jwsAlgorithm : JWSAlgorithm.HS256;


Please take a look at my suggestions for the constructor so that this value need not be checked in this way.

By setting reasonable defaults in the constructor, readying the JWK is as simple as:

OctetSequenceKey jwk = this.builder.build()

jzheaux · 2025-05-20T23:35:32Z

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

-		if (headers == null) {
-			headers = DEFAULT_JWS_HEADER;
-		}
+		headers = (headers != null) ? headers : (this.jwsHeader != null) ? this.jwsHeader : DEFAULT_JWS_HEADER;


Instead of checking for this.jwsHeader being non-null, please ensure that it is not null as part of the construction process. Then you can change this to:

headers = (headers != null) ? headers : this.jwsHeader;

We cant do it as it will break older flows which are not using builders
so this.jwsHeader will be null

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

...oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoderTests.java

jzheaux · 2025-05-21T00:06:17Z

...oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoderTests.java

+		KeyPair keyPair = keyPairGenerator.generateKeyPair();
+		NimbusJwtEncoder encoder = NimbusJwtEncoder.withKeyPair(keyPair)
+			.keyId(UUID.randomUUID().toString())
+			.signatureAlgorithm(SignatureAlgorithm.ES256)


Since EC defaults to ES256, I think this should be a different algorithm, given the test name.

Supported algorithms: oct [HS256]
Supported algorithms: EC [ES256]
Supported algorithms: RSA [RS256, RS384, RS512, PS256, PS384, PS512]

@jzheaux can we remove validation and let NimbusJwt do it

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

jzheaux

Thanks, @surajbh123, I've left a piece of feedback inline that I believe will address your other concerns about how the JWS header is handled. If you haven't already, please see my sample commit in the following places for additional illustration:

Remove setJwsHeader: jzheaux@7e1847d#diff-953f5c25f37accc56c99cbf372fde82b81359bdd56f85f8bf032226829c9cf36R68-L136
Ensure jwsHeader is non-null in constructor: jzheaux@7e1847d#diff-953f5c25f37accc56c99cbf372fde82b81359bdd56f85f8bf032226829c9cf36R135
Call new private JWK constructor: jzheaux@7e1847d#diff-953f5c25f37accc56c99cbf372fde82b81359bdd56f85f8bf032226829c9cf36R494

jzheaux · 2025-06-05T19:07:52Z

oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtEncoder.java

@@ -119,14 +133,17 @@ public void setJwkSelector(Converter<List<JWK>, JWK> jwkSelector) {
 		this.jwkSelector = jwkSelector;
 	}

+	public void setDefaultJwsHeader(JwsHeader defaultJwsHeader) {


Please remove this method. The default header can be derived in the constructor.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 2, 2025

surajbh123 force-pushed the nimbus-ecoder-builder branch 2 times, most recently from 6e3841d to abddfb3 Compare May 2, 2025 20:54

surajbh123 mentioned this pull request May 2, 2025

NimbusJwtEncoder should simplify constructing with javax.security Keys #16267

Open

surajbh123 force-pushed the nimbus-ecoder-builder branch 2 times, most recently from 78ef577 to 6370999 Compare May 4, 2025 18:43

jzheaux added this to the 7.0.x milestone May 6, 2025

jzheaux added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels May 6, 2025

jzheaux requested changes May 7, 2025

View reviewed changes

jzheaux self-assigned this May 7, 2025

surajbh123 force-pushed the nimbus-ecoder-builder branch from 6370999 to 567e729 Compare May 8, 2025 20:14

surajbh123 force-pushed the nimbus-ecoder-builder branch 3 times, most recently from bb6d143 to d6154cb Compare May 9, 2025 15:45

Add NimbusJwtEncoder Builders

52ef97d

Closes spring-projectsgh-16267 Signed-off-by: Suraj Bhadrike <[email protected]>

surajbh123 force-pushed the nimbus-ecoder-builder branch from d6154cb to 52ef97d Compare May 9, 2025 15:52

jzheaux requested changes May 9, 2025

View reviewed changes

surajbh123 force-pushed the nimbus-ecoder-builder branch 2 times, most recently from 2d1e2c6 to 24300cb Compare May 13, 2025 19:50

jzheaux requested changes May 21, 2025

View reviewed changes

surajbh123 force-pushed the nimbus-ecoder-builder branch from 24300cb to 263cf2f Compare May 24, 2025 11:55

jzheaux added the status: waiting-for-feedback We need additional information before we can continue label May 27, 2025

Merge github.com:surajbh123/spring-security into nimbus-ecoder-builder

27545c2

surajbh123 force-pushed the nimbus-ecoder-builder branch from 263cf2f to 27545c2 Compare May 29, 2025 16:44

jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-feedback We need additional information before we can continue labels May 29, 2025

surajbh123 force-pushed the nimbus-ecoder-builder branch from 769f4c4 to c6a6677 Compare June 5, 2025 15:21

Merge branch 'spring-projects:main' into nimbus-ecoder-builder

868539d

surajbh123 force-pushed the nimbus-ecoder-builder branch from c6a6677 to 868539d Compare June 5, 2025 15:26

Merge branch 'spring-projects:main' into nimbus-ecoder-builder

6e8bddf

jzheaux requested changes Jun 5, 2025

View reviewed changes

NimbusJwtEncoder should simplify constructing with javax.security Keys #17033

Are you sure you want to change the base?

NimbusJwtEncoder should simplify constructing with javax.security Keys #17033

Uh oh!

Conversation

surajbh123 commented May 2, 2025

Uh oh!

surajbh123 commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

surajbh123 commented May 4, 2025

Helper Methods for Key Generation

Uh oh!

jzheaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jzheaux commented May 7, 2025

Uh oh!

surajbh123 commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jzheaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jzheaux May 9, 2025

Choose a reason for hiding this comment

Uh oh!

surajbh123 May 13, 2025

Choose a reason for hiding this comment

Uh oh!

jzheaux May 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jzheaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jzheaux May 19, 2025

Choose a reason for hiding this comment

Uh oh!

jzheaux May 20, 2025

Choose a reason for hiding this comment

Uh oh!

surajbh123 Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jzheaux May 21, 2025

Choose a reason for hiding this comment

Uh oh!

surajbh123 Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jzheaux left a comment

Choose a reason for hiding this comment

Uh oh!

jzheaux Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

surajbh123 commented May 3, 2025 •

edited

Loading

surajbh123 commented May 8, 2025 •

edited

Loading

surajbh123 Jun 5, 2025 •

edited

Loading

jzheaux Jun 5, 2025 •

edited

Loading