injecting clock when we are generating the JWT token

[AlessandroMinoccheri]

In this PR I am trying to solve this issue: #1950
By injecting clock into the token generation

[jgrandja]

@AlessandroMinoccheri Apologies for the late response but I've been busy preparing for the 1.5 release this Tuesday. I will likely review this next week.

[jgrandja]

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?

[AlessandroMinoccheri]

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;
    }

[jgrandja]

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?

[AlessandroMinoccheri]

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.

[jgrandja]

All very good points @AlessandroMinoccheri.

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