Skip to content

Commit 82aa181

Browse files
authored
Merge pull request #20158 from mozilla/fxa-13230
fix(fxa-settings): Prevent duplicate passwordless OTP sends on page reload
2 parents 0b72bca + 15af9dd commit 82aa181

7 files changed

Lines changed: 79 additions & 7 deletions

File tree

packages/functional-tests/tests/passwordless/signinPasswordless.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,42 @@ test.describe('severity-1 #smoke', () => {
555555
});
556556
});
557557

558+
test.describe('Repeated sign-in does not trigger rate limit', () => {
559+
test('navigating back and re-entering email does not cause too many requests error', async ({
560+
target,
561+
pages: { page, signin, relier, signinPasswordlessCode },
562+
testAccountTracker,
563+
}) => {
564+
const { email } =
565+
testAccountTracker.generatePasswordlessAccountDetails();
566+
567+
// First attempt: enter the passwordless flow (sends code #1)
568+
await relier.goto('force_passwordless=true');
569+
await relier.clickEmailFirst();
570+
await signin.fillOutEmailFirstForm(email);
571+
572+
await expect(page).toHaveURL(/signin_passwordless_code/);
573+
await expect(signinPasswordlessCode.heading).toBeVisible();
574+
575+
// Second attempt: go back to the RP and re-enter email (sends code #2)
576+
await relier.goto('force_passwordless=true');
577+
await relier.clickEmailFirst();
578+
await signin.fillOutEmailFirstForm(email);
579+
580+
// Should reach the code page without a rate limit error
581+
await expect(page).toHaveURL(/signin_passwordless_code/);
582+
await expect(signinPasswordlessCode.heading).toBeVisible();
583+
584+
// Verify no error banner is displayed (e.g. "too many requests")
585+
await expect(signinPasswordlessCode.errorBanner).not.toBeVisible();
586+
587+
// Complete the flow to confirm it works end-to-end
588+
const code = await target.emailClient.getPasswordlessSignupCode(email);
589+
await signinPasswordlessCode.fillOutCodeForm(code);
590+
expect(await relier.isLoggedIn()).toBe(true);
591+
});
592+
});
593+
558594
test.describe('Error cases', () => {
559595
test('passwordless - invalid code', async ({
560596
target,

packages/fxa-auth-server/config/rate-limit-rules.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ passkeyDelete : ip_uid : 100 : 15 mi
185185
# Passwordless Authentication OTP Limits
186186
# Controls the rate at which passwordless OTP codes can be sent and verified
187187
#
188-
passwordlessSendOtp : email : 2 : 15 minutes : 15 minutes : block
189-
passwordlessSendOtp : email : 5 : 24 hours : 12 hours : block
188+
passwordlessSendOtp : email : 5 : 15 minutes : 15 minutes : block
189+
passwordlessSendOtp : email : 15 : 24 hours : 15 minutes : block
190190
passwordlessSendOtp : ip : 50 : 24 hours : 12 hours : block
191191
passwordlessSendOtp : ip : 20 : 15 minutes : 30 minutes : block
192192
passwordlessSendOtp : ip : 100 : 24 hours : 15 minutes : ban

packages/fxa-content-server/server/lib/routes/react-app/content-server-routes.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ const FRONTEND_ROUTES = [
7474
'signin_recovery_code',
7575
'signin_recovery_choice',
7676
'signin_recovery_phone',
77+
'signin_passwordless_code',
78+
'oauth/signin_passwordless_code',
7779
'signin_confirmed',
7880
// TODO: FXA-13100 - Uncomment when passkey fallback is fully implemented
7981
// 'signin_passkey_fallback',

packages/fxa-content-server/server/lib/routes/react-app/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ const getReactRouteGroups = (showReactApp, reactRoute) => {
9797
'inline_recovery_key_setup',
9898
'signin_push_code',
9999
'signin_push_code_confirm',
100+
'signin_passwordless_code',
101+
'oauth/signin_passwordless_code',
100102
]),
101103
fullProdRollout: true,
102104
},

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/container.test.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,23 @@ describe('SigninPasswordlessCode container', () => {
177177
// Even if component re-renders, should not send again
178178
expect(mockAuthClient.passwordlessSendCode).toHaveBeenCalledTimes(1);
179179
});
180+
181+
it('does not re-send code when codeSent is in location state (page refresh)', async () => {
182+
// Simulate page refresh: location state persists via History API
183+
// with codeSent: true from the previous render's history.replaceState
184+
mockLocationState = {
185+
...createMockPasswordlessLocationState(),
186+
codeSent: true,
187+
};
188+
await render();
189+
190+
await waitFor(() => {
191+
expect(screen.getByText('signin passwordless code mock')).toBeInTheDocument();
192+
});
193+
194+
// Should NOT send code because location state has codeSent: true
195+
expect(mockAuthClient.passwordlessSendCode).not.toHaveBeenCalled();
196+
});
180197
});
181198

182199
describe('isSignup flag', () => {

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/container.tsx

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,17 @@ const SigninPasswordlessCodeContainer = ({
3030
integration
3131
);
3232

33-
const [codeSent, setCodeSent] = useState(false);
34-
const [sendError, setSendError] = useState<string | null>(null);
35-
3633
const email = location.state?.email;
3734
const service = location.state?.service;
3835
const isSignup = location.state?.isSignup;
3936

37+
const [codeSent, setCodeSent] = useState(
38+
// If location state already has codeSent (persisted across page refresh
39+
// via the History API), skip sending again.
40+
() => location.state?.codeSent === true
41+
);
42+
const [sendError, setSendError] = useState<string | null>(null);
43+
4044
const cmsInfo = integration.getCmsInfo();
4145
// Use SigninTokenCodePage layout as fallback since SigninPasswordlessCodePage doesn't exist yet
4246
const splitLayout = (cmsInfo as any)?.SigninPasswordlessCodePage?.splitLayout ||
@@ -49,20 +53,28 @@ const SigninPasswordlessCodeContainer = ({
4953
}
5054
}, [email, navigateWithQuery]);
5155

52-
// Send the initial code when component mounts
56+
// Send the initial code when component mounts, but skip if already sent
57+
// (e.g. after a page refresh). On success, replace the current history
58+
// entry with codeSent: true so the browser-persisted location state
59+
// prevents re-sending on refresh.
5360
useEffect(() => {
5461
if (email && !codeSent) {
5562
const sendCode = async () => {
5663
try {
5764
await authClient.passwordlessSendCode(email, { clientId: integration.getClientId() });
5865
setCodeSent(true);
66+
// Persist codeSent in location state so it survives page refresh
67+
navigateWithQuery(location.pathname + location.search, {
68+
replace: true,
69+
state: { ...location.state, codeSent: true },
70+
});
5971
} catch (error: any) {
6072
setSendError(error.message || 'Failed to send code');
6173
}
6274
};
6375
sendCode();
6476
}
65-
}, [email, service, codeSent, authClient, integration]);
77+
}, [email, service, codeSent, authClient, integration, navigateWithQuery, location.pathname, location.search, location.state]);
6678

6779
if (!email) {
6880
return (

packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/interfaces.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ export interface PasswordlessLocationState {
1111
service?: string;
1212
// True if user came from signup flow (new account)
1313
isSignup?: boolean;
14+
// Set to true after the initial OTP code has been sent, persisted in
15+
// location state via history.replaceState so it survives page refreshes.
16+
codeSent?: boolean;
1417
}
1518

1619
export interface SigninPasswordlessCodeContainerProps {

0 commit comments

Comments
 (0)