-
Notifications
You must be signed in to change notification settings - Fork 3
Artyom / Refresh Token Bugs and a Login Redirect #162
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
base: main
Are you sure you want to change the base?
Conversation
|
oh yeah, the branch name was renamed so its a new PR, but the changes are mostly the same as last PR except also added a Login Redirect |
|
|
||
| async generateSignInLink(email: string): Promise<string> { | ||
| const actionCodeSettings = { | ||
| // Why this localhost lmao |
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.
Yeah we will replace this in production. It is localhost for now because firebase requires a valid url for email link sign in
| }; | ||
|
|
||
| // Checks if the access token has expired or not | ||
| export const validateAccessToken = (decodedToken: DecodedJWT): boolean => { |
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.
Is the logic backwards?
Notion ticket link
Ticket Name
Implementation description
Steps to test
You can test for both fixed bugs with one method
You can force a refresh on every axios request
AuthUtils.tson the frontend and make thevalidateAccessTokenalways return false (simulating the access token being expired)authService.tson the backend and make therenewToken()function always through an error, this will simulate the refresh token being expired/profileand you will be redirect to the/login, refreshing the page also redirects you to the login, so basically doing anything at all will redirect youWhat should reviewers focus on?
BaseAPIClient.ts, if we have more complicated permissions then it might need tweakingChecklist