Skip to content

feat(webapp): add MFA_REQUIRED env flag to enforce MFA for all dashboard users - #5034

Closed
MauryaQbit wants to merge 1 commit into
triggerdotdev:mainfrom
MauryaQbit:feat/mfa-required-webapp
Closed

MauryaQbit wants to merge 1 commit into
triggerdotdev:mainfrom
MauryaQbit:feat/mfa-required-webapp

Conversation

@MauryaQbit

@MauryaQbit MauryaQbit commented Oct 17, 2026 •

Copy link
Copy Markdown

Closes #5025

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

Automated

  • apps/webapp/test/mfaRequirement.test.ts — 14 unit tests, all passing. Covers the exempt-path list (including that prefixes of exempt paths are not exempt), the redirect and redirectTo propagation, sanitization of non-navigable targets, the flag-off path, the logged-out path, and throwIfMfaRequired.
  • pnpm run typecheck --filter webapp — 20/20 tasks pass.
  • pnpm run format (oxfmt) — clean.
  • pnpm run lint (oxlint) — exit 0.
  • pnpm run knip — exit 0.

Manual

Not done. My environment has no working container runtime (Could not find a working container runtime strategy), so I could not boot Postgres/Redis to drive the dashboard end to end, and the testcontainers-backed suites could not run locally either. Those will run in CI.


Changelog

Adds an opt-in MFA_REQUIRED webapp env var (default false) so self-hosted instances can require multi-factor authentication for every dashboard user.

Behaviour with the flag unset is unchanged — every check early-returns on a single boolean read.

Flag — MFA_REQUIRED: BoolEnv.default(false) in env.server.ts, alongside the other auth flags.

Enforcement — layered, because there is no single guard for all authenticated routes today:

  • requireUser (services/session.server.ts) covers /account/*, /invites, /admin/* and every other requireUser caller, including the _app layout.
  • The root.tsx loader covers routes that authenticate with requireUserId only (which stays cookie-only and has no user row to inspect). The root loader does not run for action-only resource routes, so those actions check separately.

Both call the same helper (services/mfa/mfaRequirement.server.ts), which no-ops unless the flag is on and the user is missing MFA. The auth flow (/login*, /magic, /auth/*), /logout, and the enrollment endpoint are exempt so the flow cannot deadlock.

Setup page — new /mfa/setup route with inline authenticator enrollment and recovery codes, no account menu, no way out. Already-enrolled users are forwarded on. The originally intended destination is carried in redirectTo and sanitized with the same helper the login flow uses.

Disabling blocked — at three layers: MultiFactorAuthenticationService.disableTotp throws ServiceValidationError (so every caller inherits the rule), the resource-route action refuses disable-mfa with a clean error, and MfaSetup hides the toggle and disable dialog when isRequired.

Incidental fix — enableTotp now removes a previous pending secret before creating a new one. Refreshing the forced-setup page previously orphaned the old SecretReference and its stored secret on every retry.

Reviewer notes

  • An admin impersonating a user without MFA will be bounced to setup. That's intentional — the requirement applies to the impersonated session — but worth flagging since it changes impersonation ergonomics when the flag is on.
  • /admin/api/* is not gated; those endpoints authenticate with API keys rather than dashboard sessions.

Docs — docs/self-hosting/env/webapp.mdx (new row in the Auth table) and .server-changes/mfa-required.md (release note).


Screenshots

Unavailable — no container runtime in my environment to boot the webapp (see Testing). Happy to attach screenshots of the /mfa/setup page if wanted.

💯

…ard users

Self-hosted instances under compliance programs (FedRAMP, SOC 2, HIPAA) could
not require a second factor — TOTP was opt-in per user and users could turn it
off again.

Adds an opt-in `MFA_REQUIRED` webapp env var (off by default). When set:

- A signed-in user without MFA is redirected to a new /mfa/setup page and every
  other dashboard request is refused until they enroll.
- MFA can no longer be disabled, enforced in the service, the resource route
  action and the UI.

The gate lives in `requireUser` (covers /account/*, /invites, /admin/* and
anything else calling it) and in the root loader (covers routes authenticating
with `requireUserId` only). The auth flow, /logout, /magic and the enrollment
endpoint are exempt so the flow cannot deadlock.

Closes triggerdotdev#5025

Co-Authored-By: OpenCode <noreply@opencode.ai>
@changeset-bot

changeset-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 6b0c6c1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

Copy link
Copy Markdown
Contributor

Hi @MauryaQbit, thanks for your interest in contributing!

This project requires that pull request authors are vouched, and you are not in the list of vouched users.

This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 60135c24-8450-4975-80b3-07735c8e2466
📥 Commits

Reviewing files that changed from the base of the PR and between f6238d0 and 6b0c6c1.

📒 Files selected for processing: 14
  • .server-changes/mfa-required.md
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/root.tsx
  • apps/webapp/app/routes/_app/route.tsx
  • apps/webapp/app/routes/account.security/route.tsx
  • apps/webapp/app/routes/mfa.setup/route.tsx
  • apps/webapp/app/routes/resources.account.mfa.setup/route.tsx
  • apps/webapp/app/routes/resources.account.mfa.setup/useMfaSetup.ts
  • apps/webapp/app/services/mfa/mfaRequirement.server.ts
  • apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts
  • apps/webapp/app/services/session.server.ts
  • apps/webapp/app/utils/pathBuilder.ts
  • apps/webapp/test/mfaRequirement.test.ts
  • docs/self-hosting/env/webapp.mdx
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

feat: self-hosted env flag to require MFA for every dashboard user

1 participant