chore(clerk-js): Reorganize cookies code and fix TokenUpdate event (#3362)

* fix(shared): Change createCookieHandler#.set to return void

* chore(clerk-js): Centralize cookie code into single module

* 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.

* chore(clerk-js): Refactor Clerk.#environment modifier to protected to use type assert

* chore(repo): Add changeset

* chore(clerk-js): Rename SessionCookieService to AuthCookieService for clarity

* chore(clerk-js): Add JSDoc for auth services and cookies

* chore(clerk-js): Rename urlWithAuth to decorateUrlWithDevBrowserToken method of auth service

* fix(clerk-js): Fix isSignedOut() to use both clientUat and clerk.user

This change was applied to keep the existing behaviour:
- clerk.user is used when clerk is loaded
- clientUat cookie is used when clerk is NOT loaded

Author: dimkl

.changeset/shaggy-zebras-attend.md

@@ -0,0 +1,6 @@
+---
+'@clerk/clerk-js': patch
+'@clerk/shared': patch
+---
+
+Re-organize cookie codebase into a central place, fix TokenUpdate event to be triggered on sign-out and drop duplicate event on refreshing token.

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

@@ -1,5 +1,5 @@
+import type { DevBrowser } from '../auth/devBrowser';
 import { Clerk } from '../clerk';
-import type { DevBrowser } from '../devBrowser';
 import type { DisplayConfig } from '../resources/internal';
 import { Client, Environment } from '../resources/internal';
 
@@ -10,7 +10,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(),

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/AuthCookieService.ts

@@ -0,0 +1,155 @@
+import { setDevBrowserJWTInURL } from '@clerk/shared/devBrowser';
+import { is4xxError, isClerkAPIResponseError, isNetworkError } from '@clerk/shared/error';
+import type { Clerk, EnvironmentResource } from '@clerk/types';
+
+import { clerkCoreErrorTokenRefreshFailed, clerkMissingDevBrowserJwt } from '../errors';
+import { eventBus, events } from '../events';
+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(@dimkl): make AuthCookieService singleton since it handles updating cookies using a poller
+// and we need to avoid updating them concurrently.
+/**
+ * The AuthCookieService class is a service responsible to handle
+ * all operations and helpers required in a standard browser context
+ * based on the cookies to remove the dependency between cookies
+ * and auth from the Clerk instance.
+ * This service is responsible to:
+ *   - refresh the session cookie using a poller
+ *   - refresh the session cookie on tab visibility change
+ *   - update the related cookies listening to the `token:update` event
+ *   - initialize auth related cookies for development instances (eg __client_uat, __clerk_db_jwt)
+ *   - cookie setup for production / development instances
+ * It also provides the following helpers:
+ *   - isSignedOut(): check if the current user is signed-out using cookies
+ *   - decorateUrlWithDevBrowserToken(): decorates url with auth related info (eg dev browser jwt)
+ *   - handleUnauthenticatedDevBrowser(): resets dev browser in case of invalid dev browser
+ *   - setEnvironment(): update cookies (eg client_uat) related to environment
+ */
+export class AuthCookieService {
+  private environment: EnvironmentResource | undefined;
+  private poller: SessionCookiePoller | null = null;
+  private clientUat: ClientUatCookieHandler;
+  private sessionCookie: SessionCookieHandler;
+  private devBrowser: DevBrowser;
+
+  constructor(private clerk: Clerk, fapiClient: FapiClient) {
+    // set cookie on token update
+    eventBus.on(events.TokenUpdate, ({ token }) => {
+      this.updateSessionCookie(token && token.getRawString());
+      this.setClientUatCookieForDevelopmentInstances();
+    });
+
+    this.refreshTokenOnVisibilityChange();
+    this.startPollingForToken();
+
+    this.clientUat = createClientUatCookie();
+    this.sessionCookie = createSessionCookie();
+    this.devBrowser = createDevBrowser({
+      frontendApi: clerk.frontendApi,
+      fapiClient,
+    });
+  }
+
+  // TODO(@dimkl): Replace this method call with an event listener to decouple Clerk with setEnvironment
+  public setEnvironment(environment: EnvironmentResource) {
+    this.environment = environment;
+    this.setClientUatCookieForDevelopmentInstances();
+  }
+
+  public isSignedOut() {
+    if (!this.clerk.loaded) {
+      return this.clientUat.get() <= 0;
+    }
+    return !!this.clerk.user;
+  }
+
+  public async setupDevelopment() {
+    await this.devBrowser.setup();
+  }
+
+  public setupProduction() {
+    this.devBrowser.clear();
+  }
+
+  public async handleUnauthenticatedDevBrowser() {
+    this.devBrowser.clear();
+    await this.devBrowser.setup();
+  }
+
+  public decorateUrlWithDevBrowserToken(url: URL): URL {
+    const devBrowserJwt = this.devBrowser.getDevBrowserJWT();
+    if (!devBrowserJwt) {
+      return clerkMissingDevBrowserJwt();
+    }
+
+    return setDevBrowserJWTInURL(url, devBrowserJwt);
+  }
+
+  private startPollingForToken() {
+    if (!this.poller) {
+      this.poller = new SessionCookiePoller();
+    }
+    this.poller.startPollingForSessionToken(() => this.refreshSessionToken());
+  }
+
+  private refreshTokenOnVisibilityChange() {
+    document.addEventListener('visibilitychange', () => {
+      if (document.visibilityState === 'visible') {
+        void this.refreshSessionToken();
+      }
+    });
+  }
+
+  private async refreshSessionToken(): Promise<void> {
+    if (!this.clerk.session) {
+      return;
+    }
+
+    try {
+      await this.clerk.session.getToken();
+    } catch (e) {
+      return this.handleGetTokenError(e);
+    }
+  }
+
+  private updateSessionCookie(token: string | null) {
+    return token ? this.sessionCookie.set(token) : this.sessionCookie.remove();
+  }
+
+  private setClientUatCookieForDevelopmentInstances() {
+    if (this.environment?.isDevelopmentOrStaging() && this.inCustomDevelopmentDomain()) {
+      this.clientUat.set(this.clerk.client);
+    }
+  }
+
+  private inCustomDevelopmentDomain() {
+    const domain = this.clerk.frontendApi.replace('clerk.', '');
+    return !window.location.host.endsWith(domain);
+  }
+
+  private handleGetTokenError(e: any) {
+    //throw if not a clerk error
+    if (!isClerkAPIResponseError(e)) {
+      clerkCoreErrorTokenRefreshFailed(e.message || e);
+    }
+
+    //sign user out if a 4XX error
+    if (is4xxError(e)) {
+      void this.clerk.handleUnauthenticated();
+      return;
+    }
+
+    if (isNetworkError(e)) {
+      return;
+    }
+
+    clerkCoreErrorTokenRefreshFailed(e.toString());
+  }
+}

packages/clerk-js/src/core/services/authentication/SessionCookiePoller.ts packages/clerk-js/src/core/auth/SessionCookiePoller.ts

@@ -1,6 +1,6 @@
 import { createWorkerTimers } from '@clerk/shared';
 
-import { SafeLock } from '../../../utils';
+import { SafeLock } from './safeLock';
 
 const REFRESH_SESSION_TOKEN_LOCK_KEY = 'clerk.lock.refreshSessionToken';
 const INTERVAL_IN_MS = 5 * 1000;

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

@@ -1,5 +1,5 @@
+import type { FapiClient } from '../../fapiClient';
 import { createDevBrowser } from '../devBrowser';
-import type { FapiClient } from '../fapiClient';
 
 type RecursivePartial<T> = {
   [P in keyof T]?: RecursivePartial<T[P]>;