-
Notifications
You must be signed in to change notification settings - Fork 435
Fix updateSession and header overwrite issues
#2330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
75596c0
fix: only set maxAge=0 for legcy cookie if cookies is actually presen…
tusharpandey13 5553955
Merge branch 'main' into debug/updateSession-empty-cookie
tusharpandey13 7cf494b
bugfix: make sure all set-ccokie headers are set; previously, sethead…
tusharpandey13 a09741e
Merge branch 'debug/updateSession-empty-cookie' of https://github.com…
tusharpandey13 23c0623
chore: update tests
tusharpandey13 d5e7ed5
chore: update tests
tusharpandey13 567c946
Merge branch 'main' into debug/updateSession-empty-cookie
tusharpandey13 575f8a9
Merge branch 'main' into debug/updateSession-empty-cookie
tusharpandey13 315b64b
Merge branch 'main' into debug/updateSession-empty-cookie
tusharpandey13 594e0e3
fix: address review comments
tusharpandey13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.