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

feat(server-auth): Refactor useReauthenticate to prevent double currentUser calls #10531

Merged
merged 9 commits into from
May 2, 2024

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented May 1, 2024

…by calling getCurrentUser to determine if youre logged in.

We achieve this by swapping the order in which getUserMetadata and getCurrentUser is called in reauthenticate.

Previously getUserMetadata was a short-hand for checking if the auth SDK think its logged in, however in middleware auth this is an issue because you need to getCurrentUser before getting user metadata in the case of purely cookie based auth like dbAuth (and potentially future ones like firebase, etc.) - since the cookie/token cannot be read on the browser.

We've validated under these scenarios:
☑️ dbAuth legacy (no SSR)
☑️ dbAuth SSR
☑️ supabase legacy
☑️ supabase legacy with supabase-js client
☑️ supabase legacy with supbase/ssr browser client
☑️ supabase SSR

Tasks:

  • update failing tests for auth web clients
    🆗 Netlify mocks needed updating, because we check for the presence of access_token in reauthenticate now
    🆗 Supertokens mocks needed updating, because getAccessToken was not mocked

  • Update failing tests in AuthProvider.test.tsx
    🆗 All green, had to change how we're testing a little. We no longer try to fetch currentUser and error if we don't have a token, we simply remain "not logged in"

…ntUser calls, by calling getCurrentUser to determine if youre logged in
@dac09 dac09 added the release:feature This PR introduces a new feature label May 1, 2024
@dac09 dac09 added this to the SSR milestone May 1, 2024
@dac09 dac09 requested a review from dthyresson May 1, 2024 16:00
@dac09 dac09 self-assigned this May 1, 2024
dac09 added 2 commits May 2, 2024 16:46
…auth-reauthenticate-refactor

* 'main' of github.com:redwoodjs/redwood:
  chore(canary): Avoid `workspace:*` in published package.json files (#10532)
@dac09 dac09 marked this pull request as ready for review May 2, 2024 10:19
Copy link
Contributor

@dthyresson dthyresson left a comment

Choose a reason for hiding this comment

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

We paired on logic and tried on local Supabse, Approved!

@dthyresson dthyresson merged commit 3025afc into main May 2, 2024
46 checks passed
@dthyresson dthyresson deleted the feat/server-auth-reauthenticate-refactor branch May 2, 2024 19:19
dac09 added a commit to dac09/redwood that referenced this pull request May 3, 2024
…dwood into feat/supabase-middleware-client

* 'feat/supabase-middleware-client' of github.com:dac09/redwood:
  feat(RSC): Remove `entries.ts` file (redwoodjs#10533)
  testing to see if getToken is called
  Try to see if swapping getToken order mock passes Windows CI
  feat(server-auth): Refactor useReauthenticate to prevent double currentUser calls (redwoodjs#10531)
  chore(deps): update dependency rollup to v4.17.2 (redwoodjs#10346)
  fix(deps): update docusaurus monorepo to v3.2.1 (redwoodjs#10371)
@Josh-Walker-GM Josh-Walker-GM modified the milestones: SSR, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants