diff --git a/.changeset/duplicate-session-cookies.md b/.changeset/duplicate-session-cookies.md new file mode 100644 index 00000000000..77652f37d54 --- /dev/null +++ b/.changeset/duplicate-session-cookies.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Recover from partitioned-cookie startup races by removing stale non-partitioned cookies when partitioned cookies become available. diff --git a/packages/clerk-js/src/core/auth/AuthCookieService.ts b/packages/clerk-js/src/core/auth/AuthCookieService.ts index 3ccc1dd4d38..a1c7dd72068 100644 --- a/packages/clerk-js/src/core/auth/AuthCookieService.ts +++ b/packages/clerk-js/src/core/auth/AuthCookieService.ts @@ -83,11 +83,11 @@ export class AuthCookieService { eventBus.on(events.UserSignOut, () => this.handleSignOut()); - // After Environment resolves, re-write dev browser cookies with correct - // partitioned attributes. Dev browser cookies are initially written before - // Environment is fetched, so they may have stale attributes. + // Environment can resolve after auth cookies are first written. eventBus.on(events.EnvironmentUpdate, () => { this.devBrowser.refreshCookies(); + void this.refreshSessionToken({ updateCookieImmediately: true }); + this.setClientUatCookieForDevelopmentInstances(); }); this.refreshTokenOnFocus(); @@ -266,6 +266,9 @@ export class AuthCookieService { } public setClientUatCookieForDevelopmentInstances() { + if (!this.clerk.client) { + return; + } if (this.instanceType !== 'production' && this.inCustomDevelopmentDomain()) { this.clientUat.set(this.clerk.client); } diff --git a/packages/clerk-js/src/core/auth/__tests__/AuthCookieService.test.ts b/packages/clerk-js/src/core/auth/__tests__/AuthCookieService.test.ts index b85c1552e05..e71356e5a16 100644 --- a/packages/clerk-js/src/core/auth/__tests__/AuthCookieService.test.ts +++ b/packages/clerk-js/src/core/auth/__tests__/AuthCookieService.test.ts @@ -1,11 +1,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { eventBus, events } from '../../events'; +import { Environment } from '../../resources/Environment'; const mocks = vi.hoisted(() => ({ sessionCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn() }, clientUatCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn(() => 0) }, activeContextCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn<() => string | undefined>(() => undefined) }, + devBrowser: { + clear: vi.fn(), + setup: vi.fn(() => Promise.resolve()), + getDevBrowser: vi.fn(() => 'deadbeef'), + refreshCookies: vi.fn(), + }, inCrossOriginIframe: vi.fn(() => false), })); @@ -13,14 +20,7 @@ vi.mock('../cookies/session', () => ({ createSessionCookie: () => mocks.sessionC vi.mock('../cookies/clientUat', () => ({ createClientUatCookie: () => mocks.clientUatCookie })); vi.mock('../cookies/activeContext', () => ({ createActiveContextCookie: () => mocks.activeContextCookie })); vi.mock('../cookieSuffix', () => ({ getCookieSuffix: vi.fn(() => Promise.resolve('suffix')) })); -vi.mock('../devBrowser', () => ({ - createDevBrowser: () => ({ - clear: vi.fn(), - setup: vi.fn(() => Promise.resolve()), - getDevBrowser: vi.fn(() => 'deadbeef'), - refreshCookies: vi.fn(), - }), -})); +vi.mock('../devBrowser', () => ({ createDevBrowser: () => mocks.devBrowser })); vi.mock('@clerk/shared/internal/clerk-js/runtime', async importOriginal => { const actual = await importOriginal>(); return { ...actual, inCrossOriginIframe: () => mocks.inCrossOriginIframe() }; @@ -58,6 +58,7 @@ describe('AuthCookieService session cookie refresh', () => { mocks.inCrossOriginIframe.mockReturnValue(false); mocks.activeContextCookie.get.mockReturnValue(undefined); getToken.mockResolvedValue('fresh-jwt'); + Environment.getInstance().partitionedCookies = false; setFocus(true); setVisibility('visible'); }); @@ -136,4 +137,16 @@ describe('AuthCookieService session cookie refresh', () => { expect(getToken).toHaveBeenCalled(); }); + + it('rewrites the session cookie after partitioned cookies resolve', async () => { + service = await createService(); + getToken.mockResolvedValue('jwt-after-environment'); + Environment.getInstance().partitionedCookies = true; + mocks.sessionCookie.set.mockClear(); + + eventBus.emit(events.EnvironmentUpdate, null); + + await vi.waitFor(() => expect(mocks.sessionCookie.set).toHaveBeenCalledWith('jwt-after-environment')); + expect(mocks.devBrowser.refreshCookies).toHaveBeenCalled(); + }); }); diff --git a/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts b/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts index 82f832dc6f5..d0b1dff6fd6 100644 --- a/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts +++ b/packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts @@ -20,8 +20,8 @@ describe('createClientUatCookie', () => { const mockExpires = new Date('2024-12-31'); const mockDomain = 'test.domain'; const defaultOptions = { usePartitionedCookies: () => false }; - const mockSet = vi.fn(); - const mockRemove = vi.fn(); + const mockSet = vi.fn<(name: string, value: string, attributes?: object) => void>(); + const mockRemove = vi.fn<(name: string, attributes?: object) => void>(); const mockGet = vi.fn(); beforeEach(() => { @@ -32,9 +32,13 @@ describe('createClientUatCookie', () => { (requiresSameSiteNone as ReturnType).mockReturnValue(false); (getCookieDomain as ReturnType).mockReturnValue(mockDomain); (getSecureAttribute as ReturnType).mockReturnValue(true); - (createCookieHandler as ReturnType).mockImplementation(() => ({ - set: mockSet, - remove: mockRemove, + (createCookieHandler as ReturnType).mockImplementation((name: string) => ({ + set: (value: string, attributes?: object) => { + mockSet(name, value, attributes); + }, + remove: (attributes?: object) => { + mockRemove(name, attributes); + }, get: mockGet, })); }); @@ -55,13 +59,14 @@ describe('createClientUatCookie', () => { }); expect(mockSet).toHaveBeenCalledTimes(2); - expect(mockSet).toHaveBeenCalledWith('1704067200', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', { domain: mockDomain, expires: mockExpires, sameSite: 'Strict', secure: true, partitioned: false, }); + expect(mockSet).toHaveBeenCalledWith('__client_uat', '1704067200', expect.any(Object)); }); it('should set cookies with None sameSite in cross-origin context', () => { @@ -73,7 +78,7 @@ describe('createClientUatCookie', () => { signedInSessions: ['session1'], }); - expect(mockSet).toHaveBeenCalledWith('1704067200', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', { domain: mockDomain, expires: mockExpires, sameSite: 'None', @@ -86,7 +91,7 @@ describe('createClientUatCookie', () => { const cookieHandler = createClientUatCookie(mockCookieSuffix, defaultOptions); cookieHandler.set(undefined); - expect(mockSet).toHaveBeenCalledWith('0', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '0', { domain: mockDomain, expires: mockExpires, sameSite: 'Strict', @@ -103,7 +108,7 @@ describe('createClientUatCookie', () => { signedInSessions: [], }); - expect(mockSet).toHaveBeenCalledWith('0', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '0', { domain: mockDomain, expires: mockExpires, sameSite: 'Strict', @@ -139,7 +144,7 @@ describe('createClientUatCookie', () => { signedInSessions: ['session1'], }); - expect(mockSet).toHaveBeenCalledWith('1704067200', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', { domain: mockDomain, expires: mockExpires, sameSite: 'None', @@ -156,7 +161,7 @@ describe('createClientUatCookie', () => { signedInSessions: ['session1'], }); - expect(mockSet).toHaveBeenCalledWith('1704067200', { + expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', { domain: mockDomain, expires: mockExpires, sameSite: 'None', @@ -164,4 +169,65 @@ describe('createClientUatCookie', () => { partitioned: true, }); }); + + it('clears non-partitioned domain variants before writing partitioned cookies', () => { + let usePartitionedCookies = false; + const cookieHandler = createClientUatCookie(mockCookieSuffix, { + usePartitionedCookies: () => usePartitionedCookies, + }); + const client = { + id: 'test-client', + updatedAt: new Date('2024-01-01'), + signedInSessions: ['session1'], + }; + + cookieHandler.set(client); + usePartitionedCookies = true; + mockSet.mockClear(); + mockRemove.mockClear(); + cookieHandler.set(client); + + expect(mockRemove.mock.calls).toEqual([ + ['__client_uat_test-suffix', undefined], + ['__client_uat', undefined], + ['__client_uat_test-suffix', { domain: mockDomain, sameSite: 'Strict', secure: true, partitioned: false }], + ['__client_uat', { domain: mockDomain, sameSite: 'Strict', secure: true, partitioned: false }], + ['__client_uat_test-suffix', { domain: mockDomain, sameSite: 'None', secure: true, partitioned: false }], + ['__client_uat', { domain: mockDomain, sameSite: 'None', secure: true, partitioned: false }], + ]); + expect(mockSet.mock.calls).toEqual([ + [ + '__client_uat_test-suffix', + '1704067200', + { + domain: mockDomain, + expires: mockExpires, + sameSite: 'None', + secure: true, + partitioned: true, + }, + ], + [ + '__client_uat', + '1704067200', + { + domain: mockDomain, + expires: mockExpires, + sameSite: 'None', + secure: true, + partitioned: true, + }, + ], + ]); + const firstInvocationOrder = mockRemove.mock.invocationCallOrder[0]; + expect(mockRemove.mock.invocationCallOrder).toEqual([ + firstInvocationOrder, + firstInvocationOrder + 1, + firstInvocationOrder + 4, + firstInvocationOrder + 5, + firstInvocationOrder + 6, + firstInvocationOrder + 7, + ]); + expect(mockSet.mock.invocationCallOrder).toEqual([firstInvocationOrder + 8, firstInvocationOrder + 9]); + }); }); diff --git a/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts b/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts index 2b418d38a4f..22d686e8872 100644 --- a/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts +++ b/packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts @@ -18,8 +18,8 @@ describe('createSessionCookie', () => { const mockToken = 'test-token'; const mockExpires = new Date('2024-12-31'); const defaultOptions = { usePartitionedCookies: () => false }; - const mockSet = vi.fn(); - const mockRemove = vi.fn(); + const mockSet = vi.fn<(name: string, value: string, attributes?: object) => void>(); + const mockRemove = vi.fn<(name: string, attributes?: object) => void>(); const mockGet = vi.fn(); beforeEach(() => { @@ -29,9 +29,13 @@ describe('createSessionCookie', () => { (inCrossOriginIframe as ReturnType).mockReturnValue(false); (requiresSameSiteNone as ReturnType).mockReturnValue(false); (getSecureAttribute as ReturnType).mockReturnValue(true); - (createCookieHandler as ReturnType).mockImplementation(() => ({ - set: mockSet, - remove: mockRemove, + (createCookieHandler as ReturnType).mockImplementation((name: string) => ({ + set: (value: string, attributes?: object) => { + mockSet(name, value, attributes); + }, + remove: (attributes?: object) => { + mockRemove(name, attributes); + }, get: mockGet, })); }); @@ -48,7 +52,7 @@ describe('createSessionCookie', () => { cookieHandler.set(mockToken); expect(mockSet).toHaveBeenCalledTimes(2); - expect(mockSet).toHaveBeenCalledWith(mockToken, { + expect(mockSet).toHaveBeenCalledWith('__session', mockToken, { expires: mockExpires, sameSite: 'Lax', secure: true, @@ -61,7 +65,7 @@ describe('createSessionCookie', () => { const cookieHandler = createSessionCookie(mockCookieSuffix, defaultOptions); cookieHandler.set(mockToken); - expect(mockSet).toHaveBeenCalledWith(mockToken, { + expect(mockSet).toHaveBeenCalledWith('__session', mockToken, { expires: mockExpires, sameSite: 'None', secure: true, @@ -87,17 +91,17 @@ describe('createSessionCookie', () => { partitioned: false, }; - expect(mockSet).toHaveBeenCalledWith(mockToken, { + expect(mockSet).toHaveBeenCalledWith('__session', mockToken, { expires: mockExpires, sameSite: 'Lax', secure: true, partitioned: false, }); - expect(mockRemove).toHaveBeenCalledWith(expectedAttributes); + expect(mockRemove).toHaveBeenCalledWith('__session', expectedAttributes); expect(mockRemove).toHaveBeenCalledTimes(2); - expect(mockRemove).toHaveBeenNthCalledWith(1, expectedAttributes); - expect(mockRemove).toHaveBeenNthCalledWith(2, expectedAttributes); + expect(mockRemove).toHaveBeenNthCalledWith(1, '__session', expectedAttributes); + expect(mockRemove).toHaveBeenNthCalledWith(2, '__session_test-suffix', expectedAttributes); }); it('should get cookie value from suffixed cookie first, then fallback to non-suffixed', () => { @@ -123,7 +127,7 @@ describe('createSessionCookie', () => { const cookieHandler = createSessionCookie(mockCookieSuffix, defaultOptions); cookieHandler.set(mockToken); - expect(mockSet).toHaveBeenCalledWith(mockToken, { + expect(mockSet).toHaveBeenCalledWith('__session', mockToken, { expires: mockExpires, sameSite: 'None', secure: true, @@ -135,12 +139,62 @@ describe('createSessionCookie', () => { const cookieHandler = createSessionCookie(mockCookieSuffix, { usePartitionedCookies: () => true }); cookieHandler.set(mockToken); - expect(mockRemove).toHaveBeenCalledTimes(2); - expect(mockSet).toHaveBeenCalledWith(mockToken, { + expect(mockRemove).toHaveBeenCalledTimes(4); + expect(mockSet).toHaveBeenCalledWith('__session', mockToken, { expires: mockExpires, sameSite: 'None', secure: true, partitioned: true, }); }); + + it('clears non-partitioned variants before writing partitioned cookies after the environment changes', () => { + let usePartitionedCookies = false; + const cookieHandler = createSessionCookie(mockCookieSuffix, { + usePartitionedCookies: () => usePartitionedCookies, + }); + + cookieHandler.set('non-partitioned-token'); + usePartitionedCookies = true; + mockSet.mockClear(); + mockRemove.mockClear(); + cookieHandler.set('partitioned-token'); + + expect(mockRemove.mock.calls).toEqual([ + ['__session', { sameSite: 'Lax', secure: true, partitioned: false }], + ['__session_test-suffix', { sameSite: 'Lax', secure: true, partitioned: false }], + ['__session', { sameSite: 'None', secure: true, partitioned: false }], + ['__session_test-suffix', { sameSite: 'None', secure: true, partitioned: false }], + ]); + expect(mockSet.mock.calls).toEqual([ + [ + '__session', + 'partitioned-token', + { + expires: mockExpires, + sameSite: 'None', + secure: true, + partitioned: true, + }, + ], + [ + '__session_test-suffix', + 'partitioned-token', + { + expires: mockExpires, + sameSite: 'None', + secure: true, + partitioned: true, + }, + ], + ]); + const firstInvocationOrder = mockRemove.mock.invocationCallOrder[0]; + expect(mockRemove.mock.invocationCallOrder).toEqual([ + firstInvocationOrder, + firstInvocationOrder + 1, + firstInvocationOrder + 2, + firstInvocationOrder + 3, + ]); + expect(mockSet.mock.invocationCallOrder).toEqual([firstInvocationOrder + 4, firstInvocationOrder + 5]); + }); }); diff --git a/packages/clerk-js/src/core/auth/cookies/clientUat.ts b/packages/clerk-js/src/core/auth/cookies/clientUat.ts index 4591f348a77..0187b73be75 100644 --- a/packages/clerk-js/src/core/auth/cookies/clientUat.ts +++ b/packages/clerk-js/src/core/auth/cookies/clientUat.ts @@ -63,6 +63,18 @@ export const createClientUatCookie = ( suffixedClientUatCookie.remove(); clientUatCookie.remove(); + if (partitioned) { + const nonPartitionedCookieAttributes = [ + { domain, sameSite: 'Strict', secure: getSecureAttribute('Strict'), partitioned: false }, + { domain, sameSite: 'None', secure: getSecureAttribute('None'), partitioned: false }, + ] as const; + + for (const attributes of nonPartitionedCookieAttributes) { + suffixedClientUatCookie.remove(attributes); + clientUatCookie.remove(attributes); + } + } + suffixedClientUatCookie.set(val, { domain, expires, partitioned, sameSite, secure }); clientUatCookie.set(val, { domain, expires, partitioned, sameSite, secure }); }; diff --git a/packages/clerk-js/src/core/auth/cookies/session.ts b/packages/clerk-js/src/core/auth/cookies/session.ts index 06a7e9fb2e1..2d49e0115f1 100644 --- a/packages/clerk-js/src/core/auth/cookies/session.ts +++ b/packages/clerk-js/src/core/auth/cookies/session.ts @@ -35,16 +35,25 @@ export const createSessionCookie = (cookieSuffix: string, options: SessionCookie const sessionCookie = createCookieHandler(SESSION_COOKIE_NAME); const suffixedSessionCookie = createCookieHandler(getSuffixedCookieName(SESSION_COOKIE_NAME, cookieSuffix)); + const removeNonPartitionedCookies = () => { + const nonPartitionedCookieAttributes = [ + { sameSite: 'Lax', secure: getSecureAttribute('Lax'), partitioned: false }, + { sameSite: 'None', secure: getSecureAttribute('None'), partitioned: false }, + ] as const; + + for (const attributes of nonPartitionedCookieAttributes) { + sessionCookie.remove(attributes); + suffixedSessionCookie.remove(attributes); + } + }; + const remove = () => { const attributes = getCookieAttributes(options); sessionCookie.remove(attributes); suffixedSessionCookie.remove(attributes); - // Also remove non-partitioned variants — the browser treats partitioned and - // non-partitioned cookies with the same name as distinct cookies. if (attributes.partitioned) { - sessionCookie.remove(); - suffixedSessionCookie.remove(); + removeNonPartitionedCookies(); } }; @@ -52,11 +61,8 @@ export const createSessionCookie = (cookieSuffix: string, options: SessionCookie const expires = addYears(Date.now(), 1); const { sameSite, secure, partitioned } = getCookieAttributes(options); - // Remove old non-partitioned cookies — the browser treats partitioned and - // non-partitioned cookies with the same name as distinct cookies. if (partitioned) { - sessionCookie.remove(); - suffixedSessionCookie.remove(); + removeNonPartitionedCookies(); } sessionCookie.set(token, { expires, sameSite, secure, partitioned });