diff --git a/.changeset/password-feedback-tests.md b/.changeset/password-feedback-tests.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/password-feedback-tests.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx index 8d208349fd6..8c23fb41c58 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx @@ -63,6 +63,17 @@ function renderView(overrides: Partial = {}) } describe('UserProfileSecurityPanelView', () => { + it('renders supplied password content without a separate visibility flag', () => { + renderView({ + passwordSlot:
Password
, + passkeys: undefined, + mfaMethods: undefined, + }); + + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); + expect(screen.getByText('Password')).toBeVisible(); + }); + it('composes authentication, active devices, and the danger zone', () => { renderView({ dangerSlot: }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.test.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.test.ts index 2ab0426e474..dfd988fd47e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-edit-password.controller.test.ts @@ -11,6 +11,8 @@ describe('useUserProfileEditPasswordController timing', () => { try { const { result } = renderHook(() => useUserProfileEditPasswordController({ + policy: { mode: 'set', requiresCurrentPassword: false }, + identifier: '', onSubmit: () => Promise.resolve(), validatePassword: () => Promise.reject(new Error('Failed to load strength checker')), }), @@ -30,6 +32,8 @@ describe('useUserProfileEditPasswordController timing', () => { const validatePassword = vi.fn(() => Promise.resolve(feedback)); const { result } = renderHook(() => useUserProfileEditPasswordController({ + policy: { mode: 'set', requiresCurrentPassword: false }, + identifier: '', onSubmit: () => Promise.resolve(), validatePassword, }), @@ -51,7 +55,12 @@ describe('useUserProfileEditPasswordController timing', () => { try { const validatePassword = vi.fn(() => Promise.resolve(undefined)); const { result } = renderHook(() => - useUserProfileEditPasswordController({ onSubmit: () => Promise.resolve(), validatePassword }), + useUserProfileEditPasswordController({ + policy: { mode: 'set', requiresCurrentPassword: false }, + identifier: '', + onSubmit: () => Promise.resolve(), + validatePassword, + }), ); act(() => result.current.onOpenChange(true)); act(() => result.current.form.setValue('newPassword', 'first password')); @@ -75,7 +84,12 @@ describe('useUserProfileEditPasswordController timing', () => { const newer = deferred(); const validatePassword = vi.fn().mockReturnValueOnce(older.promise).mockReturnValueOnce(newer.promise); const { result } = renderHook(() => - useUserProfileEditPasswordController({ onSubmit: () => Promise.resolve(), validatePassword }), + useUserProfileEditPasswordController({ + policy: { mode: 'set', requiresCurrentPassword: false }, + identifier: '', + onSubmit: () => Promise.resolve(), + validatePassword, + }), ); act(() => result.current.onOpenChange(true)); act(() => result.current.form.setValue('newPassword', 'first password')); @@ -101,7 +115,11 @@ describe('useUserProfileEditPasswordController timing', () => { const save = deferred(); const onSubmit = vi.fn(() => save.promise); const { result } = renderHook(() => - useUserProfileEditPasswordController({ requiresCurrentPassword: true, onSubmit }), + useUserProfileEditPasswordController({ + policy: { mode: 'change', requiresCurrentPassword: true }, + identifier: '', + onSubmit, + }), ); act(() => result.current.onOpenChange(true)); act(() => result.current.form.setValue('currentPassword', 'old-secret')); 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 f28707b9884..9062f48b3a4 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 @@ -8,6 +8,7 @@ import { useMessages } from '../../../localization'; import type { UserProfileEditPasswordValue, UserProfileEditPasswordValues, + UserProfilePasswordPolicy, } from './user-profile-password-section.types'; const initialValues: UserProfileEditPasswordValues = { @@ -18,12 +19,16 @@ const initialValues: UserProfileEditPasswordValues = { }; export interface UserProfileEditPasswordControllerOptions { - requiresCurrentPassword?: boolean; + policy: UserProfilePasswordPolicy; + identifier: string; onSubmit: (value: UserProfileEditPasswordValue) => Promise; validatePassword?: (password: string) => Promise; } export interface UserProfileEditPasswordController { + hasPassword: boolean; + identifier: string; + requiresCurrentPassword: boolean; isOpen: boolean; onOpenChange: (open: boolean) => void; form: UseFormResult; @@ -31,10 +36,13 @@ export interface UserProfileEditPasswordController { } export function useUserProfileEditPasswordController({ - requiresCurrentPassword = false, + policy, + identifier, onSubmit, validatePassword, }: UserProfileEditPasswordControllerOptions): UserProfileEditPasswordController { + const hasPassword = policy.mode === 'change'; + const requiresCurrentPassword = policy.requiresCurrentPassword; const validationError = useMessages('errors').generic; const m = useMessages('userProfilePasswordSection'); const [isOpen, setIsOpen] = useState(false); @@ -104,5 +112,5 @@ export function useUserProfileEditPasswordController({ setIsOpen(open); }; - return { isOpen, onOpenChange, form, passwordFeedback }; + return { hasPassword, identifier, requiresCurrentPassword, isOpen, onOpenChange, form, passwordFeedback }; } diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx similarity index 69% rename from packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx rename to packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx index 23aae55191b..c20b63f24c7 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx @@ -1,7 +1,6 @@ -import { screen, waitFor } from '@testing-library/react'; +import { act, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import type { ReactNode } from 'react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; import { @@ -13,20 +12,11 @@ import { fapiUser, } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; -import { - UserProfilePasswordSection, - useUserProfilePasswordSlot, -} from '../user-profile-password-section/user-profile-password-section'; -import { UserProfileSecurityPanelView } from '../user-profile-security-panel.view'; +import { UserProfilePasswordSection } from './user-profile-password-section'; const email = fapiEmailAddress({ id: 'idn_1', email_address: 'person@example.com' }); const alice = fapiUser({ id: 'user_1', email_addresses: [email] }); -function PasswordSecurityPanel({ fallback }: { fallback?: ReactNode }) { - const passwordSlot = useUserProfilePasswordSlot({ fallback }); - return ; -} - async function renderPassword(user = alice, environment = fapiEnvironment()) { const fapi = serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user })]) }); await renderWithClerk(); @@ -43,35 +33,115 @@ async function fillPassword() { } describe('Changing a password', () => { - it('omits Authentication while the only method loads without a fallback', async () => { + it.each(['sign out', 'switch user', 'switch session'] as const)( + 'discards the password draft after %s', + async change => { + const fapi = serveFapi({ + client: fapiClient([ + fapiSession({ id: 'sess_1', user: alice }), + fapiSession({ id: 'sess_2', user: change === 'switch user' ? fapiUser({ id: 'user_2' }) : alice }), + ]), + }); + const { clerk } = await renderWithClerk(); + const user = await fillPassword(); + + await act(() => (change === 'sign out' ? clerk.signOut() : clerk.setActive({ session: 'sess_2' }))); + + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.passwordUpdates).toHaveLength(0); + if (change === 'sign out') { + expect(screen.queryByText('Password')).toBeNull(); + } else { + expect(clerk.session?.id).toBe('sess_2'); + await user.click(screen.getByRole('button', { name: 'Change password' })); + expect(screen.getByLabelText('Current password')).toHaveValue(''); + expect(screen.getByLabelText('New password')).toHaveValue(''); + expect(screen.getByLabelText('Confirm password')).toHaveValue(''); + } + }, + ); + + it.each(['password mode', 'enterprise management'] as const)( + 'rejects saving an open dialog after the %s changes', + async change => { + const fapi = serveFapi({ + client: fapiClient([ + fapiSession({ + id: 'sess_1', + user: fapiUser({ + ...alice, + enterprise_accounts: [fapiEnterpriseAccount({ id: 'ent_1', active: false })], + }), + }), + ]), + }); + const { clerk } = await renderWithClerk(); + const user = await fillPassword(); + const dialog = screen.getByRole('dialog'); + const currentUser = clerk.user; + const account = currentUser?.enterpriseAccounts[0]; + if (!currentUser || !account) { + throw new Error('Expected a signed-in user with an enterprise account'); + } + if (change === 'password mode') { + currentUser.passwordEnabled = false; + } else { + account.active = true; + } + await user.click(screen.getByRole('button', { name: 'Save changes' })); + + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Password update is no longer available.'), + ); + expect(screen.getByRole('dialog')).toBe(dialog); + expect(screen.getByLabelText('New password')).toHaveValue('new-password-123'); + expect(fapi.passwordUpdates).toHaveLength(0); + }, + ); + + it('shows an unexpected update failure in the dialog and keeps the draft', async () => { serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); - const loading = renderWithClerk(); + const { clerk } = await renderWithClerk(); + if (!clerk.user) { + throw new Error('Expected a signed-in user'); + } + const update = vi.spyOn(clerk.user, 'updatePassword').mockRejectedValue(new Error('Connection interrupted')); try { - expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); + const user = await fillPassword(); + await user.click(screen.getByRole('button', { name: 'Save changes' })); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Connection interrupted')); + expect(screen.getByLabelText('Current password')).toHaveValue('old-secret'); + expect(screen.getByLabelText('New password')).toHaveValue('new-password-123'); + expect(screen.getByLabelText('Confirm password')).toHaveValue('new-password-123'); } finally { - await loading; + update.mockRestore(); } - expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); }); - it('keeps Authentication around a visible loading fallback', async () => { - serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); - const loading = renderWithClerk(Loading password section} />); + it('shows feedback when the password strength checker cannot load', async () => { + const environment = fapiEnvironment(); + environment.user_settings.password_settings.show_zxcvbn = true; + serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + const { clerk } = await renderWithClerk(); + const modules = clerk.__internal_moduleManager; + if (!modules) { + throw new Error('Expected a module manager'); + } + const load = vi.spyOn(modules, 'import').mockRejectedValue(new Error('Failed to load password strength checker')); try { - expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Loading password section'); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: 'Change password' })); + await user.type(screen.getByLabelText('New password'), 'new-password-123'); + + await waitFor(() => + expect(screen.getByLabelText('New password')).toHaveAccessibleDescription( + 'Something went wrong. Please try again.', + ), + ); } finally { - await loading; + load.mockRestore(); } - expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); - expect(screen.queryByText('Loading password section')).toBeNull(); - }); - - it('shows no password action when nobody is signed in', async () => { - serveFapi({ client: fapiClient() }); - await renderWithClerk(); - - expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); - expect(screen.queryByText('Password')).toBeNull(); }); it('sends the update to Clerk and closes after it succeeds', async () => { @@ -141,6 +211,26 @@ describe('Changing a password', () => { expect(screen.getByRole('alert').textContent).toBe(''); }); + it('shows an incorrect current password error at the field and keeps the draft', async () => { + await renderPassword(); + const user = await fillPassword(); + const update = holdRequests('post', '/v1/me/change_password'); + + await user.click(screen.getByRole('button', { name: 'Save changes' })); + await waitFor(() => expect(update.requests).toHaveLength(1)); + update.fail('form_password_incorrect', undefined, 'current_password'); + + await waitFor(() => + expect(screen.getByLabelText('Current password')).toHaveAccessibleDescription( + 'Your current password is incorrect.', + ), + ); + expect(screen.getByLabelText('Current password')).toHaveValue('old-secret'); + expect(screen.getByLabelText('New password')).toHaveValue('new-password-123'); + expect(screen.getByLabelText('Confirm password')).toHaveValue('new-password-123'); + expect(screen.getByRole('alert').textContent).toBe(''); + }); + it('sets a first password without asking for the current one', async () => { const fapi = await renderPassword(fapiUser({ ...alice, password_enabled: false })); const user = userEvent.setup(); @@ -163,22 +253,6 @@ describe('Changing a password', () => { expect(screen.queryByRole('button', { name: 'Change password' })).toBeNull(); }); - it.each(['disabled', 'editable', 'managed'])('resolves the Authentication section for %s passwords', async policy => { - const environment = fapiEnvironment(); - environment.user_settings.attributes.password.enabled = policy !== 'disabled'; - const user = fapiUser({ - ...alice, - enterprise_accounts: policy === 'managed' ? [fapiEnterpriseAccount({ id: 'ent_1' })] : [], - }); - serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user })]) }); - await renderWithClerk(); - if (policy === 'disabled') { - expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); - } else { - expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); - } - }); - it.each([true, false])('shows the managed view when passwordEnabled is %s', async passwordEnabled => { await renderPassword( fapiUser({ @@ -211,6 +285,19 @@ describe('Changing a password', () => { expect(screen.queryByRole('button', { name: 'Change password' })).toBeNull(); }); + it('shows the generic provider label when the enterprise connection name is blank', async () => { + const account = fapiEnterpriseAccount({ id: 'ent_1' }); + if (!account.enterprise_connection) { + throw new Error('Expected an enterprise connection'); + } + account.enterprise_connection.name = ''; + await renderPassword(fapiUser({ ...alice, enterprise_accounts: [account] })); + + expect(screen.getByText('Managed by your enterprise connection')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Change password' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Set password' })).toBeNull(); + }); + it('focuses the current password and clears the draft after cancellation', async () => { await renderPassword(); const user = userEvent.setup(); @@ -340,5 +427,4 @@ describe('Changing a password', () => { describe('Deferred password behavior', () => { 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'); - it.todo('shows a password section skeleton while loading without a custom fallback'); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.test.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.test.ts deleted file mode 100644 index bca8f2fc05b..00000000000 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.test.ts +++ /dev/null @@ -1,189 +0,0 @@ -import { ClerkAPIResponseError } from '@clerk/shared/error'; -import type { PasswordSettingsData } from '@clerk/shared/types'; -import { cleanup, renderHook } from '@testing-library/react'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { FormSubmitError } from '../../../components/form'; -import { useUserProfilePasswordModel } from './user-profile-password-section.model'; - -type TestUser = { - id: string; - passwordEnabled: boolean; - enterpriseAccounts: { active: boolean; enterpriseConnection?: { name: string; logoPublicUrl: string | null } }[]; - updatePassword: ReturnType; -}; - -type TestSession = { id: string; publicUserData: { identifier: string | null } }; - -const passwordSettings: PasswordSettingsData = { - min_length: 8, - max_length: 72, - require_numbers: false, - require_uppercase: false, - require_lowercase: false, - require_special_char: false, - allowed_special_characters: '', - disable_hibp: false, - show_zxcvbn: true, - min_zxcvbn_strength: 2, -}; - -function createEnvironment() { - return { userSettings: { instanceIsPasswordBased: true, passwordSettings } }; -} - -let user: TestUser | null; -let session: TestSession | null; -let environment: ReturnType; - -const clerk = { - get user() { - return user; - }, - get session() { - return session; - }, - get __internal_environment() { - return environment; - }, -}; - -vi.mock('@clerk/shared/react', () => ({ - useClerk: () => clerk, - useUser: () => ({ isLoaded: true, user }), - useSession: () => ({ isLoaded: true, session }), -})); - -beforeEach(() => { - user = { id: 'user_1', passwordEnabled: true, enterpriseAccounts: [], updatePassword: vi.fn() }; - session = { id: 'session_1', publicUserData: { identifier: 'person@example.com' } }; - environment = createEnvironment(); -}); - -afterEach(cleanup); - -describe('useUserProfilePasswordModel update errors', () => { - it('reports an unavailable strength checker instead of silently skipping it', async () => { - const { result } = renderHook(() => useUserProfilePasswordModel()); - await expect(ready(result.current).validatePassword('long password with 123')).rejects.toBeInstanceOf( - FormSubmitError, - ); - }); - it('translates API errors into form field errors before leaving the model', async () => { - if (!user) { - throw new Error('expected user'); - } - user.updatePassword.mockRejectedValue( - new ClerkAPIResponseError('Invalid', { - status: 422, - data: [{ code: 'form_password_incorrect', message: 'raw', meta: { param_name: 'current_password' } }], - }), - ); - const { result } = renderHook(() => useUserProfilePasswordModel()); - const action = ready(result.current).updatePassword; - const input = { currentPassword: 'wrong', newPassword: 'new password', signOutOfOtherSessions: true }; - await expect(action(input)).rejects.toBeInstanceOf(FormSubmitError); - await expect(action(input)).rejects.toMatchObject({ - fields: { currentPassword: 'Your current password is incorrect.' }, - }); - }); - - it('keeps unexpected failures behind the form error contract', async () => { - if (!user) { - throw new Error('expected user'); - } - user.updatePassword.mockRejectedValue(new Error('Connection interrupted')); - const { result } = renderHook(() => useUserProfilePasswordModel()); - await expect( - ready(result.current).updatePassword({ - currentPassword: 'old password', - newPassword: 'new password', - signOutOfOtherSessions: true, - }), - ).rejects.toBeInstanceOf(FormSubmitError); - }); -}); - -function ready(model: ReturnType) { - if (model.status !== 'ready') { - throw new Error('expected ready model'); - } - return model; -} - -describe('useUserProfilePasswordModel context changes', () => { - it.each(['signed out', 'different user', 'different session', 'no session', 'disabled', 'enterprise', 'mode'])( - 'rejects a captured action after %s', - async change => { - if (!user || !session) { - throw new Error('expected loaded fixtures'); - } - const updatePassword = user.updatePassword; - const { result, rerender } = renderHook(() => useUserProfilePasswordModel()); - const action = ready(result.current).updatePassword; - - switch (change) { - case 'signed out': - user = null; - break; - case 'different user': - user = { ...user, id: 'user_2' }; - break; - case 'different session': - session = { ...session, id: 'session_2' }; - break; - case 'no session': - session = null; - break; - case 'disabled': - environment.userSettings.instanceIsPasswordBased = false; - break; - case 'enterprise': - user.enterpriseAccounts = [{ active: true }]; - break; - case 'mode': - user.passwordEnabled = false; - break; - } - - const input = { currentPassword: 'old password', newPassword: 'new password', signOutOfOtherSessions: true }; - await expect(action(input)).rejects.toMatchObject({ banner: 'Password update is no longer available.' }); - rerender(); - await expect(action(input)).rejects.toMatchObject({ banner: 'Password update is no longer available.' }); - expect(updatePassword).not.toHaveBeenCalled(); - }, - ); - - it('hides the section when a loaded user has no active session', () => { - session = null; - const { result } = renderHook(() => useUserProfilePasswordModel()); - expect(result.current).toEqual({ status: 'hidden' }); - }); -}); - -describe('useUserProfilePasswordModel enterprise accounts', () => { - it('describes the managing connection as plain data', () => { - if (!user) { - throw new Error('expected user'); - } - user.enterpriseAccounts = [ - { active: false, enterpriseConnection: { name: 'Inactive', logoPublicUrl: null } }, - { active: true, enterpriseConnection: { name: 'Acme SSO', logoPublicUrl: 'https://example.com/acme.png' } }, - ]; - const { result } = renderHook(() => useUserProfilePasswordModel()); - expect(result.current).toEqual({ - status: 'readonly', - mode: 'change', - managedBy: { name: 'Acme SSO' }, - }); - }); - - it('leaves a blank connection name undefined', () => { - if (!user) { - throw new Error('expected user'); - } - user.enterpriseAccounts = [{ active: true, enterpriseConnection: { name: '', logoPublicUrl: null } }]; - const { result } = renderHook(() => useUserProfilePasswordModel()); - expect(result.current).toMatchObject({ managedBy: { name: undefined } }); - }); -}); 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 94fb2ccf737..50c943520bc 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 @@ -11,24 +11,20 @@ import { useMosaicEnvironment } from '../../../hooks/use-mosaic-environment'; import { useErrorText, useLocale, useMessages } from '../../../localization'; import { passwordFormError } from './user-profile-password-errors'; import { passwordFieldFeedback } from './user-profile-password-feedback'; -import type { UserProfileEditPasswordValue } from './user-profile-password-section.types'; - -type EditablePasswordPolicy = - | { mode: 'set'; requiresCurrentPassword: false } - | { mode: 'change'; requiresCurrentPassword: boolean }; +import type { UserProfileEditPasswordValue, UserProfilePasswordPolicy } from './user-profile-password-section.types'; type UnavailablePasswordModel = | { status: 'hidden' } | { status: 'readonly'; mode: 'set' | 'change'; - managedBy: { name?: string }; + managedBy: { name: string }; }; export type UserProfilePasswordModel = | { status: 'loading' } | UnavailablePasswordModel - | (EditablePasswordPolicy & { + | (UserProfilePasswordPolicy & { status: 'ready'; userId: string; sessionId: string; @@ -40,7 +36,8 @@ export type UserProfilePasswordModel = function getPasswordPolicy( user: UserResource | null | undefined, environment: EnvironmentResource, -): UnavailablePasswordModel | (EditablePasswordPolicy & { status: 'ready'; userId: string }) { + enterpriseConnectionName: string, +): UnavailablePasswordModel | (UserProfilePasswordPolicy & { status: 'ready'; userId: string }) { if (!user) { return { status: 'hidden' }; } @@ -50,7 +47,7 @@ function getPasswordPolicy( } // TODO: When session reverification is supported, require the current password only when reverification is disabled. - const policy: EditablePasswordPolicy = user.passwordEnabled + const policy: UserProfilePasswordPolicy = user.passwordEnabled ? { mode: 'change', requiresCurrentPassword: true } : { mode: 'set', requiresCurrentPassword: false }; @@ -59,7 +56,7 @@ function getPasswordPolicy( return { status: 'readonly', mode: policy.mode, - managedBy: { name: enterpriseAccount.enterpriseConnection?.name || undefined }, + managedBy: { name: enterpriseAccount.enterpriseConnection?.name || enterpriseConnectionName }, }; } @@ -105,7 +102,7 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { return { status: 'hidden' }; } - const policy = getPasswordPolicy(user, environment); + const policy = getPasswordPolicy(user, environment, m.enterpriseConnection); if (policy.status !== 'ready') { return policy; } 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 a89862fc1dd..b7c62f34e3f 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 @@ -6,76 +6,69 @@ import { useUserProfileEditPasswordController } from './user-profile-edit-passwo import { UserProfileEditPasswordDialog } from './user-profile-edit-password.dialog'; import type { UserProfilePasswordModel } from './user-profile-password-section.model'; import { useUserProfilePasswordModel } from './user-profile-password-section.model'; -import type { UserProfilePasswordSlot } from './user-profile-password-section.types'; import { UserProfilePasswordSectionView } from './user-profile-password-section.view'; export interface UserProfilePasswordSectionProps { fallback?: ReactNode; } -export function UserProfilePasswordSection(props: UserProfilePasswordSectionProps) { - return useUserProfilePasswordSlot(props)?.content ?? null; +export function UserProfilePasswordSection({ fallback = null }: UserProfilePasswordSectionProps) { + const model = useUserProfilePasswordModel(); + return passwordSectionNode(model, fallback); } -export function useUserProfilePasswordSlot({ - fallback = null, -}: UserProfilePasswordSectionProps = {}): UserProfilePasswordSlot | null { - const model = useUserProfilePasswordModel(); - const m = useMessages('userProfilePasswordSection'); +export function passwordSectionNode(model: UserProfilePasswordModel, fallback: ReactNode): ReactNode { if (model.status === 'loading') { - // TODO: Add a password section skeleton as the default loading fallback. - return fallback ? { content: fallback } : null; + // TODO: After https://github.com/clerk/javascript/pull/10029 lands, add the password skeleton using the shared section primitives. Keep loading timing in the connected security panel and omit the section when passwords are unavailable. + return fallback; } if (model.status === 'hidden') { return null; } if (model.status === 'readonly') { - return { - content: ( - - ), - }; - } - return { - content: ( - - ), - }; + ); + } + return ( + + ); } function PasswordEditor({ model }: { model: Extract }) { const m = useMessages('userProfilePasswordSection'); const controller = useUserProfileEditPasswordController({ + policy: model, + identifier: model.identifier, validatePassword: model.validatePassword, - requiresCurrentPassword: model.requiresCurrentPassword, onSubmit: model.updatePassword, }); return ( - {model.mode === 'change' ? m.change : m.set} + {controller.hasPassword ? m.change : m.set} } /> diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.types.ts index 9ed3bf2e5a1..93a10a02de9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.types.ts @@ -24,6 +24,6 @@ export interface UserProfilePasswordSectionViewProps { managedBy?: UserProfileManagedBy; } -export interface UserProfilePasswordSlot { - content: ReactNode; -} +export type UserProfilePasswordPolicy = + | { mode: 'set'; requiresCurrentPassword: false } + | { mode: 'change'; requiresCurrentPassword: boolean }; diff --git a/packages/mosaic/src/features/user-profile/user-profile-security-panel.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-security-panel.feature.test.tsx new file mode 100644 index 00000000000..6ac995a6cac --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-security-panel.feature.test.tsx @@ -0,0 +1,78 @@ +import { screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { serveFapi } from '../../__tests__/feature/fake-fapi'; +import { + fapiClient, + fapiEmailAddress, + fapiEnterpriseAccount, + fapiEnvironment, + fapiSession, + fapiUser, +} from '../../__tests__/feature/fapi'; +import { renderWithClerk } from '../../__tests__/feature/render'; +import { UserProfileSecurityPanel } from './user-profile-security-panel'; + +const email = fapiEmailAddress({ id: 'idn_1', email_address: 'person@example.com' }); +const alice = fapiUser({ id: 'user_1', email_addresses: [email] }); + +describe('UserProfileSecurityPanel', () => { + it('omits Authentication while the only method loads without a fallback', async () => { + serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + const loading = renderWithClerk(); + try { + expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); + } finally { + await loading; + } + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); + }); + + it('keeps Authentication around a visible loading fallback', async () => { + serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + const loading = renderWithClerk( + Loading password section} />, + ); + try { + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Loading password section'); + } finally { + await loading; + } + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); + expect(screen.queryByText('Loading password section')).toBeNull(); + }); + + it('shows no password action when nobody is signed in', async () => { + serveFapi({ client: fapiClient() }); + await renderWithClerk(); + + expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); + expect(screen.queryByText('Password')).toBeNull(); + }); + + it('keeps Authentication for other methods when passwords are disabled', async () => { + const environment = fapiEnvironment(); + environment.user_settings.attributes.password.enabled = false; + serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + await renderWithClerk(); + + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Passkeys'); + expect(screen.queryByText('Password')).toBeNull(); + }); + + it.each(['disabled', 'editable', 'managed'])('resolves the Authentication section for %s passwords', async policy => { + const environment = fapiEnvironment(); + environment.user_settings.attributes.password.enabled = policy !== 'disabled'; + const user = fapiUser({ + ...alice, + enterprise_accounts: policy === 'managed' ? [fapiEnterpriseAccount({ id: 'ent_1' })] : [], + }); + serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user })]) }); + await renderWithClerk(); + if (policy === 'disabled') { + expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); + } else { + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); + } + }); +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx b/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx new file mode 100644 index 00000000000..ac8de3e583a --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx @@ -0,0 +1,22 @@ +import type { ReactNode } from 'react'; + +import { passwordSectionNode } from './user-profile-password-section/user-profile-password-section'; +import { useUserProfilePasswordModel } from './user-profile-password-section/user-profile-password-section.model'; +import type { UserProfileSecurityPanelViewProps } from './user-profile-security-panel.view'; +import { UserProfileSecurityPanelView } from './user-profile-security-panel.view'; + +export interface UserProfileSecurityPanelProps extends Omit { + passwordFallback?: ReactNode; +} + +export function UserProfileSecurityPanel({ passwordFallback = null, ...props }: UserProfileSecurityPanelProps) { + const password = useUserProfilePasswordModel(); + const passwordSlot = passwordSectionNode(password, passwordFallback); + + return ( + + ); +} diff --git a/packages/mosaic/src/features/user-profile/user-profile-security-panel.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-security-panel.view.tsx index af6c5c8e34c..a0ac3448a51 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-security-panel.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-security-panel.view.tsx @@ -12,12 +12,11 @@ import type { UserProfileMfaAddableMethod, UserProfileMfaMethod } from './user-p import { UserProfileMfaSectionView } from './user-profile-mfa-section.view'; import type { UserProfilePasskey } from './user-profile-passkeys-section.view'; import { UserProfilePasskeysSectionView } from './user-profile-passkeys-section.view'; -import type { UserProfilePasswordSlot } from './user-profile-password-section/user-profile-password-section.types'; export type { UserProfileDevice, UserProfileMfaAddableMethod, UserProfileMfaMethod, UserProfilePasskey }; export interface UserProfileSecurityPanelViewProps extends Omit { - passwordSlot?: UserProfilePasswordSlot | null; + passwordSlot?: ReactNode; passkeys?: UserProfilePasskey[]; passkeysVisible?: boolean; mfaMethods?: UserProfileMfaMethod[]; @@ -56,9 +55,8 @@ export function UserProfileSecurityPanelView({ onSignOutAllOtherDevices, dangerSlot, }: UserProfileSecurityPanelViewProps): ReactElement { - const showPassword = Boolean(passwordSlot); const showPasskeys = passkeys !== undefined && passkeysVisible; - const hasAuthentication = showPassword || showPasskeys || mfaMethods !== undefined; + const hasAuthentication = passwordSlot != null || showPasskeys || mfaMethods !== undefined; return ( }> @@ -66,7 +64,7 @@ export function UserProfileSecurityPanelView({ {hasAuthentication ? ( - {passwordSlot?.content} + {passwordSlot} {showPasskeys ? ( { await new Promise(resolve => setTimeout(resolve, latency)); if (failWith && !hasFailed) { @@ -37,21 +38,23 @@ export function useUserProfileEditPasswordFixture({ }); return { - hasPassword, + hasPassword: controller.hasPassword, action: ( - {hasPassword ? m.change : m.set} + {controller.hasPassword ? m.change : m.set} } /> diff --git a/packages/swingset/src/stories/fixtures/user-profile.tsx b/packages/swingset/src/stories/fixtures/user-profile.tsx index b3b38a5bb60..5498973e112 100644 --- a/packages/swingset/src/stories/fixtures/user-profile.tsx +++ b/packages/swingset/src/stories/fixtures/user-profile.tsx @@ -155,7 +155,7 @@ export function useUserProfileFixture({ onAddEmail }: UserProfileFixtureOptions setPhones(current => current.map(phone => (phone.id === id ? { ...phone, isVerified: true } : phone))), }, security: { - passwordSlot: { content: }, + passwordSlot: , passkeys: passkeys.passkeys, addPasskeyError: passkeys.addError, onRenamePasskey: passkeys.onRename, diff --git a/packages/swingset/src/stories/user-profile-security-panel.mdx b/packages/swingset/src/stories/user-profile-security-panel.mdx index 2b8eb6d801d..15111d646fb 100644 --- a/packages/swingset/src/stories/user-profile-security-panel.mdx +++ b/packages/swingset/src/stories/user-profile-security-panel.mdx @@ -3,9 +3,10 @@ import * as Stories from './user-profile-security-panel.stories'; # UserProfileSecurityPanel The security panel groups the available authentication methods and active devices. For a live -password section, pass `useUserProfilePasswordSlot()` as `passwordSlot`. The hook returns `null` -when passwords are hidden or loading without a fallback, so the panel omits an empty Authentication -section. Fixtures pass `{ content: }`. +password section, render `UserProfileSecurityPanel`. It reads the password model and +passes content only when the password section is available or has a loading fallback, +so an empty Authentication section stays hidden. Fixtures pass +`` directly as `passwordSlot`. ## Example