Skip to content
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.

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

Rolling refresh token expiry introduced a completely new "iat" behavior on token refresh #268

Open
robinnydahl opened this issue Oct 30, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@robinnydahl
Copy link

Subject of the issue

Release 2.5.0 (see #256) changed the behavior of the iat (issued at) timestamp during token refresh. Previously, the iat value remained unchanged across refreshes, creating a fixed refresh window from the original token’s creation. However, the new behavior issues a fresh iat with every refresh, resulting in a rolling refresh token expiry.

The description in the config do not reflect this change and this change also causes users of the package that expected the old behavior to have possibility of indefinite sessions which was not possible in the previous version when refresh_ttl was set.

The config states the following today:

    /*
    |--------------------------------------------------------------------------
    | Refresh time to live
    |--------------------------------------------------------------------------
    |
    | Specify the length of time (in minutes) that the token can be refreshed
    | within. I.E. The user can refresh their token within a 2 week window of
    | the original token being created until they must re-authenticate.
    | Defaults to 2 weeks.
    |
    | You can also set this to null, to yield an infinite refresh time.
    | Some may want this instead of never expiring tokens for e.g. a mobile app.
    | This is not particularly recommended, so make sure you have appropriate
    | systems in place to revoke the token if necessary.
    |
    */

    'refresh_ttl' => env('JWT_REFRESH_TTL', 20160),

Impact

This change modifies how refresh token expiry is calculated, as each refresh now resets the refresh window based on the most recent refresh time. This can lead to unexpected behavior for users expecting a fixed refresh window.

Suggested Solution

To support both behaviors, we suggest to introduce an optional config parameter:

'refresh_iat' => env('JWT_REFRESH_IAT', true),

Setting refresh_iat to false reverts to the previous behavior, maintaining a fixed refresh window from the original token creation. However, this change is not yet reflected in the configuration comments, which may cause confusion.

Your environment:

Q A
Bug? yes & no
New Feature? yes
Framework Laravel
Framework version 11.x
Package version 2.7.2
PHP version 8.3
@robinnydahl
Copy link
Author

I have opened up a pull request with a suggested solution/fix to this issue, #269.

The default behavior has been set to how the package works today, so it will not break existing users on new releases from 2.5.0. The 2.5.0 release did however technically break intended behavior based on the documentation in the config.php.

@robinnydahl robinnydahl changed the title Rolling refresh token expiry introduced with completely new "iat" behavior on token refresh Rolling refresh token expiry introduced a completely new "iat" behavior on token refresh Oct 30, 2024
@mfn
Copy link
Contributor

mfn commented Nov 12, 2024

https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.6 says

The "iat" (issued at) claim identifies the time at which the JWT was
issued. This claim can be used to determine the age of the JWT. Its
value MUST be a number containing a NumericDate value. Use of this
claim is OPTIONAL.

When using the refresh flow, a copied over iat would lead to a wrong validation in \PHPOpenSourceSaver\JWTAuth\Claims\IssuedAt::validateRefresh().

Refreshing essentially creates a new token: why would we carry over the iat? What's the purpose of, let me put it in striking words, "lying" about the tokens issued at?

You describe the problem with the change of the bevaiour, but can you also describe your use-case here?

@robinnydahl
Copy link
Author

Hi @mfn,

Thanks for getting back to me! Sorry for the delayed response, I just got back from a vacation.

When using the refresh flow, a copied over iat would lead to a wrong validation in \PHPOpenSourceSaver\JWTAuth\Claims\IssuedAt::validateRefresh().

We managed to run all the rests successfully, might not be that there is a test case covering this? I'll let us sort out the other question below before we tackle that.

Refreshing essentially creates a new token: why would we carry over the iat? What's the purpose of, let me put it in striking words, "lying" about the tokens issued at?

You describe the problem with the change of the bevaiour, but can you also describe your use-case here?

The use-case; A user authenticates and checks "remember me". We should then allow the session to be active for a specified time-frame (for example 30 days) before the user is forced out and need to do the auth flow again. This way we will never get infinite activate sessions that can be misused or high-jacked without ever getting an expiry.

We do not wish to issue an access token that is usable for 30 days without making any refresh, we want short lived tokens. As this package uses a concept of JWT where the JWT (active and expired) can used to issue a new token, rather than a specific refresh token, the old implementation made a lot of sense as it combined the JWT and Refresh Token into one.

I understand this change might be helpful/needed for other use-cases, but it also causes issues based on the original intention and the documentation was not updated to reflect this or explain the reason for the change. The attached PR should allow for the old behavior without causing issues (we'll of course need to check the \PHPOpenSourceSaver\JWTAuth\Claims\IssuedAt::validateRefresh() you mentioned).

Does this clear up your question on the use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants