From fc43f129c11743f10b58e87618020218bf17e993 Mon Sep 17 00:00:00 2001 From: Clerk Cookie <136073014+clerk-cookie@users.noreply.github.com> Date: Tue, 5 Dec 2023 11:07:31 +0200 Subject: [PATCH] fix(nextjs): Skip NextResponse normalization in mergeResponses if possible (#2244) (#2260) * fix(nextjs): If res is NextResponse just return it * fix(nextjs): Support options when setting cookies * chore(repo): Add initial next middleware test * chore(repo): Make test work * chore(repo): Add changeset * Update packages/nextjs/src/utils/response.test.ts Co-authored-by: Tom Milewski --------- Co-authored-by: Tom Milewski (cherry picked from commit b892ac6cb8c0e2d5fc159a6ba3151c93716700f6) Co-authored-by: Lennart --- .changeset/old-ads-push.md | 5 + integration/tests/next-middleware.test.ts | 110 +++++++++++++++++++++ packages/nextjs/src/utils/response.test.ts | 22 +++++ packages/nextjs/src/utils/response.ts | 14 ++- 4 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 .changeset/old-ads-push.md create mode 100644 integration/tests/next-middleware.test.ts diff --git a/.changeset/old-ads-push.md b/.changeset/old-ads-push.md new file mode 100644 index 0000000000..d22559ed5b --- /dev/null +++ b/.changeset/old-ads-push.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Ensure that cookies set inside Next.js Middleware are correctly passed through while using [`authMiddleware`](https://clerk.com/docs/references/nextjs/auth-middleware). diff --git a/integration/tests/next-middleware.test.ts b/integration/tests/next-middleware.test.ts new file mode 100644 index 0000000000..3addbb1a2e --- /dev/null +++ b/integration/tests/next-middleware.test.ts @@ -0,0 +1,110 @@ +import { expect, test } from '@playwright/test'; + +import type { Application } from '../models/application'; +import { appConfigs } from '../presets'; +import { createTestUtils } from '../testUtils'; + +test.describe('next middleware @nextjs', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + + test.beforeAll(async () => { + app = await appConfigs.next.appRouter + .clone() + .addFile( + 'src/middleware.ts', + () => `import { authMiddleware } from '@clerk/nextjs/server'; +import { NextResponse } from "next/server"; + +export default authMiddleware({ + publicRoutes: ['/', '/hash/sign-in', '/hash/sign-up'], + afterAuth: async (auth, req) => { + const response = NextResponse.next(); + response.cookies.set({ + name: "first", + value: "123456789", + path: "/", + sameSite: "none", + secure: true, + }); + response.cookies.set("second", "987654321", { + sameSite: "none", + secure: true, + }); + response.cookies.set("third", "foobar", { + sameSite: "none", + secure: true, + }); + return response; + }, +}); + +export const config = { + matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], +};`, + ) + .addFile( + 'src/app/provider.tsx', + () => `'use client' +import { ClerkProvider } from "@clerk/nextjs" + +export function Provider({ children }: { children: any }) { + return ( + + {children} + + ) +}`, + ) + .addFile( + 'src/app/layout.tsx', + () => `import './globals.css'; +import { Inter } from 'next/font/google'; +import { Provider } from './provider'; + +const inter = Inter({ subsets: ['latin'] }); + +export const metadata = { + title: 'Create Next App', + description: 'Generated by create next app', +}; + +export default function RootLayout({ children }: { children: React.ReactNode }) { + return ( + + + {children} + + + ); +} + `, + ) + .commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodes); + await app.dev(); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('authMiddleware passes through all cookies', async ({ browser }) => { + // See https://playwright.dev/docs/api/class-browsercontext + const context = await browser.newContext(); + const page = await context.newPage(); + const u = createTestUtils({ app, page }); + + await page.goto(app.serverUrl); + await u.po.signIn.waitForMounted(); + + const cookies = await context.cookies(); + + expect(cookies.find(c => c.name == 'first').value).toBe('123456789'); + expect(cookies.find(c => c.name == 'second').value).toBe('987654321'); + expect(cookies.find(c => c.name == 'third').value).toBe('foobar'); + + await context.close(); + }); +}); diff --git a/packages/nextjs/src/utils/response.test.ts b/packages/nextjs/src/utils/response.test.ts index 13f8e5d15d..525b0e01c6 100644 --- a/packages/nextjs/src/utils/response.test.ts +++ b/packages/nextjs/src/utils/response.test.ts @@ -29,10 +29,32 @@ describe('mergeResponses', function () { const response1 = new NextResponse(); const response2 = new NextResponse(); response1.cookies.set('foo', '1'); + response1.cookies.set('second', '2'); response1.cookies.set('bar', '1'); response2.cookies.set('bar', '2'); const finalResponse = mergeResponses(response1, response2); expect(finalResponse!.cookies.get('foo')).toEqual(response1.cookies.get('foo')); + expect(finalResponse!.cookies.get('second')).toEqual(response1.cookies.get('second')); + expect(finalResponse!.cookies.get('bar')).toEqual(response2.cookies.get('bar')); + }); + + it('should merge the cookies with non-response values', function () { + const response2 = NextResponse.next(); + response2.cookies.set('foo', '1'); + response2.cookies.set({ + name: 'second', + value: '2', + path: '/', + sameSite: 'none', + secure: true, + }); + response2.cookies.set('bar', '1', { + sameSite: 'none', + secure: true, + }); + const finalResponse = mergeResponses(null, response2); + expect(finalResponse!.cookies.get('foo')).toEqual(response2.cookies.get('foo')); + expect(finalResponse!.cookies.get('second')).toEqual(response2.cookies.get('second')); expect(finalResponse!.cookies.get('bar')).toEqual(response2.cookies.get('bar')); }); diff --git a/packages/nextjs/src/utils/response.ts b/packages/nextjs/src/utils/response.ts index 760ea6939d..14b5943977 100644 --- a/packages/nextjs/src/utils/response.ts +++ b/packages/nextjs/src/utils/response.ts @@ -8,7 +8,15 @@ import { constants as nextConstants } from '../constants'; * but the cookies and headers of all responses are merged. */ export const mergeResponses = (...responses: (NextResponse | Response | null | undefined | void)[]) => { - const normalisedResponses = responses.filter(Boolean).map(res => new NextResponse(res!.body, res!)); + const normalisedResponses = responses.filter(Boolean).map(res => { + // If the response is a NextResponse, we can just return it + if (res instanceof NextResponse) { + return res; + } + + return new NextResponse(res!.body, res!); + }); + if (normalisedResponses.length === 0) { return; } @@ -22,7 +30,9 @@ export const mergeResponses = (...responses: (NextResponse | Response | null | u }); response.cookies.getAll().forEach(cookie => { - finalResponse.cookies.set(cookie.name, cookie.value); + const { name, value, ...options } = cookie; + + finalResponse.cookies.set(name, value, options); }); }