Make JWT token refresh optional #6433
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements a config option
disable_refresh_token_renewal, that doesn't do refresh token renewal unless a full auth is performed.Goal:
Only renew the JWT token that allows PIN unlock, bypassing 2FA, etc. if a "full authentication" is performed. This makes things like "require 2FA once every 30/90 days" possible
Rationale:
Currently it's only possible to require periodic reauthentication by setting clients to log-out instead of lock. This is inconvenient since it breaks PIN, offline functionality and requires full reauthentication for every unlock. The current lock behavior on the other hand only requires reauth if a client hasn't connected in 30/90 days, since the token is renewed even on a reduced login that used that same token. This means it effectively never expires, if the client just uses it's existing token more than once a month.
Logic:
If the new option is
falsethe old logic is used (always renew the token). If a normal Password/SSO login is performed the old logic is used.If the option is
truethe JWT isn't renewed in the_refresh_logincode path, leaving it to expire and force a full auth after 30/90 daysCode:
Having
refresh_claims.sub.clone()andSome(&refresh_claims)in the same call isn't amazing, but seems like the simplest way to implement this logic without a refactor to overloadAuthToken::newinto 2 separate, yet almost identical functions.Progress:
Compiled and running, but not yet tested to actually behave as expected (currently ongoing).
Feedback welcome