pbteja1998 / remix-auth-email-link

MIT License
92 stars 22 forks source link

Magic link should be sent without requiring a redirect #45

Open 684efs3 opened 1 year ago

684efs3 commented 1 year ago

Describe the bug

Right now, if I do not include a successRedirect upon sending of the magic link, the code will throw an error, e.g.

// Libraries
import { json } from "@remix-run/node";
import { auth } from "~/services/auth.server";
import { sessionStorage } from "~/services/session.server";

export let loader = async ({ request }) => {
  await auth.isAuthenticated(request, { successRedirect: "/profile" });
  let session = await sessionStorage.getSession(request.headers.get("Cookie"));
  return json({
    magicLinkSent: session.has("auth:magiclink"),
    magicLinkEmail: session.get("auth:email"),
  });
};

export let action = async ({ request }) => {
  await auth.authenticate("email-link", request, {
    // successRedirect: "/",
  });
};

Your Example Website or App

This is a local site, but the code can be seen above.

Steps to Reproduce the Bug or Issue

// Libraries
import { json } from "@remix-run/node";
import { auth } from "~/services/auth.server";
import { sessionStorage } from "~/services/session.server";

export let loader = async ({ request }) => {
  await auth.isAuthenticated(request, { successRedirect: "/profile" });
  let session = await sessionStorage.getSession(request.headers.get("Cookie"));
  return json({
    magicLinkSent: session.has("auth:magiclink"),
    magicLinkEmail: session.get("auth:email"),
  });
};

export let action = async ({ request }) => {
  await auth.authenticate("email-link", request, {
    // successRedirect: "/",
  });
};

With successRedirect commented out, the code will throw the following error:

Error: Missing successRedirect. The successRedirect is required for POST requests.

Expected behavior

My login is through a dropdown, so the user can login on any page, simply by clicking the dropdown in the nav menu. Therefore, it is disruptive to the user to have the page be redirected to another page when they request a login email.

Screenshots or Videos

No response

Platform

Additional context

No response

dvnrsn commented 1 year ago

Hmm yeah this does seem to be an unnecessary opinion. I would expect it to match the remix-auth typing

successRedirect?: string;

There's no reason in my mind why the authenticate with email should need a redirect. The developer just needs to make sure to give some UI that the email has been sent and that can happen after a JSON response from the action.

I want to implement this as well maybe end of next week so I'm a bit behind you or I'd take a stab at proposing a change on this

dvnrsn commented 1 year ago

On second thought, do you see this magicLink and email being committed to the session? It might be best to leave that to the library. Playing with the session can be a bit tricky.

pbteja1998 commented 1 year ago

why not just redirect to the same URL?

successRedirect: request.url

dvnrsn commented 1 year ago

why not just redirect to the same URL?

Hey Bhanu Teja, thanks for your work!

That's fine (although there is the downstream UX question that is out of scope: how do we know the email has been sent when on page) but the ergonomics here are the question. It trips one up when the API changes. Can we expand the error?

'Missing successRedirect. The successRedirect is required for POST requests.'

or provide a link to README to explain why remix-auth-email-link is unique and needs to set cookie?

pbteja1998 commented 1 year ago

The cookie is needed because of the below option.

https://github.com/pbteja1998/remix-auth-email-link/blob/main/src/index.ts#L427

maxprilutskiy commented 10 months ago

Sort of related: https://github.com/pbteja1998/remix-auth-email-link/pull/47

684efs3 commented 10 months ago

why not just redirect to the same URL?

successRedirect: request.url

I tried this and the magic link led to a page that said:

This page isn’t workinglocalhost redirected you too many times.
[Try deleting your cookies](https://support.google.com/chrome?p=rl_error&hl=en-US).
ERR_TOO_MANY_REDIRECTS
pbteja1998 commented 10 months ago

why not just redirect to the same URL? successRedirect: request.url

I tried this and the magic link led to a page that said:

This page isn’t workinglocalhost redirected you too many times.
[Try deleting your cookies](https://support.google.com/chrome?p=rl_error&hl=en-US).
ERR_TOO_MANY_REDIRECTS

Where did you write it? Please show some code snippets like the route url, loader, action etc. If you have a sample repo; that would be even better to see exactly what you wrote and where.

684efs3 commented 10 months ago

Of course! Thank you!

I followed your instructions. This is my magic.callback.tsx file:

import { auth } from '~/services/auth.server'

export let loader = async ({ request }) => {
  console.log(request)
  await auth.authenticate('email-link', request, {
    // If the user was authenticated, we redirect them to their profile page
    // This redirect is optional, if not defined the user will be returned by
    // the `authenticate` function and you can render something on this page
    // manually redirect the user.

    successRedirect: request.url,
    // If something failed we take them back to the login page
    // This redirect is optional, if not defined any error will be throw and
    // the ErrorBoundary will be rendered.
    failureRedirect: '/login'
  })
}

The only change I made was successRedirect: request.url,, which was originally successRedirect: '/' (this worked with no problems).

This is my login.jsx:

export let loader = async ({ request }) => {
  await auth.isAuthenticated(request, { successRedirect: '/profile' })
  let session = await sessionStorage.getSession(request.headers.get('Cookie'))
  return json({
    magicLinkSent: session.has('auth:magiclink'),
    magicLinkEmail: session.get('auth:email')
  })
}

export let action = async ({ request }) => {
  await auth.authenticate('email-link', request, {
    successRedirect: '/',
    failureRedirect: '/login'
  })
}

If I change the successRedirect: '/', in this action to successRedirect: request.url I get the same error. So I assume that the issue is in magic.callback.jsx.

I am looking for a similar experience as on mobbin.com. When you login, it takes you right back to the page you were on before logging in.

(I am using a dropdown login component, so the user doesn't go to a separate login page, but the idea remains the same).

ersinakinci commented 7 months ago

I just want to point out that the current behavior goes against Remix Auth's own documentation:

Custom redirect URL based on the user

Say we have /dashboard and /onboarding routes, and after the user authenticates, you need to check some value in their data to know if they are onboarded or not.

If we do not pass the successRedirect option to the authenticator.authenticate method, it will return the user data.

As a new Remix Auth user, this was very confusing.

ttizze commented 6 days ago

same issue