Repository navigation
🔐 feat: Enforce Required Two-Factor Authentication - #14731
Conversation
danny-avila
changed the title
danny-avila
changed the title
berry-13
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d092a448
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f5271ef69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f06e0deb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8f06e0d to
8254002
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82540021d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
71f3478 to
421c7f1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 421c7f1919
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f90b71243
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e098a84c0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
danny-avila
added
🗺️ Two Factor Auth
e098a84 to
be87dfc
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be87dfc4b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3f7651a7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| user: StoredTwoFactorAccount, | ||
| userId: string, | ||
| ): Promise<void> => { | ||
| await deps.deleteAllUserSessions({ userId }); |
There was a problem hiding this comment.
When password recovery races challenge redemption or finalization and the post-mint retirement lookup throws, setAuthTokens has already queued refresh and CloudFront cookies before this cleanup runs. If deleteAllUserSessions also rejects, this helper exits before any cookies are cleared, and the outer catch sends a 500 carrying the newly minted post-cutoff credentials, which can refresh once the database recovers. Fresh evidence beyond the previously resolved cleanup thread is this fallible deletion-before-cookie-clear ordering; clear the client credentials in a finally path and ensure a failed deletion cannot leave the minted session usable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d9287b0: login, local refresh and enrollment now share one withdrawMintedSession that clears the refresh, provider and CDN cookies before awaiting the session deletion, so a deletion failure cannot leave the minted cookies on the 500. Covered in twoFactor.spec.ts (rejecting deletion still clears).
| userId, | ||
| process.env.JWT_SECRET as string, | ||
| ); | ||
| const retirement = await deps.getUserById(userId, TOKEN_RETIREMENT_FIELDS); |
There was a problem hiding this comment.
When TENANT_ISOLATION_STRICT=true, protected routes outside /api/auth reach this gate with only request-level context because tenantContextMiddleware is invoked later in the onAllowed callback. This getUserById therefore queries the tenant-isolated User model without either a tenant or system scope and throws, so an already-signed-in unenrolled user sees a 500 instead of the 403 setup handoff after enforcement is enabled. Establish the authenticated tenant context around the gate or run this retirement lookup in an explicitly appropriate scope.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e92f92: the gate's retirement read now runs in runAsSystem, matching the JWT strategy's own user read, so strict tenant isolation reaches the 403 hand-off instead of throwing. packages/api/src/auth/gates.spec.ts asserts the read's scope and fails without the change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9287b076a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d9287b0 to
70fe00b
Compare
Require local and LDAP accounts to enroll in TOTP before they can use the app when ENFORCE_TWO_FACTOR_AUTHENTICATION is set. Sign-in, refresh, bearer verification, optional share auth and image authorization all route an unenrolled account into a staged setup flow, and every bearer credential minted before enrollment or a password reset is retired. Password reset retirement uses the existing credentialsChangedAt stamp, and commitPasswordReset clears any staged enrollment in the same write. Passkey sign-in follows the same enrollment gate as password sign-in.
bfd6883 to
fb74790
Compare
Summary
Adds deployment-wide two-factor authentication enforcement through
ENFORCE_TWO_FACTOR_AUTHENTICATION=true.When enabled, local password and LDAP users without 2FA receive a short-lived, purpose-scoped setup token instead of a full authenticated session. Enrollment requires TOTP verification, backup-code download, one-time acknowledgement, and one-time finalization before session tokens are issued. Refresh sessions and protected API routes also fail closed while required enrollment is incomplete.
The startup configuration advertises the policy to the client. The required setup flow preserves deep links across reload and finalization. Account settings keep the standard
Disable 2FAbutton visible, disable it with a not-allowed cursor, and showRequired by administratoron hover or keyboard focus. Federated identity providers remain responsible for their own MFA policies.The enrollment state transitions are atomic, one-time credentials cannot be replayed, sensitive enrollment fields are excluded from user responses and caches, and user-document authentication caches are invalidated after promotion.
Closes #14725
Screenshots
Required enrollment (dark): local password and LDAP users without 2FA land here instead of a full session.
Account settings (dark):
Disable 2FAstays visible, uses a not-allowed cursor, and showsRequired by administratoron hover or keyboard focus.Change Type
Testing
git diff --name-only -z 9bb599435f399a573b5284c91f7c2caadeef0db9..HEAD -- '*.js' '*.jsx' '*.ts' '*.tsx' | xargs -0 npx eslintgit diff --check 9bb599435f399a573b5284c91f7c2caadeef0db9..HEAD(cd packages/data-provider && npm run build)(cd packages/data-schemas && npm run build)(cd packages/api && npm run build)(cd client && npm run build)(cd client && npm run typecheck)(cd packages/api && npx tsc --noEmit)(cd packages/api && npx jest src/auth/twoFactor.spec.ts --runInBand --silent)(54 tests)(cd packages/data-schemas && npx jest src/methods/user.methods.spec.ts --runInBand --coverage=false --silent)(76 tests)(cd api && npx jest server/controllers/AuthController.spec.js server/controllers/__tests__/TwoFactorController.spec.js server/controllers/auth/LoginController.spec.js server/controllers/auth/TwoFactorAuthController.spec.js server/middleware/__tests__/requireJwtAuth.spec.js server/middleware/limiters/twoFactorTempLimiter.test.js server/routes/admin/auth.refresh.test.js server/routes/auth.2fa-ratelimit.test.js server/routes/auth.cloudfront.test.js server/routes/auth.reset-password-ratelimit.test.js server/routes/__tests__/config.spec.js --runInBand --detectOpenHandles --silent)(208 tests)(cd client && JEST_JUNIT_OUTPUT_DIR=/tmp JEST_JUNIT_OUTPUT_NAME=final-2fa-client.xml npx jest src/components/Nav/Settings/__tests__ src/components/Nav/SettingsTabs/Account src/components/Auth/__tests__/TwoFactorSetupScreen.spec.tsx src/hooks/__tests__/AuthContext.spec.tsx src/routes/__tests__/StartupLayout.spec.tsx src/utils/__tests__/redirect.test.ts --runInBand --coverage=false --silent)(128 tests)CI=1 npx playwright test --config=e2e/playwright.config.mock.ts e2e/specs/mock/two-factor-required.spec.ts --repeat-each=3(3 passes)Test Configuration:
Checklist