feat(mfa): require a second factor to change or reset a password (NAN-6586)

[agusayerza]

NAN-6586

MFA gates every sign-in path but neither password flow asked for it. Kelvin caught this on Aug 7.

putUserPassword only checked the old password, so a stolen session could rotate it without the second factor coming up. putResetPassword was worse: it trusted the emailed token alone, so mailbox access was enough to take an account over, which is exactly what MFA is supposed to stop.

Both endpoints now accept an optional mfa credential and require it when the user has an active factor. A TOTP code or a recovery code works, same as the login challenge. Users with nothing enrolled see no change, and the account-level feature flag still short-circuits the whole thing.

Three details worth a look:

The factor is verified after the old password check, so a wrong password returns before we touch the code and cannot burn a TOTP step or a recovery code.

It is also verified inside the same transaction that writes the password. A one-time credential is spent there, so it has to roll back with the action rather than outlive a failed one. verifyTotp and consumeRecoveryCode take an optional parent transaction for this, and verifyStepUpMfa requires one so a future call site cannot quietly reintroduce the gap.

verifyStepUpMfa in account/mfa/stepUp.ts is the shared helper, and its credential schema replaces the duplicate that was inline in the MFA controller. It takes a DBUser rather than reading res.locals, because the reset flow has no session to read from.

I did not add rateLimiterMiddleware to PUT /user/password. It now accepts codes, but an attacker needs a valid session and the current password before the code matters, so it did not seem worth the behaviour change. Reset-password is already rate limited. Say the word if you want it anyway.

Out of scope: the webapp fields to collect the code. Worth filing once you are happy with this API shape.

Test plan

• npm run ts-build
• npm run test:integration -- packages/shared/lib/services/mfa.service.integration.test.ts packages/server/lib/controllers/v1/account/ packages/server/lib/controllers/v1/user/ packages/server/lib/middleware/ (17 files, 98 passed)
• Manual check once the webapp sends the field

[linear-code]

NAN-6586

[cubic-dev-ai]

All reported issues were addressed across 12 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking defects identified in the changed backend paths.

The MFA requirement is rechecked before mutation, one-time credential consumption shares the password transaction, all affected persistence operations honor that transaction, and the endpoint contracts and integration tests cover the new outcomes.

Important Files Changed

Filename	Overview
packages/server/lib/controllers/v1/account/mfa/stepUp.ts	Adds a shared MFA credential validator and transaction-required step-up verifier that rechecks current enrollment state.
packages/server/lib/controllers/v1/account/putResetPassword.ts	Gates password resets with MFA when applicable and atomically consumes the credential, updates the password, clears the reset token, and deletes sessions.
packages/server/lib/controllers/v1/user/password/putPassword.ts	Adds MFA step-up to authenticated password changes after current-password verification and within the password-write transaction.
packages/shared/lib/services/mfa.service.ts	Allows TOTP and recovery-code consumption to participate in a caller-provided transaction while preserving standalone behavior.
packages/types/lib/mfa/credential.ts	Centralizes the public discriminated union for TOTP and recovery-code credentials.
packages/server/lib/controllers/v1/account/putResetPassword.integration.test.ts	Covers reset MFA enforcement, valid credentials, invalid tokens, feature-flag behavior, and transactional rollback.
packages/server/lib/controllers/v1/user/password/putPassword.integration.test.ts	Covers password-change MFA enforcement, credential variants, password-first validation, feature-flag behavior, and rollback.

Sequence Diagram

sequenceDiagram
    participant Client
    participant PasswordEndpoint
    participant StepUpMFA
    participant MFAStore
    participant UserStore
    participant Sessions

    Client->>PasswordEndpoint: Password request + optional MFA credential
    PasswordEndpoint->>PasswordEndpoint: Validate token/current password
    PasswordEndpoint->>StepUpMFA: Check whether MFA is required
    alt MFA required and credential missing
        StepUpMFA-->>PasswordEndpoint: required
        PasswordEndpoint-->>Client: mfa_code_required
    else Credential supplied or MFA not required
        PasswordEndpoint->>StepUpMFA: Verify inside transaction
        StepUpMFA->>MFAStore: Lock factor and consume credential
        alt Invalid credential
            StepUpMFA-->>PasswordEndpoint: invalid
            PasswordEndpoint-->>Client: invalid_mfa_code
        else Verified or not required
            PasswordEndpoint->>UserStore: Update password/reset token
            PasswordEndpoint->>Sessions: Delete existing sessions
            PasswordEndpoint-->>Client: Success
        end
    end

Loading

_{Reviews (1): Last reviewed commit: "perf(mfa): reject a missing second facto..." | Re-trigger Greptile}