chore(clerk-js): Refactor SessionCookieService and auth/cookies handlers for consistency

There is also a fix for duplicate TokenUpdate event
and for cleaning the token in sign-out flow.

Author: dimkl

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

@@ -2,12 +2,11 @@ import type { ActiveSessionResource, SignInJSON, SignUpJSON, TokenResource } fro
 import { waitFor } from '@testing-library/dom';
 
 import { mockNativeRuntime } from '../../testUtils';
+import type { DevBrowser } from '../auth/devBrowser';
 import { Clerk } from '../clerk';
-import type { DevBrowser } from '../devBrowser';
 import { eventBus, events } from '../events';
 import type { DisplayConfig, Organization } from '../resources/internal';
 import { BaseResource, Client, EmailLinkErrorCode, Environment, SignIn, SignUp } from '../resources/internal';
-import { SessionCookieService } from '../services';
 import { mockJwt } from '../test/fixtures';
 
 const mockClientFetch = jest.fn();
@@ -17,7 +16,7 @@ jest.mock('../resources/Client');
 jest.mock('../resources/Environment');
 
 // Because Jest, don't ask me why...
-jest.mock('../devBrowser', () => ({
+jest.mock('../auth/devBrowser', () => ({
   createDevBrowser: (): DevBrowser => ({
     clear: jest.fn(),
     setup: jest.fn(),
@@ -158,17 +157,17 @@ describe('Clerk singleton', () => {
       touch: jest.fn(),
       getToken: jest.fn(),
     };
-    let cookieSpy;
+    let evenBusSpy;
 
     beforeEach(() => {
-      cookieSpy = jest.spyOn(SessionCookieService.prototype, 'setAuthCookiesFromSession');
+      evenBusSpy = jest.spyOn(eventBus, 'dispatch');
     });
 
     afterEach(() => {
       mockSession.remove.mockReset();
       mockSession.touch.mockReset();
 
-      cookieSpy?.mockRestore();
+      evenBusSpy?.mockRestore();
       // cleanup global window pollution
       (window as any).__unstable__onBeforeSetActive = null;
       (window as any).__unstable__onAfterSetActive = null;
@@ -183,7 +182,7 @@ describe('Clerk singleton', () => {
       await sut.setActive({ session: null });
       await waitFor(() => {
         expect(mockSession.touch).not.toHaveBeenCalled();
-        expect(cookieSpy).toBeCalledWith(null);
+        expect(evenBusSpy).toBeCalledWith('token:update', { token: null });
       });
     });
 
@@ -194,22 +193,20 @@ describe('Clerk singleton', () => {
       const sut = new Clerk(productionPublishableKey);
       await sut.load();
       await sut.setActive({ session: mockSession as any as ActiveSessionResource });
-      await waitFor(() => {
-        expect(mockSession.touch).toHaveBeenCalled();
-        expect(cookieSpy).toBeCalledWith(mockSession);
-      });
+      expect(mockSession.touch).toHaveBeenCalled();
     });
 
     it('does not call session.touch if Clerk was initialised with touchSession set to false', async () => {
       mockSession.touch.mockReturnValueOnce(Promise.resolve());
       mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));
+      mockSession.getToken.mockResolvedValue('mocked-token');
 
       const sut = new Clerk(productionPublishableKey);
       await sut.load({ touchSession: false });
       await sut.setActive({ session: mockSession as any as ActiveSessionResource });
       await waitFor(() => {
         expect(mockSession.touch).not.toHaveBeenCalled();
-        expect(cookieSpy).toBeCalledWith(mockSession);
+        expect(mockSession.getToken).toBeCalled();
       });
     });
 
@@ -250,6 +247,7 @@ describe('Clerk singleton', () => {
         status: 'active',
         user: {},
         touch: jest.fn(),
+        getToken: jest.fn(),
       };
       mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession, mockSession2] }));
 
@@ -262,9 +260,9 @@ describe('Clerk singleton', () => {
         executionOrder.push('session.touch');
         return Promise.resolve();
       });
-      cookieSpy.mockImplementationOnce(() => {
+      mockSession2.getToken.mockImplementation(() => {
         executionOrder.push('set cookie');
-        return Promise.resolve();
+        return 'mocked-token-2';
       });
       const beforeEmitMock = jest.fn().mockImplementationOnce(() => {
         executionOrder.push('before emit');
@@ -276,7 +274,7 @@ describe('Clerk singleton', () => {
       await waitFor(() => {
         expect(executionOrder).toEqual(['session.touch', 'set cookie', 'before emit']);
         expect(mockSession2.touch).toHaveBeenCalled();
-        expect(cookieSpy).toBeCalledWith(mockSession2);
+        expect(mockSession2.getToken).toHaveBeenCalled();
         expect(beforeEmitMock).toBeCalledWith(mockSession2);
         expect(sut.session).toMatchObject(mockSession2);
       });
@@ -285,7 +283,6 @@ describe('Clerk singleton', () => {
     // TODO: @dimkl include set transitive state
     it('calls with lastActiveOrganizationId session.touch -> set cookie -> before emit -> set accessors with touched session on organization switch', async () => {
       mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));
-
       const sut = new Clerk(productionPublishableKey);
       await sut.load();
 
@@ -295,10 +292,11 @@ describe('Clerk singleton', () => {
         executionOrder.push('session.touch');
         return Promise.resolve();
       });
-      cookieSpy.mockImplementationOnce(() => {
+      mockSession.getToken.mockImplementation(() => {
         executionOrder.push('set cookie');
-        return Promise.resolve();
+        return 'mocked-token';
       });
+
       const beforeEmitMock = jest.fn().mockImplementationOnce(() => {
         executionOrder.push('before emit');
         return Promise.resolve();
@@ -309,8 +307,8 @@ describe('Clerk singleton', () => {
       await waitFor(() => {
         expect(executionOrder).toEqual(['session.touch', 'set cookie', 'before emit']);
         expect(mockSession.touch).toHaveBeenCalled();
+        expect(mockSession.getToken).toHaveBeenCalled();
         expect((mockSession as any as ActiveSessionResource)?.lastActiveOrganizationId).toEqual('org-id');
-        expect(cookieSpy).toBeCalledWith(mockSession);
         expect(beforeEmitMock).toBeCalledWith(mockSession);
         expect(sut.session).toMatchObject(mockSession);
       });
@@ -329,10 +327,6 @@ describe('Clerk singleton', () => {
           executionOrder.push('session.touch');
           return Promise.resolve();
         });
-        cookieSpy.mockImplementationOnce(() => {
-          executionOrder.push('set cookie');
-          return Promise.resolve();
-        });
         const beforeEmitMock = jest.fn().mockImplementationOnce(() => {
           executionOrder.push('before emit');
           return Promise.resolve();
@@ -343,7 +337,7 @@ describe('Clerk singleton', () => {
         expect(executionOrder).toEqual(['session.touch', 'before emit']);
         expect(mockSession.touch).toHaveBeenCalled();
         expect((mockSession as any as ActiveSessionResource)?.lastActiveOrganizationId).toEqual('org-id');
-        expect(cookieSpy).not.toHaveBeenCalled();
+        expect(mockSession.getToken).toBeCalled();
         expect(beforeEmitMock).toBeCalledWith(mockSession);
         expect(sut.session).toMatchObject(mockSession);
       });
@@ -523,7 +517,7 @@ describe('Clerk singleton', () => {
       );
 
       const sut = new Clerk(productionPublishableKey);
-      sut.setActive = jest.fn(({ beforeEmit }) => beforeEmit());
+      sut.setActive = jest.fn(async ({ beforeEmit }) => void (beforeEmit && beforeEmit()));
       sut.navigate = jest.fn();
       await sut.load();
       await sut.signOut({ sessionId: '1', redirectUrl: '/after-sign-out' });

packages/clerk-js/src/core/auth/SessionCookieService.ts

@@ -1,35 +1,74 @@
+import { setDevBrowserJWTInURL } from '@clerk/shared/devBrowser';
 import { is4xxError, isClerkAPIResponseError, isNetworkError } from '@clerk/shared/error';
-import type { Clerk, EnvironmentResource, SessionResource, TokenResource } from '@clerk/types';
+import type { Clerk, EnvironmentResource } from '@clerk/types';
 
-import { inBrowser } from '../../utils';
-import { clerkCoreErrorTokenRefreshFailed } from '../errors';
+import { clerkCoreErrorTokenRefreshFailed, clerkMissingDevBrowserJwt } from '../errors';
 import { eventBus, events } from '../events';
-import { setClientUatCookie } from './cookies/clientUat';
-import { removeSessionCookie, setSessionCookie } from './cookies/session';
+import type { FapiClient } from '../fapiClient';
+import type { ClientUatCookieHandler } from './cookies/clientUat';
+import { createClientUatCookie } from './cookies/clientUat';
+import type { SessionCookieHandler } from './cookies/session';
+import { createSessionCookie } from './cookies/session';
+import type { DevBrowser } from './devBrowser';
+import { createDevBrowser } from './devBrowser';
 import { SessionCookiePoller } from './SessionCookiePoller';
 
+// TODO: make SessionCookieService singleton since it handles updating cookies using a poller
+// and we need to avoid updating them concurrently.
 export class SessionCookieService {
   private environment: EnvironmentResource | undefined;
   private poller: SessionCookiePoller | null = null;
+  private clientUat: ClientUatCookieHandler;
+  private sessionCookie: SessionCookieHandler;
+  private devBrowser: DevBrowser;
 
-  constructor(private clerk: Clerk) {
+  constructor(private clerk: Clerk, fapiClient: FapiClient) {
     // set cookie on token update
     eventBus.on(events.TokenUpdate, ({ token }) => {
-      this.updateSessionCookie(token?.getRawString());
+      this.updateSessionCookie(token && token.getRawString());
+      this.setClientUatCookieForDevelopmentInstances();
     });
 
     this.refreshTokenOnVisibilityChange();
     this.startPollingForToken();
+
+    this.clientUat = createClientUatCookie();
+    this.sessionCookie = createSessionCookie();
+    this.devBrowser = createDevBrowser({
+      frontendApi: clerk.frontendApi,
+      fapiClient,
+    });
   }
 
   public setEnvironment(environment: EnvironmentResource) {
     this.environment = environment;
     this.setClientUatCookieForDevelopmentInstances();
   }
 
-  public async setAuthCookiesFromSession(session: SessionResource | undefined | null): Promise<void> {
-    this.updateSessionCookie(await session?.getToken());
-    this.setClientUatCookieForDevelopmentInstances();
+  public isSignedOut() {
+    return this.clientUat.get() <= 0;
+  }
+
+  public async setupDevelopment() {
+    await this.devBrowser.setup();
+  }
+
+  public setupProduction() {
+    this.devBrowser.clear();
+  }
+
+  public async handleUnauthenticatedDevBrowser() {
+    this.devBrowser.clear();
+    await this.devBrowser.setup();
+  }
+
+  public urlWithAuth(url: URL): URL {
+    const devBrowserJwt = this.devBrowser.getDevBrowserJWT();
+    if (!devBrowserJwt) {
+      return clerkMissingDevBrowserJwt();
+    }
+
+    return setDevBrowserJWTInURL(url, devBrowserJwt);
   }
 
   private startPollingForToken() {
@@ -40,10 +79,6 @@ export class SessionCookieService {
   }
 
   private refreshTokenOnVisibilityChange() {
-    if (!inBrowser()) {
-      return;
-    }
-
     document.addEventListener('visibilitychange', () => {
       if (document.visibilityState === 'visible') {
         void this.refreshSessionToken();
@@ -52,33 +87,24 @@ export class SessionCookieService {
   }
 
   private async refreshSessionToken(): Promise<void> {
-    if (!inBrowser()) {
-      return;
-    }
-
     if (!this.clerk.session) {
       return;
     }
 
     try {
-      this.updateSessionCookie(await this.clerk.session?.getToken());
+      await this.clerk.session.getToken();
     } catch (e) {
       return this.handleGetTokenError(e);
     }
   }
 
-  private updateSessionCookie(token: TokenResource | string | undefined | null) {
-    const rawToken = typeof token === 'string' ? token : token?.getRawString();
-
-    if (rawToken) {
-      return setSessionCookie(rawToken);
-    }
-    return removeSessionCookie();
+  private updateSessionCookie(token: string | null) {
+    return token ? this.sessionCookie.set(token) : this.sessionCookie.remove();
   }
 
   private setClientUatCookieForDevelopmentInstances() {
-    if (this.environment && this.environment.isDevelopmentOrStaging() && this.inCustomDevelopmentDomain()) {
-      setClientUatCookie(this.clerk.client);
+    if (this.environment?.isDevelopmentOrStaging() && this.inCustomDevelopmentDomain()) {
+      this.clientUat.set(this.clerk.client);
     }
   }
 

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

@@ -7,33 +7,45 @@ import { getCookieDomain } from '../getCookieDomain';
 
 const CLIENT_UAT_COOKIE_NAME = '__client_uat';
 
-export const clientUatCookie = createCookieHandler(CLIENT_UAT_COOKIE_NAME);
-
-export const getClientUatCookie = (): number => {
-  return parseInt(clientUatCookie.get() || '0', 10);
+export type ClientUatCookieHandler = {
+  set: (client: ClientResource | undefined) => void;
+  get: () => number;
 };
 
-export const setClientUatCookie = (client: ClientResource | undefined) => {
-  const expires = addYears(Date.now(), 1);
-  const sameSite = inCrossOriginIframe() ? 'None' : 'Strict';
-  const secure = window.location.protocol === 'https:';
-  const domain = getCookieDomain();
-
-  // '0' indicates the user is signed out
-  let val = '0';
-
-  if (client && client.updatedAt && client.activeSessions.length > 0) {
-    // truncate timestamp to seconds, since this is a unix timestamp
-    val = Math.floor(client.updatedAt.getTime() / 1000).toString();
-  }
-
-  // Removes any existing cookies without a domain specified to ensure the change doesn't break existing sessions.
-  clientUatCookie.remove();
-
-  return clientUatCookie.set(val, {
-    expires,
-    sameSite,
-    domain,
-    secure,
-  });
+export const createClientUatCookie = (): ClientUatCookieHandler => {
+  const clientUatCookie = createCookieHandler(CLIENT_UAT_COOKIE_NAME);
+
+  const get = (): number => {
+    return parseInt(clientUatCookie.get() || '0', 10);
+  };
+
+  const set = (client: ClientResource | undefined) => {
+    const expires = addYears(Date.now(), 1);
+    const sameSite = inCrossOriginIframe() ? 'None' : 'Strict';
+    const secure = window.location.protocol === 'https:';
+    const domain = getCookieDomain();
+
+    // '0' indicates the user is signed out
+    let val = '0';
+
+    if (client && client.updatedAt && client.activeSessions.length > 0) {
+      // truncate timestamp to seconds, since this is a unix timestamp
+      val = Math.floor(client.updatedAt.getTime() / 1000).toString();
+    }
+
+    // Removes any existing cookies without a domain specified to ensure the change doesn't break existing sessions.
+    clientUatCookie.remove();
+
+    return clientUatCookie.set(val, {
+      expires,
+      sameSite,
+      domain,
+      secure,
+    });
+  };
+
+  return {
+    set,
+    get,
+  };
 };