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..ff4bc8b5c9e 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 @@ -1,8 +1,9 @@ import { act, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { http, HttpResponse } from 'msw'; import { describe, expect, it, vi } from 'vitest'; -import { holdRequests, serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { fapiUrl, holdRequests, serveFapi, worker } from '../../../../__tests__/feature/fake-fapi'; import { fapiClient, fapiEmailAddress, @@ -23,6 +24,34 @@ async function renderPassword(user = alice, environment = fapiEnvironment()) { return fapi; } +async function renderWithReverification() { + const fapi = serveFapi({ + environment: fapiEnvironment({ auth_config: { reverification: true } }), + client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' }] }, + }); + let required = false; + worker.use( + http.post(fapiUrl('/v1/me/change_password'), () => { + if (required) { + return undefined; + } + required = true; + return HttpResponse.json( + { errors: [{ code: 'session_reverification_required', message: 'Verification required' }] }, + { status: 403 }, + ); + }), + ); + 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 +226,31 @@ 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('shows a direct API error and keeps the draft without retrying automatically', async () => { const fapi = await renderPassword(); const user = await fillPassword(); @@ -204,11 +258,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' })); 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..bb3db92134a 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 @@ -1,11 +1,14 @@ +import { isReverificationHint } from '@clerk/shared/authorization-errors'; +import { isReverificationCancelledError } from '@clerk/shared/error'; import { DEBOUNCE_MS } from '@clerk/shared/internal/clerk-js/constants'; import { useState } from 'react'; import type { UseFormResult } from '../../../components/form'; -import { useForm } from '../../../components/form'; +import { FormSubmitError, 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 +26,8 @@ export interface UserProfileEditPasswordControllerOptions { policy: UserProfilePasswordPolicy; onSubmit: (value: UserProfileEditPasswordValue) => Promise; validatePassword?: (password: string) => Promise; + formatError?: (error: unknown) => unknown; + reverification?: ReverificationController; } export interface UserProfileEditPasswordController { @@ -30,12 +35,16 @@ export interface UserProfileEditPasswordController { onOpenChange: (open: boolean) => void; form: UseFormResult; passwordFeedback: FieldFeedback | undefined; + reverification?: ReverificationController; + step: 'edit' | 'verify'; } export function useUserProfileEditPasswordController({ policy, onSubmit, validatePassword, + formatError = error => error, + reverification, }: UserProfileEditPasswordControllerOptions): UserProfileEditPasswordController { const requiresCurrentPassword = policy.requiresCurrentPassword; const validationError = useMessages('errors').generic; @@ -56,11 +65,22 @@ export function useUserProfileEditPasswordController({ values.confirmPassword === values.newPassword && (!requiresCurrentPassword || values.currentPassword !== ''), onSubmit: async values => { - await onSubmit({ - currentPassword: requiresCurrentPassword ? values.currentPassword : undefined, - newPassword: values.newPassword, - signOutOfOtherSessions: values.signOutOfOtherSessions, - }); + reverification?.reset(); + try { + const result = await onSubmit({ + currentPassword: requiresCurrentPassword ? values.currentPassword : undefined, + newPassword: values.newPassword, + signOutOfOtherSessions: values.signOutOfOtherSessions, + }); + if (isReverificationHint(result)) { + throw new FormSubmitError({ message: m.errors.verificationIncomplete }); + } + } catch (error) { + if (isReverificationCancelledError(error)) { + return; + } + throw formatError(error); + } // TODO: Discuss confirming the password was set or updated and other devices were signed out with a success page or toast. https://github.com/clerk/javascript/pull/9930#discussion_r4151641473 setIsOpen(false); }, @@ -78,13 +98,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, reverification, 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..3d22196c125 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 @@ -1,6 +1,6 @@ import { useMergeRefs } from '@floating-ui/react'; import type { RefObject } from 'react'; -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { Button, SubmitButton } from '../../../components/button'; import { Card } from '../../../components/card'; @@ -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 } 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,10 +42,129 @@ export function UserProfileEditPasswordDialog({ hasPassword = false, requiresCurrentPassword = false, form, + reverification, + step = 'edit', }: UserProfileEditPasswordDialogProps) { const m = useMessages('userProfilePasswordSection'); const initialFocusRef = useRef(null); const showCurrentPassword = hasPassword && requiresCurrentPassword; + const returnFocus = useRef(false); + + useEffect(() => { + if (step === 'verify') { + returnFocus.current = true; + } else if (returnFocus.current) { + returnFocus.current = false; + initialFocusRef.current?.focus({ preventScroll: true }); + } + }, [step]); + + 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} + + + + ); + + const content = reverification ? ( + + {() => ( + <> + {edit} + + + + + )} + + ) : ( + edit + ); return ( - - {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} - - + {content} diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.messages.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.messages.ts index b9055babc0b..9a55023be80 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.messages.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.messages.ts @@ -54,6 +54,7 @@ export const userProfilePasswordSectionMessages = { }, errors: { + verificationIncomplete: 'Your password was not saved. Please try verifying again.', unavailable: 'Password update is no longer available.', currentPasswordRequired: 'Current password is required.', mismatch: "Passwords don't match.", 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..780b2ed216b 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 @@ -30,7 +30,8 @@ export type UserProfilePasswordModel = sessionId: string; identifier: string; validatePassword: (password: string) => Promise; - updatePassword: (input: UserProfileEditPasswordValue) => Promise; + updatePassword: (input: UserProfileEditPasswordValue) => Promise; + formatError: (error: unknown) => unknown; }); type PasswordPolicyResult = @@ -50,9 +51,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); @@ -126,7 +126,6 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { 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 ( @@ -140,22 +139,22 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { throw new FormSubmitError({ message: m.errors.unavailable }); } - try { - await currentUser.updatePassword({ - newPassword, - signOutOfOtherSessions, - ...(policy.requiresCurrentPassword ? { currentPassword } : {}), - }); - } catch (error) { - throw passwordFormError( - error, - policy.requiresCurrentPassword, - environment.userSettings.passwordSettings, - m, - locale, - errorText, - ); - } + return currentUser.updatePassword({ + newPassword, + signOutOfOtherSessions, + ...(policy.requiresCurrentPassword ? { currentPassword } : {}), + }); }, + formatError: error => + error instanceof FormSubmitError + ? error + : passwordFormError( + error, + policy.requiresCurrentPassword, + environment.userSettings.passwordSettings, + m, + locale, + errorText, + ), }; } 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 379431a53f3..ab5fc7c5429 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 @@ -2,6 +2,7 @@ import type { ReactNode } from 'react'; import { Button } from '../../../components/button'; import { useMessages } from '../../../localization'; +import { useReverificationFlow } from '../../reverification'; import { useUserProfileEditPasswordController } from './user-profile-edit-password.controller'; import { UserProfileEditPasswordDialog } from './user-profile-edit-password.dialog'; import type { UserProfilePasswordModel } from './user-profile-password-section.model'; @@ -43,10 +44,13 @@ export function renderPasswordSection(model: UserProfilePasswordModel, fallback: function PasswordEditor({ model }: { model: Extract }) { const m = useMessages('userProfilePasswordSection'); const hasPassword = model.mode === 'change'; + const [updatePassword, reverification] = useReverificationFlow(model.updatePassword); const controller = useUserProfileEditPasswordController({ policy: model, validatePassword: model.validatePassword, - onSubmit: model.updatePassword, + onSubmit: updatePassword, + formatError: model.formatError, + reverification, }); return ( @@ -61,6 +65,8 @@ function PasswordEditor({ model }: { model: Extract