Skip to content

Commit 42b85b1

Browse files
authored
Merge pull request #19454 from mozilla/FXA-12400
bug(auth): Fix bad config for otp code expiration
2 parents 608f3e2 + 740d129 commit 42b85b1

3 files changed

Lines changed: 15 additions & 2 deletions

File tree

packages/fxa-auth-server/config/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2515,13 +2515,16 @@ const convictConf = convict({
25152515
env: 'MFA__OTP__EPOCH',
25162516
},
25172517
step: {
2518+
// The time interval to use. In this case 1 second
25182519
default: 1,
25192520
doc: 'Overrides step otp options',
25202521
format: Number,
25212522
env: 'MFA__OTP__STEP',
25222523
},
25232524
window: {
2524-
default: 60 * 5,
2525+
// Number of steps contained in the window. In this case
2526+
// 5 minutes worth of steps
2527+
default: 5 * 60,
25252528
doc: 'Overrides window otp options',
25262529
format: Number,
25272530
env: 'MFA__OTP__WINDOW',

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,20 @@ class MfaHandler {
7575
const options = this.config.mfa.otp;
7676
const code = await this.otpUtils.generateOtpCode(secret, options);
7777

78+
// Convert seconds to minutes. The step is a time interval in seconds, and
79+
// the window is the the number of intervals the code is valid for.
80+
// For example if the step is 1 second, and the interval is 30, the code
81+
// is valid for 30s.
82+
// For specifics see: https://www.npmjs.com/package/otplib
83+
const expirationTime = (options.step * options.window) / 60;
84+
7885
await this.mailer.sendVerifyAccountChangeEmail(
7986
account.emails?.map((x) => x.email) || [account.email],
8087
account,
8188
{
8289
code,
8390
uid,
84-
expirationTime: this.config.mfa.otp.window || 5,
91+
expirationTime,
8592
}
8693
);
8794

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ describe('mfa', () => {
3737
actions: ['test'],
3838
otp: {
3939
digits: 6,
40+
// Code would be valid for 30 seconds
41+
step: 1,
42+
window: 30,
4043
},
4144
jwt: {
4245
secretKey: 'foxes',

0 commit comments

Comments
 (0)