Skip to content

Commit 1e8a5d2

Browse files
austincalvelageaustincalvelage
committed
fix(mosaic): coordinate pending MFA actions
1 parent 85e56c4 commit 1e8a5d2

5 files changed

Lines changed: 89 additions & 12 deletions

File tree

‎packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,41 @@ describe('User profile MFA management', () => {
367367
}
368368
});
369369

370+
it('blocks backup-code regeneration while the default SMS method is changing', async () => {
371+
const first = fapiPhoneNumber({ ...phone, reserved_for_second_factor: true, default_second_factor: true });
372+
const second = fapiPhoneNumber({
373+
...phone,
374+
id: 'phone_2',
375+
phone_number: '+15555550202',
376+
reserved_for_second_factor: true,
377+
});
378+
const fapi = await renderMfa(
379+
fapiUser({ id: 'user_1', phone_numbers: [first, second], backup_code_enabled: true, two_factor_enabled: true }),
380+
);
381+
const held = holdRequests('post', '/v1/me/phone_numbers/phone_2');
382+
const user = userEvent.setup();
383+
try {
384+
await openMenu(user, screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0202' }));
385+
await user.click(screen.getByRole('menuitem', { name: 'Set as default' }));
386+
await waitFor(() => expect(held.requests).toHaveLength(1));
387+
await waitFor(() => expect(screen.queryByRole('menu')).toBeNull());
388+
const manage = screen.getByRole('button', { name: 'Manage Backup codes' });
389+
expect(manage).toHaveAttribute('aria-disabled', 'true');
390+
await user.click(manage);
391+
expect(screen.queryByRole('menu')).toBeNull();
392+
expect(fapi.mfa.backupCodeCreations).toBe(0);
393+
expect(screen.getByRole('button', { name: 'Add verification method' })).toBeDisabled();
394+
} finally {
395+
held.release();
396+
}
397+
const manage = screen.getByRole('button', { name: 'Manage Backup codes' });
398+
await waitFor(() => expect(manage).not.toHaveAttribute('aria-disabled', 'true'));
399+
await openMenu(user, manage);
400+
await user.click(screen.getByRole('menuitem', { name: 'Regenerate' }));
401+
await waitFor(() => expect(screen.getByText('CODE0100')).toBeVisible());
402+
expect(fapi.mfa.backupCodeCreations).toBe(1);
403+
});
404+
370405
it('returns failed regeneration to the section and retries from the menu', async () => {
371406
const fapi = await renderMfa(
372407
fapiUser({ id: 'user_1', totp_enabled: true, backup_code_enabled: true, two_factor_enabled: true }),
@@ -865,6 +900,12 @@ describe('User profile MFA management', () => {
865900
expect(screen.getAllByText('SMS verification')[0]?.closest('li')).toHaveTextContent('+1 (555) 555-0202'),
866901
);
867902
expect(screen.getAllByText('SMS verification')[0]?.closest('li')).toHaveTextContent('Default');
903+
await waitFor(() =>
904+
expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute(
905+
'aria-disabled',
906+
'true',
907+
),
908+
);
868909
await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' }));
869910
await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible());
870911
});
@@ -908,6 +949,12 @@ describe('User profile MFA management', () => {
908949
expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_2', default: true, reserved: undefined }),
909950
);
910951
expect(attempts).toBe(2);
952+
await waitFor(() =>
953+
expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute(
954+
'aria-disabled',
955+
'true',
956+
),
957+
);
911958
await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' }));
912959
await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible());
913960
});
@@ -916,7 +963,7 @@ describe('User profile MFA management', () => {
916963
{ phase: 'mutation', method: 'post', path: '/v1/me/phone_numbers/phone_2', pendingPhone: '+1 (555) 555-0202' },
917964
{ phase: 'refresh', method: 'get', path: '/v1/me', pendingPhone: '+1 (555) 555-0101' },
918965
] as const)(
919-
'keeps method menus available but hides default changes during $phase',
966+
'disables method menus while changing the default during $phase',
920967
async ({ method, path, pendingPhone }) => {
921968
const first = fapiPhoneNumber({ ...phone, reserved_for_second_factor: true, default_second_factor: true });
922969
const second = fapiPhoneNumber({
@@ -934,16 +981,21 @@ describe('User profile MFA management', () => {
934981
await waitFor(() => expect(held.requests).toHaveLength(1));
935982
try {
936983
const pendingAction = screen.getByRole('button', { name: `Manage SMS verification ${pendingPhone}` });
937-
await openMenu(user, pendingAction);
938-
await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Remove method' })).toBeVisible());
939-
expect(screen.queryByRole('menuitem', { name: 'Set as default' })).toBeNull();
940-
await user.click(await screen.findByRole('menuitem', { name: 'Remove method' }));
984+
expect(pendingAction).toHaveAttribute('aria-disabled', 'true');
985+
await user.click(pendingAction);
986+
expect(screen.queryByRole('menu')).toBeNull();
941987
expect(screen.queryByRole('alertdialog')).toBeNull();
942988
} finally {
943989
held.release();
944990
}
945991
await waitFor(() => expect(screen.getByText('+1 (555) 555-0202').closest('li')).toHaveTextContent('Default'));
946992
await user.keyboard('{Escape}');
993+
await waitFor(() =>
994+
expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute(
995+
'aria-disabled',
996+
'true',
997+
),
998+
);
947999
await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' }));
9481000
await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible());
9491001
},

‎packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-row.view.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ import type { UserProfileMfaMethod, UserProfileMfaSectionViewProps } from './use
1111

1212
export function UserProfileMfaRowView({
1313
method,
14+
isDisabled,
1415
triggerRef,
1516
onRemove,
1617
onSetDefault,
1718
regenerate,
1819
}: Pick<UserProfileMfaSectionViewProps, 'regenerate'> & {
1920
method: UserProfileMfaMethod;
21+
isDisabled?: boolean;
2022
triggerRef?: Ref<HTMLButtonElement>;
2123
onRemove?: () => void;
2224
onSetDefault?: (id: string) => void;
@@ -56,6 +58,8 @@ export function UserProfileMfaRowView({
5658
{actions.length > 0 ? (
5759
<Section.Actions>
5860
<ActionMenu
61+
disabled={isDisabled}
62+
focusableWhenDisabled
5963
triggerRef={triggerRef}
6064
actions={actions}
6165
label={manageLabel}

‎packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section-leaf.controller.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { useRef, useState } from 'react';
22

33
import { useConfirmController } from '../../../blocks/confirm';
4+
import { useActionLock } from '../../../hooks/use-action-lock';
45
import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus';
56
import { usePendingAction } from '../../../hooks/use-pending-action';
67
import { useMessages } from '../../../localization';
@@ -9,12 +10,15 @@ import type { UserProfileMfaSectionViewProps } from './user-profile-mfa-section.
910

1011
export function useUserProfileMfaSectionLeafController({
1112
methods,
13+
actionLock,
1214
onAdd,
1315
onRemove,
1416
onSetDefault,
1517
removeReverification,
1618
}: UserProfileMfaSectionViewProps) {
1719
const m = useMessages('userProfileMfa');
20+
const localActionLock = useActionLock();
21+
const lock = actionLock ?? localActionLock;
1822
const sectionRef = useRef<HTMLDivElement>(null);
1923
const removalFocus = useListRemovalFocus({
2024
ids: methods.map(method => method.id),
@@ -23,11 +27,12 @@ export function useUserProfileMfaSectionLeafController({
2327
sectionRef.current?.querySelector<HTMLButtonElement>('button:not([disabled])') ?? sectionRef.current,
2428
});
2529
const remove = useConfirmController<UserProfileMfaMethod>({
30+
actionLock: lock,
2631
onConfirm: method => removalFocus.remove(method.id),
2732
reverification: removeReverification,
2833
});
2934
const [pickerOpen, setPickerOpen] = useState(false);
30-
const defaultAction = usePendingAction({ errorFallback: m.setDefaultError });
35+
const defaultAction = usePendingAction({ actionLock: lock, errorFallback: m.setDefaultError });
3136

3237
const setDefault = (id: string) => {
3338
const method = methods.find(method => method.id === id);
@@ -39,6 +44,7 @@ export function useUserProfileMfaSectionLeafController({
3944

4045
return {
4146
sectionRef,
47+
isDisabled: lock.isPending,
4248
pickerOpen,
4349
onPickerOpenChange: setPickerOpen,
4450
addMethod: onAdd
@@ -49,7 +55,7 @@ export function useUserProfileMfaSectionLeafController({
4955
: undefined,
5056
registerTrigger: removalFocus.registerTrigger,
5157
openRemoval: (method: UserProfileMfaMethod) => {
52-
if (defaultAction.isPending) {
58+
if (lock.isPending) {
5359
return;
5460
}
5561
defaultAction.reset();
@@ -59,6 +65,6 @@ export function useUserProfileMfaSectionLeafController({
5965
finalRemovalFocus: removalFocus.finalFocus,
6066
setDefaultFinalFocus: () => (defaultAction.lastKey ? removalFocus.trigger(defaultAction.lastKey) : null),
6167
defaultError: defaultAction.error,
62-
setDefault: onSetDefault && !defaultAction.isPending ? setDefault : undefined,
68+
setDefault: onSetDefault ? setDefault : undefined,
6369
};
6470
}

‎packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { isReverificationCancelledError } from '@clerk/shared/error';
22
import { useEffect } from 'react';
33

4+
import { useActionLock } from '../../../hooks/use-action-lock';
45
import { type ErrorDescription, fill, type MosaicMessages, useErrorText, useMessages } from '../../../localization';
56
import { setup } from '../../../machine/setup';
67
import type { DoneInvokeEvent } from '../../../machine/types';
@@ -504,6 +505,7 @@ function createSetupProps(
504505
}
505506

506507
export function useUserProfileMfaController(model: ReadyModel) {
508+
const actionLock = useActionLock();
507509
const errorText = useErrorText();
508510
const backupCodeMessages = useMessages('userProfileBackupCodes');
509511
const [{ context, value }, send] = useMachine(mfaMachine);
@@ -512,9 +514,14 @@ export function useUserProfileMfaController(model: ReadyModel) {
512514
if (isBusy) {
513515
return Promise.resolve();
514516
}
515-
return new Promise<void>((resolve, reject) => {
516-
send({ type: 'RUN', key, methods: model.addableMethods, run: action, resolve, reject });
517-
});
517+
return (
518+
actionLock.tryRun(
519+
() =>
520+
new Promise<void>((resolve, reject) => {
521+
send({ type: 'RUN', key, methods: model.addableMethods, run: action, resolve, reject });
522+
}),
523+
) ?? Promise.resolve()
524+
);
518525
};
519526
const { resendSeconds } = context;
520527
const flow = context.flow;
@@ -585,6 +592,7 @@ export function useUserProfileMfaController(model: ReadyModel) {
585592
);
586593

587594
const sectionProps: UserProfileMfaSectionViewProps = {
595+
actionLock,
588596
methods: [...model.methods],
589597
addableMethods: model.addableMethods,
590598
regenerate: model.generateBackupCodes
@@ -607,7 +615,7 @@ export function useUserProfileMfaController(model: ReadyModel) {
607615
dialogOpen,
608616
showAddTrigger,
609617
showAddControl: showAddTrigger || dialogOpen,
610-
isPending: isBusy,
618+
isPending: actionLock.isPending,
611619
reverifying,
612620
onDialogOpenChange: (open: boolean) => {
613621
if (!open && !canClose) {

‎packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.view.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ReactNode, Ref } from 'react';
44
import { Confirmation } from '../../../blocks/confirmation';
55
import { ReverificationDialog } from '../../../blocks/reverification';
66
import { Section } from '../../../components/section';
7+
import type { ActionLock } from '../../../hooks/use-action-lock';
78
import { fill, type MosaicMessages, useMessages } from '../../../localization';
89
import type { ReverificationController } from '../../reverification';
910
import { UserProfileSecurityList } from '../user-profile-security-list';
@@ -17,6 +18,7 @@ export type { UserProfileMfaAddableMethod, UserProfileMfaMethod } from './user-p
1718

1819
export interface UserProfileMfaSectionViewProps {
1920
methods: UserProfileMfaMethod[];
21+
actionLock?: ActionLock;
2022
addableMethods?: readonly UserProfileMfaAddableMethod[];
2123
addButtonRef?: Ref<HTMLButtonElement>;
2224
addControl?: ReactNode;
@@ -30,6 +32,7 @@ export interface UserProfileMfaSectionViewProps {
3032

3133
export function UserProfileMfaSectionView({
3234
methods,
35+
actionLock,
3336
addableMethods,
3437
addButtonRef,
3538
addControl,
@@ -43,6 +46,7 @@ export function UserProfileMfaSectionView({
4346
const m = useMessages('userProfileMfa');
4447
const controller = useUserProfileMfaSectionLeafController({
4548
methods,
49+
actionLock,
4650
onAdd,
4751
onRemove,
4852
onSetDefault,
@@ -57,6 +61,7 @@ export function UserProfileMfaSectionView({
5761
addControl ??
5862
(controller.addMethod && addableMethods?.length ? (
5963
<UserProfileAddMfaDialog
64+
disabled={controller.isDisabled}
6065
triggerRef={addButtonRef}
6166
open={controller.pickerOpen}
6267
onOpenChange={controller.onPickerOpenChange}
@@ -77,6 +82,7 @@ export function UserProfileMfaSectionView({
7782
<UserProfileMfaRowView
7883
key={method.id}
7984
method={method}
85+
isDisabled={controller.isDisabled}
8086
triggerRef={controller.registerTrigger(method.id)}
8187
onRemove={controller.remove ? () => controller.openRemoval(method) : undefined}
8288
onSetDefault={controller.setDefault}
@@ -88,6 +94,7 @@ export function UserProfileMfaSectionView({
8894
{controller.remove ? (
8995
<Confirmation
9096
controller={controller.remove}
97+
disabled={controller.isDisabled}
9198
title={method => (method.type === 'sms' ? m.removeDialog.smsTitle : m.removeDialog.authenticatorTitle)}
9299
description={method => describeMethodRemoval(method, m)}
93100
actionLabel={m.removeDialog.confirm}

0 commit comments

Comments
 (0)