🐛 Fix infinite loop when trying to migrate legacy cookie (#1561)

.env.development

@@ -5,6 +5,9 @@ SECRET_PASSWORD=abcdef1234567890abcdef1234567890
 # Example: https://example.com
 NEXT_PUBLIC_BASE_URL=http://localhost:3000
 
+# AUTH_URL should be the same as NEXT_PUBLIC_BASE_URL
+AUTH_URL=http://localhost:3000
+
 # A connection string to your Postgres database
 DATABASE_URL="postgres://postgres:postgres@localhost:5450/rallly"
 

apps/web/.env.test

@@ -1,5 +1,6 @@
 PORT=3002
 NEXT_PUBLIC_BASE_URL=http://localhost:3002
+AUTH_URL=$NEXT_PUBLIC_BASE_URL
 SECRET_PASSWORD=abcdef1234567890abcdef1234567890
 DATABASE_URL=postgres://postgres:postgres@localhost:5450/rallly
 SUPPORT_EMAIL=support@rallly.co

apps/web/src/auth/edge/index.ts

@@ -0,0 +1 @@
+export * from "./with-auth";

apps/web/src/auth/edge/with-auth.ts

@@ -0,0 +1,63 @@
+import type { NextResponse } from "next/server";
+import type { NextAuthRequest, Session } from "next-auth";
+import NextAuth from "next-auth";
+
+import { nextAuthConfig } from "@/next-auth.config";
+
+import {
+  getLegacySession,
+  migrateLegacyJWT,
+} from "../legacy/next-auth-cookie-migration";
+
+const { auth } = NextAuth(nextAuthConfig);
+
+export const withAuth = (
+  middleware: (request: NextAuthRequest) => Promise<NextResponse>,
+) => {
+  return async (request: NextAuthRequest) => {
+    let legacySession: Session | null = null;
+
+    try {
+      legacySession = await getLegacySession();
+    } catch (e) {
+      console.error(e);
+    }
+
+    let session = legacySession;
+
+    if (!session) {
+      try {
+        session = await auth();
+      } catch (e) {
+        console.error(e);
+      }
+    }
+
+    try {
+      const res = await nextAuthConfig.callbacks.authorized({
+        request,
+        auth: session,
+      });
+
+      if (res !== true) {
+        return res;
+      }
+    } catch (e) {
+      console.error(e);
+    }
+
+    request.auth = session;
+
+    const middlewareRes = await middleware(request);
+
+    if (legacySession) {
+      try {
+        await migrateLegacyJWT(middlewareRes);
+      } catch (e) {
+        console.error(e);
+      }
+    }
+
+    return middlewareRes;
+  };
+};

apps/web/src/auth/legacy/helpers/jwt.ts

@@ -1,7 +1,6 @@
 import hkdf from "@panva/hkdf";
 import { jwtDecrypt } from "jose";
-
+import type { JWT } from "next-auth/jwt";
-import type { JWT } from "./types";
 
 /** Decodes a NextAuth.js issued JWT. */
 export async function decodeLegacyJWT(token: string): Promise<JWT | null> {

apps/web/src/auth/legacy/helpers/types.ts

@@ -1,46 +0,0 @@
-export interface DefaultJWT extends Record<string, unknown> {
-  name?: string | null;
-  email?: string | null;
-  picture?: string | null;
-  sub?: string;
-}
-
-/**
- * Returned by the `jwt` callback and `getToken`, when using JWT sessions
- *
- * [`jwt` callback](https://next-auth.js.org/configuration/callbacks#jwt-callback) | [`getToken`](https://next-auth.js.org/tutorials/securing-pages-and-api-routes#using-gettoken)
- */
-export interface JWT extends Record<string, unknown>, DefaultJWT {}
-
-export interface JWTEncodeParams {
-  /** The JWT payload. */
-  token?: JWT;
-  /**
-   * Used in combination with `secret` when deriving the encryption secret for the various NextAuth.js-issued JWTs.
-   * @note When no `salt` is passed, we assume this is a session token.
-   * This is for backwards-compatibility with currently active sessions, so they won't be invalidated when upgrading the package.
-   */
-  salt?: string;
-  /** The key material used to encode the NextAuth.js issued JWTs. Defaults to `NEXTAUTH_SECRET`. */
-  secret: string | Buffer;
-  /**
-   * The maximum age of the NextAuth.js issued JWT in seconds.
-   * @default 30 * 24 * 60 * 60 // 30 days
-   */
-  maxAge?: number;
-}
-
-export interface JWTDecodeParams {
-  /** The NextAuth.js issued JWT to be decoded */
-  token?: string;
-  /**
-   * Used in combination with `secret` when deriving the encryption secret for the various NextAuth.js-issued JWTs.
-   * @note When no `salt` is passed, we assume this is a session token.
-   * This is for backwards-compatibility with currently active sessions, so they won't be invalidated when upgrading the package.
-   */
-  salt?: string;
-  /** The key material used to decode the NextAuth.js issued JWTs. Defaults to `NEXTAUTH_SECRET`. */
-  secret: string | Buffer;
-}
-
-export type Secret = string | Buffer;

apps/web/src/auth/legacy/next-auth-cookie-migration.ts

@@ -1,65 +1,70 @@
-import type { NextRequest } from "next/server";
+import { absoluteUrl } from "@rallly/utils/absolute-url";
-import { NextResponse } from "next/server";
+import { cookies } from "next/headers";
+import type { NextResponse } from "next/server";
+import type { Session } from "next-auth";
 import { encode } from "next-auth/jwt";
 
 import { decodeLegacyJWT } from "./helpers/jwt";
 
-const isSecureCookie =
+const isSecureCookie = absoluteUrl().startsWith("https://");
-  process.env.NEXT_PUBLIC_BASE_URL?.startsWith("https://") ?? false;
 
 const prefix = isSecureCookie ? "__Secure-" : "";
 
 const oldCookieName = prefix + "next-auth.session-token";
 const newCookieName = prefix + "authjs.session-token";
 
+export async function getLegacySession(): Promise<Session | null> {
+  const cookieStore = cookies();
+  const legacySessionCookie = cookieStore.get(oldCookieName);
+  if (legacySessionCookie) {
+    const decodedCookie = await decodeLegacyJWT(legacySessionCookie.value);
+
+    if (decodedCookie?.sub) {
+      const { sub: id, ...rest } = decodedCookie;
+      return {
+        user: { id, ...rest },
+        expires: decodedCookie.exp
+          ? new Date(decodedCookie.exp * 1000).toISOString()
+          : new Date(Date.now() + 30 * 60 * 60 * 1000).toISOString(),
+      };
+    }
+  }
+
+  return null;
+}
+
+async function getLegacyJWT() {
+  const cookieStore = cookies();
+  const legacySessionCookie = cookieStore.get(oldCookieName);
+  if (legacySessionCookie) {
+    const decodedCookie = await decodeLegacyJWT(legacySessionCookie.value);
+    if (decodedCookie) {
+      return decodedCookie;
+    }
+  }
+  return null;
+}
+
 /**
- * Migrates the next-auth cookies to the new authjs cookie names
+ * Replace the old legacy cookie with the new one
- * This is needed for next-auth v5 which renamed the cookie prefix from 'next-auth' to 'authjs'
  */
-export function withAuthMigration(
+export async function migrateLegacyJWT(res: NextResponse) {
-  middleware: (req: NextRequest) => void | Response | Promise<void | Response>,
+  const legacyJWT = await getLegacyJWT();
-) {
-  return async (req: NextRequest) => {
-    const oldCookie = req.cookies.get(oldCookieName);
 
-    // If the old cookie doesn't exist, return the middleware
+  if (legacyJWT) {
-    if (!oldCookie) {
+    const newJWT = await encode({
-      return middleware(req);
+      token: legacyJWT,
-    }
-
-    const response = NextResponse.redirect(req.url);
-    response.cookies.delete(oldCookieName);
-
-    // If the new cookie exists, delete the old cookie first and rerun middleware
-    if (req.cookies.get(newCookieName)) {
-      return response;
-    }
-
-    const decodedCookie = await decodeLegacyJWT(oldCookie.value);
-
-    // If old cookie is invalid, delete the old cookie first and rerun middleware
-    if (!decodedCookie) {
-      return response;
-    }
-
-    // Set the new cookie
-    const encodedCookie = await encode({
-      token: decodedCookie,
       secret: process.env.SECRET_PASSWORD,
       salt: newCookieName,
     });
 
-    // Set the new cookie with the same value and attributes
+    res.cookies.set(newCookieName, newJWT, {
-    response.cookies.set(newCookieName, encodedCookie, {
-      path: "/",
-      secure: isSecureCookie,
-      sameSite: "lax",
       httpOnly: true,
+      secure: isSecureCookie,
+      expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 7),
+      sameSite: "lax",
+      path: "/",
     });
-
+    res.cookies.delete(oldCookieName);
-    // Delete the old cookie
+  }
-    response.cookies.delete(oldCookieName);
-
-    return response;
-  };
 }

apps/web/src/auth/middleware.ts

@@ -1,15 +0,0 @@
-import type { NextRequest } from "next/server";
-import type { NextAuthRequest } from "next-auth";
-import NextAuth from "next-auth";
-
-import { nextAuthConfig } from "@/next-auth.config";
-
-const { auth } = NextAuth(nextAuthConfig);
-
-export function withAuth(
-  middleware: (
-    req: NextAuthRequest,
-  ) => void | Response | Promise<void | Response>,
-): (req: NextRequest) => void | Response | Promise<void | Response> {
-  return (req: NextRequest) => auth(middleware)(req, undefined as never);
-}

apps/web/src/middleware.ts

@@ -3,44 +3,41 @@ import { withPostHog } from "@rallly/posthog/next/middleware";
 import { NextResponse } from "next/server";
 
 import { getLocaleFromHeader } from "@/app/guest";
-import { withAuthMigration } from "@/auth/legacy/next-auth-cookie-migration";
+import { withAuth } from "@/auth/edge";
-import { withAuth } from "@/auth/middleware";
 
 const supportedLocales = Object.keys(languages);
 
-export const middleware = withAuthMigration(
+export const middleware = withAuth(async (req) => {
-  withAuth(async (req) => {
+  const { nextUrl } = req;
-    const { nextUrl } = req;
+  const newUrl = nextUrl.clone();
-    const newUrl = nextUrl.clone();
 
-    const isLoggedIn = req.auth?.user?.email;
+  const isLoggedIn = req.auth?.user?.email;
 
-    // if the user is already logged in, don't let them access the login page
+  // if the user is already logged in, don't let them access the login page
-    if (/^\/(login)/.test(newUrl.pathname) && isLoggedIn) {
+  if (/^\/(login)/.test(newUrl.pathname) && isLoggedIn) {
-      newUrl.pathname = "/";
+    newUrl.pathname = "/";
-      return NextResponse.redirect(newUrl);
+    return NextResponse.redirect(newUrl);
-    }
+  }
 
-    // Check if locale is specified in cookie
+  // Check if locale is specified in cookie
-    let locale = req.auth?.user?.locale;
+  let locale = req.auth?.user?.locale;
-    if (locale && supportedLocales.includes(locale)) {
+  if (locale && supportedLocales.includes(locale)) {
-      newUrl.pathname = `/${locale}${newUrl.pathname}`;
+    newUrl.pathname = `/${locale}${newUrl.pathname}`;
-    } else {
+  } else {
-      // Check if locale is specified in header
+    // Check if locale is specified in header
-      locale = await getLocaleFromHeader(req);
+    locale = await getLocaleFromHeader(req);
-      newUrl.pathname = `/${locale}${newUrl.pathname}`;
+    newUrl.pathname = `/${locale}${newUrl.pathname}`;
-    }
+  }
 
-    const res = NextResponse.rewrite(newUrl);
+  const res = NextResponse.rewrite(newUrl);
-    res.headers.set("x-pathname", newUrl.pathname);
+  res.headers.set("x-pathname", newUrl.pathname);
 
-    if (req.auth?.user?.id) {
+  if (req.auth?.user?.id) {
-      await withPostHog(res, { distinctID: req.auth.user.id });
+    await withPostHog(res, { distinctID: req.auth.user.id });
-    }
+  }
 
-    return res;
+  return res;
-  }),
+});
-);
 
 export const config = {
   matcher: ["/((?!api|_next/static|_next/image|static|.*\\.).*)"],

apps/web/src/next-auth.ts

@@ -7,6 +7,7 @@ import z from "zod";
 import { CustomPrismaAdapter } from "./auth/adapters/prisma";
 import { isEmailBlocked } from "./auth/helpers/is-email-blocked";
 import { mergeGuestsIntoUser } from "./auth/helpers/merge-user";
+import { getLegacySession } from "./auth/legacy/next-auth-cookie-migration";
 import { EmailProvider } from "./auth/providers/email";
 import { GoogleProvider } from "./auth/providers/google";
 import { GuestProvider } from "./auth/providers/guest";
@@ -22,7 +23,12 @@ const sessionUpdateSchema = z.object({
   weekStart: z.number().nullish(),
 });
 
-export const { auth, handlers, signIn, signOut } = NextAuth({
+const {
+  auth: originalAuth,
+  handlers,
+  signIn,
+  signOut,
+} = NextAuth({
   ...nextAuthConfig,
   adapter: CustomPrismaAdapter({
     migrateData: async (userId) => {
@@ -169,3 +175,14 @@ export const { auth, handlers, signIn, signOut } = NextAuth({
     },
   },
 });
+
+const auth = async () => {
+  const session = await getLegacySession();
+  if (session) {
+    return session;
+  }
+
+  return originalAuth();
+};
+
+export { auth, handlers, signIn, signOut };

apps/web/src/utils/session/session-config.ts

@@ -1,8 +1,10 @@
+import { absoluteUrl } from "@rallly/utils/absolute-url";
+
 export const sessionConfig = {
   password: process.env.SECRET_PASSWORD ?? "",
   cookieName: "rallly-session",
   cookieOptions: {
-    secure: process.env.NEXT_PUBLIC_BASE_URL?.startsWith("https://") ?? false,
+    secure: absoluteUrl().startsWith("https://") ?? false,
   },
   ttl: 60 * 60 * 24 * 30, // 30 days
 };

apps/web/tests/helpers/next-auth-v4.ts

@@ -0,0 +1,36 @@
+import hkdf from "@panva/hkdf";
+import { EncryptJWT } from "jose";
+import type { JWT } from "next-auth/jwt";
+
+const now = () => (Date.now() / 1000) | 0;
+export async function getDerivedEncryptionKey(
+  keyMaterial: string | Buffer,
+  salt: string,
+) {
+  return await hkdf(
+    "sha256",
+    keyMaterial,
+    salt,
+    `NextAuth.js Generated Encryption Key${salt ? ` (${salt})` : ""}`,
+    32,
+  );
+}
+
+interface JWTEncodeParams {
+  token?: JWT;
+  salt?: string;
+  secret: string | Buffer;
+  maxAge?: number;
+}
+
+export async function encode(params: JWTEncodeParams) {
+  /** @note empty `salt` means a session token. See {@link JWTEncodeParams.salt}. */
+  const { token = {}, secret, maxAge = 30 * 24 * 60 * 60, salt = "" } = params;
+  const encryptionSecret = await getDerivedEncryptionKey(secret, salt);
+  return await new EncryptJWT(token)
+    .setProtectedHeader({ alg: "dir", enc: "A256GCM" })
+    .setIssuedAt()
+    .setExpirationTime(now() + maxAge)
+    .setJti("some-random-id")
+    .encrypt(encryptionSecret);
+}

apps/web/tests/next-auth-migration.spec.ts

@@ -0,0 +1,57 @@
+import { expect, test } from "@playwright/test";
+import { prisma } from "@rallly/database";
+
+import { encode } from "./helpers/next-auth-v4";
+
+const legacyGuestId = "user-1234";
+
+test.describe.serial(() => {
+  test.beforeAll(async () => {
+    await prisma.poll.create({
+      data: {
+        id: "legacy-guest-poll",
+        title: "Test Poll",
+        adminUrlId: "admin-url-id",
+        participantUrlId: "participant-url-id",
+        guestId: legacyGuestId,
+      },
+    });
+  });
+  test.afterAll(async () => {
+    await prisma.poll.delete({
+      where: {
+        id: "legacy-guest-poll",
+      },
+    });
+  });
+
+  test("should see poll on login page", async ({ page }) => {
+    const context = page.context();
+    const legacyToken = await encode({
+      token: {
+        sub: legacyGuestId,
+      },
+      secret: process.env.SECRET_PASSWORD,
+    });
+
+    // set cookie to simulate legacy guest
+    await context.addCookies([
+      {
+        name: "next-auth.session-token",
+        value: legacyToken,
+        httpOnly: true,
+        expires: Date.now() / 1000 + 60 * 60 * 24 * 7,
+        secure: false,
+        sameSite: "Lax",
+        domain: "localhost",
+        path: "/",
+      },
+    ]);
+
+    // For some reason it doesn't work unless we need to redirect
+    await page.goto("/login");
+
+    // Check if the poll title exists in the page content
+    await expect(page.getByText("Test Poll")).toBeVisible();
+  });
+});

scripts/docker-start.sh

@@ -2,6 +2,7 @@
 set -e
 
 export DIRECT_DATABASE_URL=$DATABASE_URL
+export AUTH_URL=$NEXT_PUBLIC_BASE_URL
 
 prisma migrate deploy --schema=./prisma/schema.prisma
 node apps/web/server.js