Validation of tokens doesn't consider OAuth2Config.tokenProvider.systemTime

[xuanswe]

How to reproduce the bug

1. Set the systemTime to a constant value to some years in the past

   MockOAuth2Server(
     OAuth2Config(
       tokenProvider = OAuth2TokenProvider(
         systemTime = Instant.parse("2020-10-10T00:00:00Z") // now is 2024
       )
     )
   )
   

2. Issue a token with 1 hour expiration
3. Validate the token
   • Expectation: token is valid
   • Actual: token is expired

Additional Information

I found Instant.now(), which is still used at 2 places.
I think it should be replaced by the value of getter OAuth2Config.tokenProvider.systemTime.

[xuanswe]

@ybelMekk could you have a look?
I am not sure how to inject the config to these places and where/how to write tests for this case.
If you could give me some guidance, I could implement and create PR.

[xuanswe]

I tried to change like this xuanswe@1b74252

fun SignedJWT.expiresIn(systemTime: Instant? = null): Int =
    Duration.between(systemTime ?: Instant.now(), this.jwtClaimsSet.expirationTime.toInstant()).seconds.toInt()

And provide the systemTime for all implementation of GrantHandler. These places look fine, I think.

Most of them are fine with a direct parameter passing.
But there is one long call chain below, that I don't feel good about it.
TokenExchangeGrantHandler.tokenResponse()
=> OAuth2HttpRequest.asTokenExchangeRequest(systemTime: Instant? = null)
=> OAuth2HttpRequest.ClientAuthentication.requirePrivateKeyJwt(..., systemTime: Instant? = null)
=> clientAssertion.expiresIn(systemTime)

I didn't try to do the same for the method toX509Certificate in SslKeystore, as I think it would be a mess if doing like that.

IMO, we should somehow have a way to allow access the OAuth2Config object directly, then we can avoid passing values everywhere. What do you think?

[ybelMekk]

I see, I had the same idea, looking over the issue. could we do it a bit hacky like this?

fun SignedJWT.expiresIn(): Int {
    return Duration.between(this.jwtClaimsSet.issueTime.toInstant(), this.jwtClaimsSet.expirationTime.toInstant()).seconds.toInt()
}

Or the idea about direct access, not sure, need to have another go to see if it breaks anything?

[xuanswe]

Yes, I like your idea, which is better than mine. The value should be derived from this.jwtClaimsSet itself. Nice!

[xuanswe]

How about SslKeystore?

[xuanswe]

Oh, wait. I don't think we can use this.jwtClaimsSet.issueTime.toInstant() in expiresIn. I understand that this must be the remain time from systemTime value.

[ybelMekk]

its should be the same as the systemTime, at the creation time in claimset?

[xuanswe]

https://datatracker.ietf.org/doc/html/rfc6749#section-5.1

expires_in
RECOMMENDED. The lifetime in seconds of the access token. For
example, the value "3600" denotes that the access token will
expire in one hour from the time the response was generated.
If omitted, the authorization server SHOULD provide the
expiration time via other means or document the default value.

"from the time the response was generated" not "from the time the token was generated"

[xuanswe]

Anyway, do you think the value of expires_in is used to validate the token expiration?
I have a feeling that it is just info in the response to the client resquest.
Could the validation logic be somewhere else?
Could you tell me the test that validates token expiration so I can run it and debug? Currently, I only see positive tests, not seeing a negative test for expired token yet.

[tommytroen]

I think you are talking past each other 😄

expiresIn should not be used for token validation, it is part of the token response and not the token itself. as a sidenote it can be manipulated so should never be used for validation, its most common purpose is for caching etc.

In your example the expectation is wrong: a token with 1 hour expiration will only be valid for 1 hour after it is issued, i.e. in 2020. So unless your validation code has changed its clock to the same as systemTime your token will always be expired. To get it to validate without changing the clock you have to give it an expiration of 4 years plus the time.

[xuanswe]

expiresIn should not be used for token validation, it is part of the token response and not the token itself. as a sidenote it can be manipulated so should never be used for validation, its most common purpose is for caching etc.

Thanks for your confirmation, @tommytroen!

So unless your validation code has changed its clock to the same as systemTime your token will always be expired.

Oh, do you mean that the mock server doesn't validate the expiration of the token, but the logic of the application does?

Assume I change my application security logic to use the mock systemTime (2020),
Let's say I have a token, which is still valid according to the mock systemTime (2020), but expired according the OS clock (2024).
If the mock server doesn't consider the configured systemTime (2020), what will happen if the application sends this token to mock server, such as to get user info, refresh, introspection or whatever that requires a valid token?

[ybelMekk]

@xuanswe, hope u had a good summer 🌞 is this still an issue?

[xuanswe]

Thanks @ybelMekk! Hope you enjoy your summer as well :).

Yes, I am still waiting for your opinion on how to continue.

Maybe we can just pass the configured systemTime everywhere for now if it works.
About the optimization to avoid variable passing, we could be in a separate ticket because it requires some research and blocking the actual issue?

What do you think? If you agree with this approach, I guess I can find time to work on it soon.

[tommytroen]

@xuanswe Sorry for the late reply, has been a long summer 😄
You are correct that the introspection and userinfo endpoints did not use the systemTime when validating tokens. The other endpoints should use the tokens as is, .i.e. not validating. I have created a PR which should solve your issue: #730

[xuanswe]

Sorry for the late reply, has been a long summer 😄

@tommytroen Hope you enjoyed your vacation :-)

I have created a PR which should solve your issue: #730

Amazing! I will try again and see if it can fix my test case. Thank you a lot!