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

Magic link should be sent without requiring a redirect #45

Open
684efs3 opened this issue Nov 9, 2023 · 10 comments
Open

Magic link should be sent without requiring a redirect #45

684efs3 opened this issue Nov 9, 2023 · 10 comments

Comments

@684efs3
Copy link

684efs3 commented Nov 9, 2023

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

  • OS: MacOS
  • Browser: Brave
  • Version: Version 1.50.121 Chromium: 112.0.5615.138 (Official Build) (arm64)

Additional context

No response

@dvnrsn
Copy link

dvnrsn commented Nov 10, 2023

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
Copy link

dvnrsn commented Nov 10, 2023

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
Copy link
Owner

why not just redirect to the same URL?

successRedirect: request.url

@dvnrsn
Copy link

dvnrsn commented Nov 10, 2023

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
Copy link
Owner

The cookie is needed because of the below option.

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

@maxprilutskiy
Copy link

Sort of related: #47

@684efs3
Copy link
Author

684efs3 commented Jan 23, 2024

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
Copy link
Owner

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
Copy link
Author

684efs3 commented Jan 23, 2024

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
Copy link

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.

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

No branches or pull requests

5 participants