diff --git a/src/server/client.ts b/src/server/client.ts index bc39d452..5424538f 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -929,7 +929,29 @@ 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. + // 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 = {}; + 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/session/abstract-session-store.ts b/src/server/session/abstract-session-store.ts index f56dca9e..ab0f963c 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 9c3e1e6c..93d31af1 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( diff --git a/src/server/updateSession-header-fix.test.ts b/src/server/updateSession-header-fix.test.ts new file mode 100644 index 00000000..176e99b9 --- /dev/null +++ b/src/server/updateSession-header-fix.test.ts @@ -0,0 +1,145 @@ +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; + 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", + 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(mockSession); + + // Mock the session store to simulate setting multiple cookies + 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"); + } + ); + }); + + it("should handle multiple set-cookie headers correctly in Pages Router", async () => { + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + ...mockSession, + user: { ...mockSession.user, nickname: "updated_user" } + }); + + // 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) => { + // 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 + } + ); + + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + ...mockSession, + user: { ...mockSession.user, 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; 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 () => { + // Mock session store to set no cookies + vi.spyOn(client["sessionStore"], "set").mockImplementation(async () => { + // Don't set any cookies + }); + + await client.updateSession(mockPagesRouterReq, mockPagesRouterRes, { + ...mockSession, + user: { ...mockSession.user, 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) => { + 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, { + ...mockSession, + user: { ...mockSession.user, 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); + }); +});