Skip to content

Commit 7550ea8

Browse files
authored
Merge pull request #19506 from mozilla/rate-limit-mfa
chore(auth): Define default rate-limit policies for the MFA dialog and fix error banner.
2 parents 1329a42 + 2f1a6c5 commit 7550ea8

5 files changed

Lines changed: 47 additions & 5 deletions

File tree

libs/accounts/rate-limit/src/lib/config.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ export function parseConfigRules(
7979
} satisfies Rule;
8080

8181
// A couple sanity checks to catch bad rule configuration
82-
if (!/^[a-zA-Z]*$/.test(action)) {
82+
if (!/^[a-zA-Z0-9]*$/.test(action)) {
8383
throw new InvalidRule(
84-
`Actions can only contain characters a-zA-Z.`,
84+
`Actions can only contain characters a-zA-Z0-9.`,
8585
line
8686
);
8787
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,18 @@ recoveryPhoneSendSetupCode : ip : 100 : 24 hou
136136
#
137137
recoveryPhoneSendSigninCode : ip_uid : 6 : 24 hours : 24 hours : block
138138
recoveryPhoneSendSigninCode : ip : 100 : 24 hours : 15 minutes : ban
139+
140+
#
141+
# MFA Dialogs
142+
# This controls the otp code sending and verification for the MfaGuard. Note that we define these per valid mfa type/action
143+
# so that dialogs don't interfere with each other. ie The counts that unblock email functionality are kept separate from
144+
# the counts that effect 2FA activity.
145+
#
146+
mfaOtpCodeRequestForEmail : uid : 10 : 5 minutes : 15 minutes : block
147+
mfaVerifyOtpCodeForEmail : uid : 5 : 5 minutes : 15 minutes : block
148+
mfaOtpCodeRequestFor2fa : uid : 10 : 5 minutes : 15 minutes : block
149+
mfaVerifyOtpCodeFor2fa : uid : 5 : 5 minutes : 15 minutes : block
150+
mfaOtpCodeRequestForPassword : uid : 10 : 5 minutes : 15 minutes : block
151+
mfaVerifyOtpCodeForPassword : uid : 5 : 5 minutes : 15 minutes : block
152+
mfaOtpCodeRequestForRecoveryKey : uid : 10 : 5 minutes : 15 minutes : block
153+
mfaVerifyOtpCodeForRecoveryKey : uid : 5 : 5 minutes : 15 minutes : block

packages/fxa-auth-server/lib/routes/mfa.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ interface DB {
4545
securityEvent: (arg: SecurityEvent) => Promise<void>;
4646
}
4747

48+
const toPascal = (str) =>
49+
str.replace(/(^|_)(\w)/g, (_, __, c) => c.toUpperCase());
50+
4851
class MfaHandler {
4952
constructor(
5053
private readonly config: ConfigType,
@@ -66,7 +69,7 @@ class MfaHandler {
6669
request,
6770
uid,
6871
account.email,
69-
'mfaOtpCodeRequest'
72+
`mfaOtpCodeRequestFor${toPascal(action)}` // turns mfaOtpCodeRequest_recovery_key into mfaOtpCodeRequestRecoveryKey
7073
);
7174

7275
let success = false;
@@ -133,7 +136,7 @@ class MfaHandler {
133136
request,
134137
uid,
135138
account.email,
136-
'mfaVerifyOtpCode'
139+
`mfaOtpCodeVerifyFor${toPascal(action)}` // turns mfaOtpCodeRequest_recovery_key into mfaOtpCodeRequestRecoveryKey
137140
);
138141

139142
let success = false;

packages/fxa-auth-server/test/local/routes/mfa.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ describe('mfa', () => {
100100
// for testing purposes this is sufficient.
101101
id: SESSION_TOKEN_ID,
102102
uid: UID,
103-
uaBrowser: UA_BROWSER
103+
uaBrowser: UA_BROWSER,
104104
});
105105

106106
Container.set(OtpUtils, otpUtils);
@@ -169,6 +169,22 @@ describe('mfa', () => {
169169
// session that this token was issued from.
170170
assert.equal(authResult.credentials.id, SESSION_TOKEN_ID);
171171
assert.equal(authResult.credentials.uaBrowser, UA_BROWSER);
172+
173+
// Make sure customs was invoked
174+
assert.calledWith(
175+
customs.checkAuthenticated,
176+
sinon.match.any,
177+
UID,
178+
TEST_EMAIL,
179+
'mfaOtpCodeRequestForTest'
180+
);
181+
assert.calledWith(
182+
customs.checkAuthenticated,
183+
sinon.match.any,
184+
UID,
185+
TEST_EMAIL,
186+
'mfaOtpCodeVerifyForTest'
187+
);
172188
});
173189

174190
it('will not allow an invalid token', async () => {

packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@ export const MfaGuard = ({
123123
handleError(err);
124124
return;
125125
}
126+
if (err.code === 429) {
127+
setShowResendSuccessBanner(false);
128+
setLocalizedErrorBannerMessage(
129+
getLocalizedErrorMessage(ftlMsgResolver, err)
130+
);
131+
return;
132+
}
133+
126134
Sentry.captureException(err);
127135
alertBar.error(getLocalizedErrorMessage(ftlMsgResolver, err));
128136
onDismiss();

0 commit comments

Comments
 (0)