diff --git a/.changeset/new-donkeys-sneeze.md b/.changeset/new-donkeys-sneeze.md new file mode 100644 index 0000000000..3af5a0ea0b --- /dev/null +++ b/.changeset/new-donkeys-sneeze.md @@ -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. diff --git a/packages/clerk-js/bundlewatch.config.json b/packages/clerk-js/bundlewatch.config.json index 8aafacbbc3..0fc1a685a5 100644 --- a/packages/clerk-js/bundlewatch.config.json +++ b/packages/clerk-js/bundlewatch.config.json @@ -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" }, diff --git a/packages/clerk-js/src/core/__tests__/clerk.test.ts b/packages/clerk-js/src/core/__tests__/clerk.test.ts index fd6777fdba..7c6a20bafd 100644 --- a/packages/clerk-js/src/core/__tests__/clerk.test.ts +++ b/packages/clerk-js/src/core/__tests__/clerk.test.ts @@ -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; @@ -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); @@ -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); @@ -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); diff --git a/packages/clerk-js/src/core/auth/AuthCookieService.ts b/packages/clerk-js/src/core/auth/AuthCookieService.ts index 60af8d5100..2887166180 100644 --- a/packages/clerk-js/src/core/auth/AuthCookieService.ts +++ b/packages/clerk-js/src/core/auth/AuthCookieService.ts @@ -56,7 +56,7 @@ export class AuthCookieService { this.setClientUatCookieForDevelopmentInstances(); }); - this.refreshTokenOnVisibilityChange(); + this.refreshTokenOnFocus(); this.startPollingForToken(); this.clientUat = createClientUatCookie(cookieSuffix); @@ -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 { + private async refreshSessionToken({ + updateCookieImmediately = false, + }: { + updateCookieImmediately?: boolean; + } = {}): Promise { 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(); } @@ -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; + } } diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 0597b2420c..7c45a8ba55 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -173,6 +173,7 @@ export class Clerk implements ClerkInterface { #listeners: Array<(emission: Resources) => void> = []; #options: ClerkOptions = {}; #pageLifecycle: ReturnType | null = null; + #touchThrottledUntil = 0; get publishableKey(): string { return this.#publishableKey; @@ -1613,6 +1614,8 @@ export class Clerk implements ClerkInterface { // set in updateClient this.updateEnvironment(environment); + this.#authService.setActiveOrganizationInStorage(); + if (await this.#redirectFAPIInitiatedFlow()) { return false; } @@ -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); } }); @@ -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(); diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts index 00d80b1509..0c3c23ee0d 100644 --- a/packages/clerk-js/src/core/resources/Session.ts +++ b/packages/clerk-js/src/core/resources/Session.ts @@ -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 { @@ -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 diff --git a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts index 7717649a43..6ac27eda98 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts @@ -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', () => { @@ -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', @@ -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(); }); }); @@ -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(() => { @@ -105,7 +163,7 @@ describe('Session', () => { const token = await session.getToken(); - expect(dispatchSpy).toBeCalledTimes(1); + expect(dispatchSpy).toHaveBeenCalledTimes(1); expect(token).toEqual(null); }); }); diff --git a/packages/clerk-js/src/core/resources/__tests__/__snapshots__/Session.test.ts.snap b/packages/clerk-js/src/core/resources/__tests__/__snapshots__/Session.test.ts.snap index 7308764b6b..be99433295 100644 --- a/packages/clerk-js/src/core/resources/__tests__/__snapshots__/Session.test.ts.snap +++ b/packages/clerk-js/src/core/resources/__tests__/__snapshots__/Session.test.ts.snap @@ -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", { diff --git a/packages/clerk-js/src/core/test/fixtures.ts b/packages/clerk-js/src/core/test/fixtures.ts index ae01b4e2b6..8b4168f5b4 100644 --- a/packages/clerk-js/src/core/test/fixtures.ts +++ b/packages/clerk-js/src/core/test/fixtures.ts @@ -1,4 +1,5 @@ import type { + Clerk, EmailAddressJSON, ExternalAccountJSON, OAuthProvider, @@ -241,11 +242,12 @@ export const createSignUp = (signUpParams: Partial = {}) => { } as SignUpJSON; }; -export const clerkMock = () => { +export const clerkMock = (params?: Partial) => { return { getFapiClient: jest.fn().mockReturnValue({ request: jest.fn().mockReturnValue({ payload: { object: 'token', jwt: mockJwt }, status: 200 }), }), + ...params, }; }; diff --git a/packages/clerk-js/src/ui/utils/test/mockHelpers.ts b/packages/clerk-js/src/ui/utils/test/mockHelpers.ts index 944c2b931a..482727622e 100644 --- a/packages/clerk-js/src/ui/utils/test/mockHelpers.ts +++ b/packages/clerk-js/src/ui/utils/test/mockHelpers.ts @@ -13,20 +13,33 @@ type DeepJestMocked = T extends FunctionLike } : T; -const mockProp = (obj: T, k: keyof T) => { +type MockMap = { + [K in { [K in keyof T]: T[K] extends (...args: any[]) => any ? K : never }[keyof T]]?: jest.Mock< + // @ts-expect-error -- the typing seems to be working in practice + T[K] + >; +}; + +const mockProp = (obj: T, k: keyof T, mocks?: MockMap) => { if (typeof obj[k] === 'function') { // @ts-ignore - obj[k] = jest.fn(); + obj[k] = mocks?.[k] ?? jest.fn(); } }; -const mockMethodsOf = | null = any>(obj: T, options?: { exclude: (keyof T)[] }) => { +const mockMethodsOf = | null = any>( + obj: T, + options?: { + exclude: (keyof T)[]; + mocks: MockMap; + }, +) => { if (!obj) { return; } Object.keys(obj) .filter(key => !options?.exclude.includes(key as keyof T)) - .forEach(k => mockProp(obj, k)); + .forEach(k => mockProp(obj, k, options?.mocks)); }; export const mockClerkMethods = (clerk: LoadedClerk): DeepJestMocked => { @@ -36,6 +49,9 @@ export const mockClerkMethods = (clerk: LoadedClerk): DeepJestMocked { mockMethodsOf(session, { exclude: ['checkAuthorization'], + mocks: { + touch: jest.fn(() => Promise.resolve(session)), + }, }); mockMethodsOf(session.user); session.user?.emailAddresses.forEach(m => mockMethodsOf(m)); diff --git a/packages/clerk-js/src/utils/pageLifecycle.ts b/packages/clerk-js/src/utils/pageLifecycle.ts index eae6ee697d..52c51396b6 100644 --- a/packages/clerk-js/src/utils/pageLifecycle.ts +++ b/packages/clerk-js/src/utils/pageLifecycle.ts @@ -16,22 +16,22 @@ const noop = () => { */ export const createPageLifecycle = () => { if (!inBrowser()) { - return { isUnloading: noop, onPageVisible: noop }; + return { onPageFocus: noop }; } const callbackQueue: Record void>> = { - 'visibilitychange:visible': [], + focus: [], }; - document.addEventListener('visibilitychange', () => { + window.addEventListener('focus', () => { if (document.visibilityState === 'visible') { - callbackQueue['visibilitychange:visible'].forEach(cb => cb()); + callbackQueue['focus'].forEach(cb => cb()); } }); - const onPageVisible = (cb: () => void) => { - callbackQueue['visibilitychange:visible'].push(cb); + const onPageFocus = (cb: () => void) => { + callbackQueue['focus'].push(cb); }; - return { onPageVisible }; + return { onPageFocus }; }; diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts index 3d29092b16..b77a0492c4 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -95,5 +95,10 @@ export interface PublicUserData { userId?: string; } -export type GetTokenOptions = { template?: string; leewayInSeconds?: number; skipCache?: boolean }; +export type GetTokenOptions = { + template?: string; + organizationId?: string; + leewayInSeconds?: number; + skipCache?: boolean; +}; export type GetToken = (options?: GetTokenOptions) => Promise;