diff --git a/.changeset/mosaic-password-reverification.md b/.changeset/mosaic-password-reverification.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-password-reverification.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/__tests__/user-profile-password-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/__tests__/user-profile-password-section.feature.test.tsx index 632d9aef0b0..47293b80fe4 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/__tests__/user-profile-password-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/__tests__/user-profile-password-section.feature.test.tsx @@ -2,7 +2,7 @@ import { act, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { describe, expect, it, vi } from 'vitest'; -import { holdRequests, serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { demandReverificationOnce, holdRequests, serveFapi } from '../../../../__tests__/feature/fake-fapi'; import { fapiClient, fapiEmailAddress, @@ -23,6 +23,23 @@ async function renderPassword(user = alice, environment = fapiEnvironment()) { return fapi; } +async function renderWithReverification(prepare?: () => void) { + const fapi = serveFapi({ + environment: fapiEnvironment({ auth_config: { reverification: true } }), + client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' }] }, + }); + prepare?.(); + demandReverificationOnce('/v1/me/change_password'); + await renderWithClerk(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Change password' })); + await user.type(screen.getByLabelText('New password'), 'new-password-123'); + await user.type(screen.getByLabelText('Confirm password'), 'new-password-123'); + await user.click(screen.getByRole('button', { name: 'Save changes' })); + return { fapi, user }; +} + async function fillPassword() { const user = userEvent.setup(); await user.click(screen.getByRole('button', { name: 'Change password' })); @@ -197,6 +214,83 @@ describe('Changing a password', () => { await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); }); + it('skips the current password and asks to verify before saving when reverification is on', async () => { + const { fapi, user } = await renderWithReverification(); + + expect(screen.queryByLabelText('Current password')).toBeNull(); + const field = await screen.findByPlaceholderText('Enter your password'); + expect(fapi.passwordUpdates).toHaveLength(0); + await user.type(field, 'hunter2{Enter}'); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull(), { timeout: 3000 }); + expect(fapi.passwordUpdates).toHaveLength(1); + }); + + it('closes without saving when verification is dismissed, and reopens on an empty form', async () => { + const { fapi, user } = await renderWithReverification(); + + await screen.findByPlaceholderText('Enter your password'); + await user.keyboard('{Escape}'); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.passwordUpdates).toHaveLength(0); + await user.click(screen.getByRole('button', { name: 'Change password' })); + expect(screen.getByLabelText('New password')).toHaveValue(''); + expect(screen.getByRole('button', { name: 'Save changes' })).toBeVisible(); + }); + + it('cancels an in-progress password verification when the active session changes', async () => { + const fapi = serveFapi({ + environment: fapiEnvironment({ auth_config: { reverification: true } }), + client: fapiClient([fapiSession({ id: 'sess_1', user: alice }), fapiSession({ id: 'sess_2', user: alice })]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' }] }, + }); + demandReverificationOnce('/v1/me/change_password'); + const { clerk } = await renderWithClerk(); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Change password' })); + await user.type(screen.getByLabelText('New password'), 'new-password-123'); + await user.type(screen.getByLabelText('Confirm password'), 'new-password-123'); + await user.click(screen.getByRole('button', { name: 'Save changes' })); + await screen.findByPlaceholderText('Enter your password'); + + await act(() => clerk.setActive({ session: 'sess_2' })); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.passwordUpdates).toHaveLength(0); + }); + + it('returns to the form with the field error when the save fails after verifying', async () => { + const updates: Array> = []; + const { fapi, user } = await renderWithReverification(() => { + updates.push(holdRequests('post', '/v1/me/change_password')); + }); + const update = updates[0]; + + await user.type(await screen.findByPlaceholderText('Enter your password'), 'hunter2{Enter}'); + await waitFor(() => expect(update.requests).toHaveLength(1)); + update.fail('form_password_pwned', undefined, 'new_password'); + + const field = await screen.findByLabelText('New password', undefined, { timeout: 3000 }); + expect(field).toHaveValue('new-password-123'); + await waitFor(() => expect(field).toHaveAccessibleDescription(/found as part of a breach/)); + await waitFor(() => expect(field).toHaveFocus()); + expect(fapi.passwordUpdates).toHaveLength(0); + }); + + it('closes the dialog from the verification error card', async () => { + const { fapi, user } = await renderWithReverification(() => + holdRequests('post', '/v1/client/sessions/sess_1/verify').fail('some_new_error', 'Nope'), + ); + + expect(await screen.findByText('Something went wrong')).toBeVisible(); + const buttons = screen.getAllByRole('button', { name: 'Close' }); + await user.click(buttons[buttons.length - 1]); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.passwordUpdates).toHaveLength(0); + }); + it('shows a direct API error and keeps the draft without retrying automatically', async () => { const fapi = await renderPassword(); const user = await fillPassword(); @@ -204,11 +298,10 @@ describe('Changing a password', () => { await user.click(screen.getByRole('button', { name: 'Save changes' })); await waitFor(() => expect(update.requests).toHaveLength(1)); - update.fail('session_reverification_required'); + update.fail('form_param_invalid'); - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('session_reverification_required')); + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('form_param_invalid')); expect(screen.getByLabelText('New password')).toHaveValue('new-password-123'); - expect(screen.queryByText('Verification required')).toBeNull(); expect(update.requests).toHaveLength(1); serveFapi(fapi); await user.click(screen.getByRole('button', { name: 'Save changes' })); @@ -472,6 +565,4 @@ describe('Changing a password', () => { describe('Deferred password behavior', () => { it.todo('shows a password section skeleton while loading without a custom fallback'); - it.todo('reverifies the session and retries the password update when Clerk requires verification'); - it.todo('omits the current password when session reverification is enabled'); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.ts index f7c18a1dc99..53aa56d49f4 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.ts @@ -6,6 +6,7 @@ import { useForm } from '../../../components/form'; import type { FieldFeedback } from '../../../components/form/form-submit-error'; import { useDebouncedAsync } from '../../../hooks/use-debounced-async'; import { useMessages } from '../../../localization'; +import type { ReverificationController } from '../../reverification'; import type { UserProfileEditPasswordValue, UserProfileEditPasswordValues, @@ -23,6 +24,7 @@ export interface UserProfileEditPasswordControllerOptions { policy: UserProfilePasswordPolicy; onSubmit: (value: UserProfileEditPasswordValue) => Promise; validatePassword?: (password: string) => Promise; + reverification?: ReverificationController; } export interface UserProfileEditPasswordController { @@ -30,12 +32,14 @@ export interface UserProfileEditPasswordController { onOpenChange: (open: boolean) => void; form: UseFormResult; passwordFeedback: FieldFeedback | undefined; + step: 'edit' | 'verify'; } export function useUserProfileEditPasswordController({ policy, onSubmit, validatePassword, + reverification, }: UserProfileEditPasswordControllerOptions): UserProfileEditPasswordController { const requiresCurrentPassword = policy.requiresCurrentPassword; const validationError = useMessages('errors').generic; @@ -78,13 +82,18 @@ export function useUserProfileEditPasswordController({ ? { type: 'error', message: validationError } : strength.data; + const step = form.isSubmitting && reverification?.visible ? 'verify' : 'edit'; + const onOpenChange = (open: boolean) => { if (form.isSubmitting) { - return; + if (open || !reverification?.onCancel) { + return; + } + reverification.onCancel(); } form.reset(); setIsOpen(open); }; - return { isOpen, onOpenChange, form, passwordFeedback }; + return { isOpen, onOpenChange, form, passwordFeedback, step }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.dialog.tsx index fb42feb1068..622a435fc98 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.dialog.tsx @@ -8,10 +8,13 @@ import { Checkbox } from '../../../components/checkbox'; import type { DialogTriggerProps } from '../../../components/dialog'; import { Dialog } from '../../../components/dialog'; import { Field } from '../../../components/field'; +import { Flow, useFlowAutoFocus } from '../../../components/flow'; import type { FieldFeedback, UseFormResult } from '../../../components/form'; import { Icon } from '../../../components/icon'; import { InputGroup } from '../../../components/input-group'; import { useMessages } from '../../../localization'; +import type { ReverificationController } from '../../reverification'; +import { Reverification } from '../../reverification'; import type { UserProfileEditPasswordField, UserProfileEditPasswordValues, @@ -26,6 +29,8 @@ export interface UserProfileEditPasswordDialogProps { hasPassword?: boolean; requiresCurrentPassword?: boolean; form: UseFormResult; + reverification?: ReverificationController; + step: 'edit' | 'verify'; } export function UserProfileEditPasswordDialog({ @@ -37,15 +42,107 @@ export function UserProfileEditPasswordDialog({ hasPassword = false, requiresCurrentPassword = false, form, + reverification, + step, }: UserProfileEditPasswordDialogProps) { const m = useMessages('userProfilePasswordSection'); const initialFocusRef = useRef(null); const showCurrentPassword = hasPassword && requiresCurrentPassword; + const edit = ( + <> + + {hasPassword ? m.dialogTitle.change : m.dialogTitle.set} + + + {form.error} + + + } + > + + {showCurrentPassword ? ( + + ) : null} + + + + form.setValue('signOutOfOtherSessions', event.target.checked)} + /> + + {m.signOutOfOtherSessionsLabel} + {m.signOutOfOtherSessionsDescription} + + + + + + {m.cancel} + + } + /> + + {m.save} + + + + ); + return ( {trigger ? : null} - - {hasPassword ? m.dialogTitle.change : m.dialogTitle.set} - - - {form.error} - - - } - > - - {showCurrentPassword ? ( - - ) : null} - - - - form.setValue('signOutOfOtherSessions', event.target.checked)} - /> - - {m.signOutOfOtherSessionsLabel} - {m.signOutOfOtherSessionsDescription} - - - - - - {m.cancel} - - } - /> - - {m.save} - - + {() => ( + <> + {edit} + + onOpenChange(false)} + /> + + + )} + + ) : ( + edit + )} @@ -167,7 +202,7 @@ function PasswordField({ const message = feedback?.type === 'error' ? feedback : advisoryFeedback; const feedbackType = message?.type === 'info' && !focused ? 'error' : message?.type; const { ref, ...control } = form.register(name); - const mergedRef = useMergeRefs([ref, inputRef]); + const mergedRef = useMergeRefs([ref, inputRef, useFlowAutoFocus()]); // TODO: Discuss enforcing the configured minimum length on the new password input or keeping the hint advisory and letting the server validate. https://github.com/clerk/javascript/pull/9930#discussion_r4151734254 return ( diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.ts index 1c6f10716a3..0c3a7d01641 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.ts @@ -1,3 +1,4 @@ +import { isReverificationCancelledError } from '@clerk/shared/error'; import { validate as validateComplexity } from '@clerk/shared/internal/clerk-js/passwords/complexity'; import { createLoadZxcvbn } from '@clerk/shared/internal/clerk-js/passwords/loadZxcvbn'; import { createValidatePasswordStrength } from '@clerk/shared/internal/clerk-js/passwords/strength'; @@ -9,6 +10,8 @@ import type { FieldFeedback } from '../../../components/form'; import { FormSubmitError } from '../../../components/form'; import { useMosaicEnvironment } from '../../../hooks/use-mosaic-environment'; import { useErrorText, useLocale, useMessages } from '../../../localization'; +import type { ReverificationController } from '../../reverification'; +import { useReverificationFlow } from '../../reverification'; import { passwordFormError } from './user-profile-password-errors'; import { passwordFieldFeedback } from './user-profile-password-feedback'; import type { UserProfileEditPasswordValue, UserProfilePasswordPolicy } from './user-profile-password-section.types'; @@ -31,8 +34,31 @@ export type UserProfilePasswordModel = identifier: string; validatePassword: (password: string) => Promise; updatePassword: (input: UserProfileEditPasswordValue) => Promise; + reverification: ReverificationController; }); +type PasswordUpdateTarget = UserProfilePasswordPolicy & { userId: string; sessionId: string }; + +function getAvailablePasswordUser( + user: UserResource | null | undefined, + sessionId: string | null | undefined, + environment: EnvironmentResource | undefined, + target: PasswordUpdateTarget, + unavailableMessage: string, +): UserResource { + if ( + !user || + user.id !== target.userId || + sessionId !== target.sessionId || + !environment?.userSettings.instanceIsPasswordBased || + user.enterpriseAccounts.some(account => account.active) || + user.passwordEnabled !== (target.mode === 'change') + ) { + throw new FormSubmitError({ message: unavailableMessage }); + } + return user; +} + type PasswordPolicyResult = | { status: 'hidden' } | { status: 'readonly'; mode: 'set' | 'change'; enterpriseConnectionName: string | undefined } @@ -50,9 +76,8 @@ function getPasswordPolicy( return { status: 'hidden' }; } - // TODO: When session reverification is supported, require the current password only when reverification is disabled. const policy: UserProfilePasswordPolicy = user.passwordEnabled - ? { mode: 'change', requiresCurrentPassword: true } + ? { mode: 'change', requiresCurrentPassword: !environment.authConfig.reverification } : { mode: 'set', requiresCurrentPassword: false }; const enterpriseAccount = user.enterpriseAccounts.find(account => account.active); @@ -98,6 +123,24 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { [passwordSettings, moduleManager, m, locale, errorMessages], ); + const [updatePasswordWithReverification, reverification] = useReverificationFlow( + async (input: UserProfileEditPasswordValue, target: PasswordUpdateTarget) => { + const currentUser = getAvailablePasswordUser( + clerk.user, + clerk.session?.id, + environment, + target, + m.errors.unavailable, + ); + + await currentUser.updatePassword({ + newPassword: input.newPassword, + signOutOfOtherSessions: input.signOutOfOtherSessions, + ...(target.requiresCurrentPassword ? { currentPassword: input.currentPassword } : {}), + }); + }, + ); + if (!isUserLoaded || !isSessionLoaded || !environment) { return { status: 'loading' }; } @@ -118,35 +161,23 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { return policy; } - const userId = policy.userId; const sessionId = session.id; + const target: PasswordUpdateTarget = { ...policy, sessionId }; return { ...policy, sessionId, identifier: session.publicUserData.identifier ?? '', validatePassword, - // TODO: Add session reverification for password updates; surface API errors until then. - updatePassword: async ({ currentPassword, newPassword, signOutOfOtherSessions }) => { - const currentUser = clerk.user; - if ( - !currentUser || - currentUser.id !== userId || - clerk.session?.id !== sessionId || - !environment.userSettings.instanceIsPasswordBased || - currentUser.enterpriseAccounts.some(account => account.active) || - currentUser.passwordEnabled !== (policy.mode === 'change') - ) { - throw new FormSubmitError({ message: m.errors.unavailable }); - } - + reverification, + updatePassword: async input => { try { - await currentUser.updatePassword({ - newPassword, - signOutOfOtherSessions, - ...(policy.requiresCurrentPassword ? { currentPassword } : {}), - }); + getAvailablePasswordUser(clerk.user, clerk.session?.id, environment, target, m.errors.unavailable); + await updatePasswordWithReverification(input, target); } catch (error) { + if (isReverificationCancelledError(error) || error instanceof FormSubmitError) { + throw error; + } throw passwordFormError( error, policy.requiresCurrentPassword, diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.tsx index 6a7272f8c4a..43d990b05ae 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.tsx @@ -43,6 +43,7 @@ function PasswordEditor({ model }: { model: Extract