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

feature: Upgrade Supabase Auth to v2 #7719

Merged
merged 20 commits into from
Mar 4, 2023

Conversation

dthyresson
Copy link
Contributor

WIP draft to show @Tobbe to discuss how we might expose the various Supabase sign in methods.

@dthyresson dthyresson requested a review from Tobbe March 1, 2023 16:09
@dthyresson dthyresson marked this pull request as draft March 1, 2023 16:09
@dthyresson
Copy link
Contributor Author

@Tobbe I reworked login() to check and see what credentials were provided and then invoke the expected login method.

Still won't work for verifyOTP.

I assume the developer can simple extract the client fro useAuth and perform verifyOTP as needed.

@dthyresson
Copy link
Contributor Author

I have an incorrect implementation in restoreAuthState as cannot re-init a client as I had attempted to replace the getSessionFromUrl from v1.

@jtoar jtoar added the release:breaking This PR is a breaking change label Mar 1, 2023
@dthyresson
Copy link
Contributor Author

Got to green.

 PASS  src/__tests__/supabase.test.tsx
  Supabase
    ✓ is not authenticated before logging in (15 ms)
    ✓ is authenticated after logging in (2 ms)
    ✓ is not authenticated after logging out (2 ms)
    ✓ has role "user" (2 ms)
    ✓ has role "admin" (2 ms)
    ✓ can specify custom hasRole function (2 ms)
    ✓ can specify custom getCurrentUser function (2 ms)

@dthyresson dthyresson self-assigned this Mar 1, 2023
@Tobbe
Copy link
Member

Tobbe commented Mar 1, 2023

@dthyresson I introduced an authType parameter. It makes the code much simpler. But it does force the user to provide that parameter when logging in. Which is bad because it's extra code that isn't strictly needed. But it's also good because it makes it more explicit exactly how they're expecting to log in.

In the future the extra authType parameter will also help TS auto-suggest the correct parameters. Like if you do authType: 'Password' it would suggest (exactly) username and password for the rest. Anything more or less and the user would get a type error. Unfortunately the TS support in Auth isn't quite that sophisticated yet. So for now that's just a theoretical bonus.

It's perfectly fine if you don't like it and rather go back to what you had before.
If you do want to go back there are still a few things we can do to the code to make it a bit nicer when it comes to the (internal) types. Just let me know what you prefer and I'm happy to help with the code.

@dthyresson
Copy link
Contributor Author

Closing in favor of #7745 with integrations v2 and types.

@dthyresson dthyresson closed this Mar 3, 2023
@Tobbe Tobbe reopened this Mar 3, 2023
@Tobbe
Copy link
Member

Tobbe commented Mar 3, 2023

Reopened and updated this PR to keep the TS changes separate in #7745

@replay-io
Copy link

replay-io bot commented Mar 3, 2023

16 replays were recorded for dc33220.

image 0 Failed
image 16 Passed
    requireAuth graphql checks
          ```
          locator.waitFor: Target closed
          =========================== logs ===========================
          waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <li><a href=https://app.replay.io/recording/b7aac179-5c49-40ba-90c0-2773b56d0963>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/9a4512cb-ea51-4a45-a5f7-679868404843>Check that <meta> tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/b7cacb27-8945-4aad-b85d-9ebe839ce9bd>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/33470ca2-90b6-471b-b73f-f5a3906d53a9>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/2f5ff74b-d38c-4b86-aaf7-5f1ccc6762aa>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/4f948214-2466-42c6-af61-3fbd15ce6516>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/4aeb4817-ca5b-4071-89e6-093883703035>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/5c5c4b06-3102-4f01-bf07-5b1f4d24ef7b>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/24330a6d-90e3-41d3-b9f4-bb1b82e531b1>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/e23b3944-7ea1-41ca-813e-000757ca01ec>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/a7994487-684a-453f-80ab-8ddf0cffd603>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/6f195327-c8fe-4271-8ac3-f09f81e0de6e>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/ef7d131b-3cdd-41f1-9406-991249b87746>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/9e6b8c6d-62cf-4783-998d-2349ad71b97f>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/81a078c9-cdf2-4331-b6da-337ca414ae3c>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

packages/auth-providers/supabase/web/src/supabase.ts Outdated Show resolved Hide resolved
docs/docs/auth/supabase.md Outdated Show resolved Hide resolved
docs/docs/auth/supabase.md Outdated Show resolved Hide resolved
docs/docs/auth/supabase.md Outdated Show resolved Hide resolved
docs/docs/auth/supabase.md Outdated Show resolved Hide resolved
@Tobbe Tobbe marked this pull request as ready for review March 4, 2023 06:42
@Tobbe Tobbe self-requested a review March 4, 2023 06:43
@Tobbe Tobbe changed the title Draft: feature(): Upgrade Supabase Auth to v2. feature: Upgrade Supabase Auth to v2. Mar 4, 2023
@Tobbe Tobbe merged commit bc5b3e5 into redwoodjs:main Mar 4, 2023
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Mar 4, 2023
@dthyresson
Copy link
Contributor Author

Implemented by #7719

@jtoar jtoar modified the milestones: next-release, v5.0.0 Mar 7, 2023
@jtoar jtoar changed the title feature: Upgrade Supabase Auth to v2. feature: Upgrade Supabase Auth to v2 Mar 29, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants