Skip to content

Commit 9cbd650

Browse files
brunol95brunol95
committed
feat(shared,ui): pass the reverification result to the retried request
A password change retried after reverification failed or overwrote the password the user had just reset: custom flows send the old `currentPassword`, and the modal could save a different password than the form's. `useReverification()` takes a `passReverificationResult` option. With it, the wrapped function receives the reverification result as its first argument (`undefined` on the first call). After a reset by code, `passwordReset` holds the new password and the sign out choice, so the retry can skip `updatePassword()`. UserProfile's PasswordForm uses it in place of `resetPassword.onPasswordChange`, which is removed. The modal records the reset attempt at the UserVerification routes level, so the result is still available when verification completes on the second factor. If an older clerk-js rejects the reset password code strategy, the code screen now shows the error instead of waiting for a code that was never sent.
1 parent 567f017 commit 9cbd650

15 files changed

Lines changed: 266 additions & 74 deletions

‎.changeset/reverification-reset-breached-password.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,14 @@ Let users whose password was found in a data breach pass reverification by reset
99
From UserProfile's change password form, the modal reuses the new password the user already entered, so they only enter the code.
1010

1111
`session.prepareFirstFactorVerification()` and `session.attemptFirstFactorVerification()` accept the `reset_password_email_code` and `reset_password_phone_code` strategies, so custom flows can build the same reset.
12+
13+
`useReverification()` accepts a new `passReverificationResult` option. With it, the fetcher receives what the reverification did as its first argument: `passwordReset` holds the new password and sign out choice when the user reset their password in the modal. Custom password change flows should adopt it: after the user resets their password to reverify, retrying `user.updatePassword()` would fail on the old `currentPassword` or overwrite the password the user just set.
14+
15+
```tsx
16+
const { user } = useUser();
17+
const updatePassword = useReverification(
18+
(reverification: ReverificationResult | undefined, params: UpdateUserPasswordParams) =>
19+
reverification?.passwordReset ? user!.reload().catch(() => user!) : user!.updatePassword(params),
20+
{ passReverificationResult: true },
21+
);
22+
```

‎packages/shared/src/react/__tests__/useReverification.spec.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { expectTypeOf } from 'expect-type';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44

55
import { reverificationError } from '../../authorization-errors';
6+
import type { ReverificationResult } from '../../types';
7+
import type { UseReverificationOptions } from '../hooks/useReverification';
68
import { useReverification as useReverificationImp } from '../hooks/useReverification';
79

810
vi.mock('../hooks/useClerk', () => {
@@ -53,6 +55,18 @@ describe('useReverification type tests', () => {
5355
>();
5456
});
5557
});
58+
describe('useReverification passReverificationResult type tests', () => {
59+
it('drops the first fetcher parameter, required or optional, from the returned function', () => {
60+
type Required = (reverification: ReverificationResult | undefined, key: string) => Promise<{ key: string }>;
61+
type Optional = (reverification?: ReverificationResult, key?: string) => Promise<{ key?: string }>;
62+
type Returned<F extends Required | Optional> = ReturnType<
63+
typeof useReverificationImp<F, { passReverificationResult: true }>
64+
>;
65+
expectTypeOf<Parameters<Returned<Required>>>().toEqualTypeOf<[key: string]>();
66+
expectTypeOf<Parameters<Returned<Optional>>>().toEqualTypeOf<[key?: string]>();
67+
});
68+
});
69+
5670
describe('useReverification', () => {
5771
const mockFetcherInner = vi.fn().mockResolvedValue({ ok: true });
5872

@@ -89,4 +103,40 @@ describe('useReverification', () => {
89103
});
90104
expect(fetchSpy).toHaveBeenCalledWith('B');
91105
});
106+
107+
describe('passReverificationResult', () => {
108+
const passwordReset = { newPassword: 'new-password-123', signOutOfOtherSessions: true };
109+
110+
// Calls the hook once: the first fetcher call needs reverification, which completes with `result`.
111+
const callWithReverification = async (options: UseReverificationOptions, result?: ReverificationResult) => {
112+
const fetchSpy = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValueOnce({ ok: true });
113+
const { result: hook } = renderHook(() =>
114+
useReverificationImp(fetchSpy, { ...options, onNeedsReverification: ({ complete }) => complete(result) }),
115+
);
116+
await act(async () => {
117+
await hook.current('arg');
118+
});
119+
return fetchSpy;
120+
};
121+
122+
it('passes undefined first, then the result given to complete, on the retry', async () => {
123+
const fetchSpy = await callWithReverification({ passReverificationResult: true }, { passwordReset });
124+
125+
expect(fetchSpy).toHaveBeenNthCalledWith(1, undefined, 'arg');
126+
expect(fetchSpy).toHaveBeenNthCalledWith(2, { passwordReset }, 'arg');
127+
});
128+
129+
it('passes undefined on the retry when complete gets no result', async () => {
130+
const fetchSpy = await callWithReverification({ passReverificationResult: true });
131+
132+
expect(fetchSpy).toHaveBeenNthCalledWith(2, undefined, 'arg');
133+
});
134+
135+
it('calls the fetcher with only its own arguments without the option', async () => {
136+
const fetchSpy = await callWithReverification({}, { passwordReset });
137+
138+
expect(fetchSpy).toHaveBeenNthCalledWith(1, 'arg');
139+
expect(fetchSpy).toHaveBeenNthCalledWith(2, 'arg');
140+
});
141+
});
92142
});

‎packages/shared/src/react/hooks/useReverification.ts‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { validateReverificationConfig } from '../../authorization';
44
import { isReverificationHint, reverificationError } from '../../authorization-errors';
55
import { ClerkRuntimeError, isClerkAPIResponseError } from '../../error';
66
import { eventMethodCalled } from '../../telemetry';
7-
import type { Clerk, SessionVerificationLevel } from '../../types';
7+
import type { Clerk, ReverificationResult, SessionVerificationLevel } from '../../types';
88
import { createDeferredPromise } from '../../utils/createDeferredPromise';
99
import { useClerk } from './useClerk';
1010
import { useSafeLayoutEffect } from './useSafeLayoutEffect';
@@ -44,8 +44,9 @@ export type NeedsReverificationParameters = {
4444
cancel: () => void;
4545
/**
4646
* Marks the reverification process as complete and retries the original request.
47+
* Pass the reverification result, if any.
4748
*/
48-
complete: () => void;
49+
complete: (result?: ReverificationResult) => void;
4950
/**
5051
* The verification level required for the reverification process.
5152
*/
@@ -67,14 +68,30 @@ export type UseReverificationOptions = {
6768
* @param properties.level - The level returned with the reverification hint.
6869
*/
6970
onNeedsReverification?: (properties: NeedsReverificationParameters) => void;
71+
/**
72+
* When `true`, the fetcher's first argument is a `ReverificationResult` (`undefined` on the first call), and the
73+
* returned function takes the remaining arguments.
74+
*
75+
* @example
76+
* ```tsx
77+
* const updatePassword = useReverification(
78+
* (reverification: ReverificationResult | undefined, params: UpdateUserPasswordParams) =>
79+
* // After a reset the new password is already saved, so skip the request.
80+
* reverification?.passwordReset ? user.reload().catch(() => user) : user.updatePassword(params),
81+
* { passReverificationResult: true },
82+
* );
83+
* ```
84+
*/
85+
passReverificationResult?: true;
7086
};
7187

7288
/**
7389
* @interface
7490
*/
75-
type UseReverificationResult<Fetcher extends (...args: any[]) => Promise<any> | undefined> = (
76-
...args: Parameters<Fetcher>
77-
) => Promise<ExcludeClerkError<Awaited<ReturnType<Fetcher>>>>;
91+
type UseReverificationResult<
92+
Fetcher extends (...args: any[]) => Promise<any> | undefined,
93+
Args extends any[] = Parameters<Fetcher>,
94+
> = (...args: Args) => Promise<ExcludeClerkError<Awaited<ReturnType<Fetcher>>>>;
7895

7996
/**
8097
* @interface
@@ -91,7 +108,9 @@ type UseReverification = <
91108
* Optional configuration object extending [`UseReverificationOptions`](https://clerk.com/docs/reference/hooks/use-reverification#usereverificationoptions).
92109
*/
93110
options?: Options,
94-
) => UseReverificationResult<Fetcher>;
111+
) => Options extends { passReverificationResult: true }
112+
? UseReverificationResult<Fetcher, Parameters<Fetcher> extends [any?, ...infer Rest] ? Rest : Parameters<Fetcher>>
113+
: UseReverificationResult<Fetcher>;
95114

96115
type CreateReverificationHandlerParams = UseReverificationOptions & {
97116
openUIComponent: Clerk['__internal_openReverification'];
@@ -108,8 +127,11 @@ function createReverificationHandler(params: CreateReverificationHandlerParams)
108127
function assertReverification<Fetcher extends (...args: any[]) => Promise<any> | undefined>(
109128
fetcher: Fetcher,
110129
): (...args: Parameters<Fetcher>) => Promise<ExcludeClerkError<Awaited<ReturnType<Fetcher>>>> {
130+
const callFetcher = (reverification: ReverificationResult | undefined, args: unknown[]) =>
131+
params.passReverificationResult ? fetcher(reverification, ...args) : fetcher(...args);
132+
111133
return (async (...args: Parameters<Fetcher>) => {
112-
let result = await resolveResult(fetcher(...args));
134+
let result = await resolveResult(callFetcher(undefined, args));
113135

114136
if (isReverificationHint(result)) {
115137
/**
@@ -129,8 +151,8 @@ function createReverificationHandler(params: CreateReverificationHandlerParams)
129151
);
130152
};
131153

132-
const complete = () => {
133-
resolvers.resolve(true);
154+
const complete = (reverificationResult?: ReverificationResult) => {
155+
resolvers.resolve(reverificationResult);
134156
};
135157

136158
if (params.onNeedsReverification === undefined) {
@@ -154,12 +176,12 @@ function createReverificationHandler(params: CreateReverificationHandlerParams)
154176
/**
155177
* Wait until the promise from above have been resolved or rejected
156178
*/
157-
await resolvers.promise;
179+
const reverificationResult = (await resolvers.promise) as ReverificationResult | undefined;
158180

159181
/**
160182
* After the promise resolved successfully try the original request one more time
161183
*/
162-
result = await resolveResult(fetcher(...args));
184+
result = await resolveResult(callFetcher(reverificationResult, args));
163185
}
164186

165187
return result;
@@ -225,6 +247,7 @@ export const useReverification: UseReverification = (fetcher, options) => {
225247
telemetry?.record(
226248
eventMethodCalled('useReverification', {
227249
onNeedsReverification: Boolean(options?.onNeedsReverification),
250+
passReverificationResult: Boolean(options?.passReverificationResult),
228251
}),
229252
);
230253

@@ -234,15 +257,17 @@ export const useReverification: UseReverification = (fetcher, options) => {
234257
optionsRef.current = options;
235258
});
236259

260+
// The returned function's parameters depend on `passReverificationResult`, which the
261+
// conditional return type of `UseReverification` describes.
237262
return useCallback(
238-
(...args) => {
263+
(...args: unknown[]) => {
239264
const handler = createReverificationHandler({
240265
openUIComponent: __internal_openReverification,
241266
telemetry,
242267
...optionsRef.current,
243268
})(fetcherRef.current);
244-
return handler(...args);
269+
return handler(...(args as Parameters<typeof fetcher>));
245270
},
246271
[__internal_openReverification, telemetry],
247-
);
272+
) as ReturnType<UseReverification>;
248273
};

‎packages/shared/src/types/clerk.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import type {
3838
SignUpForceRedirectUrl,
3939
} from './redirects';
4040
import type { SessionResource, SessionTask, SignedInSessionResource } from './session';
41-
import type { SessionVerificationLevel } from './sessionVerification';
41+
import type { ReverificationResult, SessionVerificationLevel } from './sessionVerification';
4242
import type { SignInResource } from './signIn';
4343
import type { SignUpResource } from './signUp';
4444
import type { ClientJSONSnapshot, EnvironmentJSONSnapshot } from './snapshots';
@@ -1972,7 +1972,7 @@ export type __internal_UserVerificationProps = RoutingOptions & {
19721972
/**
19731973
* Non-awaitable callback for when verification is completed successfully
19741974
*/
1975-
afterVerification?: () => void;
1975+
afterVerification?: (result?: ReverificationResult) => void;
19761976

19771977
/**
19781978
* Non-awaitable callback for when verification is cancelled, (i.e modal is closed)
@@ -1994,12 +1994,6 @@ export type __internal_UserVerificationProps = RoutingOptions & {
19941994
resetPassword?: {
19951995
password: string;
19961996
signOutOfOtherSessions: boolean;
1997-
/**
1998-
* Called when the user enters a different password in the modal because
1999-
* this one was rejected. A caller that retries its own request after
2000-
* verification must send this password instead.
2001-
*/
2002-
onPasswordChange?: (password: string) => void;
20031997
};
20041998

20051999
/**

‎packages/shared/src/types/sessionVerification.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ export interface SessionVerificationResource extends ClerkResource {
2525

2626
export type SessionVerificationStatus = 'needs_first_factor' | 'needs_second_factor' | 'complete';
2727

28+
/**
29+
* What a completed reverification did, passed to the fetcher of `useReverification` with `passReverificationResult`.
30+
*/
31+
export type ReverificationResult = {
32+
/**
33+
* Set when the user reset their password to reverify. The new password is
34+
* already saved, so a retried password change should not send it again.
35+
*/
36+
passwordReset?: {
37+
newPassword: string;
38+
signOutOfOtherSessions: boolean;
39+
};
40+
};
41+
2842
/**
2943
* @inline
3044
*/

‎packages/ui/src/Components.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
GoogleOneTapProps,
1515
InviteMembersModalProps,
1616
OrganizationProfileModalProps,
17+
ReverificationResult,
1718
SignInModalProps,
1819
SignInProps,
1920
SignUpModalProps,
@@ -468,8 +469,8 @@ const Components = (props: ComponentsProps) => {
468469
/**
469470
* When a UserVerification flow is completed, we need to close the modal without trigger a cancellation callback
470471
*/
471-
afterVerification() {
472-
props.afterVerification?.();
472+
afterVerification(result?: ReverificationResult) {
473+
props.afterVerification?.(result);
473474
componentsControls.closeModal(name, { notify: false });
474475
},
475476
},

‎packages/ui/src/components/UserProfile/PasswordForm.tsx‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useClerk, useReverification, useSession, useUser } from '@clerk/shared/react';
2-
import type { UserResource } from '@clerk/shared/types';
2+
import type { ReverificationResult, UserResource } from '@clerk/shared/types';
33
import { useRef } from 'react';
44

55
import { useCardState, withCardStateProvider } from '@/ui/elements/contexts';
@@ -44,21 +44,19 @@ export const PasswordForm = withCardStateProvider((props: PasswordFormProps) =>
4444
// to reset a breached password, the modal uses it instead of asking again.
4545
const pendingPasswordRef = useRef<UserVerificationResetPassword>();
4646
const updatePasswordWithReverification = useReverification(
47-
(user: UserResource, [params]: Parameters<UserResource['updatePassword']>) =>
48-
// The retry after reverification must send the password the modal set,
49-
// which differs from the form's when the modal rejected it.
50-
user.updatePassword({ ...params, newPassword: pendingPasswordRef.current?.password ?? params.newPassword }),
47+
(
48+
reverification: ReverificationResult | undefined,
49+
user: UserResource,
50+
opts: Parameters<UserResource['updatePassword']>,
51+
) =>
52+
// The modal already saved the password it reset; sending the form's would overwrite it.
53+
reverification?.passwordReset ? user.reload().catch(() => user) : user.updatePassword(...opts),
5154
{
55+
passReverificationResult: true,
5256
onNeedsReverification: ({ level, complete, cancel }) => {
53-
const pending = pendingPasswordRef.current;
5457
clerk.__internal_openReverification({
5558
level,
56-
resetPassword: pending && {
57-
...pending,
58-
onPasswordChange: password => {
59-
pendingPasswordRef.current = { ...pending, password };
60-
},
61-
},
59+
resetPassword: pendingPasswordRef.current,
6260
afterVerification: complete,
6361
afterVerificationCancelled: cancel,
6462
});

‎packages/ui/src/components/UserProfile/__tests__/PasswordSection.test.tsx‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -380,30 +380,33 @@ describe('PasswordSection', () => {
380380
return { fixtures, modalProps: openReverification.mock.calls[0][0]! };
381381
};
382382

383-
// The retry sends the same password; the backend treats it as already done.
384-
it('passes the new password to the modal and retries the change after verification', async () => {
383+
it('passes the new password to the modal and skips the change after the modal resets it', async () => {
385384
const { fixtures, modalProps } = await submitAndOpenReverification();
386-
expect(modalProps.resetPassword).toMatchObject({ password: 'testtest', signOutOfOtherSessions: true });
385+
expect(modalProps.resetPassword).toEqual({ password: 'testtest', signOutOfOtherSessions: true });
386+
const reload = vi.spyOn(fixtures.clerk.user!, 'reload').mockResolvedValue(fixtures.clerk.user!);
387387

388-
await act(() => modalProps.afterVerification?.());
388+
await act(() =>
389+
modalProps.afterVerification?.({
390+
passwordReset: { newPassword: 'testtest', signOutOfOtherSessions: true },
391+
}),
392+
);
389393

390-
await waitFor(() => expect(fixtures.clerk.user?.updatePassword).toHaveBeenCalledTimes(2));
391-
expect(fixtures.clerk.user?.updatePassword).toHaveBeenLastCalledWith({
392-
newPassword: 'testtest',
393-
signOutOfOtherSessions: true,
394-
});
394+
// The form closes on success; sending its password again would overwrite the one the modal saved
395+
await waitFor(() =>
396+
expect(screen.queryByRole('heading', { name: /update password/i })).not.toBeInTheDocument(),
397+
);
398+
expect(fixtures.clerk.user?.updatePassword).toHaveBeenCalledOnce();
399+
expect(reload).toHaveBeenCalledOnce();
395400
});
396401

397-
it('retries with the password the modal set after it rejected the form password', async () => {
402+
it('retries the change when the modal verified without a reset', async () => {
398403
const { fixtures, modalProps } = await submitAndOpenReverification();
399404

400-
// The modal rejected 'testtest' and the user picked another password there
401-
act(() => modalProps.resetPassword?.onPasswordChange?.('picked-in-the-modal-456'));
402-
await act(() => modalProps.afterVerification?.());
405+
await act(() => modalProps.afterVerification?.({}));
403406

404407
await waitFor(() => expect(fixtures.clerk.user?.updatePassword).toHaveBeenCalledTimes(2));
405408
expect(fixtures.clerk.user?.updatePassword).toHaveBeenLastCalledWith({
406-
newPassword: 'picked-in-the-modal-456',
409+
newPassword: 'testtest',
407410
signOutOfOtherSessions: true,
408411
});
409412
});

‎packages/ui/src/components/UserVerification/UVFactorOneCodeForm.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isKnownError } from '@clerk/shared/error';
12
import { useSession } from '@clerk/shared/react';
23
import type {
34
EmailCodeFactor,
@@ -70,7 +71,14 @@ export const UVFactorOneCodeForm = (props: UVFactorOneCodeFormProps) => {
7071
void session!
7172
.prepareFirstFactorVerification(props.factor)
7273
.then(() => props.onFactorPrepare())
73-
.catch(err => handleError(err, [], card.setError));
74+
.catch(err => {
75+
// An older clerk-js throws a plain Error for a strategy it doesn't know (a newer ui's reset
76+
// password code). Show it instead of leaving the user waiting for a code that was never sent.
77+
if (!isKnownError(err)) {
78+
return card.setError(err instanceof Error ? err.message : String(err));
79+
}
80+
return handleError(err, [], card.setError);
81+
});
7482
};
7583

7684
const action: VerificationCodeCardProps['onCodeEntryFinishedAction'] = (code, resolve, reject) => {

0 commit comments

Comments
 (0)