Skip to content

task(settings) Wrap add recovery phone and replace recovery with MFA guard#19469

Merged
vpomerleau merged 1 commit intomainfrom
FXA-12225
Sep 23, 2025
Merged

task(settings) Wrap add recovery phone and replace recovery with MFA guard#19469
vpomerleau merged 1 commit intomainfrom
FXA-12225

Conversation

@dschom
Copy link
Copy Markdown
Contributor

@dschom dschom commented Sep 17, 2025

Because

  • We want to require MFA when adding a recovery phone
  • We want to require MFA when replacing a recovery phone

This pull request

  • Wraps PageRecoveryPhoneSetup with an MfaGuard
  • Adds AuthClient.recoveryPhoneCreateWithJwt
  • Updates Account.addRecoveryPhone to use AuthClient.recoveryPhoneCreateWithJwt
  • Adds new auth endpoint /mfa/recovery_phone/create

Issue that this pull request solves

Closes: FXA-12225, FXA-12233

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@dschom dschom force-pushed the FXA-12225 branch 6 times, most recently from cf3a62a to 482c25b Compare September 18, 2025 18:20
@dschom dschom marked this pull request as ready for review September 18, 2025 18:20
@dschom dschom requested a review from a team as a code owner September 18, 2025 18:20
// Decorate session token with scope
sessionToken.scope = decoded.scope;

console.log('!!! ', sessionToken);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray log

try {
await account.addRecoveryPhoneWithJwt(phoneData.phoneNumber);
} catch (e) {
if (e && e.code === 401 && e.errno === 110) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably pull these into constants at some point.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Which reminds, @nshirley add this nice little helper function in this PR. I'll update my checks accordingly. https://github.com/mozilla/fxa/pull/19468/files#r2356422594

Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll go ahead and merge this to apply some of these changes to 2FA setup. I'll pull in the remaining non-blocking change requests to the 2FA setup PR

@vpomerleau vpomerleau merged commit 5179132 into main Sep 23, 2025
19 checks passed
@vpomerleau vpomerleau deleted the FXA-12225 branch September 23, 2025 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants