fix(nextjs): Skip NextResponse normalization in mergeResponses if pos…

…sible (#2244) * 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 <me@tm.codes> --------- Co-authored-by: Tom Milewski <me@tm.codes>
clerk · Dec 5, 2023 · b892ac6 · b892ac6
1 parent 93d05c8
commit b892ac6
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 2 deletions.
diff --git a/.changeset/old-ads-push.md 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
@@ -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 (
+    <ClerkProvider>
+      {children}
+    </ClerkProvider>
+  )
+}`,
+      )
+      .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 (
+    <Provider>
+      <html lang='en'>
+        <body className={inter.className}>{children}</body>
+      </html>
+    </Provider>
+  );
+}
+      `,
+      )
+      .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
@@ -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
@@ -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);
     });
   }