-
Notifications
You must be signed in to change notification settings - Fork 30
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
fix(lib-classifier): refresh expired bearer tokens #6604
base: master
Are you sure you want to change the base?
Conversation
d1fac3b
to
9000e48
Compare
56e4739
to
4c9d54b
Compare
4c9d54b
to
a726c16
Compare
useEffect(function onUserChange() { | ||
checkAuth() | ||
}, [authClient, userID]) |
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.
The original bug happens because this hook stores your bearer token in component state and reuses it forever, even after it has expired. The token should be checked (by running checkAuth()
) every time that it's used.
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.
Tip
General tip for reviewing React components: whenever you see useEffect
it has probably introduced a subtle bug.
a726c16
to
d296240
Compare
d296240
to
10af1e4
Compare
Fix `usePanoptesAuth` hook to refresh expired bearer tokens in the classifier. Fixes a bug where Quick Talk stops working once your bearer token expires.
10af1e4
to
377b80a
Compare
Fix
usePanoptesAuth
hook to refresh expired bearer tokens in the classifier. Fixes a bug where Quick Talk stops working once your bearer token expires.Please request review from
@zooniverse/frontend
team or an individual member of that team.Package
Linked Issue and/or Talk Post
How to Review
The original issue (#6594) happens if you leave the classifier open long enough for your bearer token to expire, so you might need to open the classifier in a tab and leave it for a couple of hours. With the changes here, an expired token should always be refreshed, as long as you don't allow your Panoptes refresh token to expire. I think refresh tokens are good for up to 2 weeks.
Checklist
PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.
General
yarn panic && yarn bootstrap
ordocker-compose up --build
and FEM works as expectedGeneral UX
Example Staging Project: i-fancy-cats
Bug Fix