mitchelvanbever / remix-auth-supabase

MIT License
102 stars 14 forks source link

Sign up example #9

Closed ianduvall closed 2 years ago

ianduvall commented 2 years ago

I was attempting to add sign up with email and password but ran into a few roadblocks. Should sign up be a separate SupabaseStrategy from signInWithEmail? I noticed signUpWithEmail returns a User instead of a Session if an email hasn't been confirmed yet. How should I handle a User instead of a Session in this case?

rphlmr commented 2 years ago

Hi, Do you want to log in user with not verified email ? (if you don't care about email verification, there is a supabase option to uncheck this verification on your admin panel).

If you want to create a user with email verified, I think you can do that in a single strategy. Something like that (rapidly tested :D )

export const supabaseStrategy = new SupabaseStrategy(
  {
    supabaseClient: supabaseAdmin,
    sessionStorage,
    sessionKey: 'sb:session',
    sessionErrorKey: 'sb:error',
  },
  async({ req, supabaseClient }) => {
    const form = await req.formData()
    const email = form?.get('email')
    const password = form?.get('password')

    if (!email) throw new AuthorizationError('Email is required')
    if (typeof email !== 'string')
      throw new AuthorizationError('Email must be a string')

    if (!password) throw new AuthorizationError('Password is required')
    if (typeof password !== 'string')
      throw new AuthorizationError('Password must be a string')

    return supabaseClient.auth.api
      .signInWithEmail(email, password)
      .then(({ data, error }): Session => {
        if (error || !data) {
          throw new AuthorizationError(
            error?.message ?? 'No user session found',
          )
        }

        if (data.user?.email_confirmed_at) throw new AuthorizationError('verify-email')

        return data
      })
  },
)

and then, where you redirect onFailure from authenticator.authenticate, in the loader :

export const loader: LoaderFunction = async({ request }) => {
  await supabaseStrategy.checkSession(request, {
    successRedirect: '/private',
  })

  const session = await sessionStorage.getSession(
    request.headers.get('Cookie'),
  )

  const error = session.get(
    authenticator.sessionErrorKey,
  ) as LoaderData['error']

  if (error?.message === 'verify-email') throw redirect('/valid-your-email')

  return json<LoaderData>({ error })
}

That's an idea, maybe @mitchelvanbever have a better solution :D

With this example, the user will be always redirected to "/valid-your-email" until email is verified

mitchelvanbever commented 2 years ago

Hi,

Signing up and signing in are two completely different actions. I understand that you would like to authenticate newly registered users but there are a few things you need to consider (some of them already pointed out by @rphlmr).

So while you technically could use sign up as a one-time authentication method, it really isn't an authentication method suitable to use for subsequent visits (which is what you'd ultimately want).

I think what you'd want is for your application to handle the signup process separately from remix-auth and use remix-auth only for the sign-in and handling the authentication process.

Closing the issue, but feel free to ask more specific questions if you're having trouble with the implementation 👍

Ethaan commented 2 years ago

NOTE: thjs solution will only work if you have "Email Confirmation" off as stated here

If "Email Confirmations" is turned off, both a user and a session will be returned

as @mitchelvanbever mentioned, you need to manually set the access token and refresh token

so doing something like this its working for me

export const action: ActionFunction = async ({ request }) => {
  const form = await request.formData();
  const email = form.get("email")!;
  const name = form.get("name")!;
  const password = form.get("password")!;

  if (
    typeof name !== "string" ||
    typeof email !== "string" ||
    typeof password !== "string"
  ) {
    throw new Error(`Form not submitted correctly.`);
  }

  // remember to do the "Email Confirmation" toggle of "supabaseSession" will be null
  const { error, session: supabaseSession } = await supabaseClient.auth.signUp(
    {
      email,
      password,
    },
    {
      data: { name },
    }
  );

  if (error) {
    return { error: true, ...error };
  }

  const session = await getSession(request.headers.get("Cookie"));
  session.set("sb:session", {
    access_token: supabaseSession!.access_token,
    refresh_token: supabaseSession!.refresh_token,
  });

  return redirect("/", {
    headers: {
      "Set-Cookie": await commitSession(session), // in my case every new signup lead to "/"
    },
  });
};

Then supabaseStrategy.checkSession(request); should return the session

export const loader: LoaderFunction = async ({ request }) => {
  const session = await supabaseStrategy.checkSession(request);

  return { session };
};

I just landed this solution today after a day of trial and error, and i'm still not sure if this is the correct way to do it, @mitchelvanbever what do you think?

mitchelvanbever commented 2 years ago

I just landed this solution today after a day of trial and error, and i'm still not sure if this is the correct way to do it, @mitchelvanbever what do you think?

It's all very subjective to how you handle the authentication in the rest of the application.

As explained previously the loader in your example will only work once. When the user is still new to the system, it will return a session but on subsequent requests for that user it will throw. So if you have a different page with the according action for logging in existing users.

Having that said there is one remark I have about your signup loader.

  1. It could also be used a strategy which would reduce the need for the last section of your loader where you set the cookie manually (like @rphlmr sketched out in his example).