fix(auth): Implement automatic token refresh on supported providers#2277
fix(auth): Implement automatic token refresh on supported providers#2277
Conversation
| ) | ||
| } | ||
|
|
||
| if ( |
There was a problem hiding this comment.
@peterp we thought it best to maintain the same behaviour between dev and production for token validation. If there wasn't another reason you had it this way, are you happy with this change?
There was a problem hiding this comment.
@dac09 My only reservation here is that test will make a HTTP request to Auth0 as will dev and as such one cannot develop w/out an internet connection or offline.
Perhaps we revert.
There was a problem hiding this comment.
For test: yeah we can keep it there I guess. Would you expect jest to run an actual test calling this? I would imagine these decoders would be mocked (but I may be wrong).
But for dev, we should atleast do what we're doing on netlify, so the expirng tokens are a visible problem to the user, so they can fix.
There was a problem hiding this comment.
Ok I will leave as is then.
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Confirmed fixed on:
|
dthyresson
left a comment
dthyresson
left a comment
There was a problem hiding this comment.
@dac09 I think we should revert the auth0 decoder so that API calls to Auth0 are not made in test -- and likely not in dev.
| ) | ||
| } | ||
|
|
||
| if ( |
There was a problem hiding this comment.
@dac09 My only reservation here is that test will make a HTTP request to Auth0 as will dev and as such one cannot develop w/out an internet connection or offline.
Perhaps we revert.
|
Have updated auth0 setup to include the information needed to setup refresh tokens. Will also include this documentation in redwoodjs.com |
| // https://auth0.com/docs/libraries/auth0-spa-js#change-storage-options | ||
| cacheLocation: 'localstorage', | ||
| audience: process.env.AUTH0_AUDIENCE, | ||
|
|
There was a problem hiding this comment.
Could we make this a little less verbose DT? Large comment blocks like this can seem daunting right after generating. Perhaps just a the @MARK and the link to refresh token rotation doc
useRefreshTokens: true should also be commented out by default, as that's the default on Auth0 server side too
There was a problem hiding this comment.
I could put the info in the setup epilogue?
There was a problem hiding this comment.
I made it less verbose and include that info as the result of the setup command.
Info is also in the .com authentication docs.
dthyresson
left a comment
dthyresson
left a comment
There was a problem hiding this comment.
I made this comment less verbose and added the content to the setup command epilogue output.
The info is also a part of the authentications docs:
| // https://auth0.com/docs/libraries/auth0-spa-js#change-storage-options | ||
| cacheLocation: 'localstorage', | ||
| audience: process.env.AUTH0_AUDIENCE, | ||
|
|
There was a problem hiding this comment.
I made it less verbose and include that info as the result of the setup command.
Info is also in the .com authentication docs.
dac09
changed the title
…erve-web * 'main' of github.com:redwoodjs/redwood: (40 commits) Support generating typescript cells and pages | Autodetect ts project (redwoodjs#2279) create-redwood-app messages: moved app start commands to end (redwoodjs#2278) Add import type to configuration files (redwoodjs#2214) Bump @reach/skip-nav from 0.13.2 to 0.15.0 (redwoodjs#2237) Adding Setup Deploy Render Command (redwoodjs#2099) Disable role linting in Routes (redwoodjs#2318) v0.30.1 (redwoodjs#2322) teardown should attempt to delete dbName table (redwoodjs#2083) Restore @storybook/addon-a11y (redwoodjs#2309) fix(auth): Implement automatic token refresh on supported providers (redwoodjs#2277) fix(cli): move api-server dep from api to cli (redwoodjs#2307) Static typing for cells (redwoodjs#2208) Recommended Babel package upgrades (dependabot) (redwoodjs#2255) v0.30.0 (redwoodjs#2301) upgrade Prisma v2.21.0 (redwoodjs#2273) Further improvements to CONTRIBUTING.md (redwoodjs#2261) Adds better messages for rwt link | Watcher does not exist on build failure | Only remove node_modules after a succesful framework build (redwoodjs#2269) Update named param types in router readme (redwoodjs#2262) Bump core-js from 3.6.5 to 3.10.1 (redwoodjs#2243) Fix: webpack optimizations for JS (redwoodjs#2235) ...
2 participants
Closes #1636
What does it do?
Adds changes for two main cases:
a) Users are able to see errors in dev, if their access tokens expire
b) Changes auth clients to support automatically refreshing tokens (where available)
This PR was tested with the polling changes to auth playground: https://github.com/redwoodjs/playground-auth/
Todo:
Coauthored with @dthyresson
Please address all bugs, critical feedback to DT.