Skip to content

Feat: Login, Logout, Signup#48

Merged
gabestein merged 13 commits into
mainfrom
tr/logins
Sep 5, 2023
Merged

Feat: Login, Logout, Signup#48
gabestein merged 13 commits into
mainfrom
tr/logins

Conversation

@isTravis
Copy link
Copy Markdown
Member

@isTravis isTravis commented Aug 22, 2023

Resolves #46

Hey folks, I had a lot of login and account code from a mix of Underlay repos (both pushed up and local). Diving into figuring out how to get login/signup/password reset, etc to work well given supabase took a few tries across a couple projects, so I thought at least sharing this could help jump start that work.

I'm happy to make this code real, but I was encouraged to not spend much time with it and just share what was easy and point folks in the right direction. Underlay was using Mantine, so there are a handful of those components that would need to be migrated to our component system. This PR won't work until a handful of type and import errors are fixed, but I think they're relatively trivial.

There is some configuring on the Supabase side that is needed to enable/configure email signups, but I believe that's also pretty straightforward. Happy to answer questions as they come up.

When testing, be sure to comment out the hard-coded userId returned in core/lib/auth/loginId.ts at the beginning of the getIdFromJWT function.

A quick overview of functionality this helps outline:

  • Creating a user (sign up)
  • Logging in
  • Logging out
  • Updating user email
  • Updating user password
  • Forgot password workflow
  • Refreshing login cookies on page load to persist signins

A quick overview of added/modified files

  • app/layout: Added <InitClient />
  • app/InitClient.tsx: Initializes client-side libraries like supabase and any future analytics/logging/etc tools. Handles managing authentication state based on cookies and supabase auth events.
  • app/confirm/page.tsx: Simple page for redirecting succesful signups to. InitClient has a check for redirecting to the landing page if a user has landed on /confirm. This check is necessary because on this first page after a successful signup, supabase provides a "SIGNED_IN" event, which allows us to set auth cookies. Until that happens, we can't server-render a user's login (i.e. their avatar, name, permissioned content, etc) because there is no cookie for us to detect on the server. So, we need a page refresh so that our server knows the user is now logged in, but we can't just refresh the page on every SIGNED_IN or TOKEN_REFRESH event because those fire on every page load (I believe TOKEN_REFRESH fires on every page load). I think I'm maybe missing some of the state logic in this explanation, but I remember playing with this configuration for quite a while and bumping into frustrating edge cases until I found my way to this configuration. This isn't an issue when logging in, because we log in from the client which sets cookies right away, and then manually redirect a user to /, thus giving us the page refresh we need. It's only really an issue for the Signup workflow, since a user has to confirm via email, and thus we don't just handle everything in a single client-side session.
  • LoginSwitcher: somewhere we'll have a "sign out" button. I suspect we'll eventually have some sort of popover menu in LoginSwitcher with that option, a link to user settings, etc. I stuck a handleSignout function there, but it can be moved to wherever needed.
  • /app/forgot: Page to initiate password reset.
  • /app/login: Page for logging in
  • /app/reset: Page for completing the process of resetting your password which a user can initiate from the 'Settings' page.
  • /app/settings: We likely don't need all the same functionality from this page, but it demonstrates how a user can update their email, initiate password reset, and some other supabase auth functions.
  • app/signup: Page for creating a user account. Note that we pass everything to our API and from the backend call to supabase to create the necessary account.
  • pages/api/user: Backend API function for creating and updating user details. Notice that we create the user in supabase first, and then create it in our backend using the id supabase returns. Supabase provides a client-side function for doing this, but if we did not do this on the backend, we would have to create a SQL function that automatically copies over new users from the auth schema into the public schema. The "Authentication" section of /core/README.md talks more about this. If we can't make this supabase call from our backend (e.g. in the case of 3rd party oauth), this POST endpoint would likely change or be removed, as we would handle it all with SQL functions. It's just a little bit easier to handle creating unique slugs, etc by writing API JS rather than SQL functions for me, which is why I opted for this when doing an email-only signup approach. Alternatively, we could do a client-side supabase signup and then pass that data onto our backend for creating users on our public schema. This would let us handle 3rd party auth I believe, but the case we have to be careful about is when our public/users table has a constraint that doesn't exist on the auth schema - something like unique user slugs, for example. In that case, if the user has put in data that isn't valid (e.g. a slug is already taken) - we need the whole process to fail (supabase auth insert, and user table insert), so only attempting to insert into our public/user table after supabase creation is risky, because we might have to go and undo/delete the the supabase auth schema changes (but it fires off an email, so no undoing that I suppose...). Anyways, we could handle this with a few checks to our public/user table first, but it introduces more roundtrip API calls, and the potential for a race condition (user slug looks unique, but someone else claims it in between the time of the two API calls). If the whole process could be atomic and kept behind a single API request, that'd be ideal. Handling it all using SQL functions allows the whole process to be atomic, because if any of the SQL functions fail, the whole signup creation fails.

Comment thread core/app/InitClient.tsx
Comment thread core/app/layout.tsx
<html lang="en">
<InitClient />
<body>{children}</body>
<body>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Putting initClient outside the body was causing hydration errors because the server refused to render a div outside the body (but the browser will).

Comment thread core/app/settings/SettingsForm.tsx Outdated
@@ -1,4 +1,3 @@
import "server-only";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using this was throwing errors no matter what, and I couldn't find much documentation for it, so I removed it. As far as I can tell, this is running on the server (using 'use client' fails), but someone should verify this isn't going to cause problems down the line.

Copy link
Copy Markdown
Member Author

@isTravis isTravis Aug 31, 2023

Choose a reason for hiding this comment

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

We should look more closely at this. server-only is used to say "I absolutely don't want this function loading on the client side because of security issues". If it's throwing an error, it could mean this sensitive function (has private keys) could be getting bundled into client code.

Documentation on the use of server-only here.

This is different than "use client" which is a flag for react rendering client components rather than server components. For plain ol' functions that are not components, there is no notion of whether it's a "client function" or "server function", so something like server-onlycan help you explicitly say, "Don't let this function be bundled into client-side code"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yup, it was loading it on the client due to next-connect. Switching to route handlers allowed me to restore it (although it's probably not needed because it's called from a routes.ts file now).

Comment on lines +10 to +12
auth: {
autoRefreshToken: true,
persistSession: true /* Persisting session is necessary for autoRefresh to function */,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we're already doing this on the client, but needed it on the server as well, or sessions weren't persisting.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm confused by this - but maybe the supabase API has changed?

getServerSupabase uses the SERVICE_ROLE key, so that we can act as admin on some server-side function. There shouldn't be any user sessions involved in this function.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yup, it was loading on the client. Switching to app routing allowed me to remove it.

Comment thread core/package.json Outdated
Comment thread core/pages/api/user.ts Outdated
Comment thread core/pages/api/user.ts Outdated
<div className="text-xs text-gray-400">{loginData.email}</div>
</div>

<LogoutButton />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this needed to be a separate client component, because loginData needs to run on the server

@gabestein
Copy link
Copy Markdown
Member

Okay, I think this is in pretty good shape to merge (once we get rid of the merge conflicts). I plan to do a fast follow with some better designed pages.

The main change from the original PR is it switches from using page API routing with next-connect to app routing with Route Handlers.

I also:

  • added user seeding that allows us to seed users with access to specific communities (which is already working decently, actually). See core/prisma/seed.ts
  • added a horribly designed logout button, just to make it easier to login and out.
  • removed some avatar stuff from forms that didn't have any real handlers.

@gabestein gabestein changed the title tr/logins Feat: Login, Logout, Signup Aug 31, 2023
@isTravis isTravis temporarily deployed to tr/logins - core PR #48 September 5, 2023 18:21 — with Render Destroyed
@isTravis isTravis temporarily deployed to tr/logins - integration-evaluations PR #48 September 5, 2023 18:21 — with Render Destroyed
@isTravis isTravis temporarily deployed to tr/logins - integration-submissions PR #48 September 5, 2023 18:21 — with Render Destroyed
@isTravis isTravis temporarily deployed to tr/logins - core PR #48 September 5, 2023 18:24 — with Render Destroyed
@isTravis isTravis temporarily deployed to tr/logins - integration-evaluations PR #48 September 5, 2023 18:24 — with Render Destroyed
@isTravis isTravis temporarily deployed to tr/logins - integration-submissions PR #48 September 5, 2023 18:24 — with Render Destroyed
@gabestein gabestein merged commit 5ff35f7 into main Sep 5, 2023
@gabestein gabestein deleted the tr/logins branch September 5, 2023 18:30
@gabestein gabestein mentioned this pull request Sep 8, 2023
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.

Review and modify login PR

2 participants