From 75596c0d5068a9fe8a273bc9665ec7062c0b1198 Mon Sep 17 00:00:00 2001 From: Tushar Pandey Date: Mon, 22 Sep 2025 01:11:48 +0530 Subject: [PATCH 1/5] fix: only set maxAge=0 for legcy cookie if cookies is actually present; perviously this was setting a SetCookie header with maxAge=0 for each call to updateSession() --- src/server/session/abstract-session-store.ts | 3 ++- src/server/session/stateful-session-store.ts | 4 ++- .../session/stateless-session-store.test.ts | 18 ++++++++++++- src/server/session/stateless-session-store.ts | 27 +++++++++++-------- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/server/session/abstract-session-store.ts b/src/server/session/abstract-session-store.ts index 5b77521f..806dd4f2 100644 --- a/src/server/session/abstract-session-store.ts +++ b/src/server/session/abstract-session-store.ts @@ -163,7 +163,8 @@ export abstract class AbstractSessionStore { updatedAt + this.inactivityDuration, createdAt + this.absoluteDuration ); - const maxAge = expiresAt - this.epoch(); + // Fix race condition: use the same updatedAt timestamp for consistency + const maxAge = expiresAt - updatedAt; return maxAge > 0 ? maxAge : 0; } diff --git a/src/server/session/stateful-session-store.ts b/src/server/session/stateful-session-store.ts index 2c291b4b..c1c089c2 100644 --- a/src/server/session/stateful-session-store.ts +++ b/src/server/session/stateful-session-store.ts @@ -144,7 +144,9 @@ export class StatefulSessionStore extends AbstractSessionStore { } const maxAge = this.calculateMaxAge(session.internal.createdAt); - const expiration = Date.now() / 1000 + maxAge; + // Use consistent timestamp to avoid race condition - align with calculateMaxAge logic + const now = this.epoch(); + const expiration = now + maxAge; const jwe = await cookies.encrypt( { id: sessionId diff --git a/src/server/session/stateless-session-store.test.ts b/src/server/session/stateless-session-store.test.ts index 8345a48c..0dade670 100644 --- a/src/server/session/stateless-session-store.test.ts +++ b/src/server/session/stateless-session-store.test.ts @@ -439,7 +439,14 @@ describe("Stateless Session Store", async () => { }); vi.spyOn(responseCookies, "set"); - vi.spyOn(requestCookies, "has").mockReturnValue(true); + + // Mock getChunkedCookie to simulate existing legacy cookie + vi.spyOn(cookies, "getChunkedCookie").mockImplementation((name) => { + if (name === LEGACY_COOKIE_NAME) { + return "legacy_session_data"; // Simulate existing legacy cookie + } + return undefined; + }); await sessionStore.set(requestCookies, responseCookies, session); @@ -477,6 +484,15 @@ describe("Stateless Session Store", async () => { }); vi.spyOn(responseCookies, "set"); + + // Mock getChunkedCookie to simulate existing legacy cookie + vi.spyOn(cookies, "getChunkedCookie").mockImplementation((name) => { + if (name === LEGACY_COOKIE_NAME) { + return "legacy_chunked_session_data"; // Simulate existing legacy cookie + } + return undefined; + }); + vi.spyOn(requestCookies, "getAll").mockReturnValue([ { name: `${LEGACY_COOKIE_NAME}.0`, value: "" }, { name: `${LEGACY_COOKIE_NAME}.1`, value: "" } diff --git a/src/server/session/stateless-session-store.ts b/src/server/session/stateless-session-store.ts index 612a49e4..12935351 100644 --- a/src/server/session/stateless-session-store.ts +++ b/src/server/session/stateless-session-store.ts @@ -103,7 +103,9 @@ export class StatelessSessionStore extends AbstractSessionStore { ) { const { connectionTokenSets, ...originalSession } = session; const maxAge = this.calculateMaxAge(session.internal.createdAt); - const expiration = Math.floor(Date.now() / 1000) + maxAge; + // Use consistent timestamp to avoid race condition - align with calculateMaxAge logic + const now = this.epoch(); + const expiration = now + maxAge; const jwe = await cookies.encrypt(originalSession, this.secret, expiration); const cookieValue = jwe.toString(); const options: CookieOptions = { @@ -136,16 +138,19 @@ export class StatelessSessionStore extends AbstractSessionStore { // Any existing v3 cookie can be deleted as soon as we have set a v4 cookie. // In stateless sessions, we do have to ensure we delete all chunks. - cookies.deleteChunkedCookie( - LEGACY_COOKIE_NAME, - reqCookies, - resCookies, - true, - { - domain: this.cookieConfig.domain, - path: this.cookieConfig.path - } - ); + // Only delete legacy cookies if they actually exist in the request. + if (cookies.getChunkedCookie(LEGACY_COOKIE_NAME, reqCookies, true)) { + cookies.deleteChunkedCookie( + LEGACY_COOKIE_NAME, + reqCookies, + resCookies, + true, + { + domain: this.cookieConfig.domain, + path: this.cookieConfig.path + } + ); + } } async delete( From 7cf494b2c0f1545ddcc9c336987317cc582139ca Mon Sep 17 00:00:00 2001 From: Tushar Pandey Date: Tue, 30 Sep 2025 11:53:50 +0530 Subject: [PATCH 2/5] bugfix: make sure all set-ccokie headers are set; previously, setheader() was called with multiple entries of set-ccokie header key, leading to the latest entries overwriting old entries, leading to missing cookies --- src/server/client.ts | 20 +++ src/server/updateSession-header-fix.test.ts | 138 ++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 src/server/updateSession-header-fix.test.ts diff --git a/src/server/client.ts b/src/server/client.ts index 6a3b9c97..6e3f2301 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -707,7 +707,27 @@ export class Auth0Client { } }); + // Handle multiple set-cookie headers properly + // resHeaders.entries() yields each set-cookie header separately, + // but res.setHeader() overwrites previous values. We need to collect + // all set-cookie values and set them as an array. + const setCookieValues: string[] = []; + const otherHeaders: Record = {}; + for (const [key, value] of resHeaders.entries()) { + if (key.toLowerCase() === "set-cookie") { + setCookieValues.push(value); + } else { + otherHeaders[key] = value; + } + } + // Set all cookies at once as an array if any exist + if (setCookieValues.length > 0) { + pagesRouterRes.setHeader("set-cookie", setCookieValues); + } + + // Set non-cookie headers normally + for (const [key, value] of Object.entries(otherHeaders)) { pagesRouterRes.setHeader(key, value); } } diff --git a/src/server/updateSession-header-fix.test.ts b/src/server/updateSession-header-fix.test.ts new file mode 100644 index 00000000..87d9d2aa --- /dev/null +++ b/src/server/updateSession-header-fix.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { Auth0Client } from "./client.js"; + +describe("UpdateSession Header Copying Fix", () => { + let client: Auth0Client; + let mockPagesRouterReq: any; + let mockPagesRouterRes: any; + + beforeEach(() => { + client = new Auth0Client({ + domain: "test.auth0.com", + clientId: "test_client_id", + clientSecret: "test_client_secret", + appBaseUrl: "http://localhost:3000", + secret: "test_secret_key_must_be_long_enough_for_hs256" + }); + + mockPagesRouterReq = { + headers: { + cookie: "appSession=mock_session_cookie" + } + }; + + mockPagesRouterRes = { + headers: {}, + setHeader: vi.fn((key: string, value: any) => { + mockPagesRouterRes.headers[key] = value; + }), + getHeaders: () => mockPagesRouterRes.headers + }; + + // Mock the session store to return a valid session + vi.spyOn(client.sessionStore, "get").mockResolvedValue({ + user: { sub: "test_user", nickname: "test" }, + internal: { iat: Date.now() / 1000, exp: Date.now() / 1000 + 3600 } + }); + + // Mock the session store to simulate setting multiple cookies + vi.spyOn(client.sessionStore, "set").mockImplementation( + async (reqCookies, resCookies, session) => { + // Simulate StatelessSessionStore setting multiple cookies + resCookies.set("appSession", "updated_session_value"); + resCookies.set("appSession.1", "chunk_data_here"); + + // If there's a legacy cookie to delete, simulate that too + if (Math.random() > 0.5) { + // Randomly simulate legacy cookie deletion + resCookies.set("__session", "", { maxAge: 0 }); + } + } + ); + }); + + it("should handle multiple set-cookie headers correctly in Pages Router", async () => { + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + nickname: "updated_user", + test_data: "updated_value" + }); + + // Verify setHeader was called properly + expect(mockPagesRouterRes.setHeader).toHaveBeenCalled(); + + // Check that the set-cookie header was set as an array + const setCookieHeader = mockPagesRouterRes.headers["set-cookie"]; + expect(setCookieHeader).toBeDefined(); + expect(Array.isArray(setCookieHeader)).toBe(true); + + // Verify we have multiple cookies + expect(setCookieHeader.length).toBeGreaterThan(1); + + // Verify the cookies contain expected values + const cookieStrings = setCookieHeader.join(" "); + expect(cookieStrings).toContain("appSession="); + expect(cookieStrings).toContain("appSession.1="); + }); + + it("should preserve all cookies including legacy deletion cookies", async () => { + // Mock session store to definitely include legacy cookie deletion + vi.spyOn(client.sessionStore, "set").mockImplementation( + async (reqCookies, resCookies, session) => { + resCookies.set("appSession", "new_session_value"); + resCookies.set("appSession.1", "chunk_1"); + resCookies.set("appSession.2", "chunk_2"); + resCookies.set("__session", "", { maxAge: 0 }); // Legacy cookie deletion + } + ); + + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + nickname: "test_user_updated" + }); + + const setCookieHeader = mockPagesRouterRes.headers["set-cookie"]; + expect(Array.isArray(setCookieHeader)).toBe(true); + expect(setCookieHeader.length).toBe(4); // All 4 cookies should be preserved + + // Verify specific cookies + const cookieString = setCookieHeader.join(" | "); + expect(cookieString).toContain("appSession=new_session_value"); + expect(cookieString).toContain("appSession.1=chunk_1"); + expect(cookieString).toContain("appSession.2=chunk_2"); + expect(cookieString).toContain("__session=; Max-Age=0"); // Legacy deletion + }); + + it("should not call setHeader for set-cookie if no cookies are set", async () => { + // Mock session store to set no cookies + vi.spyOn(client.sessionStore, "set").mockImplementation(async () => { + // Don't set any cookies + }); + + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + nickname: "test_user" + }); + + // Should not have set any set-cookie header + expect(mockPagesRouterRes.headers["set-cookie"]).toBeUndefined(); + }); + + it("should handle non-cookie headers normally", async () => { + // Mock session store to set both cookies and other headers + vi.spyOn(client.sessionStore, "set").mockImplementation( + async (reqCookies, resCookies, session) => { + resCookies.set("appSession", "test_value"); + // Simulate setting a custom header (this wouldn't normally happen in StatelessSessionStore, but test the logic) + const headers = (resCookies as any).headers || new Headers(); + headers.set("X-Custom-Header", "test-value"); + } + ); + + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + nickname: "test_user" + }); + + // Should have both the cookie array and the custom header + expect(Array.isArray(mockPagesRouterRes.headers["set-cookie"])).toBe(true); + expect(mockPagesRouterRes.headers["set-cookie"].length).toBe(1); + }); +}); From 23c0623a615b23986fdd591a6fe7c6e4a191586b Mon Sep 17 00:00:00 2001 From: Tushar Pandey Date: Tue, 30 Sep 2025 12:17:43 +0530 Subject: [PATCH 3/5] chore: update tests --- src/server/updateSession-header-fix.test.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/server/updateSession-header-fix.test.ts b/src/server/updateSession-header-fix.test.ts index 87d9d2aa..b01f0e68 100644 --- a/src/server/updateSession-header-fix.test.ts +++ b/src/server/updateSession-header-fix.test.ts @@ -79,10 +79,11 @@ describe("UpdateSession Header Copying Fix", () => { // Mock session store to definitely include legacy cookie deletion vi.spyOn(client.sessionStore, "set").mockImplementation( async (reqCookies, resCookies, session) => { - resCookies.set("appSession", "new_session_value"); - resCookies.set("appSession.1", "chunk_1"); - resCookies.set("appSession.2", "chunk_2"); - resCookies.set("__session", "", { maxAge: 0 }); // Legacy cookie deletion + // All cookies should have consistent path from cookieConfig (default: "/") + resCookies.set("appSession", "new_session_value", { path: "/" }); + resCookies.set("appSession.1", "chunk_1", { path: "/" }); + resCookies.set("appSession.2", "chunk_2", { path: "/" }); + resCookies.set("__session", "", { maxAge: 0, path: "/" }); // Legacy cookie deletion } ); @@ -96,10 +97,12 @@ describe("UpdateSession Header Copying Fix", () => { // Verify specific cookies const cookieString = setCookieHeader.join(" | "); - expect(cookieString).toContain("appSession=new_session_value"); - expect(cookieString).toContain("appSession.1=chunk_1"); - expect(cookieString).toContain("appSession.2=chunk_2"); - expect(cookieString).toContain("__session=; Max-Age=0"); // Legacy deletion + console.log(cookieString) + expect(cookieString).toContain("appSession=new_session_value; Path=/"); + expect(cookieString).toContain("appSession.1=chunk_1; Path=/"); + expect(cookieString).toContain("appSession.2=chunk_2; Path=/"); + // __session=; Path=/; Max-Age=0 + expect(cookieString).toContain("__session=; Path=/; Max-Age=0"); // Legacy deletion }); it("should not call setHeader for set-cookie if no cookies are set", async () => { From d5e7ed57ae2786d1667aeb0e0fae429e4a6c5c96 Mon Sep 17 00:00:00 2001 From: Tushar Pandey Date: Tue, 30 Sep 2025 12:28:56 +0530 Subject: [PATCH 4/5] chore: update tests --- src/server/updateSession-header-fix.test.ts | 43 +++++++++++++-------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/server/updateSession-header-fix.test.ts b/src/server/updateSession-header-fix.test.ts index b01f0e68..95fdf192 100644 --- a/src/server/updateSession-header-fix.test.ts +++ b/src/server/updateSession-header-fix.test.ts @@ -6,8 +6,19 @@ describe("UpdateSession Header Copying Fix", () => { let client: Auth0Client; let mockPagesRouterReq: any; let mockPagesRouterRes: any; + let mockSession: any; beforeEach(() => { + // Create a mock session that matches SessionData structure + mockSession = { + user: { sub: "test_user", nickname: "test" }, + tokenSet: { + accessToken: "test_token", + expiresAt: Date.now() / 1000 + 3600 + }, + internal: { sid: "test_session_id", createdAt: Date.now() / 1000 } + }; + client = new Auth0Client({ domain: "test.auth0.com", clientId: "test_client_id", @@ -31,14 +42,11 @@ describe("UpdateSession Header Copying Fix", () => { }; // Mock the session store to return a valid session - vi.spyOn(client.sessionStore, "get").mockResolvedValue({ - user: { sub: "test_user", nickname: "test" }, - internal: { iat: Date.now() / 1000, exp: Date.now() / 1000 + 3600 } - }); + vi.spyOn(client["sessionStore"], "get").mockResolvedValue(mockSession); // Mock the session store to simulate setting multiple cookies - vi.spyOn(client.sessionStore, "set").mockImplementation( - async (reqCookies, resCookies, session) => { + vi.spyOn(client["sessionStore"], "set").mockImplementation( + async (_reqCookies, resCookies) => { // Simulate StatelessSessionStore setting multiple cookies resCookies.set("appSession", "updated_session_value"); resCookies.set("appSession.1", "chunk_data_here"); @@ -54,7 +62,8 @@ describe("UpdateSession Header Copying Fix", () => { it("should handle multiple set-cookie headers correctly in Pages Router", async () => { await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { - nickname: "updated_user", + ...mockSession, + user: { ...mockSession.user, nickname: "updated_user" }, test_data: "updated_value" }); @@ -77,8 +86,8 @@ describe("UpdateSession Header Copying Fix", () => { it("should preserve all cookies including legacy deletion cookies", async () => { // Mock session store to definitely include legacy cookie deletion - vi.spyOn(client.sessionStore, "set").mockImplementation( - async (reqCookies, resCookies, session) => { + vi.spyOn(client["sessionStore"], "set").mockImplementation( + async (_reqCookies, resCookies) => { // All cookies should have consistent path from cookieConfig (default: "/") resCookies.set("appSession", "new_session_value", { path: "/" }); resCookies.set("appSession.1", "chunk_1", { path: "/" }); @@ -88,7 +97,8 @@ describe("UpdateSession Header Copying Fix", () => { ); await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { - nickname: "test_user_updated" + ...mockSession, + user: { ...mockSession.user, nickname: "test_user_updated" } }); const setCookieHeader = mockPagesRouterRes.headers["set-cookie"]; @@ -97,7 +107,6 @@ describe("UpdateSession Header Copying Fix", () => { // Verify specific cookies const cookieString = setCookieHeader.join(" | "); - console.log(cookieString) expect(cookieString).toContain("appSession=new_session_value; Path=/"); expect(cookieString).toContain("appSession.1=chunk_1; Path=/"); expect(cookieString).toContain("appSession.2=chunk_2; Path=/"); @@ -107,12 +116,13 @@ describe("UpdateSession Header Copying Fix", () => { it("should not call setHeader for set-cookie if no cookies are set", async () => { // Mock session store to set no cookies - vi.spyOn(client.sessionStore, "set").mockImplementation(async () => { + vi.spyOn(client["sessionStore"], "set").mockImplementation(async () => { // Don't set any cookies }); await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { - nickname: "test_user" + ...mockSession, + user: { ...mockSession.user, nickname: "test_user" } }); // Should not have set any set-cookie header @@ -121,8 +131,8 @@ describe("UpdateSession Header Copying Fix", () => { it("should handle non-cookie headers normally", async () => { // Mock session store to set both cookies and other headers - vi.spyOn(client.sessionStore, "set").mockImplementation( - async (reqCookies, resCookies, session) => { + vi.spyOn(client["sessionStore"], "set").mockImplementation( + async (_reqCookies, resCookies) => { resCookies.set("appSession", "test_value"); // Simulate setting a custom header (this wouldn't normally happen in StatelessSessionStore, but test the logic) const headers = (resCookies as any).headers || new Headers(); @@ -131,7 +141,8 @@ describe("UpdateSession Header Copying Fix", () => { ); await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { - nickname: "test_user" + ...mockSession, + user: { ...mockSession.user, nickname: "test_user" } }); // Should have both the cookie array and the custom header From 594e0e3349a7030f806cf98b06ebc8edaf3ec054 Mon Sep 17 00:00:00 2001 From: Tushar Pandey Date: Wed, 29 Oct 2025 12:17:02 +0530 Subject: [PATCH 5/5] fix: address review comments --- src/server/client.ts | 2 ++ src/server/updateSession-header-fix.test.ts | 9 +-------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index 940c72d3..5424538f 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -933,6 +933,8 @@ export class Auth0Client { // resHeaders.entries() yields each set-cookie header separately, // but res.setHeader() overwrites previous values. We need to collect // all set-cookie values and set them as an array. + // Note: Per the Web API specification, the Headers API normalizes header names + // to lowercase, so comparing key.toLowerCase() === "set-cookie" is safe. const setCookieValues: string[] = []; const otherHeaders: Record = {}; diff --git a/src/server/updateSession-header-fix.test.ts b/src/server/updateSession-header-fix.test.ts index 95fdf192..176e99b9 100644 --- a/src/server/updateSession-header-fix.test.ts +++ b/src/server/updateSession-header-fix.test.ts @@ -50,12 +50,6 @@ describe("UpdateSession Header Copying Fix", () => { // Simulate StatelessSessionStore setting multiple cookies resCookies.set("appSession", "updated_session_value"); resCookies.set("appSession.1", "chunk_data_here"); - - // If there's a legacy cookie to delete, simulate that too - if (Math.random() > 0.5) { - // Randomly simulate legacy cookie deletion - resCookies.set("__session", "", { maxAge: 0 }); - } } ); }); @@ -63,8 +57,7 @@ describe("UpdateSession Header Copying Fix", () => { it("should handle multiple set-cookie headers correctly in Pages Router", async () => { await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { ...mockSession, - user: { ...mockSession.user, nickname: "updated_user" }, - test_data: "updated_value" + user: { ...mockSession.user, nickname: "updated_user" } }); // Verify setHeader was called properly