From 29eab33026ed7a22f4bc6365ea71c087707473f5 Mon Sep 17 00:00:00 2001 From: LekoArts Date: Fri, 1 Dec 2023 08:30:23 +0100 Subject: [PATCH 1/6] fix(nextjs): If res is NextResponse just return it --- packages/nextjs/src/utils/response.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/utils/response.ts b/packages/nextjs/src/utils/response.ts index 760ea6939d..6ad83e148f 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; } From 7ee4e4b7182ae46ab2d2ceadf07de795941116ca Mon Sep 17 00:00:00 2001 From: LekoArts Date: Fri, 1 Dec 2023 08:30:58 +0100 Subject: [PATCH 2/6] fix(nextjs): Support options when setting cookies --- packages/nextjs/src/utils/response.test.ts | 23 ++++++++++++++++++++++ packages/nextjs/src/utils/response.ts | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/utils/response.test.ts b/packages/nextjs/src/utils/response.test.ts index 13f8e5d15d..cf782a4883 100644 --- a/packages/nextjs/src/utils/response.test.ts +++ b/packages/nextjs/src/utils/response.test.ts @@ -29,10 +29,33 @@ 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(); + console.log(response2); + 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 6ad83e148f..14b5943977 100644 --- a/packages/nextjs/src/utils/response.ts +++ b/packages/nextjs/src/utils/response.ts @@ -30,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); }); } From b63d2763f938d52ce53a5114f161a1081b2b5824 Mon Sep 17 00:00:00 2001 From: LekoArts Date: Fri, 1 Dec 2023 11:13:56 +0100 Subject: [PATCH 3/6] chore(repo): Add initial next middleware test --- integration/tests/next-middleware.test.ts | 95 +++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 integration/tests/next-middleware.test.ts diff --git a/integration/tests/next-middleware.test.ts b/integration/tests/next-middleware.test.ts new file mode 100644 index 0000000000..8acb7ed0a0 --- /dev/null +++ b/integration/tests/next-middleware.test.ts @@ -0,0 +1,95 @@ +import { expect, test } from '@playwright/test'; + +import type { Application } from '../models/application'; +import { appConfigs } from '../presets'; + +test.describe('next middleware @nextjs', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + + test.beforeAll(async () => { + app = await appConfigs.next.appRouter + .clone() + .addFile( + 'src/app/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.build(); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('authMiddleware passes through all cookies', () => { + // TODO + }); +}); From 8f05c681542733c3c218183f08fbd595cc5735f3 Mon Sep 17 00:00:00 2001 From: LekoArts Date: Mon, 4 Dec 2023 09:55:22 +0100 Subject: [PATCH 4/6] chore(repo): Make test work --- integration/tests/next-middleware.test.ts | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/integration/tests/next-middleware.test.ts b/integration/tests/next-middleware.test.ts index 8acb7ed0a0..3addbb1a2e 100644 --- a/integration/tests/next-middleware.test.ts +++ b/integration/tests/next-middleware.test.ts @@ -2,6 +2,7 @@ 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' }); @@ -11,7 +12,7 @@ test.describe('next middleware @nextjs', () => { app = await appConfigs.next.appRouter .clone() .addFile( - 'src/app/middleware.ts', + 'src/middleware.ts', () => `import { authMiddleware } from '@clerk/nextjs/server'; import { NextResponse } from "next/server"; @@ -82,14 +83,28 @@ export default function RootLayout({ children }: { children: React.ReactNode }) .commit(); await app.setup(); await app.withEnv(appConfigs.envs.withEmailCodes); - await app.build(); + await app.dev(); }); test.afterAll(async () => { await app.teardown(); }); - test('authMiddleware passes through all cookies', () => { - // TODO + 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(); }); }); From b34f243c6a460735630d2d0a08877b0b3547a54b Mon Sep 17 00:00:00 2001 From: LekoArts Date: Mon, 4 Dec 2023 09:58:00 +0100 Subject: [PATCH 5/6] chore(repo): Add changeset --- .changeset/old-ads-push.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/old-ads-push.md 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). From 18ae827f816304ea13f9c7dee7f75885e42c4eaa Mon Sep 17 00:00:00 2001 From: Lennart Date: Tue, 5 Dec 2023 08:32:43 +0100 Subject: [PATCH 6/6] Update packages/nextjs/src/utils/response.test.ts Co-authored-by: Tom Milewski --- packages/nextjs/src/utils/response.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nextjs/src/utils/response.test.ts b/packages/nextjs/src/utils/response.test.ts index cf782a4883..525b0e01c6 100644 --- a/packages/nextjs/src/utils/response.test.ts +++ b/packages/nextjs/src/utils/response.test.ts @@ -40,7 +40,6 @@ describe('mergeResponses', function () { it('should merge the cookies with non-response values', function () { const response2 = NextResponse.next(); - console.log(response2); response2.cookies.set('foo', '1'); response2.cookies.set({ name: 'second',