One-way storage of refresh tokens is better supported if the refresh token is reused

[marcin-iwanow]

Expected Behavior It should be possible to store the refresh tokens hashed, also if the oauth2 client is configured to reuse the refresh tokens.

Current Behavior If the oauth2 client is reusing the refresh tokens, the refresh token returned in the refresh token flow is returned as stored in the DB. Consequently, if the tokens are stored hashed, subsequent attempts of executing the flow will be impossible.

In the code, this comes from the fact that the refresh token returned from OAuth2RefreshTokenAuthenticationProvider:authenticate is derived from the output value of the OAuth2AuthorizationService::findByToken method.

Example:

curl --location 'authorisation-server-url/oauth2/token' \
--header 'Content-Type: application/x-www-form-urlencoded' \
--data-urlencode 'grant_type=refresh_token' \
--data-urlencode 'client_id=123456' \
--data-urlencode 'refresh_token=3BjJn--6Bxdh9Zrq3TY1GA0EWW8ujSJbW7N4Fo27tZ37Mjxf3Z2Y0dXhftmJ4z0FeI-qQjzgARuteElqWXuqALTNtHYhYgAOMvPXcOqKS3Flv-JDqDOsTgTTx9yo3XEb'

would return (if we for the sake of the example use SHA-1 for hashing):

{
    "access_token": "eyJraWQiOiJmNjNkOTE1YS0zZj....",
    "refresh_token": "0d562d542fa74228766e0ddde3aff86bff233e5d"
    "scope": "scope1,...",
    "token_type": "Bearer",
    "expires_in": 900
}

Current workaround In the implementation of the OAuth2AuthorizationService::findByToken method, substitute the value of the token returned from the persistence layer with the argument, i.e., the plaintext token value.

[jgrandja]

@marcin-iwanow I'm not sure I see the value of hashing the returned refresh_token. Can you please provide more details on how you see this as a benefit ?

FYI, validation is enforced during the refresh_token grant flow, where the client originally granted is the only one allowed to use the refresh_token.

[marcin-iwanow]

Hi @jgrandja! That's my point actually: I would expect to have the refresh_token returned in plaintext, even if I store it in it's hashed form on the DB. At this point it is the case only when the refresh_token is not reused.

Let me know if that makes sense to you, I am still not sure I am clear enough :).

[jgrandja]

@marcin-iwanow

I would expect to have the refresh_token returned in plaintext

I'm still not following you. Have you reviewed the code for the OAuth2RefreshTokenGenerator?

https://github.com/spring-projects/spring-authorization-server/blob/76322dcfde601564d871e30c2d2bceb151fdbbd4/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/token/OAuth2RefreshTokenGenerator.java#L39

The generator produces an opaque refresh_token and is returned as-is. Can you clarify on your expectation re: plaintext?

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[marcin-iwanow]

tl;dr That's right, but when the refresh_token is reused (i.e., registeredClient.getTokenSettings().isReuseRefreshTokens() returns true), the refresh token generator is not called in the refresh_token grant flow, and instead the refresh_token is returned as stored on the DB.

Deep dive into the flow:

1. the authenticate method on OAuth2RefreshTokenAuthenticationProvider is called, the refreshToken within the authentication argument comes from the client (so, is plaintext).

2. the Oauth2Authorization is pulled from the DB using an implementation of the OAuth2AuthorizationService::findByToken method here . If the token is hashed on the DB, the implementation of findByToken will reflect that (i.e., query the DB by the hashed value of the token). Though, as hash is one-way it cannot return the plaintext of the refresh token (*subject to workarounds like reusing the argument, but that's not really clean).

3. currentRefreshToken is extracted from the Oauth2Authorization returned by findByToken in 2. and returned here upstream.

to sum up, if the token is reused (and only then!) we believe that the implementation does not support well one-way storage of tokens.

[jgrandja]

@marcin-iwanow I didn't get an answer to my question in the comment.

I don't see any added benefit of storing the refresh_token in hashed form. I'm also not seeing any issues that may be caused with the current implementation.

I'm curious if other providers have implemented hashing of refresh_token?

At this point, I'm going to close this based on the explanation provided.

[marcin-iwanow]

@jgrandja thanks for looking into it!

Sorry, I misunderstood the question. Well, persistent refresh_token is a very powerful secret so we (as well as other organisations that integrate SAS, actually) store it securely as recommended in RFC.