Skip to content

Commit 2377201

Browse files
committed
fix(clerk-js): Prevent session cookie removal during offline token refresh
1 parent 06a633e commit 2377201

File tree

3 files changed

+120
-19
lines changed

3 files changed

+120
-19
lines changed

.changeset/three-ads-fold.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+
Fix random sign-outs when the browser temporarily loses network connectivity.

packages/clerk-js/src/core/resources/Session.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { createCheckAuthorization } from '@clerk/shared/authorization';
22
import { isValidBrowserOnline } from '@clerk/shared/browser';
3-
import { ClerkOfflineError, ClerkWebAuthnError, is4xxError, MissingExpiredTokenError } from '@clerk/shared/error';
3+
import {
4+
ClerkOfflineError,
5+
ClerkRuntimeError,
6+
ClerkWebAuthnError,
7+
is4xxError,
8+
MissingExpiredTokenError,
9+
} from '@clerk/shared/error';
410
import {
511
convertJSONToPublicKeyRequestOptions,
612
serializePublicKeyCredentialAssertion,
@@ -435,18 +441,28 @@ export class Session extends BaseResource implements SessionResource {
435441
// Dispatch tokenUpdate only for __session tokens with the session's active organization ID, and not JWT templates
436442
const shouldDispatchTokenUpdate = !template && organizationId === this.lastActiveOrganizationId;
437443

444+
let result: string | null;
445+
438446
if (cacheResult) {
439447
// Proactive refresh is handled by timers scheduled in the cache
440448
// Prefer synchronous read to avoid microtask overhead when token is already resolved
441449
const cachedToken = cacheResult.entry.resolvedToken ?? (await cacheResult.entry.tokenResolver);
442-
if (shouldDispatchTokenUpdate) {
450+
// Don't emit empty token update when offline — preserve session cookie
451+
if (shouldDispatchTokenUpdate && (cachedToken.getRawString() || isValidBrowserOnline())) {
443452
eventBus.emit(events.TokenUpdate, { token: cachedToken });
444453
}
445-
// Return null when raw string is empty to indicate signed-out state
446-
return cachedToken.getRawString() || null;
454+
result = cachedToken.getRawString() || null;
455+
} else {
456+
result = await this.#fetchToken(template, organizationId, tokenId, shouldDispatchTokenUpdate, skipCache);
457+
}
458+
459+
// Throw when offline and no token so retry() in getToken() can fire.
460+
// Without this, _getToken returns null (success) and retry() never calls shouldRetry.
461+
if (result === null && !isValidBrowserOnline()) {
462+
throw new ClerkRuntimeError('Network request failed while offline', { code: 'network_error' });
447463
}
448464

449-
return this.#fetchToken(template, organizationId, tokenId, shouldDispatchTokenUpdate, skipCache);
465+
return result;
450466
}
451467

452468
#createTokenResolver(
@@ -474,6 +490,12 @@ export class Session extends BaseResource implements SessionResource {
474490
return;
475491
}
476492

493+
// Don't dispatch empty tokens when offline — this would cause AuthCookieService
494+
// to remove the session cookie even though the user is still authenticated.
495+
if (!token.getRawString() && !isValidBrowserOnline()) {
496+
return;
497+
}
498+
477499
eventBus.emit(events.TokenUpdate, { token });
478500

479501
if (token.jwt) {
@@ -531,6 +553,12 @@ export class Session extends BaseResource implements SessionResource {
531553
// This allows concurrent calls to continue using the stale token
532554
tokenResolver
533555
.then(token => {
556+
// Don't cache or dispatch empty tokens from offline failures —
557+
// preserve the stale-but-valid token in cache instead.
558+
if (!token.getRawString() && !isValidBrowserOnline()) {
559+
return;
560+
}
561+
534562
// Cache the resolved token for future calls
535563
// Re-register onRefresh to handle the next refresh cycle when this token approaches expiration
536564
SessionTokenCache.set({

packages/clerk-js/src/core/resources/__tests__/Session.test.ts

Lines changed: 82 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,6 @@ describe('Session', () => {
261261

262262
describe('with offline browser and network failure', () => {
263263
beforeEach(() => {
264-
// Use real timers for offline tests to avoid unhandled rejection issues with retry logic
265-
vi.useRealTimers();
266264
Object.defineProperty(window.navigator, 'onLine', {
267265
writable: true,
268266
value: false,
@@ -274,10 +272,9 @@ describe('Session', () => {
274272
writable: true,
275273
value: true,
276274
});
277-
vi.useFakeTimers();
278275
});
279276

280-
it('throws ClerkOfflineError when offline', async () => {
277+
it('throws ClerkOfflineError after retries when offline', async () => {
281278
const session = new Session({
282279
status: 'active',
283280
id: 'session_1',
@@ -292,15 +289,15 @@ describe('Session', () => {
292289
mockNetworkFailedFetch();
293290
BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any;
294291

295-
try {
296-
await session.getToken({ skipCache: true });
297-
expect.fail('Expected ClerkOfflineError to be thrown');
298-
} catch (error) {
299-
expect(ClerkOfflineError.is(error)).toBe(true);
300-
}
292+
const errorPromise = session.getToken({ skipCache: true }).catch(e => e);
293+
294+
await vi.advanceTimersByTimeAsync(60_000);
295+
296+
const error = await errorPromise;
297+
expect(ClerkOfflineError.is(error)).toBe(true);
301298
});
302299

303-
it('throws ClerkOfflineError after fetch fails while offline', async () => {
300+
it('retries 3 times before throwing when offline', async () => {
304301
const session = new Session({
305302
status: 'active',
306303
id: 'session_1',
@@ -315,10 +312,39 @@ describe('Session', () => {
315312
mockNetworkFailedFetch();
316313
BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any;
317314

318-
await expect(session.getToken({ skipCache: true })).rejects.toThrow(ClerkOfflineError);
315+
const errorPromise = session.getToken({ skipCache: true }).catch(e => e);
316+
317+
await vi.advanceTimersByTimeAsync(60_000);
318+
319+
await errorPromise;
319320

320-
// Fetch should have been called at least once
321-
expect(global.fetch).toHaveBeenCalled();
321+
expect(global.fetch).toHaveBeenCalledTimes(4);
322+
});
323+
324+
it('does not emit token:update with an empty token when offline', async () => {
325+
const session = new Session({
326+
status: 'active',
327+
id: 'session_1',
328+
object: 'session',
329+
user: createUser({}),
330+
last_active_organization_id: null,
331+
actor: null,
332+
created_at: new Date().getTime(),
333+
updated_at: new Date().getTime(),
334+
} as SessionJSON);
335+
336+
mockNetworkFailedFetch();
337+
BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any;
338+
339+
const errorPromise = session.getToken({ skipCache: true }).catch(e => e);
340+
await vi.advanceTimersByTimeAsync(60_000);
341+
await errorPromise;
342+
343+
const emptyTokenUpdates = dispatchSpy.mock.calls.filter(
344+
(call: unknown[]) =>
345+
call[0] === 'token:update' && !(call[1] as { token: { getRawString(): string } })?.token?.getRawString(),
346+
);
347+
expect(emptyTokenUpdates).toHaveLength(0);
322348
});
323349
});
324350

@@ -588,6 +614,48 @@ describe('Session', () => {
588614
expect(requestSpy).not.toHaveBeenCalled();
589615
});
590616

617+
it('does not emit token:update with an empty token when background refresh fires while offline', async () => {
618+
BaseResource.clerk = clerkMock();
619+
const requestSpy = BaseResource.clerk.getFapiClient().request as Mock<any>;
620+
621+
const session = new Session({
622+
status: 'active',
623+
id: 'session_1',
624+
object: 'session',
625+
user: createUser({}),
626+
last_active_organization_id: null,
627+
last_active_token: { object: 'token', jwt: mockJwt },
628+
actor: null,
629+
created_at: new Date().getTime(),
630+
updated_at: new Date().getTime(),
631+
} as SessionJSON);
632+
633+
await Promise.resolve();
634+
requestSpy.mockClear();
635+
dispatchSpy.mockClear();
636+
637+
// Go offline before the refresh timer fires
638+
Object.defineProperty(window.navigator, 'onLine', { writable: true, value: false });
639+
mockNetworkFailedFetch();
640+
BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any;
641+
642+
// Advance to trigger the refresh timer (~43s) and let the refresh complete
643+
await vi.advanceTimersByTimeAsync(44 * 1000);
644+
645+
const emptyTokenUpdates = dispatchSpy.mock.calls.filter(
646+
(call: unknown[]) =>
647+
call[0] === 'token:update' && !(call[1] as { token: { getRawString(): string } })?.token?.getRawString(),
648+
);
649+
expect(emptyTokenUpdates).toHaveLength(0);
650+
651+
// Come back online and restore mock
652+
Object.defineProperty(window.navigator, 'onLine', { writable: true, value: true });
653+
BaseResource.clerk = clerkMock();
654+
655+
const token = await session.getToken();
656+
expect(token).toEqual(mockJwt);
657+
});
658+
591659
it('does not make API call when token has plenty of time remaining', async () => {
592660
BaseResource.clerk = clerkMock();
593661
const requestSpy = BaseResource.clerk.getFapiClient().request as Mock<any>;

0 commit comments

Comments
 (0)