diff --git a/.changeset/calm-codes-share.md b/.changeset/calm-codes-share.md new file mode 100644 index 00000000000..dbe6ed42efd --- /dev/null +++ b/.changeset/calm-codes-share.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +Prevent overlapping verification preparations from triggering duplicate verification sends during sign-in, sign-up, and when verifying an email address, phone number, or web3 wallet from the user profile. diff --git a/packages/clerk-js/src/core/resources/Base.ts b/packages/clerk-js/src/core/resources/Base.ts index 3fa1dacd6d4..4ad63d5d01c 100644 --- a/packages/clerk-js/src/core/resources/Base.ts +++ b/packages/clerk-js/src/core/resources/Base.ts @@ -25,8 +25,18 @@ export type BaseFetchOptions = ClerkResourceReloadParams & { export type BaseMutateParams = { action?: string; body?: any; + coalesce?: boolean; method?: HTTPMethod; path?: string; + signal?: AbortSignal; +}; + +const COALESCED_POST_TTL_MS = 30_000; + +type PendingCoalescedPost = { + promise: Promise; + controller: AbortController; + expiresAt: number; }; function assertProductionKeysOnDev(statusCode: number, payloadErrors?: ClerkAPIErrorJSON[]) { @@ -58,6 +68,8 @@ export abstract class BaseResource { id?: string; pathRoot = ''; + #pendingCoalescedPosts?: Map>; + static get fapiClient(): FapiClient { return BaseResource.clerk.getFapiClient(); } @@ -217,9 +229,9 @@ export abstract class BaseResource { } protected async _baseMutate(params: BaseMutateParams): Promise { - const { action, body, method, path } = params; + const { action, body, method, path, signal } = params; // TODO @userland-errors: - const json = await BaseResource._fetch({ method, path: path || this.path(action), body }); + const json = await BaseResource._fetch({ method, path: path || this.path(action), body, signal }); return this.fromJSON((json?.response || json) as J); } @@ -230,7 +242,32 @@ export abstract class BaseResource { } protected async _basePost(params: BaseMutateParams = {}): Promise { - return this._baseMutate({ ...params, method: 'POST' }); + if (!params.coalesce) { + return this._baseMutate({ ...params, method: 'POST' }); + } + + const key = this.#coalescedPostKey(params); + const posts = (this.#pendingCoalescedPosts ??= new Map()); + const pending = posts.get(key); + if (pending && Date.now() < pending.expiresAt) { + return pending.promise; + } + + const controller = new AbortController(); + const promise = this._baseMutate({ ...params, method: 'POST', signal: controller.signal }).finally(() => { + if (posts.get(key)?.promise === promise) { + posts.delete(key); + } + }); + posts.set(key, { promise, controller, expiresAt: Date.now() + COALESCED_POST_TTL_MS }); + return promise; + } + + #coalescedPostKey(params: BaseMutateParams): string { + const body = Object.entries(params.body ?? {}) + .filter(([, value]) => value !== undefined) + .sort(([left], [right]) => (left < right ? -1 : 1)); + return JSON.stringify([this.id, params.path, params.action, body]); } protected async _basePostBypass(params: BaseMutateParams = {}): Promise { diff --git a/packages/clerk-js/src/core/resources/EmailAddress.ts b/packages/clerk-js/src/core/resources/EmailAddress.ts index 796bfe37e16..1d4df3c6b73 100644 --- a/packages/clerk-js/src/core/resources/EmailAddress.ts +++ b/packages/clerk-js/src/core/resources/EmailAddress.ts @@ -38,6 +38,7 @@ export class EmailAddress extends BaseResource implements EmailAddressResource { prepareVerification = (params: PrepareEmailAddressVerificationParams): Promise => { return this._basePost({ action: 'prepare_verification', + coalesce: true, body: { ...params }, }); }; diff --git a/packages/clerk-js/src/core/resources/PhoneNumber.ts b/packages/clerk-js/src/core/resources/PhoneNumber.ts index 809cf27c807..2b009c705f3 100644 --- a/packages/clerk-js/src/core/resources/PhoneNumber.ts +++ b/packages/clerk-js/src/core/resources/PhoneNumber.ts @@ -34,6 +34,7 @@ export class PhoneNumber extends BaseResource implements PhoneNumberResource { prepareVerification = (): Promise => { return this._basePost({ action: 'prepare_verification', + coalesce: true, body: { strategy: 'phone_code' }, }); }; diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index 65a72ef1c92..20b206091f4 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -265,6 +265,7 @@ export class SignIn extends BaseResource implements SignInResource { return this._basePost({ body: { ...config, strategy: params.strategy }, action: 'prepare_first_factor', + coalesce: true, }); }; @@ -356,6 +357,7 @@ export class SignIn extends BaseResource implements SignInResource { return this._basePost({ body: params, action: 'prepare_second_factor', + coalesce: true, }); }; @@ -883,6 +885,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { emailAddressId, strategy: 'reset_password_email_code' }, action: 'prepare_first_factor', + coalesce: true, }); }); } @@ -926,6 +929,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { phoneNumberId, strategy: 'reset_password_phone_code' }, action: 'prepare_first_factor', + coalesce: true, }); }); } @@ -1083,6 +1087,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { emailAddressId: emailCodeFactor.emailAddressId, strategy: 'email_code' }, action: 'prepare_first_factor', + coalesce: true, }); }); } @@ -1135,6 +1140,7 @@ class SignInFuture implements SignInFutureResource { strategy: 'email_link', }, action: 'prepare_first_factor', + coalesce: true, }); }); } @@ -1187,6 +1193,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { phoneNumberId: phoneCodeFactor.phoneNumberId, strategy: 'phone_code', channel }, action: 'prepare_first_factor', + coalesce: true, }); }); } @@ -1252,6 +1259,7 @@ class SignInFuture implements SignInFutureResource { strategy: 'enterprise_sso', }, action: 'prepare_first_factor', + coalesce: true, }); } @@ -1319,6 +1327,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { web3WalletId: web3FirstFactor.web3WalletId, strategy }, action: 'prepare_first_factor', + coalesce: true, }); const { message } = this.firstFactorVerification; @@ -1390,6 +1399,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { strategy: 'passkey' }, action: 'prepare_first_factor', + coalesce: true, }); } @@ -1442,6 +1452,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { phoneNumberId, strategy: 'phone_code' }, action: 'prepare_second_factor', + coalesce: true, }); }); } @@ -1468,6 +1479,7 @@ class SignInFuture implements SignInFutureResource { await this.#resource.__internal_basePost({ body: { emailAddressId, strategy: 'email_code' }, action: 'prepare_second_factor', + coalesce: true, }); }); } diff --git a/packages/clerk-js/src/core/resources/SignUp.ts b/packages/clerk-js/src/core/resources/SignUp.ts index 2a4ed453c3d..32f8e625239 100644 --- a/packages/clerk-js/src/core/resources/SignUp.ts +++ b/packages/clerk-js/src/core/resources/SignUp.ts @@ -188,6 +188,7 @@ export class SignUp extends BaseResource implements SignUpResource { return this._basePost({ body: params, action: 'prepare_verification', + coalesce: true, }); }; @@ -973,6 +974,7 @@ class SignUpFuture implements SignUpFutureResource { await this.#resource.__internal_basePost({ body: { strategy: 'email_code' }, action: 'prepare_verification', + coalesce: true, }); }); } @@ -993,6 +995,7 @@ class SignUpFuture implements SignUpFutureResource { await this.#resource.__internal_basePost({ body: { strategy: 'phone_code', channel }, action: 'prepare_verification', + coalesce: true, }); }); } @@ -1020,6 +1023,7 @@ class SignUpFuture implements SignUpFutureResource { await this.#resource.__internal_basePost({ body: { strategy: 'email_link', redirectUrl: absoluteVerificationUrl }, action: 'prepare_verification', + coalesce: true, }); }); } @@ -1163,6 +1167,7 @@ class SignUpFuture implements SignUpFutureResource { await this.#resource.__internal_basePost({ body: { strategy }, action: 'prepare_verification', + coalesce: true, }); const { message } = this.#resource.verifications.web3Wallet; diff --git a/packages/clerk-js/src/core/resources/Web3Wallet.ts b/packages/clerk-js/src/core/resources/Web3Wallet.ts index 1ee299bde20..cd1a923ea37 100644 --- a/packages/clerk-js/src/core/resources/Web3Wallet.ts +++ b/packages/clerk-js/src/core/resources/Web3Wallet.ts @@ -30,6 +30,7 @@ export class Web3Wallet extends BaseResource implements Web3WalletResource { prepareVerification = (params: PrepareWeb3WalletVerificationParams): Promise => { return this._basePost({ action: 'prepare_verification', + coalesce: true, body: { ...params }, }); }; diff --git a/packages/clerk-js/src/core/resources/__tests__/EmailAddress.test.ts b/packages/clerk-js/src/core/resources/__tests__/EmailAddress.test.ts new file mode 100644 index 00000000000..8043aba70d8 --- /dev/null +++ b/packages/clerk-js/src/core/resources/__tests__/EmailAddress.test.ts @@ -0,0 +1,123 @@ +import type { EmailAddressJSON } from '@clerk/shared/types'; +import { createDeferredPromise } from '@clerk/shared/utils'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { BaseResource, EmailAddress } from '../internal'; + +describe('EmailAddress', () => { + describe('prepareVerification', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('coalesces concurrent identical preparations', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + // @ts-ignore + BaseResource._fetch = mockFetch; + + const emailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const params = { strategy: 'email_code' as const }; + + const first = emailAddress.prepareVerification(params); + const second = emailAddress.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + response: { id: 'email_123' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce preparations with different strategies or params', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + response: { id: 'email_123' }, + }); + BaseResource._fetch = mockFetch; + + const emailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + + await Promise.all([ + emailAddress.prepareVerification({ strategy: 'email_code' }), + emailAddress.prepareVerification({ strategy: 'email_link', redirectUrl: '/verify-one' }), + emailAddress.prepareVerification({ strategy: 'email_link', redirectUrl: '/verify-two' }), + ]); + + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('starts a new preparation after the previous request succeeds', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + response: { id: 'email_123' }, + }); + // @ts-ignore + BaseResource._fetch = mockFetch; + + const emailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const params = { strategy: 'email_code' as const }; + + await emailAddress.prepareVerification(params); + await emailAddress.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('starts a new preparation after the previous request fails', async () => { + const mockFetch = vi + .fn() + .mockRejectedValueOnce(new Error('prepare failed')) + .mockResolvedValueOnce({ + response: { id: 'email_123' }, + }); + BaseResource._fetch = mockFetch; + + const emailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const params = { strategy: 'email_code' as const }; + + await expect(emailAddress.prepareVerification(params)).rejects.toThrow('prepare failed'); + await emailAddress.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('does not coalesce preparations across EmailAddress instances', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const firstEmailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const secondEmailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const params = { strategy: 'email_code' as const }; + + const first = firstEmailAddress.prepareVerification(params); + const second = secondEmailAddress.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + response: { id: 'email_123' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce concurrent verification attempts', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const emailAddress = new EmailAddress({ id: 'email_123' } as EmailAddressJSON, '/me/email_addresses'); + const params = { code: '123456' }; + + const first = emailAddress.attemptVerification(params); + const second = emailAddress.attemptVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + response: { id: 'email_123' }, + }); + await Promise.all([first, second]); + }); + }); +}); diff --git a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts index 628b665fc7c..236b62da151 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -1,3 +1,4 @@ +import { createDeferredPromise } from '@clerk/shared/utils'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { eventBus } from '../../events'; @@ -36,6 +37,239 @@ describe('SignIn', () => { expect(snapshot).toBeDefined(); }); + describe('prepareSecondFactor', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('coalesces concurrent identical preparations', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + const first = signIn.prepareSecondFactor(params); + const second = signIn.prepareSecondFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce preparations for different factors', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + client: null, + response: { id: 'signin_123' }, + }); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + + await Promise.all([ + signIn.prepareSecondFactor({ strategy: 'email_code', emailAddressId: 'email_123' }), + signIn.prepareSecondFactor({ strategy: 'email_code', emailAddressId: 'email_456' }), + ]); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('starts a new preparation after the previous request succeeds', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + client: null, + response: { id: 'signin_123' }, + }); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + await signIn.prepareSecondFactor(params); + await signIn.prepareSecondFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('starts a new preparation after the previous request fails', async () => { + const mockFetch = vi + .fn() + .mockRejectedValueOnce(new Error('prepare failed')) + .mockResolvedValueOnce({ + client: null, + response: { id: 'signin_123' }, + }); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + await expect(signIn.prepareSecondFactor(params)).rejects.toThrow('prepare failed'); + await signIn.prepareSecondFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + }); + + describe('factor preparation across APIs', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('coalesces concurrent first-factor preparations', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + const first = signIn.prepareFirstFactor(params); + const second = signIn.prepareFirstFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, second]); + }); + + it('coalesces concurrent first-factor preparations from the future API', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + signIn.supportedFirstFactors = [ + { strategy: 'email_code', emailAddressId: 'email_123', safeIdentifier: 't***@example.com' }, + ]; + + const first = signIn.__internal_future.emailCode.sendCode({ emailAddressId: 'email_123' }); + const second = signIn.__internal_future.emailCode.sendCode({ emailAddressId: 'email_123' }); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, second]); + }); + + it('coalesces concurrent second-factor preparations from the future API', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + signIn.supportedSecondFactors = [ + { strategy: 'email_code', emailAddressId: 'email_123', safeIdentifier: 't***@example.com' }, + ]; + + const first = signIn.__internal_future.mfa.sendEmailCode(); + const second = signIn.__internal_future.mfa.sendEmailCode(); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, second]); + }); + + it('allows a new preparation after the TTL without aborting the pending request', async () => { + vi.useFakeTimers(); + try { + const firstDeferred = createDeferredPromise(); + const secondDeferred = createDeferredPromise(); + const mockFetch = vi + .fn() + .mockReturnValueOnce(firstDeferred.promise) + .mockReturnValueOnce(secondDeferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + const first = signIn.prepareFirstFactor(params); + const duplicate = signIn.prepareFirstFactor(params); + expect(mockFetch).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(30_000); + + const replacement = signIn.prepareFirstFactor(params); + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(mockFetch.mock.calls[0][0].signal.aborted).toBe(false); + expect(mockFetch.mock.calls[1][0].signal.aborted).toBe(false); + + firstDeferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, duplicate]); + + const replacementDuplicate = signIn.prepareFirstFactor(params); + expect(mockFetch).toHaveBeenCalledTimes(2); + + secondDeferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([replacement, replacementDuplicate]); + } finally { + vi.useRealTimers(); + } + }); + + it('does not coalesce preparations across different sign-in attempts', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, emailAddressId: 'email_123' }; + + const first = signIn.prepareFirstFactor(params); + signIn.id = 'signin_456'; + const second = signIn.prepareFirstFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + client: null, + response: { id: 'signin_456' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce concurrent factor attempts', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn({ id: 'signin_123' } as any); + const params = { strategy: 'email_code' as const, code: '123456' }; + + const first = signIn.attemptFirstFactor(params); + const second = signIn.attemptFirstFactor(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + client: null, + response: { id: 'signin_123' }, + }); + await Promise.all([first, second]); + }); + }); + describe('authenticateWithRedirect with an OAuth transport', () => { afterEach(() => { vi.clearAllMocks(); @@ -291,6 +525,7 @@ describe('SignIn', () => { expect(BaseResource._fetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/test_id/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_address_1', strategy: 'email_code', @@ -305,6 +540,7 @@ describe('SignIn', () => { expect(BaseResource._fetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/test_id/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_address_1', strategy: 'email_code', @@ -319,6 +555,7 @@ describe('SignIn', () => { expect(BaseResource._fetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/test_id/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_address_1', strategy: 'email_code', @@ -333,6 +570,7 @@ describe('SignIn', () => { expect(BaseResource._fetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/test_id/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_address_0', strategy: 'email_code', @@ -705,6 +943,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_123', strategy: 'email_code', @@ -739,6 +978,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_456', strategy: 'email_code', @@ -811,6 +1051,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_123', redirectUrl: 'https://example.com/verify', @@ -845,6 +1086,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_123', redirectUrl: 'https://other.com/verify', @@ -882,6 +1124,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_456', redirectUrl: 'https://example.com/verify', @@ -1030,6 +1273,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_123', strategy: 'phone_code', @@ -1061,6 +1305,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_123', strategy: 'phone_code', @@ -1096,6 +1341,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenLastCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_456', strategy: 'phone_code', @@ -1162,6 +1408,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_123', strategy: 'reset_password_email_code', @@ -1227,6 +1474,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_123', strategy: 'reset_password_phone_code', @@ -1256,6 +1504,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_123', strategy: 'reset_password_phone_code', @@ -1386,6 +1635,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_second_factor', + signal: expect.any(AbortSignal), body: { phoneNumberId: 'phone_123', strategy: 'phone_code', @@ -1428,6 +1678,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_second_factor', + signal: expect.any(AbortSignal), body: { emailAddressId: 'email_123', strategy: 'email_code', @@ -1504,6 +1755,7 @@ describe('SignIn', () => { expect.objectContaining({ method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { strategy: 'passkey' }, }), ); @@ -1811,6 +2063,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { web3WalletId: 'wallet_123', strategy: 'web3_metamask_signature', @@ -2143,6 +2396,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_123/prepare_first_factor', + signal: expect.any(AbortSignal), body: { strategy: 'enterprise_sso', redirectUrl: 'https://example.com/sso-callback', @@ -2198,6 +2452,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(2, { method: 'POST', path: '/client/sign_ins/signin_ticket/prepare_first_factor', + signal: expect.any(AbortSignal), body: { strategy: 'enterprise_sso', redirectUrl: 'https://example.com/sso-callback', @@ -2304,6 +2559,7 @@ describe('SignIn', () => { expect(mockFetch).toHaveBeenNthCalledWith(1, { method: 'POST', path: '/client/sign_ins/signin_enterprise/prepare_first_factor', + signal: expect.any(AbortSignal), body: expect.objectContaining({ strategy: 'enterprise_sso', }), diff --git a/packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts b/packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts index a639c6244d8..0bcff445b5f 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts @@ -1,3 +1,4 @@ +import { createDeferredPromise } from '@clerk/shared/utils'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { eventBus } from '../../events'; @@ -37,6 +38,144 @@ describe('SignUp', () => { expect(snapshot).toBeDefined(); }); + describe('prepareVerification', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('coalesces concurrent identical preparations', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + const params = { strategy: 'email_code' as const }; + + const first = signUp.prepareVerification(params); + const second = signUp.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signup_123' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce preparations for different verifications', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + client: null, + response: { id: 'signup_123' }, + }); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + + await Promise.all([ + signUp.prepareVerification({ strategy: 'email_code' }), + signUp.prepareVerification({ strategy: 'phone_code' }), + ]); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('starts a new preparation after the previous request succeeds', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + client: null, + response: { id: 'signup_123' }, + }); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + const params = { strategy: 'email_code' as const }; + + await signUp.prepareVerification(params); + await signUp.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('starts a new preparation after the previous request fails', async () => { + const mockFetch = vi + .fn() + .mockRejectedValueOnce(new Error('prepare failed')) + .mockResolvedValueOnce({ + client: null, + response: { id: 'signup_123' }, + }); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + const params = { strategy: 'email_code' as const }; + + await expect(signUp.prepareVerification(params)).rejects.toThrow('prepare failed'); + await signUp.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('does not coalesce preparations across different sign-up attempts', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + const params = { strategy: 'email_code' as const }; + + const first = signUp.prepareVerification(params); + signUp.id = 'signup_456'; + const second = signUp.prepareVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + client: null, + response: { id: 'signup_456' }, + }); + await Promise.all([first, second]); + }); + + it('coalesces concurrent preparations from the future API', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + + const first = signUp.__internal_future.sendEmailCode(); + const second = signUp.__internal_future.sendEmailCode(); + + expect(mockFetch).toHaveBeenCalledTimes(1); + + deferred.resolve({ + client: null, + response: { id: 'signup_123' }, + }); + await Promise.all([first, second]); + }); + + it('does not coalesce concurrent verification attempts', async () => { + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); + BaseResource._fetch = mockFetch; + + const signUp = new SignUp({ id: 'signup_123' } as any); + const params = { strategy: 'email_code' as const, code: '123456' }; + + const first = signUp.attemptVerification(params); + const second = signUp.attemptVerification(params); + + expect(mockFetch).toHaveBeenCalledTimes(2); + + deferred.resolve({ + client: null, + response: { id: 'signup_123' }, + }); + await Promise.all([first, second]); + }); + }); + describe('authenticateWithRedirect with an OAuth transport', () => { afterEach(() => { vi.clearAllMocks(); @@ -484,6 +623,7 @@ describe('SignUp', () => { expect.objectContaining({ method: 'POST', path: '/client/sign_ups/signup_123/prepare_verification', + signal: expect.any(AbortSignal), body: expect.objectContaining({ strategy: 'phone_code', }), @@ -513,6 +653,7 @@ describe('SignUp', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ups/signup_123/prepare_verification', + signal: expect.any(AbortSignal), body: { strategy: 'email_link', redirectUrl: 'https://example.com/verify', @@ -535,6 +676,7 @@ describe('SignUp', () => { expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: '/client/sign_ups/signup_123/prepare_verification', + signal: expect.any(AbortSignal), body: { strategy: 'email_link', redirectUrl: 'https://other.com/verify', diff --git a/packages/clerk-js/src/core/resources/__tests__/Web3Wallet.test.ts b/packages/clerk-js/src/core/resources/__tests__/Web3Wallet.test.ts index bb29fde3ad8..62ca2c90393 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Web3Wallet.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Web3Wallet.test.ts @@ -1,4 +1,5 @@ import type { Web3WalletJSON } from '@clerk/shared/types'; +import { createDeferredPromise } from '@clerk/shared/utils'; import { describe, expect, it, vi } from 'vitest'; import { BaseResource, Web3Wallet } from '../internal'; @@ -33,20 +34,28 @@ describe('Web3 wallet', () => { web3_wallet: '0x0000000000000000000000000000000000000000', } as Web3WalletJSON; + const deferred = createDeferredPromise(); + const mockFetch = vi.fn().mockReturnValue(deferred.promise); // @ts-ignore - BaseResource._fetch = vi.fn().mockReturnValue(Promise.resolve({ response: web3WalletJSON })); + BaseResource._fetch = mockFetch; const web3Wallet = new Web3Wallet(web3WalletJSON, '/me/web3_wallets'); - await web3Wallet.prepareVerification({ strategy: 'web3_metamask_signature' }); + const params = { strategy: 'web3_metamask_signature' as const }; + const first = web3Wallet.prepareVerification(params); + const second = web3Wallet.prepareVerification(params); - // @ts-ignore - expect(BaseResource._fetch).toHaveBeenCalledWith({ + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(mockFetch).toHaveBeenCalledWith({ method: 'POST', path: `/me/web3_wallets/${web3WalletJSON.id}/prepare_verification`, body: { strategy: 'web3_metamask_signature', }, + signal: expect.any(AbortSignal), }); + + deferred.resolve({ response: web3WalletJSON }); + await Promise.all([first, second]); }); it('attemptVerification', async () => {