generated from cfpb/open-source-project-template
-
Notifications
You must be signed in to change notification settings - Fork 1
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(auth): update auth packages to prevent 403s #738
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is going to need to be paired with a react query change to update the calls when the token changes. |
meissadia
previously approved these changes
Jun 24, 2024
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.
No 403s during Bug Bash === WIN
Based on my testing, this fixes the remaining problems with 403s. [This PR](#738) updated the react-oidc-context library so that the iframe auth injection worked when `automaticSilentRenew` was enabled and let the token be updated in between page views without any re-renders. Now we've got another problem that this PR fixes: react query. When the auth token gets refreshed, any query will continue to use the old auth token unless we add it to the `queryFn`. That totally works, but will cause a re-render unless we pepper the codebase with `useMemo`s and `useCallback`s, that I don't want to do until we upgrade to React 19 post-mvp and React Compiler [does it for us](https://react.dev/learn/react-compiler). So where does that leave us? The internet generally recommends either biting the bullet with the re-renders, or retrying with the correct authentication token and then persisting that token for future calls in either react query or an axios interceptor, or just use the axios interceptor to do the authentication entirely. I liked the 3rd option because having to retry on errors you know are coming makes me sad (like the 50x errors we have), and I think this is the cleaner method without relying on tricky state management and react query wrangling to avoid re-renders. I've [made a ticket](#752) to potentially strip auth from our queries if this PR gets approved and works. So this axios interceptor, in conjunction with the fact that iframe auth injection works and writes to session storage, uses the session storage to add the auth token to the api calls. Session storage is fast and lets the function live outside the AuthContext and be applied to any call. This means that the api calls are always using the latest auth token regardless of their react query set up, and does so without triggering re-renders. ## Future PRS - [remove auth from existing queries if this PR works](#752) - [add a catch all auth error on 403s/401s to redirect to the unauthenticated home page (if a user closes their laptop lid for 30 mins for example)](#753) ## Changes - modify `getAxiosInstance` to include an interceptor that adds auth tokens from session storage to each call - update `fetchFilingSubmissionLatest.ts` to use the modified `getAxiosInstance` function instead of creating its own so that the interceptor gets added - create a getOidcTokenOutsideContext.ts utility function that pulls the oidc token from session storage ## How to test this PR This PR depends on: #750 #738 You can test this PR locally by pointing your local env to the preview env or by testing it on the preview environment itself (check with Bill to make sure it's up on preview still) 1. Navigate to the [regtech realm settings for tokens](http://localhost:8880/admin/master/console/#/regtech/realm-settings/tokens). Change the Access Token Lifespan to 65 seconds. (oidc-client-ts [tries to renew a token one minute](https://github.com/authts/oidc-client-ts/blob/0d9dd56/src/UserManagerSettings.ts#L52C117-L52C209) before the expiration time, so this will make the frontend seek a new token every five seconds) ![Screenshot 2024-06-19 at 1 51 42 PM](https://github.com/cfpb/sbl-frontend/assets/19983248/c339cb1c-bc20-4ccc-9903-e5ecf84b318b) 3. Go to the upload page and try to upload a large file with your network tab open. 4.Notice that after each time the token is refreshed in the network tab, that the `/latest` API call's authentication token is also updated to reflect that change. (see Screenshots) 5. Notice that there are maybe no more 403s? 🤞 ## Screenshots ![before-refresh](https://github.com/cfpb/sbl-frontend/assets/19983248/9cce2586-8416-4d60-a7b5-437f44ede20e) ![token-refreshed](https://github.com/cfpb/sbl-frontend/assets/19983248/edf20e56-dceb-4173-b224-89246032657c) ![after-refresh](https://github.com/cfpb/sbl-frontend/assets/19983248/7c040f95-350f-4863-912f-87955b2dca49)
meissadia
approved these changes
Jun 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We've been experiencing flaky auth issues during the bug bashes: the
automaticSilentRenew
feature hasn't been working as advertised: it's supposed to attempt an automatic refresh using an iframe to not force a re-render, but that wasn't happening.I tried just calling the silent refresh directly so I wouldn't have to update these libraries this close to launch, but that was also kind of a bust. So here we are: I updated the packages and now it works great?
I've updated the testing instructions so it's easy to see if it's working or not. I'll also push this to the preview environment, so it's easier to see if it's actually working.
Changes
oidc-client-ts
andreact-oidc-context
librariesHow to test this PR
Navigate to the regtech realm settings for tokens. Change the Access Token Lifespan to 65 seconds. (oidc-client-ts tries to renew a token one minute before the expiration time, so this will make the frontend seek a new token every five seconds)
![Screenshot 2024-06-19 at 1 51 42 PM](https://private-user-images.githubusercontent.com/19983248/341366176-c339cb1c-bc20-4ccc-9903-e5ecf84b318b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg4NzMxNzMsIm5iZiI6MTczODg3Mjg3MywicGF0aCI6Ii8xOTk4MzI0OC8zNDEzNjYxNzYtYzMzOWNiMWMtYmMyMC00Y2NjLTk5MDMtZTVlY2Y4NGIzMThiLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA2VDIwMTQzM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTg3NDNjMzljMDRjZmI3NmIzZDRkMTJkMjE3Nzk3NzQyNTJjMjBmNWJmYjhiYTcxMDBiMWRkZDcyNDk0ZTFjMmYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.PZAt5Gi0jXbxettUEvfgukPqx5bhokbUDLidgPU7JSI)
Go to the upload page and try to upload a large file with your network tab open.
Notice that the upload page does not refresh or re-render, but notice in the network tab successfully updates the token every 5 seconds.
You can also test that nothing is re-rendering by going to point of contact, making a change to the form, and seeing that your changed content doesn't change when the token is renewed.
Screenshots