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

Configure ApolloProvider to always use fresh tokens #1609

Merged
merged 6 commits into from
Jan 7, 2021

Conversation

dac09
Copy link
Collaborator

@dac09 dac09 commented Jan 7, 2021

  • AuthContext.authToken to be deparacated

Starts fix for #1576. Implements proposal and discussion in #1608

  • Use fresh token for getCurrentUser
  • Use fresh token for other graphql requests
  • Verify and test this

Verified only with firebase, but individual provider changes will be added later.

@dac09 dac09 requested a review from peterp January 7, 2021 14:00
@github-actions
Copy link

github-actions bot commented Jan 7, 2021

@@ -20,12 +23,33 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{
config?: Omit<ApolloClientOptions<InMemoryCache>, 'cache'>
}> = ({ config = {}, children }) => {
const { uri, headers } = useFetchConfig()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove headers here since this is where the auth-token and type came from previously.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing so would save you an extra re-render.

Copy link
Collaborator Author

@dac09 dac09 Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I intentionally didn't do that, wanted to remove this if/when FetchContext is removed if it makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that make sense.

Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com>
@dthyresson
Copy link
Contributor

I like that the getFreshToken in the earlier version is now just using the getToken.

LGTM 🚀

@thedavidprice thedavidprice added this to the Next Release milestone Jan 7, 2021
@thedavidprice thedavidprice added the release:breaking This PR is a breaking change label Jan 7, 2021
@thedavidprice thedavidprice merged commit ef14ece into redwoodjs:main Jan 7, 2021
Auth automation moved this from In progress to Done Jan 7, 2021
thedavidprice added a commit that referenced this pull request Jan 7, 2021
@thedavidprice thedavidprice removed this from the Next Release milestone Jan 7, 2021
@thedavidprice
Copy link
Contributor

REVERTED

See #1611

dac09 added a commit to dac09/redwood that referenced this pull request Jan 12, 2021
…h-tokens

* 'main' of github.com:redwoodjs/redwood:
  Move whatwg-fetch from devDep to dep
  Adds mockCurrentUser() to api-side jest
  v0.22.1
  v0.22.0
  Revert "Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)" (redwoodjs#1611)
  Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)
  Ignore *.scenarios.* files. (redwoodjs#1607)
  upgrade prisma v2.12.1 (redwoodjs#1604)
  Test Scenarios (redwoodjs#1465)
  Use relative path to config stories location (redwoodjs#1509)
@thedavidprice thedavidprice added this to the unassigned-version milestone May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change topic/auth
Projects
No open projects
Auth
  
Done
Development

Successfully merging this pull request may close these issues.

Unable to refresh Auth token once RedwoodProvider is mounted. refresh auth tokens when they're invalidated
4 participants