Custom state object

[aaronadamsCA]

We've had this library implemented for some time, and we continue to encounter challenges with the current recommendation to use session storage to store auth state (like returnTo) ourselves:

• Because authenticate() calls getSession(), there is a race condition with our own getSession() calls that cause new sessions to be lost.
• There is a race condition between tabs during simultaneous authentication that causes our returnTo values to overwrite each other.

#73 (comment) includes some messy workarounds for these problems.

Between the above and the additional work to gracefully recover from #67, our auth routes have become quite the mess. 😕

My request is to add support to authenticate for an optional state object stored in session storage, plus an optional successCallback to get it back.

As prior art, passport-oauth2 has offered basically the same thing since v1.6.0:

https://medium.com/passportjs/application-state-in-oauth-2-0-1d94379164e

I think this feature would make this strategy much more robust and easier to use.

I realize feature request #1 was declined at the time, but I'm not asking to change the value of state; I just want to store additional auth-related state alongside it in session storage. This would also line up nicely with a potential future fix to #67.

[lukerollans]

I think something along these lines would be very useful.

I'm using the Auth0 strategy and was puzzled there didn't seem to be a simple way to provide additional state to the authorization URL. I looked through some code and realised it isn't made possible by the OAuth2 strategy itself.

I'm not sure if this is a part of the OAuth 2 spec, but Auth0's authorization URL supports it at least. That is, we can append a returnTo=foo search param which will also be present after Auth0 redirects to the application's callback URL. Then, my callback loader could simply redirect to its value after committing the session.

For example...

// app/routes/signin.callback.ts
export const loader = ({ request }: LoaderFunctionArgs) => {
  const params = new URLSearchParams(request.url)
  return authenticator.authenticate("auth0", request, {
    successRedirect: params.get("returnTo") || "/dashboard"
  })
}

Unless I'm missing something, this could be solved by allowing authenticate to accept additional search params at redirect time rather than only at strategy instantiation time. For example...

// app/root.tsx
export const loader = ({ request }: LoaderFunctionArgs) => {
  const user = await authenticator.isAuthenticated(request)
  if (user) {
    return null
  } else {
    return authenticator.authenticate("auth0", request, {
      authorizationParams: {
        returnTo: request.url,
      }
    })
  }
}

@sergiodxa If this sounds reasonable I'm happy to open a PR. Although, as @aaronadamsCA suggests, having the supplied state be written to and retrieved from the configured session cookie storage automagically would be far superior and more secure

[aaronadamsCA]

@lukerollans, I have observed the same Auth0 behaviour you describe. I've also noticed this library already passes through any search params from the request URL to Auth0. So if you wanted to try your approach, I think you already could:

const url = new URL(request.url);
url.searchParams.append("returnTo", returnTo);
authenticator.authenticate("auth0", new Request(url, { headers: request.headers }));

However AFAIK the only spec-compliant method uses the state parameter (and the library's passthrough of search params from request URL to authorization URL might even be a bug).

[aaronadamsCA]

I think it will help to show just how complicated it is to meet the following requirements:

• Store the user's return path in session storage.
• Associate the return path with the specific auth request.
  • This is necessary to prevent race conditions between multiple tabs. Each tab should remember its own return path.
• Use the same session as the authenticator.
  • This is necessary to prevent data loss when there is no existing session cookie. If both authenticate() and your loader call getSession(request.headers.get("Cookie")), each call will create a new separate session, and one of them will be lost.

Here's the /auth route, then:

app/routes/auth._index/route.ts

import type { DataFunctionArgs } from "@remix-run/cloudflare";
import { redirect } from "@remix-run/cloudflare";
import invariant from "tiny-invariant";

export async function loader({
  context: { authenticator, sessionStorage },
  request,
}: DataFunctionArgs): Promise<never> {
  const returnPath = new URL(request.url).searchParams.get("returnPath");
  invariant(returnPath);

  try {
    await authenticator.authenticate("auth0", request);
  } catch (exception) {
    // If an unauthenticated user is being redirected to Auth0
    if (exception instanceof Response && exception.status === 302) {
      // Get the OAuth state from the response location header
      const location = exception.headers.get("Location");
      invariant(location);
      const state = new URL(location).searchParams.get("state");
      invariant(state);

      const session = await sessionStorage.getSession(
        // Reuse the same session as the authenticator to prevent lost data
        exception.headers.get("Set-Cookie"),
      );

      // Append the user's original destination to the session
      session.set("auth:returnPath", {
        ...session.get("auth:returnPath"),
        [state]: returnPath,
      });

      throw redirect(location, {
        headers: {
          "Set-Cookie": await sessionStorage.commitSession(session),
        },
      });
    }

    throw exception;
  }

  // The user is already authenticated
  throw redirect(returnPath);
}

The /auth/callback route is equally arcane.

I think "just store custom state in session storage" is a great recommendation, but given this library directly accesses that same session storage, I think it should offer some official way to pass this extra state through from the auth route to the callback route, possibly by following the API lead of Passport.js.

[richardscarrott]

I'd second this. We're looking to maintain state from our headless shopify registration page so we can capture extra fields, namely first and last name as well as marketing consent.

Would be great to see an API like this:

// app/routes/login.tsx
import { authenticator } from "~/auth.server";

export async function action({ request }: ActionArgs) {
  const url = new URL(request.url);
  const returnTo = url.searchParams.get("returnTo") || "/";
  const formData = Object.fromEntries(await request.formData());
  return authenticator.authenticate('shopify', request, {
    failureRedirect: "/login",
    // Stored in session storage (coupled with oauth state param)
    state: {
      returnTo: safeRedirect(new URL(request.url).searchParams.get("returnTo")),
      firstName: formData.firstName,
      lastName: formData.lastName,
      emailMarketingConsent: formData.emailMarketingConsent
    }
  });
}

// app/auth.server.tsx

// ...

authenticator.use(
    new OAuth2Strategy({
      // ... 
    }, ({
      accessToken,
      refreshToken,
      extraParams,
      profile,
      context,
      request,
      state // <--
    }) => {
      console.log(state); // { returnTo: '/foo', firstName: 'bar', lastName: 'baz', marketingConsent: false  }
    })
);

[sergiodxa]

The ?state value in the URL must be a unique non guessable value for security, as per the spec. If you want to keep any extra state before sending the request, catch the thrown redirect and add any state as a cookie or session.

That's the only safe way to do it, regardless if other libraries allows it.

[lukerollans]

@sergiodxa

If you want to keep any extra state before sending the request, catch the thrown redirect and add any state as a cookie or session.

yep, this is understood - I think the point of this (now closed?) issue is to have this functionality available as a part of the library without extra work by the developer.

in my initial comment I expressed interest in opening a PR, but I was wary of committing the time and effort if a PR would not be welcome. given you've closed the issue without providing any alternatives, I'm guessing a PR would not be welcome?

[MoSattler]

The ?state value in the URL must be a unique non guessable value for security, as per the spec. If you want to keep any extra state before sending the request, catch the thrown redirect and add any state as a cookie or session.

That's the only safe way to do it, regardless if other libraries allows it.

May anyone provide any example on how to do this ?

[sergiodxa]

@MoSattler e.g. https://sergiodxa.com/tutorials/add-returnto-behavior-to-remix-auth

[aaronadamsCA]

@sergiodxa, it is a real bummer to make a request only to have it closed months later for unrelated reasons.

Could you please re-read my original request? As it says,

I'm not asking to change the value of state; I just want to store additional auth-related state alongside it in session storage.

I did my best to explain why this is so difficult to do well right now. Your linked tutorial, for one, results in the very frustrating experience where a user who opens/reopens multiple tabs will see all but one redirect to an incorrect returnTo page.

The Passport.js solution to which I linked is secure, since it does exactly what you describe: storing state in the session.

If this library offered the same capability as Passport.js:

• We could code to an interface instead of an implementation, substantially improving DX.
• Docs/tutorials could describe a returnTo not subject to race conditions, substantially improving UX.
• Security would be unaffected.