Skip to content
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 4 commits into from
Jun 24, 2024

Conversation

billhimmelsbach
Copy link
Contributor

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

  • updated the oidc-client-ts and react-oidc-context libraries

How to test this PR

  1. 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

  2. Go to the upload page and try to upload a large file with your network tab open.

  3. Notice that the upload page does not refresh or re-render, but notice in the network tab successfully updates the token every 5 seconds.

  4. 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

Screenshot 2024-06-19 at 1 55 59 PM

@billhimmelsbach
Copy link
Contributor Author

This is going to need to be paired with a react query change to update the calls when the token changes.

meissadia
meissadia previously approved these changes Jun 24, 2024
Copy link
Contributor

@meissadia meissadia left a 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants