-
Notifications
You must be signed in to change notification settings - Fork 474
fix(clerk-js,ui): clean up abandoned passkey registrations #9812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@clerk/localizations': patch | ||
| '@clerk/shared': patch | ||
| --- | ||
|
|
||
| Add a localized message for the `too_many_unverified_identifications` error, shown when adding an email address or phone number is blocked by pending verifications or an incomplete passkey setup. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@clerk/clerk-js': patch | ||
| '@clerk/ui': patch | ||
| --- | ||
|
|
||
| Fix abandoned passkey registrations counting toward the limit on unverified identifications. Cancelling or failing the browser passkey prompt left a pending registration on the account that is hidden from the user's passkey list, so it could silently block adding an email address or phone number until it expired. The pending registration is now removed as soon as the prompt is abandoned. In `<UserProfile />`, the "Add a passkey" button also shows a loading state while a registration is in progress, and a failed attempt no longer leaves its error banner on screen after a later attempt succeeds. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| import { ClerkWebAuthnError } from '@clerk/shared/error'; | ||
| import type { PasskeyJSON } from '@clerk/shared/types'; | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { clerkMock } from '../../../test/core-fixtures'; | ||
| import { BaseResource, Passkey } from '../internal'; | ||
|
|
||
| const publicKeyOptions = (authenticatorAttachment?: 'platform' | 'cross-platform') => ({ | ||
| rp: { id: 'clerk.com', name: 'Clerk' }, | ||
| user: { id: 'dXNlcl8x', name: 'user@clerk.com', displayName: 'user' }, | ||
| challenge: 'Y2hhbGxlbmdl', | ||
| pubKeyCredParams: [{ type: 'public-key', alg: -7 }], | ||
| ...(authenticatorAttachment ? { authenticatorSelection: { authenticatorAttachment } } : {}), | ||
| }); | ||
|
|
||
| const passkeyJSON = (authenticatorAttachment?: 'platform' | 'cross-platform') => | ||
| ({ | ||
| object: 'passkey', | ||
| id: 'idn_passkey', | ||
| name: 'Chrome on macOS', | ||
| last_used_at: null, | ||
| created_at: 1717430400000, | ||
| updated_at: 1717430400000, | ||
| verification: { | ||
| object: 'verification', | ||
| id: 'ver_1', | ||
| status: 'unverified', | ||
| strategy: 'passkey', | ||
| nonce: JSON.stringify(publicKeyOptions(authenticatorAttachment)), | ||
| attempts: 0, | ||
| expire_at: 1717431000000, | ||
| }, | ||
| }) as unknown as PasskeyJSON; | ||
|
|
||
| const cancelledError = new ClerkWebAuthnError('Passkey registration was cancelled or timed out.', { | ||
| code: 'passkey_registration_cancelled', | ||
| }); | ||
|
|
||
| const successfulCredential = { | ||
| type: 'public-key', | ||
| id: 'credential', | ||
| rawId: new ArrayBuffer(1), | ||
| authenticatorAttachment: 'platform', | ||
| response: { | ||
| clientDataJSON: new ArrayBuffer(1), | ||
| attestationObject: new ArrayBuffer(1), | ||
| getTransports: () => ['internal'], | ||
| }, | ||
| getClientExtensionResults: () => ({}), | ||
| }; | ||
|
|
||
| const setupClerk = (createPublicCredentials: () => Promise<any>, platformAuthenticatorSupported = true) => { | ||
| BaseResource.clerk = clerkMock({ | ||
| __internal_isWebAuthnSupported: () => true, | ||
| __internal_isWebAuthnPlatformAuthenticatorSupported: () => Promise.resolve(platformAuthenticatorSupported), | ||
| __internal_createPublicCredentials: createPublicCredentials, | ||
| } as any) as any; | ||
| }; | ||
|
|
||
| const deleteCall = { method: 'DELETE', path: '/me/passkeys/idn_passkey' }; | ||
|
|
||
| describe('Passkey', () => { | ||
| afterEach(() => { | ||
| BaseResource.clerk = null as any; | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| describe('registerPasskey', () => { | ||
| it('deletes the pending passkey when the browser prompt is cancelled', async () => { | ||
| setupClerk(() => Promise.resolve({ publicKeyCredential: null, error: cancelledError })); | ||
| const fetchMock = vi.fn().mockResolvedValue({ response: passkeyJSON() }); | ||
| (BaseResource as any)._fetch = fetchMock; | ||
|
|
||
| await expect(Passkey.registerPasskey()).rejects.toBe(cancelledError); | ||
|
|
||
| expect(fetchMock).toHaveBeenCalledWith(deleteCall); | ||
| expect(fetchMock).not.toHaveBeenCalledWith( | ||
| expect.objectContaining({ path: '/me/passkeys/idn_passkey/attempt_verification' }), | ||
| ); | ||
| }); | ||
|
|
||
| it('surfaces the original error when the cleanup itself fails', async () => { | ||
| setupClerk(() => Promise.resolve({ publicKeyCredential: null, error: cancelledError })); | ||
| const fetchMock = vi.fn().mockImplementation(({ method }) => { | ||
| if (method === 'DELETE') { | ||
| return Promise.reject(new Error('session_reverification_required')); | ||
| } | ||
| return Promise.resolve({ response: passkeyJSON() }); | ||
| }); | ||
| (BaseResource as any)._fetch = fetchMock; | ||
|
|
||
| await expect(Passkey.registerPasskey()).rejects.toBe(cancelledError); | ||
| }); | ||
|
|
||
| it('deletes the pending passkey when the device lacks a platform authenticator', async () => { | ||
| const createPublicCredentials = vi.fn(); | ||
| setupClerk(createPublicCredentials as any, false); | ||
| const fetchMock = vi.fn().mockResolvedValue({ response: passkeyJSON('platform') }); | ||
| (BaseResource as any)._fetch = fetchMock; | ||
|
|
||
| await expect(Passkey.registerPasskey()).rejects.toMatchObject({ code: 'passkey_pa_not_supported' }); | ||
|
|
||
| expect(createPublicCredentials).not.toHaveBeenCalled(); | ||
| expect(fetchMock).toHaveBeenCalledWith(deleteCall); | ||
| }); | ||
|
|
||
| it('does not attempt a delete when the created passkey has no id', async () => { | ||
| setupClerk(() => Promise.resolve({ publicKeyCredential: null, error: cancelledError })); | ||
| const fetchMock = vi.fn().mockResolvedValue(null); | ||
| (BaseResource as any)._fetch = fetchMock; | ||
|
|
||
| await expect(Passkey.registerPasskey()).rejects.toThrow(); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalledWith(expect.objectContaining({ method: 'DELETE' })); | ||
| }); | ||
|
|
||
| it('does not delete anything when registration succeeds', async () => { | ||
| setupClerk(() => Promise.resolve({ publicKeyCredential: successfulCredential, error: null })); | ||
| const fetchMock = vi.fn().mockResolvedValue({ response: passkeyJSON() }); | ||
| (BaseResource as any)._fetch = fetchMock; | ||
|
|
||
| await Passkey.registerPasskey(); | ||
|
|
||
| expect(fetchMock).toHaveBeenCalledWith( | ||
| expect.objectContaining({ method: 'POST', path: '/me/passkeys/idn_passkey/attempt_verification' }), | ||
| ); | ||
| expect(fetchMock).not.toHaveBeenCalledWith(expect.objectContaining({ method: 'DELETE' })); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2166,6 +2166,8 @@ export const enUS: LocalizationResource = { | |||||
| ticket_expired_code: 'This link has expired. Please start again or request a new link.', | ||||||
| ticket_invalid_code: | ||||||
| 'This link is no longer valid or has already been used. Please start again or request a new link.', | ||||||
| too_many_unverified_identifications: | ||||||
| 'Too many verifications are pending on this account. Remove an unverified email address, phone number, or wallet you no longer need, or wait a few minutes for an incomplete passkey setup to expire, then try again.', | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
printf '%s\n' '--- diff ---'
git diff --no-ext-diff --unified=12 0ee4ee2b49286a8cae5f251ec997f30c8a863205 bdf2bda57ec8315c37038bfa4c8a0c0881acb6f9 -- packages/localizations/src/en-US.ts
printf '%s\n' '--- localization context ---'
sed -n '2148,2180p' packages/localizations/src/en-US.tsRepository: clerk/javascript Length of output: 4970 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- passkey expiry references ---'
rg -n -i -C 3 'passkey|unverified.identification|verification.*expir|expir.*verification|ttl' --glob '*.go' --glob '*.sql' --glob '*.yaml' --glob '*.yml' | head -n 500Repository: clerk/clerk_go Length of output: 38737 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- exact error and identification references ---'
rg -n -i -C 5 'too_many_unverified_identifications|unverified.?identification|unverified.*ident|ident.*unverified' --glob '!vendor/**' --glob '!node_modules/**' | head -n 600
printf '%s\n' '--- passkey-specific files and symbols ---'
rg -l -i 'passkey' --glob '!vendor/**' --glob '!node_modules/**' | head -n 200
printf '%s\n' '--- verification expiry candidates ---'
rg -n -i -C 4 'verification.*(ttl|expire|duration)|((ttl|expire|expiration).*(verification|identification))' --glob '!vendor/**' --glob '!node_modules/**' | head -n 400Repository: clerk/clerk_go Length of output: 42074 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- passkey references in backend and FAPI tests ---'
rg -n -i -C 4 'passkey' api/shared api/fapi tests/fapi tests/shared --glob '*.go' | head -n 800
printf '%s\n' '--- verification expiration assignments near passkey paths ---'
rg -n -i -C 5 'expire_at|expireat|expire.*time|expiration|expires' api/shared api/fapi --glob '*.go' | rg -i -C 3 'passkey|verification|identification|expire' | head -n 800Repository: clerk/clerk_go Length of output: 42101 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- passkey-named files ---'
fd -i 'passkey' . --type f | head -n 200
printf '%s\n' '--- passkey registration and expiry symbols ---'
rg -n -i -C 8 'create.*passkey|passkey.*(create|register)|ExpiryTime|ExpireAt' api/fapi/v1 api/shared constants tests/fapi --glob '*.go' | rg -i -C 4 'passkey|verification|expirytime|expireat' | head -n 1000Repository: clerk/clerk_go Length of output: 41666 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- passkey handler binding ---'
rg -n -C 8 'router\.passkeys|CreatePasskey|AttemptPasskeyVerification|Passkey.*Service|NewPasskey' api/fapi --glob '*.go' | head -n 1000
printf '%s\n' '--- generic verification creation ---'
sed -n '250,335p' api/shared/strategies/strategies.go
printf '%s\n' '--- verification creation callers and expiry constant ---'
rg -n -C 5 'createVerificationParams|ExpiryTimeTransactional' api/shared api/fapi pkg model --glob '*.go' | head -n 1000Repository: clerk/clerk_go Length of output: 41576 Use duration-neutral wording for passkey expiry. Passkey registration verifications expire after 10 minutes. “Wait a few minutes” can prompt users to retry while the registration still counts toward the pending-verification limit. Suggested wording- 'Too many verifications are pending on this account. Remove an unverified email address, phone number, or wallet you no longer need, or wait a few minutes for an incomplete passkey setup to expire, then try again.',
+ 'Too many verifications are pending on this account. Remove an unverified email address, phone number, or wallet you no longer need, or wait for an incomplete passkey setup to expire, then try again.',📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Path instructions |
||||||
| web3_missing_identifier: 'A Web3 Wallet extension cannot be found. Please install one to continue.', | ||||||
| web3_signature_request_rejected: 'You have rejected the signature request. Please try again to continue.', | ||||||
| web3_solana_signature_generation_failed: | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.