From 1b512fb17ca6bd68ced29ce145c26e98243f71cc Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Tue, 23 Jun 2026 10:08:05 +0300 Subject: [PATCH] fix(auth): preserve refreshed Supabase cookies across middleware redirects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When getUser() refreshes an expired access token it rotates the refresh token and writes the new cookies onto `supabaseResponse` via setAll(). The redirect and 401-JSON branches returned brand-new NextResponse objects that didn't carry those Set-Cookie headers, so the rotated token never reached the browser. The next request replayed the old, now- consumed refresh token, the refresh failed, and the session wedged — users got a broken reload after idling and could only recover by manually clearing cookies. Copy the refreshed cookies onto every response the middleware returns. Fixes #288 Co-Authored-By: Claude Opus 4.8 --- src/middleware.test.ts | 113 +++++++++++++++++++++++++++++++++++++++++ src/middleware.ts | 25 +++++++-- 2 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 src/middleware.test.ts diff --git a/src/middleware.test.ts b/src/middleware.test.ts new file mode 100644 index 0000000..9c1a2e4 --- /dev/null +++ b/src/middleware.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +// --- Scenario knobs the mock reads ----------------------------------------- +// `mockUser` — what getUser() resolves to (a refreshed session ⇒ user, +// or null for the logged-out path). +// `refreshedCookies` — cookies Supabase writes via setAll() during getUser(), +// i.e. the freshly *rotated* auth token. The whole point +// of the test is that these must survive onto whatever +// response the middleware returns — including redirects. +let mockUser: { id: string } | null = null; +let refreshedCookies: Array<{ + name: string; + value: string; + options: Record; +}> = []; + +vi.mock("@supabase/ssr", () => ({ + createServerClient: ( + _url: string, + _key: string, + opts: { + cookies: { setAll: (c: typeof refreshedCookies) => void }; + }, + ) => ({ + auth: { + // Mirrors real auth-js: an expired access token is transparently + // refreshed inside getUser(), which rotates the refresh token and + // pushes the new cookies through setAll() before resolving. + getUser: async () => { + if (refreshedCookies.length) opts.cookies.setAll(refreshedCookies); + return { data: { user: mockUser } }; + }, + }, + }), +})); + +// Imported after the mock is registered. +const { middleware } = await import("./middleware"); + +beforeEach(() => { + process.env.NEXT_PUBLIC_SUPABASE_URL = "https://test.supabase.co"; + process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY = "anon-key"; + mockUser = null; + refreshedCookies = []; +}); + +afterEach(() => vi.clearAllMocks()); + +const ROTATED = { + name: "sb-test-auth-token", + value: "rotated-refresh-token", + options: { path: "/", httpOnly: true }, +}; + +describe("middleware — refreshed auth cookies survive redirects", () => { + it("carries the rotated token when redirecting a signed-in user off /login", async () => { + mockUser = { id: "user-1" }; + refreshedCookies = [ROTATED]; + + const res = await middleware( + new NextRequest("https://app.test/login"), + ); + + // Redirect to /dashboard… + expect(res.status).toBe(307); + expect(res.headers.get("location")).toContain("/dashboard"); + // …and the rotated cookie MUST ride along, otherwise the browser keeps + // replaying the now-consumed refresh token and the session wedges until + // the user manually clears cookies. + expect(res.cookies.get(ROTATED.name)?.value).toBe(ROTATED.value); + }); + + it("carries the rotated token when redirecting an unauth user to /login", async () => { + mockUser = null; + // Even on the logged-out path getUser() may emit cookie writes (e.g. + // clearing a dead session); those must not be dropped on the redirect. + refreshedCookies = [{ ...ROTATED, value: "cleared" }]; + + const res = await middleware( + new NextRequest("https://app.test/dashboard"), + ); + + expect(res.status).toBe(307); + expect(res.headers.get("location")).toContain("/login"); + expect(res.cookies.get(ROTATED.name)?.value).toBe("cleared"); + }); + + it("redirects a signed-in user with an invite token to /join/", async () => { + mockUser = { id: "user-1" }; + refreshedCookies = [ROTATED]; + + const res = await middleware( + new NextRequest("https://app.test/login?invite=abc123"), + ); + + expect(res.headers.get("location")).toContain("/join/abc123"); + expect(res.cookies.get(ROTATED.name)?.value).toBe(ROTATED.value); + }); + + it("passes through (no redirect) for a signed-in user on a protected page", async () => { + mockUser = { id: "user-1" }; + refreshedCookies = [ROTATED]; + + const res = await middleware( + new NextRequest("https://app.test/dashboard"), + ); + + // No redirect — the normal NextResponse.next() already carries cookies. + expect(res.headers.get("location")).toBeNull(); + expect(res.cookies.get(ROTATED.name)?.value).toBe(ROTATED.value); + }); +}); diff --git a/src/middleware.ts b/src/middleware.ts index c8056d3..2f48469 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -25,6 +25,23 @@ export async function middleware(request: NextRequest) { const { data: { user } } = await supabase.auth.getUser() + // getUser() transparently refreshes an expired access token, which + // ROTATES the refresh token and writes the new cookies onto + // `supabaseResponse` via setAll() above. Any response we return in + // place of `supabaseResponse` (every redirect / JSON branch below) + // is a fresh object that does NOT carry those Set-Cookie headers, so + // the rotated token never reaches the browser. The next request then + // replays the old, now-consumed refresh token, the refresh fails, and + // the session wedges — the user gets a broken reload after idling and + // can only recover by manually clearing cookies (issue #288). Copy the + // refreshed cookies onto whatever response we hand back to fix that. + const withRefreshedCookies = (response: T): T => { + supabaseResponse.cookies.getAll().forEach((cookie) => { + response.cookies.set(cookie) + }) + return response + } + // Auth pages - redirect to dashboard if already logged in. // Exception: when an invite token is in the query string we // send the already-signed-in user to /join/ instead so @@ -49,7 +66,7 @@ export async function middleware(request: NextRequest) { url.pathname = '/dashboard' url.search = '' } - return NextResponse.redirect(url) + return withRefreshedCookies(NextResponse.redirect(url)) } // Protected pages - redirect to login if not authenticated @@ -57,13 +74,15 @@ export async function middleware(request: NextRequest) { if (!user && protectedPaths.some(path => request.nextUrl.pathname.startsWith(path))) { const url = request.nextUrl.clone() url.pathname = '/login' - return NextResponse.redirect(url) + return withRefreshedCookies(NextResponse.redirect(url)) } // API routes that need auth (not webhooks) if (!user && request.nextUrl.pathname.startsWith('/api/whatsapp/') && !request.nextUrl.pathname.includes('/webhook')) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return withRefreshedCookies( + NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + ) } return supabaseResponse