diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index a19e6b606..7c16eb323 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -1055,7 +1055,8 @@ carries no open network exposure. The gaps below are **by design and tracked** - **MFA is built but off by default** โ€” native RFC 6238 TOTP for local accounts (WP-14, #336/#338), enabled per deployment via `[auth].require_mfa`. Still single-factor until switched on, and the factor is TOTP (shared-secret, replayable within its ~30 s step window) โ€” phishing-resistant WebAuthn/FIDO2 is the WP-14b - follow-up. AD/Kerberos MFA is delegated to the directory. + follow-up. Directory accounts are in scope too (BACKLOG #1144) -- the delegation this line used to + claim is retired. - **Off-box log shipping is built but opt-in** โ€” the structured-JSON + syslog/SIEM forwarder + cross-backend `audit_log` off-box tee shipped (sec-offbox-log #357/#361/#363); enabling it + pointing at a SIEM endpoint is the per-deployment step, and native TLS-syslog (vs a local TLS-forwarding agent) is the residual. @@ -8424,6 +8425,8 @@ filing. > ๐Ÿ”ข **Re-scored 2026-08-20 -> P2.** Value **6/10** ยท Difficulty **8/10** ยท _big bet_. Both directory legs still mint sessions MFA-verified unconditionally while local accounts face a second factor as an access gate, and require_mfa_scope still defaults to every_local_account, so the strength gap is widest on the shipped default. Value 6 rather than 7 because the shipped comment at service.py:879-882 names a reachable out-of-engine control -- an MFA-enforcing LDAP proxy in front of the simple bind makes the directory pathway genuinely second-factor-backed -- so the residual gap is that the engine asserts rather than verifies, which is an awkward workaround and not the absence of one; difficulty 8 because the routes the research must weigh are directory-side evidence the engine can verify or an AD enrollment authority the code deliberately defers, both new architectural seams in auth. _(was 7/10 ยท 8/10.)_ > Research: done 2026-08-20 > +> PREMISE MOVED 2026-09-03 by BACKLOG #1144, which is a different item and does not close this one: the constraint this row states as fixed -- that the engine does not enroll a second factor for directory identities, so flipping the literal would confine every directory administrator -- no longer holds. The enrollment path was built and the Kerberos leg now mints at the minimum. Re-read #1144's BUILT paragraph before re-scoring or researching this row; the sentence about both directory legs minting MFA-verified unconditionally is stale on both halves. +> > **Filed 2026-08-08 - not started. RESEARCH item: the goal is an HONEST pass, and "cannot honestly reach pass" is a valid finding.** ASVS **6.3.4** (L2) currently scores **partial**. The pinned verb asks that there be no undocumented pathways and that controls and authentication strength be enforced *consistently* across them. The AD and Kerberos legs mint sessions MFA-verified unconditionally (`messagefoundry/auth/service.py:794`, `:824`) while local accounts face a second factor as an access gate (`messagefoundry/api/security.py:227`). > **SHIPPED-BUT-OPEN 2026-08-22 -- PR #497, ledger pairing authored by the LANDER per [ADR 0165](adr/0165-a-builder-pr-satisfies-the-ledger-gate-with-a-paired-commit-authored-by-the-dispatcher-or-lander.md); the builder did not author this.** Layer 1 splits `ad_enabled` into bind capability vs password-login pathway, and classifies `ad_password_login_enabled` as a non-input. Landing in that PR: `messagefoundry/auth/service.py`, `messagefoundry/config/settings.py`, `tests/test_ad_login_pathway_split.py`, `tests/test_security_posture_defaults.py`. @@ -8734,6 +8737,7 @@ filing. > ๐Ÿ”ข **Re-scored 2026-08-20 -> P2.** Value **6/10** ยท Difficulty **5/10** ยท _quick win_. Gap stands on both axes: auth_time appears nowhere in the auth tree, flow.py requests no max_age, and service.py:884/:914 still mint mfa_verified=True on the directory legs with no IdP evidence, so a directory session would satisfy the step-up gate on a first deployment (value 6). The remainder prices at 5, not 6: the AD arm is deferred to #296 by the item's own text, so the deliverable is a research finding plus at most an OIDC-leg recency check, a setting and a SECURITY.md fallback, with the store limb bounded to one nullable column on a sessions table that already gained reauth_at the same way (store.py:3181). _(was 6/10 ยท 7/10.)_ > Research: done 2026-08-20 +> Build: steps 1 and 2 landed 2026-09-03 (directory-account engine-factor enrollment, and the Kerberos leg minting at the minimum, co-landed). Steps 3 through 6 stay open, so does this item. Read the BUILT paragraph before re-scoring: every line anchor in this row had drifted, one of the two directory legs it names was retired by #1137 before the build, and the deciding question the ceiling turns on is still untaken. > > **Filed 2026-08-08 - not started. RESEARCH item: the goal is an HONEST pass, and "cannot honestly reach pass" is a valid finding.** ASVS **6.8.4** (L2) currently scores **partial**. The pinned verb asks that where the application expects specific authentication strength, method or recentness for specific functions, it verifies that from what the IdP returned -- and where the IdP returns nothing, that a documented fallback assumes the MINIMUM mechanism. Recentness is unimplemented (`auth_time` appears nowhere in the auth tree; `build_authorization_url` at `auth/oidc/flow.py:193` sends no `max_age`), and on both directory legs the engine mints `mfa_verified=True` on no IdP evidence at all. > Verdict: research @@ -8760,6 +8764,20 @@ filing. **THE BUILD, under the narrower reading, in dependency order.** (1) **Directory-account engine-factor enrollment**: delete the four non-local refusals (`messagefoundry/auth/service.py:2112/:2130/:2378/:2412`) and the blanket strength exemption (`:2060-2061`). (2) **Mint at the minimum on both directory legs** (`:884`, `:911-914` -- the shipped comment at `:883` already names the parameter as the seam), **co-landed** with (1): with zero deployments the upgrade-lockout cost is vacuous, but shipping it alone is the measured confinement, since a minimum-minted directory session is refused on every route outside the six-entry exempt set at `messagefoundry/api/security.py:79-88` and neither enrollment ceremony accepts a non-local account. (3) **Federated recentness as one indivisible unit**: add a maximum-age parameter to the authorization request (`messagefoundry/auth/oidc/flow.py:206-220` builds nine fixed parameters and no such value), validate the authentication-time claim beside the gate at `messagefoundry/auth/oidc/claims.py:291-310`, **refuse** when it was requested and the claim is absent or stale, and cap the session accordingly. Building the request half alone is the ask-and-never-check shape the shipped assurance-class settings already exhibit (`messagefoundry/config/settings.py:1983` ships empty while `:1984` is requested). (4) A **federated step-up leg**, which needs a session mechanism discriminator and needs no directory enrollment -- the one arm this item asks about directly. (5) **Stop seeding the step-up window at login on the directory legs**, matching the console SSO route, or per-function binding there stays satisfied by a self-stamped timestamp; sequence it against (1), because a second step-up factory (`messagefoundry/api/security.py:577-608`, gating session termination at `messagefoundry/api/auth_routes.py:481` and `:493`) is not covered by the action-step-up requirement. (6) **Then, and only then**, the minimum-strength fallback statement in the documentation -- written today it would be false, which is this item's own trap. **What no build reaches:** strength and method on the simple-bind and SSO legs, whose protocols return no assertion; the verb routes those to the fallback clause, so any future record claiming they **verify** strength is wrong on the protocol. **A NEW CHEAP MOVE the 2026-08-16 ruling created:** arguing that on the shipped default no separate provider exists, so the conditional verb is vacuously satisfied. Rejected by name, and not-applicable is separately foreclosed because the precondition is reachable by configuration. +**BUILT 2026-09-03: steps 1 and 2 landed, co-landed, on the owner's ruling of that date. Steps 3 through 6 stay open, so this item stays open.** The owner did not resolve the deciding question below and ruled instead that steps 1 and 2 are worth funding on the merits whichever way it eventually goes. + +**What landed.** (1) The engine's second factor is now reachable by a DIRECTORY account: the non-local refusals came out of `begin_mfa_enrollment`, `confirm_mfa_enrollment`, `begin_webauthn_registration` and `finish_webauthn_registration`, the blanket provider exemption came out of `_mfa_required_for`, and the `_externally_managed` refusal came off `POST /me/mfa/enroll` in `api/auth_routes.py`, which is the only route reaching the ceremony over HTTP. (2) `authenticate_kerberos` passes `mfa_verified=False`, so a directory session is minted MFA-pending. Two sites the item's enumeration did not name were included and are called out rather than buried: `admin_reset_mfa`'s non-local refusal, without which enrollment is a one-way door for a directory user who loses an authenticator, and the console account page, which hid the MFA and passkey cards from a directory account and would have stranded the users the mint confines. `mfa_satisfied`'s directory branch is now a floor rather than an exit: with `require_mfa` on it refuses, and with it off it falls through to the shared per-user rule so an enrolled directory account must still satisfy the factor it enrolled. `docs/SECURITY.md`, `docs/CONFIGURATION.md`, `docs/SECURITY-LOOSENING.md`, `config/settings.py`, `auth/totp.py` and the `__main__` exposure-gate message all asserted the retired delegation and were corrected; the ADR texts date their claims and were left alone. + +**ANCHOR THE RE-READ ON SYMBOLS, NOT LINES -- every line number in this item had drifted before the build started.** `:2060-2061`, `:2112`, `:2130`, `:2378`, `:2412`, `:884` and `:911-914` all named unrelated code at `46ea10a78`. Worse, the item's "both directory legs" is now ONE: the LDAP simple-bind login was retired by BACKLOG #1137 in `97d37bc9a` on 2026-08-22, two days after this research was written, so only the Kerberos leg survived to be changed. A separate trap: `auth/service.py` carries a second, unrelated exemption removal citing BACKLOG #1245 -- that is the bootstrap carve-out on the 6.4.1 temp-password expiry inside `_login_local`, NOT the strength exemption this item names, which was still present in `_mfa_required_for`. + +**WHAT STAYS OPEN.** Steps 3 through 6, unchanged and out of the 2026-09-03 scope: (3) federated recentness as one indivisible unit -- a maximum-age parameter on the authorization request, the authentication-time claim validated beside the gate, a refusal when it was requested and the claim is absent or stale, and the session capped accordingly; (4) a federated step-up leg, which needs a session mechanism discriminator; (5) stop seeding the step-up window at login on the directory legs; (6) the minimum-strength fallback statement in the documentation, which is deliberately NOT written -- the doc corrections above describe mechanism and make no claim that this requirement's fallback clause is satisfied. + +**THE RESIDUAL, STRENGTHENED rather than corrected, and the correction that was withdrawn stays withdrawn.** Per-function re-checks compare a timestamp the engine stamps itself, and the window is SEEDED AT LOGIN: `_complete_ad_login` defaults `seed_reauth=True`, so the conditional seam never fires for it and the store stamps `reauth_at = now` at session creation. For the whole `step_up_max_age_seconds` window after login (300 default), `has_recent_step_up` is satisfied by the engine's own login stamp with no directory interaction. The build did not touch this -- it is step 5 -- so the residual must record the login-seeded window rather than claim recentness is met. The SSO leg still splits by caller: the console route passes a non-seeding flag under a recorded design note while the JSON negotiate route takes the seeding default, one pathway with two postures depending on which artifact drives it. + +**THE DECIDING QUESTION IS STILL UNTAKEN, and no re-score may assume an answer.** Does the 2026-08-16 "an off-by-default control can never be graded pass" ruling reach a CONDITIONAL requirement whose ANTECEDENT -- not merely whose control -- is unreachable on a stock install? Every implementing control here lives inside an identity-provider leg that ships off and cannot ship on without operator configuration (`ad_enabled = true` is refused at settings load without a server and search base, a secure scheme, a bind account and a bind password, all defaulting unset; Kerberos requires AD; federated requires AD plus four more). Under the strict reading this build moves the cell not at all, and the terminal state is a recorded partial. Two cheap moves remain foreclosed by name: arguing that on the shipped default no separate provider exists so the conditional verb is vacuously satisfied, and not-applicable, which fails because the precondition is reachable by configuration. + +**WHAT NO BUILD REACHES, and no record may claim otherwise.** Strength and method on the simple-bind and SSO legs, whose protocols return no assertion. The verb routes those to the fallback clause, so any future record claiming they VERIFY strength is wrong on the protocol. The shipped `oidc_require_mfa_claim` default was not changed; the residual names it as a worse-direction re-score trigger. + ## 1145. research an honest pass for ASVS 7.1.3 -- federated session-lifetime coordination when the Kerberos leg exposes no ticket lifetime > ๐Ÿ”ข **Re-scored 2026-08-20 -> P3.** Value **3/10** ยท Difficulty **5/10** ยท _fill-in_. Two of three mechanisms ship and the Kerberos limb is untouched: no ticket end time is read anywhere in ldap.py and ADR 0079 still records the cell as accepted-not-built with its promotion trigger unfired. Value is low because the cell carries a signed risk acceptance with a 2027-01-14 review and there is no product effect; difficulty stays mid-band because the honest answer depends on measuring whether any non-SSPI ticket-lifetime source exists on the targeted platforms without forking pyspnego, which the project has ruled out. _(was 3/10 ยท 5/10.)_ diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 7bfb54468..e292f9387 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1532,7 +1532,7 @@ and a PHI weakening under **strict enforcement** (`enforcement = enforce`, the d | `encrypt_stored_data` | bool | `true` | PHI encrypted at rest (key from the environment) | | `allow_unencrypted_phi` | bool | `false` | audited escape: start a PHI instance with **no** key | | `allow_unencrypted_phi_under_strict_enforcement` | bool | `false` | the **second acknowledgment** required to start a PHI instance keyless under strict enforcement ([ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md)). Under `enforcement = enforce`, `allow_unencrypted_phi = true` on its own is **not** enough โ€” `serve` still refuses to start (exit 2) unless this is also set, so the highest-risk posture (real PHI + strict enforcement) is never one flag away from plaintext at rest. Under `enforcement = warn` the single `allow_unencrypted_phi` flag still governs. With both set the instance starts with PHI bodies, summary/metadata and the error columns **unencrypted at rest**, and the startup AUDIT line names **both** flags. A **loosening** โ€” `security_loosenings()` reports it, so it is never silent | -| `allow_single_factor_admin_when_exposed` | bool | `false` | permit **single-factor admin on an exposed PHI instance** (ADR 0140). With `require_sign_in` on, `require_mfa` explicitly off, and the instance exposed โ€” a **non-loopback bind**, **or** a declared TLS-terminating proxy (`[api].tls_terminated_upstream`) โ€” a PHI instance under `enforcement = enforce` **refuses to start** (exit 2) โ€” the Administrator role would authenticate with a single factor over the network. Setting this permits that start; it is recorded in a WARNING-level AUDIT line and the ordinary exposure warning still prints. A **loosening** โ€” `security_loosenings()` reports it. **The exposure test does not consult the browser console** ([BACKLOG #326](archive/backlog/BACKLOG-CLOSED.md#326-mfa-at-exposure-refusal-reads-serve_ui-after-it-is-flipped-off); ADR 0140 amendment). It did, and that made the arm miss the topology this document recommends: a loopback bind behind a declared terminator with `serve_web_console` left at its default, where the ADR 0143 auto-degrade clears the console flag in place before the gate reads it. The exposed surface that authenticates with one factor is the **JSON operator API**, which the proxy serves whether or not `/ui` is mounted, so the predicate is the bind-and-proxy posture alone and the refusal fires on at least: an off-loopback bind; a declared proxy with the console left default-on; and a declared proxy with `serve_web_console = false`. **This refusal is the one exception to the "a new refusal fires only on a new opt-in" scoping rule** stated three rows below on `require_memory_encryption_declaration` โ€” by owner ruling of 2026-08-04, recorded in the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment, which is the single source for why. Nothing new gates it. **One residual is deliberately left open:** an **undeclared** proxy โ€” `web_console_public_address` set with no `tls_terminated_upstream` โ€” does not count as exposed here, because nothing was declared, so exposure would be an *inference*, and an inference must not refuse. It **warns** instead, on its own dedicated arm: on a PHI instance with `require_mfa` explicitly off, startup prints that if that origin is served by an undeclared proxy the Administrator role is single-factor over the network and this refusal cannot see it. Do **not** read the ADR 0068 ยง8 undeclared-proxy warning as that control โ€” it is about the `/ui` session cookie and HSTS, and it is suppressed entirely when the ADR 0143 auto-degrade clears the console flag, which the same `web_console_public_address` triggers. **Prefer `require_mfa = true` โ€” and know its scope.** Under the shipped `require_mfa_scope = "every_local_account"` it requires a second factor from **every** local account, *not* only Administrators, so a non-interactive **local** bearer-token service account becomes MFA-pending and cannot enrol unattended. **There are two remedies, not three.** Either make it a **directory (AD/Kerberos) principal** โ€” those are out of scope under either value, their factor delegated to the directory โ€” or set `require_mfa_scope = "administrators"` (itself reported as a loosening, and it leaves every local Administrator in scope) โ€” see that row below. **mTLS is *not* the third.** A `[api].tls_client_cert_identities` mapping does grant a cert-identity that never meets the MFA gate, but that plane is admitted on exactly **one** route (`GET /service/identity`, `require_service_cert`) and carries no session, so an account "moved to mTLS" can read back its own identity and nothing else โ€” it cannot replay, purge, poll status, or do any work a service account exists for. The `[api].tls_client_cert_identities` row above is the authority on that reach. Directory identities being out of scope also means an AD-only deployment is safe **for its AD users**; its local bootstrap admin and any local service accounts are still in scope | +| `allow_single_factor_admin_when_exposed` | bool | `false` | permit **single-factor admin on an exposed PHI instance** (ADR 0140). With `require_sign_in` on, `require_mfa` explicitly off, and the instance exposed โ€” a **non-loopback bind**, **or** a declared TLS-terminating proxy (`[api].tls_terminated_upstream`) โ€” a PHI instance under `enforcement = enforce` **refuses to start** (exit 2) โ€” the Administrator role would authenticate with a single factor over the network. Setting this permits that start; it is recorded in a WARNING-level AUDIT line and the ordinary exposure warning still prints. A **loosening** โ€” `security_loosenings()` reports it. **The exposure test does not consult the browser console** ([BACKLOG #326](archive/backlog/BACKLOG-CLOSED.md#326-mfa-at-exposure-refusal-reads-serve_ui-after-it-is-flipped-off); ADR 0140 amendment). It did, and that made the arm miss the topology this document recommends: a loopback bind behind a declared terminator with `serve_web_console` left at its default, where the ADR 0143 auto-degrade clears the console flag in place before the gate reads it. The exposed surface that authenticates with one factor is the **JSON operator API**, which the proxy serves whether or not `/ui` is mounted, so the predicate is the bind-and-proxy posture alone and the refusal fires on at least: an off-loopback bind; a declared proxy with the console left default-on; and a declared proxy with `serve_web_console = false`. **This refusal is the one exception to the "a new refusal fires only on a new opt-in" scoping rule** stated three rows below on `require_memory_encryption_declaration` โ€” by owner ruling of 2026-08-04, recorded in the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment, which is the single source for why. Nothing new gates it. **One residual is deliberately left open:** an **undeclared** proxy โ€” `web_console_public_address` set with no `tls_terminated_upstream` โ€” does not count as exposed here, because nothing was declared, so exposure would be an *inference*, and an inference must not refuse. It **warns** instead, on its own dedicated arm: on a PHI instance with `require_mfa` explicitly off, startup prints that if that origin is served by an undeclared proxy the Administrator role is single-factor over the network and this refusal cannot see it. Do **not** read the ADR 0068 ยง8 undeclared-proxy warning as that control โ€” it is about the `/ui` session cookie and HSTS, and it is suppressed entirely when the ADR 0143 auto-degrade clears the console flag, which the same `web_console_public_address` triggers. **Prefer `require_mfa = true` โ€” and know its scope.** Under the shipped `require_mfa_scope = "every_local_account"` it requires a second factor from **every** account, *not* only Administrators and (since BACKLOG #1144) *not* only local ones, so a non-interactive bearer-token service account becomes MFA-pending and cannot enrol unattended. **There is one remedy.** Set `require_mfa_scope = "administrators"` (itself reported as a loosening, and it leaves every Administrator in scope) โ€” see that row below. **Making it a directory (AD/Kerberos) principal is no longer the other one:** directory identities used to be out of scope under either value, their factor delegated to the directory, and BACKLOG #1144 retired that. **mTLS is not one either.** A `[api].tls_client_cert_identities` mapping does grant a cert-identity that never meets the MFA gate, but that plane is admitted on exactly **one** route (`GET /service/identity`, `require_service_cert`) and carries no session, so an account "moved to mTLS" can read back its own identity and nothing else โ€” it cannot replay, purge, poll status, or do any work a service account exists for. The `[api].tls_client_cert_identities` row above is the authority on that reach. An AD-only deployment is therefore in scope for **all** of its accounts โ€” its directory principals, its local bootstrap admin, and any local service accounts | | `allow_unverified_alert_smtp_tls` | bool | `false` | the **acknowledgment** required to start an enforcing PHI instance whose `[alerts]` SMTP hop does not authenticate the relay โ€” i.e. `[alerts].email_use_tls = false` (cleartext) or `[alerts].email_tls_verify = false` (encrypted but accepts any certificate) ([#323](archive/backlog/BACKLOG-CLOSED.md#323-smtp-tls-is-unverified-on-all-three-send-paths)). Covers BOTH shapes deliberately: cleartext is strictly worse than unauthenticated TLS, so gating only the second would hand an operator a bypass onto the worse posture. Without it `serve` refuses to start (exit 2); with it the start is permitted and named in a WARNING-level `AUDIT:` line. An **acknowledgment switch rather than the clamped `MEFOR_ALLOW_INSECURE_TLS` escape** the connectors use, because this cell is constructed outside the `active_hop_posture` scope where that clamp would be inert. A **loosening** โ€” `security_loosenings()` reports it, so it is never silent | | `memory_encryption_operator_declared` | bool | `false` | **`[BUILT]` ([ADR 0152](adr/0152-in-use-data-protection-for-phi-platform-memory-encryption-attestation-asvs-11-7-1.md) rung 2, ASVS 11.7.1):** the operator's **declaration** that this host provides hardware memory encryption (AMD SEV-SNP / Intel TDX), so PHI is protected in RAM **while it is being processed**. The engine cannot verify it โ€” a local CPU flag is emitted by the OS whose integrity the requirement protects against โ€” so this records **who took responsibility**, the same discipline as `MEFOR_TLS_REVOCATION_ATTESTED`. It is deliberately **not** called "attested": in confidential computing that word means a CPU-signed quote verified against the silicon vendor's root PKI (ADR 0152 rung 3, **not built**). An **exposed** PHI instance without it **warns and starts** โ€” on every environment, at both `enforcement` settings; it refuses only if `require_memory_encryption_declaration` is also set. A **positive platform read-out does not substitute for it** (a read-out must never relax a control). **Loopback and synthetic instances are byte-identical** (never consulted). If the platform read-out positively contradicts this, the contradiction is **warned at start and reported** as `memory_encryption_readout_contradicts_declaration` on `GET /security/posture` โ€” but **never refused** (the read-out is a self-report, not evidence, and has known false negatives: driver not loaded, container without the device node mapped, Azure CVM paravisor). **Setting this does not make the instance ASVS 11.7.1-compliant** โ€” see the read-out note below the table. Env: `MEFOR_SECURITY_MEMORY_ENCRYPTION_OPERATOR_DECLARED` | | `require_memory_encryption_declaration` | bool | `false` | **`[BUILT]` (ADR 0152 rung 2):** turn the row-12 warning above into a **refusal** โ€” an **exposed** PHI instance with no `memory_encryption_operator_declared` then **refuses to start** under `enforcement=enforce` (and still warns under `warn`). **Opt-in by design, and the default is load-bearing:** the property is a **host** property that no operator can satisfy on Windows (the read-out is always `null` there), and "exposed" includes the recommended loopback-behind-proxy topology, so a refusal by default would stop working dev/staging/prod deployments from booting on upgrade over something they cannot change. Same scoping rule as `[security].allowed_client_networks`' companion refusal (ADR 0151): a new refusal fires only on a new opt-in. **One exception exists, and it is recorded:** the `allow_single_factor_admin_when_exposed` refusal three rows above was corrected under BACKLOG #326 and fires with no new opt-in gating it โ€” see that row and the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment for the reasoning; do not generalise it. Set it in an estate that has standardized on confidential-computing hosts and wants a missing declaration to be fatal. Env: `MEFOR_SECURITY_REQUIRE_MEMORY_ENCRYPTION_DECLARATION` | @@ -1541,7 +1541,7 @@ and a PHI weakening under **strict enforcement** (`enforcement = enforce`, the d | `external_link_allowlist` | list[str] | `[]` | **`[BUILT]` (ASVS 3.7.3) โ€” โš ๏ธ THE AUDITED ESCAPE, AND IT LOWERS SECURITY.** Destinations listed here are navigated to with **no notification and no cancel**, which is precisely what the requirement asks for. It exists for legitimate high-volume external destinations an operator does not want to declare as their own domain. Same label-boundary matching. Non-empty makes `serve` print a warning **naming every entry individually** โ€” never a count, because "3 destinations exempted" is the shape of message that lets an entry nobody intended sit in a list for a year. **Prefer `organization_domains`**: declaring a domain you control is a statement about scope; allowlisting one you do not is a waiver. Env: `MEFOR_SECURITY_EXTERNAL_LINK_ALLOWLIST` | | `require_sign_in` | bool | `true` | authenticate every request | | `require_mfa` | bool | `true` | second factor (native TOTP or a WebAuthn passkey), enforced as an **access gate** since ASVS 6.3.3 โ€” an MFA-pending session is refused on *every* authorized route with `403` + `X-MFA-Required: 1`, and a browser session is redirected to `/ui/mfa`. **The enrolment path is a deliberate exemption, not a re-route:** `/ui/mfa` itself and the account/enrolment routes (`GET /ui/account`, the password and factor-enrolment routes) are declared `allow_mfa_pending=True` ([`messagefoundry_webconsole/_auth.py`](../messagefoundry_webconsole/_auth.py), `routes/account.py`), so a user with **no** factor enrolled is not stranded โ€” send them to **`/ui/account`** to enrol TOTP or a passkey. Say "redirected to", not "confined to": `/ui/mfa` renders a code field only once TOTP is enrolled and a passkey button only once WebAuthn is, so a zero-factor user who is told they cannot leave that page is looking at a page with no form. | -| `require_mfa_scope` | `"administrators"` \| `"every_local_account"` | `"every_local_account"` | **Which local accounts must ENROL a factor** when `require_mfa` is on (ASVS 6.3.3). An account that has already enrolled one must always satisfy it, under either value โ€” this dial only decides who is required to enrol in the first place. `administrators` restores the pre-6.3.3 posture and is reported as a **loosening** on `GET /security/posture` (advisory, not a refusal: refusing to boot on it would break every existing deployment on upgrade). Directory (AD/Kerberos) identities are out of scope under either value โ€” their MFA is delegated to the directory. **Operator note:** under the default a non-interactive **local bearer-token service account** becomes MFA-pending and cannot enrol unattended. The two available fixes are **make it an AD principal** or **set this to `administrators`** โ€” mTLS is **not** a third: a cert-identity is exempt from the MFA gate but is admitted on exactly one route (`GET /service/identity`), so it cannot carry a working service account (see the [`[api]`](#api) `tls_client_cert_identities` row). Env: `MEFOR_SECURITY_REQUIRE_MFA_SCOPE` | +| `require_mfa_scope` | `"administrators"` \| `"every_local_account"` | `"every_local_account"` | **Which accounts must ENROL a factor** when `require_mfa` is on (ASVS 6.3.3). An account that has already enrolled one must always satisfy it, under either value โ€” this dial only decides who is required to enrol in the first place. `administrators` restores the pre-6.3.3 posture and is reported as a **loosening** on `GET /security/posture` (advisory, not a refusal: refusing to boot on it would break every existing deployment on upgrade). **The `every_local_account` value is wider than its name** (BACKLOG #1144): directory (AD/Kerberos) identities used to be out of scope under either value, and they are not any more โ€” the directory legs assert no strength the engine can read, so a Kerberos session mints MFA-pending and its holder enrols an engine factor. The value's spelling is stale; renaming a `Literal` reaches the settings model, this table and the tests that pin both, which is its own coherent change rather than a rider on a security fix. **Operator note:** under the default a non-interactive **bearer-token service account** becomes MFA-pending and cannot enrol unattended. The one remaining fix is **set this to `administrators`** โ€” making it an AD principal is **no longer** an escape, and mTLS is not one either: a cert-identity is exempt from the MFA gate but is admitted on exactly one route (`GET /service/identity`), so it cannot carry a working service account (see the [`[api]`](#api) `tls_client_cert_identities` row). Env: `MEFOR_SECURITY_REQUIRE_MFA_SCOPE` | | `sign_out_after_idle_minutes` | int | `30` | session idle timeout | | `max_session_hours` | int | `12` | session absolute lifetime | | `block_unlisted_outbound` | bool | `true` | deny-by-default egress โ€” only allow-listed destinations send. **Leaving it unset does not apply `true`** โ€” the internal flag stays `false` and the `[egress]` startup gate decides; see the note under this table | diff --git a/docs/CONTAINER-EXPOSURE-EVALUATION.md b/docs/CONTAINER-EXPOSURE-EVALUATION.md index bd48e0dff..5a68233d5 100644 --- a/docs/CONTAINER-EXPOSURE-EVALUATION.md +++ b/docs/CONTAINER-EXPOSURE-EVALUATION.md @@ -384,7 +384,8 @@ The console stays a **host-side process**; only its target URL changes. 7. **Production-PHI + local accounts must enable MFA to even start.** The MFA-at-exposure gate *refuses* a non-loopback bind on a production PHI instance with local accounts unless - `[auth].require_mfa = true` (or all accounts are AD, where MFA is delegated). The container's + `[auth].require_mfa = true`. An all-AD deployment is no exception: BACKLOG #1144 retired the + directory delegation, so directory accounts are in scope too. The container's default config and docs must make `require_mfa = true` the production default, or the operator hits a hard startup refusal โ€” which is correct, but should be expected, not surprising. diff --git a/docs/REMOTE-CONSOLE.md b/docs/REMOTE-CONSOLE.md index 0c0008186..ce22b40ba 100644 --- a/docs/REMOTE-CONSOLE.md +++ b/docs/REMOTE-CONSOLE.md @@ -194,13 +194,13 @@ Auth is on by default; remote users sign in with local accounts (ยฑ TOTP MFA) or is about the `/ui` cookie and HSTS and is suppressed by ยง3's auto-degrade in the same posture. See the `allow_single_factor_admin_when_exposed` row in [`CONFIGURATION.md`](CONFIGURATION.md) and [`SECURITY-LOOSENING.md`](SECURITY-LOOSENING.md). -- Under the shipped `require_mfa_scope = "every_local_account"`, a non-interactive **local** +- Under the shipped `require_mfa_scope = "every_local_account"`, a non-interactive bearer-token service account becomes MFA-pending and cannot enrol unattended. Settle this **before** - you turn exposure on, and note there are only **two** real destinations for such an account: - **make it a directory (AD/Kerberos) principal** โ€” those are out of MFA scope under either value, - their factor being delegated to the directory โ€” or set + you turn exposure on, and note there is only **one** real destination for such an account: set `[security].require_mfa_scope = "administrators"`, which is itself reported as a loosening on - `GET /security/posture` and leaves *any* local Administrator still in scope. + `GET /security/posture` and leaves *any* Administrator still in scope. + **Not** a directory (AD/Kerberos) principal: those used to be out of MFA scope under either + value, their factor delegated to the directory, and BACKLOG #1144 retired that. **Not** mTLS: `[api].tls_client_cert_identities` maps a verified client cert to a principal, but that plane is admitted on exactly one route (`GET /service/identity`) and carries no bearer/session access, so a service account moved there can read back its own identity and nothing else โ€” it diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index ea9fcec27..f479afa97 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -191,7 +191,8 @@ the call to the Console on 2026-09-02; the Console decided ([ADR 0118](adr/0118- ### `require_mfa = false` โ€” single-factor admin - **What you lose:** the Administrator role authenticates with a password only (no native TOTP second - factor). AD/Kerberos MFA is delegated to the directory and is unaffected. + factor). Directory accounts lose it too (BACKLOG #1144): a Kerberos session mints MFA-pending, and + this knob is the only thing that lets it through the gate without an engine factor. - **When acceptable:** a loopback single-operator box where the second factor adds friction without a network exposure. - **Compensating controls:** keep the bind loopback; enable `admin_new_ip_step_up` if exposed. @@ -261,8 +262,9 @@ the call to the Console on 2026-09-02; the Console decided ([ADR 0118](adr/0118- off; read that arm, not the ADR 0068 ยง8 undeclared-proxy warning, as the control for this case (ยง8 is about the `/ui` cookie and HSTS, and the ADR 0143 auto-degrade suppresses it in the same posture). - **When acceptable:** a production exposure where the second factor is supplied by a **compensating control - outside MessageFoundry** โ€” an authenticating reverse proxy / mTLS admin gateway, or AD/Kerberos MFA - delegated to the directory (this flag gates only local Administrator accounts). + outside MessageFoundry** โ€” an authenticating reverse proxy / mTLS admin gateway. AD/Kerberos MFA + delegated to the directory is **no longer** one of them: BACKLOG #1144 retired that delegation, so + this flag gates every Administrator, directory ones included. - **Compensating controls:** front the admin surface with an MFA-enforcing proxy; prefer `require_mfa = true` (native TOTP); enable `admin_new_ip_step_up`. A startup **AUDIT** line records the override and the posture view (`GET /security/posture`) names it. diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 82d258bc9..4a101ac6c 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -811,15 +811,17 @@ returns the **single-use recovery codes** (shown once), and `POST /auth/mfa-veri second factor with a TOTP code or a recovery code. `DELETE /me/mfa` disables it; an administrator clears a lost authenticator via `POST /users/{id}/reset-mfa` (which also revokes the user's sessions). With `[security].require_mfa` on โ€” **the default since BACKLOG #187 (secure-by-default, including the -loopback bind)** โ€” **every local account** must satisfy MFA: the scope is `every_local_account` by -default, and `administrators` narrows it to the **Administrator** role. It is an **access gate, not +loopback bind)** โ€” **every account** must satisfy MFA: the scope is `every_local_account` by +default (a value now wider than its name, BACKLOG #1144), and `administrators` narrows it to the +**Administrator** role. It is an **access gate, not only a step-up gate** โ€” the gate returns `403` + `X-MFA-Required: 1` on **every** authorized route until verified (console twin: a 303 to `/ui/mfa`), with the account and factor-enrolment routes exempt so an un-enrolled user is not stranded. A required-but-unenrolled admin is never locked out โ€” the enroll/confirm routes sit behind an action-bound **password** step-up, not the MFA gate, so the bootstrap admin enrolls then satisfies it. The documented org opt-out is -`[security].require_mfa = false` (the retired `[auth].require_mfa` spelling is refused at load). **AD/Kerberos MFA is delegated to the directory** (Entra Conditional Access -/ an MFA proxy) โ€” a directory login is never prompted for an engine TOTP and is MFA-satisfied at issuance. +`[security].require_mfa = false` (the retired `[auth].require_mfa` spelling is refused at load). **A +directory account is in scope like any other** (BACKLOG #1144): the Kerberos leg mints MFA-pending, and +a directory user enrols and satisfies an engine factor on the same routes a local user does. The TOTP secret is stored **encrypted at rest** (the store cipher) and recovery codes are **argon2id-hashed**; verification uses the server clock and a constant-time compare over a **configurable clock-skew window** (`[auth].totp_skew_steps`, **default `0` = the current 30 s step only** โ€” strictest @@ -868,8 +870,8 @@ set; on a plain loopback deployment it derives from the request URL, and behind proxy it fails closed** until `public_origin` is configured (anchoring the RP to a proxy-forwardable Host header would defeat the origin binding that makes WebAuthn phishing-resistant). Credentials are pinned to their mint-time `rp_id` โ€” **changing `public_origin`'s host renders enrolled passkeys visibly -"unusable (origin changed)"** (re-enroll after an origin migration). AD/Kerberos users are excluded -exactly as with TOTP (directory-delegated MFA). +"unusable (origin changed)"** (re-enroll after an origin migration). Directory users may enrol a +passkey, exactly as with TOTP (BACKLOG #1144). ### Off-loopback browser console (L5b, ADR 0068 ยง8) @@ -909,7 +911,8 @@ declared (proxy not sending `X-Forwarded-Proto`, or its peer IP not matched by ` **Browser AD login (L5b).** When AD is enabled, `/ui/login` offers a provider selector; an AD password verifies through the **same** `auth.login` directory-bind seam as the JSON surface โ€” allow-listed provider values only, one session per form POST (the AD role-resync/revocation side -effect fires once at login, never per navigation), MFA stays delegated to the directory. +effect fires once at login, never per navigation). MFA is **not** delegated: the engine's own second +factor binds a directory account like any other (BACKLOG #1144). `require_mfa` defaults **on** (BACKLOG #187 โ€” secure-by-default, including the loopback bind; the documented org opt-out is `[security].require_mfa = false` โ€” the `[auth]` spelling of this key is @@ -918,14 +921,17 @@ opt-out**: when the API is bound **off-loopback** with `require_mfa` *turned off posture explicit at startup โ€” it **refuses to start** on a **production PHI** instance and **warns** on a non-production PHI instance (a synthetic instance stays quiet), mirroring the keyless-store and open-egress startup gates. So an exposed PHI deployment can't silently run the Administrator interface -single-factor. `require_mfa` is safe to keep on for an **AD-only** deployment's *directory* users: -AD/Kerberos identities are exempt under either `require_mfa_scope` value, their factor delegated to the -directory. An earlier revision of this sentence said it "gates only **local** Administrator accounts"; -that was wrong. Under the shipped `[security] require_mfa_scope = "every_local_account"` it covers **every** local -account, which on an AD-only deployment still means the local bootstrap admin and any local service -accounts โ€” a non-interactive local bearer-token account becomes MFA-pending and cannot enrol +single-factor. **`require_mfa` now binds an AD-only deployment's *directory* users too** (BACKLOG +#1144): directory identities used to be exempt under either `require_mfa_scope` value, their factor +delegated to the directory, and they are not any more โ€” a Kerberos session mints MFA-pending and its +holder enrols an engine factor to get past the gate. An earlier revision of this sentence said it +"gates only **local** Administrator accounts"; that was wrong. +Under the shipped `[security] require_mfa_scope = "every_local_account"` it covers **every** account +โ€” the value's name is narrower than its behaviour โ€” the local bootstrap admin, any service account, and every +directory principal. A non-interactive bearer-token account becomes MFA-pending and cannot enrol unattended. **That is a decision a deploying site must make before first start:** either such an -account becomes an AD principal, or the scope is set to `administrators`. An operator who opts out at +account moves to the mTLS service-identity plane, or the scope is set to `administrators`. Making it +an AD principal is **no longer** an escape. An operator who opts out at exposure re-enables `[security].require_mfa = true` (or keeps the bind on loopback). [CONFIGURATION.md](CONFIGURATION.md) `[security].require_mfa_scope` is the authority on the two remedies and on why mTLS is not a third. @@ -1185,7 +1191,7 @@ one-to-one โ€” that is why the bind/exposure posture occupies two rows and the A | New client IP during a session | this request's address vs `session.client` | knob on **and** a session exists, is unrevoked, has an anchor, and the two are not the same host (both-loopback counts as one host) | **CHALLENGE** โ€” force a fresh step-up; first sighting also writes `auth.admin_action_new_ip` + an out-of-band notice; repeats WARNING-log only. **Never** an RBAC deny | **off** | `[auth].admin_new_ip_step_up` | | Credential recency | age of `session.reauth_at` | `now โˆ’ reauth_at > step_up_max_age_seconds`, or `reauth_at is None` | **DENY** 403 + `X-Step-Up-Required: 1` (console: 303 โ†’ `/ui/reauth`) | 300 s | `[auth].step_up_max_age_seconds` | | Action-bound step-up grant | a single-use grant minted only by `reauth(purpose=โ€ฆ)`, on the **monotonic** clock | no unconsumed grant for this route's action | **DENY** 403 + `X-Step-Up-Required` + `X-Step-Up-Action: `; opting out falls back to the session window | on | `[auth].require_action_step_up` | -| MFA state | `session.mfa_verified_at` ร— factor enrollment ร— account roles | AD account โ†’ never required here (directory MFA is delegated); LOCAL + enrolled โ†’ always, whatever the scope says; LOCAL + un-enrolled โ†’ required when the knob is on **and** the scope covers the account โ€” **`every_local_account` by default**, i.e. every local account, or the Administrator role only under `administrators` | **DENY** 403 + `X-MFA-Required: 1` on **every** authorized route โ€” an **access gate**, not only a step-up gate; the console twin is a 303 to `/ui/mfa`, with the account and factor-enrolment routes exempt so an un-enrolled user is not stranded. An earlier revision of this row said Administrator-only and step-up-boundary-only; both were wrong | on; scope `every_local_account` | `[security].require_mfa`, `[security].require_mfa_scope` (the `[auth]` spellings are rejected at load) | +| MFA state | `session.mfa_verified_at` ร— factor enrollment ร— account roles | the rule is **provider-blind** (BACKLOG #1144 โ€” an AD account used to be exempt here, on a delegation the directory never asserted): enrolled โ†’ always required, whatever the scope says; un-enrolled โ†’ required when the knob is on **and** the scope covers the account โ€” **`every_local_account` by default**, i.e. every account despite the value's narrower name, or the Administrator role only under `administrators`. A directory session that was minted without an engine-verified factor is refused outright while the knob is on | **DENY** 403 + `X-MFA-Required: 1` on **every** authorized route โ€” an **access gate**, not only a step-up gate; the console twin is a 303 to `/ui/mfa`, with the account and factor-enrolment routes exempt so an un-enrolled user is not stranded. An earlier revision of this row said Administrator-only and step-up-boundary-only; both were wrong | on; scope `every_local_account` | `[security].require_mfa`, `[security].require_mfa_scope` (the `[auth]` spellings are rejected at load) | | Identity provider โ€” local credential rotation | `identity.auth_provider` | the provider is AD (the credential is the directory's, not the engine's) | **DENY** `POST /me/password` with **400**; the step-up re-proof for that identity becomes a **live directory re-bind** instead of a local hash compare, so a disabled AD account cannot refresh its window, and the engine MFA gate never fires for it | n/a | `[auth].ad_enabled` | | Authentication ambience | how the session was minted | browser Kerberos SSO and the OIDC callback mint with `seed_reauth=False` | **CHALLENGE** โ€” the session is born **without** step-up freshness, so its first sensitive action forces an explicit credential step-up (the *second* signal in this table whose action is a challenge rather than a hard decision) | n/a | (by design) | | Session age | `created_at` / `last_used_at` / `expires_at` vs wall clock, on **every** request | idle > 30 min; past the absolute expiry (12 h, or a tighter signature-verified federated `id_token.exp` cap); or a **backward** wall-clock step (NTP step-back, VM snapshot revert) | **DENY** โ€” the session is revoked in the store, then 401. The idle clock is refreshed only by user-driven requests, so a background poll cannot keep a session alive | 30 min / 12 h | `[security].sign_out_after_idle_minutes`, `max_session_hours` (the ADR 0118 homes; `[auth].session_idle_timeout_minutes` / `session_absolute_hours` are the retired aliases), plus `[auth].oidc_session_max_hours` for a tighter federated cap | @@ -1535,8 +1541,8 @@ retired โ€” and the non-interactive mTLS service-identity plane. | Pathway | Factor | Brute-force defense | Notes | |---|---|---|---| | **Local** (argon2id) | **password** (argon2id) **plus an engine second factor** โ€” RFC 6238 TOTP, single-use recovery codes, or a WebAuthn/FIDO2 passkey. That factor is an **access gate, not merely a step-up boundary**: an MFA-pending session is refused on *every* authorized route with `X-MFA-Required: 1`, and a browser session is **redirected** to `/ui/mfa` โ€” *not* confined to it, as an earlier revision of this cell said, because the account and factor-enrolment routes are declared MFA-pending-exempt, so a user with no factor yet enrols at `/ui/account`. It binds any local account that has enrolled a factor, plus every account `[security].require_mfa_scope` covers โ€” **`every_local_account` by default** (`[security].require_mfa` defaults **on**; both keys are rejected under `[auth]` and fail the start). Set the scope to `administrators` for the earlier, narrower posture, in which a non-admin, un-enrolled local session is **password-only end to end**. Caveat: a passkey is asserted at `user_verification=preferred`, so for a passkey-only account the second factor may be **device possession alone** | **per-account lockout** (5/15 min), fed by **both** the password and the TOTP/recovery leg + breach/context policy + the per-IP **and** global sign-in window | the only pathway the engine itself can lock out; the only one with a phishing-resistant factor | -| **AD** (LDAP simple-bind, LDAPS by default) โ€” **step-up re-authentication only; the sign-in was retired** | password, verified by a bind **as the user** against the DC. It no longer mints a session: `POST /auth/login` with `provider=ad` is refused and audited, and the bind survives only at `POST /me/reauth`, where it re-proves a session **another** pathway minted. So this row carries no MFA grant of its own โ€” the session's MFA state was decided at sign-in by Kerberos or OIDC, and the delegated-directory relaxation now sits on the Kerberos row that still exercises it | the **directory's** lockout/complexity policy; engine-side, a **per-actor** step-up budget, **not** the sign-in limiter โ€” the bind is post-session, so an unauthenticated flood cannot reach it, and `[auth].login_rate_limit_enabled=false` no longer strips this pathway bare โ€” and **no** engine per-account lockout | password strength + lockout are the AD domain's responsibility. LDAPS is the default, not a structural guarantee: `[auth].ad_allow_insecure_ldap` opts into a plain bind, and `ad_tls_verify=false` is refused at startup unless the `MEFOR_ALLOW_INSECURE_TLS` dev escape is set | -| **Kerberos / SPNEGO** | domain ticket; MFA is **delegated and unverifiable at the engine** โ€” the session is issued MFA-satisfied and no `amr`-equivalent evidence is received | the **domain's** controls; engine-side, the sign-in window on the token-bearing leg (`[auth].login_rate_limit_enabled`, default on โ€” **off leaves this pathway with no engine-side control at all**; the RFC 4559 challenge leg is deliberately unthrottled either way) | experimental, off by default, **single-leg โ€” no mutual authentication**, channel binding deliberately un-enforced. The browser leg (`GET /ui/sso`) mints with no step-up window, so the first sensitive action forces a step-up; the JSON `POST /auth/negotiate` seeds it | +| **AD** (LDAP simple-bind, LDAPS by default) โ€” **step-up re-authentication only; the sign-in was retired** | password, verified by a bind **as the user** against the DC. It no longer mints a session: `POST /auth/login` with `provider=ad` is refused and audited, and the bind survives only at `POST /me/reauth`, where it re-proves a session **another** pathway minted. So this row carries no MFA grant of its own โ€” the session's MFA state was decided at sign-in by Kerberos or OIDC. The delegated-directory relaxation it used to carry is **retired** (BACKLOG #1144): no pathway grants MFA satisfaction on a directory assertion the engine cannot read | the **directory's** lockout/complexity policy; engine-side, a **per-actor** step-up budget, **not** the sign-in limiter โ€” the bind is post-session, so an unauthenticated flood cannot reach it, and `[auth].login_rate_limit_enabled=false` no longer strips this pathway bare โ€” and **no** engine per-account lockout | password strength + lockout are the AD domain's responsibility. LDAPS is the default, not a structural guarantee: `[auth].ad_allow_insecure_ldap` opts into a plain bind, and `ad_tls_verify=false` is refused at startup unless the `MEFOR_ALLOW_INSECURE_TLS` dev escape is set | +| **Kerberos / SPNEGO** | domain ticket **plus an engine second factor**. No `amr`-equivalent evidence reaches the engine, so the ticket proves nothing about directory-side factor strength and the session is issued **MFA-pending** (BACKLOG #1144). It used to be issued **MFA-satisfied** under a delegated-directory relaxation, which cleared every engine MFA gate on zero engine-readable evidence; that grant is retired. While `[security].require_mfa` is on, an un-satisfied directory session reaches only the MFA-pending-exempt routes, and its holder enrols a TOTP or a passkey on the same routes a local account uses. Set `require_mfa = false` for the earlier single-factor posture | the **domain's** controls; engine-side, the sign-in window on the token-bearing leg (`[auth].login_rate_limit_enabled`, default on โ€” **off leaves this pathway with no engine-side control at all**; the RFC 4559 challenge leg is deliberately unthrottled either way) | experimental, off by default, **single-leg โ€” no mutual authentication**, channel binding deliberately un-enforced. The browser leg (`GET /ui/sso`) mints with no step-up window, so the first sensitive action forces a step-up; the JSON `POST /auth/negotiate` seeds it | | **OIDC federation** (browser only, hybrid AD-backed) | IdP-asserted, gated on a **signature-verified** `amr`/`acr` claim (`[auth].oidc_require_mfa_claim` defaults **on**) โ€” an assertion, not a proof | no engine credential to guess, so no per-account lockout; both legs (`/ui/oidc/start`, `/ui/oidc/callback`) charge the sign-in window (`[auth].login_rate_limit_enabled`, default on โ€” **off leaves this pathway with no engine-side control at all**, though the bounded pending-flow cache still caps concurrent start legs), plus the IdP's own lockout | hybrid-only: a federated principal with no on-prem AD object is refused. Roles come from LDAP, never from a token claim. When `[auth].oidc_username_strip_domain` is on (default), the claim's UPN suffix must match `oidc_allowed_username_domains` (or `[auth].ad_domain`); with stripping **off** the claim is used verbatim and no suffix check applies. The session's absolute lifetime is capped at the verified `id_token.exp`; minted with no step-up window | | **mTLS service identity** (non-interactive, ADR 0083) | a **verified** client certificate mapped through a deny-by-default, name-space-qualified allow-list (`CN:` / `SAN::`) | **not applicable** โ€” no guessable secret and no lockout; admission requires a chain verifying to the pinned client CA plus a listed qualified name | no session, no MFA, no step-up โ€” which is why it is **PHI-fenced**: `require_service_cert` raises at **app construction** if asked to gate a PHI-view permission. One route only (`GET /service/identity`); every success is audited `service_cert_auth` | @@ -1545,8 +1551,8 @@ Comparative properties on the dimensions the table's four columns cannot carry: | Pathway | Phishing resistance | Replay resistance | Credential stored by the engine | MFA support | Revocation | |---|---|---|---|---|---| | **Local** | passkeys only (WebAuthn origin-bound, `attestation=none`, `user_verification=preferred`); password/TOTP are phishable | TOTP is single-use per 30 s step (`totp_skew_steps` default `0`); recovery codes single-use; passkey challenges are 64-byte CSPRNG, single-use, 120 s TTL, with a strict sign-counter compare-and-set | argon2id password hash (t=3, m=64 MiB, p=4); TOTP secret **cipher-encrypted**; recovery codes argon2id-hashed; COSE public keys **plaintext by design** | built (TOTP + passkeys) | disable the account or revoke sessions โ€” immediate | -| **AD** | none | none beyond TLS | **none** โ€” only the service-account bind password (env or a `[secrets]` reference, fail-closed) | **not asserted here** โ€” the bind re-proves an existing session and grants nothing; the delegated, **unverifiable** MFA grant belongs to the Kerberos row | disabling in AD does **not** end a live session on its own; `[auth].ad_session_recheck_seconds` (default **300 s**) closes it, bounded by interval โ€” strikes | -| **Kerberos** | none (single-leg, no channel binding) | ticket lifetime is the domain's | **none** โ€” the acceptor keytab/SPN is OS-owned | delegated, unverifiable | as AD | +| **AD** | none | none beyond TLS | **none** โ€” only the service-account bind password (env or a `[secrets]` reference, fail-closed) | **not asserted here** โ€” the bind re-proves an existing session and grants nothing; the delegated, engine-unreadable MFA grant that used to belong to the Kerberos row is retired (BACKLOG #1144) | disabling in AD does **not** end a live session on its own; `[auth].ad_session_recheck_seconds` (default **300 s**) closes it, bounded by interval โ€” strikes | +| **Kerberos** | none (single-leg, no channel binding) | ticket lifetime is the domain's | **none** โ€” the acceptor keytab/SPN is OS-owned | an **engine** factor (TOTP or passkey), enrolled and satisfied at the engine โ€” the ticket asserts nothing the engine can read, so nothing is delegated (BACKLOG #1144) | as AD | | **OIDC** | the IdP's, not the engine's | strongest of the four: server-side PKCE verifier + `state` (constant-time compare) + `nonce`, single-use flow, a `__Host-`-prefixed browser-binding cookie the callback requires, and a `typ`/kid/alg/signature/`events`/`iss`/`aud`/`exp`/`iat`/`nbf`/`nonce`/`sub` ladder under a bounded clock skew โ€” `typ` and `events` assert the token **class** (an access token or a logout token carries the same issuer and key), and `sub`/`iat` are required rather than optional | **none** โ€” only the confidential-client secret (env-only or a `[secrets]` reference, resolved eagerly at startup) | asserted via `amr`/`acr` **and enforced** โ€” with `[auth].oidc_require_mfa_claim` on (default) a token carrying no configured `amr`/`acr` is refused at claims validation, and only then is the session minted MFA-verified; switch it off and the federated session is minted **un**verified, which `mfa_satisfied` refuses. This is the one directory leg whose factor the engine actually verifies | as AD, plus the `id_token.exp` cap; no refresh tokens and no RP-initiated logout | | **mTLS** | n/a (no interactive ceremony) | n/a | **none** โ€” the engine holds only the pinned client CA and the name map | none, structurally | **no revocation checking** โ€” `VERIFY_X509_STRICT` is strict path validation, not OCSP/CRL; live revocation is the org's PKI. Engine-side: remove the allow-list entry (config change โ†’ restart) or disable the mapped account | @@ -1603,22 +1609,28 @@ only at the step-up boundary** โ€” an MFA-pending session is refused on every au revision of this sentence asserted the opposite on both counts and named the `[auth]` keys the loader rejects; it also contradicted the Local row of the table above, which was right (see [Multi-factor authentication](#multi-factor-authentication-totp-wp-14)). -AD/Kerberos MFA is delegated to the directory; OIDC's is asserted by the IdP and gated on a -signature-verified claim. **The consequence, stated plainly:** the AD and Kerberos pathways satisfy the -engine's MFA gates without an engine-verified factor, so a *domain ticket* on the Kerberos pathway -reaches the same PHI surface as a passkey-backed local Administrator. The mechanism is a **per-mechanism argument**, not a -blanket literal: `mfa_verified` is a keyword parameter of `_complete_ad_login`, and the Kerberos leg -passes `True` under the signed delegated-directory relaxation while the federated leg passes +**No pathway grants MFA satisfaction on an assertion the engine cannot read** (BACKLOG #1144). The +mechanism is a **per-mechanism argument**, not a blanket literal: `mfa_verified` is a keyword parameter +of `_complete_ad_login`. The Kerberos leg passes `False` โ€” a service ticket carries no factor-strength +assertion `pyspnego` surfaces, so the engine grants nothing on it and mints the session MFA-pending, +leaving its own second factor to decide the rest. The federated leg passes `[auth].oidc_require_mfa_claim` itself โ€” on by default, and reached only after the claim gate has already refused any token carrying no configured `amr`/`acr`, so the grant there is engine-verified rather than -assumed. Turn that setting off and the federated session mints **un**verified, and `mfa_satisfied` -refuses it while `[security].require_mfa` is on (the default). An earlier revision of this sentence said -`_complete_ad_login` mints all three `mfa_verified=True` unconditionally; that was wrong about the -mechanism and contradicted the OIDC row of the table above. OIDC therefore remains the only delegated -pathway carrying engine-side evidence at all; AD and Kerberos carry none, and **closing that gap is the -deploying site's job, in the directory** โ€” the engine accepts whatever the directory asserts, so the -domain's own MFA policy (Entra Conditional Access or an MFA proxy) is the only control over those two -pathways. +assumed. Turn that setting off and the federated session mints **un**verified too, and `mfa_satisfied` +refuses it while `[security].require_mfa` is on (the default). + +Two earlier revisions of this paragraph were wrong in opposite directions and are recorded so neither +returns: one said `_complete_ad_login` mints all three `mfa_verified=True` unconditionally, which was +wrong about the mechanism; the next said the AD and Kerberos pathways satisfy the engine's MFA gates +without an engine-verified factor, so a *domain ticket* reaches the same PHI surface as a +passkey-backed local Administrator. That was true of the shipped code when it was written, and the +code has since changed rather than the sentence being mistaken. + +**What is still delegated, and what is not.** The directory keeps its own MFA policy (Entra Conditional +Access, an MFA proxy) and the engine still cannot read the outcome โ€” that limb is protocol, not policy, +and no build changes it. What is no longer delegated is the *decision*: the engine does not treat an +unreadable directory assertion as a satisfied factor. OIDC remains the only directory leg carrying +engine-side evidence of strength at all. ## Brute-force & abuse protection @@ -2021,7 +2033,7 @@ SLSA + the Sigstore identity check) per [INSTALL-GUIDE.md](INSTALL-GUIDE.md#veri The remaining `code:edit` / `config:validate` / `service:configure` endpoints those permissions will gate. (**OIDC federation is now built** โ€” see "Federated sign-in" under *Local vs Active Directory* โ€” and **custom roles shipped** -in 0.2.10; both were listed here after the fact.) **Transport TLS is built** โ€” API/WS (WP-13a), the reverse-proxy / forwarded-header path (WP-15), and MLLP-over-TLS (WP-13b, per-connection `tls`/`tls_*`), per [ADR 0002](adr/0002-phase2-transport-security-and-strong-auth.md) (*Accepted*). The ยง0 **exposed-gate is enforced** โ€” a non-loopback *plaintext* API or MLLP bind is refused at startup unless `serve --allow-insecure-bind`. ADR-0002 **MFA (WP-14) is now built** โ€” native TOTP for local accounts (see "Multi-factor authentication" above); AD/Kerberos MFA is delegated to the directory. The **DICOM C-STORE SCP inbound** (ADR 0025 Phase 1) carries the same posture: it accepts only allowlisted calling AE titles + peer IPs, supports **DICOM-over-TLS**, and a non-loopback bind is refused unless explicitly overridden. **Outbound egress auth** for the FHIR/REST connector is built as a **SMART Backend Services token provider** (ADR 0024) โ€” OAuth2 `client_credentials` with a signed-JWT (RS384/ES384) client assertion (extending the ADR 0018 signing core, no new dependency), opted in per connection via `with_smart_backend()`; it mints a per-request bearer and re-mints on `401`, and the token endpoint is gated by `[egress].allowed_http`. It is **client-only** โ€” no App Launch flow and no authorization-server facade. **SMART trust boundary (BACKLOG #204, ASVS 10.4.16):** the engine *presents* a `private_key_jwt` client assertion (RFC 7523) to the token endpoint, but *enforcing* that method โ€” validating the assertion signature/audience/expiry, refusing a weaker `client_secret_post`/`client_secret_basic` for this client, and replay-protecting the `jti` โ€” is the **authorization server's responsibility**, a boundary the client engine does not and cannot police. MessageFoundry assumes an AS that mandates private_key_jwt for Backend Services clients; an AS that *also* accepts a weaker authentication method is an AS-side misconfiguration, not a client-engine defect. (Encryption at rest, audit hash-chaining, +in 0.2.10; both were listed here after the fact.) **Transport TLS is built** โ€” API/WS (WP-13a), the reverse-proxy / forwarded-header path (WP-15), and MLLP-over-TLS (WP-13b, per-connection `tls`/`tls_*`), per [ADR 0002](adr/0002-phase2-transport-security-and-strong-auth.md) (*Accepted*). The ยง0 **exposed-gate is enforced** โ€” a non-loopback *plaintext* API or MLLP bind is refused at startup unless `serve --allow-insecure-bind`. ADR-0002 **MFA (WP-14) is now built** โ€” native TOTP and passkeys for every account, directory ones included (see "Multi-factor authentication" above). The **DICOM C-STORE SCP inbound** (ADR 0025 Phase 1) carries the same posture: it accepts only allowlisted calling AE titles + peer IPs, supports **DICOM-over-TLS**, and a non-loopback bind is refused unless explicitly overridden. **Outbound egress auth** for the FHIR/REST connector is built as a **SMART Backend Services token provider** (ADR 0024) โ€” OAuth2 `client_credentials` with a signed-JWT (RS384/ES384) client assertion (extending the ADR 0018 signing core, no new dependency), opted in per connection via `with_smart_backend()`; it mints a per-request bearer and re-mints on `401`, and the token endpoint is gated by `[egress].allowed_http`. It is **client-only** โ€” no App Launch flow and no authorization-server facade. **SMART trust boundary (BACKLOG #204, ASVS 10.4.16):** the engine *presents* a `private_key_jwt` client assertion (RFC 7523) to the token endpoint, but *enforcing* that method โ€” validating the assertion signature/audience/expiry, refusing a weaker `client_secret_post`/`client_secret_basic` for this client, and replay-protecting the `jti` โ€” is the **authorization server's responsibility**, a boundary the client engine does not and cannot police. MessageFoundry assumes an AS that mandates private_key_jwt for Backend Services clients; an AS that *also* accepts a weaker authentication method is an AS-side misconfiguration, not a client-engine defect. (Encryption at rest, audit hash-chaining, **per-channel RBAC** โ€” including the web console scope editor and AD-groupโ†’scope mapping โ€” and the **committed dependency lockfile** are now built; see [PHI.md ยง3](PHI.md#3-encryption-at-rest), *Audit*, the per-channel-scoping note, and *Dependency lockfile (DEP-1)* above.) diff --git a/docs/Secure_Development_Standards.md b/docs/Secure_Development_Standards.md index 9d3cf569b..613388e53 100644 --- a/docs/Secure_Development_Standards.md +++ b/docs/Secure_Development_Standards.md @@ -624,11 +624,12 @@ current position on it. - **Transport TLS:** native **API HTTPS/WSS** and **MLLP-over-TLS** (TLS 1.2+, opt-in via cert config), with optional client-certificate **mTLS** (API `tls_client_ca_file`; MLLP `tls_ca_file`), an off-loopback bind guard, and a certificate-expiry monitor โ€” ADR 0002 / WP-13a/13b. -- **Operator strong-auth (control plane):** native **RFC 6238 TOTP MFA** for **local** accounts +- **Operator strong-auth (control plane):** native **RFC 6238 TOTP MFA** for **every** account (ADR 0002 WP-14, built 2026-06-17) โ€” enrolled per user, enforced for the Administrator role via - `[auth].require_mfa` and re-verified at the sensitive-operation step-up boundary; AD/Entra users' - MFA is delegated to the IdP. Recovery codes are argon2id-hashed; the TOTP secret is store-cipher - protected. + `[auth].require_mfa` and re-verified at the sensitive-operation step-up boundary. AD/Entra users' + MFA was delegated to the IdP until BACKLOG #1144, which retired that: a directory ticket asserts + no strength the engine can read, so the engine asks for its own factor. Recovery codes are + argon2id-hashed; the TOTP secret is store-cipher protected. - **Operator / directory (control plane, not interface auth):** **LDAPS** directory bind (certificate-validated; cleartext `ldap://` refused fail-closed), **Kerberos / SPNEGO** Windows SSO, and **AD security-group โ†’ role** mapping for RBAC. These authenticate **human operators** to the @@ -701,10 +702,10 @@ per-requirement verdicts: `security/ASVS-L3-ASSESSMENT.md`. Reviewed at each release and on any trigger below. Those are maintainer-internal documents; [`SECURITY-DOCS-POLICY.md`](SECURITY-DOCS-POLICY.md) explains what is withheld and what you can request. -- **6.3.3 โ€” multi-factor authentication.** **Satisfied for local accounts** โ€” native RFC 6238 TOTP MFA +- **6.3.3 โ€” multi-factor authentication.** **Satisfied** โ€” native RFC 6238 TOTP MFA is **built** (ADR 0002 WP-14, 2026-06-17), enforced for the Administrator role via `[auth].require_mfa` - at the step-up boundary; **AD/Entra-account MFA is delegated to the IdP** (the supported enterprise - path). No longer a deferred Fail. *(Hardware/WebAuthn second factors are now **built** โ€” browser + at the step-up boundary, and it covers **every** account since BACKLOG #1144 retired the + AD/Entra delegation to the IdP. No longer a deferred Fail. *(Hardware/WebAuthn second factors are now **built** โ€” browser WebAuthn passkeys as the phishing-resistant second factor at the step-up boundary, ADR 0068 / WP-14b, behind the `[webauthn]` extra.)* - **4.1.5 โ€” per-message digital signatures on the PHI data plane.** Deferred-by-design. Transport-level diff --git a/harness/_login.py b/harness/_login.py index 2adeddb2c..1b41175cc 100644 --- a/harness/_login.py +++ b/harness/_login.py @@ -103,8 +103,9 @@ def _attempt(self) -> None: # Hand the must-change flag and the just-entered plaintext back to _authenticate. self.must_change_password = result.must_change_password self.entered_password = password - # The engine accepted the password but wants a second factor (local MFA-enrolled / required - # admin); _authenticate prompts for the TOTP code before opening the window (always False for - # an MFA-delegated AD login). + # The engine accepted the credential but wants a second factor; _authenticate prompts for the + # TOTP code before opening the window. This used to be documented as always False for a + # directory login; that delegation is retired (BACKLOG #1144), and a directory session now + # reports the debt like any other. self.mfa_required = result.mfa_required self.accept() diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index 04475589d..e562fcb25 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1998,9 +1998,9 @@ def _serve(args: argparse.Namespace) -> int: # keyless-store / open-egress posture: refuse on a production PHI instance (the prod fail-closed # analogue), warn on a non-production PHI instance, stay quiet on a synthetic instance. Reached only # for an otherwise-permitted exposed bind (the TLS gate above ran first); the loopback default (now - # require_mfa on) never trips it. AD/Kerberos MFA is delegated to the directory, so require_mfa only - # gates LOCAL Administrator accounts (the bootstrap admin is one) โ€” it is safe to leave on even on - # an AD-only deployment. + # require_mfa on) never trips it. Since BACKLOG #1144 require_mfa gates DIRECTORY accounts too โ€” a + # ticket asserts no factor strength the engine can read, so the engine asks for its own factor โ€” + # which makes leaving it on correct on an AD-only deployment rather than merely harmless there. # # L5b review fix (ADR 0068 ยง8), corrected by BACKLOG #326: the gate keys on the same EXPOSURE signal # as the ladder above, not the bind host alone โ€” the runbook's RECOMMENDED topology (loopback bind @@ -2023,8 +2023,8 @@ def _serve(args: argparse.Namespace) -> int: f"instance ({env_name!r}) with [security].require_mfa off; refusing to start โ€” the " "Administrator role would authenticate with a single factor over the network. " "Enable native TOTP MFA with [security].require_mfa=true (WP-14) before exposing the " - "API (safe even on an AD-only deployment โ€” it gates only local Administrator " - "accounts); or set [security].allow_single_factor_admin_when_exposed=true to " + "API (on an AD-only deployment it binds directory principals too, each enrolling " + "an engine factor); or set [security].allow_single_factor_admin_when_exposed=true to " "deliberately permit single-factor admin at exposure (audited).", file=sys.stderr, ) diff --git a/messagefoundry/api/auth_routes.py b/messagefoundry/api/auth_routes.py index 0be44ee9d..c07097c03 100644 --- a/messagefoundry/api/auth_routes.py +++ b/messagefoundry/api/auth_routes.py @@ -119,11 +119,17 @@ def _session_info(session: SessionRecord, current_token_hash: str) -> SessionInf def _externally_managed(provider: AuthProvider | str) -> bool: """True when an account's credentials/roles come from an external directory (anything but LOCAL). - Mirrors the service layer's ``!= AuthProvider.LOCAL.value`` gate (``auth/service.py``). The API - previously spot-checked ``is AuthProvider.AD`` / ``== AuthProvider.AD.value`` here, which diverges - the instant a third provider exists: an ``== AD`` test would wave a non-LOCAL, non-AD account - *through* a LOCAL-only ceremony (change engine password, enroll an engine TOTP, get engine-managed - roles) that the service then rejects. Gating on ``!= LOCAL`` keeps every provider consistent. + The API previously spot-checked ``is AuthProvider.AD`` / ``== AuthProvider.AD.value`` here, which + diverges the instant a third provider exists: an ``== AD`` test would wave a non-LOCAL, non-AD + account *through* a ceremony the engine does not own for it. Gating on ``!= LOCAL`` keeps every + provider consistent. + + **TWO LIVE CALL SITES, and this is the SOLE enforcement point for both** โ€” ``POST /me/password`` + and the role update. It used to describe itself as mirroring a service-layer twin; that mirror + survives only for ``admin_reset_password``, and MFA enrollment left this set entirely (BACKLOG + #1144), because an engine second factor IS engine-held state that any provider may hold. What + remains is the narrower rule: the engine is not the system of record for a directory account's + password or roles. Accepts either the ``AuthProvider`` enum (``Identity.auth_provider``) or its stored string value (``UserRecord.auth_provider``).""" value = provider.value if isinstance(provider, AuthProvider) else provider @@ -280,7 +286,16 @@ async def negotiate( outcome = await service.authenticate_kerberos(token_bytes, client=_client(request)) if not outcome.ok or outcome.token is None or outcome.identity is None: raise HTTPException(status.HTTP_401_UNAUTHORIZED, "SSO authentication failed") - return _login_response(outcome.token, outcome.identity, outcome.must_change_password) + # mfa_required is FORWARDED here, not defaulted (BACKLOG #1144). This route used to omit it + # because a directory session was minted MFA-satisfied and the answer was always False; the + # Kerberos leg now mints at the minimum, so omitting it would tell the client no second factor + # is needed and let the next call be refused with no explanation it was given. + return _login_response( + outcome.token, + outcome.identity, + outcome.must_change_password, + mfa_required=outcome.mfa_required, + ) @app.post("/auth/logout", response_model=SimpleMessage) async def logout( @@ -395,11 +410,9 @@ async def enroll_mfa( """Begin TOTP enrollment: stage a secret and return it + the ``otpauth://`` URI for the QR. Gated by a fresh **password** step-up BOUND to this enroll action (ADR 0077 โ€” not MFA, you may have none yet; and not the shared login window, so a hijacked session can't bind a factor); not - active until confirmed via ``/me/mfa/confirm``.""" - if _externally_managed(identity.auth_provider): - raise HTTPException( - status.HTTP_400_BAD_REQUEST, "AD accounts use directory MFA, not an engine TOTP" - ) + active until confirmed via ``/me/mfa/confirm``. No provider gate: an engine factor is engine- + held state and every provider may hold one (BACKLOG #1144, and see + :meth:`~messagefoundry.auth.service.AuthService.begin_mfa_enrollment` for why).""" try: enroll = await service.begin_mfa_enrollment(identity) except ValueError as exc: diff --git a/messagefoundry/auth/service.py b/messagefoundry/auth/service.py index 6b284d856..f4bf70131 100644 --- a/messagefoundry/auth/service.py +++ b/messagefoundry/auth/service.py @@ -152,9 +152,12 @@ class LoginOutcome: identity: Identity | None = None must_change_password: bool = False error: str | None = None - #: The password was accepted but the session still needs a second factor (TOTP / recovery code) - #: before it may perform step-up (sensitive) operations โ€” the client should prompt for a code and - #: call ``POST /auth/mfa-verify`` (WP-14, ASVS 6.3.3). Always False for an MFA-delegated AD login. + #: The credential was accepted but the session still owes a second factor before it may reach an + #: authorized route โ€” the client should prompt for a code and call ``POST /auth/mfa-verify``, or + #: enrol a factor first if it has none (WP-14, ASVS 6.3.3). It used to be documented as always + #: False for a directory login; that stopped being true when the Kerberos leg began minting at the + #: minimum (BACKLOG #1144), and reporting False there would tell a JSON client no factor is needed + #: seconds before the gate refuses it with ``X-MFA-Required: 1``. mfa_required: bool = False #: A CLOSED-SET reject slug for the federated path (ADR 0142), so the browser layer can pick an #: allow-listed error code without parsing ``error`` (free prose) or seeing any IdP-supplied text. @@ -946,12 +949,20 @@ async def authenticate_kerberos( if principal is None: await self._directory_reject_audit(username, "kerberos", "not_in_directory") return LoginOutcome(ok=False, error="user not found in directory") - # The signed delegated-directory relaxation (ASVS 6.3.4): a Kerberos service ticket carries no - # factor-strength assertion that pyspnego surfaces, so directory delegation stands. Since the - # AD password sign-in was retired (BACKLOG #1137) this is the ONLY leg passing a hard True -- - # docs/SECURITY.md's Kerberos rows are where that grant is now disclosed. + # MINT AT THE MINIMUM (BACKLOG #1144, ASVS 6.8.4). A Kerberos service ticket carries no + # factor-strength assertion that pyspnego surfaces, so the engine learns NOTHING about what + # the domain enforced. It used to pass a hard True here under the signed delegated-directory + # relaxation, which is the inverted fallback: the requirement's clause says an application + # that receives no assertion must assume the MINIMUM mechanism was used, and minting verified + # assumes the maximum. False is that minimum -- one factor proven, none asserted -- so the + # session is MFA-pending and the engine's own second factor decides the rest. + # + # This is only safe CO-LANDED with directory-account engine-factor enrollment (the same item): + # a minimum-minted directory session reaches nothing outside api/security.py's six-entry + # MFA-exempt set, so without an enrollment ceremony that accepts a directory account it is a + # lockout rather than a control. return await self._complete_ad_login( - principal, client, mfa_verified=True, seed_reauth=seed_reauth + principal, client, mfa_verified=False, seed_reauth=seed_reauth ) def _oidc_policy(self, nonce: str) -> oidc.OidcClaimPolicy: @@ -1354,10 +1365,10 @@ async def _complete_ad_login( allowed_channels=_allowed_channels(user, ad_roles), extra_permissions=ad_custom_permissions, ) - # ASVS 6.3.4: the second-factor grant is the CALLER's per-mechanism decision, not a blanket - # literal. AD simple-bind and Kerberos pass True under the owner-signed delegated-directory-MFA - # relaxation (the bind/ticket teaches the engine nothing about directory-side strength); the - # federated leg passes the engine-verified amr/acr result. See the callers for each rationale. + # ASVS 6.3.4 / 6.8.4: the second-factor grant is the CALLER's per-mechanism decision, not a + # blanket literal. Kerberos passes False -- a ticket asserts nothing about directory-side + # strength, so the engine assumes the minimum (BACKLOG #1144); the federated leg passes the + # engine-verified amr/acr result. See the callers for each rationale. token = await self._issue_session( user.id, client, @@ -1376,7 +1387,16 @@ async def _complete_ad_login( await self._audit( "auth.login_success", actor=user.username, detail=_json(detail), client=client ) - return LoginOutcome(ok=True, token=token, identity=identity) + # Ask the GATE, not the grant (BACKLOG #1144). A leg that granted nothing has not necessarily + # left a debt: with require_mfa off and no factor enrolled the shared rule still admits the + # session, so `not mfa_verified` would over-report and prompt for a factor the caller does not + # owe. One extra read on a rare path buys a single source for the answer. + return LoginOutcome( + ok=True, + token=token, + identity=identity, + mfa_required=not await self.mfa_satisfied(token), + ) async def _sync_ad_channel_scope( self, user: UserRecord, roles: frozenset[Role], groups: Iterable[str] @@ -1687,9 +1707,10 @@ async def _issue_session( seed_reauth=mfa_verified if seed_reauth is None else seed_reauth, ) if mfa_verified: - # No second factor pending (MFA not required for this user, or delegated to AD/Kerberos): - # mark the session's 2nd factor satisfied at issuance so the step-up gate never blocks it. - # An MFA-required local login leaves it NULL until POST /auth/mfa-verify (WP-14). + # No second factor pending (MFA is not required for this user, or the federated IdP + # asserted one): mark the session's 2nd factor satisfied at issuance so the step-up gate + # never blocks it. An MFA-required login leaves it NULL until POST /auth/mfa-verify + # (WP-14) -- including the Kerberos leg, which asserts nothing and mints at the minimum. await self._store.mark_session_mfa_verified(token_hash) cap = self._settings.max_sessions_per_user if cap and cap > 0: @@ -2145,7 +2166,7 @@ async def change_password( ) return [] - # --- MFA: native TOTP second factor (local accounts, WP-14, ASVS 6.3.3) -- + # --- MFA: native TOTP second factor (every account, WP-14, ASVS 6.3.3) ----- async def _second_factor_enrolled(self, user: UserRecord) -> bool: """Any second factor enrolled โ€” TOTP **or** โ‰ฅ1 WebAuthn passkey (ADR 0068 decision 5). The @@ -2162,14 +2183,12 @@ def _mfa_required_for( covers them โ€” ``every_local_account`` (default, ASVS 6.3.3) or, under ``administrators``, only the Administrator role. - **AD is an ALLOW-LIST exemption, not a denylist.** Directory MFA is delegated to the directory - (Entra Conditional Access / an MFA proxy) under the owner-signed relaxation, so an ``ad`` user - is exempt โ€” but any OTHER provider value falls through to the local rules and is REQUIRED. - Failing closed matters because :meth:`_identity_for_user` maps an unrecognized provider back to - ``LOCAL`` when building the :class:`Identity`; a denylist (``!= LOCAL``) would let such a row - present as local everywhere else while silently skipping the second factor here.""" - if user.auth_provider == AuthProvider.AD.value: - return False + **THE RULE READS NO PROVIDER (BACKLOG #1144, ASVS 6.8.4),** which is what keeps it closed + against an unrecognized value โ€” :meth:`_identity_for_user` maps one back to ``LOCAL`` when it + builds the :class:`Identity`, so a row that skipped the factor here would present as local + everywhere else. It used to open with a blanket ``auth_provider == ad -> False`` exemption; a + ticket or a bind asserts nothing about what the directory enforced, so that exempted on no + evidence, and the enrollment ceremonies now accept a directory account.""" if second_factor_enrolled: return True if not self._settings.require_mfa: @@ -2193,21 +2212,22 @@ async def mfa_satisfied(self, token: str | None) -> bool: user = await self._store.get_user(session.user_id) if user is None: return False - if user.auth_provider == AuthProvider.AD.value: - # ASVS 6.3.4 โ€” the directory leg, decided per SESSION rather than per user. Reaching here - # means the session was minted WITHOUT an engine-verified factor (:_issue_session stamps - # mfa_verified_at only when mfa_verified was True). AD simple-bind and Kerberos ALWAYS - # mint verified under the owner-signed delegated-MFA relaxation, so the ONLY way to be - # here is a FEDERATED (OIDC) session issued while [auth].oidc_require_mfa_claim was off โ€” - # i.e. the engine verified nothing about the IdP's factor strength. + if user.auth_provider == AuthProvider.AD.value and self._settings.require_mfa: + # THE DIRECTORY FLOOR, decided per SESSION rather than per user (ASVS 6.3.4 / 6.8.4). + # Reaching here means the session was minted with NO factor asserted at all -- every + # Kerberos session, and any federated one issued while oidc_require_mfa_claim was off. + # + # It decides exactly one case the shared rule below would decide differently: + # require_mfa_scope="administrators" + a non-Administrator + no factor enrolled. A LOCAL + # non-admin is satisfied there, having at least proven a password to the ENGINE; this + # session proved nothing to the engine, so the scope dial does not reach it. Every other + # combination is already produced by the shared rule, and when require_mfa is off this + # falls through to it -- an enrolled directory account satisfies the factor it enrolled. # - # Deciding it here, not in _mfa_required_for, is deliberate: that helper is keyed on the - # USER and is also consulted by mfa_status / the last-factor-delete guard, where "is this - # person exempt" is the right question. Only the session knows what was actually proven. - # Without this, making the OIDC mint conditional would move a timestamp and gate nothing. - # [auth].require_mfa remains the global off-switch so an operator who has deliberately - # opted out of the claim gate is not left without one. - return not self._settings.require_mfa + # Keyed on the SESSION rather than folded into _mfa_required_for on purpose: that helper + # answers "is this person exempt" for mfa_status and the last-factor-delete guard, and + # only the session knows what was actually proven at mint time. + return False roles = _roles_from_ids(await self._store.get_user_role_ids(user.id)) # The extra store read only executes for sessions not already MFA-verified (the # mfa_verified_at early-return above short-circuits the common case). @@ -2215,12 +2235,20 @@ async def mfa_satisfied(self, token: str | None) -> bool: return not self._mfa_required_for(user, roles, second_factor_enrolled=enrolled) async def begin_mfa_enrollment(self, identity: Identity) -> MfaEnrollment: - """Stage a fresh TOTP secret for a local user and return it + the ``otpauth://`` URI for the - QR. Not active until proven via :meth:`confirm_mfa_enrollment`. Raises :class:`ValueError` for - an AD account or when MFA is already enabled (disable it first to re-enroll).""" + """Stage a fresh TOTP secret and return it + the ``otpauth://`` URI for the QR. Not active + until proven via :meth:`confirm_mfa_enrollment`. Raises :class:`ValueError` for an unknown + account or when MFA is already enabled (disable it first to re-enroll). + + **A DIRECTORY ACCOUNT MAY ENROLL (BACKLOG #1144, ASVS 6.8.4).** This refused anything but + ``LOCAL``, which made the delegated-directory relaxation self-sealing: the engine could not + assume the minimum on a leg that asserts nothing, because assuming it locked out every + directory operator. The secret and its recovery codes are engine-held state on the engine's + own user row, which a directory account already has (``_upsert_ad_user``); nothing here reads + or writes the directory. The step-up in front of this ceremony re-proves a directory + credential by a live bind (:meth:`_reauth_ad`), so the proof is real on both providers.""" user = await self._store.get_user(identity.user_id) - if user is None or user.auth_provider != AuthProvider.LOCAL.value: - raise ValueError("only local users can enroll a TOTP authenticator") + if user is None: + raise ValueError("no such user") if user.totp_enabled: raise ValueError("MFA is already enabled; disable it before re-enrolling") secret = totp.generate_secret() @@ -2235,10 +2263,11 @@ async def confirm_mfa_enrollment( single-use recovery codes (returned **once**, plaintext, for the user to save), mark the current session MFA-verified, audit + notify. Returns the recovery codes, or ``None`` when the code was wrong or its time-step was already consumed (single-use, BACKLOG #1021). Raises - :class:`ValueError` if no enrollment is staged / the user isn't local.""" + :class:`ValueError` for an unknown account or when no enrollment is staged. Accepts a + directory account, for the reason :meth:`begin_mfa_enrollment` states.""" user = await self._store.get_user(identity.user_id) - if user is None or user.auth_provider != AuthProvider.LOCAL.value: - raise ValueError("only local users can enroll a TOTP authenticator") + if user is None: + raise ValueError("no such user") secret = await self._store.get_totp_secret(identity.user_id) if not secret: raise ValueError("no enrollment in progress") @@ -2423,12 +2452,16 @@ async def admin_reset_mfa(self, user_id: str, *, actor: str) -> None: """Admin: clear a user's MFA โ€” TOTP **and** every WebAuthn passkey (lost authenticator + no recovery path; ADR 0068 extends this to credentials) โ€” and revoke their sessions so they re-enroll. The always-available recovery for a locked-out passkey user. Raises - :class:`ValueError` for an unknown or non-local user.""" + :class:`ValueError` for an unknown user. + + **IT COVERS A DIRECTORY ACCOUNT (BACKLOG #1144).** The non-local refusal that stood here was + true while no directory account could hold an engine factor. Once one can, keeping it would + make enrollment a one-way door: a directory user who lost the authenticator would have no + recovery at all, because every route that could help stands behind the factor they lost. This + is the widest of the refusals the item names, and it is included for that reason.""" user = await self._store.get_user(user_id) if user is None: raise ValueError("no such user") - if user.auth_provider != AuthProvider.LOCAL.value: - raise ValueError("only local users have MFA to reset") await self._store.disable_totp(user_id) removed = await self._store.delete_all_webauthn_credentials(user_id) await self._store.revoke_user_sessions(user_id) @@ -2472,7 +2505,7 @@ async def mfa_status(self, identity: Identity) -> MfaStatus: webauthn_enrolled=webauthn_enrolled, ) - # --- MFA: WebAuthn passkeys second factor (local accounts, WP-14b / ADR 0068) --- + # --- MFA: WebAuthn passkeys second factor (every account, WP-14b / ADR 0068) --- def webauthn_available(self) -> bool: """Whether the optional ``[webauthn]`` extra is installed (the UI hides the passkey surface @@ -2501,11 +2534,12 @@ async def begin_webauthn_registration( """Stage a passkey registration ceremony; returns the browser creation-options JSON. The API gates this behind the password-only re-proof (``require_ui_reauth_only`` โ€” WP-14: a stolen pre-MFA cookie must never bind an attacker's passkey). Raises :class:`ValueError` - for an AD account (parity with :meth:`begin_mfa_enrollment`); a full challenge cache - raises :class:`webauthn.ChallengeCacheFullError` (cause-naming, rendered legibly).""" + for an unknown account; a full challenge cache raises + :class:`webauthn.ChallengeCacheFullError` (cause-naming, rendered legibly). Accepts a + directory account, in parity with :meth:`begin_mfa_enrollment` and for its stated reason.""" user = await self._store.get_user(identity.user_id) - if user is None or user.auth_provider != AuthProvider.LOCAL.value: - raise ValueError("only local users can enroll a passkey") + if user is None: + raise ValueError("no such user") existing = await self._store.list_webauthn_credentials(identity.user_id) challenge = webauthn.new_challenge() options = webauthn.registration_options( @@ -2533,13 +2567,14 @@ async def finish_webauthn_registration( ) -> bool: """Verify an attestation response and persist the passkey. Returns ``False`` when the response fails verification (audited โ€” parity with a wrong TOTP code); raises - :class:`ValueError` for flow errors with safe, renderable messages (AD account, bad label, - expired ceremony, duplicate label/credential). On success the enrolling session is marked - MFA-verified (exact :meth:`confirm_mfa_enrollment` parity) โ€” **no recovery codes are - minted** (ADR 0068 decision 5).""" + :class:`ValueError` for flow errors with safe, renderable messages (unknown account, bad + label, expired ceremony, duplicate label/credential). On success the enrolling session is + marked MFA-verified (exact :meth:`confirm_mfa_enrollment` parity) โ€” **no recovery codes are + minted** (ADR 0068 decision 5). Accepts a directory account, for the reason + :meth:`begin_mfa_enrollment` states.""" user = await self._store.get_user(identity.user_id) - if user is None or user.auth_provider != AuthProvider.LOCAL.value: - raise ValueError("only local users can enroll a passkey") + if user is None: + raise ValueError("no such user") label = label.strip() if not label or len(label) > self._WEBAUTHN_LABEL_MAX: raise ValueError("label must be 1-100 characters") diff --git a/messagefoundry/auth/totp.py b/messagefoundry/auth/totp.py index afe62ee8a..ba82cfffb 100644 --- a/messagefoundry/auth/totp.py +++ b/messagefoundry/auth/totp.py @@ -1,13 +1,15 @@ # SPDX-License-Identifier: AGPL-3.0-or-later # Copyright (C) 2026 MessageFoundry Organization and contributors -"""RFC 6238 TOTP โ€” the local-account second factor (WP-14, ADR 0002 ยง3). +"""RFC 6238 TOTP โ€” the engine's own second factor (WP-14, ADR 0002 ยง3). Pure standard-library crypto (``hmac`` / ``hashlib`` / ``base64`` / ``secrets``) so MFA adds **no new dependency**: a software authenticator app (Google/Microsoft Authenticator, Authy, 1Password) and the engine independently compute the same short code from a shared base32 secret plus the current -30-second time step. Used only for **local** users โ€” AD/Kerberos MFA is delegated to the directory -(see :class:`~messagefoundry.auth.service.AuthService`). This module is side-effect-free and unit- -tested against the RFC 6238 vectors; it never touches the store, the event loop, or config. +30-second time step. Available to **every** account, directory ones included โ€” a Kerberos ticket +asserts no factor strength the engine can read, so the engine asks for its own factor rather than +assuming one (BACKLOG #1144; see :class:`~messagefoundry.auth.service.AuthService`). This module is +side-effect-free and unit-tested against the RFC 6238 vectors; it never touches the store, the event +loop, or config. Security notes: diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index 0214873f9..72ee312b8 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -1794,9 +1794,10 @@ class AuthSettings(_Section): require_action_step_up: bool = True # Multi-factor authentication (WP-14, ADR 0002 ยง3; ASVS 6.3.3) โ€” a native RFC 6238 TOTP second - # factor for LOCAL accounts. AD/Kerberos MFA is delegated to the directory (Entra Conditional - # Access / an MFA proxy), so a directory login is never prompted for an engine TOTP. When - # require_mfa is on, an in-scope local account (see require_mfa_scope) MUST enroll a factor and + # factor. It covers EVERY account, directory ones included (BACKLOG #1144, ASVS 6.8.4): a ticket + # or a bind asserts nothing about what the directory enforced, so the engine grants nothing on it + # and asks for its own factor instead of exempting the leg. When require_mfa is on, an in-scope + # account (see require_mfa_scope) MUST enroll a factor and # satisfy it before its session may reach ANY authorized route โ€” MFA is an ACCESS gate, not only # a step-up gate (ASVS 6.3.3). A user who has already enrolled a factor is always required to # satisfy it, whatever the scope. @@ -1818,16 +1819,22 @@ class AuthSettings(_Section): # session, not merely step-up operations โ€” an MFA-pending session is refused with 403 + # ``X-MFA-Required: 1`` (api/security.py:require) and, in the browser, confined to /ui/mfa. require_mfa: bool = True - # WHICH local accounts an un-enrolled session's access gate covers when require_mfa is on (ASVS - # 6.3.3). ``every_local_account`` (default) means any local account must carry a second factor; + # WHICH accounts an un-enrolled session's access gate covers when require_mfa is on (ASVS 6.3.3). + # ``every_local_account`` (default) means any account must carry a second factor; # ``administrators`` is the pre-6.3.3 posture where only the Administrator role must. An account # that has ALREADY enrolled a factor is required to satisfy it under either value โ€” this dial only - # decides who must enroll in the first place. Directory (AD/Kerberos) identities are out of scope - # under either value: their MFA is delegated to the directory (owner-signed relaxation). + # decides who must enroll in the first place. # - # OPERATOR NOTE: under ``every_local_account`` a non-interactive LOCAL bearer-token service account + # THE ``every_local_account`` SPELLING IS NOW WIDER THAN ITS NAME (BACKLOG #1144). Directory + # identities used to be exempt under either value; they are not, because the directory legs assert + # no strength and the engine grants nothing on that. Renaming the Literal reaches this model, the + # CONFIGURATION.md table and the tests that pin both -- its own coherent change, not a rider on a + # security fix. THIS IS THE SINGLE PLACE that mismatch is explained; do not restate it (SDS-3.5). + # + # OPERATOR NOTE: under ``every_local_account`` a non-interactive bearer-token service account # becomes MFA-pending and cannot enroll unattended โ€” move it to mTLS (api/security.py: - # require_service_cert, which is exempt by design) or to AD, or set this to ``administrators``. + # require_service_cert, which is exempt by design) or set this to ``administrators``. Moving it to + # AD is NO LONGER an escape: a directory account is in scope like any other. require_mfa_scope: Literal["administrators", "every_local_account"] = "every_local_account" # TOTP clock-skew tolerance, in 30-second time steps, applied when verifying a submitted code # (BACKLOG #187; ASVS 6.5.5). Default 0 = STRICT: only the current 30 s step is accepted, so a @@ -4439,7 +4446,8 @@ def security_loosenings( out.append( ( "require_mfa", - "every local account is single-factor โ€” no native TOTP second factor is required", + "every account is single-factor โ€” no engine second factor is required, and a " + "directory session is admitted on a ticket that asserts no strength", ) ) elif sec.require_mfa_scope != "every_local_account": @@ -4448,8 +4456,8 @@ def security_loosenings( out.append( ( "require_mfa_scope", - "only Administrators must enroll a second factor โ€” every other local account is " - "single-factor until it opts in by enrolling", + "only Administrators must enroll a second factor โ€” every other account, local or " + "directory, is single-factor until it opts in by enrolling", ) ) if sec.allow_single_factor_admin_when_exposed: diff --git a/messagefoundry_webconsole/pages/account.py b/messagefoundry_webconsole/pages/account.py index 2573310b7..6d9842f2e 100644 --- a/messagefoundry_webconsole/pages/account.py +++ b/messagefoundry_webconsole/pages/account.py @@ -436,43 +436,43 @@ def account_page( el("p", "Roles: " + (", ".join(me.roles) or "(none)"), class_="muted"), class_="card", ) - if is_ad: - pw_section = el("p", "AD passwords are managed in Active Directory.", class_="muted") - mfa_section: Markup = el( - "p", "AD accounts use directory MFA, not an engine TOTP.", class_="muted" + # Only the password card is directory-gated: an engine password is the one credential a directory + # account does not have. The MFA and passkey cards are not (BACKLOG #1144) โ€” hiding them would + # strand the users the Kerberos leg's minimum-strength mint now confines. + pw_section = ( + el("p", "AD passwords are managed in Active Directory.", class_="muted") + if is_ad + else el("p", el("a", "Change password", href="/ui/account/password")) + ) + if mfa.enabled: + status_line = el( + "p", + f"Enabled โ€” {mfa.recovery_codes_remaining} recovery code(s) remaining.", + class_="muted", + ) + action = el( + "form", + el("button", "Disable MFA", type="submit"), + method="post", + action="/ui/account/mfa/disable", + class_="ctl", ) else: - pw_section = el("p", el("a", "Change password", href="/ui/account/password")) - if mfa.enabled: - status_line = el( - "p", - f"Enabled โ€” {mfa.recovery_codes_remaining} recovery code(s) remaining.", - class_="muted", - ) - action = el( - "form", - el("button", "Disable MFA", type="submit"), - method="post", - action="/ui/account/mfa/disable", - class_="ctl", - ) - else: - status_line = el( - "p", - "Not enrolled." - + (" This account REQUIRES MFA โ€” enroll now." if mfa.required else ""), - class_="muted", - ) - action = el( - "form", - el("button", "Enroll an authenticator", type="submit"), - method="post", - action="/ui/account/mfa/enroll", - class_="ctl", - ) - mfa_section = Markup(status_line + action) - passkey_card = Markup("") if is_ad else _passkey_card(mfa, passkeys or (), webauthn_notice) - # Active-session management is self-service for EVERY account (local + AD), unlike password/MFA. + status_line = el( + "p", + "Not enrolled." + (" This account REQUIRES MFA โ€” enroll now." if mfa.required else ""), + class_="muted", + ) + action = el( + "form", + el("button", "Enroll an authenticator", type="submit"), + method="post", + action="/ui/account/mfa/enroll", + class_="ctl", + ) + mfa_section = Markup(status_line + action) + passkey_card = _passkey_card(mfa, passkeys or (), webauthn_notice) + # Active-session management is self-service for EVERY account (local + AD), as MFA now is. sessions_card = el( "div", el("h2", "Active sessions"), diff --git a/messagefoundry_webconsole/pages/admin.py b/messagefoundry_webconsole/pages/admin.py index c956b47b5..44a22417c 100644 --- a/messagefoundry_webconsole/pages/admin.py +++ b/messagefoundry_webconsole/pages/admin.py @@ -221,6 +221,8 @@ def user_detail_page( ) danger: list[object] = [] if not is_ad: + # Reset password stays directory-gated: an engine password is the one credential a directory + # account does not have, and admin_reset_password still refuses one. danger.append( el( "form", @@ -230,15 +232,18 @@ def user_detail_page( class_="ctl", ) ) - danger.append( - el( - "form", - el("button", "Reset MFA", type="submit"), - method="post", - action=f"/ui/users/{user.id}/reset-mfa", - class_="ctl", - ) + # Reset MFA is NOT gated (BACKLOG #1144). A directory account can hold an engine factor now, so + # this is its only recovery from a lost authenticator -- every route that could otherwise help + # stands behind the factor it lost. Hiding the button would make enrollment a one-way door. + danger.append( + el( + "form", + el("button", "Reset MFA", type="submit"), + method="post", + action=f"/ui/users/{user.id}/reset-mfa", + class_="ctl", ) + ) danger.append( el( "form", diff --git a/packaging/messagefoundry-webconsole/tests/test_webui.py b/packaging/messagefoundry-webconsole/tests/test_webui.py index 9f024fc40..844cadc96 100644 --- a/packaging/messagefoundry-webconsole/tests/test_webui.py +++ b/packaging/messagefoundry-webconsole/tests/test_webui.py @@ -2760,6 +2760,9 @@ async def test_error_banner_escapes_hostile_input(engine: Engine) -> None: async def test_ad_user_carveouts_on_ui_surface(engine: Engine) -> None: # An AD account: roles come from the AD-group map and the password from the directory โ€” the # detail page hides those forms, and a forged direct POST is refused by the handler guards. + # Reset MFA is NOT among them since BACKLOG #1144: a directory account can hold an engine factor, + # so the button is its only recovery from a lost authenticator and hiding it would make + # enrollment a one-way door. That is the half of the carve-out set this test now pins OPEN. service = await _service(engine) await service.store.create_user( user_id="ad-user-1", username="aduser", auth_provider="ad", display_name="AD User" @@ -2770,6 +2773,7 @@ async def test_ad_user_carveouts_on_ui_surface(engine: Engine) -> None: assert "AD users get roles from the AD-group map" in detail.text assert 'action="/ui/users/ad-user-1/roles"' not in detail.text assert 'action="/ui/users/ad-user-1/reset-password"' not in detail.text + assert 'action="/ui/users/ad-user-1/reset-mfa"' in detail.text r = await _post_pairs(c, "/ui/users/ad-user-1/roles", [("roles", "viewer")]) assert r.status_code == 400 and "AD-group map" in r.text assert await service.store.get_user_role_ids("ad-user-1") == [] @@ -4150,6 +4154,14 @@ async def test_sso_success_mints_one_cookie_session( assert len(sessions) == 1 # ONE session per navigation into the route r = await c.get("/ui/account") assert r.status_code == 200 and "Signed in as jdoe (ad)" in r.text + # BACKLOG #1144: the enrolment surface is REACHABLE for a directory account. The Kerberos leg + # mints MFA-pending, and /ui/account is MFA-pending-exempt, so this page is where such a user + # lands and where the confinement has to be survivable. The page used to say "AD accounts use + # directory MFA, not an engine TOTP" and offer nothing. + assert 'action="/ui/account/mfa/enroll"' in r.text + assert "Add a passkey" in r.text + # Still directory-gated, and correctly so: there is no engine password to change. + assert 'href="/ui/account/password"' not in r.text async def test_sso_session_not_reauth_seeded( @@ -4169,7 +4181,12 @@ async def test_sso_session_not_reauth_seeded( jdoe = await service.store.get_user_by_username("jdoe") sessions = await service.store.list_sessions(jdoe.id) assert sessions[0].reauth_at is None # seed_reauth=False (ADR 0068 ยง9) - assert sessions[0].mfa_verified_at is not None # directory-delegated MFA + # BACKLOG #1144: the ticket asserts no factor strength the engine can read, so the leg grants + # nothing and the session is born MFA-pending. This assertion used to read `is not None`, + # under the delegated-directory relaxation that is now retired. The two stamps are + # INDEPENDENT and both must be pinned: reauth_at is the step-up window (seeding is ADR 0068 + # ยง9 and unchanged here), mfa_verified_at is the second-factor grant. + assert sessions[0].mfa_verified_at is None # The directory-password step-up completes at /ui/reauth (auth.reauth live-rebinds AD). r = await c.post( @@ -4190,6 +4207,8 @@ async def test_sso_session_not_reauth_seeded( assert out.ok and out.token is not None session = await service.store.get_session(hash_token(out.token)) assert session is not None and session.reauth_at is not None + # The grant is the same on BOTH Kerberos legs -- only the step-up seeding differs (BACKLOG #1144). + assert session.mfa_verified_at is None async def test_sso_cross_site_hygiene(engine: Engine, monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/_mfa_grant.py b/tests/_mfa_grant.py new file mode 100644 index 000000000..c67bcde04 --- /dev/null +++ b/tests/_mfa_grant.py @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Read a login leg's ``mfa_verified`` grant off its SOURCE (ASVS 6.3.4 / 6.8.4). + +Two suites assert on the same fact from different angles -- ``tests/test_mfa_access_gate.py`` pins the +behaviour and ``tests/test_docs_security_pathways.py`` pins the disclosure in ``docs/SECURITY.md`` +against it -- so the reading lives here rather than being written twice. Both copies existed briefly +and had to be flipped together when BACKLOG #1144 retired the delegated-directory relaxation; the next +change to a grant would have been free to leave one file green and the other red. + +**Why source and not behaviour.** The grant is a literal at one call site. Driving a live SPNEGO +exchange to observe it would let a fixture that happens to take an early-return path satisfy the +assertion without the grant ever being evaluated -- an instrument answering the adjacent question. +Callers pair this with a behavioural test rather than relying on it alone. +""" + +from __future__ import annotations + +import ast +import inspect +import textwrap +from typing import Any + +__all__ = ["mfa_grant_values"] + + +def mfa_grant_values(func: Any) -> list[ast.expr]: + """Every expression passed as ``mfa_verified=`` inside ``func``'s source, in source order. + + An empty list means the seam moved -- the leg passes no such keyword at all -- which callers must + treat as a failure rather than as "no constant found", or the assertion passes vacuously. + """ + tree = ast.parse(textwrap.dedent(inspect.getsource(func))) + return [ + kw.value + for node in ast.walk(tree) + if isinstance(node, ast.Call) + for kw in node.keywords + if kw.arg == "mfa_verified" + ] diff --git a/tests/test_cli.py b/tests/test_cli.py index b265a8996..db4f50aaf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -966,9 +966,9 @@ def test_serve_exposed_prod_phi_single_factor_ack_starts_with_warning( def test_serve_refuses_exposed_without_mfa_even_with_ad_enabled( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - # The gate keys on require_mfa only โ€” AD/Kerberos MFA is delegated to the directory โ€” so an - # AD-enabled prod exposed bind with require_mfa off is STILL refused, pinning the error text's - # "safe even on an AD-only deployment (it gates only local Administrator accounts)". + # The gate keys on require_mfa only, so an AD-enabled prod exposed bind with require_mfa off is + # STILL refused. Since BACKLOG #1144 the error text no longer promises the knob is a no-op for + # directory users: turning it on binds them too, each enrolling an engine factor. monkeypatch.chdir(tmp_path) monkeypatch.setenv("MEFOR_STORE_ENCRYPTION_KEY", "x" * 44) monkeypatch.setenv("MEFOR_AUTH_AD_BIND_PASSWORD", "s3cret-pw") @@ -983,7 +983,7 @@ def test_serve_refuses_exposed_without_mfa_even_with_ad_enabled( '[api]\ntls_terminated_upstream = true\ntrusted_proxies = ["10.0.0.1"]\n' 'proxy_intra_service_auth = "network"\nproxy_tls_min_version = "1.2"\n' # Opt out of the BACKLOG #187 secure default so the single-factor-at-exposure gate fires even - # on an AD-enabled bind (the gate keys on require_mfa only; AD MFA is delegated to the directory). + # on an AD-enabled bind (the gate keys on require_mfa only, whatever the providers in play). "[auth]\nad_enabled = true\n" 'ad_server = "ldaps://dc1.example.com:636"\n' 'ad_user_search_base = "ou=users,dc=example,dc=com"\n' diff --git a/tests/test_docs_security_pathways.py b/tests/test_docs_security_pathways.py index ca90c59b6..e2500f941 100644 --- a/tests/test_docs_security_pathways.py +++ b/tests/test_docs_security_pathways.py @@ -17,11 +17,11 @@ import ast import inspect -import textwrap from pathlib import Path from types import SimpleNamespace import pytest +from _mfa_grant import mfa_grant_values from pydantic import BaseModel from messagefoundry.api import security as api_security @@ -418,65 +418,61 @@ def test_local_row_scopes_the_second_factor_to_step_up_and_administrator() -> No ) -def test_the_delegated_row_discloses_the_unconditional_mfa_satisfied_grant() -> None: - """The delegated directory pathway is the dominant one in the scored posture, so its MFA truth - belongs in the TABLE. +def test_the_directory_rows_disclose_what_each_leg_actually_grants() -> None: + """The directory pathways are the dominant ones in the scored posture, so their MFA truth belongs + in the TABLE. Since ASVS 6.3.4 the grant is a per-mechanism ARGUMENT rather than a literal inside - ``_complete_ad_login``, so this is pinned at the call sites: the delegated leg still passes a hard - ``True`` (the owner-signed relaxation โ€” a directory sign-in clears every engine MFA gate with zero - engine-verified evidence, which is what its row must disclose), while the federated leg must NOT, - because its grant is derived from ``oidc_require_mfa_claim``. Asserting both halves keeps the two - legs from silently converging in either direction. - - RETIREMENT NOTE (BACKLOG #1137): the hard ``True`` used to be read off ``_login_ad``. That leg is - gone, so the assertion follows the FACT to the caller that still makes the grant โ€” Kerberos โ€” - rather than being dropped. The disclosure did not change; only which pathway carries it did. + ``_complete_ad_login``, so this is pinned at the call sites: Kerberos passes a hard ``False`` (it + receives no assertion the engine can read, so it grants nothing โ€” BACKLOG #1144), while the + federated leg must pass no constant at all, because its grant is derived from + ``oidc_require_mfa_claim``. Asserting both halves keeps the two legs from silently converging in + either direction. + + TWO RETIREMENT NOTES. BACKLOG #1137: the grant used to be read off ``_login_ad``; that leg is + gone, so the assertion follows the fact to the caller that still makes it. BACKLOG #1144: the + grant this test pinned was a hard ``True`` under the owner-signed delegated-directory relaxation. + That relaxation is retired, so the polarity here is inverted rather than the test dropped โ€” + the table must disclose the current grant, whichever way it points. """ - def _mfa_grant(func: object) -> list[ast.expr]: - tree = ast.parse(textwrap.dedent(inspect.getsource(func))) # type: ignore[arg-type] - return [ - kw.value - for node in ast.walk(tree) - if isinstance(node, ast.Call) - for kw in node.keywords - if kw.arg == "mfa_verified" - ] - - bind_grant = _mfa_grant(AuthService.authenticate_kerberos) - assert bind_grant and all( - isinstance(v, ast.Constant) and v.value is True for v in bind_grant + kerberos_grant = mfa_grant_values(AuthService.authenticate_kerberos) + assert kerberos_grant and all( + isinstance(v, ast.Constant) and v.value is False for v in kerberos_grant ), ( - "the Kerberos leg no longer mints sessions mfa_verified=True under the signed relaxation; the " - "Kerberos rows' disclosure is stale โ€” re-derive it." + "the Kerberos leg mints sessions mfa_verified=True again; the Kerberos rows say it grants " + "nothing on an unreadable assertion โ€” re-derive the disclosure." ) - oidc_grant = _mfa_grant(AuthService.authenticate_oidc) + oidc_grant = mfa_grant_values(AuthService.authenticate_oidc) assert oidc_grant and not any(isinstance(v, ast.Constant) for v in oidc_grant), ( "the OIDC leg passes a CONSTANT mfa_verified; 6.3.4 requires it to be derived from " "[auth].oidc_require_mfa_claim, and the OIDC row claims the engine verifies it." ) factor = next(r for r in _primary_table()[1:] if r[0].startswith("**Kerberos"))[1] - for token in ("MFA-satisfied", "unverifiable"): + for token in ("MFA-pending", "engine second factor"): assert token in factor, ( - f"the Kerberos Factor cell must state {token!r}: the engine grants MFA satisfaction with " - "no evidence, which is a comparative-strength fact, not a footnote." + f"the Kerberos Factor cell must state {token!r}: what the engine does with a ticket that " + "asserts nothing is a comparative-strength fact, not a footnote." ) companion = next( t for t in _tables(_section()) if t[0][:2] == ["Pathway", "Phishing resistance"] ) mfa_col = companion[0].index("MFA support") kerb_mfa = next(r for r in companion[1:] if r[0].startswith("**Kerberos"))[mfa_col] - assert "unverifiable" in kerb_mfa, ( - "the companion Kerberos row reads as an enforcement claim ('delegated to the directory'); it " - "must say the grant is unconditional and unverifiable at the engine." + assert "engine" in kerb_mfa, ( + "the companion Kerberos row reads as a delegation claim; it must say the factor is an ENGINE " + "factor, because nothing the ticket asserts reaches the engine." ) block = _section() marker = "ASVS 6.1.3" paragraph = " ".join(block[block.index(marker) :].split()) - assert "same PHI surface" in paragraph, ( - "the 6.1.3 paragraph must state the consequence: every directory pathway satisfies the " - "engine's MFA gates without an engine-verified factor." + assert ( + "No pathway grants MFA satisfaction on an assertion the engine cannot read" in paragraph + ), ( + "the paragraph after the 6.1.3 block must state the current consequence. It used to assert " + "the opposite โ€” that a domain ticket reaches the same PHI surface as a passkey-backed local " + "Administrator โ€” and that sentence is now a recorded retraction, not the live disclosure, so " + "matching on it would answer the adjacent question (SDS-3.8)." ) diff --git a/tests/test_mfa.py b/tests/test_mfa.py index be01edf66..29d896563 100644 --- a/tests/test_mfa.py +++ b/tests/test_mfa.py @@ -2,10 +2,11 @@ # Copyright (C) 2026 MessageFoundry Organization and contributors """AuthService-level MFA (TOTP) tests (WP-14, ASVS 6.3.3). -Covers the full second-factor lifecycle on local accounts โ€” enrollment โ†’ confirm โ†’ recovery codes, -the step-up MFA gate, the ``require_mfa`` administrator enforcement, recovery-code single-use, and -disable/admin-reset โ€” plus the AD/Kerberos **delegation** guarantee (a directory login is never -prompted for an engine TOTP and is MFA-satisfied at issuance). +Covers the full second-factor lifecycle โ€” enrollment to confirm to recovery codes, the step-up MFA +gate, the ``require_mfa`` administrator enforcement, recovery-code single-use, and +disable/admin-reset โ€” on a **local and a directory** account alike. The AD/Kerberos delegation +guarantee this file used to pin is retired (BACKLOG #1144): a directory sign-in no longer clears the +engine's MFA gates on an assertion the engine never receives. """ from __future__ import annotations @@ -268,7 +269,18 @@ async def test_disable_and_admin_reset_clear_mfa(monkeypatch: pytest.MonkeyPatch await store.close() -async def test_ad_login_is_mfa_satisfied_by_delegation() -> None: +async def test_a_directory_account_enrolls_and_satisfies_an_engine_factor( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """RED when: ``_mfa_required_for`` re-adds a provider exemption, or an enrollment ceremony + re-adds a non-local refusal. + + This used to be ``test_ad_login_is_mfa_satisfied_by_delegation`` and asserted the reverse + (BACKLOG #1144): a directory sign-in cleared every engine MFA gate on an assertion the engine + never received. The two halves are one change โ€” the exemption can only go once the account has a + factor it can enrol โ€” so this walks the whole path: mint, find the session unsatisfied, enrol, + verify, find it satisfied. + """ store = await _store() try: principal = AdPrincipal( @@ -287,7 +299,7 @@ def resolve_principal(self, username: str) -> AdPrincipal | None: return principal if username == "jdoe" else None settings = AuthSettings( - require_mfa=True, # even with MFA required + an admin role, AD MFA is delegated + require_mfa=True, # MFA required + an admin role: the directory earns no exemption ad_enabled=True, ad_server="ldaps://x", ad_user_search_base="DC=x", @@ -298,13 +310,34 @@ def resolve_principal(self, username: str) -> AdPrincipal | None: await service.initialize() await service.set_ad_group_map([("CN=MF-Admins,DC=x", "administrator")], actor="admin") - # The subject is DELEGATED MFA, not the login mechanism. The simple-bind pathway is retired - # (BACKLOG #1137), so this mints the session through _complete_ad_login -- the shared tail - # where mfa_verified is stamped, reached identically by Kerberos and OIDC. - out = await service._complete_ad_login(principal, None, mfa_verified=True) - assert out.ok and out.token is not None - assert out.mfa_required is False # delegated to the directory, never an engine TOTP + # The subject is the engine factor on a DIRECTORY account, not the login mechanism. The + # simple-bind pathway is retired (BACKLOG #1137), so this mints through _complete_ad_login -- + # the shared tail where mfa_verified is stamped, reached identically by Kerberos and OIDC. + # False is what the Kerberos leg passes: the ticket asserts nothing the engine can read. + out = await service._complete_ad_login(principal, None, mfa_verified=False) + assert out.ok and out.token is not None and out.identity is not None + assert await service.mfa_satisfied(out.token) is False + + # The ceremony accepts the directory account -- the half that makes the mint above safe. + t0 = 1_700_000_000.0 + pin_totp_clock(monkeypatch, t0) + enroll = await service.begin_mfa_enrollment(out.identity) + codes = await service.confirm_mfa_enrollment( + out.identity, totp.totp(enroll.secret, now=t0), token=out.token + ) + assert codes # recovery codes are minted for a directory account like any other assert await service.mfa_satisfied(out.token) is True + + # A NEW directory session is still unsatisfied: the factor is now enrolled, so it is required + # under either require_mfa_scope value, and only proving it lifts the gate. The later code + # must live in a HIGHER step -- enrollment consumed t0's (BACKLOG #1021). + second = await service._complete_ad_login(principal, None, mfa_verified=False) + assert second.ok and second.token is not None + assert await service.mfa_satisfied(second.token) is False + t1 = t0 + totp.DEFAULT_PERIOD + pin_totp_clock(monkeypatch, t1) + assert await service.verify_mfa(second.token, totp.totp(enroll.secret, now=t1)) is True + assert await service.mfa_satisfied(second.token) is True finally: await store.close() diff --git a/tests/test_mfa_access_gate.py b/tests/test_mfa_access_gate.py index bba15d82a..26b289bfc 100644 --- a/tests/test_mfa_access_gate.py +++ b/tests/test_mfa_access_gate.py @@ -4,7 +4,9 @@ 6.3.3 moved the second factor from the step-up boundary to the front door: an MFA-pending session is refused on every authorized route, not merely on sensitive ones. 6.3.4 stopped minting every directory -session MFA-verified regardless of what the directory actually enforced. +session MFA-verified regardless of what the directory actually enforced. 6.8.4 finished that: the +Kerberos leg asserts nothing the engine can read, so it grants nothing, and a directory account enrols +an engine factor like any other (BACKLOG #1144). Each test names the mutation that must turn it RED, because several of these would pass either way if written carelessly โ€” a session's ``mfa_verified_at`` column can be correct while nothing gates on it. @@ -12,11 +14,13 @@ from __future__ import annotations +import ast from collections.abc import AsyncIterator from pathlib import Path import httpx import pytest +from _mfa_grant import mfa_grant_values from messagefoundry.api import create_app from messagefoundry.auth import Role, totp @@ -306,11 +310,11 @@ async def test_a_federated_session_minted_unverified_is_not_mfa_satisfied( async def test_a_directory_session_minted_verified_is_satisfied(engine: Engine) -> None: - """RED when: the AD branch starts refusing unconditionally. + """RED when: the directory floor starts refusing unconditionally. - The delegated-directory relaxation must survive: AD simple-bind and Kerberos mint verified, and - those sessions have to keep working or every directory operator is locked out with no enrollment - path. This is the guard against over-correcting 6.3.4 into a mass lockout. + A verified mint has to keep working: it is what the federated leg produces once the claim gate has + checked the token's ``amr``/``acr``, and refusing it would strand every directory operator whose + IdP did assert a factor. This is the guard against over-correcting into a mass lockout. """ service = await _service(engine) out = await service._complete_ad_login(_principal("aduser2"), None, mfa_verified=True) @@ -321,9 +325,11 @@ async def test_a_directory_session_minted_verified_is_satisfied(engine: Engine) async def test_require_mfa_off_is_still_a_global_escape_for_the_directory_leg( engine: Engine, ) -> None: - """RED when: the AD branch stops honouring require_mfa. + """RED when: the directory floor stops honouring require_mfa. An operator who deliberately turned the claim gate off must not be left without an off-switch. + With the knob off and no factor enrolled, the floor falls through to the shared per-user rule, + which also says not required โ€” so an un-enrolled directory session is satisfied. """ service = await _service( engine, AuthSettings(login_rate_limit_enabled=False, require_mfa=False) @@ -333,21 +339,96 @@ async def test_require_mfa_off_is_still_a_global_escape_for_the_directory_leg( assert await service.mfa_satisfied(out.token) is True -def test_an_unknown_auth_provider_fails_closed(engine: Engine) -> None: - """RED when: _mfa_required_for reverts to the ``!= LOCAL`` denylist. +def test_the_per_user_rule_reads_no_provider_at_all(engine: Engine) -> None: + """RED when: ``_mfa_required_for`` re-adds ANY provider branch, allow-list or deny-list. - ``_identity_for_user`` maps an unrecognized provider back to LOCAL when building the Identity, so - a denylist would let such a row present as local everywhere else while silently skipping the - second factor here. The allowlist (``== AD``) makes it required instead. + Two failures the shape guards against at once. A ``!= LOCAL`` denylist would exempt an + unrecognized provider, which ``_identity_for_user`` maps back to LOCAL when it builds the + Identity โ€” so such a row would present as local everywhere else while silently skipping the second + factor here. An ``== AD`` allow-list was the shipped code until BACKLOG #1144 and exempted every + directory account on a delegation the directory never asserted. Reading no provider closes both. """ from types import SimpleNamespace service = AuthService.__new__(AuthService) service._settings = AuthSettings() # type: ignore[attr-defined] - rogue = SimpleNamespace(auth_provider="saml-from-the-future") - assert ( - service._mfa_required_for( # type: ignore[arg-type] - rogue, frozenset({Role.VIEWER}), second_factor_enrolled=False - ) - is True + for provider in ("saml-from-the-future", "ad", "local"): + rogue = SimpleNamespace(auth_provider=provider) + assert ( + service._mfa_required_for( # type: ignore[arg-type] + rogue, frozenset({Role.VIEWER}), second_factor_enrolled=False + ) + is True + ), f"provider {provider!r} must not change the answer" + + +# --- 6.8.4: the directory legs assert nothing, so the engine assumes nothing -- + + +def test_the_kerberos_leg_mints_at_the_minimum() -> None: + """RED when: ``authenticate_kerberos`` goes back to passing ``mfa_verified=True``. + + A Kerberos service ticket carries no factor-strength assertion ``pyspnego`` surfaces, so the leg + must grant nothing. The source read lives in ``tests/_mfa_grant.py``, shared with the doc-drift + guard that pins ``docs/SECURITY.md``'s Kerberos rows against this same fact. The paired + behavioural test is + ``test_a_directory_session_minted_at_the_minimum_is_confined_until_it_enrolls``. + """ + grants = mfa_grant_values(AuthService.authenticate_kerberos) + assert grants, "the Kerberos leg passes no mfa_verified at all โ€” the seam moved" + assert all(isinstance(v, ast.Constant) and v.value is False for v in grants), ( + "the Kerberos leg mints MFA-satisfied again; docs/SECURITY.md's Kerberos rows say it does " + "not, and every engine MFA gate would clear on evidence the engine never received." + ) + + +async def test_a_directory_session_minted_at_the_minimum_is_confined_until_it_enrolls( + engine: Engine, +) -> None: + """RED when: minting at the minimum ships WITHOUT directory-account factor enrollment. + + This is the co-landing invariant as one test. A minimum-minted directory session must be refused + on an ordinary authorized route (otherwise the mint gates nothing) AND must be able to reach the + enrollment ceremony and satisfy the gate (otherwise the mint is a lockout, not a control). Either + half alone passes trivially; asserting both is what pins them together. + """ + service = await _service(engine) + out = await service._complete_ad_login(_principal("aduser4"), None, mfa_verified=False) + assert out.ok and out.token is not None and out.identity is not None + async with _client(engine, service) as c: + headers = {"Authorization": f"Bearer {out.token}"} + refused = await c.get("/connections", headers=headers) + assert refused.status_code == 403 + assert refused.headers.get("X-MFA-Required") == "1" + + # The confinement is survivable: the ceremony accepts the directory account. + enroll = await service.begin_mfa_enrollment(out.identity) + codes = await service.confirm_mfa_enrollment( + out.identity, totp.totp(enroll.secret), token=out.token ) + assert codes is not None + assert await service.mfa_satisfied(out.token) is True + + +async def test_the_directory_login_outcome_reports_the_debt_it_created(engine: Engine) -> None: + """RED when: the directory tail returns ``mfa_required=False`` for a session the gate will refuse. + + A JSON client (``POST /auth/negotiate``) is told whether it still owes a factor. Before BACKLOG + #1144 a directory session was minted satisfied and the answer was always False, so the tail + hardcoded it. Minting at the minimum without moving this would tell a client no factor is needed + and then refuse its very next call with ``X-MFA-Required: 1`` and no earlier signal. + + The second half pins the flag to the GATE rather than to the grant: with ``require_mfa`` off, an + un-enrolled directory session grants nothing yet owes nothing, and reporting True there would + prompt for a factor the caller does not need. + """ + service = await _service(engine) + owes = await service._complete_ad_login(_principal("aduser5"), None, mfa_verified=False) + assert owes.ok and owes.mfa_required is True + + granted = await service._complete_ad_login(_principal("aduser6"), None, mfa_verified=True) + assert granted.ok and granted.mfa_required is False + + lax = await _service(engine, AuthSettings(login_rate_limit_enabled=False, require_mfa=False)) + off = await lax._complete_ad_login(_principal("aduser7"), None, mfa_verified=False) + assert off.ok and off.mfa_required is False diff --git a/tests/test_webauthn.py b/tests/test_webauthn.py index 9b94e1d72..feb4ba7c6 100644 --- a/tests/test_webauthn.py +++ b/tests/test_webauthn.py @@ -305,7 +305,15 @@ async def test_challenge_single_use_ttl_and_per_user_bound() -> None: # --- lifecycle + factor generalization ------------------------------------------- -async def test_ad_user_cannot_enroll_passkey() -> None: +async def test_a_directory_account_can_enroll_a_passkey() -> None: + """RED when: either passkey ceremony re-adds a ``!= AuthProvider.LOCAL`` refusal. + + This test used to assert the opposite (BACKLOG #1144). The refusal was true while the directory + legs minted MFA-satisfied, and it is what made that relaxation self-sealing: the engine could not + stop granting satisfaction on an assertion it cannot read, because a directory account had no + engine factor to fall back on. Both halves move together or the change is a lockout, so this + exercises the WHOLE ceremony rather than the guard alone. + """ from messagefoundry.auth.ldap import AdPrincipal store = await MessageStore.open(":memory:") @@ -319,14 +327,13 @@ async def test_ad_user_cannot_enroll_passkey() -> None: dn="CN=aduser,DC=x", groups=frozenset(), ) - # mfa_verified is the per-mechanism grant (ASVS 6.3.4); the simple-bind leg passes True under - # the signed delegated-directory relaxation, which is what this AD principal stands in for. + # mfa_verified is the per-mechanism grant (ASVS 6.3.4). Passing True here stands in for a + # federated sign-in that DID carry a verified claim; what is under test is the enrollment + # ceremony, and a satisfied session keeps the fixture free of the MFA gate. out = await service._complete_ad_login(principal, None, mfa_verified=True) assert out.ok and out.identity is not None and out.token is not None - with pytest.raises(ValueError, match="only local users"): - await service.begin_webauthn_registration( - out.identity, token=out.token, rp_id=RP, rp_name="MessageFoundry" - ) + await _enroll(service, out.identity, out.token, label="ad-key") + assert await service.store.has_webauthn_credentials(out.identity.user_id) is True finally: await store.close()