NimbusJwtEncoder should simplify constructing with javax.security Keys

[surajbh123]

No description provided.

[surajbh123]

Working on test cases and code refactoring

[surajbh123]

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 ...

[jzheaux]

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]

Working on reviews comments
Not all resolved yet

[jzheaux]

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

[jzheaux]

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.

[surajbh123]

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();

[jzheaux]

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]

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

[jzheaux]

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]

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;

[surajbh123]

done

[jzheaux]

Instead of re-constructing the generator, please move this to a member variable. Also, I feel this will be simpler if you use RSAKeyGenerator from Nimbus instead:

private final RSAKeyGenerator rsaGenerator = new RSAKeyGenerator(2048);

Then in this test you can do:

RSAKey jwk = this.rsaGenerator.generate();

and then use that instance to construct the builder. The other unit tests would benefit from this same polish.

[jzheaux]

Instead of re-constructing the generator, please move this to a member variable. Also, I feel this will be simpler if you use ECKeyGenerator from Nimbus instead:

private final ECKeyGenerator ecGnerator = new ECKeyGenerator(Curve.P_256);

Then in this test you can do:

ECKey jwk = this.ecGnerator.generate();

and then use that instance to construct the builder.

[jzheaux]

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