Skip to content

feat(profile-sync): add MFA step-up sessions - #10267

Open
mathieuartu wants to merge 2 commits into
mfa/controller-enrollmentfrom
mfa/controller-stepup
Open

mathieuartu wants to merge 2 commits into
mfa/controller-enrollmentfrom
mfa/controller-stepup

Conversation

@mathieuartu

@mathieuartu mathieuartu commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Explanation

Adds MFA step-up verification and a memory-only elevated AAL2 session on AuthenticationController.

  • beginStepUp, completeStepUp, getElevatedProfileToken, clearStepUpSession.
  • In-memory elevated token with TTL; AuthenticationController:stepUpSession on open/close.
  • Session is torn down on lock, sign-out, wallet reset, successful enrollment, and when getElevatedProfileToken observes expiry.
  • Completing enrollment or step-up re-asserts the wallet is still unlocked after network calls so a lock during the request cannot reopen an AAL2 session.
  • MFA network steps are traced with operation / credentialType tags and span attributes for outcome / mfaCode.

References

Depends on #10266. Stack: stack #10268.
Related to: https://consensyssoftware.atlassian.net/browse/MUL-2263

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

High Risk
Introduces MFA step-up and in-memory elevated token handling in the authentication controller, including session lifecycle and mid-flight session invalidation—security-sensitive behavior for sensitive operations.

Overview
Adds MFA step-up verification and a short-lived elevated (AAL2) session on AuthenticationController, with messenger actions for beginStepUp, completeStepUp, getElevatedProfileToken, and clearStepUpSession.

After completeStepUp, the controller exchanges the MFA assertion for an access token, validates AAL2 JWT claims, and keeps the token in memory only; persisted state exposes only stepUpSessionExpiresAt (plus configurable stepUpSessionTtlMs, default 60s, clamped to token exp). getElevatedProfileToken can enforce caller freshness via maxSessionAgeMs without tearing down the session.

Sessions are cleared on wallet lock, sign-out, reset, successful credential enrollment, TTL/token expiry, and explicit clearStepUpSession. In-flight step-up/enrollment is tied to #authSessionEpoch so a lock mid-request cannot reopen an elevated session.

Related hardening: MFA authentication_required while unlocked invalidates the primary SRP session; MFA tracing records outcome/mfaCode via trace span attributes instead of mutating trace data. README, changelog, and broad unit tests cover the new flow.

Reviewed by Cursor Bugbot for commit 07823f4. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@mathieuartu
mathieuartu force-pushed the mfa/controller-stepup branch 4 times, most recently from 7826fcb to e84159d Compare September 16, 2026 21:03
@mathieuartu
mathieuartu force-pushed the mfa/controller-stepup branch 2 times, most recently from 9c60aec to 26775e8 Compare September 17, 2026 07:46
@mathieuartu
mathieuartu force-pushed the mfa/controller-stepup branch 2 times, most recently from b53422a to d418aab Compare September 17, 2026 13:41

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

pull Bot pushed a commit to Reality2byte/core that referenced this pull request Sep 17, 2026
## Explanation

Adds the MFA validation layer in `@metamask/profile-sync-controller`
that later PRs in this stack consume.

- Superstruct schemas and inferred types for auth-service MFA responses,
WebAuthn ceremony payloads, and controller-boundary requests (passkey
and email OTP only).
- `MfaError` family with a stable enumerable `mfaCode`, plus helpers
that survive JSON-RPC serialization (`getMfaErrorCode`, `isMfaError`,
`getMfaRetryAfterMs`).
- Shared `decodeJwtPayload` (no signature verification) and slightly
stricter login JWT `exp` handling.
- Expanded `HTTP_STATUS_CODES` for later MFA error mapping.

This PR does not call MFA HTTP endpoints or change controller behavior.

## References

Stacked under [stack
#10268](https://github.com/MetaMask/core/pull/10268). Follow-ups:
MetaMask#10265, MetaMask#10266, MetaMask#10267.
Related to: https://consensyssoftware.atlassian.net/browse/MUL-2260

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches authentication-adjacent validation, JWT claim handling, and
structured MFA errors, though runtime sign-in/MFA flows are unchanged
until follow-up PRs.
> 
> **Overview**
> Introduces the **MFA validation layer** in
`@metamask/profile-sync-controller` for later stack PRs—no MFA HTTP
calls or controller behavior changes in this diff.
> 
> Adds Superstruct schemas and inferred types under
`authentication-jwt-bearer/mfa` for passkey and email OTP flows:
auth-service responses, WebAuthn ceremony payloads, enrollment/step-up
requests, and **AAL2 elevated-token** claim parsing (including Hydra
`ext` nesting). `assertValidMfaRequest` / `assertValidMfaResponse` map
validation failures to `MfaError` with path details.
> 
> Expands **`MfaError`** with typed subclasses, stable enumerable
**`mfaCode`**, and helpers (`getMfaErrorCode`, `isMfaError`,
`getMfaRetryAfterMs`) that work after JSON-RPC serialization. Adds
shared **`decodeJwtPayload`** and refactors login JWT expiry checks to
use it with stricter `exp` typing. **`HTTP_STATUS_CODES`** gains `401`
and `502` for upcoming error mapping.
> 
> Dependencies: **`@metamask/superstruct`** (runtime),
**`@metamask/rpc-errors`** (tests). Changelog updated.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
d6d147b. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
@mathieuartu
mathieuartu force-pushed the mfa/controller-stepup branch 2 times, most recently from 8e7c721 to b6ae9e2 Compare September 17, 2026 16:32

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 98c36cc. Configure here.

@mathieuartu
mathieuartu force-pushed the mfa/controller-stepup branch 2 times, most recently from b04a6c3 to 89d2052 Compare September 18, 2026 13:47
pull Bot pushed a commit to Reality2byte/core that referenced this pull request Sep 18, 2026
## Explanation

Wires the MFA HTTP client into the SRP JWT auth SDK on top of the
validation foundation.

- `mfa/services` calls `/api/v2/mfa/*` (enroll, enroll complete, verify,
verify complete, credentials), validates request/response bodies, maps
server codes to `MfaError` subclasses, and handles OTP cooldown /
`Retry-After`.
- `SRPJwtBearerAuth` exposes begin/complete enrollment and verification,
credential listing, and assertion-to-token exchange.
- Public `JwtBearerAuth` forwards those methods and rejects non-SRP auth
types.
- Nock fixtures and unit tests cover happy paths and error mapping.

Clients still go through `AuthenticationController` in later PRs; this
layer is not UI-facing.

## References

Depends on MetaMask#10264. Follow-ups: MetaMask#10266, MetaMask#10267. Stack: [stack
#10268](https://github.com/MetaMask/core/pull/10268).
Related to: https://consensyssoftware.atlassian.net/browse/MUL-2261

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **High Risk**
> Touches authentication, step-up tokens, and passkey/OTP handling;
mistakes could weaken session elevation or mishandle credentials, though
coverage is extensive.
> 
> **Overview**
> Adds **passkey and email OTP MFA** to the profile-sync JWT auth SDK: a
new HTTP layer calls `/api/v2/mfa/*` (enroll, complete, verify,
credentials), validates payloads, maps server codes to `MfaError`
subclasses, and handles OTP cooldown plus `Retry-After`.
> 
> **`SRPJwtBearerAuth`** and public **`JwtBearerAuth`** expose
enrollment/step-up flows, credential listing, and
**`exchangeMfaAssertion`** (AAL2 JWT → elevated access token via
existing OIDC). MFA is **SRP-only**; email enrollment uses a separate
`email` option from `entropySourceId`.
> 
> Schemas mark challenges, OTP codes, passkey payloads, and tokens with
**`sensitive()`**. Tests, nock fixtures, changelog, and SDK exports for
MFA types are included.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
56b3671. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant