diff --git a/.changeset/quiet-wallet-recovery.md b/.changeset/quiet-wallet-recovery.md new file mode 100644 index 00000000000..3877c549d70 --- /dev/null +++ b/.changeset/quiet-wallet-recovery.md @@ -0,0 +1,5 @@ +--- +"@clerk/mosaic": patch +--- + +Show a clear cancellation message for rejected wallet requests in UserProfile, and explain when a request is still pending in the wallet. Ignore late wallet responses after closing the profile so reopening starts a fresh connection flow. diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index a0a5a82f369..2cc79d187cf 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -32,40 +32,40 @@ A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is th Most problems a feature hits already have a shared answer. Find the row for your problem and use it. Do not write a local version, even a small one. -| Problem | Use | -| ------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------- | -| An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | -| A form: values, validation, submit, field errors, success UI | `useForm` (`src/components/form/`), with `validateAsync` for server checks and `onSuccess` for mounted success UI | -| A `
` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | -| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | -| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | -| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | -| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | -| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | -| Showing any error to the user | One of the owners above. They call `toLocalizableError` (or `toLocalizableErrors` for all errors) and resolve copy with `useErrorText` | -| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | -| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | -| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | -| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | -| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | -| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | -| Current time for display labels | `useNow` (`src/hooks/`) | -| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | -| An inline message row; height is undefined until measured | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | -| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | -| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | -| An organization permission checked with `checkAuthorization` | `ORGANIZATION_PERMISSIONS` (`src/utils/permissions.ts`) | -| A verification code's length or resend cooldown | `CODE_LENGTH`, `CODE_RESEND_SECONDS` (`src/utils/verification.ts`) | -| An avatar or logo file picker and its rejection errors | `AVATAR_UPLOAD`, `AVATAR_REJECTION_ERRORS` (`src/utils/avatar-upload.ts`) | -| An avatar or logo row: picker, drop target, Upload button or manage menu | `AvatarUpload` (`src/blocks/avatar-upload/`), laid out by the caller | -| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | -| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | -| A panel that animates open and closed to its measured size | `usePanelSize` (`src/primitives/hooks/`) | -| One screen replacing another on the same card or dialog | `Flow` (`src/components/flow/`), see [Views](docs/views.md) | -| A form on a card or dialog step | `useForm` in the controller, passed to the view as `form`, see [Views](docs/views.md) | -| Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | -| An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | -| Styles several components share | `src/styles/*.styles.ts` | +| Problem | Use | +| --------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | +| An async action from a button or row: pending, an error, one lock, unmount signal | `usePendingAction` (`src/hooks/`) | +| A form: values, validation, submit, field errors, success UI | `useForm` (`src/components/form/`), with `validateAsync` for server checks and `onSuccess` for mounted success UI | +| A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | +| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | +| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | +| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | +| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | +| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | +| Showing any error to the user | One of the owners above. They call `toLocalizableError` (or `toLocalizableErrors` for all errors) and resolve copy with `useErrorText` | +| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | +| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | +| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | +| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | +| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | +| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | +| Current time for display labels | `useNow` (`src/hooks/`) | +| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | +| An inline message row; height is undefined until measured | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | +| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | +| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | +| An organization permission checked with `checkAuthorization` | `ORGANIZATION_PERMISSIONS` (`src/utils/permissions.ts`) | +| A verification code's length or resend cooldown | `CODE_LENGTH`, `CODE_RESEND_SECONDS` (`src/utils/verification.ts`) | +| An avatar or logo file picker and its rejection errors | `AVATAR_UPLOAD`, `AVATAR_REJECTION_ERRORS` (`src/utils/avatar-upload.ts`) | +| An avatar or logo row: picker, drop target, Upload button or manage menu | `AvatarUpload` (`src/blocks/avatar-upload/`), laid out by the caller | +| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | +| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | +| A panel that animates open and closed to its measured size | `usePanelSize` (`src/primitives/hooks/`) | +| One screen replacing another on the same card or dialog | `Flow` (`src/components/flow/`), see [Views](docs/views.md) | +| A form on a card or dialog step | `useForm` in the controller, passed to the view as `form`, see [Views](docs/views.md) | +| Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | +| An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | +| Styles several components share | `src/styles/*.styles.ts` | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 94f05e37503..075cd5bc96a 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -55,7 +55,12 @@ return { focus. To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). When the user cancels a reverification challenge, `run` resolves `false` without showing an -error and releases the lock. It is not for: +error and releases the lock. The action receives an `AbortSignal` that aborts on +unmount. Completions after unmount cannot update the hook's state. Aborting the +signal does not stop an underlying provider request or undo a server mutation. +Check the signal after external waits and before side effects. +Callers must also guard their own completion UI with the signal. `reset()` only +clears errors. It is not for: - a form → `useForm` - a confirm step → `useConfirmController` with `Confirmation` / `Destructive` diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx new file mode 100644 index 00000000000..a41ff9d46dd --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx @@ -0,0 +1,142 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { openMenu } from '../../../../__tests__/feature/menu'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { signedInWallets } from './user-profile-web3-wallets.fixtures'; + +const request = vi.fn<(params: { method: string }) => Promise>(); +const provider = { request }; +const announceOKX = () => + window.dispatchEvent( + new CustomEvent('eip6963:announceProvider', { + detail: { info: { name: 'OKX Wallet', uuid: 'recovery-okx', icon: '', rdns: 'com.okx.wallet' }, provider }, + }), + ); + +afterEach(() => window.removeEventListener('eip6963:requestProvider', announceOKX)); + +describe.each([ + ['Base', 'web3_base_signature', '@base-org/account'], + ['Coinbase Wallet', 'web3_coinbase_wallet_signature', '@coinbase/wallet-sdk'], + ['OKX Wallet', 'web3_okx_wallet_signature', null], +] as const)('%s recovery', (name, strategy, moduleName) => { + async function setup() { + vi.stubGlobal('ethereum', { + request: () => { + throw new Error('The selected wallet must not use the fallback Ethereum provider.'); + }, + }); + const fapi = serveFapi(signedInWallets([strategy])); + const view = await renderWithClerk(); + Object.defineProperty(view.clerk, '__internal_moduleManager', { + value: { + import: (module: string) => { + expect(module).toBe(moduleName); + return Promise.resolve({ + createBaseAccountSDK: () => ({ getProvider: () => provider }), + createCoinbaseWalletSDK: () => ({ getProvider: () => provider }), + }); + }, + }, + }); + if (moduleName === null) { + window.addEventListener('eip6963:requestProvider', announceOKX); + announceOKX(); + } + return { fapi, view, user: userEvent.setup() }; + } + + it.each(['eth_requestAccounts', 'personal_sign'] as const)( + 'recovers from cancellation during %s and verifies on retry', + async method => { + request.mockImplementation(({ method: requested }) => { + if (requested === method) { + return Promise.reject(Object.assign(new Error('User rejected the request.'), { code: 4001 })); + } + return Promise.resolve(['0x1234567890abcdef']); + }); + const { fapi, user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'), + ); + request.mockImplementation(({ method: requested }) => + Promise.resolve(requested === 'personal_sign' ? 'signature' : ['0x1234567890abcdef']), + ); + if (method === 'personal_sign') { + await openMenu(user, screen.getByRole('button', { name: `Manage ${name}` })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await user.click(screen.getByRole('button', { name: `Connect ${name}` })); + } + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }, + ); + + it('explains a pending provider request and releases its busy state', async () => { + request.mockRejectedValue({ code: -32002 }); + const { user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent( + 'A request is already pending in your wallet. Open your wallet to complete or reject it, then try again.', + ), + ); + expect(screen.getByRole('button', { name: `Connect ${name}` })).toBeEnabled(); + }); + + it.each([ + ['eth_requestAccounts', 'resolve'], + ['eth_requestAccounts', 'reject'], + ['personal_sign', 'resolve'], + ['personal_sign', 'reject'], + ] as const)('ignores abandoned %s %s after reopening and completes the new attempt', async (method, outcome) => { + const old = deferred(); + const next = deferred(); + request.mockImplementation(async ({ method: requested }) => + requested === method ? old.promise : ['0x1234567890abcdef'], + ); + const { fapi, view, user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => expect(request).toHaveBeenCalledWith(expect.objectContaining({ method }))); + view.rerender(
); + view.rerender(); + request.mockImplementation(async ({ method: requested }) => { + if (requested === method) { + return next.promise; + } + return requested === 'personal_sign' ? 'signature' : ['0x1234567890abcdef']; + }); + if (method === 'personal_sign') { + await openMenu(user, await screen.findByRole('button', { name: `Manage ${name}` })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + } + await waitFor(() => expect(request.mock.calls.filter(([params]) => params.method === method)).toHaveLength(2)); + await act(() => { + if (outcome === 'resolve') { + old.resolve(method === 'personal_sign' ? 'old-signature' : ['0xold']); + } else { + old.reject({ code: 4001 }); + } + }); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(method === 'personal_sign' ? 1 : 0); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).not.toBe('verified'); + expect( + screen.getByRole('button', { name: `${method === 'personal_sign' ? 'Manage' : 'Connect'} ${name}` }), + ).toHaveAttribute('aria-disabled', 'true'); + await act(() => next.resolve(method === 'personal_sign' ? 'signature' : ['0x1234567890abcdef'])); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + }); +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx new file mode 100644 index 00000000000..55a1ec7b5b1 --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx @@ -0,0 +1,101 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { signedInWallets } from './user-profile-web3-wallets.fixtures'; + +it('handles cancellation in MetaMask without offering a Clerk cancel button and allows a fresh connection', async () => { + const accounts = deferred(); + const request = vi.fn(({ method }: { method: string }) => + method === 'personal_sign' ? Promise.resolve('signature') : accounts.promise, + ); + vi.stubGlobal('ethereum', { request }); + const fapi = serveFapi(signedInWallets(['web3_metamask_signature', 'web3_solana_signature'])); + await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledWith({ method: 'eth_requestAccounts' })); + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeDisabled(); + expect(screen.queryByRole('button', { name: /cancel/i })).not.toBeInTheDocument(); + await act(() => accounts.reject({ code: 4001, message: 'User rejected the request.' })); + + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeEnabled(); + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + expect(request).not.toHaveBeenCalledWith(expect.objectContaining({ method: 'personal_sign' })); + + request.mockImplementation(({ method }) => + Promise.resolve(method === 'personal_sign' ? 'signature' : ['0xabcdef1234567890']), + ); + await user.click(screen.getByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.web3_wallet).toBe('0xabcdef1234567890'); +}); + +it('explains a rejected MetaMask connection and releases the pending state', async () => { + const request = vi.fn().mockRejectedValue({ code: 4001, message: 'User rejected the request.' }); + vi.stubGlobal('ethereum', { request }); + serveFapi(signedInWallets(['web3_metamask_signature'])); + await renderWithClerk(); + await userEvent.setup().click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toBeEnabled()); + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'); +}); + +it('explains that an existing MetaMask request needs attention in the wallet', async () => { + vi.stubGlobal('ethereum', { request: vi.fn().mockRejectedValue({ code: -32002 }) }); + serveFapi(signedInWallets(['web3_metamask_signature'])); + await renderWithClerk(); + await userEvent.setup().click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + expect( + await screen.findByText( + 'A request is already pending in your wallet. Open your wallet to complete or reject it, then try again.', + ), + ).toBeVisible(); +}); + +it.each(['resolve', 'reject'] as const)( + 'ignores a late provider %s after closing and reopening the section', + async outcome => { + const old = deferred(); + const next = deferred(); + const request = vi.fn(({ method }: { method: string }) => + method === 'personal_sign' ? Promise.resolve('signature') : old.promise, + ); + vi.stubGlobal('ethereum', { request }); + const fapi = serveFapi(signedInWallets(['web3_metamask_signature'])); + const view = await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledTimes(1)); + view.rerender(
); + view.rerender(); + request.mockImplementation(({ method }) => + method === 'personal_sign' ? Promise.resolve('signature') : next.promise, + ); + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledTimes(2)); + await act(() => { + if (outcome === 'resolve') { + old.resolve(['0xold']); + } else { + old.reject({ code: -32002 }); + } + }); + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await act(() => next.resolve(['0xnew'])); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.web3_wallet).toBe('0xnew'); + }, +); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx index 7bf2d1ef0e7..666300f257d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -1064,8 +1064,4 @@ describe('Web3 wallets', () => { // TODO: Share Solana discovery and filtering only after verifying parity with the legacy UI. // TODO: Share identification sorting only after verifying parity for legacy email, phone, and wallet sections. - // TODO: Add recovery for abandoned MetaMask connections. In Brave, closing the locked wallet prompt - // left wallet_requestPermissions pending. Refresh cleared Clerk's error, but a new connection returned -32002. - // Clerk cancellation must release the UI and ignore late responses without claiming to cancel the wallet request. - // The injected MetaMask API exposes no supported method to cancel that request, and clerk_go cannot clear it. }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx new file mode 100644 index 00000000000..268315ac9c4 --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx @@ -0,0 +1,124 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { openMenu } from '../../../../__tests__/feature/menu'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { registerWallets, signedInWallets } from './user-profile-web3-wallets.fixtures'; + +const account = { + address: 'SolanaRecoveryAccount', + publicKey: new Uint8Array([1, 2, 3]), + chains: ['solana:mainnet' as const], + features: ['solana:signMessage' as const], +}; +const signatures = [{ signature: new Uint8Array([4, 5, 6]) }]; + +function registerSolana( + connect: () => Promise<{ accounts: (typeof account)[] }>, + signMessage: () => Promise, +) { + return registerWallets({ + version: '1.0.0', + name: 'Recovery Solana', + icon: 'data:image/svg+xml;base64,', + chains: ['solana:mainnet'], + accounts: [account], + features: { + 'standard:connect': { version: '1.0.0', connect }, + 'solana:signMessage': { version: '1.0.0', signMessage }, + }, + }); +} + +it('explains Solana connection cancellation and connects on retry', async () => { + const connect = vi + .fn() + .mockRejectedValueOnce({ code: 4001 }) + .mockResolvedValue({ accounts: [account] }); + const unregister = registerSolana(connect, () => Promise.resolve(signatures)); + try { + const fapi = serveFapi(signedInWallets(['web3_solana_signature'])); + await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'), + ); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await user.click(screen.getByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + } finally { + unregister(); + } +}); + +it.each([ + ['connect', 'resolve'], + ['connect', 'reject'], + ['signMessage', 'resolve'], + ['signMessage', 'reject'], +] as const)('ignores abandoned Solana %s %s and verifies after reopening', async (stage, outcome) => { + const oldConnection = deferred<{ accounts: (typeof account)[] }>(); + const nextConnection = deferred<{ accounts: (typeof account)[] }>(); + const oldSignature = deferred(); + const nextSignature = deferred(); + const connect = vi.fn(() => (stage === 'connect' ? oldConnection.promise : Promise.resolve({ accounts: [account] }))); + const signMessage = vi.fn(() => (stage === 'signMessage' ? oldSignature.promise : Promise.resolve(signatures))); + const unregister = registerSolana(connect, signMessage); + try { + const fapi = serveFapi(signedInWallets(['web3_solana_signature'])); + const view = await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(stage === 'connect' ? connect : signMessage).toHaveBeenCalledTimes(1)); + view.rerender(
); + view.rerender(); + connect.mockImplementation(() => + stage === 'connect' ? nextConnection.promise : Promise.resolve({ accounts: [account] }), + ); + signMessage.mockImplementation(() => + stage === 'signMessage' ? nextSignature.promise : Promise.resolve(signatures), + ); + if (stage === 'signMessage') { + await openMenu(user, await screen.findByRole('button', { name: 'Manage Solana' })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + } + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(stage === 'connect' ? connect : signMessage).toHaveBeenCalledTimes(2)); + await act(() => { + if (outcome === 'reject') { + (stage === 'connect' ? oldConnection : oldSignature).reject(new Error('User rejected the request.')); + } else if (stage === 'connect') { + oldConnection.resolve({ accounts: [account] }); + } else { + oldSignature.resolve(signatures); + } + }); + expect(screen.getByRole('alert')).toHaveTextContent(/^$/); + expect(screen.getByRole('button', { name: 'Recovery Solana' })).toHaveAttribute('aria-disabled', 'true'); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(stage === 'connect' ? 0 : 1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).not.toBe('verified'); + await act(() => { + if (stage === 'connect') { + nextConnection.resolve({ accounts: [account] }); + } else { + nextSignature.resolve(signatures); + } + }); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + } finally { + unregister(); + } +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 9d242b0b1ae..84c4e19a3a3 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts @@ -65,36 +65,50 @@ export function useUserProfileWeb3WalletsController({ }); const runConnection = (provider: UserProfileWeb3Provider, walletName?: string) => - action.run(provider.id, async () => { + action.run(provider.id, async signal => { setOwner({ kind: 'connect', providerId: provider.id, target: null }); if (walletName) { setPicker({ open: true, kind: 'connect', provider, walletName }); } try { - await connect(provider.id, walletName, target => { - setOwner({ kind: 'connect', providerId: provider.id, target }); - if (provider.walletPicker === 'solana' && target.kind === 'wallet') { - setPicker({ open: false }); - } - }); - setPicker({ open: false }); - } finally { - setPicker(current => - current.open && current.kind === 'connect' ? { ...current, walletName: undefined } : current, + await connect( + provider.id, + walletName, + target => { + if (signal.aborted) { + return; + } + setOwner({ kind: 'connect', providerId: provider.id, target }); + if (provider.walletPicker === 'solana' && target.kind === 'wallet') { + setPicker({ open: false }); + } + }, + signal, ); + if (!signal.aborted) { + setPicker({ open: false }); + } + } finally { + if (!signal.aborted) { + setPicker(current => + current.open && current.kind === 'connect' ? { ...current, walletName: undefined } : current, + ); + } } }); const runVerification = (walletId: string, walletName?: string) => - action.run(walletId, async () => { + action.run(walletId, async signal => { setOwner({ kind: 'verify', walletId }); if (walletName) { setPicker({ open: true, kind: 'verify', walletId, walletName }); } try { - await verify(walletId, walletName); + await verify(walletId, walletName, signal); } finally { - setPicker({ open: false }); + if (!signal.aborted) { + setPicker({ open: false }); + } } }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts index 9da99dda1c5..5d23ebdc16b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts @@ -106,6 +106,18 @@ function walletUnavailable() { return new ClerkRuntimeError('This wallet provider is unavailable.', { code: 'web3_provider_unavailable' }); } +function rethrowProviderError(cause: unknown): never { + if (typeof cause === 'object' && cause !== null && 'code' in cause) { + if (cause.code === 4001) { + throw new ClerkRuntimeError('Wallet connection was cancelled.', { code: 'web3_connection_cancelled' }); + } + if (cause.code === -32002) { + throw new ClerkRuntimeError('A wallet request is already pending.', { code: 'web3_request_pending' }); + } + } + throw cause; +} + export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { const clerk = useClerk(); const { isLoaded, user } = useUser(); @@ -120,8 +132,11 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { const [setPrimaryWallet, setPrimaryReverification] = useReverificationFlow( (current: UserResource, walletId: string) => current.update({ primaryWeb3WalletId: walletId }), ); - const [createWallet, createReverification] = useReverificationFlow((current: UserResource, web3Wallet: string) => - current.createWeb3Wallet({ web3Wallet }), + const [createWallet, createReverification] = useReverificationFlow( + (current: UserResource, web3Wallet: string, signal?: AbortSignal) => { + signal?.throwIfAborted(); + return current.createWeb3Wallet({ web3Wallet }); + }, ); if (!isLoaded || !environment) { @@ -155,14 +170,18 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { identifier, walletName, web3, + signal, }: { wallet: Web3WalletResource; provider: (typeof WEB3_PROVIDERS)[number]; identifier: string; walletName?: string; web3: ReturnType; + signal?: AbortSignal; }) => { + signal?.throwIfAborted(); const prepared = await wallet.prepareVerification({ strategy: provider.strategy }); + signal?.throwIfAborted(); requireCurrentUser(); const nonce = prepared.verification.message; if (!nonce) { @@ -170,7 +189,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { code: 'web3_verification_message_unavailable', }); } - const signature = await web3.generateWeb3Signature({ identifier, nonce, provider: provider.provider, walletName }); + const signature = await web3 + .generateWeb3Signature({ identifier, nonce, provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); requireCurrentUser(); if (!signature) { throw new ClerkRuntimeError('The wallet signature is unavailable.', { code: 'web3_signature_unavailable' }); @@ -180,7 +202,8 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { return { ...projection, userId, - connect: async (strategy, walletName, onTarget) => { + connect: async (strategy, walletName, onTarget, signal) => { + signal?.throwIfAborted(); requireCurrentUser(); const provider = WEB3_PROVIDERS.find(candidate => candidate.strategy === strategy); const available = projection.availableProviders.some(candidate => candidate.id === strategy); @@ -189,7 +212,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw new ClerkRuntimeError('This wallet provider is unavailable.', { code: 'web3_provider_unavailable' }); } const web3 = createWeb3(manager); - const identifier = await web3.getWeb3Identifier({ provider: provider.provider, walletName }); + const identifier = await web3 + .getWeb3Identifier({ provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); if (!identifier) { throw new ClerkRuntimeError('A Web3 Wallet extension cannot be found.', { code: 'web3_missing_identifier' }); } @@ -213,15 +239,17 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw walletUnavailable(); } onTarget?.(existing ? { kind: 'wallet', id: existing.id } : { kind: 'address', address: normalizedIdentifier }); - const wallet = existing ?? (await createWallet(current, identifier)); + const wallet = existing ?? (await createWallet(current, identifier, signal)); + signal?.throwIfAborted(); requireCurrentUser(); if (!wallet) { throw new ClerkRuntimeError('The wallet could not be created.', { code: 'web3_wallet_creation_failed' }); } onTarget?.({ kind: 'wallet', id: wallet.id }); - await attemptVerification({ wallet, provider, identifier, walletName, web3 }); + await attemptVerification({ wallet, provider, identifier, walletName, web3, signal }); }, - verify: async (walletId, walletName) => { + verify: async (walletId, walletName, signal) => { + signal?.throwIfAborted(); const current = requireCurrentUser(); const wallet = current.web3Wallets.find(candidate => candidate.id === walletId); const provider = WEB3_PROVIDERS.find(candidate => candidate.strategy === wallet?.verification.strategy); @@ -235,7 +263,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw walletUnavailable(); } const web3 = createWeb3(manager); - const identifier = await web3.getWeb3Identifier({ provider: provider.provider, walletName }); + const identifier = await web3 + .getWeb3Identifier({ provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); if (!identifier) { throw new ClerkRuntimeError('A Web3 Wallet extension cannot be found.', { code: 'web3_missing_identifier' }); } @@ -254,7 +285,7 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { code: 'web3_wallet_account_mismatch', }); } - await attemptVerification({ wallet: latest, provider, identifier, walletName, web3 }); + await attemptVerification({ wallet: latest, provider, identifier, walletName, web3, signal }); }, setPrimary: async walletId => { const current = requireCurrentUser(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts index a0ce8fec689..4c53961c2cd 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts @@ -62,8 +62,9 @@ export type UserProfileWeb3WalletsModel = strategy: Web3Strategy, walletName?: string, onTarget?: (target: Web3ConnectionTarget) => void, + signal?: AbortSignal, ) => Promise; - verify: (walletId: string, walletName?: string) => Promise; + verify: (walletId: string, walletName?: string, signal?: AbortSignal) => Promise; setPrimary: (walletId: string) => Promise; remove?: (walletId: string) => Promise; removeReverification: ReverificationController; diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx index b2e39244861..253ae6f353f 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -264,3 +264,26 @@ describe('usePendingAction', () => { expect(result.current.errorKey).toBeUndefined(); }); }); + +it.each(['resolve', 'reject'] as const)('ignores a late %s after unmount', async outcome => { + const pending = deferred(); + const { result, unmount } = renderHook(() => usePendingAction()); + let signal: AbortSignal | undefined; + let run: Promise | undefined; + act(() => { + run = result.current.run('a', current => { + signal = current; + return pending.promise; + }); + }); + unmount(); + expect(signal?.aborted).toBe(true); + await act(async () => { + if (outcome === 'resolve') { + pending.resolve(); + } else { + pending.reject(blocked()); + } + expect(await run).toBe(false); + }); +}); diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index bfeb88f3274..c3010acd691 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -1,5 +1,5 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import type { ErrorDescription } from '../localization'; import { useErrorText } from '../localization'; @@ -11,7 +11,11 @@ export interface PendingActionOptions { } export interface PendingAction { - run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; + run: ( + key: TKey, + action: (signal: AbortSignal) => Promise | void, + options?: PendingActionOptions, + ) => Promise; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -28,7 +32,8 @@ interface PendingActionFailure { /** * Runs inline actions, such as row buttons, under one lock and tracks which key is pending, which key ran last, and which key last failed. - * `run` resolves `true` on success and `false` on failure, ignores calls while any run is in flight, and clears the last error when it starts. + * `run` resolves `true` on success and `false` on failure or cancellation, ignores calls while a run owns the lock, and clears the last error when it starts. + * Unmount aborts the action signal; callers guard their own side effects and completion UI. */ export function usePendingAction({ errorFallback, @@ -37,27 +42,42 @@ export function usePendingAction({ const [pendingKey, setPendingKey] = useState(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); - const running = useRef(false); + const running = useRef(); - const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + useEffect( + () => () => { + running.current?.abort(); + running.current = undefined; + }, + [], + ); + + const run = async ( + key: TKey, + action: (signal: AbortSignal) => Promise | void, + options: PendingActionOptions = {}, + ) => { if (running.current) { return false; } - running.current = true; + const controller = new AbortController(); + running.current = controller; setPendingKey(key); setLastKey(key); setFailure(undefined); try { - await action(); - return true; + await action(controller.signal); + return !controller.signal.aborted; } catch (cause) { - if (!isReverificationCancelledError(cause)) { + if (!controller.signal.aborted && !isReverificationCancelledError(cause)) { setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); } return false; } finally { - running.current = false; - setPendingKey(undefined); + if (running.current === controller) { + running.current = undefined; + setPendingKey(undefined); + } } }; diff --git a/packages/mosaic/src/localization/errors.messages.ts b/packages/mosaic/src/localization/errors.messages.ts index 1522b27442f..b6a5c67f62e 100644 --- a/packages/mosaic/src/localization/errors.messages.ts +++ b/packages/mosaic/src/localization/errors.messages.ts @@ -93,6 +93,9 @@ export const errorMessages: { readonly generic: string } & Readonly