-
Notifications
You must be signed in to change notification settings - Fork 3
chore: change leeway and reject tokens expiring and in less than #775
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
chore: change leeway and reject tokens expiring and in less than #775
Conversation
|
Rejection might be right to remove. But I think we should keep leeway at zero. |
|
why do you think we should keep leeway at zero? |
|
ok i think we should actually have both, leeway to account for potential clock skew between fusion auth and our own auth, and then the reject early to prevent downstream 401s that initially pass. there's a separate issue in the front end though about ensuring that our refresh propagates correctly. useUserInfo for instance will 401 due to a jwt expiry and then never refresh its false state which is why we see the banner in the Dock component. if we can a) maximize a token's lifetime but also b) ensure that the occasional rejection flows through to the auth state in the front end we'll be able to fix this |
| let mut validation = Validation::new(Algorithm::HS256); | ||
|
|
||
| validation.leeway = 0; | ||
| validation.leeway = 30; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the default so you could just remove thisa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whutchinson98
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead lets have leeway set to something reasonable like 5 to account for any clock skew and then remove the reject_tokens_expiring_in_less_than all together.
|
reverting to default because we already have things hardcoded elsewhere to account for 60s leeway, e.g. packages/core/signal/token.ts. don't want to sneakily break things between this and macro-api too so both will just use default for now |
Summary
Screenshots, GIFs, and Videos