Skip to content

Commit

Permalink
chore(clerk-js): Reorganize cookies code and fix TokenUpdate event (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
dimkl authored May 15, 2024
1 parent 4d3dc00 commit ec84d51
Show file tree
Hide file tree
Showing 21 changed files with 396 additions and 309 deletions.
6 changes: 6 additions & 0 deletions .changeset/shaggy-zebras-attend.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions packages/clerk-js/src/core/__tests__/clerk.redirects.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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(),
Expand Down
44 changes: 19 additions & 25 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(),
Expand Down Expand Up @@ -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;
Expand All @@ -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 });
});
});

Expand All @@ -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();
});
});

Expand Down Expand Up @@ -250,6 +247,7 @@ describe('Clerk singleton', () => {
status: 'active',
user: {},
touch: jest.fn(),
getToken: jest.fn(),
};
mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession, mockSession2] }));

Expand All @@ -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');
Expand All @@ -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);
});
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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);
});
Expand All @@ -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();
Expand All @@ -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);
});
Expand Down Expand Up @@ -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' });
Expand Down
155 changes: 155 additions & 0 deletions packages/clerk-js/src/core/auth/AuthCookieService.ts
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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]>;
Expand Down
Loading

0 comments on commit ec84d51

Please sign in to comment.