Skip to content

Commit bf1b62a

Browse files
thiskevinwangthiskevinwang
authored
fix(*): recover from duplicate session cookies (#9286)
1 parent 6b24470 commit bf1b62a

7 files changed

Lines changed: 203 additions & 44 deletions

File tree

‎.changeset/duplicate-session-cookies.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Recover from partitioned-cookie startup races by removing stale non-partitioned cookies when partitioned cookies become available.

‎packages/clerk-js/src/core/auth/AuthCookieService.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,11 @@ export class AuthCookieService {
8383

8484
eventBus.on(events.UserSignOut, () => this.handleSignOut());
8585

86-
// After Environment resolves, re-write dev browser cookies with correct
87-
// partitioned attributes. Dev browser cookies are initially written before
88-
// Environment is fetched, so they may have stale attributes.
86+
// Environment can resolve after auth cookies are first written.
8987
eventBus.on(events.EnvironmentUpdate, () => {
9088
this.devBrowser.refreshCookies();
89+
void this.refreshSessionToken({ updateCookieImmediately: true });
90+
this.setClientUatCookieForDevelopmentInstances();
9191
});
9292

9393
this.refreshTokenOnFocus();
@@ -266,6 +266,9 @@ export class AuthCookieService {
266266
}
267267

268268
public setClientUatCookieForDevelopmentInstances() {
269+
if (!this.clerk.client) {
270+
return;
271+
}
269272
if (this.instanceType !== 'production' && this.inCustomDevelopmentDomain()) {
270273
this.clientUat.set(this.clerk.client);
271274
}

‎packages/clerk-js/src/core/auth/__tests__/AuthCookieService.test.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
22

33
import { eventBus, events } from '../../events';
4+
import { Environment } from '../../resources/Environment';
45

56
const mocks = vi.hoisted(() => ({
67
sessionCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn() },
78
clientUatCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn(() => 0) },
89
activeContextCookie: { set: vi.fn(), remove: vi.fn(), get: vi.fn<() => string | undefined>(() => undefined) },
10+
devBrowser: {
11+
clear: vi.fn(),
12+
setup: vi.fn(() => Promise.resolve()),
13+
getDevBrowser: vi.fn(() => 'deadbeef'),
14+
refreshCookies: vi.fn(),
15+
},
916
inCrossOriginIframe: vi.fn(() => false),
1017
}));
1118

1219
vi.mock('../cookies/session', () => ({ createSessionCookie: () => mocks.sessionCookie }));
1320
vi.mock('../cookies/clientUat', () => ({ createClientUatCookie: () => mocks.clientUatCookie }));
1421
vi.mock('../cookies/activeContext', () => ({ createActiveContextCookie: () => mocks.activeContextCookie }));
1522
vi.mock('../cookieSuffix', () => ({ getCookieSuffix: vi.fn(() => Promise.resolve('suffix')) }));
16-
vi.mock('../devBrowser', () => ({
17-
createDevBrowser: () => ({
18-
clear: vi.fn(),
19-
setup: vi.fn(() => Promise.resolve()),
20-
getDevBrowser: vi.fn(() => 'deadbeef'),
21-
refreshCookies: vi.fn(),
22-
}),
23-
}));
23+
vi.mock('../devBrowser', () => ({ createDevBrowser: () => mocks.devBrowser }));
2424
vi.mock('@clerk/shared/internal/clerk-js/runtime', async importOriginal => {
2525
const actual = await importOriginal<Record<string, unknown>>();
2626
return { ...actual, inCrossOriginIframe: () => mocks.inCrossOriginIframe() };
@@ -58,6 +58,7 @@ describe('AuthCookieService session cookie refresh', () => {
5858
mocks.inCrossOriginIframe.mockReturnValue(false);
5959
mocks.activeContextCookie.get.mockReturnValue(undefined);
6060
getToken.mockResolvedValue('fresh-jwt');
61+
Environment.getInstance().partitionedCookies = false;
6162
setFocus(true);
6263
setVisibility('visible');
6364
});
@@ -136,4 +137,16 @@ describe('AuthCookieService session cookie refresh', () => {
136137

137138
expect(getToken).toHaveBeenCalled();
138139
});
140+
141+
it('rewrites the session cookie after partitioned cookies resolve', async () => {
142+
service = await createService();
143+
getToken.mockResolvedValue('jwt-after-environment');
144+
Environment.getInstance().partitionedCookies = true;
145+
mocks.sessionCookie.set.mockClear();
146+
147+
eventBus.emit(events.EnvironmentUpdate, null);
148+
149+
await vi.waitFor(() => expect(mocks.sessionCookie.set).toHaveBeenCalledWith('jwt-after-environment'));
150+
expect(mocks.devBrowser.refreshCookies).toHaveBeenCalled();
151+
});
139152
});

‎packages/clerk-js/src/core/auth/cookies/__tests__/clientUat.test.ts‎

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ describe('createClientUatCookie', () => {
2020
const mockExpires = new Date('2024-12-31');
2121
const mockDomain = 'test.domain';
2222
const defaultOptions = { usePartitionedCookies: () => false };
23-
const mockSet = vi.fn();
24-
const mockRemove = vi.fn();
23+
const mockSet = vi.fn<(name: string, value: string, attributes?: object) => void>();
24+
const mockRemove = vi.fn<(name: string, attributes?: object) => void>();
2525
const mockGet = vi.fn();
2626

2727
beforeEach(() => {
@@ -32,9 +32,13 @@ describe('createClientUatCookie', () => {
3232
(requiresSameSiteNone as ReturnType<typeof vi.fn>).mockReturnValue(false);
3333
(getCookieDomain as ReturnType<typeof vi.fn>).mockReturnValue(mockDomain);
3434
(getSecureAttribute as ReturnType<typeof vi.fn>).mockReturnValue(true);
35-
(createCookieHandler as ReturnType<typeof vi.fn>).mockImplementation(() => ({
36-
set: mockSet,
37-
remove: mockRemove,
35+
(createCookieHandler as ReturnType<typeof vi.fn>).mockImplementation((name: string) => ({
36+
set: (value: string, attributes?: object) => {
37+
mockSet(name, value, attributes);
38+
},
39+
remove: (attributes?: object) => {
40+
mockRemove(name, attributes);
41+
},
3842
get: mockGet,
3943
}));
4044
});
@@ -55,13 +59,14 @@ describe('createClientUatCookie', () => {
5559
});
5660

5761
expect(mockSet).toHaveBeenCalledTimes(2);
58-
expect(mockSet).toHaveBeenCalledWith('1704067200', {
62+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', {
5963
domain: mockDomain,
6064
expires: mockExpires,
6165
sameSite: 'Strict',
6266
secure: true,
6367
partitioned: false,
6468
});
69+
expect(mockSet).toHaveBeenCalledWith('__client_uat', '1704067200', expect.any(Object));
6570
});
6671

6772
it('should set cookies with None sameSite in cross-origin context', () => {
@@ -73,7 +78,7 @@ describe('createClientUatCookie', () => {
7378
signedInSessions: ['session1'],
7479
});
7580

76-
expect(mockSet).toHaveBeenCalledWith('1704067200', {
81+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', {
7782
domain: mockDomain,
7883
expires: mockExpires,
7984
sameSite: 'None',
@@ -86,7 +91,7 @@ describe('createClientUatCookie', () => {
8691
const cookieHandler = createClientUatCookie(mockCookieSuffix, defaultOptions);
8792
cookieHandler.set(undefined);
8893

89-
expect(mockSet).toHaveBeenCalledWith('0', {
94+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '0', {
9095
domain: mockDomain,
9196
expires: mockExpires,
9297
sameSite: 'Strict',
@@ -103,7 +108,7 @@ describe('createClientUatCookie', () => {
103108
signedInSessions: [],
104109
});
105110

106-
expect(mockSet).toHaveBeenCalledWith('0', {
111+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '0', {
107112
domain: mockDomain,
108113
expires: mockExpires,
109114
sameSite: 'Strict',
@@ -139,7 +144,7 @@ describe('createClientUatCookie', () => {
139144
signedInSessions: ['session1'],
140145
});
141146

142-
expect(mockSet).toHaveBeenCalledWith('1704067200', {
147+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', {
143148
domain: mockDomain,
144149
expires: mockExpires,
145150
sameSite: 'None',
@@ -156,12 +161,73 @@ describe('createClientUatCookie', () => {
156161
signedInSessions: ['session1'],
157162
});
158163

159-
expect(mockSet).toHaveBeenCalledWith('1704067200', {
164+
expect(mockSet).toHaveBeenCalledWith('__client_uat_test-suffix', '1704067200', {
160165
domain: mockDomain,
161166
expires: mockExpires,
162167
sameSite: 'None',
163168
secure: true,
164169
partitioned: true,
165170
});
166171
});
172+
173+
it('clears non-partitioned domain variants before writing partitioned cookies', () => {
174+
let usePartitionedCookies = false;
175+
const cookieHandler = createClientUatCookie(mockCookieSuffix, {
176+
usePartitionedCookies: () => usePartitionedCookies,
177+
});
178+
const client = {
179+
id: 'test-client',
180+
updatedAt: new Date('2024-01-01'),
181+
signedInSessions: ['session1'],
182+
};
183+
184+
cookieHandler.set(client);
185+
usePartitionedCookies = true;
186+
mockSet.mockClear();
187+
mockRemove.mockClear();
188+
cookieHandler.set(client);
189+
190+
expect(mockRemove.mock.calls).toEqual([
191+
['__client_uat_test-suffix', undefined],
192+
['__client_uat', undefined],
193+
['__client_uat_test-suffix', { domain: mockDomain, sameSite: 'Strict', secure: true, partitioned: false }],
194+
['__client_uat', { domain: mockDomain, sameSite: 'Strict', secure: true, partitioned: false }],
195+
['__client_uat_test-suffix', { domain: mockDomain, sameSite: 'None', secure: true, partitioned: false }],
196+
['__client_uat', { domain: mockDomain, sameSite: 'None', secure: true, partitioned: false }],
197+
]);
198+
expect(mockSet.mock.calls).toEqual([
199+
[
200+
'__client_uat_test-suffix',
201+
'1704067200',
202+
{
203+
domain: mockDomain,
204+
expires: mockExpires,
205+
sameSite: 'None',
206+
secure: true,
207+
partitioned: true,
208+
},
209+
],
210+
[
211+
'__client_uat',
212+
'1704067200',
213+
{
214+
domain: mockDomain,
215+
expires: mockExpires,
216+
sameSite: 'None',
217+
secure: true,
218+
partitioned: true,
219+
},
220+
],
221+
]);
222+
const firstInvocationOrder = mockRemove.mock.invocationCallOrder[0];
223+
expect(mockRemove.mock.invocationCallOrder).toEqual([
224+
firstInvocationOrder,
225+
firstInvocationOrder + 1,
226+
firstInvocationOrder + 4,
227+
firstInvocationOrder + 5,
228+
firstInvocationOrder + 6,
229+
firstInvocationOrder + 7,
230+
]);
231+
expect(mockSet.mock.invocationCallOrder).toEqual([firstInvocationOrder + 8, firstInvocationOrder + 9]);
232+
});
167233
});

‎packages/clerk-js/src/core/auth/cookies/__tests__/session.test.ts‎

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ describe('createSessionCookie', () => {
1818
const mockToken = 'test-token';
1919
const mockExpires = new Date('2024-12-31');
2020
const defaultOptions = { usePartitionedCookies: () => false };
21-
const mockSet = vi.fn();
22-
const mockRemove = vi.fn();
21+
const mockSet = vi.fn<(name: string, value: string, attributes?: object) => void>();
22+
const mockRemove = vi.fn<(name: string, attributes?: object) => void>();
2323
const mockGet = vi.fn();
2424

2525
beforeEach(() => {
@@ -29,9 +29,13 @@ describe('createSessionCookie', () => {
2929
(inCrossOriginIframe as ReturnType<typeof vi.fn>).mockReturnValue(false);
3030
(requiresSameSiteNone as ReturnType<typeof vi.fn>).mockReturnValue(false);
3131
(getSecureAttribute as ReturnType<typeof vi.fn>).mockReturnValue(true);
32-
(createCookieHandler as ReturnType<typeof vi.fn>).mockImplementation(() => ({
33-
set: mockSet,
34-
remove: mockRemove,
32+
(createCookieHandler as ReturnType<typeof vi.fn>).mockImplementation((name: string) => ({
33+
set: (value: string, attributes?: object) => {
34+
mockSet(name, value, attributes);
35+
},
36+
remove: (attributes?: object) => {
37+
mockRemove(name, attributes);
38+
},
3539
get: mockGet,
3640
}));
3741
});
@@ -48,7 +52,7 @@ describe('createSessionCookie', () => {
4852
cookieHandler.set(mockToken);
4953

5054
expect(mockSet).toHaveBeenCalledTimes(2);
51-
expect(mockSet).toHaveBeenCalledWith(mockToken, {
55+
expect(mockSet).toHaveBeenCalledWith('__session', mockToken, {
5256
expires: mockExpires,
5357
sameSite: 'Lax',
5458
secure: true,
@@ -61,7 +65,7 @@ describe('createSessionCookie', () => {
6165
const cookieHandler = createSessionCookie(mockCookieSuffix, defaultOptions);
6266
cookieHandler.set(mockToken);
6367

64-
expect(mockSet).toHaveBeenCalledWith(mockToken, {
68+
expect(mockSet).toHaveBeenCalledWith('__session', mockToken, {
6569
expires: mockExpires,
6670
sameSite: 'None',
6771
secure: true,
@@ -87,17 +91,17 @@ describe('createSessionCookie', () => {
8791
partitioned: false,
8892
};
8993

90-
expect(mockSet).toHaveBeenCalledWith(mockToken, {
94+
expect(mockSet).toHaveBeenCalledWith('__session', mockToken, {
9195
expires: mockExpires,
9296
sameSite: 'Lax',
9397
secure: true,
9498
partitioned: false,
9599
});
96100

97-
expect(mockRemove).toHaveBeenCalledWith(expectedAttributes);
101+
expect(mockRemove).toHaveBeenCalledWith('__session', expectedAttributes);
98102
expect(mockRemove).toHaveBeenCalledTimes(2);
99-
expect(mockRemove).toHaveBeenNthCalledWith(1, expectedAttributes);
100-
expect(mockRemove).toHaveBeenNthCalledWith(2, expectedAttributes);
103+
expect(mockRemove).toHaveBeenNthCalledWith(1, '__session', expectedAttributes);
104+
expect(mockRemove).toHaveBeenNthCalledWith(2, '__session_test-suffix', expectedAttributes);
101105
});
102106

103107
it('should get cookie value from suffixed cookie first, then fallback to non-suffixed', () => {
@@ -123,7 +127,7 @@ describe('createSessionCookie', () => {
123127
const cookieHandler = createSessionCookie(mockCookieSuffix, defaultOptions);
124128
cookieHandler.set(mockToken);
125129

126-
expect(mockSet).toHaveBeenCalledWith(mockToken, {
130+
expect(mockSet).toHaveBeenCalledWith('__session', mockToken, {
127131
expires: mockExpires,
128132
sameSite: 'None',
129133
secure: true,
@@ -135,12 +139,62 @@ describe('createSessionCookie', () => {
135139
const cookieHandler = createSessionCookie(mockCookieSuffix, { usePartitionedCookies: () => true });
136140
cookieHandler.set(mockToken);
137141

138-
expect(mockRemove).toHaveBeenCalledTimes(2);
139-
expect(mockSet).toHaveBeenCalledWith(mockToken, {
142+
expect(mockRemove).toHaveBeenCalledTimes(4);
143+
expect(mockSet).toHaveBeenCalledWith('__session', mockToken, {
140144
expires: mockExpires,
141145
sameSite: 'None',
142146
secure: true,
143147
partitioned: true,
144148
});
145149
});
150+
151+
it('clears non-partitioned variants before writing partitioned cookies after the environment changes', () => {
152+
let usePartitionedCookies = false;
153+
const cookieHandler = createSessionCookie(mockCookieSuffix, {
154+
usePartitionedCookies: () => usePartitionedCookies,
155+
});
156+
157+
cookieHandler.set('non-partitioned-token');
158+
usePartitionedCookies = true;
159+
mockSet.mockClear();
160+
mockRemove.mockClear();
161+
cookieHandler.set('partitioned-token');
162+
163+
expect(mockRemove.mock.calls).toEqual([
164+
['__session', { sameSite: 'Lax', secure: true, partitioned: false }],
165+
['__session_test-suffix', { sameSite: 'Lax', secure: true, partitioned: false }],
166+
['__session', { sameSite: 'None', secure: true, partitioned: false }],
167+
['__session_test-suffix', { sameSite: 'None', secure: true, partitioned: false }],
168+
]);
169+
expect(mockSet.mock.calls).toEqual([
170+
[
171+
'__session',
172+
'partitioned-token',
173+
{
174+
expires: mockExpires,
175+
sameSite: 'None',
176+
secure: true,
177+
partitioned: true,
178+
},
179+
],
180+
[
181+
'__session_test-suffix',
182+
'partitioned-token',
183+
{
184+
expires: mockExpires,
185+
sameSite: 'None',
186+
secure: true,
187+
partitioned: true,
188+
},
189+
],
190+
]);
191+
const firstInvocationOrder = mockRemove.mock.invocationCallOrder[0];
192+
expect(mockRemove.mock.invocationCallOrder).toEqual([
193+
firstInvocationOrder,
194+
firstInvocationOrder + 1,
195+
firstInvocationOrder + 2,
196+
firstInvocationOrder + 3,
197+
]);
198+
expect(mockSet.mock.invocationCallOrder).toEqual([firstInvocationOrder + 4, firstInvocationOrder + 5]);
199+
});
146200
});

0 commit comments

Comments
 (0)