Skip to content

injecting clock when we are generating the JWT token #2004

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −17
Open

injecting clock when we are generating the JWT token #2004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/modules/ROOT/pages/core-model-components.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ The following example shows how to register an `OAuth2TokenGenerator` `@Bean`:
@Bean
public OAuth2TokenGenerator<?> tokenGenerator() {
JwtEncoder jwtEncoder = ...
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder, Clock.systemUTC());
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
return new DelegatingOAuth2TokenGenerator(
Expand Down Expand Up @@ -440,7 +440,7 @@ The following example shows how to implement an `OAuth2TokenCustomizer<OAuth2Tok
@Bean
public OAuth2TokenGenerator<?> tokenGenerator() {
JwtEncoder jwtEncoder = ...
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder, Clock.systemUTC());
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
accessTokenGenerator.setAccessTokenCustomizer(accessTokenCustomizer());
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
Expand Down Expand Up @@ -472,7 +472,7 @@ The following example shows how to implement an `OAuth2TokenCustomizer<JwtEncodi
@Bean
public OAuth2TokenGenerator<?> tokenGenerator() {
JwtEncoder jwtEncoder = ...
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder, Clock.systemUTC());
jwtGenerator.setJwtCustomizer(jwtCustomizer());
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
Expand Down
3 changes: 2 additions & 1 deletion docs/src/main/java/sample/extgrant/SecurityConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package sample.extgrant;

import java.time.Clock;
import java.util.UUID;

import com.nimbusds.jose.jwk.source.JWKSource;
Expand Down Expand Up @@ -99,7 +100,7 @@ OAuth2AuthorizationService authorizationService() {

@Bean
OAuth2TokenGenerator<?> tokenGenerator(JWKSource<SecurityContext> jwkSource) {
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource));
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource), Clock.systemUTC());
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
return new DelegatingOAuth2TokenGenerator(
Expand Down
3 changes: 2 additions & 1 deletion .../oauth2/server/authorization/config/annotation/web/configurers/OAuth2ConfigurerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers;

import java.time.Clock;
import java.util.Map;

import com.nimbusds.jose.jwk.source.JWKSource;
Expand Down Expand Up @@ -127,7 +128,7 @@ private static JwtGenerator getJwtGenerator(HttpSecurity httpSecurity) {
if (jwtGenerator == null) {
JwtEncoder jwtEncoder = getJwtEncoder(httpSecurity);
if (jwtEncoder != null) {
jwtGenerator = new JwtGenerator(jwtEncoder);
jwtGenerator = new JwtGenerator(jwtEncoder, Clock.systemUTC());
jwtGenerator.setJwtCustomizer(getJwtCustomizer(httpSecurity));
httpSecurity.setSharedObject(JwtGenerator.class, jwtGenerator);
}
Expand Down
8 changes: 6 additions & 2 deletions ...ain/java/org/springframework/security/oauth2/server/authorization/token/JwtGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.security.oauth2.server.authorization.token;

import java.time.Clock;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
Expand Down Expand Up @@ -61,16 +62,19 @@
public final class JwtGenerator implements OAuth2TokenGenerator<Jwt> {

private final JwtEncoder jwtEncoder;
private final Clock clock;

private OAuth2TokenCustomizer<JwtEncodingContext> jwtCustomizer;

/**
* Constructs a {@code JwtGenerator} using the provided parameters.
* @param jwtEncoder the jwt encoder
*/
public JwtGenerator(JwtEncoder jwtEncoder) {
public JwtGenerator(JwtEncoder jwtEncoder, Clock clock) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Alternatively, we could add an overloaded constructor with JwtEncoder, Clock, but we typically favour setters (e.g. setClock(Clock)) when the attribute is not required.

However, even with the setter approach, there are too many classes that require this public API change and my preference would be to minimize this.

Another possible solution would be to obtain the Clock from AuthorizationServerContext, which is available in OAuth2TokenContext. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @jgrandja for your comment.

Let me try to summarize to understand if I got your points:

  • adding a new param it's a breaking change and we are trying to avoid it
  • we can add a setter for the clock but this will be an open public method and we need to minimize them
    How much is problematic to add a new public method in terms of integrations?
    For me, this is the best option on the table, devs will decide if they would like to inject the Clock or not.
    I am not a big fun of setters by the way.
  • we can get the clock from one of the contexts
    Uhm... taking things from contexts seems everytime and overhead of resources and it can create side effects. Also, it can complicate tests and integrations..

Another option..
what about add the parameter Clock optional?

   // Constructor with optional Clock
    public JwtGenerator(JwtEncoder jwtEncoder) {
        this(jwtEncoder, Clock.systemUTC()); // Default to system UTC clock
    }

    // Constructor with explicit Clock
    public JwtGenerator(JwtEncoder jwtEncoder, Clock clock) {
        this.jwtEncoder = jwtEncoder;
        this.clock = clock;
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much is problematic to add a new public method in terms of integrations?

It's not problematic but configuration could be verbose as there are up to 6x OAuth2TokenGenerator that would need to be configured.

taking things from contexts seems everytime and overhead of resources and it can create side effects.

I'm not sure I understand. Why do you think it would be an overhead in resources and what side effects are you referring to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I will rephrase my sentence with more info.
For me, use context has different cons:
AuthorizationServerContext is request-scoped

  • The AuthorizationServerContext is tied to the current request lifecycle.
  • If you try to extract the Clock from this context outside of a token generation request (e.g. in background jobs, unit tests, or beans instantiated at application startup), the context won't exist, leading to IllegalStateException or null values.

It violates separation of concerns

  • The Clock is a core time abstraction often needed for testing, validation, and system logic.

  • Getting it from a request-scoped context couples your logic too tightly to the OAuth2 token issuance flow, making your code harder to test.

  • When using Clock from AuthorizationServerContext, you cannot easily inject mock clocks in unit or integration tests.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All very good points @AlessandroMinoccheri.

Ok, let's keep it simple and add setClock() to all OAuth2TokenGenerator.

Assert.notNull(jwtEncoder, "jwtEncoder cannot be null");
Assert.notNull(clock, "clock cannot be null");
this.jwtEncoder = jwtEncoder;
this.clock = clock;
}

@Nullable
Expand All @@ -94,7 +98,7 @@ public Jwt generate(OAuth2TokenContext context) {
}
RegisteredClient registeredClient = context.getRegisteredClient();

Instant issuedAt = Instant.now();
Instant issuedAt = clock.instant();
Instant expiresAt;
JwsAlgorithm jwsAlgorithm = SignatureAlgorithm.RS256;
if (OidcParameterNames.ID_TOKEN.equals(context.getTokenType().getValue())) {
Expand Down
3 changes: 2 additions & 1 deletion ...rver/authorization/authentication/OAuth2AuthorizationCodeAuthenticationProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.Principal;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
Expand Down Expand Up @@ -132,7 +133,7 @@ public void setUp() {
this.authorizationService = mock(OAuth2AuthorizationService.class);
this.jwtEncoder = mock(JwtEncoder.class);
this.jwtCustomizer = mock(OAuth2TokenCustomizer.class);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder, Clock.systemUTC());
jwtGenerator.setJwtCustomizer(this.jwtCustomizer);
this.accessTokenCustomizer = mock(OAuth2TokenCustomizer.class);
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
Expand Down
3 changes: 2 additions & 1 deletion ...rver/authorization/authentication/OAuth2ClientCredentialsAuthenticationProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.security.oauth2.server.authorization.authentication;

import java.time.Clock;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
Expand Down Expand Up @@ -104,7 +105,7 @@ public void setUp() {
this.authorizationService = mock(OAuth2AuthorizationService.class);
this.jwtEncoder = mock(JwtEncoder.class);
this.jwtCustomizer = mock(OAuth2TokenCustomizer.class);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder, Clock.systemUTC());
jwtGenerator.setJwtCustomizer(this.jwtCustomizer);
this.accessTokenCustomizer = mock(OAuth2TokenCustomizer.class);
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
Expand Down
3 changes: 2 additions & 1 deletion ...h2/server/authorization/authentication/OAuth2RefreshTokenAuthenticationProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.security.oauth2.server.authorization.authentication;

import java.security.Principal;
import java.time.Clock;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
Expand Down Expand Up @@ -119,7 +120,7 @@ public void setUp() {
this.jwtEncoder = mock(JwtEncoder.class);
given(this.jwtEncoder.encode(any())).willReturn(createJwt(Collections.singleton("scope1")));
this.jwtCustomizer = mock(OAuth2TokenCustomizer.class);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder, Clock.systemUTC());
jwtGenerator.setJwtCustomizer(this.jwtCustomizer);
this.accessTokenCustomizer = mock(OAuth2TokenCustomizer.class);
OAuth2AccessTokenGenerator accessTokenGenerator = new OAuth2AccessTokenGenerator();
Expand Down
5 changes: 3 additions & 2 deletions ...er/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.charset.StandardCharsets;
import java.security.Principal;
import java.text.MessageFormat;
import java.time.Clock;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Arrays;
Expand Down Expand Up @@ -1233,7 +1234,7 @@ JwtEncoder jwtEncoder() {

@Bean
OAuth2TokenGenerator<?> tokenGenerator() {
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder());
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder(), Clock.systemUTC());
jwtGenerator.setJwtCustomizer(jwtCustomizer());
OAuth2TokenGenerator<OAuth2RefreshToken> refreshTokenGenerator = new CustomRefreshTokenGenerator();
return new DelegatingOAuth2TokenGenerator(jwtGenerator, refreshTokenGenerator);
Expand Down Expand Up @@ -1295,7 +1296,7 @@ JwtEncoder jwtEncoder() {

@Bean
OAuth2TokenGenerator<?> tokenGenerator() {
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder());
JwtGenerator jwtGenerator = new JwtGenerator(jwtEncoder(), Clock.systemUTC());
jwtGenerator.setJwtCustomizer(jwtCustomizer());
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
OAuth2TokenGenerator<OAuth2Token> delegatingTokenGenerator = new DelegatingOAuth2TokenGenerator(
Expand Down
5 changes: 3 additions & 2 deletions ...ork/security/oauth2/server/authorization/config/annotation/web/configurers/OidcTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.security.Principal;
import java.time.Clock;
import java.util.Base64;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -719,7 +720,7 @@ SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) th

@Bean
OAuth2TokenGenerator<?> tokenGenerator() {
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource()));
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource()), Clock.systemUTC());
jwtGenerator.setJwtCustomizer(jwtCustomizer());
OAuth2RefreshTokenGenerator refreshTokenGenerator = new OAuth2RefreshTokenGenerator();
OAuth2TokenGenerator<OAuth2Token> delegatingTokenGenerator = new DelegatingOAuth2TokenGenerator(
Expand Down Expand Up @@ -760,7 +761,7 @@ SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) th

@Bean
OAuth2TokenGenerator<?> tokenGenerator() {
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource()));
JwtGenerator jwtGenerator = new JwtGenerator(new NimbusJwtEncoder(jwkSource()), Clock.systemUTC());
jwtGenerator.setJwtCustomizer(jwtCustomizer());
OAuth2TokenGenerator<OAuth2RefreshToken> refreshTokenGenerator = new CustomRefreshTokenGenerator();
return new DelegatingOAuth2TokenGenerator(jwtGenerator, refreshTokenGenerator);
Expand Down
3 changes: 2 additions & 1 deletion .../authorization/oidc/authentication/OidcClientRegistrationAuthenticationProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.security.oauth2.server.authorization.oidc.authentication;

import java.time.Clock;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -107,7 +108,7 @@ public void setUp() {
this.registeredClientRepository = mock(RegisteredClientRepository.class);
this.authorizationService = mock(OAuth2AuthorizationService.class);
this.jwtEncoder = mock(JwtEncoder.class);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder);
JwtGenerator jwtGenerator = new JwtGenerator(this.jwtEncoder, Clock.systemUTC());
this.tokenGenerator = spy(new OAuth2TokenGenerator<Jwt>() {
@Override
public Jwt generate(OAuth2TokenContext context) {
Expand Down
11 changes: 9 additions & 2 deletions ...ava/org/springframework/security/oauth2/server/authorization/token/JwtGeneratorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.security.oauth2.server.authorization.token;

import java.security.Principal;
import java.time.Clock;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Date;
Expand Down Expand Up @@ -82,7 +83,7 @@ public class JwtGeneratorTests {
public void setUp() {
this.jwtEncoder = mock(JwtEncoder.class);
this.jwtCustomizer = mock(OAuth2TokenCustomizer.class);
this.jwtGenerator = new JwtGenerator(this.jwtEncoder);
this.jwtGenerator = new JwtGenerator(this.jwtEncoder, Clock.systemUTC());
this.jwtGenerator.setJwtCustomizer(this.jwtCustomizer);
AuthorizationServerSettings authorizationServerSettings = AuthorizationServerSettings.builder()
.issuer("https://provider.com")
Expand All @@ -92,10 +93,16 @@ public void setUp() {

@Test
public void constructorWhenJwtEncoderNullThenThrowIllegalArgumentException() {
assertThatThrownBy(() -> new JwtGenerator(null)).isInstanceOf(IllegalArgumentException.class)
assertThatThrownBy(() -> new JwtGenerator(null, Clock.systemUTC())).isInstanceOf(IllegalArgumentException.class)
.hasMessage("jwtEncoder cannot be null");
}

@Test
public void constructorWhenClockNullThenThrowIllegalArgumentException() {
assertThatThrownBy(() -> new JwtGenerator(this.jwtEncoder, null)).isInstanceOf(IllegalArgumentException.class)
.hasMessage("clock cannot be null");
}

@Test
public void setJwtCustomizerWhenNullThenThrowIllegalArgumentException() {
assertThatThrownBy(() -> this.jwtGenerator.setJwtCustomizer(null)).isInstanceOf(IllegalArgumentException.class)
Expand Down