Skip to content

Commit 144f0fb

Browse files
dschomdschom
authored
Merge pull request #20121 from mozilla/FXA-13162
Commits on Feb 27, 2026 bug(settings): Entering invalid ARK breaks reset flow
2 parents 4c9d8d7 + 37ca5f3 commit 144f0fb

File tree

8 files changed

+347
-2
lines changed

8 files changed

+347
-2
lines changed

‎packages/db-migrations/databases/fxa/patches/patch-185-186.sql‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
SET NAMES utf8mb4 COLLATE utf8mb4_bin;
2+
3+
CALL assertPatchLevel('185');
4+
5+
CREATE PROCEDURE `forgotPasswordVerified_10` (
6+
IN `inPasswordForgotTokenId` BINARY(32),
7+
IN `inAccountResetTokenId` BINARY(32),
8+
IN `inTokenData` BINARY(32),
9+
IN `inUid` BINARY(16),
10+
IN `inCreatedAt` BIGINT UNSIGNED,
11+
IN `inVerificationMethod` INT
12+
)
13+
BEGIN
14+
DECLARE EXIT HANDLER FOR SQLEXCEPTION
15+
BEGIN
16+
-- ERROR
17+
ROLLBACK;
18+
RESIGNAL;
19+
END;
20+
21+
START TRANSACTION;
22+
23+
-- Pass `inVerificationMethod` to the password forgot token table
24+
REPLACE INTO accountResetTokens(
25+
tokenId,
26+
tokenData,
27+
uid,
28+
createdAt,
29+
verificationMethod
30+
)
31+
VALUES(
32+
inAccountResetTokenId,
33+
inTokenData,
34+
inUid,
35+
inCreatedAt,
36+
inVerificationMethod
37+
);
38+
39+
UPDATE accounts SET emailVerified = true WHERE uid = inUid;
40+
UPDATE emails SET isVerified = true, verifiedAt = (UNIX_TIMESTAMP(NOW(3)) * 1000) WHERE isPrimary = true AND uid = inUid;
41+
42+
COMMIT;
43+
END;
44+
45+
UPDATE dbMetadata SET value = '186' WHERE name = 'schema-patch-level';

‎packages/db-migrations/databases/fxa/patches/patch-186-185.sql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- SET NAMES utf8mb4 COLLATE utf8mb4_bin;
2+
3+
-- DROP PROCEDURE `forgotPasswordVerified_10`;
4+
5+
-- UPDATE dbMetadata SET value = '185' WHERE name = 'schema-patch-level';

‎packages/db-migrations/databases/fxa/target-patch.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"level": 185
2+
"level": 186
33
}

‎packages/functional-tests/pages/confirmTotpResetPassword.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import { BaseTokenCodePage } from './baseTokenCode';
6+
7+
export class ConfirmTotpResetPassword extends BaseTokenCodePage {
8+
readonly path = '/confirm_totp_reset_password';
9+
}

‎packages/functional-tests/pages/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { AvatarPage } from './settings/avatar';
77
import { BaseTarget } from '../lib/targets/base';
88
import { ConfigPage } from './config';
99
import { ConfirmSignupCodePage } from './confirmSignupCode';
10+
import { ConfirmTotpResetPassword } from './confirmTotpResetPassword';
1011
import { ConnectAnotherDevicePage } from './connectAnotherDevice';
1112
import { CookiesDisabledPage } from './cookiesDisabled';
1213
import { ChangePasswordPage } from './settings/changePassword';
@@ -47,6 +48,7 @@ export function create(page: Page, target: BaseTarget) {
4748
changePassword: new ChangePasswordPage(page, target),
4849
configPage: new ConfigPage(page, target),
4950
confirmSignupCode: new ConfirmSignupCodePage(page, target),
51+
confirmTotpResetPassword: new ConfirmTotpResetPassword(page, target),
5052
connectAnotherDevice: new ConnectAnotherDevicePage(page, target),
5153
cookiesDisabled: new CookiesDisabledPage(page, target),
5254
deleteAccount: new DeleteAccountPage(page, target),

‎packages/functional-tests/pages/settings/totp.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ export class TotpPage extends SettingsLayout {
9494
return this.page.getByRole('button', { name: 'Finish' });
9595
}
9696

97+
get confirmBackupCodeConfirmButton() {
98+
return this.page.getByRole('button', { name: 'Confirm' });
99+
}
100+
97101
get confirmBackupCodeTextbox() {
98102
return (
99103
this.page

‎packages/functional-tests/tests/resetPassword/resetPassword2FA.spec.ts‎

Lines changed: 280 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,176 @@ test.describe('severity-1 #smoke', () => {
338338
await expect(settings.recoveryKey.status).toHaveText('Not Set');
339339
});
340340

341+
test('provide invalid recovery key then reset with totp authenticator code', async ({
342+
page,
343+
target,
344+
pages: {
345+
signin,
346+
resetPassword,
347+
settings,
348+
totp,
349+
confirmTotpResetPassword,
350+
recoveryKey,
351+
},
352+
testAccountTracker,
353+
}) => {
354+
const credentials = await testAccountTracker.signUp();
355+
356+
// Sign Into Settings
357+
await signin.goto();
358+
await signin.fillOutEmailFirstForm(credentials.email);
359+
await signin.fillOutPasswordForm(credentials.password);
360+
await expect(page).toHaveURL(/settings/);
361+
362+
// Create Recovery Key
363+
await settings.recoveryKey.createButton.click();
364+
await settings.confirmMfaGuard(credentials.email);
365+
await recoveryKey.createRecoveryKey(credentials.password, 'hint');
366+
await expect(settings.settingsHeading).toBeVisible();
367+
await expect(settings.recoveryKey.status).toHaveText('Enabled');
368+
369+
// Enable 2FA
370+
await expect(settings.totp.status).toHaveText('Disabled');
371+
await settings.totp.addButton.click();
372+
await settings.confirmMfaGuard(credentials.email);
373+
const { secret } =
374+
await totp.setUpTwoStepAuthWithQrAndBackupCodesChoice(credentials);
375+
await expect(settings.settingsHeading).toBeVisible();
376+
await expect(settings.alertBar).toContainText(
377+
'Two-step authentication has been enabled'
378+
);
379+
await expect(settings.totp.status).toHaveText('Enabled');
380+
381+
// Start Reset Password Flow
382+
await settings.signOut();
383+
await signin.goto();
384+
await signin.fillOutEmailFirstForm(credentials.email);
385+
await signin.forgotPasswordLink.click();
386+
await resetPassword.fillOutEmailForm(credentials.email);
387+
const code = await target.emailClient.getResetPasswordCode(
388+
credentials.email
389+
);
390+
await await resetPassword.fillOutResetPasswordCodeForm(code);
391+
392+
// Enter Invalid Recovery Key
393+
await expect(resetPassword.confirmRecoveryKeyHeading).toBeVisible();
394+
await resetPassword.recoveryKeyTextbox.fill(
395+
'12345678-12345678-12345678-12345678'
396+
);
397+
await resetPassword.confirmRecoveryKeyButton.click();
398+
await expect(resetPassword.errorBanner).toBeVisible();
399+
400+
// Note! This is the start of edge case this test validates. When we provided
401+
// a recovery key, we took our password forgot token and exchange it for an
402+
// account reset token, which resulted in the passwordForgotToken becoming
403+
// invalid. We therefore must use the account reset token for the rest of
404+
// the web requests in this flow.
405+
406+
// Click Forgot Key Link
407+
await resetPassword.forgotKeyLink.click();
408+
409+
// Provide TOTP Code from Authenticator
410+
await page.waitForURL(/confirm_totp_reset_password/);
411+
await expect(page.getByLabel('Enter 6-digit code')).toBeVisible();
412+
const totpCode = await getTotpCode(secret);
413+
await confirmTotpResetPassword.fillOutCodeForm(totpCode);
414+
415+
// Create a New Password
416+
await expect(resetPassword.dataLossWarning).toBeVisible();
417+
const newPassword = testAccountTracker.generatePassword();
418+
await resetPassword.fillOutNewPasswordForm(newPassword);
419+
testAccountTracker.updateAccountPassword(credentials.email, newPassword);
420+
421+
// Observe Settings
422+
await expect(settings.settingsHeading).toBeVisible();
423+
await expect(settings.alertBar).toHaveText('Your password has been reset');
424+
});
425+
426+
test('provide invalid recovery key then reset with totp back up code', async ({
427+
page,
428+
target,
429+
pages: { signin, resetPassword, settings, totp, recoveryKey },
430+
testAccountTracker,
431+
}) => {
432+
const credentials = await testAccountTracker.signUp();
433+
434+
// Sign Into Settings
435+
await signin.goto();
436+
await signin.fillOutEmailFirstForm(credentials.email);
437+
await signin.fillOutPasswordForm(credentials.password);
438+
439+
// Create Recovery Key
440+
await expect(page).toHaveURL(/settings/);
441+
await settings.recoveryKey.createButton.click();
442+
await settings.confirmMfaGuard(credentials.email);
443+
await recoveryKey.createRecoveryKey(credentials.password, 'hint');
444+
445+
await expect(settings.settingsHeading).toBeVisible();
446+
await expect(settings.recoveryKey.status).toHaveText('Enabled');
447+
448+
// Enable 2FA
449+
await expect(settings.settingsHeading).toBeVisible();
450+
await expect(settings.totp.status).toHaveText('Disabled');
451+
await settings.totp.addButton.click();
452+
await settings.confirmMfaGuard(credentials.email);
453+
const { recoveryCodes } =
454+
await totp.setUpTwoStepAuthWithQrAndBackupCodesChoice(credentials);
455+
456+
await expect(settings.settingsHeading).toBeVisible();
457+
await expect(settings.alertBar).toContainText(
458+
'Two-step authentication has been enabled'
459+
);
460+
await expect(settings.totp.status).toHaveText('Enabled');
461+
462+
// Start Reset Password Flow
463+
await settings.signOut();
464+
await signin.goto();
465+
await signin.fillOutEmailFirstForm(credentials.email);
466+
await signin.forgotPasswordLink.click();
467+
await resetPassword.fillOutEmailForm(credentials.email);
468+
const code = await target.emailClient.getResetPasswordCode(
469+
credentials.email
470+
);
471+
await resetPassword.fillOutResetPasswordCodeForm(code);
472+
473+
// Enter Invalid Recovery Key
474+
await expect(resetPassword.confirmRecoveryKeyHeading).toBeVisible();
475+
await resetPassword.recoveryKeyTextbox.fill(
476+
'12345678-12345678-12345678-12345678'
477+
);
478+
await resetPassword.confirmRecoveryKeyButton.click();
479+
await expect(resetPassword.errorBanner).toBeVisible();
480+
481+
/// Note! This is the start of edge case this test validates. When we provided
482+
// a recovery key, we took our password forgot token and exchange it for an
483+
// account reset token, which resulted in the passwordForgotToken becoming
484+
// invalid. We therefore must use the account reset token for the rest of
485+
// the web requests in this flow.
486+
487+
// Click Forgot Key Link
488+
await resetPassword.forgotKeyLink.click();
489+
490+
// Verify TOTP code entry page is shown
491+
await page.waitForURL(/confirm_totp_reset_password/);
492+
await expect(page.getByLabel('Enter 6-digit code')).toBeVisible();
493+
await resetPassword.clickTroubleEnteringCode();
494+
495+
// Provide a Backup TOTP Codes
496+
await expect(totp.confirmBackupCodeHeading).toBeVisible();
497+
await totp.confirmBackupCodeTextbox.fill(recoveryCodes[0]);
498+
await totp.confirmBackupCodeConfirmButton.click();
499+
500+
// Create a New Password
501+
await expect(resetPassword.dataLossWarning).toBeVisible();
502+
const newPassword = testAccountTracker.generatePassword();
503+
await resetPassword.fillOutNewPasswordForm(newPassword);
504+
testAccountTracker.updateAccountPassword(credentials.email, newPassword);
505+
506+
// Observe Settings Page
507+
await expect(settings.settingsHeading).toBeVisible();
508+
await expect(settings.alertBar).toHaveText('Your password has been reset');
509+
});
510+
341511
test('can reset password with unverified 2FA and skip recovery key', async ({
342512
page,
343513
target,
@@ -570,4 +740,114 @@ test.describe('reset password with recovery phone', () => {
570740

571741
await expect(settings.settingsHeading).toBeVisible();
572742
});
743+
744+
test('provide invalid recovery key then reset with recovery phone', async ({
745+
page,
746+
target,
747+
pages: {
748+
signin,
749+
resetPassword,
750+
settings,
751+
totp,
752+
recoveryKey,
753+
recoveryPhone,
754+
},
755+
testAccountTracker,
756+
}) => {
757+
const credentials = await testAccountTracker.signUp();
758+
const testNumber = target.smsClient.getPhoneNumber();
759+
760+
// Sign Into Settings
761+
await signin.goto();
762+
await signin.fillOutEmailFirstForm(credentials.email);
763+
await signin.fillOutPasswordForm(credentials.password);
764+
await expect(page).toHaveURL(/settings/);
765+
766+
// Create Recovery Key
767+
await settings.recoveryKey.createButton.click();
768+
await settings.confirmMfaGuard(credentials.email);
769+
await recoveryKey.createRecoveryKey(credentials.password, 'hint');
770+
await expect(settings.settingsHeading).toBeVisible();
771+
await expect(settings.recoveryKey.status).toHaveText('Enabled');
772+
773+
// Enable 2FA
774+
await expect(settings.settingsHeading).toBeVisible();
775+
await expect(settings.totp.status).toHaveText('Disabled');
776+
await settings.totp.addButton.click();
777+
await settings.confirmMfaGuard(credentials.email);
778+
await totp.setUpTwoStepAuthWithQrAndBackupCodesChoice(credentials);
779+
await expect(settings.settingsHeading).toBeVisible();
780+
await expect(settings.alertBar).toContainText(
781+
'Two-step authentication has been enabled'
782+
);
783+
await expect(settings.totp.status).toHaveText('Enabled');
784+
785+
// Enable Recovery Phone
786+
await settings.totp.addRecoveryPhoneButton.click();
787+
await page.waitForURL(/recovery_phone\/setup/);
788+
await expect(recoveryPhone.addHeader()).toBeVisible();
789+
await recoveryPhone.enterPhoneNumber(testNumber);
790+
await recoveryPhone.clickSendCode();
791+
await expect(recoveryPhone.confirmHeader).toBeVisible();
792+
let smsCode = await target.smsClient.getCode({ ...credentials });
793+
await recoveryPhone.enterCode(smsCode);
794+
await recoveryPhone.clickConfirm();
795+
await page.waitForURL(/settings/);
796+
await expect(settings.alertBar).toHaveText('Recovery phone added');
797+
798+
// Start Reset Password Flow
799+
await settings.signOut();
800+
await signin.goto();
801+
await signin.fillOutEmailFirstForm(credentials.email);
802+
await signin.forgotPasswordLink.click();
803+
await resetPassword.fillOutEmailForm(credentials.email);
804+
const code = await target.emailClient.getResetPasswordCode(
805+
credentials.email
806+
);
807+
await resetPassword.fillOutResetPasswordCodeForm(code);
808+
await expect(resetPassword.confirmRecoveryKeyHeading).toBeVisible();
809+
810+
// Enter Invalid Recovery Key
811+
await resetPassword.recoveryKeyTextbox.fill(
812+
'12345678-12345678-12345678-12345678'
813+
);
814+
await resetPassword.confirmRecoveryKeyButton.click();
815+
await expect(resetPassword.errorBanner).toBeVisible();
816+
817+
// Note! This is the start of edge case this test validates. When we provided
818+
// a recovery key, we took our password forgot token and exchange it for an
819+
// account reset token, which resulted in the passwordForgotToken becoming
820+
// invalid. We therefore must use the account reset token for the rest of
821+
// the web requests in this flow.
822+
823+
// Click Forgot Key Link
824+
await resetPassword.forgotKeyLink.click();
825+
826+
// Verify TOTP code entry page is shown
827+
await page.waitForURL(/confirm_totp_reset_password/);
828+
await expect(page.getByLabel('Enter 6-digit code')).toBeVisible();
829+
await resetPassword.clickTroubleEnteringCode();
830+
831+
// Choose Recovery Phone Option
832+
await page.waitForURL(/reset_password_totp_recovery_choice/);
833+
await resetPassword.clickChoosePhone();
834+
await resetPassword.clickContinueButton();
835+
836+
// Provide SMS Code
837+
await page.waitForURL(/reset_password_recovery_phone/);
838+
839+
smsCode = await target.smsClient.getCode({ ...credentials });
840+
await resetPassword.fillRecoveryPhoneCodeForm(smsCode);
841+
await resetPassword.clickConfirmButton();
842+
843+
// Create a New Password
844+
await expect(resetPassword.dataLossWarning).toBeVisible();
845+
const newPassword = testAccountTracker.generatePassword();
846+
await resetPassword.fillOutNewPasswordForm(newPassword);
847+
testAccountTracker.updateAccountPassword(credentials.email, newPassword);
848+
849+
// Observe Settings Page
850+
await expect(settings.settingsHeading).toBeVisible();
851+
await expect(settings.alertBar).toHaveText('Your password has been reset');
852+
});
573853
});

‎packages/fxa-shared/db/models/auth/base-auth.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export enum Proc {
4343
DeviceFromRefreshTokenId = 'deviceFromRefreshTokenId_1',
4444
EmailBounces = 'fetchEmailBounces_4',
4545
FindLargeAccounts = 'findLargeAccounts_1',
46-
ForgotPasswordVerified = 'forgotPasswordVerified_9',
46+
ForgotPasswordVerified = 'forgotPasswordVerified_10',
4747
KeyFetchToken = 'keyFetchToken_1',
4848
KeyFetchTokenWithVerificationStatus = 'keyFetchTokenWithVerificationStatus_2',
4949
LimitSessions = 'limitSessions_3',

0 commit comments

Comments
 (0)