dev-xo / remix-auth-totp

A Time-Based One-Time Password (TOTP) Authentication Strategy for Remix-Auth.
https://totp.fly.dev
MIT License
418 stars 28 forks source link

Wrapping authenticate in try/catch prevents redirect #26

Closed andersr closed 1 year ago

andersr commented 1 year ago

Hi, I am seeing an issue that I think is specific to the TOTP strategy. but please let me know if that is incorrect.

I am getting intermittent Unhandled Promise Rejection errors for the deployed version of the app specifically during a login POST request. In order to be able to catch these errors, I would like to be able to wrap the authenticate call in a try/catch, as follows:

  try {
    await authenticator.authenticate(TOTP_STRATEGY, request, {
      successRedirect: route("/verify"),
      failureRedirect: currentPath,
      throwOnError: true,
    });
  } catch (error) {
    console.error("error: ", error);
    return json({
      authError: "Sorry, something went wrong.  Please try again.",
    });
  }

This is based on the recommended way to handle errors in the main auth docs.

However, if wrapping an authenticate call in try/catch there is no redirect to the verify route.

Any suggestions re. how to enable catching errors, while still also supporting a redirect to verify? Thanks for any tips or insights!

dev-xo commented 1 year ago

Hello @andersr, Remix Auth TOTP is a strategy built on top of remix-auth, so handling errors and authentication should be done the way remix-auth outlines (as you already mentioned).

I had a talk with Sergio about this on Discord (time ago). He also suggested handling errors just like the provided docs by you recommend.

To redirect after catching an error, you might wanna clone the request at the beginning (if required). Then, in the catch block, use that to redirect and set any cookies from the original request, preserving the auth flow and data.

Haven't had much time to dive in, but you could probably go with something like this:

export async function action({ request }: DataFunctionArgs) {
  const url = new URL(request.url)
  const currentPath = url.pathname

  try {
    await authenticator.authenticate('TOTP', request, {
      successRedirect: '/verify',

      // If error, redirect back to current path.
      failureRedirect: currentPath,
    })
  } catch (exception) {
    if (exception instanceof Response ) {
      // You can also check for the current path for more granular control.
      // if(exception.headers.get('location') === '/login') ...

      // Do something else.
      // ...

      // Redirect with response headers.
      // This will ensure that everything in the original request is preserved.
      return redirect('/verify', {
        headers: {
          'Set-Cookie': await commitSession(
            await getSession(exception.headers.get('Set-Cookie')),
          ),
        },
      })
    }

    // Bubble up any other exception.
    throw exception
  }
}

To be honest I was in this situation before and not sure if I was able to handle it properly either, seems kinda tricky.

Give it a try and notice me how it goes.

andersr commented 1 year ago

Hey @dev-xo - thanks for the quick reply! I updated my try/catch block to match what you have above and now it redirects as expected. However, one thing I wasn't clear on is why one would redirect to /verify if there is an exception(?) Thanks again!

dev-xo commented 1 year ago

Navigate to the route you need, (in this case, probably currentPath) @andersr. The code example was just a quick draft trying to assist with your current issue.

Everything working as expected? If so, I'm gonna close this for now.

andersr commented 1 year ago

Ok, thanks, makes sense. Yes, everything is working as expected.

andersr commented 1 year ago

Oh, sorry, I just realized that the reason the redirect was working was due to the verify redirect in the exception. When I remove it, the redirect stops working.

dev-xo commented 1 year ago

Been kinda busy lately, but I'd like to keep improving the library. As soon as I get some time, maybe I can come up with an error handling example.

If you have any examples or find some better ways to handle errors in both remix-auth and remix-auth-totp, feel free to share it, @andersr!