fix: create request cookies provider from cloned headers

awinogrodzki · Sep 30, 2024 · d17c376 · d17c376
1 parent f834e4a
commit d17c376
Show file tree

Hide file tree

Showing 10 changed files with 92 additions and 113 deletions.
diff --git a/docs/pages/docs/usage/remove-credentials.mdx b/docs/pages/docs/usage/remove-credentials.mdx
@@ -14,7 +14,7 @@ import {removeCookies} from 'next-firebase-auth-edge/lib/next/cookies';
 function forceLogout(request: NextRequest) {
   const response = NextResponse.redirect(new URL('/login', request.url));
 
-  removeCookies(request.cookies, response, {
+  removeCookies(request.headers, response, {
     cookieName: 'AuthToken',
     cookieSerializeOptions: {
       path: '/',

diff --git a/src/next/cookies/index.test.ts b/src/next/cookies/index.test.ts
@@ -1,5 +1,5 @@
-import {NextResponse} from 'next/server';
 import type {NextRequest} from 'next/server';
+import {NextResponse} from 'next/server';
 import {
   SetAuthCookiesOptions,
   appendAuthCookies,
@@ -46,6 +46,8 @@ describe('cookies', () => {
   let MOCK_REQUEST: jest.Mocked<NextRequest>;
 
   beforeEach(() => {
+    const mockHeaders = new Headers();
+    mockHeaders.set('Cookie', `TestCookie=${jwt}`);
     MOCK_REQUEST = {
       cookies: {
         has: (key: string) => {
@@ -65,9 +67,7 @@ describe('cookies', () => {
         set: jest.fn(),
         delete: jest.fn()
       },
-      headers: {
-        get: jest.fn()
-      }
+      headers: mockHeaders
     } as unknown as jest.Mocked<NextRequest>;
   });
 
@@ -172,8 +172,8 @@ describe('cookies', () => {
         append: jest.fn()
       }
     } as unknown as jest.Mocked<NextResponse>;
-    (MOCK_REQUEST.cookies.get as jest.Mock).mockImplementation(() => undefined);
-    await appendAuthCookies(MOCK_REQUEST.cookies, MOCK_RESPONSE, customTokens, {
+    const mockHeaders = new Headers();
+    await appendAuthCookies(mockHeaders, MOCK_RESPONSE, customTokens, {
       ...MOCK_OPTIONS,
       enableMultipleCookies: true
     });
@@ -211,11 +211,7 @@ describe('cookies', () => {
       }
     } as unknown as jest.Mocked<NextResponse>;
 
-    await setAuthCookies(
-      MOCK_REQUEST.cookies,
-      MOCK_RESPONSE.headers,
-      MOCK_OPTIONS
-    );
+    await setAuthCookies(MOCK_RESPONSE.headers, MOCK_OPTIONS);
 
     expect(MOCK_RESPONSE.headers.get).toHaveBeenCalledWith(
       'Next-Authorization'

diff --git a/src/next/cookies/index.ts b/src/next/cookies/index.ts
@@ -15,23 +15,22 @@ import {CookieRemoverFactory} from './remover/CookieRemoverFactory.js';
 import {CookiesObject, SetAuthCookiesOptions} from './types.js';
 
 export async function appendAuthCookies(
-  cookies: RequestCookies | ReadonlyRequestCookies,
+  headers: Headers,
   response: NextResponse,
   tokens: CustomTokens,
   options: SetAuthCookiesOptions
 ) {
   debug('Updating response headers with authenticated cookies');
 
   const authCookies = new AuthCookies(
-    new RequestCookiesProvider(cookies),
+    RequestCookiesProvider.fromHeaders(headers),
     options
   );
 
   await authCookies.setAuthHeaders(tokens, response.headers);
 }
 
 export async function setAuthCookies(
-  cookies: RequestCookies | ReadonlyRequestCookies,
   headers: Headers,
   options: SetAuthCookiesOptions
 ): Promise<NextResponse> {
@@ -70,7 +69,7 @@ export async function setAuthCookies(
     headers: {'content-type': 'application/json'}
   });
 
-  await appendAuthCookies(cookies, response, customTokens, options);
+  await appendAuthCookies(headers, response, customTokens, options);
 
   return response;
 }
@@ -81,29 +80,29 @@ export interface RemoveAuthCookiesOptions {
 }
 
 export function removeCookies(
-  cookies: RequestCookies | ReadonlyRequestCookies,
+  headers: Headers,
   response: NextResponse,
   options: RemoveAuthCookiesOptions
 ) {
   const remover = CookieRemoverFactory.fromHeaders(
     response.headers,
-    new RequestCookiesProvider(cookies),
+    RequestCookiesProvider.fromHeaders(headers),
     options.cookieName
   );
 
   return remover.removeCookies(options.cookieSerializeOptions);
 }
 
 export function removeAuthCookies(
-  cookies: RequestCookies | ReadonlyRequestCookies,
+  headers: Headers,
   options: RemoveAuthCookiesOptions
 ): NextResponse {
   const response = new NextResponse(JSON.stringify({success: true}), {
     status: 200,
     headers: {'content-type': 'application/json'}
   });
 
-  removeCookies(cookies, response, options);
+  removeCookies(headers, response, options);
 
   debug('Updating response with empty authentication cookie headers', {
     cookieName: options.cookieName
@@ -189,7 +188,7 @@ export async function refreshCredentials(
   );
 
   const cookies = new AuthCookies(
-    new RequestCookiesProvider(request.cookies),
+    RequestCookiesProvider.fromHeaders(request.headers),
     options
   );
   await cookies.setAuthCookies(customTokens, request.cookies);
@@ -229,7 +228,7 @@ export async function refreshNextResponseCookiesWithToken(
     referer
   });
 
-  await appendAuthCookies(request.cookies, response, customTokens, options);
+  await appendAuthCookies(request.headers, response, customTokens, options);
 
   return response;
 }
@@ -255,7 +254,7 @@ export async function refreshCookiesWithIdToken(
   });
 
   const authCookies = new AuthCookies(
-    new RequestCookiesProvider(cookies),
+    RequestCookiesProvider.fromHeaders(headers),
     options
   );
 
@@ -273,7 +272,7 @@ export async function refreshNextResponseCookies(
     options
   );
 
-  await appendAuthCookies(request.cookies, response, customTokens, options);
+  await appendAuthCookies(request.headers, response, customTokens, options);
 
   return response;
 }
@@ -285,7 +284,7 @@ export async function refreshServerCookies(
 ): Promise<void> {
   const customTokens = await refreshNextCookies(cookies, headers, options);
   const authCookies = new AuthCookies(
-    new RequestCookiesProvider(cookies),
+    RequestCookiesProvider.fromHeaders(headers),
     options
   );
 

diff --git a/src/next/cookies/parser/CookieParserFactory.test.ts b/src/next/cookies/parser/CookieParserFactory.test.ts
@@ -1,7 +1,6 @@
-import type {RequestCookies} from 'next/dist/server/web/spec-extension/cookies';
 import {InvalidTokenError, InvalidTokenReason} from '../../../auth/error.ts';
-import {GetCookiesTokensOptions} from '../../tokens.ts';
 import {Cookie} from '../builder/CookieBuilder.js';
+import {GetCookiesTokensOptions} from '../types.ts';
 import {CookieParserFactory} from './CookieParserFactory.js';
 import {MultipleCookiesParser} from './MultipleCookiesParser.ts';
 import {SingleCookieParser} from './SingleCookieParser.ts';
@@ -12,6 +11,12 @@ const testCookie = {
     'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZF90b2tlbiI6ImlkLXRva2VuIiwicmVmcmVzaF90b2tlbiI6InJlZnJlc2gtdG9rZW4iLCJjdXN0b21fdG9rZW4iOiJjdXN0b20tdG9rZW4ifQ.ExxN2rNayg2XCR6WNeZmY8tAyc_qyiZ2YdzITRbQocs'
 };
 
+const testCookieHeader = toHeader(testCookie);
+
+function toHeader(cookie: Cookie): string {
+  return `${cookie.name}=${cookie.value}`;
+}
+
 const testCookies: Cookie[] = [
   {
     name: 'TestCookie.id',
@@ -31,6 +36,8 @@ const testCookies: Cookie[] = [
   }
 ];
 
+const testCookiesHeader = testCookies.map(toHeader).join(';');
+
 const legacyTestCookies: Cookie[] = [
   {
     name: 'TestCookie',
@@ -46,6 +53,8 @@ const legacyTestCookies: Cookie[] = [
   }
 ];
 
+const legacyCookiesHeader = legacyTestCookies.map(toHeader).join(';');
+
 const testCookiesObj = testCookies.reduce(
   (acc, cookie) => ({
     ...acc,
@@ -72,50 +81,18 @@ const mockOptions = {
 } as unknown as GetCookiesTokensOptions;
 
 describe('CookieParserFactory', () => {
-  describe('fromRequestCookies', () => {
-    let mockCookies: jest.Mocked<RequestCookies>;
-
-    beforeEach(() => {
-      mockCookies = {
-        get: jest.fn(),
-        has: jest.fn()
-      } as unknown as jest.Mocked<RequestCookies>;
-    });
-
+  describe('fromHeaders', () => {
     it('should create single cookie parser if request does not have multiple cookies', () => {
-      (mockCookies.get as jest.Mock).mockImplementation((name: string) => {
-        if (name === 'TestCookie') {
-          return testCookie;
-        }
-
-        return undefined;
-      });
-      (mockCookies.has as jest.Mock).mockImplementation((name: string) => {
-        if (name === 'TestCookie') {
-          return true;
-        }
-
-        return false;
-      });
-
-      const result = CookieParserFactory.fromRequestCookies(
-        mockCookies,
-        mockOptions
-      );
+      const mockHeaders = new Headers();
+      mockHeaders.set('Cookie', testCookieHeader);
 
+      const result = CookieParserFactory.fromHeaders(mockHeaders, mockOptions);
       expect(result).toBeInstanceOf(SingleCookieParser);
     });
 
     it('should create single cookie parser if request does not have any cookies', () => {
-      (mockCookies.get as jest.Mock).mockImplementation(() => {
-        return undefined;
-      });
-      (mockCookies.has as jest.Mock).mockImplementation(() => {
-        return false;
-      });
-
-      const result = CookieParserFactory.fromRequestCookies(
-        mockCookies,
+      const result = CookieParserFactory.fromHeaders(
+        new Headers(),
         mockOptions
       );
 
@@ -127,57 +104,31 @@ describe('CookieParserFactory', () => {
     });
 
     it('should create multiple cookie parser if request does have all required cookies', () => {
-      (mockCookies.get as jest.Mock).mockImplementation((name: string) => {
-        return testCookies.find((it) => it.name === name);
-      });
-      (mockCookies.has as jest.Mock).mockImplementation((name: string) => {
-        return testCookies.findIndex((it) => it.name === name) > -1;
-      });
+      const mockHeaders = new Headers();
+      mockHeaders.set('Cookie', testCookiesHeader);
 
-      const result = CookieParserFactory.fromRequestCookies(
-        mockCookies,
-        mockOptions
-      );
+      const result = CookieParserFactory.fromHeaders(mockHeaders, mockOptions);
 
       expect(result).toBeInstanceOf(MultipleCookiesParser);
     });
 
     it('should throw invalid credentials error if deprecated notation is used', () => {
-      (mockCookies.get as jest.Mock).mockImplementation((name: string) => {
-        if (name === 'TestCookie') {
-          return {
-            ...testCookie,
-            value: `${testCookies[0].value}:${testCookies[1].value}`
-          };
-        }
-
-        return undefined;
-      });
-      (mockCookies.has as jest.Mock).mockImplementation((name: string) => {
-        if (name === 'TestCookie') {
-          return true;
-        }
-
-        return false;
-      });
+      const mockHeaders = new Headers();
+      mockHeaders.set(
+        'Cookie',
+        `TestCookie=${testCookies[0].value}:${testCookies[1].value}`
+      );
 
       return expect(() =>
-        CookieParserFactory.fromRequestCookies(mockCookies, mockOptions)
+        CookieParserFactory.fromHeaders(mockHeaders, mockOptions)
       ).toThrow(new InvalidTokenError(InvalidTokenReason.INVALID_CREDENTIALS));
     });
 
     it('should create multiple cookie parser if request has legacy cookies', async () => {
-      (mockCookies.get as jest.Mock).mockImplementation((name: string) => {
-        return legacyTestCookies.find((it) => it.name === name);
-      });
-      (mockCookies.has as jest.Mock).mockImplementation((name: string) => {
-        return legacyTestCookies.findIndex((it) => it.name === name) > -1;
-      });
+      const mockHeaders = new Headers();
+      mockHeaders.set('Cookie', legacyCookiesHeader);
 
-      const parser = CookieParserFactory.fromRequestCookies(
-        mockCookies,
-        mockOptions
-      );
+      const parser = CookieParserFactory.fromHeaders(mockHeaders, mockOptions);
 
       expect(parser).toBeInstanceOf(MultipleCookiesParser);
 

diff --git a/src/next/cookies/parser/CookieParserFactory.ts b/src/next/cookies/parser/CookieParserFactory.ts
@@ -2,8 +2,7 @@ import type {ReadonlyRequestCookies} from 'next/dist/server/web/spec-extension/a
 import type {RequestCookies} from 'next/dist/server/web/spec-extension/cookies';
 import {InvalidTokenError, InvalidTokenReason} from '../../../auth/error.js';
 import {debug} from '../../../debug/index.js';
-import {GetCookiesTokensOptions} from '../types.js';
-import {CookiesObject} from '../types.js';
+import {CookiesObject, GetCookiesTokensOptions} from '../types.js';
 import {CookiesProvider} from './CookiesProvider.js';
 import {MultipleCookiesParser} from './MultipleCookiesParser.js';
 import {ObjectCookiesProvider} from './ObjectCookiesProvider.js';
@@ -101,7 +100,13 @@ export class CookieParserFactory {
     cookies: RequestCookies | ReadonlyRequestCookies,
     options: GetCookiesTokensOptions
   ) {
-    const provider = new RequestCookiesProvider(cookies);
+    const provider = RequestCookiesProvider.fromRequestCookies(cookies);
+
+    return CookieParserFactory.fromProvider(provider, options);
+  }
+
+  static fromHeaders(headers: Headers, options: GetCookiesTokensOptions) {
+    const provider = RequestCookiesProvider.fromHeaders(headers);
 
     return CookieParserFactory.fromProvider(provider, options);
   }

diff --git a/src/next/cookies/parser/RequestCookiesProvider.test.ts b/src/next/cookies/parser/RequestCookiesProvider.test.ts
@@ -0,0 +1,16 @@
+import {RequestCookiesProvider} from './RequestCookiesProvider.js';
+describe('RequestCookiesProvider', () => {
+  it('should copy initial headers', () => {
+    const headers = new Headers();
+    headers.set('Cookie', 'TestCookie=TestToken');
+
+    const provider = RequestCookiesProvider.fromHeaders(headers);
+
+    expect(provider.get('TestCookie')).toEqual('TestToken');
+
+    headers.set('Cookie', 'NewCookie=SomeNewCookie');
+
+    expect(provider.get('TestCookie')).toEqual('TestToken');
+    expect(provider.get('NewCookie')).toEqual(undefined);
+  });
+});