Refresh token allowed as Access Token

[rafollett]

Passing the refresh token instead of the access token when using JWT seems to be accepted. I added an "else" case to the if(expiry) check in RestAuthenticationProvider to throw an exception in this case, since this seems to be a security vulnerability (since refresh token does not expire). If my understanding of JWT is incorrect here, please let me know, but thought this would be an improvement to the otherwise great plugin.

[vishnuaggarwal23]

Hi team,
Is there any update on this. Since, what is as per my understanding too, the refresh-token should not be able to be used as an access-token and they should have an expiry associated with them or, there should be mechanism provided to revoke the refresh-token if at all it is needed.

[longwa]

Adding an expiration to the refresh token would break the current plugin as the code specifically uses:

boolean isRefreshToken = jwt.JWTClaimsSet.expirationTime == null

to check if it is doing a refresh or not.

I agree, it seems like those are two separate concerns. The refresh token should probably just have a claim that indicates specifically it is for refresh. Or the loadbyUserName could just take an optional flag indicating it's a refresh operation.

[longwa]

I tested this as well locally and can confirm that you can replace the access_token with the refresh_token and use it to authenticate successfully.

The best solution might be to have the AbstractJwtTokenGenerator.generateRefreshToken method add a claim to the refresh_token that it generates indicating that it is a refresh token.

That would allow the refresh token to have a configurable expiration date (if desired) and still be identified as a refresh_token via the custom claim.

I can fix this and submit a PR

[alvarosanchez]

Adding a custom claim sounds good to me

[6footgeek]

@alvarosanchez Any update on this? I see you merged a fix but it never passed the build :)