From 1d60b0afe97d2a0d3e8e4c6c1aa205c8cd410442 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 11:45:23 -0600 Subject: [PATCH 1/8] test(mosaic): consolidate password model coverage in feature tests --- .changeset/password-feedback-tests.md | 2 + .../skills/mosaic/references/controllers.md | 4 + .../user-profile-password.feature.test.tsx | 110 +++++++++- ...ser-profile-password-section.model.test.ts | 189 ------------------ references/mosaic-architecture.md | 6 + 5 files changed, 120 insertions(+), 191 deletions(-) create mode 100644 .changeset/password-feedback-tests.md delete mode 100644 packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.model.test.ts 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/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index bc51468a921..472e3477956 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -120,6 +120,10 @@ have a machine. ## Rules +- **Pass through model values the view needs.** Return them alongside interaction + state and actions. Compose view props from the controller result, without mixing + in model values directly. See `references/mosaic-architecture.md` for the layer + contract and Swingset reuse. - **No Clerk imports.** If a controller needs a Clerk fact, the model supplies it as data. - **No machine snapshot in the return value.** Return the plain props the view diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx index 23aae55191b..7a276da1a92 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx @@ -1,7 +1,7 @@ -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 { @@ -74,6 +74,79 @@ describe('Changing a password', () => { expect(screen.queryByText('Password')).toBeNull(); }); + 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('shows an unexpected update failure in the dialog and keeps the draft', async () => { + serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + 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 { + 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 { + update.mockRestore(); + } + }); + + 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 { + 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 { + load.mockRestore(); + } + }); + it('sends the update to Clerk and closes after it succeeds', async () => { const fapi = await renderPassword(); const user = await fillPassword(); @@ -141,6 +214,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(); @@ -211,6 +304,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(); 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/references/mosaic-architecture.md b/references/mosaic-architecture.md index bba64c8a0e5..25a24c2f73c 100644 --- a/references/mosaic-architecture.md +++ b/references/mosaic-architecture.md @@ -378,6 +378,12 @@ export function useUserButtonController(model: UserButtonModel, options = {}): U The controller passes the model's `status` through, so the wrapper has one thing to branch on rather than two. +Pass model values needed by the view through the controller, even when they need +no transformation. When composing a view with a controller, read those values from +the controller result instead of mixing model and controller values in view props. +This gives the view one interface. Swingset can mock the controller's inputs and +pass its result to the same view. + A controller whose interaction is a single boolean with no async and no second value to keep in step is the same layer with `useState` inside it — still no Clerk, still returning plain props: From d28b834bea240d18d658cc3f0a33f6cb0e9793b3 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 11:59:45 -0600 Subject: [PATCH 2/8] refactor(mosaic): pass password view data through controller --- .../user-profile-edit-password.controller.ts | 9 ++++++++- .../user-profile-password-section.tsx | 12 +++++++----- .../stories/fixtures/user-profile-edit-password.tsx | 9 +++++---- 3 files changed, 20 insertions(+), 10 deletions(-) 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 4404801d7dc..162d37a8166 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 @@ -19,12 +19,17 @@ const initialValues: UserProfileEditPasswordValues = { }; export interface UserProfileEditPasswordControllerOptions { + hasPassword?: boolean; + identifier?: string; requiresCurrentPassword?: boolean; 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; @@ -32,6 +37,8 @@ export interface UserProfileEditPasswordController { } export function useUserProfileEditPasswordController({ + hasPassword = false, + identifier = '', requiresCurrentPassword = false, onSubmit, validatePassword, @@ -84,5 +91,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/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..0ab000ec822 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 @@ -52,6 +52,8 @@ export function useUserProfilePasswordSlot({ function PasswordEditor({ model }: { model: Extract }) { const m = useMessages('userProfilePasswordSection'); const controller = useUserProfileEditPasswordController({ + hasPassword: model.mode === 'change', + identifier: model.identifier, validatePassword: model.validatePassword, requiresCurrentPassword: model.requiresCurrentPassword, onSubmit: model.updatePassword, @@ -59,23 +61,23 @@ function PasswordEditor({ model }: { model: Extract - {model.mode === 'change' ? m.change : m.set} + {controller.hasPassword ? m.change : m.set} } /> diff --git a/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx b/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx index 6d2fd3c4f61..3efeea81b91 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx +++ b/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx @@ -25,6 +25,7 @@ export function useUserProfileEditPasswordFixture({ const [hasPassword, setHasPassword] = useState(initialHasPassword); const [hasFailed, setHasFailed] = useState(false); const controller = useUserProfileEditPasswordController({ + hasPassword, requiresCurrentPassword: hasPassword && requiresCurrentPassword, onSubmit: async (_value: UserProfileEditPasswordValue) => { await new Promise(resolve => setTimeout(resolve, latency)); @@ -37,21 +38,21 @@ export function useUserProfileEditPasswordFixture({ }); return { - hasPassword, + hasPassword: controller.hasPassword, action: ( - {hasPassword ? m.change : m.set} + {controller.hasPassword ? m.change : m.set} } /> From eb5462a526715bfbc1b684e1e29293914c6ec949 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 13:45:03 -0600 Subject: [PATCH 3/8] refactor(mosaic): simplify password section composition --- .../user-profile-password.feature.test.tsx | 36 +++++++------ .../user-profile-security-panel.view.test.tsx | 12 +++++ .../user-profile-password-section.tsx | 54 +++++++++---------- .../user-profile-password-section.types.ts | 4 -- .../user-profile-security-panel.tsx | 26 +++++++++ .../user-profile-security-panel.view.tsx | 10 ++-- .../src/stories/fixtures/user-profile.tsx | 3 +- .../stories/user-profile-security-panel.mdx | 7 +-- 8 files changed, 96 insertions(+), 56 deletions(-) create mode 100644 packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx index 7a276da1a92..009bd3aacda 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx @@ -1,6 +1,5 @@ import { act, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import type { ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; @@ -13,20 +12,12 @@ 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/user-profile-password-section'; +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] }); -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(); @@ -45,7 +36,7 @@ async function fillPassword() { describe('Changing a password', () => { it('omits Authentication while the only method loads without a fallback', async () => { serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); - const loading = renderWithClerk(); + const loading = renderWithClerk(); try { expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); } finally { @@ -56,7 +47,9 @@ describe('Changing a 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} />); + const loading = renderWithClerk( + Loading password section} />, + ); try { expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Loading password section'); } finally { @@ -68,7 +61,7 @@ describe('Changing a password', () => { it('shows no password action when nobody is signed in', async () => { serveFapi({ client: fapiClient() }); - await renderWithClerk(); + await renderWithClerk(); expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); expect(screen.queryByText('Password')).toBeNull(); @@ -147,6 +140,16 @@ describe('Changing a password', () => { } }); + 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('sends the update to Clerk and closes after it succeeds', async () => { const fapi = await renderPassword(); const user = await fillPassword(); @@ -264,7 +267,7 @@ describe('Changing a password', () => { enterprise_accounts: policy === 'managed' ? [fapiEnterpriseAccount({ id: 'ent_1' })] : [], }); serveFapi({ environment, client: fapiClient([fapiSession({ id: 'sess_1', user })]) }); - await renderWithClerk(); + await renderWithClerk(); if (policy === 'disabled') { expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); } else { @@ -446,5 +449,6 @@ 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'); }); + +// 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. 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..cccb3c6ee21 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,18 @@ function renderView(overrides: Partial = {}) } describe('UserProfileSecurityPanelView', () => { + it('omits Authentication when supplied password content is explicitly hidden', () => { + renderView({ + passwordSlot:
Password
, + passwordVisible: false, + passkeys: undefined, + mfaMethods: undefined, + }); + + expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); + expect(screen.queryByText('Password')).toBeNull(); + }); + 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-password-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.tsx index 0ab000ec822..7fddbc06819 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,56 +6,56 @@ 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 = null): ReactNode { if (model.status === 'loading') { - // TODO: Add a password section skeleton as the default loading fallback. - return fallback ? { content: fallback } : null; + return fallback; } if (model.status === 'hidden') { return null; } + return ; +} + +function UserProfilePasswordSectionContent({ + model, +}: { + model: Extract; +}) { + const m = useMessages('userProfilePasswordSection'); if (model.status === 'readonly') { - return { - content: ( - - ), - }; - } - return { - content: ( - - ), - }; + ); + } + return ( + + ); } function PasswordEditor({ model }: { model: Extract }) { const m = useMessages('userProfilePasswordSection'); const controller = useUserProfileEditPasswordController({ hasPassword: model.mode === 'change', + requiresCurrentPassword: model.requiresCurrentPassword, identifier: model.identifier, validatePassword: model.validatePassword, - requiresCurrentPassword: model.requiresCurrentPassword, onSubmit: model.updatePassword, }); 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..a45172b84ad 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 @@ -23,7 +23,3 @@ export interface UserProfilePasswordSectionViewProps { /** Replaces the edit action with the enterprise provider’s name. */ managedBy?: UserProfileManagedBy; } - -export interface UserProfilePasswordSlot { - content: ReactNode; -} 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..6d7522f3778 --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx @@ -0,0 +1,26 @@ +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< + UserProfileSecurityPanelViewProps, + 'passwordSlot' | 'passwordVisible' +> { + 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..6fc6440aca4 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,12 @@ 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; + passwordVisible?: boolean; passkeys?: UserProfilePasskey[]; passkeysVisible?: boolean; mfaMethods?: UserProfileMfaMethod[]; @@ -38,6 +38,7 @@ export interface UserProfileSecurityPanelViewProps extends Omit}> @@ -66,7 +66,7 @@ export function UserProfileSecurityPanelView({ {hasAuthentication ? ( - {passwordSlot?.content} + {passwordVisible ? passwordSlot : null} {showPasskeys ? ( current.map(phone => (phone.id === id ? { ...phone, isVerified: true } : phone))), }, security: { - passwordSlot: { content: }, + passwordVisible: true, + 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..e028f4ffc1a 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` and set `passwordVisible` explicitly. ## Example From a5de9f0b0397201b7881ea9e2a2f6bb117a81206 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 13:45:29 -0600 Subject: [PATCH 4/8] refactor(mosaic): require a typed password policy --- ...r-profile-edit-password.controller.test.ts | 24 ++++++++++++++++--- .../user-profile-edit-password.controller.ts | 13 +++++----- .../user-profile-password-section.model.ts | 18 ++++++-------- .../user-profile-password-section.tsx | 3 +-- .../user-profile-password-section.types.ts | 2 ++ .../fixtures/user-profile-edit-password.tsx | 4 ++-- 6 files changed, 40 insertions(+), 24 deletions(-) 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..8f697584ae5 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' }, + 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' }, + 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' }, + 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' }, + 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 162d37a8166..68b0d7a4d4a 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 @@ -9,6 +9,7 @@ import { useMessages } from '../../../localization'; import type { UserProfileEditPasswordValue, UserProfileEditPasswordValues, + UserProfilePasswordPolicy, } from './user-profile-password-section.types'; const initialValues: UserProfileEditPasswordValues = { @@ -19,9 +20,8 @@ const initialValues: UserProfileEditPasswordValues = { }; export interface UserProfileEditPasswordControllerOptions { - hasPassword?: boolean; - identifier?: string; - requiresCurrentPassword?: boolean; + policy: UserProfilePasswordPolicy; + identifier: string; onSubmit: (value: UserProfileEditPasswordValue) => Promise; validatePassword?: (password: string) => Promise; } @@ -37,12 +37,13 @@ export interface UserProfileEditPasswordController { } export function useUserProfileEditPasswordController({ - hasPassword = false, - identifier = '', - requiresCurrentPassword = false, + policy, + identifier, onSubmit, validatePassword, }: UserProfileEditPasswordControllerOptions): UserProfileEditPasswordController { + const hasPassword = policy.mode === 'change'; + const requiresCurrentPassword = policy.mode === 'change' && policy.requiresCurrentPassword; const validationError = useMessages('errors').generic; const m = useMessages('userProfilePasswordSection'); const [isOpen, setIsOpen] = useState(false); 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..02a4c81f24b 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,11 +11,7 @@ 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' } @@ -28,7 +24,7 @@ type UnavailablePasswordModel = export type UserProfilePasswordModel = | { status: 'loading' } | UnavailablePasswordModel - | (EditablePasswordPolicy & { + | (UserProfilePasswordPolicy & { status: 'ready'; userId: string; sessionId: string; @@ -40,7 +36,7 @@ export type UserProfilePasswordModel = function getPasswordPolicy( user: UserResource | null | undefined, environment: EnvironmentResource, -): UnavailablePasswordModel | (EditablePasswordPolicy & { status: 'ready'; userId: string }) { +): UnavailablePasswordModel | (UserProfilePasswordPolicy & { status: 'ready'; userId: string }) { if (!user) { return { status: 'hidden' }; } @@ -50,9 +46,9 @@ 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 }; + : { mode: 'set' }; const enterpriseAccount = user.enterpriseAccounts.find(account => account.active); if (enterpriseAccount) { @@ -136,12 +132,12 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { await currentUser.updatePassword({ newPassword, signOutOfOtherSessions, - ...(policy.requiresCurrentPassword ? { currentPassword } : {}), + ...(policy.mode === 'change' && policy.requiresCurrentPassword ? { currentPassword } : {}), }); } catch (error) { throw passwordFormError( error, - policy.requiresCurrentPassword, + policy.mode === 'change' && policy.requiresCurrentPassword, environment.userSettings.passwordSettings, m, locale, 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 7fddbc06819..43f14c7efaa 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 @@ -52,8 +52,7 @@ function UserProfilePasswordSectionContent({ function PasswordEditor({ model }: { model: Extract }) { const m = useMessages('userProfilePasswordSection'); const controller = useUserProfileEditPasswordController({ - hasPassword: model.mode === 'change', - requiresCurrentPassword: model.requiresCurrentPassword, + policy: model, identifier: model.identifier, validatePassword: model.validatePassword, onSubmit: model.updatePassword, 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 a45172b84ad..5f0e9d7d31c 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 @@ -23,3 +23,5 @@ export interface UserProfilePasswordSectionViewProps { /** Replaces the edit action with the enterprise provider’s name. */ managedBy?: UserProfileManagedBy; } + +export type UserProfilePasswordPolicy = { mode: 'set' } | { mode: 'change'; requiresCurrentPassword: boolean }; diff --git a/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx b/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx index 3efeea81b91..dfc9ea1b98c 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx +++ b/packages/swingset/src/stories/fixtures/user-profile-edit-password.tsx @@ -25,8 +25,8 @@ export function useUserProfileEditPasswordFixture({ const [hasPassword, setHasPassword] = useState(initialHasPassword); const [hasFailed, setHasFailed] = useState(false); const controller = useUserProfileEditPasswordController({ - hasPassword, - requiresCurrentPassword: hasPassword && requiresCurrentPassword, + policy: hasPassword ? { mode: 'change', requiresCurrentPassword } : { mode: 'set' }, + identifier: '', onSubmit: async (_value: UserProfileEditPasswordValue) => { await new Promise(resolve => setTimeout(resolve, latency)); if (failWith && !hasFailed) { From 11aaa577aeaf1a0b210fb83c7635a3e2ff557ecf Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 13:45:51 -0600 Subject: [PATCH 5/8] test(mosaic): cover password policy changes in an open dialog --- .../user-profile-password.feature.test.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx index 009bd3aacda..3bb68f706c1 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-password.feature.test.tsx @@ -95,6 +95,44 @@ describe('Changing a password', () => { }, ); + 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 { clerk } = await renderWithClerk(); From 23f7e42aeb9122d72dd57d4f4bfb3fd71d45e58d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 14:59:26 -0600 Subject: [PATCH 6/8] test(mosaic): colocate password section feature tests --- .../user-profile-password-section.feature.test.tsx} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename packages/mosaic/src/features/user-profile/{__tests__/user-profile-password.feature.test.tsx => user-profile-password-section/user-profile-password-section.feature.test.tsx} (99%) 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 99% 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 3bb68f706c1..a8a81994912 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 @@ -12,7 +12,7 @@ import { fapiUser, } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; -import { UserProfilePasswordSection } from '../user-profile-password-section/user-profile-password-section'; +import { UserProfilePasswordSection } from './user-profile-password-section'; import { UserProfileSecurityPanel } from '../user-profile-security-panel'; const email = fapiEmailAddress({ id: 'idn_1', email_address: 'person@example.com' }); From 19e131b279d5105e0cd1f57cfae9227b5aebc8f9 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Mon, 5 Oct 2026 15:04:57 -0600 Subject: [PATCH 7/8] refactor(mosaic): simplify password visibility and policy --- .../skills/mosaic/references/controllers.md | 4 - .../user-profile-security-panel.view.test.tsx | 7 +- ...r-profile-edit-password.controller.test.ts | 8 +- .../user-profile-edit-password.controller.ts | 2 +- ...-profile-password-section.feature.test.tsx | 62 --------------- .../user-profile-password-section.model.ts | 13 ++-- .../user-profile-password-section.tsx | 14 +--- .../user-profile-password-section.types.ts | 4 +- ...er-profile-security-panel.feature.test.tsx | 78 +++++++++++++++++++ .../user-profile-security-panel.tsx | 6 +- .../user-profile-security-panel.view.tsx | 6 +- .../fixtures/user-profile-edit-password.tsx | 4 +- .../src/stories/fixtures/user-profile.tsx | 1 - .../stories/user-profile-security-panel.mdx | 2 +- references/mosaic-architecture.md | 6 -- 15 files changed, 106 insertions(+), 111 deletions(-) create mode 100644 packages/mosaic/src/features/user-profile/user-profile-security-panel.feature.test.tsx diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index 472e3477956..bc51468a921 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -120,10 +120,6 @@ have a machine. ## Rules -- **Pass through model values the view needs.** Return them alongside interaction - state and actions. Compose view props from the controller result, without mixing - in model values directly. See `references/mosaic-architecture.md` for the layer - contract and Swingset reuse. - **No Clerk imports.** If a controller needs a Clerk fact, the model supplies it as data. - **No machine snapshot in the return value.** Return the plain props the view 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 cccb3c6ee21..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,16 +63,15 @@ function renderView(overrides: Partial = {}) } describe('UserProfileSecurityPanelView', () => { - it('omits Authentication when supplied password content is explicitly hidden', () => { + it('renders supplied password content without a separate visibility flag', () => { renderView({ passwordSlot:
Password
, - passwordVisible: false, passkeys: undefined, mfaMethods: undefined, }); - expect(screen.queryByRole('region', { name: 'Authentication' })).toBeNull(); - expect(screen.queryByText('Password')).toBeNull(); + expect(screen.getByRole('region', { name: 'Authentication' })).toHaveTextContent('Password'); + expect(screen.getByText('Password')).toBeVisible(); }); it('composes authentication, active devices, and the danger zone', () => { 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 8f697584ae5..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,7 +11,7 @@ describe('useUserProfileEditPasswordController timing', () => { try { const { result } = renderHook(() => useUserProfileEditPasswordController({ - policy: { mode: 'set' }, + policy: { mode: 'set', requiresCurrentPassword: false }, identifier: '', onSubmit: () => Promise.resolve(), validatePassword: () => Promise.reject(new Error('Failed to load strength checker')), @@ -32,7 +32,7 @@ describe('useUserProfileEditPasswordController timing', () => { const validatePassword = vi.fn(() => Promise.resolve(feedback)); const { result } = renderHook(() => useUserProfileEditPasswordController({ - policy: { mode: 'set' }, + policy: { mode: 'set', requiresCurrentPassword: false }, identifier: '', onSubmit: () => Promise.resolve(), validatePassword, @@ -56,7 +56,7 @@ describe('useUserProfileEditPasswordController timing', () => { const validatePassword = vi.fn(() => Promise.resolve(undefined)); const { result } = renderHook(() => useUserProfileEditPasswordController({ - policy: { mode: 'set' }, + policy: { mode: 'set', requiresCurrentPassword: false }, identifier: '', onSubmit: () => Promise.resolve(), validatePassword, @@ -85,7 +85,7 @@ describe('useUserProfileEditPasswordController timing', () => { const validatePassword = vi.fn().mockReturnValueOnce(older.promise).mockReturnValueOnce(newer.promise); const { result } = renderHook(() => useUserProfileEditPasswordController({ - policy: { mode: 'set' }, + policy: { mode: 'set', requiresCurrentPassword: false }, identifier: '', onSubmit: () => Promise.resolve(), validatePassword, 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 68b0d7a4d4a..e74e34067fd 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 @@ -43,7 +43,7 @@ export function useUserProfileEditPasswordController({ validatePassword, }: UserProfileEditPasswordControllerOptions): UserProfileEditPasswordController { const hasPassword = policy.mode === 'change'; - const requiresCurrentPassword = policy.mode === 'change' && policy.requiresCurrentPassword; + const requiresCurrentPassword = policy.requiresCurrentPassword; const validationError = useMessages('errors').generic; const m = useMessages('userProfilePasswordSection'); const [isOpen, setIsOpen] = useState(false); diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx index a8a81994912..c20b63f24c7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx @@ -13,7 +13,6 @@ import { } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; import { UserProfilePasswordSection } from './user-profile-password-section'; -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] }); @@ -34,39 +33,6 @@ async function fillPassword() { } describe('Changing a password', () => { - 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.each(['sign out', 'switch user', 'switch session'] as const)( 'discards the password draft after %s', async change => { @@ -178,16 +144,6 @@ describe('Changing a password', () => { } }); - 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('sends the update to Clerk and closes after it succeeds', async () => { const fapi = await renderPassword(); const user = await fillPassword(); @@ -297,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({ @@ -488,5 +428,3 @@ 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'); }); - -// 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. 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 02a4c81f24b..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 @@ -18,7 +18,7 @@ type UnavailablePasswordModel = | { status: 'readonly'; mode: 'set' | 'change'; - managedBy: { name?: string }; + managedBy: { name: string }; }; export type UserProfilePasswordModel = @@ -36,6 +36,7 @@ export type UserProfilePasswordModel = function getPasswordPolicy( user: UserResource | null | undefined, environment: EnvironmentResource, + enterpriseConnectionName: string, ): UnavailablePasswordModel | (UserProfilePasswordPolicy & { status: 'ready'; userId: string }) { if (!user) { return { status: 'hidden' }; @@ -48,14 +49,14 @@ function getPasswordPolicy( // 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: 'set' }; + : { mode: 'set', requiresCurrentPassword: false }; const enterpriseAccount = user.enterpriseAccounts.find(account => account.active); if (enterpriseAccount) { return { status: 'readonly', mode: policy.mode, - managedBy: { name: enterpriseAccount.enterpriseConnection?.name || undefined }, + managedBy: { name: enterpriseAccount.enterpriseConnection?.name || enterpriseConnectionName }, }; } @@ -101,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; } @@ -132,12 +133,12 @@ export function useUserProfilePasswordModel(): UserProfilePasswordModel { await currentUser.updatePassword({ newPassword, signOutOfOtherSessions, - ...(policy.mode === 'change' && policy.requiresCurrentPassword ? { currentPassword } : {}), + ...(policy.requiresCurrentPassword ? { currentPassword } : {}), }); } catch (error) { throw passwordFormError( error, - policy.mode === 'change' && policy.requiresCurrentPassword, + policy.requiresCurrentPassword, environment.userSettings.passwordSettings, m, locale, 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 43f14c7efaa..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 @@ -17,27 +17,19 @@ export function UserProfilePasswordSection({ fallback = null }: UserProfilePassw return passwordSectionNode(model, fallback); } -export function passwordSectionNode(model: UserProfilePasswordModel, fallback: ReactNode = null): ReactNode { +export function passwordSectionNode(model: UserProfilePasswordModel, fallback: ReactNode): ReactNode { if (model.status === 'loading') { + // 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; } - return ; -} - -function UserProfilePasswordSectionContent({ - model, -}: { - model: Extract; -}) { - const m = useMessages('userProfilePasswordSection'); if (model.status === 'readonly') { return ( ); } 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 5f0e9d7d31c..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,4 +24,6 @@ export interface UserProfilePasswordSectionViewProps { managedBy?: UserProfileManagedBy; } -export type UserProfilePasswordPolicy = { mode: 'set' } | { mode: 'change'; requiresCurrentPassword: boolean }; +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 index 6d7522f3778..ac8de3e583a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-security-panel.tsx @@ -5,10 +5,7 @@ import { useUserProfilePasswordModel } from './user-profile-password-section/use import type { UserProfileSecurityPanelViewProps } from './user-profile-security-panel.view'; import { UserProfileSecurityPanelView } from './user-profile-security-panel.view'; -export interface UserProfileSecurityPanelProps extends Omit< - UserProfileSecurityPanelViewProps, - 'passwordSlot' | 'passwordVisible' -> { +export interface UserProfileSecurityPanelProps extends Omit { passwordFallback?: ReactNode; } @@ -20,7 +17,6 @@ export function UserProfileSecurityPanel({ passwordFallback = null, ...props }: ); } 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 6fc6440aca4..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 @@ -17,7 +17,6 @@ export type { UserProfileDevice, UserProfileMfaAddableMethod, UserProfileMfaMeth export interface UserProfileSecurityPanelViewProps extends Omit { passwordSlot?: ReactNode; - passwordVisible?: boolean; passkeys?: UserProfilePasskey[]; passkeysVisible?: boolean; mfaMethods?: UserProfileMfaMethod[]; @@ -38,7 +37,6 @@ export interface UserProfileSecurityPanelViewProps extends Omit}> @@ -66,7 +64,7 @@ export function UserProfileSecurityPanelView({ {hasAuthentication ? ( - {passwordVisible ? passwordSlot : null} + {passwordSlot} {showPasskeys ? ( { await new Promise(resolve => setTimeout(resolve, latency)); @@ -42,6 +42,8 @@ export function useUserProfileEditPasswordFixture({ action: ( current.map(phone => (phone.id === id ? { ...phone, isVerified: true } : phone))), }, security: { - passwordVisible: true, passwordSlot: , passkeys: passkeys.passkeys, addPasskeyError: passkeys.addError, diff --git a/packages/swingset/src/stories/user-profile-security-panel.mdx b/packages/swingset/src/stories/user-profile-security-panel.mdx index e028f4ffc1a..15111d646fb 100644 --- a/packages/swingset/src/stories/user-profile-security-panel.mdx +++ b/packages/swingset/src/stories/user-profile-security-panel.mdx @@ -6,7 +6,7 @@ The security panel groups the available authentication methods and active device 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` and set `passwordVisible` explicitly. +`` directly as `passwordSlot`. ## Example diff --git a/references/mosaic-architecture.md b/references/mosaic-architecture.md index 25a24c2f73c..bba64c8a0e5 100644 --- a/references/mosaic-architecture.md +++ b/references/mosaic-architecture.md @@ -378,12 +378,6 @@ export function useUserButtonController(model: UserButtonModel, options = {}): U The controller passes the model's `status` through, so the wrapper has one thing to branch on rather than two. -Pass model values needed by the view through the controller, even when they need -no transformation. When composing a view with a controller, read those values from -the controller result instead of mixing model and controller values in view props. -This gives the view one interface. Swingset can mock the controller's inputs and -pass its result to the same view. - A controller whose interaction is a single boolean with no async and no second value to keep in step is the same layer with `useState` inside it — still no Clerk, still returning plain props: From 271fca92ff08b1df50ae079bd9f60d5338d14f13 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 6 Oct 2026 09:53:35 -0600 Subject: [PATCH 8/8] test(mosaic): align password failure assertion with form fallback --- ...user-profile-password-section.feature.test.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx index c20b63f24c7..75c170957e7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-section.feature.test.tsx @@ -100,20 +100,29 @@ describe('Changing a password', () => { ); it('shows an unexpected update failure in the dialog and keeps the draft', async () => { - serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); + const fapi = serveFapi({ client: fapiClient([fapiSession({ id: 'sess_1', user: alice })]) }); 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')); + const update = vi.spyOn(clerk.user, 'updatePassword').mockRejectedValueOnce(new Error('Connection interrupted')); try { const user = await fillPassword(); + const dialog = screen.getByRole('dialog'); await user.click(screen.getByRole('button', { name: 'Save changes' })); - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Connection interrupted')); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Something went wrong. Please try again.'), + ); + expect(dialog).not.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'); + expect(fapi.passwordUpdates).toHaveLength(0); + + await user.click(screen.getByRole('button', { name: 'Save changes' })); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(fapi.passwordUpdates).toHaveLength(1); } finally { update.mockRestore(); }