Skip to content

Commit

Permalink
fix(clerk-js): Use focus event to trigger session touch (#3786)
Browse files Browse the repository at this point in the history
Co-authored-by: Nikos Douvlis <[email protected]>
  • Loading branch information
BRKalow and nikosdouvlis authored Aug 22, 2024
1 parent cb64c10 commit edbb1d4
Show file tree
Hide file tree
Showing 12 changed files with 197 additions and 44 deletions.
7 changes: 7 additions & 0 deletions .changeset/new-donkeys-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@clerk/clerk-js": minor
---

Fixes a bug where multiple tabs with different active organizations would not always respect the selected organization. Going forward, when a tab is focused the active organization will immediately be updated to the tab's last active organization.

Additionally, `Clerk.session.getToken()` now accepts an `organizationId` option. The provided organization ID will be used to set organization-related claims in the generated session token.
2 changes: 1 addition & 1 deletion packages/clerk-js/bundlewatch.config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"files": [
{ "path": "./dist/clerk.browser.js", "maxSize": "62kB" },
{ "path": "./dist/clerk.browser.js", "maxSize": "63kB" },
{ "path": "./dist/clerk.headless.js", "maxSize": "43kB" },
{ "path": "./dist/ui-common*.js", "maxSize": "85KB" },
{ "path": "./dist/vendors*.js", "maxSize": "70KB" },
Expand Down
16 changes: 8 additions & 8 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,21 @@ describe('Clerk singleton', () => {
remove: jest.fn(),
status: 'active',
user: {},
touch: jest.fn(),
touch: jest.fn(() => Promise.resolve()),
getToken: jest.fn(),
lastActiveToken: { getRawString: () => 'mocked-token' },
};
let evenBusSpy;
let eventBusSpy;

beforeEach(() => {
evenBusSpy = jest.spyOn(eventBus, 'dispatch');
eventBusSpy = jest.spyOn(eventBus, 'dispatch');
});

afterEach(() => {
mockSession.remove.mockReset();
mockSession.touch.mockReset();

evenBusSpy?.mockRestore();
eventBusSpy?.mockRestore();
// cleanup global window pollution
(window as any).__unstable__onBeforeSetActive = null;
(window as any).__unstable__onAfterSetActive = null;
Expand All @@ -183,12 +183,12 @@ describe('Clerk singleton', () => {
await sut.setActive({ session: null });
await waitFor(() => {
expect(mockSession.touch).not.toHaveBeenCalled();
expect(evenBusSpy).toHaveBeenCalledWith('token:update', { token: null });
expect(eventBusSpy).toHaveBeenCalledWith('token:update', { token: null });
});
});

it('calls session.touch by default', async () => {
mockSession.touch.mockReturnValueOnce(Promise.resolve());
mockSession.touch.mockReturnValue(Promise.resolve());
mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));

const sut = new Clerk(productionPublishableKey);
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('Clerk singleton', () => {
mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));

(window as any).__unstable__onBeforeSetActive = () => {
expect(evenBusSpy).toHaveBeenCalledWith('token:update', { token: null });
expect(eventBusSpy).toHaveBeenCalledWith('token:update', { token: null });
};

const sut = new Clerk(productionPublishableKey);
Expand All @@ -249,7 +249,7 @@ describe('Clerk singleton', () => {
mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));

(window as any).__unstable__onBeforeSetActive = () => {
expect(evenBusSpy).toHaveBeenCalledWith('token:update', { token: mockSession.lastActiveToken });
expect(eventBusSpy).toHaveBeenCalledWith('token:update', { token: mockSession.lastActiveToken });
};

const sut = new Clerk(productionPublishableKey);
Expand Down
58 changes: 52 additions & 6 deletions packages/clerk-js/src/core/auth/AuthCookieService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class AuthCookieService {
this.setClientUatCookieForDevelopmentInstances();
});

this.refreshTokenOnVisibilityChange();
this.refreshTokenOnFocus();
this.startPollingForToken();

this.clientUat = createClientUatCookie(cookieSuffix);
Expand Down Expand Up @@ -110,27 +110,47 @@ export class AuthCookieService {
this.poller.startPollingForSessionToken(() => this.refreshSessionToken());
}

private refreshTokenOnVisibilityChange() {
document.addEventListener('visibilitychange', () => {
private refreshTokenOnFocus() {
window.addEventListener('focus', () => {
if (document.visibilityState === 'visible') {
void this.refreshSessionToken();
// Certain data-fetching libraries that refetch on focus (such as swr) use setTimeout(cb, 0) to schedule a task on the event loop.
// This gives us an opportunity to ensure the session cookie is updated with a fresh token before the fetch occurs, but it needs to
// be done with a microtask. Promises schedule microtasks, and so by using `updateCookieImmediately: true`, we ensure that the cookie
// is updated as part of the scheduled microtask. Our existing event-based mechanism to update the cookie schedules a task, and so the cookie
// is updated too late and not guaranteed to be fresh before the refetch occurs.
void this.refreshSessionToken({ updateCookieImmediately: true });
}
});
}

private async refreshSessionToken(): Promise<void> {
private async refreshSessionToken({
updateCookieImmediately = false,
}: {
updateCookieImmediately?: boolean;
} = {}): Promise<void> {
if (!this.clerk.session) {
return;
}

try {
await this.clerk.session.getToken();
const token = await this.clerk.session.getToken();
if (updateCookieImmediately) {
this.updateSessionCookie(token);
}
} catch (e) {
return this.handleGetTokenError(e);
}
}

private updateSessionCookie(token: string | null) {
// only update session cookie from the active tab,
// or if the tab's selected organization matches the session's active organization
if (!document.hasFocus() && !this.isCurrentOrganizationActive()) {
return;
}

this.setActiveOrganizationInStorage();

return token ? this.sessionCookie.set(token) : this.sessionCookie.remove();
}

Expand Down Expand Up @@ -163,4 +183,30 @@ export class AuthCookieService {

clerkCoreErrorTokenRefreshFailed(e.toString());
}

/**
* The below methods are used to determine whether or not an unfocused tab can be responsible
* for setting the session cookie. A session cookie should only be set by a tab who's selected
* organization matches the session's active organization. By storing the active organization
* ID in local storage, we can check the value across tabs. If a tab's organization ID does not
* match the value in local storage, it is not responsible for updating the session cookie.
*/

public setActiveOrganizationInStorage() {
if (this.clerk.organization?.id) {
localStorage.setItem('clerk_active_org', this.clerk.organization.id);
} else {
localStorage.removeItem('clerk_active_org');
}
}

private isCurrentOrganizationActive() {
const activeOrganizationId = localStorage.getItem('clerk_active_org');

if (!activeOrganizationId && !this.clerk.organization?.id) {
return true;
}

return this.clerk.organization?.id === activeOrganizationId;
}
}
11 changes: 10 additions & 1 deletion packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export class Clerk implements ClerkInterface {
#listeners: Array<(emission: Resources) => void> = [];
#options: ClerkOptions = {};
#pageLifecycle: ReturnType<typeof createPageLifecycle> | null = null;
#touchThrottledUntil = 0;

get publishableKey(): string {
return this.#publishableKey;
Expand Down Expand Up @@ -1613,6 +1614,8 @@ export class Clerk implements ClerkInterface {
// set in updateClient
this.updateEnvironment(environment);

this.#authService.setActiveOrganizationInStorage();

if (await this.#redirectFAPIInitiatedFlow()) {
return false;
}
Expand Down Expand Up @@ -1678,8 +1681,13 @@ export class Clerk implements ClerkInterface {
return;
}

this.#pageLifecycle?.onPageVisible(() => {
this.#pageLifecycle?.onPageFocus(() => {
if (this.session) {
if (this.#touchThrottledUntil > Date.now()) {
return;
}
this.#touchThrottledUntil = Date.now() + 5_000;

void this.#touchLastActiveSession(this.session);
}
});
Expand All @@ -1696,6 +1704,7 @@ export class Clerk implements ClerkInterface {
if (!session || !this.#options.touchSession) {
return Promise.resolve();
}

await session.touch().catch(e => {
if (is4xxError(e)) {
void this.handleUnauthenticated();
Expand Down
26 changes: 18 additions & 8 deletions packages/clerk-js/src/core/resources/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ export class Session extends BaseResource implements SessionResource {
// and retrieve it using the session id concatenated with the jwt template name.
// e.g. session id is 'sess_abc12345' and jwt template name is 'haris'
// The session token ID will be 'sess_abc12345' and the jwt template token ID will be 'sess_abc12345-haris'
#getCacheId(template?: string) {
return `${template ? `${this.id}-${template}` : this.id}-${this.updatedAt.getTime()}`;
#getCacheId(template?: string, organizationId?: string) {
return [this.id, template, organizationId, this.updatedAt.getTime()].filter(Boolean).join('-');
}

protected fromJSON(data: SessionJSON | null): this {
Expand Down Expand Up @@ -151,28 +151,38 @@ export class Session extends BaseResource implements SessionResource {
return null;
}

const { leewayInSeconds, template, skipCache = false } = options || {};
const {
leewayInSeconds,
template,
skipCache = false,
organizationId = Session.clerk.organization?.id,
} = options || {};

if (!template && Number(leewayInSeconds) >= 60) {
throw new Error('Leeway can not exceed the token lifespan (60 seconds)');
}

const tokenId = this.#getCacheId(template);
const tokenId = this.#getCacheId(template, organizationId);
const cachedEntry = skipCache ? undefined : SessionTokenCache.get({ tokenId }, leewayInSeconds);

// Dispatch tokenUpdate only for __session tokens with the session's active organization ID, and not JWT templates
const shouldDispatchTokenUpdate = !template && options?.organizationId === Session.clerk.organization?.id;

if (cachedEntry) {
const cachedToken = await cachedEntry.tokenResolver;
if (!template) {
if (shouldDispatchTokenUpdate) {
eventBus.dispatch(events.TokenUpdate, { token: cachedToken });
}
// Return null when raw string is empty to indicate that there it's signed-out
return cachedToken.getRawString() || null;
}
const path = template ? `${this.path()}/tokens/${template}` : `${this.path()}/tokens`;
const tokenResolver = Token.create(path);
// TODO: update template endpoint to accept organizationId
const params = template ? {} : { ...(organizationId && { organizationId }) };
const tokenResolver = Token.create(path, params);
SessionTokenCache.set({ tokenId, tokenResolver });
return tokenResolver.then(token => {
// Dispatch tokenUpdate only for __session tokens and not JWT templates
if (!template) {
if (shouldDispatchTokenUpdate) {
eventBus.dispatch(events.TokenUpdate, { token });
}
// Return null when raw string is empty to indicate that there it's signed-out
Expand Down
70 changes: 64 additions & 6 deletions packages/clerk-js/src/core/resources/__tests__/Session.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { SessionJSON } from '@clerk/types';
import type { OrganizationJSON, SessionJSON } from '@clerk/types';

import { eventBus } from '../../events';
import { createFapiClient } from '../../fapiClient';
import { clerkMock, createUser, mockDevClerkInstance, mockJwt, mockNetworkFailedFetch } from '../../test/fixtures';
import { BaseResource, Session } from '../internal';
import { BaseResource, Organization, Session } from '../internal';

describe('Session', () => {
describe('creating new session', () => {
Expand All @@ -21,10 +21,11 @@ describe('Session', () => {
global.fetch?.mockClear();
});

it('dispatches token:update event on initilization with lastActiveToken', () => {
it('dispatches token:update event on initialization with lastActiveToken', () => {
new Session({
status: 'active',
id: 'session_1',

object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
Expand All @@ -34,7 +35,7 @@ describe('Session', () => {
updated_at: new Date().getTime(),
} as SessionJSON);

expect(dispatchSpy).toBeCalledTimes(1);
expect(dispatchSpy).toHaveBeenCalledTimes(1);
expect(dispatchSpy.mock.calls[0]).toMatchSnapshot();
});
});
Expand Down Expand Up @@ -66,10 +67,67 @@ describe('Session', () => {

await session.getToken();

expect(dispatchSpy).toBeCalledTimes(1);
expect(dispatchSpy).toHaveBeenCalledTimes(1);
expect(dispatchSpy.mock.calls[0]).toMatchSnapshot();
});

it('does not dispatch token:update if template is provided', async () => {
const session = new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

await session.getToken({ template: 'foobar' });

expect(dispatchSpy).toHaveBeenCalledTimes(0);
});

it('dispatches token:update when provided organization ID matches current active organization', async () => {
BaseResource.clerk = clerkMock({
organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON),
}) as any;
const session = new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

await session.getToken({ organizationId: 'activeOrganization' });

expect(dispatchSpy).toHaveBeenCalledTimes(1);
});

it('does not dispatch token:update when provided organization ID does not match current active organization', async () => {
BaseResource.clerk = clerkMock({
organization: new Organization({ id: 'anotherOrganization' } as OrganizationJSON),
}) as any;
const session = new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

await session.getToken({ organizationId: 'activeOrganization' });

expect(dispatchSpy).toHaveBeenCalledTimes(0);
});

describe('with offline browser and network failure', () => {
let warnSpy;
beforeEach(() => {
Expand Down Expand Up @@ -105,7 +163,7 @@ describe('Session', () => {

const token = await session.getToken();

expect(dispatchSpy).toBeCalledTimes(1);
expect(dispatchSpy).toHaveBeenCalledTimes(1);
expect(token).toEqual(null);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Session creating new session dispatches token:update event on initilization with lastActiveToken 1`] = `
exports[`Session creating new session dispatches token:update event on initialization with lastActiveToken 1`] = `
[
"token:update",
{
Expand Down
Loading

0 comments on commit edbb1d4

Please sign in to comment.