Skip to content

Create MFA Guard that Manages JWTs#19383

Merged
dschom merged 1 commit intomainfrom
FXA-12220
Sep 3, 2025
Merged

Create MFA Guard that Manages JWTs#19383
dschom merged 1 commit intomainfrom
FXA-12220

Conversation

@dschom
Copy link
Copy Markdown
Contributor

@dschom dschom commented Sep 2, 2025

Because

  • We want to wrap certain pages with an MFA requirement

This pull request

  • Updates the cache to hold a JWT
  • Creates a guard component that invokes the MFA Modal if the JWT is missing
  • Creates an error boundary that clears invalid or expired JWTs

Issue that this pull request solves

Closes: FXA-12220, FXA-12305, FXA-12304

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)

Test driver page:
image

image

Other information (Optional)

There will be a couple follow ups filed for error handling. This PR doesn't currently address the following things:

  • The invalid code errors. We need to trigger the tool tip if the server reports back saying the OTP code is invalid.
  • Rate limiting errors. We have some changes planned around how we handle unblock codes anyways, so this wasn't addressed in this PR.

To test manually, create account / login so that you are on the /settings page. Then manually navigate to /settings/mfa_guard/test/auth_client and test flow / error boundary.

@dschom dschom requested review from a team as code owners September 2, 2025 16:14
@dschom dschom force-pushed the FXA-12220 branch 6 times, most recently from 3f5372b to 2e41dec Compare September 2, 2025 19:10
Comment thread packages/fxa-settings/src/components/Settings/MfaGuard/error-boundary.test.tsx Outdated
@dschom dschom force-pushed the FXA-12220 branch 3 times, most recently from c62d3bf to 8e01d54 Compare September 2, 2025 20:38
@dschom dschom changed the title Fxa 12220 Create MFA Guard that Manages JWTs Sep 2, 2025
recoveryCodes?: string[];
};

export type MfaScope = 'test';
Copy link
Copy Markdown
Contributor

@chenba chenba Sep 2, 2025

Choose a reason for hiding this comment

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

The MfaScope type is just 'test' doesn't look right. I thought it's a string that represents of describes the action the user wants to take with the token?

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.

What you thought is correct. Currently the endpoint we use for the POC takes a 'test' scope. I think this 'test' value will go away once start actually applying this endpoints.

@chenba
Copy link
Copy Markdown
Contributor

chenba commented Sep 2, 2025

My http://localhost:3030/settings/mfa_guard/test/auth_client looks different than your screenshot. What steps do I need to perform first?

Copy link
Copy Markdown
Contributor

@chenba chenba left a comment

Choose a reason for hiding this comment

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

r+wc

Because:
- We want to wrap certain pages with an MFA requirement

This Commit:
- Updates the cache to hold a JWT
- Creates a guard component that invokes the MFA Modal if the JWT is missing
- Creates an error boundary that clears invalid or expired JWTs
@dschom
Copy link
Copy Markdown
Contributor Author

dschom commented Sep 3, 2025

My http://localhost:3030/settings/mfa_guard/test/auth_client looks different than your screenshot. What steps do I need to perform first?

@chenba

I just restarted the stack, and verified that it appears to be work. Also attached is video of expected interactions. I noticed a test failed after rebasing on main and pushing this commit. Perhaps you were affected by this. It should be working now though. If you like you can test again when reviewing this PR, which also has an example of how to do this gql.

Screen.Recording.2025-09-02.at.5.25.31.PM.mov

@dschom dschom merged commit fe1e42b into main Sep 3, 2025
19 checks passed
@dschom dschom deleted the FXA-12220 branch September 3, 2025 00:59
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.

2 participants