diff --git a/.changeset/30201-org-mfa-needs-enrollment.md b/.changeset/30201-org-mfa-needs-enrollment.md new file mode 100644 index 00000000000..32d9db0c0e7 --- /dev/null +++ b/.changeset/30201-org-mfa-needs-enrollment.md @@ -0,0 +1,21 @@ +--- +'@objectstack/plugin-auth': patch +--- + +fix(plugin-auth)!: an organization cannot require multi-factor authentication while two-factor enrollment is off, and the MFA gate never holds a user who has no way to enroll (#30201) + +Clause-②: no (narrowing) — while the two-factor feature is off, a write that turns `sys_organization.require_mfa` on (an insert with it on, or an update of a row that stores it off) is refused `400 VALIDATION_FAILED`, a value an administrator could save before; re-saving a row that already stores it on passes. No published export, error code or settings key is added; nothing is widened. +Metadata-protocol: none +Skills-docs: follow-up owed — content/docs/permissions/authentication.mdx (the `mfa_required` row says `require_mfa` is settable per organization with no condition; it needs two-factor on) + + + +**BREAKING** accept-set narrowing on `sys_organization.require_mfa` writes, served by every data door (the organization's edit form, REST, MCP), on a deployment where two-factor is off. + +- **Before.** An administrator could tick Require Multi-Factor Auth on an organization while two-factor was off (the default, `features.twoFactor: false`), and it saved. Once the grace period ended, every member, the owner included, was held `403 MFA_REQUIRED` and sent to enroll, and the enrollment endpoint (`POST /api/v1/auth/two-factor/enable`) answered 404: the twoFactor plugin that serves it was never turned on. Nobody could undo it from inside the product, because Setup sits behind the same gate. The global switch never had this hole: the `auth.mfa_required` setting turns the requirement and the plugin on as one piece. +- **Now.** While two-factor is off, a write that turns `require_mfa` on is refused: an insert with it on, or an update of a row whose stored value is off. The refusal carries one `fields[]` entry on `require_mfa` (`invalid_value`) saying why, who would have been locked out, and the ways forward. It applies to every write context, the system's included, on the by-id door, an insert, and a multi-row update, which is judged row by row (one row turning it on refuses the whole write, with nothing written). Re-sending `require_mfa: true` for a row that already stores it (a whole-row update, an import re-run, a save with no change) passes, so other edits of that row are not blocked. Clearing the column and writing other columns pass as before, and every write passes once two-factor is on (read per write, so no restart is needed). +- **The gate.** ADR-0069 D3's MFA gate holds a user only while they can enroll. With two-factor off, an MFA requirement (an organization's `require_mfa`, or `auth.mfa_required` under an `OS_AUTH_TWO_FACTOR=false` pin, or a host that sets `mfaRequired` without `plugins.twoFactor`) holds nobody and starts no grace clock, and the server says so once in a warning with the remedy. An organization row stored with `require_mfa = true` before this release therefore locks no one out; it starts holding unenrolled members once two-factor is turned on. +- **One answer.** Whether two-factor is on is decided in one place, the same `OS_AUTH_TWO_FACTOR` env, then `plugins.twoFactor`, then off, for the plugin list, `features.twoFactor` in `/api/v1/auth/config`, the gate and the write refusal. +- **Unchanged.** An enrolled user is never held. With two-factor on, the requirement and its grace period behave exactly as before. The `auth.mfa_required` setting still turns two-factor on with the requirement. + +**The remedy.** To require multi-factor authentication for one organization, turn two-factor enrollment on for the deployment first, with `OS_AUTH_TWO_FACTOR=true`, and then tick the organization's box. Unless `OS_AUTH_TWO_FACTOR` is set to `false` (the env var outranks the settings), turning on the `auth.mfa_required` setting instead also works: it requires multi-factor authentication for every organization and turns enrollment on with it. diff --git a/packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts b/packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts index 93a97d4a2ae..6c5f9f92a82 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts @@ -62,7 +62,9 @@ export interface PasswordSessionPolicyHost { _orgMfaCache: AuthManager['_orgMfaCache']; _orgMfaRefreshing: AuthManager['_orgMfaRefreshing']; getDataEngine: AuthManager['getDataEngine']; + isTwoFactorEnabled: AuthManager['isTwoFactorEnabled']; logDurabilityDegradation: AuthManager['logDurabilityDegradation']; + noteMfaRequirementWithoutEnrollment: AuthManager['noteMfaRequirementWithoutEnrollment']; parseHashes: AuthManager['parseHashes']; refreshMustChangeCacheIfStale: AuthManager['refreshMustChangeCacheIfStale']; refreshOrgMfaCacheIfStale: AuthManager['refreshOrgMfaCacheIfStale']; @@ -136,7 +138,7 @@ export function refreshOrgMfaCacheIfStale(this: Pick, + this: Pick, userId: string, options: { organizationId?: string } = {}, ): Promise { @@ -205,7 +207,21 @@ export async function resolveUserAuthGates( // A user without TOTP enrolled is blocked once their grace window // elapses. The clock (`mfa_required_at`) starts the first time we see // them required-but-unenrolled (stamped lazily, best-effort). - if (mfaRequired && !(u?.two_factor_enabled === true || u?.two_factor_enabled === 1)) { + // + // [#30201] Only while they CAN enroll. The twoFactor plugin's + // `/two-factor/*` endpoints are the one enrollment path; with the plugin + // off, a hold here answers `MFA_REQUIRED` to a user whose remediation + // endpoint does not exist, which locks every member of the organization + // out, the owner included, behind the very gate that guards Setup. So an + // unenrollable requirement is not enforced (no hold, and no grace clock + // started: the window opens when enrollment does) and is reported once. + // The `require_mfa` write guard refuses a new requirement in that state + // (`organization-mfa-enrollment-guard.ts`); this covers rows stored before + // it, and an `OS_AUTH_TWO_FACTOR=false` pin under the global setting. + const unenrolled = !(u?.two_factor_enabled === true || u?.two_factor_enabled === 1); + if (mfaRequired && unenrolled && !this.isTwoFactorEnabled()) { + this.noteMfaRequirementWithoutEnrollment(); + } else if (mfaRequired && unenrolled) { const graceDays = Math.max(0, Math.floor(Number(this.config.mfaGracePeriodDays ?? 7))); let requiredAt = u?.mfa_required_at; if (!requiredAt) { diff --git a/packages/plugins/plugin-auth/src/auth-manager-plugin-list.ts b/packages/plugins/plugin-auth/src/auth-manager-plugin-list.ts index 811bf7cbc61..b83a3e4a164 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-plugin-list.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-plugin-list.ts @@ -71,6 +71,7 @@ import { resolveDcrEnabled, resolveOidcProviderEnabled, resolveScimEnabled, + resolveTwoFactorEnabled, type AuthManager, } from './auth-manager.js'; import { @@ -188,11 +189,13 @@ export async function buildPluginList(this: PluginListHost): Promise { // better-auth build would otherwise mount SCIM with its ADR-0134 // deprovisioning path silently declined. assertScimAdminCoherence(pluginConfig); - const twoFactorFromEnv = readBooleanEnv('OS_AUTH_TWO_FACTOR'); const hibpFromEnv = readBooleanEnv('OS_AUTH_PASSWORD_REJECT_BREACHED'); const enabled = { organization: pluginConfig.organization ?? true, - twoFactor: twoFactorFromEnv ?? pluginConfig.twoFactor ?? false, + // [#30201] Shared decision point (env → config), see + // resolveTwoFactorEnabled — the MFA gate and the require_mfa write guard + // ask the same question, so a requirement is never enforced unenrollable. + twoFactor: resolveTwoFactorEnabled(pluginConfig), passwordRejectBreached: hibpFromEnv ?? pluginConfig.passwordRejectBreached ?? false, passkeys: pluginConfig.passkeys ?? false, magicLink: pluginConfig.magicLink ?? false, diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index 430002245a6..bf3a6b03f97 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -3876,7 +3876,8 @@ describe('AuthManager', () => { const IMPERSONATION = { impersonatedBy: 'u-admin' }; // Every posture computable at once: enforced MFA with no grace, and a // one-day expiry. Each row below decides which postures its user holds. - const POLICY = { mfaRequired: true, mfaGracePeriodDays: 0, passwordExpiryDays: 1 }; + // [#30201] Two-factor on: the gate holds a user for MFA only while they can enroll. + const POLICY = { mfaRequired: true, mfaGracePeriodDays: 0, passwordExpiryDays: 1, plugins: { twoFactor: true } }; const sessionGateFor = async (row: Record, session: Record) => { const engine = makeDataEngine({ platformAdmin: false }); // The block's double, its `findOne` answering the posture read of the user row. @@ -4695,9 +4696,13 @@ describe('AuthManager', () => { }), update: vi.fn(async () => ({})), }); + // [#30201] Two-factor on by default here: the gate holds a user for MFA only + // while they can enroll. The cases that turn it off say so (`plugins: {}`). const mgr = (engine: any, extra: any = {}) => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const m = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine, ...extra }); + const m = new AuthManager({ + secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine, plugins: { twoFactor: true }, ...extra, + }); warn.mockRestore(); return m; }; @@ -4749,6 +4754,63 @@ describe('AuthManager', () => { const g = await (m as any).computeAuthGate('u1', undefined, false); expect(g?.code).toBe('MFA_REQUIRED'); }); + + // [#30201] The twoFactor plugin's endpoints are the only enrollment path. + // A hold while it is off answers MFA_REQUIRED to a user whose remediation + // endpoint answers 404 — the whole-tenant lockout the card measured. + describe('only a user who can enroll is held (#30201)', () => { + const OLD = () => new Date(Date.now() - 30 * 86_400_000).toISOString(); + const logger = () => ({ warn: vi.fn(), info: vi.fn() }); + + it('two-factor off: no hold and no grace clock started, said once with the remedy', async () => { + const log = logger(); + const lapsed = makeEngine({ id: 'u1', two_factor_enabled: false, mfa_required_at: OLD() }); + const m = mgr(lapsed, { mfaRequired: true, mfaGracePeriodDays: 0, plugins: {}, logger: log }); + expect(m.isTwoFactorEnabled()).toBe(false); + await expect((m as any).computeAuthGate('u1', undefined, false)).resolves.toBeUndefined(); + expect(await m.resolveUserAuthGates('u1')).toEqual([]); + const fresh = makeEngine({ id: 'u2', two_factor_enabled: false, mfa_required_at: null }); + const m2 = mgr(fresh, { mfaRequired: true, plugins: {}, logger: log }); + expect(await m2.resolveUserAuthGates('u2')).toEqual([]); + // The window opens when enrollment does, not before. + expect(fresh.update).not.toHaveBeenCalled(); + // Once per manager, however many requests ask. + expect(log.warn).toHaveBeenCalledTimes(2); + expect(String(log.warn.mock.calls[0][0])).toMatch(/NOT enforced/); + expect(String(log.warn.mock.calls[0][0])).toMatch(/OS_AUTH_TWO_FACTOR=true/); + // The settings remedy is conditional: the env var outranks it. + expect(String(log.warn.mock.calls[0][0])).toMatch(/unless OS_AUTH_TWO_FACTOR is set to false/); + }); + + it('an OS_AUTH_TWO_FACTOR=false pin outranks plugins.twoFactor, so the requirement is not enforced', async () => { + const prior = process.env.OS_AUTH_TWO_FACTOR; + process.env.OS_AUTH_TWO_FACTOR = 'false'; + try { + const m = mgr(makeEngine({ id: 'u1', two_factor_enabled: false, mfa_required_at: OLD() }), { + mfaRequired: true, mfaGracePeriodDays: 0, logger: logger(), + }); + expect(m.isTwoFactorEnabled()).toBe(false); + // The one answer: what is wired, what is advertised, what the gate asks. + expect(m.getPublicConfig().features.twoFactor).toBe(false); + expect(await m.resolveUserAuthGates('u1')).toEqual([]); + } finally { + if (prior === undefined) delete process.env.OS_AUTH_TWO_FACTOR; + else process.env.OS_AUTH_TWO_FACTOR = prior; + } + }); + + it('control: the same user is held once two-factor is on, and an enrolled user never is', async () => { + const m = mgr(makeEngine({ id: 'u1', two_factor_enabled: false, mfa_required_at: OLD() }), { + mfaRequired: true, mfaGracePeriodDays: 0, + }); + expect(m.isTwoFactorEnabled()).toBe(true); + expect((await m.resolveUserAuthGates('u1')).map((g) => g.code)).toEqual(['MFA_REQUIRED']); + const enrolled = mgr(makeEngine({ id: 'u1', two_factor_enabled: true, mfa_required_at: OLD() }), { + mfaRequired: true, mfaGracePeriodDays: 0, plugins: {}, + }); + expect(await enrolled.resolveUserAuthGates('u1')).toEqual([]); + }); + }); }); // ADR-0069 D3 — per-org MFA tightening (an org may require MFA above the @@ -4766,9 +4828,12 @@ describe('AuthManager', () => { update: vi.fn(async () => ({})), count: vi.fn(async () => 1), }); + // [#30201] Two-factor on by default, as in the block above. const mgr = (engine: any, extra: any = {}) => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const m = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine, ...extra }); + const m = new AuthManager({ + secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine, plugins: { twoFactor: true }, ...extra, + }); warn.mockRestore(); return m; }; @@ -4803,6 +4868,17 @@ describe('AuthManager', () => { await expect((m as any).computeAuthGate('u1', 'org1', false)).resolves.toBeUndefined(); }); + it('[#30201] does NOT block for the active org\'s require_mfa while two-factor is off, nor start the clock', async () => { + const engine = makeEngine( + { id: 'u1', two_factor_enabled: false, mfa_required_at: null }, + { id: 'org1', require_mfa: true }, + ); + const m = mgr(engine, { mfaRequired: false, mfaGracePeriodDays: 0, plugins: {}, logger: { warn: vi.fn() } }); + (m as any)._orgMfaCache = { value: true, at: Date.now() }; + await expect((m as any).computeAuthGate('u1', 'org1', false)).resolves.toBeUndefined(); + expect(engine.update).not.toHaveBeenCalled(); + }); + it('is a no-op when no org requires MFA and there is no active org', async () => { const engine = makeEngine({ id: 'u1', two_factor_enabled: false }, null); const m = mgr(engine, { mfaRequired: false }); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 7c46b7cb160..844e0dfa47a 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -432,6 +432,23 @@ export function resolveScimEnabled(pluginConfig?: Partial): bo return pluginConfig?.scim ?? readBooleanEnv('OS_SCIM_ENABLED') ?? false; } +/** + * [#30201] SINGLE decision point for "can a user enroll a second factor + * here?" — whether better-auth's `twoFactor` plugin is wired, whose + * `/two-factor/*` endpoints are the only enrollment path. Shared by + * `buildPluginList()` (what is wired), the `/auth/config` features block (what + * is advertised), the per-user MFA gate (ADR-0069 D3 holds a user only while + * they can enroll) and the `sys_organization.require_mfa` write guard + * (`organization-mfa-enrollment-guard.ts`), so the four can never disagree. + * + * Resolution order: `OS_AUTH_TWO_FACTOR` env (operator override, wins) → + * config (`plugins.twoFactor`, which the `auth.mfa_required` setting turns on + * together with the requirement) → off. + */ +export function resolveTwoFactorEnabled(pluginConfig?: Partial): boolean { + return readBooleanEnv('OS_AUTH_TWO_FACTOR') ?? pluginConfig?.twoFactor ?? false; +} + /** * Entry validation (#13816, maintainer ruling 2026-09-01) — REFUSES the one * incoherent corner of the `admin` tri-state at the point the config enters @@ -1547,6 +1564,10 @@ export class AuthManager { // the node that issues a flag (other nodes catch up within the TTL). private _mustChangeCache: { value: boolean; at: number } = { value: false, at: 0 }; private _mustChangeRefreshing = false; + // [#30201] Set the first time the MFA gate meets a requirement it does not + // enforce because no one can enroll (see `noteMfaRequirementWithoutEnrollment`), + // so the warning is said once per manager, not once per request. + private _mfaWithoutEnrollmentNoted = false; // Optional auth features whose better-auth plugin failed to initialize and // was skipped so core auth stays up (feature key → error message). Rebuilt // by every buildPluginList() run; see addOptionalPlugin(). @@ -4584,7 +4605,6 @@ export class AuthManager { const termsUrl = resolveLegalUrl(rawTermsUrl, DEFAULT_TERMS_URL); const privacyUrl = resolveLegalUrl(rawPrivacyUrl, DEFAULT_PRIVACY_URL); - const twoFactorFromEnv = readBooleanEnv('OS_AUTH_TWO_FACTOR'); const turnstileKeys = resolveTurnstileKeys(); // [#7481, maintainer ruling 2026-08-11] `passkeys` / `magicLink` are @@ -4595,7 +4615,7 @@ export class AuthManager { // `magicLink`'s server endpoints are unaffected: `plugins.magicLink` still // wires better-auth's magic-link plugin in buildPluginList(). const features = { - twoFactor: twoFactorFromEnv ?? pluginConfig.twoFactor ?? false, + twoFactor: resolveTwoFactorEnabled(pluginConfig), organization: pluginConfig.organization ?? true, multiOrgEnabled, // ADR-0105 D1 — WHICH posture is in force. `multiOrgEnabled` stays the @@ -5233,6 +5253,41 @@ export class AuthManager { return ((this.config.plugins ?? {}) as any).phoneNumber === true; } + /** + * @internal Package-internal: read by the MFA gate and by the organization + * MFA enrollment guard `AuthPlugin` registers; not a supported entry point. + * + * [#30201] Can a user enroll a second factor on this deployment right now — + * is the twoFactor plugin wired? {@link resolveTwoFactorEnabled} over the + * live config, the one answer `buildPluginList()` and `features.twoFactor` + * give too. Read per call, so a settings pass that turns the plugin on is + * seen at once. + */ + public isTwoFactorEnabled(): boolean { + return resolveTwoFactorEnabled(this.config.plugins); + } + + /** + * [#30201] Called by the MFA gate when a user is under an MFA requirement + * (the global `mfaRequired` or their organization's `require_mfa`) that it + * does not enforce, because the twoFactor plugin is off and the user has no + * way to enroll. A functional degradation (AGENTS.md, degradation log + * levels): the requirement is visibly not in force, nothing stored is lost. + * Said once per manager, with the remedy. + */ + private noteMfaRequirementWithoutEnrollment(): void { + if (this._mfaWithoutEnrollmentNoted) return; + this._mfaWithoutEnrollmentNoted = true; + this.config.logger?.warn( + '[auth] multi-factor authentication is required (the auth.mfa_required setting, or an organization\'s ' + + 'require_mfa) but is NOT enforced: the two-factor feature is off, so nobody can enroll an authenticator ' + + 'app, and holding them would lock every affected member out, the owner included. Remedy: set ' + + 'OS_AUTH_TWO_FACTOR=true; or, unless OS_AUTH_TWO_FACTOR is set to false (it overrides the settings), turn ' + + 'on the auth.mfa_required setting, which turns two-factor on with the requirement; or clear require_mfa on ' + + 'the organization.', + ); + } + /** * #2766 V2 — is an email transport wired? The identity import endpoint * rejects the invite password policy up front when it isn't, instead of @@ -5441,11 +5496,13 @@ export class AuthManager { const readMustChangeCache = () => this._mustChangeCache; const readOrgMfaCache = () => this._orgMfaCache; const readConfig = () => this.config; - const host: Pick = { + const host: Pick = { get _mustChangeCache() { return readMustChangeCache(); }, get _orgMfaCache() { return readOrgMfaCache(); }, get config() { return readConfig(); }, getDataEngine: (...a) => this.getDataEngine(...a), + isTwoFactorEnabled: (...a) => this.isTwoFactorEnabled(...a), + noteMfaRequirementWithoutEnrollment: (...a) => this.noteMfaRequirementWithoutEnrollment(...a), refreshMustChangeCacheIfStale: (...a) => this.refreshMustChangeCacheIfStale(...a), refreshOrgMfaCacheIfStale: (...a) => this.refreshOrgMfaCacheIfStale(...a), }; diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 1a10724e0d2..948f1a53186 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -68,6 +68,7 @@ import { } from './identity-write-guard.js'; import { registerLastAdminGuard } from './last-admin-guard.js'; import { registerApiKeyRestoreGuard } from './api-key-restore-guard.js'; +import { registerOrganizationMfaEnrollmentGuard } from './organization-mfa-enrollment-guard.js'; import { registerPersonalViewHooks } from './personal-view-hooks.js'; import { registerUserPreferenceHooks } from './user-preference-hooks.js'; import { personalViewCallerResolver, registerPersonalViewRoutes, PERSONAL_VIEW_REMOVE_ROUTE, type PersonalViewEngine } from './personal-view-routes.js'; @@ -1878,6 +1879,14 @@ export class AuthPlugin implements Plugin { packageId: 'com.objectstack.plugin-auth.api-key-restore-guard', logger: ctx.logger, }); + // [#30201] ADR-0069 D3 — an organization's `require_mfa` is refused + // while nobody can enroll a second factor, the pairing the global + // `auth.mfa_required` switch gets from its settings piece below. Asks + // the live manager per write. See organization-mfa-enrollment-guard.ts. + registerOrganizationMfaEnrollmentGuard(engine, { + packageId: 'com.objectstack.plugin-auth.organization-mfa-enrollment-guard', + isTwoFactorEnabled: () => this.authManager?.isTwoFactorEnabled() ?? false, + }); // A reader's own named list views: the body is judged on every save as // a ListView for the row's own object. See personal-view-hooks.ts. registerPersonalViewHooks(engine, { diff --git a/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.test.ts b/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.test.ts new file mode 100644 index 00000000000..8b68b6de5be --- /dev/null +++ b/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.test.ts @@ -0,0 +1,429 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * An organization's MFA requirement never strands its members (ADR-0069 D3). + * + * Before this guard an administrator could tick `require_mfa` on an + * organization while the two-factor feature was off. The save went through, + * and once the grace period ended every member, the owner included, was held + * with `MFA_REQUIRED` and sent to an enrollment endpoint that answered 404. + * Three blocks hold the two halves of the fix and their conjunction: + * + * 1. the guard's rule over its own hook (which payloads it refuses, with + * what envelope, on which object); + * 2. the guard on a real ObjectQL engine over the real `sys_organization` + * object, composed behind the ADR-0092 D2 identity write guard the way + * `auth-plugin.ts` composes it: the by-id door an administrator's edit + * form uses, a multi-row update, a system insert, and the row left as it + * was after a refusal; + * 3. end to end on the real better-auth pipeline: for a member of an + * organization that requires MFA, past the grace window, the session + * never carries `MFA_REQUIRED` while `/two-factor/enable` answers 404 — + * with two-factor off the session is not held, and with it on the + * session is held AND the enrollment endpoint answers. + */ + +import { describe, it, expect, afterAll, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { PluginContext } from '@objectstack/core'; +import { SysOrganization } from '@objectstack/platform-objects/identity'; +import { AuthManager } from './auth-manager'; +import { AuthPlugin } from './auth-plugin.js'; +import { + ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY, + organizationMfaWithoutEnrollmentRefusal, + registerOrganizationMfaEnrollmentGuard, + turnsMfaRequirementOn, +} from './organization-mfa-enrollment-guard.js'; +import { registerIdentityWriteGuard, registerManagedUpdateWhitelist } from './identity-write-guard.js'; +import { managedExtensionEditableFields } from './managed-extension-fields.js'; +// The SAME in-memory engine the sibling two-factor suites drive: a new fake +// would be a new `check:engine-double-contract` ledger entry for no fidelity. +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; + +/** ADR-0112 / ADR-0114: the refusal is the per-field `VALIDATION_FAILED` envelope on `require_mfa`. */ +function expectRequireMfaRefusal(err: any): void { + expect(err, 'the write was admitted').toBeTruthy(); + // `mapDataError` in `@objectstack/rest` keys the 400 on exactly these two + // discriminators, so pinning both is what pins the status. + expect(err.code).toBe('VALIDATION_FAILED'); + expect(err.name).toBe('ValidationError'); + expect(err.fields).toEqual([ + expect.objectContaining({ field: 'require_mfa', code: 'invalid_value' }), + ]); +} + +// ── 1. The rule ───────────────────────────────────────────────────────────── + +/** Fake engine capturing registrations, the shape the real engine builds. */ +function captureEngine() { + const registrations: Array<{ event: string; handler: (ctx: any) => Promise; options: any }> = []; + return { + registrations, + registerHook: (event: string, handler: (ctx: any) => Promise, options?: any) => { + registrations.push({ event, handler, options }); + }, + }; +} + +/** + * Run every registered handler for `event` over a payload, with the stored row + * as `previous` when given (the engine binds it on every update path); returns + * the thrown error, or null. + */ +async function judge( + enrollable: boolean, + event: 'beforeInsert' | 'beforeUpdate', + data: Record, + previous?: Record, +): Promise { + const engine = captureEngine(); + registerOrganizationMfaEnrollmentGuard(engine, { + packageId: 'test.organization-mfa-enrollment-guard', + isTwoFactorEnabled: () => enrollable, + }); + try { + for (const r of engine.registrations.filter((x) => x.event === event)) { + await r.handler({ + object: 'sys_organization', event, input: { id: 'org_1', data }, + ...(previous ? { previous } : {}), + }); + } + return null; + } catch (e) { + return e; + } +} + +describe('organization MFA enrollment guard — the rule', () => { + it('registers insert and update hooks on sys_organization alone, after the D2 identity write guard', () => { + const engine = captureEngine(); + registerOrganizationMfaEnrollmentGuard(engine, { packageId: 'p', isTwoFactorEnabled: () => false }); + expect(engine.registrations.map((r) => r.event).sort()).toEqual(['beforeInsert', 'beforeUpdate']); + for (const r of engine.registrations) { + expect(r.options).toEqual({ object: 'sys_organization', priority: 15, packageId: 'p' }); + } + // D2 runs at 10: a write it refuses is refused there first. + expect(ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY).toBeGreaterThan(10); + }); + + it.each([true, 1, 'true', 'TRUE', '1', 'yes'])('judges %j as turning the requirement on', (value) => { + expect(turnsMfaRequirementOn(value)).toBe(true); + }); + + it.each([false, 0, '0', 'false', ' False ', null, undefined])('judges %j as leaving it off', (value) => { + expect(turnsMfaRequirementOn(value)).toBe(false); + }); + + it('refuses require_mfa turned on, on insert and on update, while two-factor is off', async () => { + for (const event of ['beforeInsert', 'beforeUpdate'] as const) { + expectRequireMfaRefusal(await judge(false, event, { require_mfa: true })); + expectRequireMfaRefusal(await judge(false, event, { require_mfa: true, timezone: 'Asia/Shanghai' })); + } + }); + + it('admits the same write once two-factor is on (control: the refusal is the enrollment state\'s)', async () => { + for (const event of ['beforeInsert', 'beforeUpdate'] as const) { + expect(await judge(true, event, { require_mfa: true })).toBeNull(); + } + }); + + it('refuses an update that turns it on for a row stored off, and one with no stored row to prove otherwise', async () => { + expectRequireMfaRefusal(await judge(false, 'beforeUpdate', { require_mfa: true }, { require_mfa: false })); + expectRequireMfaRefusal(await judge(false, 'beforeUpdate', { require_mfa: true }, { require_mfa: null })); + expectRequireMfaRefusal(await judge(false, 'beforeUpdate', { require_mfa: true }, { name: 'Acme' })); + // No `previous` at all: an unproven prior state is judged as a change. + expectRequireMfaRefusal(await judge(false, 'beforeUpdate', { require_mfa: true })); + }); + + it('admits re-sending it for a row that already stores it on: the write turns nothing on', async () => { + // A whole-row SDK/MCP update, an import re-run, a save with no change. + expect(await judge(false, 'beforeUpdate', { require_mfa: true }, { require_mfa: true })).toBeNull(); + expect(await judge(false, 'beforeUpdate', { require_mfa: 1, name: 'Acme' }, { require_mfa: 1 })).toBeNull(); + // An insert has no stored value to keep: a `previous` there changes nothing. + expectRequireMfaRefusal(await judge(false, 'beforeInsert', { require_mfa: true }, { require_mfa: true })); + }); + + it('never judges a write that clears the requirement or leaves it alone', async () => { + for (const event of ['beforeInsert', 'beforeUpdate'] as const) { + expect(await judge(false, event, { require_mfa: false })).toBeNull(); + expect(await judge(false, event, { timezone: 'Asia/Shanghai' })).toBeNull(); + expect(await judge(false, event, { name: 'Acme' })).toBeNull(); + } + }); + + it('says why and what to do, with no tracker number', () => { + const err: any = organizationMfaWithoutEnrollmentRefusal(); + expectRequireMfaRefusal(err); + // The wording is not a parsed contract; these are the facts the administrator + // needs, asserted by their named subjects rather than by the sentence. + expect(err.message).toMatch(/two-factor enrollment is off/); + expect(err.message).toMatch(/owner included/); + expect(err.message).toMatch(/OS_AUTH_TWO_FACTOR=true/); + expect(err.message).toMatch(/auth\.mfa_required/); + // The env var outranks the settings: the settings remedy is offered only where it can work. + expect(err.message).toMatch(/Unless OS_AUTH_TWO_FACTOR is set to false/); + expect(err.message).not.toMatch(/#\d/); + expect(err.fields[0].message).toBe(err.message); + }); +}); + +describe('auth-plugin registers the guard on `kernel:ready` and asks the live manager', () => { + it('both hooks on sys_organization, after the D2 guard, judging the manager\'s two-factor state per write', async () => { + const registered: Array<{ event: string; handler: (ctx: any) => Promise; options: any }> = []; + const engine = { + registerHook: (event: string, handler: (ctx: any) => Promise, options: any) => { + registered.push({ event, handler, options }); + }, + getSchema: () => undefined, + }; + const ready: Array<() => Promise> = []; + const ctx = { + registerService: vi.fn(), + getService: vi.fn((name: string): any => { + if (name === 'manifest') return { register: vi.fn() }; + if (name === 'objectql') return engine; + return undefined; + }), + getServices: vi.fn(() => new Map()), + hook: vi.fn((name: string, handler: () => Promise) => { if (name === 'kernel:ready') ready.push(handler); }), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + } as unknown as PluginContext; + const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long', baseUrl: 'http://localhost:3000' }); + await plugin.init(ctx); + await plugin.start(ctx); + for (const handler of ready) { + try { await handler(); } catch { /* other kernel:ready work needs services this double does not offer */ } + } + + const ours = registered.filter( + (r) => r.options?.packageId === 'com.objectstack.plugin-auth.organization-mfa-enrollment-guard', + ); + expect(ours.map((r) => [r.event, r.options.object, r.options.priority]).sort()).toEqual([ + ['beforeInsert', 'sys_organization', ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY], + ['beforeUpdate', 'sys_organization', ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY], + ]); + const d2 = registered.find( + (r) => r.event === 'beforeUpdate' && r.options?.packageId === 'com.objectstack.plugin-auth.identity-write-guard', + ); + expect(ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY).toBeGreaterThan(d2!.options.priority); + + const update = ours.find((r) => r.event === 'beforeUpdate')!; + const turnOn = () => update.handler({ object: 'sys_organization', input: { id: 'org_1', data: { require_mfa: true } } }); + // The default deployment: two-factor off. + expectRequireMfaRefusal(await turnOn().then(() => null, (e) => e)); + // The settings pass turns the plugin on (as `auth.mfa_required` does): the next write is admitted. + (plugin as any).authManager.applyConfigPatch({ plugins: { twoFactor: true } }); + await expect(turnOn()).resolves.toBeUndefined(); + }); +}); + +// ── 2. On a real engine ───────────────────────────────────────────────────── + +const engines: ObjectQL[] = []; +afterAll(async () => { + for (const engine of engines) await (engine as any).destroy?.().catch?.(() => undefined); +}); + +const SYSTEM = { context: { isSystem: true, positions: [] as string[], permissions: [] as string[] } }; +/** A user-context write: the administrator saving the organization's edit form. */ +const ADMIN = { context: { userId: 'usr_admin', positions: [] as string[], permissions: [] as string[] } }; + +/** A real sqlite engine with the real organization object and the production hook composition. */ +async function bootOrganizationEngine(enrollable: { value: boolean }): Promise { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + engine.registry.registerObject(SysOrganization as never); + await engine.syncSchemas(); + registerManagedUpdateWhitelist('sys_organization', managedExtensionEditableFields('sys_organization')); + registerIdentityWriteGuard(engine, { packageId: 'test.identity-write-guard' }); + registerOrganizationMfaEnrollmentGuard(engine, { + packageId: 'test.organization-mfa-enrollment-guard', + isTwoFactorEnabled: () => enrollable.value, + }); + await engine.insert('sys_organization', { id: 'org_a', name: 'Acme', slug: 'acme' }, SYSTEM); + await engine.insert('sys_organization', { id: 'org_b', name: 'Bolt', slug: 'bolt' }, SYSTEM); + return engine; +} + +async function storedRequireMfa(engine: ObjectQL, id: string): Promise { + const row: any = await engine.findOne('sys_organization', { where: { id }, fields: ['require_mfa'], ...SYSTEM }); + return row?.require_mfa; +} + +async function caught(p: Promise): Promise { + try { + await p; + return null; + } catch (e) { + return e; + } +} + +describe('organization MFA enrollment guard — on a real engine', () => { + it('refuses the administrator\'s save while two-factor is off, and the row keeps its value', async () => { + const engine = await bootOrganizationEngine({ value: false }); + expectRequireMfaRefusal(await caught(engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN))); + expect(await storedRequireMfa(engine, 'org_a')).toBe(false); + }); + + it('admits the same save once two-factor is on, and the row stores it', async () => { + const enrollable = { value: false }; + const engine = await bootOrganizationEngine(enrollable); + expectRequireMfaRefusal(await caught(engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN))); + // Read per write: turning the plugin on admits the next write, no rebind. + enrollable.value = true; + await engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN); + expect(await storedRequireMfa(engine, 'org_a')).toBe(true); + }); + + it('refuses it for a system write and for a multi-row update too', async () => { + const engine = await bootOrganizationEngine({ value: false }); + expectRequireMfaRefusal(await caught(engine.update('sys_organization', { id: 'org_a', require_mfa: true }, SYSTEM))); + expectRequireMfaRefusal(await caught( + engine.update('sys_organization', { require_mfa: true }, { ...SYSTEM, where: { slug: 'bolt' }, multi: true } as any), + )); + expectRequireMfaRefusal(await caught( + engine.insert('sys_organization', { id: 'org_c', name: 'Cove', slug: 'cove', require_mfa: true }, SYSTEM), + )); + expect(await storedRequireMfa(engine, 'org_a')).toBe(false); + expect(await storedRequireMfa(engine, 'org_b')).toBe(false); + expect(await engine.findOne('sys_organization', { where: { id: 'org_c' }, ...SYSTEM })).toBeNull(); + }); + + it('admits re-saving a row that already requires MFA, so its other edits are not blocked', async () => { + const enrollable = { value: true }; + const engine = await bootOrganizationEngine(enrollable); + await engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN); + // Two-factor goes off afterwards (an env pin, a settings change). + enrollable.value = false; + // A door that sends the whole editable record re-sends the stored `true`. + await engine.update('sys_organization', { id: 'org_a', require_mfa: true, timezone: 'Asia/Shanghai' }, ADMIN); + await engine.update('sys_organization', { id: 'org_a', require_mfa: true }, SYSTEM); + const row: any = await engine.findOne('sys_organization', { where: { id: 'org_a' }, ...SYSTEM }); + expect(row.timezone).toBe('Asia/Shanghai'); + expect(row.require_mfa).toBe(true); + // Control: the other row, stored off, is still refused. + expectRequireMfaRefusal(await caught(engine.update('sys_organization', { id: 'org_b', require_mfa: true }, ADMIN))); + }); + + it('judges a multi-row update row by row: one row turning it on refuses the whole write, with nothing written', async () => { + const enrollable = { value: true }; + const engine = await bootOrganizationEngine(enrollable); + await engine.update('sys_organization', { id: 'org_a', require_mfa: true }, ADMIN); + enrollable.value = false; + // org_a already requires it, org_b does not: the batch turns it on for org_b. + expectRequireMfaRefusal(await caught( + engine.update('sys_organization', { require_mfa: true, timezone: 'UTC' }, { ...SYSTEM, where: {}, multi: true } as any), + )); + expect(await storedRequireMfa(engine, 'org_b')).toBe(false); + const a: any = await engine.findOne('sys_organization', { where: { id: 'org_a' }, ...SYSTEM }); + expect(a.timezone).not.toBe('UTC'); + // Every matched row already requires it: the batch turns nothing on and lands. + await engine.update('sys_organization', { require_mfa: true, timezone: 'UTC' }, { ...SYSTEM, where: { slug: 'acme' }, multi: true } as any); + const a2: any = await engine.findOne('sys_organization', { where: { id: 'org_a' }, ...SYSTEM }); + expect(a2.timezone).toBe('UTC'); + }); + + it('leaves every other organization write alone while two-factor is off', async () => { + const engine = await bootOrganizationEngine({ value: false }); + await engine.update('sys_organization', { id: 'org_a', timezone: 'Asia/Shanghai' }, ADMIN); + await engine.update('sys_organization', { id: 'org_a', require_mfa: false }, ADMIN); + const row: any = await engine.findOne('sys_organization', { where: { id: 'org_a' }, ...SYSTEM }); + expect(row.timezone).toBe('Asia/Shanghai'); + expect(row.require_mfa).toBe(false); + }); +}); + +// ── 3. End to end ─────────────────────────────────────────────────────────── + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-org-mfa'; +const EMAIL = 'member@example.com'; +const BASE = 'http://localhost:3000/api/v1/auth'; +const ORG = 'org_requires_mfa'; + +const cookieHeader = (res: Response): string => + (res.headers.getSetCookie?.() ?? []).map((c) => c.split(';')[0]).join('; '); + +const post = (manager: AuthManager, path: string, body: unknown, headers: Record = {}) => + manager.handleRequest( + new Request(`${BASE}${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...headers }, + body: JSON.stringify(body ?? {}), + }), + ); + +/** + * A member of an organization that requires MFA, unenrolled, whose grace clock + * started 30 days ago, in a session whose active organization is that one — + * the card's measured state, on a deployment with two-factor on or off. + */ +async function arrangeHeldMember(twoFactor: boolean) { + const engine: any = createMemoryEngine(); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + mfaGracePeriodDays: 7, + ...(twoFactor ? { plugins: { twoFactor: true } } : {}), + } as any); + const signedUp = await post(manager, '/sign-up/email', { email: EMAIL, password: PASSWORD, name: 'Member' }); + expect(signedUp.status, `sign-up: ${await signedUp.clone().text()}`).toBe(200); + const cookie = cookieHeader(signedUp); + const user = (engine.tables.get('sys_user') as any[]).find((r) => r.email === EMAIL); + // Stored before the guard existed: the guard judges writes, the gate judges this row. + await engine.insert('sys_organization', { id: ORG, name: 'Acme', slug: 'acme', require_mfa: true }); + user.mfa_required_at = new Date(Date.now() - 30 * 86_400_000).toISOString(); + for (const s of engine.tables.get('sys_session') as any[]) { + if (s.user_id === user.id) s.active_organization_id = ORG; + } + // The "any organization requires MFA" cache is refreshed in the background + // on the session path; let it land so the gate reads the organization row. + manager.isAuthGateActive(); + await vi.waitFor(() => expect(manager.isAuthGateActive()).toBe(true)); + return { engine, manager, cookie, userId: String(user.id) }; +} + +async function sessionAuthGate(manager: AuthManager, cookie: string): Promise<{ code?: string } | undefined> { + const auth: any = await manager.getAuthInstance(); + const session = await auth.api.getSession({ headers: new Headers({ cookie }) }); + expect(session?.user, 'no session for the member').toBeTruthy(); + return session.user.authGate; +} + +describe('end to end — a member is never held for MFA while the enrollment endpoint answers 404', () => { + it('two-factor off: /two-factor/enable answers 404, so the session is not held and no grace clock is started', async () => { + const { engine, manager, cookie, userId } = await arrangeHeldMember(false); + const warn = vi.fn(); + (manager as any).config.logger = { warn, info: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const enable = await post(manager, '/two-factor/enable', { password: PASSWORD }, { cookie }); + // The card's measurement: the enrollment path does not exist here. + expect(enable.status).toBe(404); + expect((await sessionAuthGate(manager, cookie))?.code).not.toBe('MFA_REQUIRED'); + expect(await manager.resolveUserAuthGates(userId, { organizationId: ORG })).toEqual([]); + // Said once, with the remedy — not once per request. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toMatch(/NOT enforced/); + // The stored clock is left as it was (no restamp while unenrollable). + const user = (engine.tables.get('sys_user') as any[]).find((r) => r.id === userId); + expect(new Date(user.mfa_required_at).getTime()).toBeLessThan(Date.now() - 29 * 86_400_000); + }); + + it('two-factor on: the session is held MFA_REQUIRED, and /two-factor/enable answers (the way forward exists)', async () => { + const { manager, cookie } = await arrangeHeldMember(true); + expect((await sessionAuthGate(manager, cookie))?.code).toBe('MFA_REQUIRED'); + const enable = await post(manager, '/two-factor/enable', { password: PASSWORD }, { cookie }); + expect(enable.status, `two-factor/enable: ${await enable.clone().text()}`).toBe(200); + expect(((await enable.clone().json()) as { totpURI?: string }).totpURI).toMatch(/^otpauth:\/\/totp\//); + }); +}); diff --git a/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.ts b/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.ts new file mode 100644 index 00000000000..ebc1b7ce619 --- /dev/null +++ b/packages/plugins/plugin-auth/src/organization-mfa-enrollment-guard.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30201] An organization cannot require multi-factor authentication while + * nobody can enroll a second factor (ADR-0069 D3). + * + * ## The dead end this closes + * + * `sys_organization.require_mfa` is a generically editable extension column + * (`MANAGED_EXTENSION_EDITABLE_FIELDS`): an administrator ticks it on the + * organization's edit form and it saves. The per-user MFA gate then holds every + * unenrolled member once their grace period ends (`resolveUserAuthGates`, + * `MFA_REQUIRED`). The only way to enroll is better-auth's `twoFactor` plugin + * (`/two-factor/*`), which is off by default; on such a deployment the held + * member was sent to enroll and the enrollment endpoint answered 404. Every + * member, the owner included, was locked out, and nobody could undo it from + * inside the product, because Setup sits behind the same gate. + * + * The global switch never had this hole: the `auth.mfa_required` setting + * applies the requirement and the plugin as ONE piece (`auth-plugin.ts`, the + * settings binding), so the requirement is never in force without its + * enrollment path. The organization column had no such pairing. + * + * ## Why the write is refused rather than paired + * + * The plugin's switch is process configuration — the settings pass and the + * `OS_AUTH_TWO_FACTOR` env var, re-applied at every boot — and an organization + * row is data. Turning the plugin on from an organization write would hold in + * the process that took the write only: it would not survive a restart or + * reach another node, because nothing that builds the plugin list reads + * organization rows. And turning it off again when the last organization + * clears the column would silently drop the second factor of every member who + * enrolled. So the card's own fallback is taken: the save is refused, and says + * why and what to do. + * + * ## The rule + * + * While {@link OrganizationMfaEnrollmentGuardOptions.isTwoFactorEnabled} + * answers false, a write that TURNS the requirement on is refused — + * `400 VALIDATION_FAILED` with one `fields[]` entry, `require_mfa` / + * `invalid_value` (the closed ADR-0114 catalog), built by `validationFailure`: + * + * - an insert whose payload sets `require_mfa` to anything but an explicit + * off value; + * - an update whose payload does so for a row whose stored value is off. + * + * Clearing the column, writing other columns, re-sending `require_mfa: true` + * for a row that already stores it (a whole-row SDK or MCP update, an import + * re-run, a save with no change), and every write while two-factor is on pass + * untouched. Refusing the unchanged value would block every other edit of + * that row through a door that sends the whole record, for a value the gate + * already does not enforce (below). + * + * The stored value is the update context's `previous`, which the engine binds + * before `beforeUpdate` on BOTH update paths: the by-id write binds the row it + * read, and a multi-row update fans out one context per matched row, each + * with that row's `previous` and the one shared payload, before anything is + * written. So a multi-row update is judged row by row, and one row turning + * the requirement on refuses the whole write, rows that already stored it + * included, with nothing written. An update context WITHOUT a `previous` is + * judged as a change (refused): no engine path produces one, and an unproven + * prior state is not evidence the value was already on. + * + * Judged for EVERY write context, the system's included, like the last-admin + * guard: the lockout does not depend on who wrote the row. + * + * Rows stored before this guard are not touched. The gate itself does not + * enforce a requirement nobody can comply with (`resolveUserAuthGates`), so + * such a row locks no one out; it starts holding members, with a fresh grace + * window, once two-factor is turned on. + */ + +import { SystemObjectName } from '@objectstack/spec/system'; +import { validationFailure } from '@objectstack/types'; + +/** After the ADR-0092 D2 identity write guard (10), before default-priority hooks (100). */ +export const ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY = 15; + +export const REQUIRE_MFA_FIELD = 'require_mfa'; + +export interface OrganizationMfaEnrollmentGuardOptions { + packageId: string; + /** + * Can a user enroll a second factor right now — is the twoFactor plugin + * wired? Read per write (`AuthManager.isTwoFactorEnabled`, the same answer + * the plugin list and `features.twoFactor` give), so a settings pass that + * turns the plugin on admits the next write. + */ + isTwoFactorEnabled: () => boolean; +} + +/** The engine surface this guard uses. */ +export interface OrganizationMfaEnrollmentGuardEngine { + registerHook( + event: string, + handler: (ctx: any) => Promise, + options?: { object?: string; priority?: number; packageId?: string }, + ): void; +} + +/** + * The values that leave the requirement off. Anything else a payload carries + * for the column — `true`, `1`, `'true'`, or a value the engine would refuse + * later anyway — is judged as turning it on, so the guard never admits a value + * the gate could read as a requirement. + */ +const OFF_VALUES: ReadonlySet = new Set([false, 0, '0', 'false', null, undefined]); + +/** True when `value`, written to `require_mfa`, turns the requirement on. */ +export function turnsMfaRequirementOn(value: unknown): boolean { + if (typeof value === 'string') return !OFF_VALUES.has(value.trim().toLowerCase()); + return !OFF_VALUES.has(value); +} + +/** + * The refusal: what was refused, why, who it would have locked out, and the + * ways forward. The second way is conditional because it is: the + * `OS_AUTH_TWO_FACTOR` env var outranks the settings (`resolveTwoFactorEnabled`), + * so under an `OS_AUTH_TWO_FACTOR=false` pin turning `auth.mfa_required` on + * does not turn enrollment on. + */ +export function organizationMfaWithoutEnrollmentRefusal(): Error { + const message = + 'This organization cannot require multi-factor authentication yet: two-factor enrollment is off on this ' + + 'deployment, so once the grace period ended every member, the owner included, would be blocked with no way ' + + 'to enroll an authenticator app. Turn two-factor on first, then require it here: set OS_AUTH_TWO_FACTOR=true ' + + 'on the deployment. Unless OS_AUTH_TWO_FACTOR is set to false there (it overrides the settings), turning on ' + + 'the auth.mfa_required setting also works: it requires multi-factor authentication for every organization ' + + 'and turns enrollment on with it.'; + return validationFailure(message, [{ field: REQUIRE_MFA_FIELD, code: 'invalid_value', message }]); +} + +/** True when an update context proves the row already stores the requirement on. */ +function alreadyRequiresMfa(ctx: any): boolean { + const previous = ctx?.previous as Record | undefined; + return !!previous && typeof previous === 'object' && turnsMfaRequirementOn(previous[REQUIRE_MFA_FIELD]); +} + +/** Register the guard's insert and update hooks on `sys_organization`. */ +export function registerOrganizationMfaEnrollmentGuard( + engine: OrganizationMfaEnrollmentGuardEngine, + opts: OrganizationMfaEnrollmentGuardOptions, +): void { + const judge = (isUpdate: boolean) => async (ctx: any): Promise => { + const data = ctx?.input?.data as Record | undefined; + if (!data || typeof data !== 'object' || !(REQUIRE_MFA_FIELD in data)) return; + if (!turnsMfaRequirementOn(data[REQUIRE_MFA_FIELD])) return; + // Unchanged: the row already stores it on, so this write turns nothing on. + if (isUpdate && alreadyRequiresMfa(ctx)) return; + if (opts.isTwoFactorEnabled()) return; + throw organizationMfaWithoutEnrollmentRefusal(); + }; + const options = { + object: SystemObjectName.ORGANIZATION, + priority: ORGANIZATION_MFA_ENROLLMENT_GUARD_PRIORITY, + packageId: opts.packageId, + }; + engine.registerHook('beforeInsert', judge(false), options); + engine.registerHook('beforeUpdate', judge(true), options); +} diff --git a/packages/verify/src/held-session-impersonation-target.pin.test.ts b/packages/verify/src/held-session-impersonation-target.pin.test.ts index 75142b80a0c..b6f94fde566 100644 --- a/packages/verify/src/held-session-impersonation-target.pin.test.ts +++ b/packages/verify/src/held-session-impersonation-target.pin.test.ts @@ -160,7 +160,9 @@ async function clearReset(): Promise { /** Enforced MFA that holds the target (grace lapsed) and not the administrator (inside the grace window). */ async function armMfa(): Promise { - auth.applyConfigPatch({ mfaRequired: true, mfaGracePeriodDays: 7 }); + // Two-factor with it, as the `auth.mfa_required` setting applies them: the + // gate holds a user for MFA only while they can enroll. + auth.applyConfigPatch({ mfaRequired: true, mfaGracePeriodDays: 7, plugins: { twoFactor: true } }); await ql.update('sys_user', { id: targetId, mfa_required_at: new Date(Date.now() - 10 * DAY_MS) }, SYS); await ql.update('sys_user', { id: adminId, mfa_required_at: new Date() }, SYS); } diff --git a/packages/verify/src/held-session-vendor-admin-doors.pin.test.ts b/packages/verify/src/held-session-vendor-admin-doors.pin.test.ts index 5741dfa690a..138c8ec7743 100644 --- a/packages/verify/src/held-session-vendor-admin-doors.pin.test.ts +++ b/packages/verify/src/held-session-vendor-admin-doors.pin.test.ts @@ -255,7 +255,9 @@ const HELD_STATES: readonly HeldState[] = [ { code: 'MFA_REQUIRED', async arm() { - auth.applyConfigPatch({ mfaRequired: true, mfaGracePeriodDays: 0 }); + // Two-factor with it, as the `auth.mfa_required` setting applies them: the + // gate holds a user for MFA only while they can enroll. + auth.applyConfigPatch({ mfaRequired: true, mfaGracePeriodDays: 0, plugins: { twoFactor: true } }); const lapsed = new Date(Date.now() - 10 * DAY_MS); for (const id of everyone()) await ql.update('sys_user', { id, mfa_required_at: lapsed }, SYS); }, @@ -330,10 +332,16 @@ for (const state of HELD_STATES) { describe("a hold only the session's organization declares (MFA required by the organization)", () => { let organizationId: string | undefined; + let twoFactorBefore: boolean | undefined; beforeAll(async () => { organizationId = (await sessionOf(await stack.signIn())).organizationId; if (!organizationId) throw new Error('the platform admin session operates in no organization'); + // Two-factor on, by this block itself: an organization cannot require MFA + // while nobody can enroll (the write is refused), and the gate holds no one + // then. Not inherited from whichever held state ran before. + twoFactorBefore = auth.isTwoFactorEnabled(); + auth.applyConfigPatch({ plugins: { twoFactor: true } }); await ql.update('sys_organization', { id: organizationId, require_mfa: true }, SYS); // The 60-second "does any organization require MFA" cache, primed as the // background refresh would after this write. @@ -346,6 +354,7 @@ describe("a hold only the session's organization declares (MFA required by the o afterAll(async () => { if (organizationId) await ql.update('sys_organization', { id: organizationId, require_mfa: false }, SYS); auth._orgMfaCache = { value: false, at: Date.now() }; + if (twoFactorBefore === false) auth.applyConfigPatch({ plugins: { twoFactor: false } }); }); it('holds the admin in that organization and in no other reading (the floor holds no one)', async () => {