diff --git a/changelog.d/2434.security.md b/changelog.d/2434.security.md index 0a3e06797..fc09433c7 100644 --- a/changelog.d/2434.security.md +++ b/changelog.d/2434.security.md @@ -3,7 +3,9 @@ directory row with no `objectGUID` and no federated binding is now judged without a lookup: it gets no roles written, and its sessions end at the strike threshold with the reason `directory_object_id_missing`, as an absent account's do. It never moves the ADR 0195 hold. No - sign-in or step-up admits such a row, so a session it holds is anomalous. + sign-in or step-up admits such a row, so a session it holds is anomalous. The count on the + outage's `auth.ad_reconcile_skipped` audit row is now the `asked` field, the lookups the pass + sent, and it leaves such a row out. It was the `probed` field. - **An empty password on the AD step-up re-bind is no longer counted as a wrong guess.** It is refused before the directory is asked, charged to neither the account lockout nor the session's re-proof budget, and audited on `auth.reauth` as `empty_password`. The caller sees a refused diff --git a/changelog.d/2610.security.md b/changelog.d/2610.security.md index af9dcb5c8..b8df3fa63 100644 --- a/changelog.d/2610.security.md +++ b/changelog.d/2610.security.md @@ -6,4 +6,8 @@ `user:`, with `via` naming the route and `granted_by` the fixed marker ``. A sign-in refused after the role was written still raises it, since a later sign-in gains nothing to page on. A sign-in that gains only other roles, or by an account that already held - Administrator, raises nothing. (`vault BACKLOG #2610`) + Administrator, raises nothing. At least two cases still raise no alert. A sign-in that fails with + a server error after the role write loses the page, because the retry gains nothing. And an + account that never signs in again and holds no live session raises nothing, because the engine + never sees the grant. A directory reconcile pass that applies the grant first raises the alert + itself (vault BACKLOG #3141). See docs/SECURITY.md, *A directory grant*. (`vault BACKLOG #2610`) diff --git a/changelog.d/2950-sign-in-timeout.changed.md b/changelog.d/2950-sign-in-timeout.changed.md new file mode 100644 index 000000000..011693e04 --- /dev/null +++ b/changelog.d/2950-sign-in-timeout.changed.md @@ -0,0 +1,8 @@ +- **The API client waits longer for a sign-in's reply.** `EngineClient.login`, `reauth`, + `verify_mfa` and `confirm_mfa` now wait up to 30 seconds for the engine's reply, matching the + IDE's sign-in timeout, instead of the client's 5 second default. Each can wait in the engine's + per-account queue, and a request the client gave up on could still end or rotate the session it + held, leaving the client signed out (and, for `confirm_mfa`, without its recovery codes). Only the + read phase is raised: an unreachable engine is still reported within the client's own timeout. + The longer wait makes a lost reply rarer, not impossible, because the queue has no bound. Every + other request keeps the client's timeout. (`vault BACKLOG #2950`) diff --git a/changelog.d/3029.security.md b/changelog.d/3029.security.md index 5c17c92e4..27dc3f97e 100644 --- a/changelog.d/3029.security.md +++ b/changelog.d/3029.security.md @@ -7,5 +7,6 @@ `auth.logout` rows carry `detail.outcome`, `revoked` or `nothing_to_revoke`, and a console sign-out's row names the owner of the session it ended instead of a NULL actor. Both sign-out routes still answer the same either way. A self-revoke that - ends nothing answers 404 and writes no row. A sign-out or self-revoke that loses that race still + ends nothing answers 404 and writes no `auth.session_revoked` row, only the + `auth.session_revoke_missed` row an unknown id writes (vault BACKLOG #3260). A sign-out or self-revoke that loses that race still leaves the session live under its new token; ending it there is not attempted. (`BACKLOG #3029`) diff --git a/changelog.d/3141.security.md b/changelog.d/3141.security.md new file mode 100644 index 000000000..8ba45684c --- /dev/null +++ b/changelog.d/3141.security.md @@ -0,0 +1,10 @@ +- **A directory reconcile pass that newly grants Administrator now raises the + `administrator_granted` operator alert.** The session reconciler re-diffs a signed-in directory + account's roles and writes them. When that write gave the account Administrator, it raised only + `ad_session_revoked`, which does not name the role, and the account's next sign-in then gained + nothing, so no `administrator_granted` alert ever fired. The pass now raises it right after the + role write, keyed `user:`, with `via` set to `directory_reconcile` and `granted_by` the + fixed marker ``. An account that already held Administrator raises nothing. A pass the + mass-revoke breaker aborts writes no roles, so it raises nothing either. Like every + `administrator_granted` alert, it clears the account's `no_enabled_administrator` cooldown in the + engine process that raises it. (`vault BACKLOG #3141`) diff --git a/changelog.d/3145-oidc-step-up-follow-ups.security.md b/changelog.d/3145-oidc-step-up-follow-ups.security.md new file mode 100644 index 000000000..c40a970b0 --- /dev/null +++ b/changelog.d/3145-oidc-step-up-follow-ups.security.md @@ -0,0 +1,8 @@ +- **A refused federated step-up records where it started.** An `auth.reauth` row for a step-up + refused after its flow was redeemed now carries `start_client` and `client_moved`, as the row of + a completed step-up does. Both come from the flow the start leg staged, never from the token. A + refusal before the `state` check carries neither. The refusal a step-up gate sends an `oidc` + session now says to repeat the action in the web console, because a bare `/ui/reauth` names no + action and returns to `/ui`. A store fault while that refusal reads the session's sign-in + mechanism now keeps the `403`, with the password wording, rather than answering `500`. See + [SECURITY.md](../docs/SECURITY.md). (`vault BACKLOG #3145`) diff --git a/changelog.d/3170.security.md b/changelog.d/3170.security.md new file mode 100644 index 000000000..900837501 --- /dev/null +++ b/changelog.d/3170.security.md @@ -0,0 +1,11 @@ +- **The session reconciler counts a referral over the lookups it sent, and strikes an id-less row + on every pass.** A directory row with no `objectGUID` is judged without a lookup, so no referral + can answer it. The referral's `auth.ad_reconcile_referred` audit row now carries the `asked` + field in place of `probed`, and that row, the referral's latched message and its + `ad_reconcile_aborted` alert count only the lookups the pass sent. The breaker's own alert still + counts every account it judged. Such a row is also struck now on a pass that is a directory + outage or aborted on referrals or probe faults, as on any other pass. Such a pass still revokes + nothing. Only a + pass that is not aborted can, once the row's strikes reach the threshold and the row is in that + pass's probe budget, so while every pass aborts its sessions last to their own expiry. Its revocation still counts toward the mass-revoke breaker, as before. + (`vault BACKLOG #3170`) diff --git a/changelog.d/3229.security.md b/changelog.d/3229.security.md index c7f2f0580..5f893b237 100644 --- a/changelog.d/3229.security.md +++ b/changelog.d/3229.security.md @@ -7,8 +7,8 @@ `auth.no_enabled_administrator` audit row. With no enabled local Administrator holding a usable password, `provision-admin` runs again at the host (BACKLOG #2711). An `administrator_granted` alert for an account clears that account's `no_enabled_administrator` cooldown. It clears it only in the engine process that raises the - `administrator_granted` alert. A reconciler re-grant raises no `administrator_granted` alert, so - it leaves the cooldown in place. A directory disable or deletion of the last Administrator raises - no `no_enabled_administrator` alert. For at least these cases + `administrator_granted` alert. A reconciler re-grant raises `administrator_granted` too (vault + BACKLOG #3141), so it clears the cooldown the same way. A directory disable or deletion of the + last Administrator raises no `no_enabled_administrator` alert. For at least these cases and others that raise no `no_enabled_administrator` alert, see docs/SECURITY.md, *A directory role change can remove the last Administrator, and it alerts*. (`vault BACKLOG #3229`) diff --git a/changelog.d/3260-session-revoke-miss.security.md b/changelog.d/3260-session-revoke-miss.security.md new file mode 100644 index 000000000..d5c8cc27c --- /dev/null +++ b/changelog.d/3260-session-revoke-miss.security.md @@ -0,0 +1,7 @@ +- **A self-service revoke of one session that ends nothing is now audited.** `DELETE + /me/sessions/{session_id}` and the console's revoke button answer as before when the id names no + session or another account's. Each such miss now writes one `auth.session_revoke_missed` row. It + names the actor, the caller's address and the first 12 characters of the id, and nothing else, so + the row does not tell an unknown id from another account's any more than the answer does. Before, + the miss left no row of its own on either plane. The `auth.session_revoked` row now carries the + caller's address too. See [SECURITY.md](../docs/SECURITY.md). (`vault BACKLOG #3260`) diff --git a/docs/ASVS-L2-PHASE0-CHANGES.md b/docs/ASVS-L2-PHASE0-CHANGES.md index aceef0d5d..35c24c992 100644 --- a/docs/ASVS-L2-PHASE0-CHANGES.md +++ b/docs/ASVS-L2-PHASE0-CHANGES.md @@ -468,7 +468,7 @@ is right. | Off-box log-forward client key | 4, on the same condition as the API TLS server key | Loads the combined PEM in `_build_tls_context` ([`logging_setup.py`](../messagefoundry/logging_setup.py)). There is no passphrase setting | As for the API TLS server key. The file's permissions are the key's only protection, so setting them is the site's job | | Native API client key | 4, on the same condition as the API TLS server key | Holds no copy. The client process loads it (`_build_verify_context` in [`apiclient/client.py`](../messagefoundry/apiclient/client.py)) | Every function in this row belongs to the operator of the client machine | | Non-production self-signed key | Two keys, each 4, private authentication key, like the API TLS placeholder key. The `cert self-signed` key is a placeholder a site may serve. The load-test harness pair is also test keying material under section 8.1.4 | Mints both with `make_self_signed` in [`pki.py`](../messagefoundry/pki.py), the CLI key on request. The CLI writes its key through `_write_private_key`, which refuses to overwrite an existing file. The harness writes its pair with a plain write and `chmod`, as the lifecycle row above says. Nothing is audited | The `cert self-signed` key is a placeholder key, like the API TLS placeholder key: a site may serve it until it supplies a chain. Set its cryptoperiod under section 5.3.6 item 4 through `--days`, because the engine never renews it. Replace it with an operator-supplied chain, then delete both files. The load-test harness pair is test keying material, which section 8.1.4 says shall not be used operationally, and the engine does not stop that | -| TOTP shared secret | 3, symmetric authentication key: an HMAC key shared by exactly two parties | **Pre-activation (7.1):** `begin_mfa_enrollment` in [`auth/service.py`](../messagefoundry/auth/service.py) stores a fresh staged secret and audits `auth.mfa_enroll_started`. A later call replaces it. A staged secret nobody confirms stays in the column until then, or until a disable or reset; nothing moves it on by time. **Key confirmation, then active (7.1, transition 4):** `confirm_mfa_enrollment` turns MFA on only after a live code proves the user's copy, and audits `auth.mfa_enrolled`. **Out of use:** `disable_totp` sets the live column to NULL on a disable or an administrator reset, audited as `auth.mfa_disabled` or `auth.mfa_reset`. That removes the live copy. It is not destruction to the 8.3.4 bar (point 3), because older copies can survive. **Backup:** where a DR archive carries the store, it carries the secret; the lifecycle row above says when discarding the DEK erases those copies. Table 7 allows backup of this key type, and B.3.2 (its case 3) says a key that can be re-established need not be backed up. Re-enrolment re-establishes this one. **Cryptoperiod: none.** The secret is exempt from a calendar lifetime (owner ruling 2026-09-27, BACKLOG #1931). Section 5.3.6 item 3 suggests an originator-usage period of no more than two years. So this is a recorded departure from that suggestion, not an alignment with it | Keep the user's copy in one authenticator. After an administrator reset, tell the user to delete the old entry. Section 8.3.5 says the party revoking a key shared by two shall inform the other | +| TOTP shared secret | 3, symmetric authentication key: an HMAC key shared by exactly two parties | **Pre-activation (7.1):** `begin_mfa_enrollment` in [`auth/service.py`](../messagefoundry/auth/service.py) stores a fresh staged secret and audits `auth.mfa_enroll_started`. A later call replaces it. A staged secret nobody confirms stays in the column until then, or until a disable or reset; nothing moves it on by time. **Key confirmation, then active (7.1, transition 4):** `confirm_mfa_enrollment` turns MFA on only after a live code checks out against the secret it read, and audits `auth.mfa_enrolled`. The enable is a compare-and-set on the staged secret the code proved, so a secret re-staged in between is refused rather than enabled; [`SECURITY.md`](SECURITY.md), in its TOTP paragraph, states the refusals (BACKLOG #3025; this sentence corrected under vault BACKLOG #3113, and again once #3025 merged). **Out of use:** `disable_totp` sets the live column to NULL on a disable or an administrator reset, audited as `auth.mfa_disabled` or `auth.mfa_reset`. That removes the live copy. It is not destruction to the 8.3.4 bar (point 3), because older copies can survive. **Backup:** where a DR archive carries the store, it carries the secret; the lifecycle row above says when discarding the DEK erases those copies. Table 7 allows backup of this key type, and B.3.2 (its case 3) says a key that can be re-established need not be backed up. Re-enrolment re-establishes this one. **Cryptoperiod: none.** The secret is exempt from a calendar lifetime (owner ruling 2026-09-27, BACKLOG #1931). Section 5.3.6 item 3 suggests an originator-usage period of no more than two years. So this is a recorded departure from that suggestion, not an alignment with it | Keep the user's copy in one authenticator. After an administrator reset, tell the user to delete the old entry. Section 8.3.5 says the party revoking a key shared by two shall inform the other | | Anonymizer re-identification salt | None of the 19 types in 5.1.1 fits. It keys a hash that pseudonymizes one dataset. The nearest fit is 5.1.2's *other secret information*, so this row borrows 5.3.7's rule for that and takes no cryptoperiod from 5.3.6 | Never mints or stores it. `Keyer` in [`anon/keying.py`](../messagefoundry/anon/keying.py) refuses a visibly weak salt, and the export reads it from the environment for one run | Mint it with a CSPRNG, one per dataset, and hold it with one person. Section 5.3.7 says other secret information shall not be kept longer than necessary, so its period is one dataset. Keep no backup unless the dataset must be regenerated, and destroy it when that need ends | | Sealed-cache key | 6, symmetric data-encryption key. It protects the confidentiality and integrity of decoded cache values in the engine's own memory | Mints it per process (`_Sealer` in [`store/sealed_cache.py`](../messagefoundry/store/sealed_cache.py)) and uses it only to seal and open cache values, with each value bound to its cache key as associated data. Pre-activation and active are one step, and the key is destroyed with the process, so its cryptoperiod (5.3.6) is the process lifetime. There is no backup or archive of it (Table 7 is moot) | Protect the engine process's memory: the in-use residual in `docs/PHI.md` §3 and the host requirements in §10 apply, because this key sits in the same memory as the values it seals | diff --git a/docs/PHI.md b/docs/PHI.md index 36c6d0a10..0ae730e32 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -194,7 +194,7 @@ destruction) are documented in [§3](#3-encryption-at-rest) under the matching h | `queue.handler_name` / `destination_name` / `channel_id` | all three | No — names, not bodies | No (metadata, deliberately not ciphered) | **PL-4** | The handler the transform worker runs; the destination the delivery worker drains | `n/a — not PHI` | | `messages.control_id`, `messages.message_type` | all three | Low (MSH-10/MSH-9) | **No** — plaintext by design | **PL-4** | Needed plaintext for dedup/routing/indexes (`ix_messages_control`). Covered only by the whole-DB / volume layer | `keep-forever by design` — dedup/routing keys that live and die with the message row | | `messages.origin`, `messages.origin_actor` (vault BACKLOG #2615) | all three | **No** — a fixed label and a username, never message content | **No** — plaintext by design | **PL-4** | How the row came to exist: `partner`, `operator_edit`, `operator_upload` or `reingress`, and for the two operator origins the acting user. Written at insert and never updated. Plain on purpose, because it must outlive the body and the ciphered `metadata`, which retention blanks. `NULL` on a row written before the column existed means not recorded. See [§6](#6-audit--accountability) | `keep-forever by design` — provenance that lives and dies with the message row | -| `audit_log.detail` | all three | Low — exposed IDs/counts, not bodies | **No** — plaintext by design | **PL-4** | JSON metadata about PHI *access* and other actions, not the PHI itself. Its writers store at least filter shapes, counts and ids. Some also store more: an edit-resend row holds **keyed** HMAC-SHA256 digests of the original and the edited body, and an upload-inject row one of the injected body. Without the store key a digest cannot be recovered or tested against a guess ([§6](#6-audit--accountability), vault BACKLOG #2615). And some store bounded engine-authored text: at least the `connection_test` and `connection_credential_test` rows carry the probe outcome, a fixed line or `safe_text()`/`safe_exc()`-scrubbed connector error text (BACKLOG #2372). None stores a message body. *Corrected 2026-10-07:* this said the writers "only ever store filter shapes, counts and ids" | `keep-forever by design` — 45 CFR 164.316(b)(2)(i) six-year documentation retention. **Not** chain-breakage: which rows a delete removes decides that, and the reasoning is stated once, in the `audit_days` row of [CONFIGURATION.md](CONFIGURATION.md#retention) | +| `audit_log.detail` | all three | Low — exposed IDs/counts, not bodies | **No** — plaintext by design | **PL-4** | JSON metadata about PHI *access* and other actions, not the PHI itself. Its writers store at least filter shapes, counts and ids. Some also store more: an edit-resend row holds **keyed** HMAC-SHA256 digests of the original and the edited body, and an upload-inject row one of the injected body. Without the store key a digest cannot be recovered or tested against a guess ([§6](#6-audit--accountability), vault BACKLOG #2615). And some store bounded engine-authored text: at least the `connection_test` and `connection_credential_test` rows carry the probe outcome, a fixed line or `safe_text()`/`safe_exc()`-scrubbed connector error text (BACKLOG #2372). Some store a client address beside the row's own `client`: at least the federated step-up's `auth.reauth` row, whose `start_client` is the address that started the step-up and whose `client_moved` says whether the callback came from another host (BACKLOG #2160, vault BACKLOG #3145). None stores a message body. *Corrected 2026-10-07:* this said the writers "only ever store filter shapes, counts and ids" | `keep-forever by design` — 45 CFR 164.316(b)(2)(i) six-year documentation retention. **Not** chain-breakage: which rows a delete removes decides that, and the reasoning is stated once, in the `audit_days` row of [CONFIGURATION.md](CONFIGURATION.md#retention) | | `audit_log.client` (ADR 0150) | all three | **No** — a network address; identifies a *host*, not a patient | No (metadata, deliberately not ciphered) | **PL-4** | The caller's client address — the "from where" of an audited action; `NULL` for engine-internal/`system` writes. **Personal data, but not PHI**, and exactly what HIPAA §164.312(b) audit controls exist to capture. Plaintext by decision: it must stay greppable/indexable for incident response, it already appears in the clear in `sessions.client`, and it is folded **inside** the tamper-evident hash chain — so it carries **integrity** protection even without confidentiality. Widens a store-file compromise from *who did what* to *who did what from where*; volume encryption + owner-only ACLs on whichever host owns the files — the engine's own `_secure_file` covers the **SQLite** store only ([§10](#10-secure-deployment--operations-checklist)) — are the control | `keep-forever by design` — same `audit_log` row lifetime as `detail`; the value is folded **inside** the hash chain | | `delivered_keys` (H2 idempotency ledger) | all three | **No** — hashes + ids only | No (deliberately not ciphered — nothing to protect) | **PL-4** | One row per completed outbound delivery: a SHA-256 `delivery_key` over non-PHI ids + a replay-stable seq, plus `outbox_id`/`message_id`/`destination_name`/`delivery_seq`. **Never a body or any PHI** — `control_id` is only *folded into the hash input*, never stored in the clear here. Lets the FIFO claim skip-and-complete a re-claimed already-delivered head without re-sending | `keep-forever by design` — the idempotency ledger a re-claimed already-delivered row checks instead of re-sending | | `state.namespace` / `state.key` | all three | **Possibly** — a Handler that keys correlation state on a raw MRN stores that identifier here in the clear | **No** — plaintext by construction: the pair is the composite primary key **and** the AAD input for `state.value`, so it cannot be ciphered without losing the lookup | **PL-4** | Authors must key state on a **surrogate, never a raw identifier**. Covered only by the whole-DB / volume layer. Rides `[retention].state_max_age_days` with its value | ``rides `[retention].state_max_age_days` `` | @@ -1818,10 +1818,10 @@ with materially different PHI profiles, so they get their own rows; stream 4 is | **2. `uvicorn` request/access log** (sub-stream of 1) | one line per HTTP request — method, **full request line including the query string**, status, timing | inherits stream 1's format | inherits stream 1's sink | request tracing, latency and error triage | inherits stream 1's | inherits stream 1's | **Can contain PHI.** `configure_logging` clears uvicorn's own handlers and propagates to the root, so the four filters apply; `serve` passes `log_config=None` and never disables `access_log`, so at the default `INFO` level every request is logged. OIDC `code`/`state` **are** scrubbed. **Not** scrubbed: a `?content=…` / `?field_value=…` that arrives anyway — no route has declared either since BACKLOG #1184, but the access line is built from the raw `query_string` rather than from what a route binds (the single-token residual above) | | **3. `messagefoundry.audit` off-box tee** (sub-stream of 1) | one JSON object per **committed** `audit_log` row: `event`/`ts`/`action`/`actor`/`channel_id`/`client`/`detail`, plus the `row_id`/`row_hash` anchor pair (BACKLOG #1198) that lets a collector tie the copy back to the row it came from and detect a gap in the chain | JSON | emitted after the row is durably committed and **outside** the store write lock; rides stream 1's handlers | shipping audit evidence to a SIEM so it survives a host compromise | inherits stream 1's | inherits stream 1's | `detail` is passed through the `safe_text` PHI chokepoint **before** it leaves the process; `client` is forwarded verbatim as a discrete field so a SIEM can index it. The `row_hash` anchor carries no PHI directly, but on a **keyless** store it is a plain SHA-256 whose other preimage members travel in the same record or the one before it, so a reader of the forwarded stream could test offline guesses at a span `safe_text` cut; it is a keyed MAC once the store cipher is active. Best-effort: a logging failure is caught, never raised into the audit write. **Pinned to `INFO`** — it is emitted even at `[logging].level = WARNING` | | **4. Off-box syslog/SIEM forwarder** — the shared **transport** for 1–3 | a copy of every record from 1–3 | `forward_format`, default **JSON** (independent of the stdout format) | the operator's collector (`forward_host`/`_port`) | off-box evidence retention / SIEM correlation | **default-on when a collector is named.** Transport: `udp` (default) / `tcp` / **`tls`** (RFC 5425, CA-anchored, verified by default). `serve` gates the hop on the shared posture gradient before the handler is installed: verified TLS ungated; otherwise loopback / attested ALLOW, non-enforcing WARN, **enforcing REFUSE (exit 2)** — there is no synthetic arm, and [ADR 0186](adr/0186-retire-the-synthetic-data-declaration-every-instance-carries-patient-data.md) left this cell with no per-hop way to accept the risk at all | the collector's, not the engine's | the identical four filters are installed on this handler. So the forwarded copy gets the same best-effort PHI redaction and credential scrub as stdout. A single-token identifier can survive it, as it can there. The copy also carries usernames, connection names, message ids, client addresses and the audit chain. That is the engine's own stated reason for gating the hop | -| **5. `audit_log` table** (SQLite, Postgres, SQL Server) | who / what / **where-from** / when of auth + PHI *access* and admin actions — plus, while `[security].audit_all_authorization_decisions` is on (**default `true`** since BACKLOG #1277, 2026-09-02, which reversed the ADR 0118 §5 `false`; the internal field it desugars to is `audit_all_authz`, whose old `[diagnostics]` TOML spelling is **refused at load** — ADR 0118), an `authz` row for **every** authorization decision including successes. That is the shipped volume of this stream, not an opt-in addition to it: one row per authenticated request on each `require()`-gated route, and this row's retention cell records that **nothing prunes the table** — `actor`, `action`, `channel_id`, `client`, `detail`, `row_hash` | JSON `detail`; **tamper-evident hash chain** over `prev_hash` + the row (the `client` address is **inside** the chained payload — ADR 0150) | the store database | HIPAA §164.312(b) audit controls; incident response; `verify_audit_chain` integrity checks | `GET /audit` requires **`audit:read`**; `GET /audit/export` requires the separate **`audit:export`** and streams CSV with formula-injection neutralisation, recording its own `audit.export` row *before* streaming; `GET /me/security-events` is a per-user view of the same table | **`[retention].audit_days` is reserved and NOT enforced — keep-forever by design** (the audit-retention requirement, ~6 years — **not** chain-breakage; [§8](#8-retention--purge) states the position and cites its source of record) | `detail` is stored **in the clear** (it is not a cipher-covered column): its protection is that writers only ever store filter shapes, counts and ids — never bodies or credentials — plus the store ACL and the volume layer | +| **5. `audit_log` table** (SQLite, Postgres, SQL Server) | who / what / **where-from** / when of auth + PHI *access* and admin actions — plus, while `[security].audit_all_authorization_decisions` is on (**default `true`** since BACKLOG #1277, 2026-09-02, which reversed the ADR 0118 §5 `false`; the internal field it desugars to is `audit_all_authz`, whose old `[diagnostics]` TOML spelling is **refused at load** — ADR 0118), an `authz` row for **every** authorization decision including successes. That is the shipped volume of this stream, not an opt-in addition to it: one row per authenticated request on each `require()`-gated route, and this row's retention cell records that **nothing prunes the table** — `actor`, `action`, `channel_id`, `client`, `detail`, `row_hash` | JSON `detail`; **tamper-evident hash chain** over `prev_hash` + the row (the `client` address is **inside** the chained payload — ADR 0150) | the store database | HIPAA §164.312(b) audit controls; incident response; `verify_audit_chain` integrity checks | `GET /audit` requires **`audit:read`**; `GET /audit/export` requires the separate **`audit:export`** and streams CSV with formula-injection neutralisation, recording its own `audit.export` row *before* streaming; `GET /me/security-events` is a per-user view of the same table | **`[retention].audit_days` is reserved and NOT enforced — keep-forever by design** (the audit-retention requirement, ~6 years — **not** chain-breakage; [§8](#8-retention--purge) states the position and cites its source of record) | `detail` is stored **in the clear** (it is not a cipher-covered column): its protection is that writers store at least filter shapes, counts, ids, keyed digests, bounded engine-authored text and client addresses, the `audit_log.detail` row above says which — never bodies or credentials — plus the store ACL and the volume layer. *Corrected 2026-10-16 (vault BACKLOG #3145):* this said writers "only ever store filter shapes, counts and ids" | | **6. `message_events` table** | the per-message disposition timeline — the **complete** vocabulary is `received`, `routed`, `unrouted`, `filtered`, `transformed`, `delivered`, `failed`, `dead`, `error`, `replayed`, `resent`, `reingressed`, `passthrough`, `passthrough_dropped`, `cancelled`, `edit_resend`, `edit_resubmit`, `viewed`, `not_deployed`, the ADR 0154 synchronous-reply pair `reply_returned` / `reply_timeout` (names, counts and `waited_ms` only — **never** a fragment of the partner's reply body), and `reply_dropped`, a delivery whose 2xx reply body was over the byte bound and dropped (the status and the bound only, vault BACKLOG #3040) (CI asserts this list against the engine's own `MESSAGE_EVENT_KINDS`). `[diagnostics].message_events` can thin the set, but never below the compliance floor `viewed` / `dead` / `error` / `failed` / `not_deployed` / `reply_timeout` / `reply_dropped` | rows: `message_id`, `ts`, `event`, `destination`, `detail` | the store database | operator timeline on the message-detail view; the `viewed` row is the HIPAA PHI-access record | `GET /messages/{id}` under **`messages:view_raw`** + `require_phi_read`; the read itself writes a `viewed` event **and** a `message_view` audit row | no dedicated window — `purge_message_bodies` sets `message_events.detail` to `NULL` in the same transaction that blanks the body, so it inherits `[retention].messages_days` | `detail` is `safe_text()`-scrubbed **then** cipher-encrypted (AAD `("message_events","detail",message_id,ts,event)`). Verbosity gate `[diagnostics].message_events` = `all` (default) / `errors` / `off`, with a **compliance floor that can never be thinned**: `viewed`, `dead`, `error`, `failed`, `not_deployed`, `reply_timeout`, `reply_dropped` are retained at every level (`reply_timeout` is the one row that explains a "we called you and got a 504" complaint, so an instance that thinned its logs would lose exactly the record it is later asked for; `reply_dropped` is the one row that answers "you ignored our reply", for the same reason) | | **7. `connection_event` table — DEFAULT ON** (`[diagnostics].connection_events = true`) | transport/lifecycle events per connection: `established`, `closed` (reason `eof`, `idle_timeout` or `frame_deadline` — a fixed vocabulary chosen by the listener, never free text), `idle_timeout`, `at_capacity`, `peer_not_allowlisted`, `frame_oversize`, `framing_error`, `peer_reset`, the MLLP listener's `handler_error` (BACKLOG #1619 -- the inbound handler faulted on a frame the listener read cleanly, such as a store outage at the ingress commit; the reason is scrubbed at the source like every other), the MLLP and HTTP listeners' `tls_handshake_failed` (vault BACKLOG #2613 and #3302 -- an inbound TLS handshake failed, on any event loop; the first at once, then one per listener per minute carrying the count since the last. The reason is a count, an exception class name and OpenSSL's reason mnemonic, never the peer's bytes or certificate. The event names a peer only where the MLLP listener runs its own handshakes, which is the standard asyncio loop; where the loop runs them, which is uvloop and every HTTP listener, the peer is not known, and a handshake that only runs past its bound is not counted), the DATABASE poll source's `row_undecodable` (BACKLOG #1662 — a polled row the source cannot turn into a body; the reason is scrubbed at the source and names the exception type or the operator's own configured column, never a row value), the FILE poll source's quarantine kinds `file_oversize` / `file_decompress_failed` / `file_content_mismatch` / `file_scan_rejected` (BACKLOG #1621 — a drop moved to `.error`; the reason is a size, a content type or a scrubbed codec or scanner message, never the file name), the inbound-HTTP intake-auth refusals `intake_auth_failed` / `auth_subject_denied` / `auth_rate_limited` (ADR 0154 D6 — peer address and mode only; **never** the credential, a prefix of it, or its length. Each of these also writes a tamper-evident audit-log row — the copy that survives an operator turning this diagnostics stream off), plus the runner's `connection_lost` / `connection_restored`. That is the whole vocabulary, asserted in CI against the literal emit call sites in `transports/` and the pipeline runner **and** cross-checked against the console's own filter tuple. The MLLP, raw-TCP, X12 and HTTP listeners emit these; the **DATABASE poll source** emits `row_undecodable` and nothing else (it dials out, so there is no lifecycle to report and its rows carry a **NULL peer column**); the **FILE poll source** emits only its four quarantine kinds, also with a NULL peer column; the **DICOM inbound C-STORE SCP** emits none — the runner injects the sink onto **every** source (`wiring_runner.py`, over the base-class `on_connection_event` field), so that connector *has* the wiring and simply never calls it — so this stream covers those four listeners plus those poll-source events and the runner's outbound-lane transitions — not literally every connection. A DICOM association's connects and refusals are therefore **absent** from this stream. The **`ISA`/`IEA`-framed X12 inbound** was equally absent until BACKLOG #1665 and is not any more: it emits the same seven kinds as the raw-TCP listener | rows: `ts`, `connection`, `transport`, `direction`, `kind`, `peer_host`, `message_id` (correlation hint), `reason` | the store database, **all three backends** | Corepoint-style transport diagnostics — "did the sender connect, and why did it drop" | `GET /events` and `GET /connections/{name}/events` under **`monitoring:read`** (**not** a PHI permission) with per-channel RBAC — an out-of-scope `connection=` is 403'd *and* audited — server-clamped to ≤1000 rows. `reason` alone is gated on `messages:view_summary` and masked as `****` until the audited per-event `reveal=` act (BACKLOG #2443; the `connection_event.reason` row of §3's PL-2 block says how) | `[retention].connection_event_retention_hours` (its own **hours** window); 0 inherits `[retention].messages_days`; both 0 = keep forever. Plain age `DELETE` (metadata-only) | **`reason` is free text that can carry sensitive fragments.** Defended twice — `safe_exc()` at the source, `safe_text(reason, limit=200)` at the store (the bound §3's PL-2 block states) — then cipher-encrypted (AAD `("connection_event","reason",connection,ts,kind)`). Every other column is config metadata; the table is documented **metadata-only** — never a frame, body or HL7 field value. Writes are a pure side observer: a bounded in-memory queue drained by a background task outside any handoff transaction, so a flood can never block a listener or pin a message disposition | -| **8. `alert_instance` table — default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `initial_credential_expiring` (an UNCLAIMED admin-issued temporary password nearing the instant the login gate stops accepting it -- ASVS 6.4.5, BACKLOG #1141; keyed on `user:`, and the payload is the ISO deadline plus whole hours remaining, repeated in the reason column, never the password), `integrity_drift`, `update_available`, `backup_failed`, `store_privilege_warning` (the store privilege preflight found the store principal over-granted or could not read it -- BACKLOG #305; the payload is the finding, a count and the preflight's summary line, which names the login, the database and role names, never a password or message content), `leadership_acquired`, `dr_activated`, `log_write_failed` (an application-log sink was rolled after a write failure, or is UNWRITABLE and this process's connections were stopped -- BACKLOG #122, ADR 0162; the payload is the sink LABEL, the stage, a `safe_exc` reason and a count of connections stopped, never the record whose write failed), `log_forward_failed` (the off-box log forwarder is absent, losing records or not sending -- BACKLOG #2612; keyed on `forwarder:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, fixed reason words and a count, never a record, the collector's address or an error text; an unpinned cluster node's `node_id` holds this engine's own host name and process id, so the key carries them), `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn — ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes), `approval_stale_requester` (a dual-control release refused because the requester no longer holds the authority the operation needs -- ASVS 8.3.2, BACKLOG #289; keyed on the approval id, and the payload is that id, the operation key and a closed-set reason slug, never the requester's name or the captured params), `approval_too_early` (a dual-control release refused because it arrived before `[approvals].min_dwell_seconds` -- ASVS 2.4.2, BACKLOG #287; keyed on `approval:`, and the payload is that key, the operation key and a fixed reason string, never the approver's or requester's name or the captured params), `audit_write_failed` (the approval gate could not write one of its audit rows -- vault BACKLOG #2255; keyed on `approval:`, or on `config_reload:inline` when an ungated config reload could not write its `config_reload_attempted` row -- vault BACKLOG #2254; the payload is that key and the lost row's audit action name, never the row's detail, a username or the captured params; the ERROR log line beside it holds the detail on some paths and only the exception on others), `ad_reconcile_aborted` (a directory reconciler pass left signed-in accounts unrevoked and needs an operator: the mass-revoke breaker tripped and nothing was revoked, or one or more probes were answered with an LDAP referral and those accounts were left unjudged while the rest of the pass was applied, BACKLOG #2538, or one or more probes raised a fault the engine could not read and those accounts were left unjudged the same way, BACKLOG #3100, or an account's group search was refused for the engine's own deadline or cap, so a role or scope change in the directory does not revoke that account's sessions, though a disable or deletion still does, vault BACKLOG #3696; for the breaker, a referral and a probe fault the payload is a fixed source label, `directory-reconciler` for the breaker, `directory-reconciler-referral` for a referral and `directory-reconciler-probe-fault` for a probe fault so each is its own instance, the closed-set reason slug `mass_revoke_breaker`, `directory_referral` or `probe_fault`, the probed count and the latched operator explanation, never a username; a paging refusal is instead keyed on that operator account's username, as `directory-reconciler-paging:`, so each account is its own instance, raised as the account enters that state and again after a restart while it stands, and the payload is that key, the closed-set reason slug `group_search_deadline` or `group_search_cap`, the probed count and a fixed sentence naming the cause, never anything else about the account), `ad_reconcile_held` (the reconciler is holding accounts whose `userAccountControl` it cannot read, ADR 0195; the payload is the same fixed source label, a closed-set reason slug, the count of undetermined accounts and the latched operator explanation, never a username), `ad_session_revoked` (the reconciler revoked a directory principal's sessions; keyed on that operator account's username, with a `directory_absent`, `directory_disabled`, `directory_undetermined`, `roles_changed` or `scope_changed` reason), `approval_approver_provenance` (a dual-control release, or the resolution of an interrupted one, went ahead with an approver or resolver account created, re-passworded or TOTP-enrolled after the request -- BACKLOG #315, #2125; keyed on `approval:`, or `approval::resolve` for a resolution, and the payload is the operation key and the closed-set change slugs, never a username or the captured params), `administrator_granted` (the built-in Administrator role was granted by account creation, a role change, a directory group newly mapped to it -- BACKLOG #315 -- or a directory sign-in whose role sync newly gave the account the role -- vault BACKLOG #2610; keyed on `user:` or `ad-group:`, with the grant route and the granting administrator's username, or `` for a directory sign-in), `no_enabled_administrator` (a directory sign-in's role sync or the session reconciler took Administrator from its last enabled holder -- vault BACKLOG #3229; keyed on `user:`, and the payload is `via`, `ad-sync` or `ad-reconcile`, and a fixed reason sentence, never message content), `intake_paused` (the engine paused intake because the staged backlog went over `[inbound].max_staged_depth` or the SQLite volume fell below `[retention].min_free_disk_mb` -- BACKLOG #290; keyed on `intake:staged_depth` or `intake:disk_floor`, with `@node:` or `@shard:` appended when several engine processes share the store (BACKLOG #2272), and the payload is the reason, a count or a MiB figure, its limit and the store backend's name, never message content), `config_changed` (a start loaded a config whose fingerprint differs from the store's baseline -- vault BACKLOG #2597; keyed on `config:`, and the payload is the two digests, the node and engine shard labels, and the action, actor username and time of the row the older digest came from, never the config directory, a git commit or message content), `crl_reload_refused` (the CRL reload refused a replaced CRL file, so running TLS contexts keep the copy they hold -- vault BACKLOG #2904; keyed on `crl_reload:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the file's path, a short class token for the remedy, a count of contexts, the engine's own fixed remedy sentence and, when a start would refuse the file too, a `start_refuses` flag, never the refusal's reason, which quotes CRL issuer names and dates; that reason is in the application log, and in the `detail` of the CRL expiry alert once the held copy nears its `nextUpdate`), `remote_debug_refused` (the engine's remote-debugging guard refused scripts that another process injected into the engine process -- vault BACKLOG #2742; keyed on `remote_debug:refused`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the count of scripts refused since the alert was last raised, the count since the process started and a fixed sentence built from the two, never a script's file name, which the injecting process chooses; that name is in the application log, in the guard's WARNING line), `sandbox_worker_refused` (a `[sandbox].mode = "subprocess"` worker was refused because the graph it loaded differs from the engine's, most often a config file edited on disk with no reload -- vault BACKLOG #2724; keyed on the inbound's name, raised once per refusal, and the payload is that name and a fixed sentence naming the cause and the remedy, never a message body or the refusal text, which names config names and digests and is in the application log), and the six security signals of the audit-stream rule layer -- vault BACKLOG #2613: `signin_failure_burst` (keyed on `signin:`, never the typed username), `access_denied_burst`, `body_view_burst` and `bulk_export` (each keyed on `account:`), `log_level_debug` (keyed on `logging:debug`) and `posture_loosened` (keyed on `posture:start`); the payload is a count and a fixed sentence holding a level name, switch names or a username, never a message body, a message id or an audit row's detail. The reachable **inverse** signals — `connection_restored`, `leadership_lost`, `dr_released`, `store_privilege_clean`, `intake_resumed`, `crl_reload_applied` (it resolves `crl_reload_refused` under the same key, so for the same file and the same engine identity, at least once the reload's record no longer holds the refusal, once no hop holds any CRL, and when the reload stops -- vault BACKLOG #2904; it is also raised once per held file outside a CA bundle, at the first pass that judges it with no refusal standing, whether or not an alert is open, so it can resolve nothing, and it then clears one an earlier run under that identity left open -- vault BACKLOG #3543. An alert for a file no hop holds now stays open, and so does one an unpinned cluster node raised, because that node's identity changes on every start; [CONFIGURATION.md](CONFIGURATION.md), *The alert resolves itself*, is the source of record), `ad_reconcile_breaker_cleared`, `ad_reconcile_hold_released` (the last two resolve `ad_reconcile_held` and `ad_reconcile_aborted`, the latter only under the breaker's or the referral's source: the probe fault's and a paging refusal's instances have no inverse; when they do, and where they never do, is stated once in [CONFIGURATION.md](CONFIGURATION.md#when-the-reconcilers-alerts-resolve-themselves) -- BACKLOG #2136) — are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (One more mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list — acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope, and `reason` alone gated on `messages:view_summary` and masked as `****` until the audited per-alert `reveal=` act (BACKLOG #2443; the `alert_instance.reason` row of §3's PL-2 block says how); ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` — an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`: `safe_text(reason, limit=200)` (the bound §3's PL-2 block states) then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` — the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). | +| **8. `alert_instance` table — default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `initial_credential_expiring` (an UNCLAIMED admin-issued temporary password nearing the instant the login gate stops accepting it -- ASVS 6.4.5, BACKLOG #1141; keyed on `user:`, and the payload is the ISO deadline plus whole hours remaining, repeated in the reason column, never the password), `integrity_drift`, `update_available`, `backup_failed`, `store_privilege_warning` (the store privilege preflight found the store principal over-granted or could not read it -- BACKLOG #305; the payload is the finding, a count and the preflight's summary line, which names the login, the database and role names, never a password or message content), `leadership_acquired`, `dr_activated`, `log_write_failed` (an application-log sink was rolled after a write failure, or is UNWRITABLE and this process's connections were stopped -- BACKLOG #122, ADR 0162; the payload is the sink LABEL, the stage, a `safe_exc` reason and a count of connections stopped, never the record whose write failed), `log_forward_failed` (the off-box log forwarder is absent, losing records or not sending -- BACKLOG #2612; keyed on `forwarder:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, fixed reason words and a count, never a record, the collector's address or an error text; an unpinned cluster node's `node_id` holds this engine's own host name and process id, so the key carries them), `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn — ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes), `approval_stale_requester` (a dual-control release refused because the requester no longer holds the authority the operation needs -- ASVS 8.3.2, BACKLOG #289; keyed on the approval id, and the payload is that id, the operation key and a closed-set reason slug, never the requester's name or the captured params), `approval_too_early` (a dual-control release refused because it arrived before `[approvals].min_dwell_seconds` -- ASVS 2.4.2, BACKLOG #287; keyed on `approval:`, and the payload is that key, the operation key and a fixed reason string, never the approver's or requester's name or the captured params), `audit_write_failed` (the approval gate could not write one of its audit rows -- vault BACKLOG #2255; keyed on `approval:`, or on `config_reload:inline` when an ungated config reload could not write its `config_reload_attempted` row -- vault BACKLOG #2254; the payload is that key and the lost row's audit action name, never the row's detail, a username or the captured params; the ERROR log line beside it holds the detail on some paths and only the exception on others), `ad_reconcile_aborted` (a directory reconciler pass left signed-in accounts unrevoked and needs an operator: the mass-revoke breaker tripped and nothing was revoked, or one or more probes were answered with an LDAP referral and those accounts were left unjudged while the rest of the pass was applied, BACKLOG #2538, or one or more probes raised a fault the engine could not read and those accounts were left unjudged the same way, BACKLOG #3100, or an account's group search was refused for the engine's own deadline or cap, so a role or scope change in the directory does not revoke that account's sessions, though a disable or deletion still does, vault BACKLOG #3696; for the breaker, a referral and a probe fault the payload is a fixed source label, `directory-reconciler` for the breaker, `directory-reconciler-referral` for a referral and `directory-reconciler-probe-fault` for a probe fault so each is its own instance, the closed-set reason slug `mass_revoke_breaker`, `directory_referral` or `probe_fault`, a count (the accounts the breaker judged; for a probe fault the same total of candidates the pass took up, asked or not, faulted ones included; or for a referral the lookups the pass sent, which leaves out an id-less row -- vault BACKLOG #3170) and the latched operator explanation, never a username; a paging refusal is instead keyed on that operator account's username, as `directory-reconciler-paging:`, so each account is its own instance, raised as the account enters that state and again after a restart while it stands, and the payload is that key, the closed-set reason slug `group_search_deadline` or `group_search_cap`, the same count as a probe fault's and a fixed sentence naming the cause, never anything else about the account), `ad_reconcile_held` (the reconciler is holding accounts whose `userAccountControl` it cannot read, ADR 0195; the payload is the same fixed source label, a closed-set reason slug, the count of undetermined accounts and the latched operator explanation, never a username), `ad_session_revoked` (the reconciler revoked a directory principal's sessions; keyed on that operator account's username, with at least a `directory_absent`, `directory_disabled`, `directory_undetermined`, `directory_object_id_missing`, `roles_changed` or `scope_changed` reason), `approval_approver_provenance` (a dual-control release, or the resolution of an interrupted one, went ahead with an approver or resolver account created, re-passworded or TOTP-enrolled after the request -- BACKLOG #315, #2125; keyed on `approval:`, or `approval::resolve` for a resolution, and the payload is the operation key and the closed-set change slugs, never a username or the captured params), `administrator_granted` (the built-in Administrator role was granted by account creation, a role change, a directory group newly mapped to it -- BACKLOG #315 --, a directory sign-in whose role sync newly gave the account the role -- vault BACKLOG #2610 -- or a directory reconcile pass whose role re-diff did -- vault BACKLOG #3141; keyed on `user:` or `ad-group:`, with the grant route and the granting administrator's username, or `` for a directory sign-in or reconcile pass), `no_enabled_administrator` (a directory sign-in's role sync or the session reconciler took Administrator from its last enabled holder -- vault BACKLOG #3229; keyed on `user:`, and the payload is `via`, `ad-sync` or `ad-reconcile`, and a fixed reason sentence, never message content), `intake_paused` (the engine paused intake because the staged backlog went over `[inbound].max_staged_depth` or the SQLite volume fell below `[retention].min_free_disk_mb` -- BACKLOG #290; keyed on `intake:staged_depth` or `intake:disk_floor`, with `@node:` or `@shard:` appended when several engine processes share the store (BACKLOG #2272), and the payload is the reason, a count or a MiB figure, its limit and the store backend's name, never message content), `config_changed` (a start loaded a config whose fingerprint differs from the store's baseline -- vault BACKLOG #2597; keyed on `config:`, and the payload is the two digests, the node and engine shard labels, and the action, actor username and time of the row the older digest came from, never the config directory, a git commit or message content), `crl_reload_refused` (the CRL reload refused a replaced CRL file, so running TLS contexts keep the copy they hold -- vault BACKLOG #2904; keyed on `crl_reload:`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the file's path, a short class token for the remedy, a count of contexts, the engine's own fixed remedy sentence and, when a start would refuse the file too, a `start_refuses` flag, never the refusal's reason, which quotes CRL issuer names and dates; that reason is in the application log, and in the `detail` of the CRL expiry alert once the held copy nears its `nextUpdate`), `remote_debug_refused` (the engine's remote-debugging guard refused scripts that another process injected into the engine process -- vault BACKLOG #2742; keyed on `remote_debug:refused`, with `@node:` or `@shard:` appended when several engine processes share the store, and the payload is that key, the count of scripts refused since the alert was last raised, the count since the process started and a fixed sentence built from the two, never a script's file name, which the injecting process chooses; that name is in the application log, in the guard's WARNING line), `sandbox_worker_refused` (a `[sandbox].mode = "subprocess"` worker was refused because the graph it loaded differs from the engine's, most often a config file edited on disk with no reload -- vault BACKLOG #2724; keyed on the inbound's name, raised once per refusal, and the payload is that name and a fixed sentence naming the cause and the remedy, never a message body or the refusal text, which names config names and digests and is in the application log), and the six security signals of the audit-stream rule layer -- vault BACKLOG #2613: `signin_failure_burst` (keyed on `signin:`, never the typed username), `access_denied_burst`, `body_view_burst` and `bulk_export` (each keyed on `account:`), `log_level_debug` (keyed on `logging:debug`) and `posture_loosened` (keyed on `posture:start`); the payload is a count and a fixed sentence holding a level name, switch names or a username, never a message body, a message id or an audit row's detail. The reachable **inverse** signals — `connection_restored`, `leadership_lost`, `dr_released`, `store_privilege_clean`, `intake_resumed`, `crl_reload_applied` (it resolves `crl_reload_refused` under the same key, so for the same file and the same engine identity, at least once the reload's record no longer holds the refusal, once no hop holds any CRL, and when the reload stops -- vault BACKLOG #2904; it is also raised once per held file outside a CA bundle, at the first pass that judges it with no refusal standing, whether or not an alert is open, so it can resolve nothing, and it then clears one an earlier run under that identity left open -- vault BACKLOG #3543. An alert for a file no hop holds now stays open, and so does one an unpinned cluster node raised, because that node's identity changes on every start; [CONFIGURATION.md](CONFIGURATION.md), *The alert resolves itself*, is the source of record), `ad_reconcile_breaker_cleared`, `ad_reconcile_hold_released` (the last two resolve `ad_reconcile_held` and `ad_reconcile_aborted`, the latter only under the breaker's or the referral's source: the probe fault's and a paging refusal's instances have no inverse; when they do, and where they never do, is stated once in [CONFIGURATION.md](CONFIGURATION.md#when-the-reconcilers-alerts-resolve-themselves) -- BACKLOG #2136) — are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (One more mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list — acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope, and `reason` alone gated on `messages:view_summary` and masked as `****` until the audited per-alert `reveal=` act (BACKLOG #2443; the `alert_instance.reason` row of §3's PL-2 block says how); ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` — an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`: `safe_text(reason, limit=200)` (the bound §3's PL-2 block states) then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` — the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). | | **9. `response` rows with `kind='ack_sent'` — DEFAULT ON** (`[diagnostics].response_sent = true`) | the ACK/NAK the engine returned to an inbound sender, under a sentinel destination `\x1fack:` | rows: `ack_code` (`AA`/`AE`/`AR`/`CA`/`CE`/`CR`), `ack_phase` (`decode`/`parse`/`strict`/`ingest`), `outcome`, `body`, `detail` | the store database | "what did we actually reply, and why" — the operator's answer to a sender disputing an ACK | `GET /messages/{id}/responses` under `messages:read` + `require_phi_read`; the `body` only for a caller who also holds `messages:view_raw` and `messages:view_summary`; every read writes a `response.read` audit row | `body`, `detail` and `resp_headers` are set to `NULL` in place by `purge_message_bodies` on the message-body window, on all three backends | **PHI fail-safe:** the ACK **body** is stored **only when the store cipher is active** — on a keyless store it is `NULL` rather than plaintext — and every NAK passes no body at all, so the offending field value is never persisted. The disposition metadata (`ack_code`/`ack_phase`/`outcome`) is non-PHI and always captured; `detail` is `safe_text`-scrubbed, held to the 200-character bound §3's PL-2 block states, and encrypted | | **10. `[alerts]` webhook transport** (off by default — `webhook_url` unset) | one HTTPS POST per alert, carrying every non-underscore event key as JSON | JSON | the operator's webhook endpoint (Slack/Teams/PagerDuty/custom) | operator notification | **`https` only** — a plaintext `http://` webhook URL is refused at construction unless the `MEFOR_ALLOW_INSECURE_TLS` escape is set (and then a warning is logged); since #329 this path routes that escape through the clamped `weakened_tls_escape_permitted(posture)` (the instance posture threaded from the API lifespan), so on an enforcing-PHI instance the escape is inert and a cleartext webhook POST stays refused — the same clamp as the connectors, no longer the raw escape. Redirects are refused; an optional `webhook_allowed_hosts` egress allowlist gates the host | the endpoint's | **carries the alert's `detail`/`reason` free text** (`safe_exc()`-scrubbed at the emit sites, but **not** re-run through `safe_text` on this path). Internal `_`-prefixed keys (per-rule recipients, rule id, cooldown) are stripped before send, so recipient addresses never cross the wire | | **11. `[alerts]` SMTP transport — operator alert list** (off unless `email_smtp_host` + `email_from` + ≥1 `email_to`) | one email per alert; default subject `[MessageFoundry] — `, default body every non-underscore event key as `k: v` | plain text (always kept — never HTML-only); optional HTML alternative | the operators' mailboxes | operator notification | `smtp_allowed_hosts` egress allowlist; the SMTP password comes from `MEFOR_ALERTS_EMAIL_PASSWORD` or a `[secrets]` provider, never the config file; per-send timeout `email_timeout` | the mail system's | carries the same `detail`/`reason` free text as the webhook. #138 operator templates are constrained to a **closed non-PHI variable allowlist** validated fail-closed at config load. **Transport posture:** `send_plain_email` builds an explicit **verifying** context (chain + hostname + strict RFC 5280, TLS 1.2 floor) via `tls_policy.build_smtp_tls_context()` and passes it to `starttls()`, anchored to the OS roots, `[alerts].email_tls_ca_file`, or `[tls].internal_ca_file` — the same factory the EMAIL and DIRECT *message destinations* use, so all three SMTP cells now share one policy ([#323](BACKLOG.md), closed 2026-08-02). Before that this call passed **no** context and Python's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname = False`), leaving the hop encrypted but unauthenticated. There is still **no hop gradient or attestation on this path** — unlike the connectors, this cell is constructed outside the `active_hop_posture` scope, so its deviations (`email_use_tls = false`, or `email_tls_verify = false`) are gated by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch at the serve gate** rather than by the clamped escape: on an enforcing PHI instance `serve` refuses to start without it, and permits + `AUDIT`-logs the start with it. Both deviations are named by `security_loosenings()` and reported by `messagefoundry check`'s `alert-smtp-tls` advisory | diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 2f16b507e..9e0773243 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -824,8 +824,14 @@ AD group-map and scope-map saves and the custom-role edit and delete can still e sessions on the login window. The row names the actor, the route as `op`, the caller's stored id and the caller's address. The guard refuses when the path's spelling or the stored id matches the caller (vault BACKLOG #3259); `_refuse_if_self` in `api/auth_routes.py` says why the -stored id matters on the console. The `404` that `DELETE /me/sessions/{session_id}` answers for -another account's session still writes no row of its own. +stored id matters on the console. A self-service revoke of one session that ends nothing, the +`404` of `DELETE /me/sessions/{session_id}` and the console's "Nothing was revoked" notice, writes +one `auth.session_revoke_missed` row (vault BACKLOG #3260, limb 2). It names the actor, the +caller's address and the first 12 characters of the id, and nothing else: an unknown id and +another account's id get the same answer and the same row. The console takes the id as plain +text, so an id that is not lower-case hex is recorded as `malformed` rather than copied. The +write is soft: if the audit log refuses it, the answer still stands and the loss is logged at +ERROR, so the row is the record of a miss only while the audit log accepts writes. **Delegated identity & admin device posture (#193 sibling; ASVS 13.2.1 / 13.3.2 / 8.4.2 — the delegation boundary).** Three controls whose enforcement is largely the deploying organization's to provide. @@ -2588,7 +2594,7 @@ What the engine does instead is make the cheap routes loud. It refuses none of t | Signal | When | Where it goes | |---|---|---| | `approval.approver_provenance` audit row and `approval_approver_provenance` alert | A release goes ahead and the approver's account was created, had its password changed, or enrolled TOTP **after** the request was made. A resolution of an `interrupted` release does the same for the resolver's account (BACKLOG #2125) | Audit row against the approver or resolver, with their `client` address (ADR 0150). Its `act` field is `release` or `resolve`. Alert keyed `approval:` for a release and `approval::resolve` for a resolution, carrying the changed facts only | -| `administrator_granted` alert | `POST /users` creates an account with the Administrator role, `PUT /users/{id}/roles` adds it, `PUT /ad-group-map` newly maps a group to it, or a directory sign-in's role sync newly gives an account the role (vault BACKLOG #2610) | Alert keyed `user:` or `ad-group:`, naming the granting administrator, or `` for a directory sign-in | +| `administrator_granted` alert | `POST /users` creates an account with the Administrator role, `PUT /users/{id}/roles` adds it, `PUT /ad-group-map` newly maps a group to it, or a directory sign-in's role sync or a directory reconcile pass newly gives an account the role (vault BACKLOG #2610, #3141) | Alert keyed `user:` or `ad-group:`, naming the granting administrator, or `` for a directory sign-in or reconcile pass | | `client` on the `user.created` audit row | An account created through `POST /users` or `POST /users/directory` (BACKLOG #2021) | The creating administrator's address, like the approval rows | | `account_created` notice | An account created through `POST /users` or `POST /users/directory`, neither of which creates one without a notification address (BACKLOG #2018, #2021) | The new account's own notification address | @@ -2611,15 +2617,18 @@ with no page. `account_created` notice. - **A directory grant.** An account can get Administrator because the *directory* added it to a group already mapped to Administrator. The engine sees that grant in one of the two places below, - whichever comes first. A sign-in that sees it first raises `administrator_granted`, unless that - sign-in fails with a server error after the role write; the retry then gains nothing. A reconciler - pass that sees it first raises `ad_session_revoked`, which does not name the role. The later - sign-in then gains nothing, so it raises no `administrator_granted`. An account that never signs - in again and holds no live session raises nothing. So watch membership of the mapped group in the - directory itself. + whichever comes first. Either one raises `administrator_granted`, but at least these cases raise + none. A sign-in that sees it first and fails with a server error after the role write loses it; + the retry then gains nothing. A reconciler pass the mass-revoke breaker aborts writes no roles. + An account that never signs in again raises nothing when no pass reaches it: it holds no live + session, or `[auth].ad_session_recheck_seconds` is 0. So watch membership of the mapped group in + the directory itself. **CORRECTED 2026-10-01:** this read "raises no alert. The engine never sees that grant." **CORRECTED 2026-10-06:** this read "raises no `administrator_granted` alert", and the sign-in case below read "It raises no alert." Vault BACKLOG #2610 added the sign-in alert. + **CORRECTED 2026-10-16:** this read "A reconciler pass that sees it first raises + `ad_session_revoked`, which does not name the role. The later sign-in then gains nothing, so it + raises no `administrator_granted`." Vault BACKLOG #3141 added the reconciler's alert. - **At the account's next sign-in.** The engine writes an `auth.ad_roles_resynced` audit row with the old and new roles. It also sends a best-effort roles-changed notice to that account's own notification address, when the account has one and security notices are set @@ -2633,8 +2642,14 @@ with no page. [directory reconciler](#directory-session-reconciliation--propagating-an-ad-disable-adr-0079-mechanism-2) that completes stores the new roles and revokes the session. It writes `auth.ad_session_revoked` with the new roles, sends the same notice, and raises - `ad_session_revoked` with reason `roles_changed`. The next sign-in then writes no - `auth.ad_roles_resynced` row. That section lists when a pass revokes or alerts nothing. + `ad_session_revoked` with reason `roles_changed`. Right after the role write it raises + `administrator_granted`, keyed `user:`, with `granted_by` set to `` and + `via` set to `directory_reconcile`. That alert clears the account's `no_enabled_administrator` + cooldown, as a sign-in's does. An account that already held Administrator raises nothing. The + next sign-in then writes no `auth.ad_roles_resynced` row and raises no second + `administrator_granted`. A sign-in that runs at the same time as the pass can still raise a + second one, since each judges against its own read of the roles; a duplicate page is the safe + direction. That section lists when a pass revokes or alerts nothing. No pass runs at `[auth].ad_session_recheck_seconds = 0`. A pass probes at most `ad_session_recheck_max_users` accounts (200 by default), least recently probed first. When more directory accounts than that hold live sessions, an account can wait several passes. @@ -3090,16 +3105,20 @@ and TOTP is still off (BACKLOG #2224, #3025): the enable is a compare-and-set on `totp_secret` cell the code was checked against. If a reset clears the secret during the confirm, another confirm turns TOTP on first, or a second enrolment start re-stages a different secret inside the same window, the confirm turns nothing on and returns no recovery codes. The refused -confirm ends the session it just rotated and answers as a lost session: `401` on +confirm asks the store to end the session it just rotated, and answers as a lost session: `401` on `POST /me/mfa/confirm`, and the sign-in page in the web console. It is audited `auth.mfa_enroll_refused`. The `reason` values it records include `account_gone`, `already_enabled`, `secret_cleared`, `secret_replaced` and `write_lost`. Each is read from the row after the refusal, -so a `reason` points an operator at the likely cause rather than proving it. The compare is on the -stored ciphertext, so at least a store-key rotation that re-encrypts the column inside the window -refuses too. That refusal reads as `secret_replaced` although no second enrolment started. +so a `reason` points an operator at the likely cause rather than proving it. The row's +`session_ended` is false when the store refused to end the rotated session. The token is still not +handed back, and the row is written whatever the revoke raised, unless its own write fails, +which is logged at ERROR (vault BACKLOG #3113). The compare is +on the stored ciphertext, so at least a store-key rotation that re-encrypts the column inside the +window refuses too. That refusal reads as `secret_replaced` although no second enrolment started. `write_lost` means the row still held the verified secret with TOTP off, so the enable lost to some other concurrent write. After a `write_lost` or a store-key rotation the staged secret still -stands. The user signs in again, because the refused confirm ended the session, and passes the +stands. The user signs in again, because the refused confirm hands no token back and asks the +store to end the session, and passes the step-up `POST /me/mfa/confirm` asks of any confirm: a password proof bound to the confirm action (ADR 0077), or a recent step-up window where `[auth].require_action_step_up` is off. The user then confirms with a code from a later step, because the refused confirm spent @@ -4291,7 +4310,14 @@ token claim**. There is no new `auth_provider` value: a federated login resolves `/ui/login?e=rate_limited` and a WARNING log), a return the `Sec-Fetch-Mode` check refuses (audited `auth.login_failed`), and any refusal at the start leg `POST /ui/reauth/oidc`. A refusal those two methods decide changes nothing and re-renders the step-up page with a plain message, except one that finds the session gone, which sends the operator to sign in. A refusal after the `state` - check is filed under the staged session's account. A cancel at the IdP returns to the same page. + check is filed under the staged session's account. That row, and the row of a completed step-up, + also carry `start_client`, the address the start leg staged on the flow, and `client_moved`, + whether the callback came from another host by the new-address comparison, or null when either + address is unknown (BACKLOG #2160, vault BACKLOG #3145). Neither is ever read from the token. A + refusal before the `state` check names neither, nor does a sign-in flow refused as + `flow_purpose_mismatch`, whose start was another ceremony. A move is recorded, never refused. Behind a proxy + the engine does not trust, both legs carry the proxy's address, so `client_moved` reads false + there. A cancel at the IdP returns to the same page. A callback without the flow's own `state` consumes nothing, and the flow waits for the real return. The sign-in callback follows the same rule, an `error=` or malformed one included (BACKLOG #3198). A password is @@ -4557,7 +4583,11 @@ account's entry and write its roles onto the row. No sign-in or step-up admits s writes no roles, and it strikes and revokes as an absent account does, with the reason `directory_object_id_missing`. It is not a `userAccountControl` answer, so it never moves the ADR 0195 hold. It is never asked, so it is no part of an outage either: a pass whose every lookup fails -still audits `auth.ad_reconcile_skipped`, and that row's `asked` count leaves it out. Such a +still audits `auth.ad_reconcile_skipped`, and that row's `asked` count leaves it out. Such a pass, +and one that aborts because its lookups were only referred, faulted or failed, still strikes the +row, because its answer never came from the directory (BACKLOG #3170). No such pass revokes it: only a pass that is not aborted can, once +the row's strikes reach the threshold and the row is in that pass's probe budget. A referral's `auth.ad_reconcile_referred` row and alert count only +the lookups sent too. Its revocation does count toward the mass-revoke breaker below. Such a row cannot take a federated binding: the bind refuses it, so every binding the bind has made since BACKLOG #1143 slice C sits on a row probed by its id (ADR 0184 AC-5). A binding already on an id-less row, made before that refusal, is **never probed by name** (BACKLOG #2027). The pass skips the row and @@ -4571,7 +4601,8 @@ Removing the binding (`DELETE /users/{user_id}/federated-identity`) returns the The pass has these safety properties, because the lookup still returns one indistinguishable "not found" for *deleted*, *moved out of the search base* and *the search base was never right*: -- **Fail-OPEN.** An unreachable domain controller revokes **nothing** and does not even accrue a strike. +- **Fail-OPEN.** An unreachable domain controller revokes **nothing**, and an account the pass asked + about accrues no strike. An id-less row, never asked, is still struck (BACKLOG #3170). A fail-closed re-check would turn a directory blip into a total console outage during exactly the incident when operators need the console. - **Two strikes** (`ad_session_recheck_strikes`, default 2) before any revocation. @@ -4636,7 +4667,7 @@ refers on every pass. Read as an outage, it would stop revocation on a first dep page. The two bases fail differently. A referring user search base refers every probe that searches, -so the pass judges nothing and revokes nothing, like an outage, but it still pages. A referring +so the pass judges no account it asked about and revokes nothing, like an outage, but it still pages. A referring group search base refers only the accounts the user search found enabled. Disables and deletions still revoke, but a role or channel-scope change in the directory does not reach those accounts' sessions while it @@ -4651,7 +4682,8 @@ directory outage (BACKLOG #3100). Each such probe is logged at WARNING by name, estate is still reconciled. A pass with any fault logs one ERROR naming no account and audits `auth.ad_reconcile_faulted`. It also raises the `ad_reconcile_aborted` alert with reason `probe_fault`, under its own source `directory-reconciler-probe-fault`. If every probe faults or -fails, the pass judges nothing and revokes nothing, like an outage, but it still pages. A +fails, the pass judges no account it asked about and revokes nothing, like an outage, but it still +pages, and it still strikes an id-less row (BACKLOG #3170). A connection, timeout or name-lookup error that escapes the directory layer is still read as unreachable. So is at least an operating-system error saying the network or host is unreachable, down or reset. Another network-class error can still read as a probe fault and page. @@ -4876,14 +4908,24 @@ Users are notified of security-relevant changes to their account through **two** alert distribution list). Fired on: account **lockout** and the **first successful login after ≥3 failed attempts**, or a step-up re-auth that clears such a run (suspicious-login signals, 6.3.5); and **password change**, **email change**, **role change**, **account disable**, and a **directory rename** of the username (credential changes, - 6.3.7). When the directory session reconciler ends an account's sessions for a reason other than - a read disabled bit, it sends `directory_sessions_ended`, not `account_disabled` (vault BACKLOG - #2140). At least `directory_absent`, `directory_undetermined` and `directory_object_id_missing` - send it. The comment on `DIRECTORY_SESSIONS_ENDED` in `auth/notifications.py` says what each one - means. For at least `directory_object_id_missing` the engine never asks the directory (BACKLOG - #2434). So the notice names no directory check. It says only that the engine could not confirm - from the directory that the account may still sign in, which is true for each current reason. A - new reason must be checked against that wording (vault BACKLOG #3171). + 6.3.7). + A notice for a change the directory drove names the directory, not an administrator, as what made + the change. It closes with "If you did not expect this change, contact your MessageFoundry + administrator" rather than "If this was you". That covers at least the + directory rename, the directory's email change, a role change applied by the session reconciler + or by a directory sign-in's re-sync, and the reconciler's `account_disabled` when it read the + account disabled (vault BACKLOG #3121). The role notice says the roles were changed to match the + directory's groups, not that the console played no part: those groups map to roles through the + group-to-role map an administrator edits. Like the disable notice, it says the sessions already + open on the account were ended, which both of its senders do first. When the directory session + reconciler ends an account's sessions for a reason other than a read disabled bit, it sends + `directory_sessions_ended`, not `account_disabled` (vault BACKLOG #2140). At least + `directory_absent`, `directory_undetermined` and `directory_object_id_missing` send it. The + comment on `DIRECTORY_SESSIONS_ENDED` in `auth/notifications.py` says what each one means. For at + least `directory_object_id_missing` the engine never asks the directory (BACKLOG #2434). So the + notice names no directory check. It says only that the engine could not confirm from the + directory that the account may still sign in, which is true for each current reason. A new + reason must be checked against that wording (vault BACKLOG #3171). The rename notice, `username_changed`, names the old and the new name, and is sent only when the new name was written (BACKLOG #2017). An **unreplaced temporary password** near its deadline (ASVS 6.4.5, BACKLOG #2007) sends two reminders, beside the operator's `initial_credential_expiring` @@ -5694,7 +5736,7 @@ Every authentication and authorization event is written to the durable `audit_lo user: `auth.login_success` / `auth.login_failed` / `auth.login_locked` / `auth.logout` / `auth.login_new_ip` / `auth.login_address_unevaluated` (the first-seen sign-in address, BACKLOG #288; on a directory sign-in the row's `mech` names the leg, `kerberos` or `oidc`) / -`auth.permission_denied` / `auth.channel_denied` / `auth.self_target_refused`, the 6.3.5 events `auth.account_locked` / +`auth.permission_denied` / `auth.channel_denied` / `auth.self_target_refused` / `auth.session_revoke_missed`, the 6.3.5 events `auth.account_locked` / `auth.login_after_failures`, the re-proof rows `auth.reauth` / `auth.password_change_failed`, plus `user.created` / `user.roles_changed` / `user.channel_scope_changed` / `user.channel_scope_change_refused` / `user.deleted`, `ad_group_map.updated` / `ad_group_scope_map.updated`, and `auth.ad_scope_resynced`. PHI access (viewing a raw message or displaying patient summaries) is recorded diff --git a/ide/src/stepUp.ts b/ide/src/stepUp.ts index c73683797..f8e63787b 100644 --- a/ide/src/stepUp.ts +++ b/ide/src/stepUp.ts @@ -30,7 +30,7 @@ export class IdpStepUpRequiredError extends Error { constructor() { super( "this session signed in through the identity provider, so it confirms a sensitive action " + - "there, in a browser: re-authenticate in the web console at /ui/reauth, then try again", + "there, in a browser: repeat the action in the web console, which opens /ui/reauth for it", ); this.name = "IdpStepUpRequiredError"; } diff --git a/messagefoundry/api/_ui_seam.py b/messagefoundry/api/_ui_seam.py index 975e64c41..33e82a8ee 100644 --- a/messagefoundry/api/_ui_seam.py +++ b/messagefoundry/api/_ui_seam.py @@ -404,6 +404,12 @@ #: surface grew. An older console on this engine is refused at mount; a newer console on an older #: engine would call a method that does not exist. #: +#: Vault BACKLOG #3260, limb 2: ``AuthService.revoke_own_session`` takes a required keyword +#: ``client``, the address on its new ``auth.session_revoke_missed`` row and on its +#: ``auth.session_revoked`` row, and ``/ui/account/sessions/{id}/revoke`` passes it. Unlike the +#: ``AdminHandlers`` fields of #3259, an ``AuthService`` method is on the discovered surface with +#: its signature, so the digest moved and a skew in either direction is refused at mount. +#: #: Vault BACKLOG #2709: the digest now covers each console route's gate. The vault BACKLOG #2581 #: entry above records the gap: the console calls ``dr_activate`` and ``dr_release`` in process, #: which never runs their gates, so the console route's own gate is the only one on that path. That @@ -452,7 +458,13 @@ #: BACKLOG #3015: ``ReloadResult`` gained ``restart_required``, the settings trust anchors a #: restart must apply. The console's reload result and approval pages read it by attribute, so it #: forces a bump. Re-derived on batch/b260-c, the tree merged with BACKLOG #3091, #3105 and #2731, -#: none of which moved it. The value below covers all four. +#: none of which moved it. The value below covers all four, and vault BACKLOG #3260 above: it was +#: re-derived again on the batch 257 branch's merge of those changes. +#: +#: BACKLOG #3029: `AuthService.logout` returns a `LogoutOutcome` where it returned `None`. The +#: console's `/ui/logout` ignores the value, but the return type is part of the discovered +#: signature, so the digest moved on main with it. The value below covers it too: it was +#: re-derived on the batch 257 branch's fourth merge of main. #: #: Vault BACKLOG #2709 merged with BACKLOG #3015: each moved the digest on its own (#2709 to #: ca9d9d31c1e53c1b, #3015 to fe903ffc23dbda0d on main). The value below was re-derived with @@ -500,6 +512,15 @@ #: Re-derived with ``--write`` on the merged tree; the digest did not move, so the value below #: covers every entry above. #: +#: Vault BACKLOG #3260 merged with vault BACKLOG #2709 and #3192: each side moved the digest on its +#: own (the batch 257 branch to 41c0a8c59f83685a, main to 6d7dcabe1103f08d). The value below was +#: re-derived with ``--write`` on the batch 257 branch's merge of main at cc768e40a, so it covers +#: both. +#: +#: The batch 257 branch (at 909ed2c80270abb3) merged again with main (at da529979e55cb966, which +#: carries at least BACKLOG #3189, #3031 and #3052). The value below was re-derived with +#: ``--write`` on that merged tree, so it covers both sides. +#: #: BACKLOG #3082: ``Elevation.directory_unconfirmed`` was replaced by the new #: ``directory_refusal`` field, a ``DirectoryRefusal``, so the console can tell a lost role, which a #: new sign-in clears, from an outage, which it does not, and show each its own fix. The console @@ -523,6 +544,10 @@ #: the move comes from the merge. #: Re-derived with ``--write`` on the merged tree. #: +#: The batch 257 branch (at 51458c2cec66fd46) merged with main again (at 8b2fa06dd4c4938b, which +#: carries at least vault BACKLOG #3316). The value below was re-derived with ``--write`` on that +#: merged tree, so it covers both sides. +#: #: BACKLOG #3082 (at ecbe601fd4934dcb) merged with main again after vault BACKLOG #3316 #: (PR 2396) moved main's seam to 8b2fa06dd4c4938b. Re-derived with ``--write`` on the merged #: tree, from both values to 685dca559a2f6600, so the value below covers every entry above. @@ -535,7 +560,8 @@ #: Vault BACKLOG #2954 (and #2956, which builds on it) merged with main after the #3189 / #3031 #: entries above landed there (main at 440d71aaf, seam digest da529979e55cb966). #2956 moves no #: seam field itself. Re-derived with ``--write`` on that merged tree to 40dde1530423fece, which -#: covered the entries above it at the time, not the #3316 or #3082 entries above it now. +#: covered the entries above it at the time. It does not cover at least the #3316, #3082 and +#: batch 257 (#3260) entries above it now. #: #: Vault BACKLOG #2954 / #2956 merged with main again after BACKLOG #3316 landed there (main at #: fc0d05e1b, seam digest 8b2fa06dd4c4938b; the branch at 40dde1530423fece). The #3316 entry @@ -562,12 +588,18 @@ #: ``--write`` on the merged tree; the digest did not move and the golden matches main's, because #: #2954's field was this branch's only seam change and main already carries it. #: +#: The batch 257 branch (vault BACKLOG #3260 and its siblings, at 9dffd8fa7f1c4ba2) merged with +#: main again after BACKLOG #3082 (PR 2373) and #2954 (PR 2377) moved main's seam (main at +#: 501bfdcf5, seam digest 5b6fd34b36a52c84; PR 2359 moved no seam field). Neither parent's value +#: covered both, so the move comes from the merge. +#: Re-derived with ``--write`` on the merged tree, so the value below covers every entry above. +#: #: The digest below covers the surface DISCOVERED from the console's own imports and uses, which is #: strictly larger than the five hand-maintained tuples it replaced -- those had drifted, and the #: proof is that commit 40a4d5d9 added a REQUIRED ``UploadedFileList.scope`` field the console renders #: unconditionally while touching no seam file at all. Regenerate with #: ``python scripts/webconsole_seam_snapshot.py --write``; never hand-edit it to silence a gate. -ENGINE_UI_SEAM: str = "5b6fd34b36a52c84" +ENGINE_UI_SEAM: str = "5d9c162790077dfc" @dataclass(frozen=True, slots=True) diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index cdc9ffc35..00b89f482 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -229,6 +229,7 @@ from messagefoundry.api.request_timeout import RequestTimeoutMiddleware from messagefoundry.api.security import ( AUTH_NOT_CONFIGURED, + DIRECTORY_GRANTED_BY, AuthenticatedBeforeBodyRoute, ExportStopped, alert_sink_for, @@ -418,6 +419,7 @@ from messagefoundry.pipeline.alerts import ( AlertSink, LoggingAlertSink, + administrator_granted_alert, authority_seal_alert, no_enabled_administrator_alert, store_cipher_refusal_forwarder, @@ -10393,8 +10395,9 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin point. Their rows stand, except a row that was refused: that account has neither its row nor its alert. ``AuthService._audit_reconciler_row`` says which errors are soft. - Each standing alert but the probe fault's also gets its inverse, which pages nobody and - resolves the open instance (BACKLOG #2136): ``ad_reconcile_breaker_cleared`` and ``ad_reconcile_hold_released``, and + Each standing alert but the probe fault's and a paging refusal's also gets its inverse, which + pages nobody and resolves the open instance (BACKLOG #2136): ``ad_reconcile_breaker_cleared`` + and ``ad_reconcile_hold_released``, and ``ad_reconcile_breaker_cleared`` under the referral's source for the referral's instance. The auth service decides when a pass is evidence of a clear (``plan.breaker_clear``, ``plan.hold_clear``, ``plan.referral_clear``; see ``AuthService._mark_reconcile_clears``), so @@ -10429,7 +10432,9 @@ def _alert_reconcile_plan(plan: ReconcilePlan, auth: AuthService, sink: AlertSin sink.ad_reconcile_aborted( _REFERRAL_ALERT_SOURCE, reason=REFERRAL_ABORT, - probed=plan.probed, + # The probes ASKED (BACKLOG #3170): an UNKEYED row was never sent, so no referral + # could answer it. The breaker's count above stays its judged denominator. + probed=plan.asked, detail=auth.directory_reconcile_referral or REFERRAL_ABORT, ) if plan.faulted: @@ -11300,7 +11305,9 @@ async def _audit_convergence_failure( engine.loaded_config_fingerprint = None engine.config_baseline_unchecked = True # Raised here, before engine.start(), so new bytes that make the start itself fail - # still page on every attempt of a crash loop. Never raises. + # still page on every attempt of a crash loop. Never raises. Vault BACKLOG #3145 + # item 1 (a gate-refused restart pages on every attempt) is declined: that is this + # contract, pinned by test_the_alert_fires_even_when_the_start_then_fails (#2597). if start_config is not None and start_config.changed: _raise_config_changed( notifier or LoggingAlertSink(), engine, start_outcome, start_config @@ -11384,6 +11391,11 @@ async def _audit_convergence_failure( # Vault BACKLOG #3229: a directory role change that leaves no enabled # Administrator is applied, and raises no_enabled_administrator here. admin_loss_alert=no_enabled_administrator_alert(notifier or LoggingAlertSink()), + # Vault BACKLOG #3141: a reconcile pass that newly grants Administrator raises + # administrator_granted here, as a directory sign-in does in its route. + admin_grant_alert=administrator_granted_alert( + notifier or LoggingAlertSink(), granted_by=DIRECTORY_GRANTED_BY + ), ) # Started only once the service is built, so a constructor refusal starts no task. if security_notifier is not None: @@ -11393,7 +11405,10 @@ async def _audit_convergence_failure( # any connection starts rather than after. Both touch only the store: initialize() # seeds the built-in roles and the gate reads the users. A refusal here also comes # before the start's opt-in audit-chain walk, so that walk cannot alert on such a - # start. Everything else that uses the service stays below the start. + # start. Vault BACKLOG #3145 item 2 is declined: the walk runs inside engine.start(), + # and moving it ahead of this gate reorders the start whose failure paging + # test_the_alert_fires_even_when_the_start_then_fails pins (#2597). Everything else + # that uses the service stays below the start. await auth.initialize() app.state.auth = auth await _assert_security_notice_is_deliverable( @@ -11521,9 +11536,12 @@ async def _audit_upload_prune(meta: UploadedFileMeta) -> None: # ADR 0197 Amendment A, Manager decision 2026-09-29: learn at start, not at the first # account creation, that the password policy leaves no credential to issue. It logs # an ERROR and returns, and is guarded like the census: a policy-level fact must - # never refuse the start, whatever a later policy clause raises. + # never refuse the start, whatever a later policy clause raises. It runs off the loop + # (vault BACKLOG #3164): a pathological context-word list makes the generator retry + # its policy screens many times, and the engine's listeners and workers already run + # on the loop by now. try: - auth.probe_credential_generation() + await asyncio.to_thread(auth.probe_credential_generation) except Exception: # noqa: BLE001 -- see the comment above _log.exception( "the credential-generation probe could not run; startup continues" diff --git a/messagefoundry/api/auth_models.py b/messagefoundry/api/auth_models.py index 44bf3467f..a60bc941d 100644 --- a/messagefoundry/api/auth_models.py +++ b/messagefoundry/api/auth_models.py @@ -34,6 +34,13 @@ # A session token is 43 URL-safe characters (auth/tokens.py); the cap only refuses absurd input. _TOKEN_MAX = 256 +#: The response header an ``oidc`` session's step-up refusal carries, set to +#: :data:`STEP_UP_VIA_IDP`, so a client can branch on a header rather than parse the detail +#: (BACKLOG #2158). Here, with the auth wire models, so the engine's raise sites and the +#: FastAPI-free ``apiclient`` share one spelling (vault BACKLOG #3145 item 6). +STEP_UP_VIA_HEADER = "X-Step-Up-Via" +STEP_UP_VIA_IDP = "idp" + class CredentialReply(BaseModel): """A response model whose body carries a live credential: a session token, a staged TOTP seed, diff --git a/messagefoundry/api/auth_routes.py b/messagefoundry/api/auth_routes.py index c1967bdb8..17a195be8 100644 --- a/messagefoundry/api/auth_routes.py +++ b/messagefoundry/api/auth_routes.py @@ -29,6 +29,8 @@ from messagefoundry.api._ui_seam import AdminHandlers from messagefoundry.api.auth_models import ( _GROUP_MAX, + STEP_UP_VIA_HEADER, + STEP_UP_VIA_IDP, AdGroupMap, AdGroupMapEntry, AdGroupScopeEntry, @@ -795,13 +797,14 @@ async def reauth( # the message names the leg that works. The header is the one the step-up gates # send such a session (BACKLOG #2158), so a client branches on it here too rather # than parsing the detail. No X-Step-Up-Required: this is the step-up itself. - # Typed out because api/security.py keeps its constant private; a test pins the - # name and the value to what the gates send. + # The constants the gates send too (vault BACKLOG #3145); a test pins the two + # to each other. raise HTTPException( status.HTTP_403_FORBIDDEN, "this session was signed in through the identity provider; re-authenticate" - " there through the web console at /ui/reauth, not with a password", - headers={"X-Step-Up-Via": "idp"}, + " there, not with a password: repeat the action in the web console, which" + " opens /ui/reauth for it", + headers={STEP_UP_VIA_HEADER: STEP_UP_VIA_IDP}, ) if elevation.directory_refusal is not None: # BACKLOG #2027: the directory could not judge the password, so "failed" would @@ -852,8 +855,9 @@ async def mfa_verify( raise HTTPException( status.HTTP_403_FORBIDDEN, "this session was signed in through the identity provider and owes no code;" - " step up there through the web console at /ui/reauth", - headers={"X-Step-Up-Via": "idp"}, + " step up there by repeating the action in the web console, which opens" + " /ui/reauth for it", + headers={STEP_UP_VIA_HEADER: STEP_UP_VIA_IDP}, ) if elevation.directory_refusal is not None: # BACKLOG #2023: the code was never checked, and the token still authenticates, so a @@ -1028,6 +1032,7 @@ async def count(pin: float | None) -> int: @app.delete("/me/sessions/{session_id}", response_model=SimpleMessage) async def revoke_my_session( session_id: DigestId, + request: Request, service: AuthService = Depends(_service), # 7.5.2 (ASVS): terminating a session needs a fresh PASSWORD re-proof BOUND TO THIS ACTION # (BACKLOG #1149) — single-use, so the login-seeded window no longer satisfies it. Still the @@ -1037,7 +1042,11 @@ async def revoke_my_session( identity: Identity = Depends(require_reauth_only_action(STEP_UP_ACTION_SESSION_TERMINATE)), ) -> SimpleMessage: # Ownership-checked in the service: a 404 (not 403) avoids confirming another user's session id. - if not await service.revoke_own_session(identity, session_id, actor=identity.username): + # A miss writes one audit row there, the same for an unknown and a foreign id (vault + # BACKLOG #3260, limb 2), and the answer here is unchanged. + if not await service.revoke_own_session( + identity, session_id, actor=identity.username, client=client_ip(request) + ): raise HTTPException(status.HTTP_404_NOT_FOUND, "no such session") return SimpleMessage(detail="session revoked") diff --git a/messagefoundry/api/security.py b/messagefoundry/api/security.py index 65250965b..5b65fe3a4 100644 --- a/messagefoundry/api/security.py +++ b/messagefoundry/api/security.py @@ -36,11 +36,12 @@ from starlette.requests import HTTPConnection from starlette.routing import BaseRoute -from messagefoundry.api.auth_models import CredentialReply +from messagefoundry.api.auth_models import STEP_UP_VIA_HEADER, STEP_UP_VIA_IDP, CredentialReply from messagefoundry.api.tls_client_cert import ( MF_CLIENT_PEERCERT_STATE_KEY, peercert_from_ssl_object, ) +from messagefoundry.audit_write import AUDIT_WRITE_DEFECTS from messagefoundry.auth import AuthProvider, Identity, Permission, Role from messagefoundry.auth.notifications import deadline_utc as deadline_utc # re-export from messagefoundry.auth.service import AuthService, LoginOutcome @@ -54,6 +55,7 @@ from messagefoundry.pipeline.alerts import AlertSink, LoggingAlertSink from messagefoundry.pipeline.cert_expiry import peer_cert_expiry from messagefoundry.redaction import safe_exc +from messagefoundry.store.base import store_call_errors from messagefoundry.store.store import UserRecord log = logging.getLogger(__name__) @@ -1229,11 +1231,9 @@ async def dependency(request: Request) -> Identity: #: session (BACKLOG #2142, ADR 0142 Amendment F). _IDP_STEP_UP_DETAIL = ( "step-up re-verification required; this session steps up at the identity provider, in a" - " browser, through the web console at /ui/reauth, not with a password" + " browser, not with a password: repeat the action in the web console, which opens /ui/reauth" + " for it" ) -#: Set to ``idp`` on an ``oidc`` session's step-up refusal, so a client can branch on a header -#: rather than parse the detail. Absent on every other refusal. -_STEP_UP_VIA_HEADER = "X-Step-Up-Via" async def _step_up_refusal( @@ -1248,8 +1248,27 @@ async def _step_up_refusal( headers = {"X-Step-Up-Required": "1"} if action is not None: headers["X-Step-Up-Action"] = action - if await auth.session_steps_up_at_idp(token): - headers[_STEP_UP_VIA_HEADER] = "idp" + try: + via_idp = await auth.session_steps_up_at_idp(token) + except AUDIT_WRITE_DEFECTS: + raise + except store_call_errors() as exc: + # Vault BACKLOG #3145 item 6: the read only picks the wording, and the gate has already + # refused, so a store fault keeps the 403 rather than becoming a 500. It answers with the + # password wording; an oidc client's POST /me/reauth then asks again and is told the IdP. + # With its traceback: ValueError and RuntimeError are also what a bug raises, and the log + # line is the only place a fault and a defect can be told apart. + log.warning( + "a step-up refusal could not read the session's sign-in mechanism (%s); it answers" + " with the password wording", + type(exc).__name__, + exc_info=True, + ) + via_idp = False + if via_idp: + # Set to ``idp`` on an ``oidc`` session's step-up refusal, so a client can branch on a + # header rather than parse the detail. Absent on every other refusal. + headers[STEP_UP_VIA_HEADER] = STEP_UP_VIA_IDP return HTTPException(status.HTTP_403_FORBIDDEN, _IDP_STEP_UP_DETAIL, headers=headers) return HTTPException(status.HTTP_403_FORBIDDEN, _STEP_UP_DETAIL, headers=headers) diff --git a/messagefoundry/apiclient/client.py b/messagefoundry/apiclient/client.py index 0d8c58d09..818f437f7 100644 --- a/messagefoundry/apiclient/client.py +++ b/messagefoundry/apiclient/client.py @@ -29,6 +29,8 @@ from pydantic import BaseModel, ValidationError from messagefoundry.api.auth_models import ( + STEP_UP_VIA_HEADER, + STEP_UP_VIA_IDP, AdGroupMap, AdGroupMapEntry, AuditList, @@ -133,6 +135,16 @@ # handlers by test_apiclient_approval_hold.py, which reads the code back rather than trusting 202. _HTTP_PENDING_APPROVAL = 202 +#: How long the reply to a request that can wait in the engine's per-account queue may take, in +#: seconds (vault BACKLOG #2950): the sign-in, and the elevations that rotate the token, which +#: queue on that account too: ``reauth``, ``verify_mfa`` and ``confirm_mfa`` (each rotation runs +#: under the account's re-proof lock, and ``verify_mfa`` also under its credential lock). Matches +#: the IDE's ``SIGN_IN_TIMEOUT_MS`` (``ide/src/signInSession.ts``). The queue has no bound +#: (``keyed_lock.hold_keyed_lock`` takes no timeout), so no value covers every wait: this makes a +#: lost reply rarer than the 5 s default did, and does not rule one out. It raises the read phase +#: only (:meth:`EngineClient._sign_in_timeout`); every other request keeps the client's timeout. +SIGN_IN_TIMEOUT_SECONDS = 30.0 + class ApiError(RuntimeError): """An API call failed (transport error, a non-2xx response, or an undecodable 2xx body).""" @@ -147,8 +159,10 @@ class IdpStepUpRequired(ApiError): Raised on a 403 carrying ``X-Step-Up-Via: idp``, which the engine sends for a session minted by the federated (``oidc``) sign-in. ``POST /me/reauth`` refuses such a session, so this client - raises instead of prompting for a password. Its step-up runs in the web console at - ``/ui/reauth``, through the IdP, and this client cannot perform it: that leg needs a browser and + raises instead of prompting for a password. Its step-up runs in the web console, through the + IdP: repeating the action there sends the browser to ``/ui/reauth`` with that action named + (a bare ``/ui/reauth`` names none and returns to ``/ui``; vault BACKLOG #3145). This client + cannot perform it: that leg needs a browser and re-keys the session it steps up. A subclass of :class:`ApiError`, so a caller that catches only that still sees the refusal.""" @@ -1034,8 +1048,13 @@ def _request( **kw, ) # BACKLOG #2158: an ``oidc`` session steps up at the IdP, in a browser. POST /me/reauth - # refuses it, so the password prompt below would only earn a second 403. - if response.status_code == 403 and response.headers.get("X-Step-Up-Via") == "idp": + # refuses it, so the password prompt below would only earn a second 403. Vault BACKLOG + # #3145 item 6 asked to run the registered handler first; declined, because that handler + # can only re-prove a password, which is the second 403. + if ( + response.status_code == 403 + and response.headers.get(STEP_UP_VIA_HEADER) == STEP_UP_VIA_IDP + ): raise IdpStepUpRequired(_error_detail(response), status=response.status_code) # Step-up re-verification (ASVS 7.5.3): the engine refuses a sensitive op with 403 + # X-Step-Up-Required when this session hasn't re-proved its credential recently. Prompt the @@ -1065,6 +1084,13 @@ def _request( raise ApiError(_error_detail(response), status=response.status_code) return response + def _sign_in_timeout(self) -> httpx.Timeout: + """This client's timeout with only the READ phase raised to :data:`SIGN_IN_TIMEOUT_SECONDS` + (vault BACKLOG #2950). The engine's per-account queue is waited out after the request is + sent, so connect, write and pool keep the client's own bound: a caller on a GUI thread + still learns of an unreachable engine as fast as before.""" + return httpx.Timeout(self._timeout, read=SIGN_IN_TIMEOUT_SECONDS) + def _adopt_rotated(self, response: httpx.Response) -> None: """Adopt the re-keyed session token an elevation route hands back (ASVS 7.2.4). @@ -1115,7 +1141,17 @@ def reauth(self, password: str, *, purpose: str | None = None) -> None: body: dict[str, str] = {"password": password} if action is not None: body["purpose"] = action - self._adopt_rotated(self._request("POST", "/me/reauth", json=body, _allow_step_up=False)) + # Vault BACKLOG #2950: it rotates the token and queues behind the account's re-proof lock, + # so it gets the sign-in's timeout (see SIGN_IN_TIMEOUT_SECONDS). + self._adopt_rotated( + self._request( + "POST", + "/me/reauth", + json=body, + _allow_step_up=False, + timeout=self._sign_in_timeout(), + ) + ) def set_mfa_handler(self, handler: Callable[[], bool] | None) -> None: """Register the callback invoked when the engine demands a second factor (403 + @@ -1149,7 +1185,11 @@ def confirm_mfa(self, code: str) -> list[str]: ``MfaConfirmResponse.token`` is a required ``str`` today, so a token-less body fails in ``_decode`` rather than clearing the session. That is a property of a different file, not of the rule, and the rule is the one thing all three sites must share.""" - response = self._request("POST", "/me/mfa/confirm", json={"code": code}) + # Vault BACKLOG #2950: the rotation queues behind the account's re-proof lock, and a lost + # reply here also loses the one-time recovery codes, so it gets the sign-in's timeout. + response = self._request( + "POST", "/me/mfa/confirm", json={"code": code}, timeout=self._sign_in_timeout() + ) self._adopt_rotated(response) return _decode(response, MfaConfirmResponse).recovery_codes @@ -1160,8 +1200,16 @@ def verify_mfa(self, code: str) -> None: for an ``oidc`` session that already met its factor, whose code was not checked either (BACKLOG #2142). Does not itself trigger the MFA handler.""" self._refuse_credential_on_cleartext("a second factor") + # Vault BACKLOG #2950: it rotates the token and queues behind the account's credential + # lock, so it gets the sign-in's timeout (see SIGN_IN_TIMEOUT_SECONDS). self._adopt_rotated( - self._request("POST", "/auth/mfa-verify", json={"code": code}, _allow_mfa=False) + self._request( + "POST", + "/auth/mfa-verify", + json={"code": code}, + _allow_mfa=False, + timeout=self._sign_in_timeout(), + ) ) def disable_mfa(self) -> None: @@ -1642,10 +1690,12 @@ def login( ends the old one, and the reply then fails to arrive, times out or will not decode, this client still holds the old token, which is dead, and never learns the new one. ``login`` raises :class:`ApiError` and every later call on the held token is refused - with a 401 until the caller signs in again. A timeout is the likely way in: the client - timeout (5 s per phase by default) also bounds this request, and a sign-in waiting in the - engine's per-account queue can take longer, while the engine goes on to finish it. - Before, the old token stayed live in that case. The new session nobody holds stays valid + with a 401 until the caller signs in again. A timeout is the likely way in: a sign-in + waiting in the engine's per-account queue can outlast the client, while the engine goes + on to finish it. This request therefore waits :data:`SIGN_IN_TIMEOUT_SECONDS` (30 s) for + its reply rather than the client's 5 s default (vault BACKLOG #2950). That makes a lost + reply rarer, not impossible: the queue has no bound, so a long enough queue still + outlasts it. Before, the old token stayed live in that case. The new session nobody holds stays valid until it idles out or expires, as it did before. This is the accepted price of ending the old session ahead of the cap. It is not harmless. @@ -1657,7 +1707,10 @@ def login( prior = self._token if prior and self._token_cell.issued_here: body["supersedes"] = prior - result = _decode(self._request("POST", "/auth/login", json=body), LoginResponse) + result = _decode( + self._request("POST", "/auth/login", json=body, timeout=self._sign_in_timeout()), + LoginResponse, + ) self._hold_issued(result.token) self._user = result.user # A step-up purpose stashed for the old session must not bind the new one's next reauth(). diff --git a/messagefoundry/auth/notifications.py b/messagefoundry/auth/notifications.py index 96e93a62d..323fbf3d6 100644 --- a/messagefoundry/auth/notifications.py +++ b/messagefoundry/auth/notifications.py @@ -50,7 +50,10 @@ TEMPORARY_CREDENTIAL_EXPIRING = "temporary_credential_expiring" TEMPORARY_CREDENTIAL_EXPIRING_FOR_ISSUER = "temporary_credential_expiring_issuer" EMAIL_CHANGED = "email_changed" # 6.3.7 — the account's email address was changed -ROLES_CHANGED = "roles_changed" # 6.3.7 — an admin changed the account's roles +# 6.3.7 -- the account's roles changed: an administrator's act, or a re-sync from the directory's +# groups, which the reconciler and the sign-in re-sync tag ``detail["source"] == "directory"`` +# (vault BACKLOG #3121). +ROLES_CHANGED = "roles_changed" # 6.3.7 -- the directory renamed the account and the engine copied the new name onto the row # (BACKLOG #2017). When it is sent is stated once, at the send site in # ``AuthService._refresh_cached_username``. ``detail`` carries ``old_username``, ``new_username`` and @@ -67,7 +70,9 @@ # address the account held BEFORE the takeover, because a moved address belongs to the operator who # ran the command, not to the holder being told. FIRST_ADMINISTRATOR_TAKEOVER = "first_administrator_takeover" -ACCOUNT_DISABLED = "account_disabled" # 6.3.7 — an admin disabled the account +# 6.3.7 -- an administrator disabled the account, or the reconciler read it disabled in the +# directory, tagged ``detail["source"] == "directory"`` (vault BACKLOG #3121). +ACCOUNT_DISABLED = "account_disabled" # 6.3.7 -- the directory session reconciler ended the account's sessions for a reason that is NOT a # read disabled bit. At least these send it: the probe could not match the account to exactly one # entry (``directory_absent``: not found, more than one entry, or its stored id did not match or @@ -75,8 +80,8 @@ # vault BACKLOG #2140), or the row carries no ``directory_object_id``, so the pass never asked the # directory (``directory_object_id_missing``; BACKLOG #2434). Any whole-account reason added later # sends it too, so the notice's one wording must be checked against that one as well (vault BACKLOG -# #3171). ACCOUNT_DISABLED would tell the holder an administrator disabled the account, which -# nothing established. Which reason sends which kind is decided once, in +# #3171). ACCOUNT_DISABLED would tell the holder the account is disabled, by an administrator or in +# the directory, which nothing established. Which reason sends which kind is decided once, in # ``AuthService._apply_reconcile_revocation``. DIRECTORY_SESSIONS_ENDED = "directory_sessions_ended" MFA_ENABLED = "mfa_enabled" # 6.3.7 — a second factor (TOTP) was enrolled on the account diff --git a/messagefoundry/auth/oidc_http.py b/messagefoundry/auth/oidc_http.py index cce854392..dd88c86b8 100644 --- a/messagefoundry/auth/oidc_http.py +++ b/messagefoundry/auth/oidc_http.py @@ -42,6 +42,7 @@ import ssl import urllib.request from collections.abc import Callable +from typing import TYPE_CHECKING from messagefoundry.auth.oidc.jwks import _MAX_JWKS_BYTES from messagefoundry.auth.trust_anchors import oidc_anchor_spec, verified_anchor_cadata @@ -60,11 +61,15 @@ reply_framing_fault, ) +if TYPE_CHECKING: + from messagefoundry.config.settings import AuthSettings + __all__ = [ "IdpReplyCutShortError", "IdpReplyError", "IdpReplyUnusableError", "build_idp_opener", + "build_idp_opener_from_settings", "is_idp_transport_http_exception", "jwks_fetcher", ] @@ -184,6 +189,28 @@ def build_idp_opener( return build_strict_opener(_NoRedirectHandler, urllib.request.HTTPSHandler(context=ctx)) +def build_idp_opener_from_settings( + settings: AuthSettings, *, enforcing: bool, anchor: bool = True +) -> urllib.request.OpenerDirector: + """:func:`build_idp_opener` with its arguments read from ``[auth]``, so the engine and the + federation check cannot read the anchor, pin or CRL file differently (vault BACKLOG #3145 item + 3). Only the argument assembly is shared: each caller keeps its own answer to a failure, a + refused start in ``AuthService`` and a check row in ``verify.federation``. + + ``anchor=False`` leaves the anchor and its pin out, for the federation check's revocation row + alone. The CRL file is the hop's own knob (BACKLOG #299) and is passed either way. + + The settings preamble the check legs in ``messagefoundry/checks.py`` repeat is not shared here: + that refactor is declined, because it would collide with vault BACKLOG #3063 on the same + helper.""" + return build_idp_opener( + settings.oidc_tls_ca_cert_file if anchor else None, + pin=settings.oidc_tls_ca_cert_pin if anchor else None, + enforcing=enforcing, + crl_file=settings.oidc_tls_crl_file, + ) + + def jwks_fetcher( jwks_uri: str, opener: urllib.request.OpenerDirector, diff --git a/messagefoundry/auth/reconcile.py b/messagefoundry/auth/reconcile.py index 3e92c0861..9f50743af 100644 --- a/messagefoundry/auth/reconcile.py +++ b/messagefoundry/auth/reconcile.py @@ -33,13 +33,15 @@ A referral is not an outage, and not a reason to stop the pass either (BACKLOG #2538). A :attr:`ProbeOutcome.REFERRED` account is left unjudged, like an unreachable one, and every other probe in the pass is judged as usual. Unlike an outage, a referral alerts. A pass whose only answers are -referrals and failures aborts as ``directory_referral``, which judges nothing and alerts. +referrals and failures aborts as ``directory_referral``, which judges no account it asked about +and alerts. Such a pass, like an outage, still strikes an UNKEYED row (BACKLOG #3170). A probe fault is not an outage either (BACKLOG #3100). A :attr:`ProbeOutcome.FAULT` account, whose probe raised something that is not an LDAP error, or a response ldap3 could not decode, is left unjudged like an unreachable one, and unlike one it alerts on every pass that probes it. A pass whose only answers are faults and failures, -at least one a fault, aborts as ``probe_fault``, which judges nothing and alerts. +at least one a fault, aborts as ``probe_fault``, which judges no account it asked about and alerts. +It too still strikes an UNKEYED row (BACKLOG #3170). The pass also re-diffs channel scope (ADR 0198, BACKLOG #1957). It revokes when the directory would withdraw or narrow a principal's scope, and it never writes the scope: the next login does, through @@ -229,14 +231,19 @@ class ReconcilePlan: #: count``. A PRESENT probe maps to 0 (reset), and so does a HELD one (ADR 0195 rule item 7; #: ``held`` tells the two apart). An UNAVAILABLE, REFERRED or FAULT probe is absent from this mapping, #: leaving whatever strike the user already carried untouched. Populated even on a breaker - #: abort — see :func:`plan_pass`. + #: abort — see :func:`plan_pass`. On an outage or a pass of referrals or faults only it holds + #: the UNKEYED rows' strikes alone (BACKLOG #3170), since those were judged without the + #: directory. strikes: Mapping[str, int] = field(default_factory=dict) #: Candidates this pass judged, asked or not. Excludes the per-pass budget remainder. An UNKEYED #: row counts here, because it can revoke. This is the breaker's denominator (``judged``). probed: int = 0 #: Probes this pass actually sent to the directory: ``probed`` less its UNKEYED rows (BACKLOG - #: #2434). An outage and a pass of referrals or faults only are judged over these, and the outage's log - #: line and the ``asked`` field of its ``auth.ad_reconcile_skipped`` row count these. + #: #2434). An outage and a pass of referrals or faults only are judged over these, and the + #: outage's log line and the ``asked`` field of its ``auth.ad_reconcile_skipped`` row count + #: these. So do the referral's latched message, the ``asked`` field of its + #: ``auth.ad_reconcile_referred`` row and its alert's count (BACKLOG #3170): a referral can + #: only answer a probe that was sent. asked: int = 0 unavailable: int = 0 #: ``user_id`` of every probe the directory answered with a referral (BACKLOG #2538). Any one @@ -311,7 +318,9 @@ def probe_fault(self) -> bool: def judged_nothing(self) -> bool: """True when the pass aborted before judging any account: an outage, a pass of referrals and failures only, or one of faults and failures only. Such a pass leaves the hold's state, - its message and every clear alone, because it learned nothing about the accounts.""" + its message and every clear alone, because it learned nothing about the accounts it asked + about. It still strikes an UNKEYED row, which was judged without the directory (BACKLOG + #3170).""" return self.directory_outage or self.directory_referral or self.probe_fault @property @@ -399,6 +408,20 @@ def select_candidates( return ordered[: max(budget, 0)] +def _unkeyed_strikes(probes: list[Probe], prior_strikes: Mapping[str, int]) -> dict[str, int]: + """The strikes of a pass that judged nothing (BACKLOG #3170 item 3). + + An UNKEYED row was judged without the directory, so an outage or a pass of referrals or faults + only (BACKLOG #3100) still strikes it, as any other pass does. Only the strike: its outcome is + not recorded, so the hold's record, and so its carry-over, stays as it was, and an aborted pass + revokes nothing.""" + return { + p.user_id: prior_strikes.get(p.user_id, 0) + 1 + for p in probes + if p.outcome is ProbeOutcome.UNKEYED + } + + def plan_pass( probes: Iterable[Probe], *, @@ -442,6 +465,7 @@ def plan_pass( # user search base in another domain of the forest refers every probe, and an entry or a # defect that raises does so on every pass. A referral wins over a fault: it names the fix. return ReconcilePlan( + strikes=_unkeyed_strikes(probes, prior_strikes), probed=len(probes), asked=asked, unavailable=len(unavailable), @@ -458,6 +482,7 @@ def plan_pass( # zero-revocation pass. The hold is not judged on a pass that learned nothing, so its state # carries over. return ReconcilePlan( + strikes=_unkeyed_strikes(probes, prior_strikes), probed=len(probes), asked=asked, unavailable=len(unavailable), @@ -562,6 +587,9 @@ def plan_pass( readable=readable, ) # Held and referred probes both stay in the breaker's denominator; ReconcilePlan.judged says why. + # UNKEYED revocations stay in its numerator, so while such rows hold sessions they push toward + # a trip on every pass. Exempting them is declined (BACKLOG #3170 item 2): it would change what + # the mass-revoke breaker counts, a security control, and that is not a follow-up's call. if breaker_tripped( revoke_count=len(revocations), probed=plan.judged, diff --git a/messagefoundry/auth/service.py b/messagefoundry/auth/service.py index 4d76af0ad..807121b5e 100644 --- a/messagefoundry/auth/service.py +++ b/messagefoundry/auth/service.py @@ -20,6 +20,7 @@ import logging import math import os +import re import secrets import socket import time @@ -100,7 +101,7 @@ ) from messagefoundry.auth.oidc import PendingFlow from messagefoundry.auth.oidc_http import ( - build_idp_opener, + build_idp_opener_from_settings, is_idp_transport_http_exception, jwks_fetcher, ) @@ -139,7 +140,7 @@ from messagefoundry.credential import constant_time_equal from messagefoundry.keyed_lock import KeyedLock as _KeyedLock from messagefoundry.keyed_lock import hold_keyed_lock as _hold_keyed_lock -from messagefoundry.store.base import AdminStore, store_driver_errors, store_open_errors +from messagefoundry.store.base import AdminStore, store_call_errors, store_open_errors from messagefoundry.store.crypto import MARKER_PREFIX, CipherError from messagefoundry.store.row_seal import ( ADSET_KIND, @@ -229,14 +230,16 @@ def _audit_write_errors() -> tuple[type[Exception], ...]: """What a refused audit write raises, at least on the store backends and key modes named here (BACKLOG #2137). - The drivers' own errors, plus the two that :func:`~messagefoundry.store.base.store_driver_errors` - asks a caller to add: ``RuntimeError`` for the engine's own refusals (the acquire timeout, a - keyless audit append) and ``OSError`` for a connection lost at the socket. And ``CipherError``, - which a ``vault_transit`` store raises when Transit cannot compute the audit-chain MAC (ADR - 0138). Built on first use, because naming a server driver imports it; an ``AuthService`` that + :func:`~messagefoundry.store.base.store_call_errors`: the drivers' own errors, ``RuntimeError`` + for the engine's own refusals (the acquire timeout, a keyless audit append), ``OSError`` for a + connection lost at the socket, and ``ValueError``, which aiosqlite raises for a closed + connection before sqlite3 is reached. And ``CipherError``, which a ``vault_transit`` store + raises when Transit cannot compute the audit-chain MAC (ADR 0138). A caller re-raises + :data:`~messagefoundry.audit_write.AUDIT_WRITE_DEFECTS` first where a bug must not read as a + refusal. Built on first use, because naming a server driver imports it; an ``AuthService`` that runs the reconciler builds it at construction, so that import never runs on the event loop in the middle of a store incident.""" - return (RuntimeError, OSError, CipherError, *store_driver_errors()) + return (CipherError, *store_call_errors()) @cache @@ -1056,11 +1059,14 @@ class Elevation: the session was ended because its temporary password lapsed, with the proof checked or not, as that ``auth.temp_password_expired`` row records; or (``confirm_mfa_enrollment`` only, BACKLOG #2224) the proof was good but turning TOTP on matched no row, so the confirm ended - the session it had just rotated, as its ``auth.mfa_enroll_refused`` row records; or (the TOTP - and passkey legs, BACKLOG #3068) the session or its account went while the leg waited on the + the session it had just rotated, as its ``auth.mfa_enroll_refused`` row records. That row's + ``session_ended`` is false when the store refused the revoke (vault BACKLOG #3113): the + rotated session then still exists, but its token was never handed back. Or (the TOTP and + passkey legs, BACKLOG #3068) the session or its account went while the leg waited on the directory lookup, the account queue or the proof check, with the proof checked or not. Fails - CLOSED: no token is handed back. Held apart from a wrong proof so a route answers 401 rather than re-prompting on a - session that no longer exists. The ``auth.reauth`` row's ``session_revoked`` says which. + CLOSED: no token is handed back. Held apart from a wrong proof so a route answers 401 rather + than re-prompting on a session that is gone or unreachable. For a password re-auth, the + ``auth.reauth`` row's ``session_revoked`` says which cause applied. ``recovery_codes`` is populated only by :meth:`AuthService.confirm_mfa_enrollment` (shown once). @@ -1072,7 +1078,7 @@ class Elevation: incorrect. A password re-proof is never refused for a lock (BACKLOG #1138). ``ok`` is DERIVED, not stored, and that is load-bearing rather than tidiness. Held as a field it - was a second spelling of ``token is not None`` -- true of all 19 constructions -- so the type + was a second spelling of ``token is not None`` -- true of every construction then -- so the type could represent a state the system never produces, mypy could not narrow ``token`` from ``ok``, and every consuming site paid ``if not elevation.ok or elevation.token is None``: a second clause whose only job was to re-derive the first in a form the checker accepts. As a property the @@ -1837,17 +1843,25 @@ def _holds_federated_binding(user: UserRecord) -> bool: return user.oidc_issuer is not None or user.oidc_subject is not None +def _directory_key(user: UserRecord) -> str | None: + """The ``directory_object_id`` the engine may ask the directory about ``user`` by, or ``None`` + when the row carries none (an empty value counts as none). :func:`_holds_directory_key` states + the rule; a caller that goes on to send the id takes it from here, so the type narrows.""" + return user.directory_object_id or None + + def _holds_directory_key(user: UserRecord) -> bool: - """Whether the reconciler may ask the directory about ``user``: it carries a + """Whether the engine may ask the directory about ``user``: it carries a ``directory_object_id`` (BACKLOG #2434). - **The reconcile pass's one statement of the rule**; the sign-in, step-up and bind refusals - still test the column inline. A row without one is never asked about by name, so it is no - directory evidence: the pass reads it as UNKEYED without a lookup, and a trip or a referral - leaves it unmarked, because no pass can ever read it PRESENT. ``reconcile.plan_pass`` sees only - the UNKEYED outcome this produces, and counts what was asked from that. + **The one statement of the rule**: the reconcile pass and the sign-in, step-up and bind + refusals all read it (BACKLOG #3170 item 7). A row without one is never asked about by name, + so on the reconcile pass it is no directory evidence: the pass reads it as UNKEYED without a + lookup, and a trip or a referral leaves it unmarked, because no pass can ever read it PRESENT. + ``reconcile.plan_pass`` sees only the UNKEYED outcome this produces, and counts what was asked + from that. """ - return bool(user.directory_object_id) + return _directory_key(user) is not None def _holds_unkeyed_federated_binding(user: UserRecord) -> bool: @@ -2374,6 +2388,13 @@ def _floor_exempt_marker(floor_exempt_amr: Sequence[str]) -> dict[str, object]: #: current credential's is the newest, so a page this size is ample. _REMINDER_MARK_PAGE: Final = 50 +#: Vault BACKLOG #3260, limb 2 (owner ruling 2026-10-16): the row a self-service revoke of one +#: session writes when it ends nothing, because the id names no session or another account's. +#: One action for both, and the same detail, so the row cannot tell the two apart either. +SESSION_REVOKE_MISSED_ACTION: Final = "auth.session_revoke_missed" +#: What a session id can look like: a token hash, lower-case hex, cut to at most its full length. +_SESSION_ID_SHAPE: Final = re.compile(r"[0-9a-f]{1,64}") + #: Vault BACKLOG #3260: the row an admin route writes when it refuses the caller's own account. #: Its detail's ``op`` names the route, one of :data:`SelfTargetOp`. SELF_TARGET_REFUSED_ACTION: Final = "auth.self_target_refused" @@ -2720,6 +2741,7 @@ def __init__( hop_posture: HopPosture | None = None, integrity_alert: Callable[[str], None] | None = None, admin_loss_alert: Callable[[str, str], None] | None = None, + admin_grant_alert: Callable[[str, str], None] | None = None, ) -> None: self._store = store # A copy: the caller's object is also stashed as app.state.auth_settings, and the section @@ -2734,6 +2756,9 @@ def __init__( # Vault BACKLOG #3229: called with (username, via) when a directory-driven role change leaves # no enabled Administrator. Injected by the API lifespan, as above. None logs only. self._admin_loss_alert = admin_loss_alert + # Vault BACKLOG #3141: called with (username, via) when a reconcile pass's role write newly + # gives an account Administrator. Injected by the API lifespan, as above. None logs only. + self._admin_grant_alert = admin_grant_alert # Session token-hash prefix -> monotonic time its seal failure was last reported, so one # bad row presented on every request writes one audit row and one alert per interval. self._seal_failures_reported = SealFailureThrottle() @@ -3006,6 +3031,9 @@ def __init__( # rename refresh's holder re-read, which catches `_session_write_errors` (#3070). _audit_write_errors() _session_write_errors() + # The same for the soft catches that read store_call_errors (a refused enrolment's revoke, + # the step-up refusal's session read), whichever plane meets a store fault first. + store_call_errors() # Advisory, NON-STICKY federated-IdP health (ADR 0142 AC-8) — see the oidc_available docstring. self._oidc_unavailable_reason: str | None = None self._oidc_client_auth: oidc.ClientAuthentication | None = None @@ -3026,13 +3054,10 @@ def __init__( # preflight honors [security].enforcement (warn ≠ refuse) — a pin mismatch always refuses, # a group/world-writable DACL refuses only at enforce. Without the dial this seam would # hardcode enforce and make warn-mode startup unreachable for the OIDC anchor. - self._oidc_opener = build_idp_opener( - settings.oidc_tls_ca_cert_file, - pin=settings.oidc_tls_ca_cert_pin, - enforcing=self._trust_anchors_enforcing, - # BACKLOG #299: revocation checking against the IdP certificate. This hop resolves no - # trust anchor, so it carries its own CRL setting rather than inheriting [tls].crl_file. - crl_file=settings.oidc_tls_crl_file, + # BACKLOG #299: the CRL file is this hop's own knob, not [tls].crl_file; the helper + # passes it. The federation check builds through the same helper (vault #3145). + self._oidc_opener = build_idp_opener_from_settings( + settings, enforcing=self._trust_anchors_enforcing ) # BACKLOG #1887: must follow the opener, whose finished context it reads. _refuse_idp_revocation(settings, self._oidc_opener, hop_posture) @@ -3609,15 +3634,19 @@ async def provision_first_administrator( # another run that enabled TOTP first, or another run that staged its own secret, since # the secret was staged above makes it match no row. Refused before the role, so the # row stays roleless and a re-run completes it. + # Each enable is bound to the cell its own run staged, so no other run can turn on + # THIS run's secret, and the entry it showed is never the live one: the words say to + # remove it. (Vault BACKLOG #3113 item 1 had them say keep it, for the re-stage race + # the #3025 compare-and-set closed.) if not await self._store.enable_totp( user_id, recovery_code_hashes=hashes, expected_secret_cell=staged_cell ): raise FirstAdministratorRefused( "the authenticator enrolment was changed while this command ran, so this run " "did not turn TOTP on and granted no role; remove the authenticator entry it " - "showed. " - "If another provision-admin run is in progress, let it finish. Then run the " - "command again" + "showed. If another provision-admin run is in progress, let it finish. Then " + "run the command again: it reads that account and either completes it or says " + "why it cannot" ) # BACKLOG #2019: a repair can take over an account somebody else holds -- one an administrator # created with no roles, say -- so its earlier holder is told, at the address they held. @@ -5221,7 +5250,8 @@ async def _authenticate_oidc( username, "oidc", "local_account_conflict", client=client, extra=floor_marker ) return LoginOutcome(ok=False, error="account conflict", reason="local_account_conflict") - if not bound.directory_object_id: + object_id = _directory_key(bound) + if object_id is None: # BACKLOG #2027 (ADR 0184 AC-5). A BOUND ROW WITH NO IMMUTABLE ID IS REFUSED, NOT # RE-RESOLVED BY NAME. The bind refuses such a row since BACKLOG #1143 slice C, so this # reaches only a binding written before that, or planted in the store. Its only @@ -5252,7 +5282,7 @@ async def _authenticate_oidc( # re-resolve is never keyed on the name alone (BACKLOG #2027). try: principal = await _ask_directory( - self._ldap.resolve_principal, bound.username, object_id=bound.directory_object_id + self._ldap.resolve_principal, bound.username, object_id=object_id ) except LdapError as exc: await self._audit( @@ -5274,7 +5304,7 @@ async def _authenticate_oidc( ) # BACKLOG #3174: the answer must name the bound row's own directory object, by the test the # IdP step-up leg applies, before its groups or name are used. - mismatch = _directory_answer_mismatch(principal, bound.directory_object_id) + mismatch = _directory_answer_mismatch(principal, object_id) if mismatch is not None: await self._directory_reject_audit( username, "oidc", mismatch, client=client, extra=floor_marker @@ -5480,7 +5510,7 @@ async def abandon_oidc_step_up( if flow.step_up_session_hash is None: return await self._step_up_refused(FLOW_PURPOSE_MISMATCH, actor=actor, client=client) return await self._step_up_refused( - reason, actor=actor, client=client, return_to=flow.return_to + reason, actor=actor, client=client, return_to=flow.return_to, flow=flow ) async def _redeem_step_up_flow( @@ -5540,8 +5570,10 @@ async def complete_oidc_step_up( The session is re-anchored to the callback's address when the callback has one. The ``auth.reauth`` row a completed step-up writes, including one whose rotation lost the session, also records the start leg's address as ``start_client`` and whether the two are - different hosts as ``client_moved`` (BACKLOG #2160). The refusal rows above carry neither. - A move is recorded, never refused. Behind a proxy the engine does not trust, both legs carry + different hosts as ``client_moved`` (BACKLOG #2160). So does a refusal made after a + step-up flow was redeemed (vault BACKLOG #3145); :meth:`_step_up_refused` names the rows + that carry neither. A move is recorded, + never refused. Behind a proxy the engine does not trust, both legs carry the proxy's address, so ``client_moved`` reads false there. Not padded to a deadline, unlike the sign-in callback: the caller already holds a session, @@ -5557,6 +5589,11 @@ async def complete_oidc_step_up( token_hash = flow.step_up_session_hash if token_hash is None: return await self._step_up_refused(FLOW_PURPOSE_MISMATCH, actor=actor, client=client) + # Every refusal below is made after the flow was redeemed, so each carries the address + # fields (vault BACKLOG #3145): bound once, so a new arm cannot leave them out. + refuse = partial( + self._step_up_refused, actor=actor, client=client, return_to=return_to, flow=flow + ) # BACKLOG #2301 (ASVS 2.4.2): max_age=0 and prompt=login make the person authenticate at the # IdP, so a step-up callback faster than a person can do that is refused. Before the code is # redeemed, so the refused flow spends nothing at the token endpoint. @@ -5568,47 +5605,25 @@ async def complete_oidc_step_up( self._oidc_callback_floor_exempt_values() if arrived_too_early else frozenset() ) if arrived_too_early and not exempt_values: - return await self._step_up_refused( - TOO_EARLY, actor=actor, client=client, return_to=return_to - ) + return await refuse(TOO_EARLY) try: principal_claims = await asyncio.to_thread( self._exchange_and_validate, code, flow, self._oidc_redirect_uri(public_origin) ) except oidc.ClaimsError as exc: # exc.reason is closed-set, so nothing IdP-influenced reaches the audit row. - return await self._step_up_refused( - exc.reason, actor=actor, client=client, return_to=return_to - ) + return await refuse(exc.reason) except oidc.TokenRefusedError as exc: # The endpoint answered. As on the sign-in leg, that is not an outage (#1948), and the # status is what tells a junk code from a misconfiguration or a failing IdP. - return await self._step_up_refused( - "token_refused", - actor=actor, - client=client, - return_to=return_to, - extra={"status": exc.status}, - ) + return await refuse("token_refused", extra={"status": exc.status}) except (OSError, ValueError, http.client.HTTPException) as exc: # The sign-in leg's outage arm, for the same reasons (#1948). Only the exception TYPE, # and a received status, are kept. if not _is_idp_transport_failure(exc): - return await self._step_up_refused( - IDP_ANSWER_UNUSABLE, - actor=actor, - client=client, - return_to=return_to, - extra=_idp_error_fields(exc), - ) + return await refuse(IDP_ANSWER_UNUSABLE, extra=_idp_error_fields(exc)) self.mark_oidc_unavailable(type(exc).__name__) - return await self._step_up_refused( - "idp_unavailable", - actor=actor, - client=client, - return_to=return_to, - extra=_idp_error_fields(exc), - ) + return await refuse("idp_unavailable", extra=_idp_error_fields(exc)) self.clear_oidc_unavailable() # a verified token: the IdP answered (BACKLOG #3221) floor_exempt_amr = ( self._oidc_callback_floor_exemption(exempt_values, principal_claims.amr) @@ -5618,11 +5633,11 @@ async def complete_oidc_step_up( # BACKLOG #3220: every refusal past the floor carries the marker when the exemption applied, # so a failed exempted step-up is as visible to the loosening's review as a served one. floor_marker = _floor_exempt_marker(floor_exempt_amr) + # Bound once, as the address fields are above, so a new arm cannot leave the marker out. + refuse = partial(refuse, extra=floor_marker) if arrived_too_early and not floor_exempt_amr: # The opted-in site's too-early callback whose amr names no exempt value (BACKLOG #2388). - return await self._step_up_refused( - TOO_EARLY, actor=actor, client=client, return_to=return_to - ) + return await refuse(TOO_EARLY) now = time.time() session = await self._load_session(token_hash) if ( @@ -5634,24 +5649,10 @@ async def complete_oidc_step_up( # stepped up, so the operator is not told "verified" and then signed out. or not session.is_live(now=now, idle_seconds=self.session_idle_seconds) ): - return await self._step_up_refused( - "session_gone", - actor=actor, - client=client, - return_to=return_to, - lost=True, - extra=floor_marker, - ) + return await refuse("session_gone", lost=True) user = await self._load_user(session.user_id) if user is None or user.disabled or user.auth_provider != AuthProvider.AD.value: - return await self._step_up_refused( - "session_gone", - actor=actor, - client=client, - return_to=return_to, - lost=True, - extra=floor_marker, - ) + return await refuse("session_gone", lost=True) if (user.oidc_issuer, user.oidc_subject) != ( principal_claims.issuer, principal_claims.subject, @@ -5664,26 +5665,14 @@ async def complete_oidc_step_up( # BEFORE both freshness tests (BACKLOG #2143), so another person's answer that passed # the claims ladder is filed as a subject mismatch whatever its auth_time. Both tests # below judge this session's identity, so neither is the right name for someone else. - return await self._step_up_refused( - STEP_UP_SUBJECT_MISMATCH, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse(STEP_UP_SUBJECT_MISMATCH) # FRESHNESS, two tests, and both must pass. max_age=0 and prompt=login ask the IdP to # authenticate the user afresh, so a conforming IdP's auth_time postdates this request. # (a) Against our clock: auth_time is IdP clock and issued_at is ours, so the floor allows # the configured skew for an IdP clock that runs behind. skew = self._settings.oidc_clock_skew_seconds if flow.issued_at <= 0 or principal_claims.auth_time < flow.issued_at - skew: - return await self._step_up_refused( - STEP_UP_NOT_FRESH, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse(STEP_UP_NOT_FRESH) # (b) Against the IdP's own clock (BACKLOG #2143): auth_time must be LATER than the one the # session holds, which is the sign-in's or the last step-up's. No skew applies, because # both values come from the IdP. This closes most of what (a) alone left: an IdP that @@ -5697,98 +5686,50 @@ async def complete_oidc_step_up( # row and the operator's text say which one refused. held_auth_time = session.idp_auth_time if held_auth_time is None: - return await self._step_up_refused( - STEP_UP_IDP_AUTH_TIME_MISSING, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse(STEP_UP_IDP_AUTH_TIME_MISSING) if principal_claims.auth_time <= held_auth_time: - return await self._step_up_refused( - STEP_UP_IDP_AUTH_TIME_NOT_LATER, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse(STEP_UP_IDP_AUTH_TIME_NOT_LATER) # The DIRECTORY still has the account. The password re-bind this leg replaces failed for a # disabled or deleted AD object, and the sign-in leg refuses one as not_in_directory. An IdP # can still sign such a user in (sync lag, or its own credential store), so the engine asks # the directory itself, by the row's immutable id as the sign-in leg does (BACKLOG #1532). if self._ldap is None: - return await self._step_up_refused( - "not_configured", - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) - if not user.directory_object_id: + return await refuse("not_configured") + object_id = _directory_key(user) + if object_id is None: # Never a name-only lookup: a directory may reissue a freed name to someone else. The # sign-in leg refuses such a row for the same reason (BACKLOG #2027). - return await self._step_up_refused( - DIRECTORY_OBJECT_ID_MISSING, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse(DIRECTORY_OBJECT_ID_MISSING) try: principal = await _ask_directory( - self._ldap.resolve_principal, user.username, object_id=user.directory_object_id + self._ldap.resolve_principal, user.username, object_id=object_id ) except LdapError as exc: _log.warning( "directory lookup failed during a federated step-up: %s", type(exc).__name__ ) - return await self._step_up_refused( - "directory_unavailable", - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse("directory_unavailable") if principal is None: - return await self._step_up_refused( - "not_in_directory", - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse("not_in_directory") # BACKLOG #3174: the answer must name the row's own directory object before its groups are # trusted, by the test the sign-in leg and the AD step-up re-bind apply. - mismatch = _directory_answer_mismatch(principal, user.directory_object_id) + mismatch = _directory_answer_mismatch(principal, object_id) if mismatch is not None: - return await self._step_up_refused( - mismatch, actor=actor, client=client, return_to=return_to, extra=floor_marker - ) + return await refuse(mismatch) # BACKLOG #2154: the account's stored roles must all be among the roles its current groups # map to, the same test _directory_step_up_refusal applies on the TOTP and passkey legs # (#2240), through the one shared comparison. Not through that helper, because its probe - # would change this leg's not_in_directory reason. Only a LOST role refuses; refusing - # writes nothing, and the reconciler or the next sign-in re-syncs the roles. Read after the - # directory round trip, as before, through the loader (BACKLOG #2592). + # would change this leg's not_in_directory reason. Only a LOST role refuses, or a role map + # that fails its seal, as DIRECTORY_ROLE_MAP_UNSEALED rather than a demotion (BACKLOG + # #3082). Refusing writes nothing; for a lost role the reconciler or the next sign-in + # re-syncs the roles. Read after the directory round trip, as before, through the loader + # (BACKLOG #2592). fresh = await self._load_user(user.id) if fresh is None: - return await self._step_up_refused( - "session_gone", - actor=actor, - client=client, - return_to=return_to, - lost=True, - extra=floor_marker, - ) - lost = await self._directory_roles_lost(fresh.role_ids, principal.groups) - if lost is not None: - return await self._step_up_refused( - lost, - actor=actor, - client=client, - return_to=return_to, - extra=floor_marker, - ) + return await refuse("session_gone", lost=True) + refusal = await self._directory_roles_lost(fresh.role_ids, principal.groups) + if refusal is not None: + return await refuse(refusal) purpose = flow.step_up_purpose # The password leg's three ORDER-CRITICAL steps (see :meth:`reauth`), against the hash. # (1) Every stamp against the OLD hash, re-anchoring the session to this client address. @@ -5813,14 +5754,9 @@ async def complete_oidc_step_up( # (3) The purpose-bound grant, against the NEW hash. self._grant_action_step_up(hash_token(elevation.token), purpose) # BACKLOG #2160: the start leg staged its caller's address, and the session is re-anchored - # to the callback's above. Record whether the address moved mid-ceremony, compared as the - # new-address signal compares. Recorded only: refusing a move, or anchoring to the start - # address, would change ADR 0142 Amendment B and needs a ruling first. None when either - # address is unknown, since an empty string matches nothing and proves no move. - start_client = flow.client_ip or None - client_moved = ( - not self._same_host(start_client, client) if start_client and client else None - ) + # to the callback's above. Record whether the address moved mid-ceremony. Recorded only: + # refusing a move, or anchoring to the start address, would change ADR 0142 Amendment B + # and needs a ruling first. detail: dict[str, object] = { "ok": elevation.ok, "provider": AuthProvider.AD.value, @@ -5829,8 +5765,7 @@ async def complete_oidc_step_up( "session_lost": elevation.session_lost, "grant_refused": grant_refused, "session_revoked": False, - "start_client": start_client, - "client_moved": client_moved, + **self._step_up_address_fields(flow, client), } # BACKLOG #2388: present only when the exemption let a too-early callback through, and # holding only configured values, never the token's own. @@ -5849,6 +5784,21 @@ async def _step_up_actor(self, flow: PendingFlow) -> str: user = await self._load_user(session.user_id) if session is not None else None return user.username if user is not None else "" + def _step_up_address_fields(self, flow: PendingFlow, client: str | None) -> dict[str, object]: + """``start_client`` and ``client_moved`` for a step-up row (BACKLOG #2160, vault #3145). + + ``start_client`` is the address the start leg staged on the flow, never a value from the + IdP's token. ``client_moved`` compares it with the callback's as the new-address signal + compares (:meth:`_same_host`), and is None when either address is unknown, since an empty + string matches nothing and proves no move.""" + start_client = flow.client_ip or None + return { + "start_client": start_client, + "client_moved": ( + not self._same_host(start_client, client) if start_client and client else None + ), + } + async def _step_up_refused( self, reason: str, @@ -5859,11 +5809,18 @@ async def _step_up_refused( lost: bool = False, extra: Mapping[str, object] | None = None, flow_consumed: bool = True, + flow: PendingFlow | None = None, ) -> OidcStepUp: """Audit and package one refusal of the federated step-up leg. ``reason`` is closed-set. ``extra`` adds fields to the audit row only, such as a received status or an exception - type (BACKLOG #1948). It never carries IdP text.""" + type (BACKLOG #1948). It never carries IdP text. + + ``flow`` adds ``start_client`` and ``client_moved``, as the success row carries them (vault + BACKLOG #3145). Only a refusal made after a STEP-UP flow was redeemed passes it: before + ``state`` matched, the callback is not shown to belong to the flow; a sign-in flow refused + as ``flow_purpose_mismatch`` started another ceremony, so its address is not a step-up's; + and some refusals have no flow at all. Those rows carry neither field.""" # `extra` first, so it can never overwrite a closed-set field. detail: dict[str, object] = { **(extra or {}), @@ -5871,6 +5828,7 @@ async def _step_up_refused( "provider": AuthProvider.AD.value, "mech": "oidc", "reason": reason, + **(self._step_up_address_fields(flow, client) if flow is not None else {}), } await self._audit("auth.reauth", actor=actor, detail=_json(detail), client=client) return OidcStepUp( @@ -6197,12 +6155,14 @@ async def _complete_ad_login( # A directory-pushed privilege change is the same privilege change to the same user as a # local one, so notify the affected user out-of-band too (ASVS 6.3.7), matching set_roles(). # Best-effort; the change is also visible in the audited /me/security-events feed. + # ``source`` words it for the directory, as the reconciler's notice is (vault BACKLOG + # #3121): the holder did not make this change, so "if this was you" cannot apply. await self._notify_security( ROLES_CHANGED, username=user.username, email=user.notify_email, client=client, - detail={"roles": role_ids}, + detail={"roles": role_ids, "source": "directory"}, ) ad_roles = _roles_from_ids(role_ids) ad_custom_permissions = await self._custom_permissions_for_ids(role_ids) @@ -8343,7 +8303,7 @@ async def _record_reconcile_referral( self._reconcile_referral_alert = ( "LDAP referral: check [auth].ad_user_search_base and, for nested groups, " "[auth].ad_group_search_base; one names a base in another domain of the forest. " - f"{len(plan.referred)} of {plan.probed} signed-in directory account(s) probed were " + f"{len(plan.referred)} of {plan.asked} signed-in directory account(s) probed were " f"referred and left unjudged. {others} A referred account is never revoked while it " "is referred, because the engine does not follow referrals. Use a base in the bound " "domain controller's own domain, or a global catalog. The ERROR log line for each such " @@ -8366,7 +8326,9 @@ async def _record_reconcile_referral( "reason": reconcile.REFERRAL_ABORT, "referred": len(plan.referred), "unavailable": plan.unavailable, - "probed": plan.probed, + # Over the probes ASKED (BACKLOG #3170), as the outage's row is (#2434): an + # UNKEYED row was never sent, so no referral could answer it. + "asked": plan.asked, "aborted": plan.aborted, } ), @@ -8617,6 +8579,12 @@ async def _apply_reconcile_revocation( await self._report_directory_admin_loss( user, before=user.role_ids, after=revocation.role_ids, via="ad-reconcile" ) + # Vault BACKLOG #3141. The gain side, from the same sealed read and at the same point, + # so a later step's failure cannot lose the page. The next sign-in finds the role + # already written and gains nothing, so this pass is the only chance to raise it. + self._report_directory_admin_grant( + user, before=user.role_ids, after=revocation.role_ids + ) revoked = await self._store.revoke_user_sessions(revocation.user_id) # Owner ruling 2026-10-17 narrows "audit always": a disabled or absent account whose # revocation ended nothing still reports, unless a verified row shows another pass revoked @@ -8677,9 +8645,13 @@ async def _apply_reconcile_revocation( # ``ad_session_revoked`` alert record it. # # ACCOUNT_DISABLED only when the pass READ the disabled bit (vault BACKLOG #2140). That - # notice says an administrator disabled the account, which an absent account or an unreadable - # attribute does not establish, so every other whole-account reason, including any added - # later, gets the neutral DIRECTORY_SESSIONS_ENDED. + # notice says the directory reports the account disabled, which an absent account or an + # unreadable attribute does not establish, so every other whole-account reason, including + # any added later, gets the neutral DIRECTORY_SESSIONS_ENDED. + # + # ``source`` makes the renderer word ROLES_CHANGED and ACCOUNT_DISABLED for the directory, + # with no administrator in them and no "if this was you" (vault BACKLOG #3121). The + # login-path re-sync tags its ROLES_CHANGED the same way. if revocation.role_ids is not None: kind = ROLES_CHANGED elif revocation.reason == reconcile.REVOKE_REASONS[reconcile.ProbeOutcome.DISABLED]: @@ -8691,10 +8663,36 @@ async def _apply_reconcile_revocation( kind, username=user.username, email=user.notify_email, - detail={"reason": revocation.reason}, + detail={"reason": revocation.reason, "source": "directory"}, ) return "reported" + def _report_directory_admin_grant( + self, user: UserRecord, *, before: Iterable[str], after: Iterable[str] + ) -> None: + """Raise ``administrator_granted`` when a reconcile pass's role write newly gave ``user`` + the Administrator role (vault BACKLOG #3141). + + The sign-in rule (vault BACKLOG #2610): only a role this write ADDED pages, so an account + that already held Administrator raises nothing. ``before`` is the account's roles as read + through its seal just before the write, never the plan's, which can be a pass old. Only an + applied revocation reaches here, and only a pass that did not abort applies one. The alert + pops the account's ``no_enabled_administrator`` cooldown in the sink, which is intended: + a later loss of the role pages again. + + Never raises, a defect in the callable included: it runs between the role write and the + session revocation, and a raise would skip the revocation (see + :meth:`_report_directory_admin_loss`).""" + admin = Role.ADMINISTRATOR.value + if admin in set(before) or admin not in set(after): + return + _log.warning("a directory reconcile pass gave an account the Administrator role") + if self._admin_grant_alert is not None: + try: + self._admin_grant_alert(user.username, "directory_reconcile") + except Exception: # noqa: BLE001 -- logged with its traceback; see the docstring + _log.exception("the administrator_granted alert callable raised") + async def _report_directory_admin_loss( self, user: UserRecord, @@ -10040,24 +10038,53 @@ async def list_sessions(self, user_id: str) -> list[SessionRecord]: await self._session_seal_failed(row.token_hash) # see identity_for_token return kept - async def revoke_own_session(self, identity: Identity, session_id: str, *, actor: str) -> bool: + async def revoke_own_session( + self, identity: Identity, session_id: str, *, actor: str, client: str | None + ) -> bool: """Revoke one of ``identity``'s **own** sessions by id (its ``token_hash``). Returns ``False`` if the session doesn't exist or isn't the caller's — so the API answers 404 without revealing or letting a user touch another's session. Audited. - Also ``False``, with no audit row, when the revoke matched no unrevoked row (BACKLOG #3029): - at least a rotation that re-keyed the session after the read, which leaves it live under - its new id, or a concurrent revoke that ended it first. The revoke reports whether it - matched, so neither the return nor the trail claims a revoke that did not happen.""" + A miss writes :data:`SESSION_REVOKE_MISSED_ACTION` (vault BACKLOG #3260, limb 2; owner + ruling 2026-10-16). Its detail is the id's first 12 characters (or ``malformed``), as the success row's is, + and nothing else: an unknown id and another account's id write the same row, so the trail + records no ownership a caller could not already learn. The write is soft with no defect + re-raised, for the reason ``api.auth_routes._refuse_if_self`` gives: a raise would replace + the answer the caller is owed. ``client`` is required, as on + :meth:`audit_self_target_refused`, because both planes call this from a request. + + Also ``False`` when the revoke matched no unrevoked row (BACKLOG #3029): at least a + rotation that re-keyed the session after the read, which leaves it live under its new id, + or a concurrent revoke that ended it first. The revoke reports whether it matched, so + neither the return nor the trail claims a revoke that did not happen. That miss answers the + same 404, so it writes the same miss row and no ``auth.session_revoked`` row.""" session = await self._load_session(session_id) - if session is None or session.user_id != identity.user_id: - return False - if await self._store.supersede_session(session_id, now=time.time()) is None: + if ( + session is None + or session.user_id != identity.user_id + or await self._store.supersede_session(session_id, now=time.time()) is None + ): + # The console passes the id as plain text, so only an id-shaped one is copied into the + # trail; anything else is recorded as malformed, never as the caller's own characters. + shown = session_id[:12] if _SESSION_ID_SHAPE.fullmatch(session_id) else "malformed" + await write_audit_soft( + lambda: self._audit( + SESSION_REVOKE_MISSED_ACTION, + actor=actor, + detail=_json({"session": shown}), + client=client, + ), + log=_log, + message="a missed session revoke by %s stands, but its audit row failed", + args=lambda: (scrub_log_argument(actor),), + defects=(), + ) return False await self._audit( "auth.session_revoked", actor=actor, detail=_json({"scope": "self", "session": session_id[:12]}), + client=client, ) return True @@ -10469,7 +10496,8 @@ async def _reproof_serialized( reason: str | None = None groups: frozenset[str] = frozenset() if directory: - if not user.directory_object_id: + object_id = _directory_key(user) + if object_id is None: # BACKLOG #2027, ADR 0184 AC-5. The re-bind finds its entry by the row's id, and a # row with none has only its name, which a directory may have reissued to someone # else whose password would then step this session up. Refused before the directory @@ -10477,9 +10505,7 @@ async def _reproof_serialized( # did not guess wrong. verdict, reason = None, DIRECTORY_OBJECT_ID_MISSING else: - rebind = await self._reauth_ad( - user.username, password, object_id=user.directory_object_id - ) + rebind = await self._reauth_ad(user.username, password, object_id=object_id) verdict, reason, groups = rebind.verdict, rebind.reason, rebind.groups else: verdict = user.password_hash is not None and await self._argon2( @@ -11726,7 +11752,8 @@ async def confirm_mfa_enrollment( does a good code whose activation matches no row, for example because a reset cleared the staged secret, a second confirm enabled first (BACKLOG #2224), or a second begin staged a secret this confirm never verified (BACKLOG #3025). That confirm ends the session it rotated, - hands back no codes, and audits ``auth.mfa_enroll_refused``. + hands back no codes, and audits ``auth.mfa_enroll_refused``, whose ``session_ended`` says + whether the store took the revoke (vault BACKLOG #3113). This is one of the two legs that turn an MFA-pending session into an MFA-satisfied one for a FIRST enrolment, so it rotates for the same reason ``verify_mfa`` does: without it a pre-MFA @@ -11834,7 +11861,10 @@ async def confirm_mfa_enrollment( # the rotation has succeeded; on a lost session MFA stays off and the staged secret stays put, # and the user signs in again and confirms with a code from a later step (this one is spent). # The cost: if enable_totp itself fails after a good rotation, the new token is never handed - # back, so the user is signed out with MFA still off. That fails closed, and a retry works. + # back, so the user is signed out with MFA still off. That fails closed. A retry is not + # always a plain one (vault BACKLOG #3113): it needs a code from a later step, since this one + # is spent, and where the enable matched no row (#2224, below) a reset may have cleared the + # staged secret, so the user starts the enrolment again. # Stamp against the OLD hash, then rotate — never the other way round. async with self._sealed_stamp(hash_token(token)): await self._store.mark_session_mfa_verified(hash_token(token)) @@ -11860,31 +11890,43 @@ async def confirm_mfa_enrollment( # # Known cost: ``_elevated`` already ran the session cap for the rotated session, so # at the cap that run may have ended the account's oldest full session, and it stays - # ended. The order is #1902's, rotate first and enable last, and is kept. - await self._store.revoke_session(hash_token(elevation.token)) - # The reason is the row's state read AFTER the refusal, not the write's own verdict, - # so a later write can blur it; it is a pointer for an operator, not a proof. - after = await self._load_user(identity.user_id) - if after is None: - reason = "account_gone" - elif after.totp_enabled: - reason = "already_enabled" - elif after.totp_secret_cell is None: - reason = "secret_cleared" - elif after.totp_secret_cell != verified_cell: - reason = "secret_replaced" - else: - # The row still holds the verified cell with TOTP off, so the write lost to some - # other concurrent write to the row. The session is revoked above, so a plain - # retry is a lost session; recovery is as for a session lost before the rotation - # (the ROTATE BEFORE ENABLING comment above). - reason = "write_lost" - await self._audit( - "auth.mfa_enroll_refused", - actor=identity.username, - detail=_json({"reason": reason, "session_ended": True}), - client=client, - ) + # ended. The order is #1902's, rotate first and enable last, and is kept. Vault BACKLOG + # #3113 item 8 is declined for that reason: enabling first would undo #1902. + # + # A store refusal of the revoke is logged, not raised, so the confirm still answers + # ``session_lost`` (vault BACKLOG #3113). That covers aiosqlite's closed connection + # too, which it raises as a bare ``ValueError`` before sqlite3 is reached. A defect + # the helper re-raises still propagates, and the ``finally`` writes the refusal row + # first, so no exception the revoke raises can skip it. ``session_ended`` records + # what the revoke did. The token is not handed back either way, so the caller still + # cannot use the session; a session the revoke left live expires on its own timers. + # The row's own write is soft too, so a store fault there is logged at ERROR rather + # than replacing the answer, or a defect already propagating. Known limit: a + # cancellation can land after the store committed the revoke, and the row then says + # ``session_ended: false`` for a session that did end. + rotated = hash_token(elevation.token) + refused = (*store_call_errors(), CipherError) + session_ended = False + try: + session_ended = await write_audit_soft( + lambda: self._store.revoke_session(rotated), + log=_log, + message="a refused TOTP enrolment for %s could not end the session it rotated", + args=lambda: (scrub_log_argument(identity.username),), + errors=refused, + defects=AUDIT_WRITE_DEFECTS, + ) + finally: + await write_audit_soft( + lambda: self._audit_enroll_refused( + identity, session_ended, verified_cell=verified_cell, client=client + ), + log=_log, + message="a refused TOTP enrolment for %s could not write its refusal row", + args=lambda: (scrub_log_argument(identity.username),), + errors=refused, + defects=AUDIT_WRITE_DEFECTS, + ) return Elevation(session_lost=True) await self._audit("auth.mfa_enrolled", actor=identity.username, client=client) # ADR 0197 Amendment A: enrolment now comes BEFORE the first rotation, so whoever intercepts @@ -11904,6 +11946,43 @@ async def confirm_mfa_enrollment( await self._mark_login_address_known(identity.user_id, client, enrolment=user) return elevation + async def _audit_enroll_refused( + self, + identity: Identity, + session_ended: bool, + *, + verified_cell: str | None, + client: str | None, + ) -> None: + """Write the ``auth.mfa_enroll_refused`` row for a confirm whose enable matched no row. + + The reason is the row's state read AFTER the refusal, not the write's own verdict, so a + later write can blur it; it is a pointer for an operator, not a proof. Vault BACKLOG #3113 + item 9 is declined: the write's own verdict would need ``enable_totp`` to return a reason + on all three store backends.""" + after = await self._load_user(identity.user_id) + if after is None: + reason = "account_gone" + elif after.totp_enabled: + reason = "already_enabled" + elif after.totp_secret_cell is None: + reason = "secret_cleared" + elif after.totp_secret_cell != verified_cell: + reason = "secret_replaced" + else: + # The row still holds the verified cell with TOTP off, so the write lost to some other + # concurrent write to the row (BACKLOG #3025). The caller asked the store to revoke the + # session first (``session_ended`` says whether it did) and hands no token back, so a + # plain retry is a lost session; recovery is as for a session lost before the rotation + # (the ROTATE BEFORE ENABLING comment in ``confirm_mfa_enrollment``). + reason = "write_lost" + await self._audit( + "auth.mfa_enroll_refused", + actor=identity.username, + detail=_json({"reason": reason, "session_ended": session_ended}), + client=client, + ) + async def verify_mfa( self, token: str | None, code: str, *, client: str | None = None ) -> Elevation: @@ -12547,11 +12626,11 @@ async def _directory_presence(self, user: UserRecord) -> reconcile.Probe | str: """ if self._ldap is None: return "not_configured" - if not user.directory_object_id: + if not _holds_directory_key(user): return DIRECTORY_OBJECT_ID_MISSING # The reconciler's own probe, so both ask the same question by the same key, off the loop. - # They differ on an id-less row only: this leg refuses it above, and the reconciler still - # probes an unbound one by name (BACKLOG #2027). + # Neither asks about an id-less row: this leg refuses it above, and the reconciler reads it + # as UNKEYED without a lookup (BACKLOG #2434, #3170 item 6). probe = await self._probe_principal(user) if probe.outcome is not reconcile.ProbeOutcome.PRESENT: return str(probe.outcome.value) @@ -14442,7 +14521,7 @@ async def audit_refused(reason: str, username: str, **extra: object) -> None: if user.auth_provider != AuthProvider.AD.value: await audit_refused("not_a_directory_account", user.username) raise ValueError("only a directory (AD) account can take a federated binding") - if not user.directory_object_id: + if not _holds_directory_key(user): # BACKLOG #1143 slice C: see DirectoryObjectIdMissing. After the no-op check, so a # legacy binding resubmitted as it stands gets the harmless answer above, while moving # it to another pair is still refused. Unbinding it stays open. diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index c197b9699..bcaecf6b1 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -5101,7 +5101,8 @@ class AlertSeverity(str, Enum): # noqa: UP042 "approval_too_early", # BACKLOG #315: a release by an approver account changed after the request, and an # Administrator grant through the console API or, since vault BACKLOG #2610, by a directory - # sign-in's role sync (granted_by ``). + # sign-in's role sync, and since vault BACKLOG #3141 by a directory reconcile pass (both + # granted_by ``). "approval_approver_provenance", "administrator_granted", # vault BACKLOG #3229: a directory-driven role change left no enabled Administrator. Keyed diff --git a/messagefoundry/pipeline/alert_sinks.py b/messagefoundry/pipeline/alert_sinks.py index 664d27e33..90cddabaf 100644 --- a/messagefoundry/pipeline/alert_sinks.py +++ b/messagefoundry/pipeline/alert_sinks.py @@ -1154,12 +1154,14 @@ def administrator_granted(self, name: str, *, via: str, granted_by: str) -> None # BACKLOG #315: the Administrator role was granted. `user:` or `ad-group:` # stands in for "connection" (the key's grammar is on AlertSink). `granted_by` is the granting # administrator's operator account name, or the fixed marker `` when a directory - # sign-in's role sync made the grant (vault BACKLOG #2610). Never message content. + # sign-in's role sync or a reconcile pass made the grant (vault BACKLOG #2610, #3141). Never + # message content. # Vault BACKLOG #3229: every administrator_granted call pops the _last_sent key built from # the no_enabled_administrator alert type and the granted target, so a loss after an # administrator_granted re-grant is not held back by the earlier loss's cooldown. A - # reconciler re-grant calls no administrator_granted, so it pops nothing. The pop builds that key with its own - # f-string in `_emit`'s key format; tests/test_directory_last_admin_alert.py pins the match. + # reconciler re-grant raises administrator_granted too (vault BACKLOG #3141), so it pops the + # key the same way. The pop builds that key with its own f-string in `_emit`'s key format; + # tests/test_directory_last_admin_alert.py pins the match. # Only a `user:` target can match a loss key, so an `ad-group:` grant pops nothing. _last_sent is # per process, so this clears the cooldown in this process only. _occurrences and _suspended # are kept. Unlike crl_reload_applied, this resolves no open instance. diff --git a/messagefoundry/pipeline/alerts.py b/messagefoundry/pipeline/alerts.py index d6b4327c9..b247ffa40 100644 --- a/messagefoundry/pipeline/alerts.py +++ b/messagefoundry/pipeline/alerts.py @@ -559,12 +559,15 @@ def administrator_granted(self, name: str, *, via: str, granted_by: str) -> None ``ad_group_map`` with ``name`` = ``ad-group:`` when a group newly maps to the role. A directory sign-in whose role sync newly grants the role (vault BACKLOG #2610) raises it with ``name`` = ``user:`` and ``via`` = ``directory_sign_in_negotiate``, - ``directory_sign_in_sso`` or ``directory_sign_in_oidc``, naming the route. + ``directory_sign_in_sso`` or ``directory_sign_in_oidc``, naming the route. A directory + reconcile pass whose role re-diff newly grants it (vault BACKLOG #3141) raises it with + ``name`` = ``user:`` and ``via`` = ``directory_reconcile``. Both keys are outside the connection-name grammar for the same reason as :meth:`approval_approver_provenance`. ``granted_by`` is the acting administrator's username, - or ```` on a directory sign-in, where no administrator acted. - No PHI. Emitted by the API's user-administration and directory sign-in routes, never from - ``auth/``.""" + or ```` on a directory sign-in or reconcile pass, where no administrator acted. + No PHI. Emitted by the API's user-administration and directory sign-in routes, and for the + reconciler through the callable :func:`administrator_granted_alert` builds, which the API + lifespan hands to the auth service, so ``auth/`` never imports a sink.""" ... def no_enabled_administrator(self, name: str, *, via: str) -> None: @@ -600,10 +603,16 @@ def ad_reconcile_aborted(self, name: str, *, reason: str, probed: int, detail: s ``"directory-reconciler-referral"`` for a referral, ``"directory-reconciler-probe-fault"`` for a probe fault, and ``"directory-reconciler-paging:"`` for a paging refusal, so each is its own - instance and throttle. ``reason`` is one of those five closed-set slugs. ``probed`` is how many - principals the pass probed. ``detail`` is the operator-facing explanation the auth service - latches. No PHI. Emitted by the API-lifespan - reconciler task, never from ``auth/``. A whole-directory outage is NOT this event: it is + instance and throttle. ``reason`` is one of those five closed-set slugs. ``probed`` is, for + the breaker, how many principals the pass judged, its denominator; for a probe fault and a + paging refusal, the same total of candidates the pass took up, asked or not, which counts + any id-less row and, for a probe fault, the faulted accounts it left unjudged; and for a + referral how many probes the pass sent to the directory, which leaves out an id-less row + (BACKLOG #3170). A paging refusal concerns one account, and its count is still the pass's. + ``detail`` is the operator-facing explanation the auth service latches, or for a paging + refusal a fixed sentence naming its cause. No PHI. Emitted by the + API-lifespan reconciler task, never from ``auth/``. A whole-directory outage is NOT this + event: it is audited as ``auth.ad_reconcile_skipped`` and pages nothing, because the accounts are fine.""" ... @@ -1169,7 +1178,11 @@ def no_enabled_administrator(self, name: str, *, via: str) -> None: def ad_reconcile_aborted(self, name: str, *, reason: str, probed: int, detail: str) -> None: log.warning( - "ALERT ad_reconcile_aborted: %r left accounts unrevoked in a pass of %d principal(s) " + # The count is the candidates the pass took up for the breaker, a probe fault and a + # paging refusal, or a referral's probes asked (BACKLOG #3170); see + # AlertSink.ad_reconcile_aborted. + "ALERT ad_reconcile_aborted: %r left accounts unrevoked or unjudged, in a pass counting " + "%d principal(s) " "(%s): %s", name, probed, @@ -1387,6 +1400,23 @@ def _raise(username: str, via: str) -> None: return _raise +def administrator_granted_alert(sink: AlertSink, *, granted_by: str) -> Callable[[str, str], None]: + """An alert callable for :class:`~messagefoundry.auth.service.AuthService`: each call, with a + username and a ``via``, raises :meth:`AlertSink.administrator_granted` keyed ``user:`` + and naming ``granted_by`` (vault BACKLOG #3141). The API passes its directory marker, because + the service calls this only for a grant the directory made. + + Never raises: an alert failure must not change what a reconcile pass does.""" + + def _raise(username: str, via: str) -> None: + try: + sink.administrator_granted(f"user:{username}", via=via, granted_by=granted_by) + except Exception: # noqa: BLE001 — an alert-sink failure must never break a request + log.warning("the administrator_granted alert could not be delivered", exc_info=True) + + return _raise + + def alert_store_cipher_refusal(sink: AlertSink, table: str, column: str) -> None: """Raise the refusal alert for an unmarked value in ``table.column``: ``upload-cipher`` for the uploaded-file store, ``store-cipher`` for everything else. diff --git a/messagefoundry/pipeline/security_notify.py b/messagefoundry/pipeline/security_notify.py index 1e06fb59a..5fb39f852 100644 --- a/messagefoundry/pipeline/security_notify.py +++ b/messagefoundry/pipeline/security_notify.py @@ -171,6 +171,45 @@ ), } +# Vault BACKLOG #3121. The reconciler's and the sign-in re-sync's role change and disable name no +# administrator and close with "If you did not expect this change". Each carries its own source in +# its words, so neither takes the generic directory line: a re-sync's roles come from the +# directory's groups through the group-to-role map, which an administrator edits in the console, +# so "not from the MessageFoundry console" would be false for it. The disable says what the +# reconciler did, which is end the sessions; no MessageFoundry row was disabled. Both senders +# of the role change end the sessions already open on the account first, the reconciler's pass +# and the sign-in re-sync alike, so the role notice says so as the disable notice does. +_DIRECTORY_DESCRIPTIONS = { + ROLES_CHANGED: ( + "MessageFoundry read your groups in your organization's directory, changed your " + "account's roles to match them, and ended any sessions already open on your account." + ), + ACCOUNT_DISABLED: ( + "Your organization's directory reports your account as disabled, so MessageFoundry ended " + "your sessions." + ), +} +# The administrator subject says the MessageFoundry account was disabled, which is false here. +_DIRECTORY_SUBJECTS = { + ACCOUNT_DISABLED: "Your MessageFoundry sessions were ended: your directory account is disabled", +} + + +def _from_directory(event: SecurityEvent) -> bool: + """Whether the directory, not the holder or an administrator, made this change (BACKLOG #1139, + #2291, vault #3121). Gated on the kind as well as the tag: the reconciler tags every notice it + sends, and its ``directory_sessions_ended`` must not claim the directory changed anything.""" + return ( + event.event_type in (EMAIL_CHANGED, USERNAME_CHANGED, *_DIRECTORY_DESCRIPTIONS) + and event.detail.get("source") == "directory" + ) + + +def _subject(event: SecurityEvent) -> str: + if _from_directory(event) and event.event_type in _DIRECTORY_SUBJECTS: + return _DIRECTORY_SUBJECTS[event.event_type] + return _SUBJECTS.get(event.event_type, "MessageFoundry security alert") + _LOCAL_REMEDY = ( "ask your MessageFoundry administrator for a password reset, or the host operator to run " @@ -261,10 +300,7 @@ def _build_body(event: SecurityEvent) -> str: set_by_admin = ( event.event_type == NOTIFY_EMAIL_SET and event.detail.get("set_by") == "administrator" ) - # BACKLOG #1139, #2291: the directory made this change, not the holder and not the console. - from_directory = event.event_type in (EMAIL_CHANGED, USERNAME_CHANGED) and ( - event.detail.get("source") == "directory" - ) + from_directory = _from_directory(event) if moved_by_admin: description = ( "An administrator changed the address that receives security notices for your account." @@ -273,6 +309,8 @@ def _build_body(event: SecurityEvent) -> str: description = ( "An administrator set this address to receive security notices about your account." ) + elif from_directory and event.event_type in _DIRECTORY_DESCRIPTIONS: + description = _DIRECTORY_DESCRIPTIONS[event.event_type] else: description = _DESCRIPTIONS.get( event.event_type, "A security event occurred on your account." @@ -378,11 +416,12 @@ def _build_body(event: SecurityEvent) -> str: scrub_log_argument(event.username), key, ) - if from_directory: + if from_directory and event.event_type not in _DIRECTORY_DESCRIPTIONS: # BACKLOG #1139. Say WHERE the change came from, because it changes what the reader can do # about it: a directory-driven change is not editable in the console, so "contact your # administrator" is the only action, and an unexplained change the holder cannot find a - # cause for reads as a compromise. + # cause for reads as a compromise. The vault #3121 kinds name their source in their own + # description instead (see ``_DIRECTORY_DESCRIPTIONS``). lines.append( "This change came from your organization's directory, not from the MessageFoundry " "console." @@ -442,7 +481,9 @@ def _build_body(event: SecurityEvent) -> str: # and a bare "Source IP" line would read as the address of whoever made it. Measured # 2026-10-06, the directory notices that pass an address come from ``_upsert_ad_user``, on # a directory sign-in to this account, which copies the change down BEFORE the sign-in is - # finished: a later step can still refuse it, or it can still owe a second factor. So the + # finished: a later step can still refuse it, or it can still owe a second factor. Since + # vault BACKLOG #3121 the role re-sync in ``_complete_ad_login`` is one more such sender, + # and it too runs inside a directory sign-in before the session is issued. So the # line says a sign-in was under way, never that one succeeded. Nothing on the event says # the address came from a sign-in, so a new sender passing another kind of address on a # directory event would make this line false; such a sender must change it. @@ -486,8 +527,10 @@ def _build_body(event: SecurityEvent) -> str: DIRECTORY_SESSIONS_ENDED, ) ): - # A directory rename or email change is an administrator's act, so "if this was you" - # cannot apply to it (the email half is BACKLOG #2291). Nor to a federated link or unlink: + # The holder did not make a directory change, so "if this was you" cannot apply to a + # directory rename or email change (the email half is BACKLOG #2291), nor to a role + # change or disable the directory drove (vault BACKLOG #3121). Nor to a federated link or + # unlink: # both routes refuse an administrator changing their own account's binding. Nor to the # engine ending sessions on a directory answer (vault BACKLOG #2140). closing = "If you did not expect this change, contact your MessageFoundry administrator." @@ -668,7 +711,7 @@ def _send(self, event: SecurityEvent) -> None: port=self._port, sender=self._sender, recipients=[event.email], - subject=_SUBJECTS.get(event.event_type, "MessageFoundry security alert"), + subject=_subject(event), body=_build_body(event), use_tls=self._use_tls, username=self._username, diff --git a/messagefoundry/store/base.py b/messagefoundry/store/base.py index 3e238c0f9..ba25d1093 100644 --- a/messagefoundry/store/base.py +++ b/messagefoundry/store/base.py @@ -27,7 +27,7 @@ import logging from collections.abc import AsyncIterator, Callable, Collection, Iterable, Mapping, Sequence from dataclasses import dataclass -from functools import partial +from functools import cache, partial from pathlib import Path from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable @@ -3345,6 +3345,17 @@ def store_connect_errors() -> tuple[type[Exception], ...]: return tuple(errors) +@cache +def store_call_errors() -> tuple[type[Exception], ...]: + """What a refused store call can raise, on any backend: :func:`store_driver_errors`, plus + ``RuntimeError`` and ``OSError`` as that function asks a caller to add, and ``ValueError``, + which aiosqlite raises for a closed connection before sqlite3 is reached. A caller that must not + read a bug as a refusal re-raises :data:`~messagefoundry.audit_write.AUDIT_WRITE_DEFECTS` + first. Cached, because naming a server driver imports it: build it at startup, so the first + refusal does not run that import on the event loop (vault BACKLOG #3145, #3113).""" + return (RuntimeError, OSError, ValueError, *store_driver_errors()) + + def store_open_errors() -> tuple[type[Exception], ...]: """:func:`store_driver_errors` and :func:`store_connect_errors`: at least the driver and connection errors a store open can raise, on any backend.""" diff --git a/messagefoundry/verify/federation.py b/messagefoundry/verify/federation.py index bd2e1d7ba..6ee2094a0 100644 --- a/messagefoundry/verify/federation.py +++ b/messagefoundry/verify/federation.py @@ -280,17 +280,15 @@ def _idp_opener( return the ``fed.idp_tls`` FAIL or ERROR row that says why it did not build. Reads the anchor and CRL files; opens no socket. ``anchor=False`` leaves the anchor and its pin out, for :func:`idp_revocation_result` alone.""" - from messagefoundry.auth.oidc_http import build_idp_opener + from messagefoundry.auth.oidc_http import build_idp_opener_from_settings from messagefoundry.auth.trust_anchors import TrustAnchorError from messagefoundry.config.settings import SecurityEnforcement - auth = settings.auth try: - return build_idp_opener( - auth.oidc_tls_ca_cert_file if anchor else None, - pin=auth.oidc_tls_ca_cert_pin if anchor else None, + return build_idp_opener_from_settings( + settings.auth, enforcing=settings.security.enforcement is SecurityEnforcement.ENFORCE, - crl_file=auth.oidc_tls_crl_file, + anchor=anchor, ) except OSError as exc: return CheckResult(*_TLS_ROW, Status.FAIL, f"could not build the pinned TLS context: {exc}") diff --git a/messagefoundry_webconsole/__init__.py b/messagefoundry_webconsole/__init__.py index 654468c57..f44c26887 100644 --- a/messagefoundry_webconsole/__init__.py +++ b/messagefoundry_webconsole/__init__.py @@ -44,7 +44,7 @@ # If cross-seam support is ever genuinely wanted, re-widen this set AND add the CI matrix that # installs the MIN and MAX supported engine builds — the claim and its test land together, or not # at all. -SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"5b6fd34b36a52c84"}) +SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"5d9c162790077dfc"}) #: The vendored static assets shipped in THIS wheel (mounted at /ui/static by :func:`mount_ui`). STATIC_DIR = Path(__file__).parent / "static" diff --git a/messagefoundry_webconsole/routes/account.py b/messagefoundry_webconsole/routes/account.py index 47c9630d3..ed6ca7f0c 100644 --- a/messagefoundry_webconsole/routes/account.py +++ b/messagefoundry_webconsole/routes/account.py @@ -639,8 +639,11 @@ async def ui_revoke_session( # time the POST lands: the target may have completed MFA or a step-up on its own device in # between. Reporting "revoked" then would tell the operator a session had ended while it was # still live under its new id. An unknown id and a foreign id - # read the same, so the notice discloses nothing the JSON twin's 404 does not. - revoked = await service.revoke_own_session(identity, session_id, actor=identity.username) + # read the same, so the notice discloses nothing the JSON twin's 404 does not. A miss writes + # the same audit row the JSON twin's does, in the service (vault BACKLOG #3260, limb 2). + revoked = await service.revoke_own_session( + identity, session_id, actor=identity.username, client=client_ip(request) + ) outcome = "revoked" if revoked else "revoke_missed" return RedirectResponse(f"/ui/account/sessions?m={outcome}", status_code=303) diff --git a/packaging/messagefoundry-webconsole/CHANGELOG.md b/packaging/messagefoundry-webconsole/CHANGELOG.md index aaff554f2..a60af3dca 100644 --- a/packaging/messagefoundry-webconsole/CHANGELOG.md +++ b/packaging/messagefoundry-webconsole/CHANGELOG.md @@ -171,6 +171,12 @@ this line.** value are judged as that value, so no browser request changes. Seam only: the gate helpers it edits are hashed into the seam since BACKLOG #2709, so the supported digest moved. (vault BACKLOG #3316) +- **A session revoke that ends nothing is audited, with your address.** The revoke button on + `/ui/account/sessions` now passes the request's address to the engine, which writes one + `auth.session_revoke_missed` row when the id names no session or another account's. The notice + is unchanged. `AuthService.revoke_own_session` takes a required keyword `client`; the seam + digest records that signature, so it moved, and a console and engine on either side of this + change refuse each other at mount. (vault BACKLOG #3260) - **The passkey registration finish draws on the per-actor ceremony budget.** `POST /ui/account/webauthn/verify` now charges the same budget as `POST /ui/account/mfa/verify`, and answers a throttle with a JSON 429 the page can show. No seam change. (BACKLOG #3014) diff --git a/packaging/messagefoundry-webconsole/tests/test_ui_directory_admin_alert.py b/packaging/messagefoundry-webconsole/tests/test_ui_directory_admin_alert.py index f21e51f61..368b8e3c9 100644 --- a/packaging/messagefoundry-webconsole/tests/test_ui_directory_admin_alert.py +++ b/packaging/messagefoundry-webconsole/tests/test_ui_directory_admin_alert.py @@ -105,6 +105,13 @@ async def _service(engine: Engine, *, held: Role | None, bind: bool) -> AuthServ return service +async def _jdoe_holds_administrator(engine: Engine) -> bool: + """Whether the role write landed: a refused-after-sync test proves nothing without it.""" + jdoe = await engine.store.get_user_by_username("jdoe") + assert jdoe is not None + return Role.ADMINISTRATOR.value in await engine.store.get_user_role_ids(jdoe.id) + + def _client(engine: Engine, service: AuthService, sink: _Sink) -> httpx.AsyncClient: app = create_app(engine, auth=service, serve_ui=True, public_origin="https://ops.example") app.state.notifier = sink @@ -208,6 +215,7 @@ async def test_ui_sso_refused_after_the_role_write_still_raises_the_alert( page.""" sink = await _sso(engine, monkeypatch, held=Role.OPERATOR, bind_lands_mid_login=True) assert sink.events == [_grant("directory_sign_in_sso")] + assert await _jdoe_holds_administrator(engine) async def test_ui_oidc_that_newly_grants_administrator_raises_one_alert( @@ -231,3 +239,4 @@ async def test_ui_oidc_refused_after_the_role_write_still_raises_the_alert( failed, and a later sign-in gains nothing, so this refusal is the one chance to page.""" sink = await _oidc(engine, monkeypatch, held=None, bind_lands_mid_login=True) assert sink.events == [_grant("directory_sign_in_oidc")] + assert await _jdoe_holds_administrator(engine) diff --git a/packaging/messagefoundry-webconsole/tests/test_ui_session_revoke_miss_audit.py b/packaging/messagefoundry-webconsole/tests/test_ui_session_revoke_miss_audit.py new file mode 100644 index 000000000..b7b584f44 --- /dev/null +++ b/packaging/messagefoundry-webconsole/tests/test_ui_session_revoke_miss_audit.py @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""Vault BACKLOG #3260, limb 2, on the console plane: a session revoke that ends nothing is audited. + +Owner ruling 2026-10-16: one ``auth.session_revoke_missed`` row per miss, with the id cut to its +first 12 characters, and the answer unchanged and the same for an unknown id and another account's. +The console takes the id as plain text, so one that is not lower-case hex is recorded as malformed. +The JSON plane's twin is ``tests/test_self_target_refusal_audit.py``. +""" + +from __future__ import annotations + +import json + +import pytest +from _ui_clients import PW, SAME_ORIGIN, cookie_login, mint_bound_proof, provision, ui_client + +from messagefoundry.auth import Role +from messagefoundry.auth.service import AuthService +from messagefoundry.auth.tokens import hash_token +from messagefoundry.config.settings import AuthSettings +from messagefoundry.pipeline import Engine + + +@pytest.mark.parametrize( + "kind", + [ + "unknown", + "foreign", + # Plain text on this plane, so the row must cut it, not copy it. + "long", + ], +) +async def test_a_missed_console_revoke_writes_one_row_and_the_same_answer( + engine: Engine, kind: str +) -> None: + """RED when: the console's revoke of a session it cannot end writes no audit row of its own, or + its answer or row tells an unknown id from another account's (vault BACKLOG #3260, limb 2).""" + service = AuthService( + engine.store, AuthSettings(require_mfa=False, admin_write_min_interval_seconds=0) + ) + await service.initialize() + await provision(service, "a", [Role.VIEWER.value]) + await provision(service, "b", [Role.VIEWER.value]) + theirs = await service.login("b", PW) + assert theirs.token is not None + target = { + "unknown": "0" * 64, + "foreign": hash_token(theirs.token), + "long": "x" * 200, + }[kind] + path = f"/ui/account/sessions/{target}/revoke" + async with ui_client(engine, service) as c: + await cookie_login(c, "a") + await mint_bound_proof(c, path) + r = await c.post(path, headers=SAME_ORIGIN) + assert r.status_code == 303, (r.status_code, r.text[:300]) + assert r.headers["location"] == "/ui/account/sessions?m=revoke_missed" + assert await service.identity_for_token(theirs.token, activity=False) is not None, ( + "a missed revoke ended another account's session" + ) + rows = await engine.store.list_audit(action="auth.session_revoke_missed", limit=10) + assert len(rows) == 1, "one miss, one row" + assert rows[0]["actor"] == "a" + assert rows[0]["client"] == "127.0.0.1", "the miss row dropped the caller's address" + # The cut id and nothing else, so the row records no ownership either. Text that cannot be a + # session id is recorded as malformed, never copied. + shown = "malformed" if kind == "long" else target[:12] + assert json.loads(str(rows[0]["detail"])) == {"session": shown} diff --git a/packaging/messagefoundry-webconsole/tests/test_webui_search_sso.py b/packaging/messagefoundry-webconsole/tests/test_webui_search_sso.py index 87efc8354..cb01af8da 100644 --- a/packaging/messagefoundry-webconsole/tests/test_webui_search_sso.py +++ b/packaging/messagefoundry-webconsole/tests/test_webui_search_sso.py @@ -309,8 +309,12 @@ async def test_reauth_both_factors_enrolled_renders_code_and_passkey(engine: Eng secret = totp_mod.generate_secret() staged_cell = await service.store.set_totp_secret(boss_id, secret=secret) - await service.store.enable_totp( - boss_id, expected_secret_cell=staged_cell, recovery_code_hashes=[] + # Conditional since BACKLOG #2224, so the premise is asserted (vault BACKLOG #3113 item 6). + assert ( + await service.store.enable_totp( + boss_id, expected_secret_cell=staged_cell, recovery_code_hashes=[] + ) + is True ) await _cookie_login(c, "boss") # fresh MFA-pending session diff --git a/tests/_admin_account.py b/tests/_admin_account.py index 3fce51f42..df1e6331c 100644 --- a/tests/_admin_account.py +++ b/tests/_admin_account.py @@ -129,16 +129,17 @@ async def create_local_user_chosen(service: AuthService, *, password: str, **kwa _provision_steps = itertools.count(1) -def provision_totp() -> dict[str, Any]: +def provision_totp(secret: str = PROVISION_TOTP_SECRET) -> dict[str, Any]: """The TOTP arguments ``provision_first_administrator`` needs under the shipped ``require_mfa`` (ADR 0197 Amendment A): the secret, a code for it, and the instant that code was "read". Each call lands on its own later 30-second step, so repeated provisions of one row - (the repair tests) never replay a step the row already consumed.""" + (the repair tests) never replay a step the row already consumed. ``secret`` is for a test + whose runs must show different entries.""" from messagefoundry.auth import totp at = time.time() + totp.DEFAULT_PERIOD * next(_provision_steps) return { - "totp_secret": PROVISION_TOTP_SECRET, - "totp_code": totp.totp(PROVISION_TOTP_SECRET, now=at), + "totp_secret": secret, + "totp_code": totp.totp(secret, now=at), "totp_code_read_at": at, } diff --git a/tests/golden/webconsole_seam.snapshot b/tests/golden/webconsole_seam.snapshot index cdab31be4..b48e407d1 100644 --- a/tests/golden/webconsole_seam.snapshot +++ b/tests/golden/webconsole_seam.snapshot @@ -7,7 +7,7 @@ # by hand (BACKLOG #1220) - so a newly rendered DTO is covered with nobody editing a list. ## ENGINE_UI_SEAM -5b6fd34b36a52c84 +5d9c162790077dfc ## dataclass messagefoundry.api._ui_seam.UiDeps engine_seam @@ -196,7 +196,7 @@ oidc_flow_ttl_seconds: property -> 'int' password_change_owes_factor: (self, token: 'str | None') -> 'bool' reauth: (self, identity: 'Identity', password: 'str', *, token: 'str', client: 'str | None' = None, purpose: 'str | None' = None) -> 'Elevation' revoke_other_sessions: (self, identity: 'Identity', current_token_hash: 'str', *, actor: 'str') -> 'int' -revoke_own_session: (self, identity: 'Identity', session_id: 'str', *, actor: 'str') -> 'bool' +revoke_own_session: (self, identity: 'Identity', session_id: 'str', *, actor: 'str', client: 'str | None') -> 'bool' session_steps_up_at_idp: (self, token: 'str | None') -> 'bool' store: property -> 'AdminStore' suggested_notify_email: (profile_email: 'str | None') -> 'str | None' diff --git a/tests/test_ad_session_reconcile.py b/tests/test_ad_session_reconcile.py index 5aae0f14f..66a113e1e 100644 --- a/tests/test_ad_session_reconcile.py +++ b/tests/test_ad_session_reconcile.py @@ -697,6 +697,58 @@ def test_an_unkeyed_probe_does_not_hide_an_outage_or_a_referral_only_pass() -> N assert alone.aborted is None and (alone.probed, alone.asked) == (1, 0) +@pytest.mark.parametrize( + ("asked", "aborted"), + [ + ([reconcile.Probe("a", "a", reconcile.ProbeOutcome.UNAVAILABLE)], "directory_unavailable"), + ([reconcile.Probe("r", "r", reconcile.ProbeOutcome.REFERRED)], reconcile.REFERRAL_ABORT), + ( + [ + reconcile.Probe("a", "a", reconcile.ProbeOutcome.UNAVAILABLE), + reconcile.Probe("r", "r", reconcile.ProbeOutcome.REFERRED), + ], + reconcile.REFERRAL_ABORT, + ), + # BACKLOG #3100 shares the referral's early return, so a pass of faults only is one too. + ([reconcile.Probe("f", "f", reconcile.ProbeOutcome.FAULT)], reconcile.PROBE_FAULT_ABORT), + ( + [ + reconcile.Probe("a", "a", reconcile.ProbeOutcome.UNAVAILABLE), + reconcile.Probe("f", "f", reconcile.ProbeOutcome.FAULT), + ], + reconcile.PROBE_FAULT_ABORT, + ), + ], + ids=[ + "outage", + "referrals-only", + "referrals-and-failures", + "faults-only", + "faults-and-failures", + ], +) +def test_an_unkeyed_row_is_struck_on_a_pass_that_judged_nothing( + asked: list[reconcile.Probe], aborted: str +) -> None: + """BACKLOG #3170 item 3. An UNKEYED row is judged without the directory, so a pass that learned + nothing from it still strikes the row. Only the strike: the asked probes' strikes, the outcome + record the hold reads, and the hold's latch all stay as they were. The abort slug is pinned + too, so an id-less row cannot hide which kind of pass this was (BACKLOG #2434, #3100). + + RED before #3170: both early returns carried no strikes, so such a row went unstruck.""" + unkeyed = reconcile.Probe("k", "k", reconcile.ProbeOutcome.UNKEYED) + plan = _plan([*asked, unkeyed], prior_strikes={"k": 1, "a": 1, "r": 1, "f": 1}, latched=True) + assert plan.aborted == aborted + assert plan.judged_nothing + assert plan.strikes == {"k": 2} + assert plan.outcomes == {} and plan.latched and not plan.hold + assert plan.revocations == () + # The control: the same row on an ordinary pass strikes the same way, and revokes there. + ordinary = _plan([unkeyed], prior_strikes={"k": 1}) + assert ordinary.strikes == {"k": 2} + assert [r.reason for r in ordinary.revocations] == ["directory_object_id_missing"] + + def test_breaker_ceiling_reports_the_larger_of_the_two_thresholds() -> None: assert reconcile.breaker_ceiling(probed=10, max_absolute=5, max_fraction=0.34) == 5 assert reconcile.breaker_ceiling(probed=300, max_absolute=5, max_fraction=0.34) == 102 @@ -960,7 +1012,7 @@ async def test_a_referring_user_search_base_judges_nothing_and_pages_every_pass( "reason": "directory_referral", "referred": 2, "unavailable": 0, - "probed": 2, + "asked": 2, "aborted": "directory_referral", } assert len(referred) == 3 and all(d == expected for d in referred) @@ -1796,6 +1848,57 @@ async def test_a_referral_does_not_mark_an_id_less_row_it_can_never_confirm() -> await store.close() +async def test_a_referral_is_counted_over_the_probes_asked() -> None: + """BACKLOG #3170 item 1. An id-less row is never sent to the directory, so no referral could + answer it. The referral's row, its latched message and its alert count the probes asked, and + the row names the field ``asked``, as the outage's row does (#2434). + + RED before #3170: all three counted ``probed``, which takes in the id-less row.""" + from messagefoundry.api.app import _alert_reconcile_plan + from messagefoundry.pipeline.alerts import LoggingAlertSink + + store = await MessageStore.open(":memory:") + try: + ldap = _FakeLdap( + { + "alice": _principal("alice"), + "bob": _principal("bob"), + "jdoe": replace(_principal("jdoe"), directory_object_id=None), + } + ) + service = AuthService(store, _ad_settings(), ldap=ldap) # type: ignore[arg-type] + await service.initialize() + for name in ("alice", "bob", "jdoe"): + assert await _signed_in_ad_user(service, store, name) is not None + ldap.referring.add("alice") + + plan = await service.reconcile_directory_sessions() + + assert (plan.probed, plan.asked, len(plan.referred)) == (3, 2, 1) + [row] = _referred_rows(await store.list_audit()) + assert row["asked"] == 2 and "probed" not in row + message = service.directory_reconcile_referral + assert message is not None and "1 of 2 signed-in" in message + + counts: list[tuple[str, int]] = [] + + class _Sink(LoggingAlertSink): + def ad_reconcile_aborted( + self, name: str, *, reason: str, probed: int, detail: str + ) -> None: + counts.append((reason, probed)) + + _alert_reconcile_plan(plan, service, _Sink()) + assert counts == [("directory_referral", 2)] + # The breaker's page keeps its judged denominator, which counts the id-less row. + counts.clear() + tripped = replace(plan, aborted="mass_revoke_breaker") + _alert_reconcile_plan(tripped, service, _Sink()) + assert counts == [("mass_revoke_breaker", 3), ("directory_referral", 2)] + finally: + await store.close() + + async def test_a_breaker_trip_does_not_mark_an_id_less_row_or_forfeit_its_clear() -> None: """BACKLOG #2434, the breaker's half of the referral test above. A trip marks every candidate unconfirmed until a pass reads it PRESENT, and revoking a marked account forfeits the clear. An @@ -2251,8 +2354,13 @@ async def _reported(store: MessageStore) -> list[str]: @pytest.mark.parametrize( "refusal", - [sqlite3.OperationalError("synthetic: disk I/O error"), CipherError("synthetic: Transit down")], - ids=["driver", "transit-mac"], + [ + sqlite3.OperationalError("synthetic: disk I/O error"), + CipherError("synthetic: Transit down"), + # aiosqlite's closed connection, raised before sqlite3 is reached (BACKLOG #3145 carry-over). + ValueError("no active connection"), + ], + ids=["driver", "transit-mac", "closed-connection"], ) async def test_a_refused_skip_report_logs_once_per_pass_and_tries_the_next_account_next( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, refusal: Exception @@ -2261,7 +2369,8 @@ async def test_a_refused_skip_report_logs_once_per_pass_and_tries_the_next_accou the first failure ended the pass; now the report stops at the first refusal, which also keeps a pool that times out each write to one timeout per pass. The account it refused goes last next pass, so the next account is tried. Both are reported once the - store accepts the write. A Transit outage refuses like a driver error (ADR 0138).""" + store accepts the write. A Transit outage refuses like a driver error (ADR 0138), and so does a + closed aiosqlite connection.""" store = await MessageStore.open(":memory:") try: service = await _two_unkeyed_bindings(store) @@ -4299,8 +4408,8 @@ async def test_a_scope_revocation_sends_no_account_disabled_notice() -> None: #: vault BACKLOG #2140. The notice each revocation reason sends, decided once. ``None`` is no notice. -#: ACCOUNT_DISABLED says an administrator disabled the account, so only the reason that READ the -#: disabled bit may send it. +#: ACCOUNT_DISABLED says the directory reports the account disabled, so only the reason that READ +#: the disabled bit may send it. _NOTICE_FOR_REASON: dict[str, str | None] = { "directory_disabled": ACCOUNT_DISABLED, "directory_absent": DIRECTORY_SESSIONS_ENDED, @@ -4345,7 +4454,9 @@ async def test_each_revocation_reason_sends_its_decided_notice(reason: str) -> N expected = _NOTICE_FOR_REASON[reason] assert [e.event_type for e in notifier.sent] == ([] if expected is None else [expected]) if expected is not None: - assert notifier.sent[0].detail == {"reason": reason} + # ``source`` words ROLES_CHANGED and ACCOUNT_DISABLED for the directory (vault + # BACKLOG #3121); every reconciler notice carries it. + assert notifier.sent[0].detail == {"reason": reason, "source": "directory"} finally: await store.close() diff --git a/tests/test_apiclient_sign_in_timeout.py b/tests/test_apiclient_sign_in_timeout.py new file mode 100644 index 000000000..7a8c74b15 --- /dev/null +++ b/tests/test_apiclient_sign_in_timeout.py @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""The sign-in and the two elevations that queue per account get a longer timeout (vault BACKLOG +#2950). + +``EngineClient.login`` names the token it replaces, and the engine ends that session inside the +sign-in (BACKLOG #2281). A sign-in waiting in the engine's per-account queue could outlast the +client's 5 s default, and a lost reply then leaves the client holding a token the engine ended. +``login``, ``reauth``, ``verify_mfa`` and ``confirm_mfa`` now wait ``SIGN_IN_TIMEOUT_SECONDS`` +for their reply, which matches the IDE's ``SIGN_IN_TIMEOUT_MS``; their other phases, and every +other request, keep the client's own timeout. The queue has no bound, so this makes the lost reply +rarer, not impossible. + +The transport is an ``httpx.MockTransport``: what is pinned is the timeout each request carries, +read from ``request.extensions["timeout"]``, not what an engine does with it. +""" + +from __future__ import annotations + +from pathlib import Path + +import httpx + +from messagefoundry.apiclient import EngineClient +from messagefoundry.apiclient.client import SIGN_IN_TIMEOUT_SECONDS + +_BASE = "http://127.0.0.1:8765" +_USER = { + "user_id": "u1", + "username": "jdoe", + "auth_provider": "local", + "roles": ["viewer"], + "permissions": [], +} +#: Synthetic, low-entropy stand-ins for a session token. +_TOKEN = "t" * 43 +_ROTATED = "r" * 43 + + +def _reply(request: httpx.Request) -> httpx.Response: + path = request.url.path + if path == "/auth/login": + body: object = {"token": _TOKEN, "user": _USER} + elif path in ("/me/reauth", "/auth/mfa-verify"): + body = {"detail": "re-verified", "token": _ROTATED} + elif path == "/me/mfa/confirm": + body = {"recovery_codes": ["code-one"], "token": _ROTATED} + else: + body = _USER + return httpx.Response(200, json=body) + + +def _recording_client() -> tuple[EngineClient, dict[str, dict[str, float | None]]]: + """An ``EngineClient`` over a mock transport, and the timeout each path's request carried.""" + seen: dict[str, dict[str, float | None]] = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen[request.url.path] = dict(request.extensions["timeout"]) + return _reply(request) + + client = EngineClient(_BASE) + default = client._http.timeout + client._http.close() + # The client's own timeout, as _open_transport sets it, so the default is the one under test. + client._http = httpx.Client( + base_url=_BASE, timeout=default, transport=httpx.MockTransport(handler) + ) + return client, seen + + +def _every_phase(seconds: float) -> dict[str, float | None]: + return {"connect": seconds, "read": seconds, "write": seconds, "pool": seconds} + + +#: The requests that wait in the engine's per-account queue. +_QUEUED = ("/auth/login", "/me/reauth", "/auth/mfa-verify", "/me/mfa/confirm") + + +def test_the_sign_in_and_the_queued_elevations_wait_longer_for_their_reply() -> None: + """RED when: ``/auth/login``, ``/me/reauth``, ``/auth/mfa-verify`` or ``/me/mfa/confirm`` + waits only the client's 5 s default for its reply (vault BACKLOG #2950), when anything but + the read phase is raised, or when ``/auth/me`` loses that default.""" + client, seen = _recording_client() + try: + client.login("jdoe", "a-strong-test-passphrase") + client.reauth("a-strong-test-passphrase") + client.verify_mfa("123456") + assert client.confirm_mfa("123456") == ["code-one"] + client.me() + finally: + client.close() + # Only the read phase waits out the queue; connect, write and pool keep the client's bound, + # so an unreachable engine is still reported as fast as before. + longer = {**_every_phase(5.0), "read": SIGN_IN_TIMEOUT_SECONDS} + for path in _QUEUED: + assert seen[path] == longer, path + # The control: any other request keeps the client's default. + assert seen["/auth/me"] == _every_phase(5.0) + + +def test_the_timeout_matches_the_ides_sign_in_timeout() -> None: + """The two clients of the same queue wait the same time. The IDE's constant is read from its + source, so a change on either side reds here.""" + source = Path(__file__).resolve().parents[1] / "ide" / "src" / "signInSession.ts" + line = next( + ln for ln in source.read_text(encoding="utf-8").splitlines() if "SIGN_IN_TIMEOUT_MS =" in ln + ) + milliseconds = int(line.split("=", 1)[1].strip().rstrip(";").replace("_", "")) + assert milliseconds == SIGN_IN_TIMEOUT_SECONDS * 1000 diff --git a/tests/test_auth_oidc_service.py b/tests/test_auth_oidc_service.py index f09c0fd5c..2c61b681e 100644 --- a/tests/test_auth_oidc_service.py +++ b/tests/test_auth_oidc_service.py @@ -348,7 +348,8 @@ def spy( ) return urllib.request.build_opener() - monkeypatch.setattr("messagefoundry.auth.service.build_idp_opener", spy) + # The helper both build sites share (vault BACKLOG #3145) calls the module-level builder. + monkeypatch.setattr("messagefoundry.auth.oidc_http.build_idp_opener", spy) # The CRL path must name a real file: [auth].oidc_tls_crl_file refuses a missing one at load # (BACKLOG #1997). The spy never reads it, so its content does not matter. crl = tmp_path / "idp-crl.pem" diff --git a/tests/test_auth_service.py b/tests/test_auth_service.py index 095533667..5d94184cc 100644 --- a/tests/test_auth_service.py +++ b/tests/test_auth_service.py @@ -828,6 +828,8 @@ def role_changes() -> list[SecurityEvent]: assert len(changes) == 2 assert changes[-1].username == "jdoe" and changes[-1].email == "jdoe@example.org" assert changes[-1].detail.get("roles") == ["viewer"] + # Worded for the directory, as the reconciler's notice is (vault BACKLOG #3121). + assert changes[-1].detail.get("source") == "directory" finally: await store.close() @@ -945,6 +947,8 @@ async def test_clearing_the_profile_address_leaves_the_account_still_notifiable( later = notifier.events[before:] assert [e.event_type for e in later] == [ROLES_CHANGED] assert later[0].email == "bob@example.org" + # An administrator's act keeps the administrator wording (vault BACKLOG #3121). + assert "source" not in later[0].detail finally: await store.close() diff --git a/tests/test_credential_probe_off_loop.py b/tests/test_credential_probe_off_loop.py new file mode 100644 index 000000000..fc008e4a9 --- /dev/null +++ b/tests/test_credential_probe_off_loop.py @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""The start-up credential-generation probe runs off the event loop (vault BACKLOG #3164). + +``AuthService.probe_credential_generation`` runs the temporary-credential generator once at start. +A pathological ``[auth].password_extra_context_words`` list makes that generator retry its policy +screens many times, so the lifespan must not run it on the loop thread, the way the request-path +callers already do not (BACKLOG #2359). These drive the real lifespan and record where the +generator's screen ran. + +Severity is conditional (CLAUDE.md section 0): zero deployments, so a held loop is what a deploying +site would meet at start, never a live exposure. +""" + +from __future__ import annotations + +import asyncio +import logging +from pathlib import Path + +import pytest +from fastapi import FastAPI + +from messagefoundry.api import create_managed_app +from messagefoundry.auth import service as service_module +from messagefoundry.auth.policy import PasswordPolicy +from messagefoundry.config.settings import ( + AlertsSettings, + AuthSettings, + EgressSettings, + SecurityEnforcement, + SecuritySettings, + StoreSettings, +) +from messagefoundry.store.crypto import generate_key + + +def _app(db: Path) -> FastAPI: + """An engine that starts on an empty store: ``warn`` lets the ADR 0167 gate pass with no + Administrator, so the lifespan reaches the probe.""" + return create_managed_app( + store_settings=StoreSettings(path=str(db), encryption_key=generate_key()), + poll_interval=0.05, + auth_settings=AuthSettings(), + alerts_settings=AlertsSettings( + email_smtp_host="smtp.example.invalid", email_from="alerts@example.invalid" + ), + security_settings=SecuritySettings(enforcement=SecurityEnforcement.WARN), + egress_settings=EgressSettings(deny_by_default=False), + ) + + +async def _start(app: FastAPI) -> None: + async with app.router.lifespan_context(app): + pass + + +def test_the_lifespan_runs_the_generator_off_the_loop_thread( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + on_loop: list[bool] = [] + + def recording(policy: PasswordPolicy) -> str | None: + # A worker thread has no running loop; the loop thread does. + try: + asyncio.get_running_loop() + except RuntimeError: + on_loop.append(False) + else: + on_loop.append(True) + return None + + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(service_module, "credential_generation_problem", recording) + asyncio.run(_start(_app(tmp_path / "probe.db"))) + # The count is the control: an empty list would pass the assertion below vacuously. + assert on_loop == [False], "the probe ran once, and not on the event loop thread" + + +def test_a_probe_that_raises_off_the_loop_still_lets_the_start_continue( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """The guard still wraps the call now that it is awaited: a raise in the worker thread comes + back through ``asyncio.to_thread`` and is logged, and the start does not refuse.""" + + def broken(policy: PasswordPolicy) -> str | None: + raise ValueError("a later policy clause raised") + + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(service_module, "credential_generation_problem", broken) + with caplog.at_level(logging.ERROR, logger="messagefoundry.api.app"): + asyncio.run(_start(_app(tmp_path / "probe.db"))) # must not raise + assert any( + "the credential-generation probe could not run" in r.getMessage() for r in caplog.records + ) diff --git a/tests/test_directory_last_admin_alert.py b/tests/test_directory_last_admin_alert.py index e07178b99..ad4530b86 100644 --- a/tests/test_directory_last_admin_alert.py +++ b/tests/test_directory_last_admin_alert.py @@ -26,15 +26,26 @@ import pytest +from messagefoundry.api.security import DIRECTORY_GRANTED_BY from messagefoundry.auth.ldap import AdPrincipal, DirectoryAnswer, DirectoryProbe from messagefoundry.auth.permissions import Role -from messagefoundry.auth.service import NO_ENABLED_ADMINISTRATOR_ACTION, AuthService +from messagefoundry.auth.reconcile import SessionRevocation +from messagefoundry.auth.service import ( + NO_ENABLED_ADMINISTRATOR_ACTION, + AuthService, + _AuditWatermark, + _RevocationApplied, +) from messagefoundry.config.settings import ( _ALERT_CONTROL_EVENT_TYPES, _ALERT_EVENT_TYPES, AuthSettings, ) -from messagefoundry.pipeline.alerts import LoggingAlertSink, no_enabled_administrator_alert +from messagefoundry.pipeline.alerts import ( + LoggingAlertSink, + administrator_granted_alert, + no_enabled_administrator_alert, +) from messagefoundry.store.store import MessageStore ADMINS = "cn=mf-admins,dc=test,dc=invalid" @@ -75,10 +86,12 @@ def probe_principal(self, username: str, *, object_id: str | None = None) -> Dir class _Rig: - def __init__(self, store: MessageStore) -> None: + def __init__(self, store: MessageStore, **overrides: Any) -> None: self.store = store self.ldap = _FakeLdap() self.alerts: list[tuple[str, str]] = [] + #: What the vault BACKLOG #3141 grant callable was handed, ``(username, via)``. + self.grants: list[tuple[str, str]] = [] settings = AuthSettings( ad_enabled=True, ad_server="ldaps://dc.test.invalid", @@ -86,12 +99,14 @@ def __init__(self, store: MessageStore) -> None: ad_bind_dn="CN=svc-mefor,OU=Service,DC=test,DC=invalid", ad_bind_password="synthetic", ad_session_recheck_seconds=60, + **overrides, ) self.service = AuthService( store, settings, ldap=self.ldap, # type: ignore[arg-type] admin_loss_alert=lambda username, via: self.alerts.append((username, via)), + admin_grant_alert=lambda username, via: self.grants.append((username, via)), ) async def start(self) -> None: @@ -322,3 +337,173 @@ async def test_a_loss_after_a_regrant_is_not_held_back_by_the_cooldown() -> None losses = [e["connection"] for e in transport.events if e["type"] == "no_enabled_administrator"] assert losses.count("user:jdoe") == 2 assert losses.count("user:asmith") == 1 + + +# --- vault BACKLOG #3141: a reconcile pass that newly grants Administrator pages ---------------- +# +# The reconciler's role re-diff can give an account Administrator. It raised only +# ad_session_revoked, and the account's next sign-in then found the role already written and +# gained nothing, so no administrator_granted ever fired. The pass now raises it right after the +# role write, judged on the roles read through the account's seal at apply time. + +#: The ``via`` a reconcile pass's grant reports. +RECONCILE_VIA = "directory_reconcile" + + +async def test_a_reconcile_pass_that_newly_grants_administrator_pages_once(rig: _Rig) -> None: + """RED before #3141: the pass raised no grant, and the next sign-in gains nothing.""" + rig.ldap.present["jdoe"] = _principal("jdoe", OPERATORS) + await rig.sign_in("jdoe") + + await rig.directory_change("jdoe", ADMINS, via="ad-reconcile") + + assert await rig.roles("jdoe") == [Role.ADMINISTRATOR.value] + assert rig.grants == [("jdoe", RECONCILE_VIA)] + assert rig.alerts == [] + # Why the pass must page: the account's next sign-in finds the role written and gains nothing. + outcome = await rig.service._complete_ad_login( + rig.ldap.present["jdoe"], None, mfa_verified=True + ) + assert outcome.ok and outcome.roles_gained is None + assert rig.grants == [("jdoe", RECONCILE_VIA)] + + +async def test_a_reconcile_change_to_an_account_that_held_administrator_is_quiet( + rig: _Rig, +) -> None: + """Negative control: Administrator held before the write, so nothing was granted.""" + rig.ldap.present["jdoe"] = _principal("jdoe", ADMINS) + await rig.sign_in("jdoe") + + await rig.directory_change("jdoe", ADMINS, OPERATORS, via="ad-reconcile") + + assert sorted(await rig.roles("jdoe")) == sorted( + [Role.ADMINISTRATOR.value, Role.OPERATOR.value] + ) + assert rig.grants == [] + + +async def test_a_reconcile_gain_of_another_role_only_is_quiet(rig: _Rig) -> None: + """Negative control: a role was gained, but not Administrator.""" + rig.ldap.present["jdoe"] = _principal("jdoe") + await rig.sign_in("jdoe") + + await rig.directory_change("jdoe", OPERATORS, via="ad-reconcile") + + assert await rig.roles("jdoe") == [Role.OPERATOR.value] + assert rig.grants == [] + + +async def test_the_grant_is_judged_on_the_roles_read_at_apply_time_not_the_plans( + rig: _Rig, monkeypatch: pytest.MonkeyPatch +) -> None: + """A sign-in that lands between the pass's read and its write has already written + Administrator, and paged for it. The write then adds nothing, so the pass must not page again. + RED when the before-set is taken from the plan, which read the roles before that sign-in. + + The sign-in writes Operator beside Administrator, which the pass's write then removes. Were the + stored roles equal to the pass's target, #3085's apply-time check would skip the write and the + grant check with it, and this test would pass whatever before-set the service used.""" + rig.ldap.present["jdoe"] = _principal("jdoe", OPERATORS) + await rig.sign_in("jdoe") + rig.ldap.present["jdoe"] = _principal("jdoe", ADMINS) + real = rig.service._apply_reconcile_revocation + + async def a_sign_in_lands_first( + revocation: SessionRevocation, *, watermark: _AuditWatermark | None + ) -> _RevocationApplied: + await rig.store.set_user_roles( + revocation.user_id, + [Role.ADMINISTRATOR.value, Role.OPERATOR.value], + assigned_by="ad-sync", + ) + return await real(revocation, watermark=watermark) + + monkeypatch.setattr(rig.service, "_apply_reconcile_revocation", a_sign_in_lands_first) + + plan = await rig.service.reconcile_directory_sessions() + + assert [(r.username, r.reason) for r in plan.revocations] == [("jdoe", "roles_changed")] + # The control: the pass's own write ran (Operator is gone), so the grant check was reached. + assert await rig.roles("jdoe") == [Role.ADMINISTRATOR.value] + assert rig.grants == [] + + +async def test_a_pass_the_breaker_aborts_grants_nothing_and_pages_nothing() -> None: + """An aborted pass writes no roles, so there is no grant to page (ADR 0079 mechanism 2).""" + store = await MessageStore.open(":memory:") + try: + # One revocation of one probed account trips a breaker this tight. + rig = _Rig(store, ad_session_revoke_max=0, ad_session_revoke_max_fraction=0.01) + await rig.start() + rig.ldap.present["jdoe"] = _principal("jdoe", OPERATORS) + await rig.sign_in("jdoe") + rig.ldap.present["jdoe"] = _principal("jdoe", ADMINS) + + plan = await rig.service.reconcile_directory_sessions() + + assert plan.aborted == "mass_revoke_breaker" + assert await rig.roles("jdoe") == [Role.OPERATOR.value] + assert rig.grants == [] + finally: + await store.close() + + +async def test_a_raising_grant_callable_still_ends_the_sessions(rig: _Rig) -> None: + """Only the production wrapper promises not to raise, so the service contains a raise: the + session revocation after the role write still runs.""" + + def raising(username: str, via: str) -> None: + raise RuntimeError("synthetic: the alert path is down") + + rig.service._admin_grant_alert = raising + rig.ldap.present["jdoe"] = _principal("jdoe", OPERATORS) + await rig.sign_in("jdoe") + + await rig.directory_change("jdoe", ADMINS, via="ad-reconcile") + + assert await rig.roles("jdoe") == [Role.ADMINISTRATOR.value] + actions = [r["action"] for r in await rig.store.list_audit()] + assert "auth.ad_session_revoked" in actions + + +def test_the_grant_callable_keys_on_the_account_and_never_raises() -> None: + raised: list[tuple[str, str, str]] = [] + + class _Sink(LoggingAlertSink): + def administrator_granted(self, name: str, *, via: str, granted_by: str) -> None: + raised.append((name, via, granted_by)) + + administrator_granted_alert(_Sink(), granted_by=DIRECTORY_GRANTED_BY)("jdoe", RECONCILE_VIA) + assert raised == [("user:jdoe", RECONCILE_VIA, DIRECTORY_GRANTED_BY)] + + class _Broken(LoggingAlertSink): + def administrator_granted(self, name: str, *, via: str, granted_by: str) -> None: + raise RuntimeError("synthetic: the sink is down") + + administrator_granted_alert(_Broken(), granted_by=DIRECTORY_GRANTED_BY)("jdoe", RECONCILE_VIA) + + +async def test_a_loss_after_a_reconciler_regrant_is_not_held_back_by_the_cooldown() -> None: + """The #3229 cooldown pop, through the reconciler's own grant callable: the second loss is + sent.""" + from messagefoundry.pipeline.alert_sinks import NotifierAlertSink + from tests.test_alert_sinks import _drain, _RecordingTransport + + transport = _RecordingTransport("webhook") + sink = NotifierAlertSink([transport]) # the default cooldown, far longer than this test + lose = no_enabled_administrator_alert(sink) + regrant = administrator_granted_alert(sink, granted_by=DIRECTORY_GRANTED_BY) + + lose("jdoe", "ad-reconcile") + # The control: a grant that cleared every account's cooldown would let asmith's second loss + # through and fail the count below. + lose("asmith", "ad-reconcile") + regrant("jdoe", RECONCILE_VIA) + lose("jdoe", "ad-reconcile") + lose("asmith", "ad-reconcile") + await _drain(sink) + + losses = [e["connection"] for e in transport.events if e["type"] == "no_enabled_administrator"] + assert losses.count("user:jdoe") == 2 + assert losses.count("user:asmith") == 1 diff --git a/tests/test_directory_sign_in_admin_alert.py b/tests/test_directory_sign_in_admin_alert.py index 9c306a550..38a935db1 100644 --- a/tests/test_directory_sign_in_admin_alert.py +++ b/tests/test_directory_sign_in_admin_alert.py @@ -245,4 +245,7 @@ async def test_a_sink_that_raises_does_not_break_the_sign_in( r = await _negotiate(c) assert r.status_code == 200, r.text assert sink.events == [_GRANT] - assert any("failed to emit" in rec.getMessage() for rec in caplog.records) + assert any( + rec.name == "messagefoundry.api.security" and "failed to emit" in rec.getMessage() + for rec in caplog.records + ) diff --git a/tests/test_mfa.py b/tests/test_mfa.py index fd4070ef1..e8e63f175 100644 --- a/tests/test_mfa.py +++ b/tests/test_mfa.py @@ -18,6 +18,8 @@ import asyncio import heapq import inspect +import json +import logging import time import unicodedata from collections.abc import Awaitable @@ -2503,7 +2505,12 @@ async def reset_then_enable( rows = await store.list_audit( actor=ADMIN_USERNAME, action="auth.mfa_enroll_refused", limit=10 ) - assert len(rows) == 1 and '"reason": "secret_cleared"' in str(rows[0]["detail"]) + # ``session_ended`` is read from the revoke's outcome, not a constant (vault BACKLOG #3113). + assert len(rows) == 1 + assert json.loads(str(rows[0]["detail"])) == { + "reason": "secret_cleared", + "session_ended": True, + } actions = [e["action"] for e in await service.security_events_for(ADMIN_USERNAME)] assert "auth.mfa_enrolled" not in actions finally: @@ -2605,6 +2612,116 @@ async def other_confirm_first( await store.close() +@pytest.mark.parametrize( + "fault", + [ + RuntimeError("the store refused the revoke"), + # aiosqlite's own refusal of a closed connection, raised before sqlite3 is reached. + ValueError("Connection closed"), + ], + ids=["store-refusal", "closed-connection"], +) +async def test_a_refused_enable_whose_revoke_raises_still_writes_its_refusal_row( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, fault: Exception +) -> None: + """RED when: a store refusal of the revoke skips the ``auth.mfa_enroll_refused`` row (vault + BACKLOG #3113 item 2), or the row claims the session ended when it did not (item 7).""" + store = await _store() + try: + service = AuthService(store, AuthSettings(mfa_verify_min_elapsed_seconds=0)) + identity, token, _ = await login_admin(service) + enroll = await service.begin_mfa_enrollment(identity) + + real_enable = store.enable_totp + + async def reset_then_enable( + user_id: str, + *, + recovery_code_hashes: list[str], + expected_secret_cell: str | None, + now: float | None = None, + ) -> bool: + await store.disable_totp(user_id) + return await real_enable( + user_id, + recovery_code_hashes=recovery_code_hashes, + expected_secret_cell=expected_secret_cell, + now=now, + ) + + async def refused_revoke(token_hash: str, *, now: float | None = None) -> None: + raise fault + + monkeypatch.setattr(store, "enable_totp", reset_then_enable) + monkeypatch.setattr(store, "revoke_session", refused_revoke) + with caplog.at_level(logging.ERROR, logger="messagefoundry.auth.service"): + refused = await service.confirm_mfa_enrollment( + identity, fresh_totp(enroll.secret), token=token + ) + # Fails closed all the same: no token and no codes come back. + assert refused.ok is False and refused.session_lost is True + assert refused.token is None and refused.recovery_codes == () + rows = await store.list_audit( + actor=ADMIN_USERNAME, action="auth.mfa_enroll_refused", limit=10 + ) + assert len(rows) == 1, "the refusal row was skipped when the revoke raised" + detail = json.loads(str(rows[0]["detail"])) + assert detail == {"reason": "secret_cleared", "session_ended": False} + assert any( + "could not end the session it rotated" in r.getMessage() for r in caplog.records + ), "the refused revoke was swallowed without a log line" + finally: + await store.close() + + +async def test_a_refused_enable_whose_revoke_hits_a_defect_still_writes_its_refusal_row( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """RED when: a defect the revoke raises, which the soft write re-raises rather than logs, skips + the ``auth.mfa_enroll_refused`` row (vault BACKLOG #3113 item 2, round two). The defect still + propagates, so a bug is not passed over as a store refusal.""" + store = await _store() + try: + service = AuthService(store, AuthSettings(mfa_verify_min_elapsed_seconds=0)) + identity, token, _ = await login_admin(service) + enroll = await service.begin_mfa_enrollment(identity) + + real_enable = store.enable_totp + + async def reset_then_enable( + user_id: str, + *, + recovery_code_hashes: list[str], + expected_secret_cell: str | None, + now: float | None = None, + ) -> bool: + await store.disable_totp(user_id) + return await real_enable( + user_id, + recovery_code_hashes=recovery_code_hashes, + expected_secret_cell=expected_secret_cell, + now=now, + ) + + async def broken_revoke(token_hash: str, *, now: float | None = None) -> None: + raise NotImplementedError("a backend without the revoke") + + monkeypatch.setattr(store, "enable_totp", reset_then_enable) + monkeypatch.setattr(store, "revoke_session", broken_revoke) + with pytest.raises(NotImplementedError): + await service.confirm_mfa_enrollment(identity, fresh_totp(enroll.secret), token=token) + rows = await store.list_audit( + actor=ADMIN_USERNAME, action="auth.mfa_enroll_refused", limit=10 + ) + assert len(rows) == 1, "the refusal row was skipped when the revoke raised a defect" + assert json.loads(str(rows[0]["detail"])) == { + "reason": "secret_cleared", + "session_ended": False, + } + finally: + await store.close() + + # --- BACKLOG #3025: the enable turns on only the secret the confirm verified ----------------------- diff --git a/tests/test_oidc_callback_floor_exempt_amr.py b/tests/test_oidc_callback_floor_exempt_amr.py index 47e050287..7945c6b60 100644 --- a/tests/test_oidc_callback_floor_exempt_amr.py +++ b/tests/test_oidc_callback_floor_exempt_amr.py @@ -14,17 +14,24 @@ import json import time import urllib.parse +from dataclasses import replace from typing import Any import pytest from cryptography.hazmat.primitives.asymmetric import rsa from messagefoundry.auth import oidc +from messagefoundry.auth import service as service_module from messagefoundry.auth.audit_visibility import ( DIRECTORY_LOCKED_REFUSAL_DETAIL, HIDDEN_FROM_READERS_WITHOUT_USERS_MANAGE, ) -from messagefoundry.auth.service import TOO_EARLY, AuthService +from messagefoundry.auth.service import ( + DIRECTORY_ROLE_MAP_UNSEALED, + DIRECTORY_ROLES_DEMOTED, + TOO_EARLY, + AuthService, +) from messagefoundry.config.settings import AuthSettings, _auth_limit_loosenings from messagefoundry.store.store import MessageStore from tests.test_auth_oidc_service import ( @@ -410,7 +417,15 @@ async def test_a_step_up_past_the_floor_records_no_exemption( await store.close() -@pytest.mark.parametrize("refusal", ["not_in_directory", "step_up_subject_mismatch"]) +@pytest.mark.parametrize( + "refusal", + [ + "not_in_directory", + "step_up_subject_mismatch", + DIRECTORY_ROLES_DEMOTED, + DIRECTORY_ROLE_MAP_UNSEALED, + ], +) @pytest.mark.parametrize(("elapsed", "marked"), [(FLOOR - 0.001, True), (FLOOR + 0.001, False)]) async def test_a_refused_step_up_carries_the_marker_only_when_the_exemption_applied( rsa_key: rsa.RSAPrivateKey, @@ -420,7 +435,8 @@ async def test_a_refused_step_up_carries_the_marker_only_when_the_exemption_appl marked: bool, ) -> None: """BACKLOG #3220, the step-up leg: a refusal after the exchange carries the marker when the - exemption let the too-early callback past the floor, and only then.""" + exemption let the too-early callback past the floor, and only then. The two role arms ride the + bound refuse partial that carries the marker since main's #3082 reason merged into it.""" store = await MessageStore.open(":memory:") try: service, clock, token = await _signed_in( @@ -429,6 +445,12 @@ async def test_a_refused_step_up_carries_the_marker_only_when_the_exemption_appl claims: dict[str, Any] = {"amr": ["mfa", EXEMPT, "x-token-only"]} if refusal == "not_in_directory": service._ldap._principal = None # type: ignore[union-attr] + elif refusal == DIRECTORY_ROLES_DEMOTED: + # The account signed in as operator; its groups now map to no role. + ldap: Any = service._ldap + ldap._principal = replace(ldap._principal, groups=frozenset()) + elif refusal == DIRECTORY_ROLE_MAP_UNSEALED: + monkeypatch.setattr(service_module, "ad_map_seal_ok", lambda _sealer, _current: False) else: claims["sub"] = "S-1-5-21-someone-else" flow_id, _url = await _begin(service, token) diff --git a/tests/test_oidc_json_step_up_hint.py b/tests/test_oidc_json_step_up_hint.py index 8d22372c3..5b90a5378 100644 --- a/tests/test_oidc_json_step_up_hint.py +++ b/tests/test_oidc_json_step_up_hint.py @@ -31,7 +31,8 @@ from fastapi import Depends, FastAPI from messagefoundry.api import create_app -from messagefoundry.api.security import _STEP_UP_VIA_HEADER, require_reauth_only +from messagefoundry.api.auth_models import STEP_UP_VIA_HEADER as _STEP_UP_VIA_HEADER +from messagefoundry.api.security import require_reauth_only from messagefoundry.apiclient import ApiError, EngineClient, IdpStepUpRequired from messagefoundry.auth import Role, totp from messagefoundry.auth.identity import ALL_CHANNELS, Identity @@ -172,6 +173,9 @@ def _assert_idp_hint(site: str, r: httpx.Response, action: str | None) -> None: assert r.headers.get("X-Step-Up-Action") == action, site detail = r.json()["detail"] assert "/ui/reauth" in detail, (site, detail) + # Vault BACKLOG #3145 item 6: a bare /ui/reauth names no action and bounces to /ui, so the + # hint sends the reader back through the action, which reaches /ui/reauth with it named. + assert "repeat the action in the web console" in detail, (site, detail) assert "/me/reauth" not in detail, (site, detail) # BACKLOG #2142: a code opens no window on an oidc session, so the hint must not name it. assert "mfa-verify" not in detail, (site, detail) @@ -404,3 +408,30 @@ def _prompt() -> bool: client.close() assert not isinstance(raised.value, IdpStepUpRequired) assert prompted == ["step-up"] + + +def test_the_shared_step_up_header_keeps_its_wire_spelling() -> None: + """Vault BACKLOG #3145 item 6: the gates, the two route refusals and the FastAPI-free client + read one constant from the auth wire models. Its spelling is the wire contract, so a rename + there would break every older client; the replay tests above pin that each side uses it.""" + from messagefoundry.api import auth_models + + assert auth_models.STEP_UP_VIA_HEADER == "X-Step-Up-Via" + assert auth_models.STEP_UP_VIA_IDP == "idp" + + +async def test_a_store_fault_reading_the_mechanism_keeps_the_403( + engine: Engine, service: AuthService, monkeypatch: pytest.MonkeyPatch +) -> None: + """RED when: a store fault in the read that picks the refusal's wording turns the gate's 403 + into a 500 (vault BACKLOG #3145 item 6). It keeps the refusal and the password wording.""" + uid = await _add_admin(service) + token = await _session(service, uid, "tok-fault-oidc", mechanism="oidc") + + async def refused_read(token: str | None) -> bool: + raise OSError("the store connection dropped") + + monkeypatch.setattr(service, "session_steps_up_at_idp", refused_read) + async with _client_at(engine, service, HOME) as c: + r = await c.post("/me/mfa/enroll", headers=_auth(token)) + _assert_password_refusal("require_reauth_only_action", r, STEP_UP_ACTION_MFA_ENROLL) diff --git a/tests/test_oidc_step_up.py b/tests/test_oidc_step_up.py index 0540636f6..03b8975d8 100644 --- a/tests/test_oidc_step_up.py +++ b/tests/test_oidc_step_up.py @@ -413,6 +413,51 @@ async def test_the_step_up_row_records_whether_the_address_moved_mid_ceremony( await store.close() +async def test_a_refusal_after_the_flow_is_redeemed_records_the_address_fields( + rsa_key: rsa.RSAPrivateKey, monkeypatch: pytest.MonkeyPatch +) -> None: + """RED when: a step-up refused after its flow was redeemed leaves out ``start_client`` and + ``client_moved``, which the success row carries (vault BACKLOG #3145 item 4). They come from the + flow the start leg staged, never from the token.""" + store = await MessageStore.open(":memory:") + try: + service = await _service(store, rsa_key) + token = await _oidc_session(service, monkeypatch, rsa_key) + flow_id, _url = await _begin(service, token, client="10.0.0.8") + stale = _staged(service, flow_id).issued_at - 2 * service._settings.oidc_clock_skew_seconds + out = await _return_from_idp( + service, monkeypatch, rsa_key, flow_id, client="10.0.0.9", auth_time=stale - 1 + ) + + assert not out.ok and out.reason == STEP_UP_NOT_FRESH + detail = json.loads(str((await _audit_rows(store, "auth.reauth"))[-1]["detail"])) + assert detail["reason"] == STEP_UP_NOT_FRESH + assert (detail["start_client"], detail["client_moved"]) == ("10.0.0.8", True) + finally: + await store.close() + + +async def test_a_refusal_before_the_state_matches_records_no_address_fields( + rsa_key: rsa.RSAPrivateKey, monkeypatch: pytest.MonkeyPatch +) -> None: + """The control for vault BACKLOG #3145 item 4: before ``state`` matches, the callback is not + shown to belong to the flow, so its row compares nothing with the flow's start address.""" + store = await MessageStore.open(":memory:") + try: + service = await _service(store, rsa_key) + token = await _oidc_session(service, monkeypatch, rsa_key) + flow_id, _url = await _begin(service, token, client="10.0.0.8") + out = await service.complete_oidc_step_up( + flow_id=flow_id, state="forged", code=AUTH_CODE, client="10.0.0.9", public_origin=ORIGIN + ) + + assert not out.ok and out.reason == "state_mismatch" + detail = json.loads(str((await _audit_rows(store, "auth.reauth"))[-1]["detail"])) + assert "start_client" not in detail and "client_moved" not in detail + finally: + await store.close() + + # --- the callback: every refusal elevates nothing ----------------------------------------------------- @@ -815,6 +860,9 @@ async def test_an_account_demoted_in_the_directory_since_sign_in_is_refused( "provider": "ad", "mech": "oidc", "reason": DIRECTORY_ROLES_DEMOTED, + # Both legs at the helpers' default address (vault BACKLOG #3145 item 4). + "start_client": "10.0.0.9", + "client_moved": False, } await _assert_untouched(service, token) assert set(await store.get_user_role_ids(user_id)) == {"operator"}, "roles were written" @@ -847,6 +895,20 @@ async def test_a_role_map_that_fails_its_seal_is_not_called_a_demotion( assert not out.ok and out.reason == DIRECTORY_ROLE_MAP_UNSEALED assert out.error and "sign out" not in out.error.lower(), out.error assert "administrator" in out.error, out.error + # The row comes from the bound refuse partial, so it carries the flow's address fields as + # the demotion arm's row does (vault BACKLOG #3145 item 4). Pins the merge of main's #3082 + # reason with that partial. + rows = [json.loads(str(r["detail"])) for r in await _audit_rows(store, "auth.reauth")] + assert rows == [ + { + "ok": False, + "provider": "ad", + "mech": "oidc", + "reason": DIRECTORY_ROLE_MAP_UNSEALED, + "start_client": "10.0.0.9", + "client_moved": False, + } + ], rows await _assert_untouched(service, token) finally: await store.close() diff --git a/tests/test_provision_first_administrator.py b/tests/test_provision_first_administrator.py index 64155a0c0..a3eacb6ca 100644 --- a/tests/test_provision_first_administrator.py +++ b/tests/test_provision_first_administrator.py @@ -19,6 +19,7 @@ import time import types from collections.abc import Callable, Sequence +from contextvars import ContextVar from pathlib import Path import pytest @@ -35,6 +36,7 @@ HOLDER_NOTICE_NO_PRIOR_ADDRESS, AuthService, FirstAdministratorRefused, + ProvisionedAdministrator, ) from messagefoundry.config.secretprovider import SecretProviderError from messagefoundry.config.settings import AlertsSettings, AuthSettings @@ -2859,3 +2861,91 @@ def test_the_last_wrong_code_is_not_told_to_try_again( != 0 ) assert sum("did not match" in text for text in terminal.shown) == 4 + + +# --- vault BACKLOG #3113 item 1: the run that loses an enrolment race -------------------------- + +#: A second synthetic base32 secret, so the two runs show different entries. Two symbols on +#: purpose, for the reason ``_PASSWORD`` above is low-entropy: it must not look like a secret. +_OTHER_TOTP_SECRET = "ABABABABABABABAB" + + +async def test_the_run_that_loses_an_enrolment_race_is_told_to_remove_its_dead_entry( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Two runs repair one roleless row. The second stages its secret after the first, and both + reach their enables in a fixed order, the first run's first. That is the re-stage race of + vault BACKLOG #3025. Since #3025 each enable is bound to the cell its own run staged, so the + first run's enable is refused and the second run turns on its OWN secret, the one it showed. + So the refused run's entry is dead, and its words tell its operator to remove it (vault + BACKLOG #3113 item 1 had them say keep it, for the race #3025 closed). The waits are bounded, + so a run that refuses before its enable fails the test rather than hanging it.""" + store = await MessageStore.open(":memory:") + try: + service = AuthService(store, AuthSettings()) + await service.initialize() + await _roleless_account(store, email=None) + first = provision_totp() + second = provision_totp(_OTHER_TOTP_SECRET) + real_enable = store.enable_totp + second_at_enable = asyncio.Event() + first_enabled = asyncio.Event() + run: ContextVar[str] = ContextVar("run", default="first") + tasks: list[asyncio.Task[ProvisionedAdministrator]] = [] + + async def run_second() -> ProvisionedAdministrator: + run.set("second") + return await service.provision_first_administrator( + username="site-admin", password=_PASSWORD, actor="second", **second + ) + + async def racing_enable( + user_id: str, + *, + recovery_code_hashes: list[str], + expected_secret_cell: str | None, + now: float | None = None, + ) -> bool: + if run.get() == "first": + # The first run has staged its secret. The second run now stages its own over it, + # and stops at its own enable. + tasks.append(asyncio.create_task(run_second())) + await asyncio.wait_for(second_at_enable.wait(), timeout=10) + try: + return await real_enable( + user_id, + recovery_code_hashes=recovery_code_hashes, + expected_secret_cell=expected_secret_cell, + now=now, + ) + finally: + first_enabled.set() + second_at_enable.set() + await asyncio.wait_for(first_enabled.wait(), timeout=10) + return await real_enable( + user_id, + recovery_code_hashes=recovery_code_hashes, + expected_secret_cell=expected_secret_cell, + now=now, + ) + + monkeypatch.setattr(store, "enable_totp", racing_enable) + with pytest.raises(FirstAdministratorRefused) as lost: + await service.provision_first_administrator( + username="site-admin", password=_PASSWORD, actor="first", **first + ) + won = await tasks[0] + assert won.repaired + # The #3025 invariant, measured: the secret turned on is the one the winning run showed. + row = await store.get_user("roleless") + assert row is not None and row.totp_enabled + assert await store.get_totp_secret("roleless") == _OTHER_TOTP_SECRET + message = str(lost.value) + assert "did not turn TOTP on and granted no role" in message + assert "remove the authenticator entry it showed" in message + finally: + # A failure above must not leave the second run running against a closed store. + for task in tasks: + task.cancel() + await asyncio.gather(*tasks, return_exceptions=True) + await store.close() diff --git a/tests/test_revocation_guard_census.py b/tests/test_revocation_guard_census.py index 75153d4f5..15947c4fc 100644 --- a/tests/test_revocation_guard_census.py +++ b/tests/test_revocation_guard_census.py @@ -69,6 +69,9 @@ "_verifying_context", "_build_tls_context", "build_idp_opener", + # The settings-reading wrapper both IdP opener sites call (vault BACKLOG #3145), so those + # two stay the sites and the wrapper hands its opener on, as build_idp_opener does. + "build_idp_opener_from_settings", "NarrowedTls", } ) diff --git a/tests/test_security_notify.py b/tests/test_security_notify.py index 7a13d90d8..273a6b299 100644 --- a/tests/test_security_notify.py +++ b/tests/test_security_notify.py @@ -15,6 +15,7 @@ import pytest from messagefoundry.auth.notifications import ( + ACCOUNT_DISABLED, ACCOUNT_LOCKED, DIRECTORY_SESSIONS_ENDED, EMAIL_CHANGED, @@ -24,6 +25,7 @@ NOTIFY_EMAIL_SET, PASSWORD_RESET, RECOVERY_CODE_USED, + ROLES_CHANGED, TEMPORARY_CREDENTIAL_EXPIRING, TEMPORARY_CREDENTIAL_EXPIRING_FOR_ISSUER, USERNAME_CHANGED, @@ -36,6 +38,7 @@ _SUBJECTS, SecurityEventNotifier, _build_body, + _subject, security_notifier_from_settings, ) @@ -203,6 +206,11 @@ def test_a_non_last_factor_removal_renders_its_own_subject_and_body() -> None: assert "disabled" not in body.lower() +#: The generic line a directory email change or rename carries (BACKLOG #1139). +_DIRECTORY_LINE = ( + "This change came from your organization's directory, not from the MessageFoundry console." +) + #: Every reconcile reason that sends DIRECTORY_SESSIONS_ENDED (vault BACKLOG #2140, #3171): all of #: ``REVOKE_REASONS`` but the read disabled bit, which sends ACCOUNT_DISABLED. Derived, so a reason #: added there reaches the per-reason render test below with no edit here. @@ -216,27 +224,33 @@ def test_the_sessions_ended_reasons_include_the_unkeyed_one() -> None: assert "directory_object_id_missing" in _SESSIONS_ENDED_REASONS +@pytest.mark.parametrize("tagged", [False, True], ids=["untagged", "as-sent"]) @pytest.mark.parametrize("reason", _SESSIONS_ENDED_REASONS) -def test_a_directory_sessions_ended_notice_is_true_for_each_reason(reason: str) -> None: +def test_a_directory_sessions_ended_notice_is_true_for_each_reason( + reason: str, tagged: bool +) -> None: """vault BACKLOG #2140, #3171. Both renderers fall back silently, so a half-wired kind would mail the generic wording. The DESCRIPTION is pinned here, not words the subject alone supplies. For ``directory_object_id_missing`` no directory lookup ran, so neither the subject nor the body may say a check against the directory ended the sessions. The directory never answered that the - account is disabled or gone, and an engine act cannot carry the "if this was you" closing.""" + account is disabled or gone, nor changed anything, and an engine act cannot carry the "if this + was you" closing. ``as-sent`` is the shape the reconciler mails since vault BACKLOG #3121, which + tags it ``source`` directory; the tag must not bring in the generic directory line.""" assert DIRECTORY_SESSIONS_ENDED in _SUBJECTS - subject = _SUBJECTS[DIRECTORY_SESSIONS_ENDED] + detail: dict[str, Any] = {"reason": reason} + if tagged: + detail["source"] = "directory" + event = SecurityEvent(DIRECTORY_SESSIONS_ENDED, username="jdoe", email=None, detail=detail) + subject = _subject(event) # Pinned exactly: a subject naming the directory as the actor is false for the unkeyed reason. assert subject == "Your MessageFoundry sessions were ended" - body = _build_body( - SecurityEvent( - DIRECTORY_SESSIONS_ENDED, username="jdoe", email=None, detail={"reason": reason} - ) - ) + body = _build_body(event) description = ( "MessageFoundry ended your sessions. It could not confirm from your organization's " "directory that your account may still sign in." ) assert description in body.splitlines() + assert _DIRECTORY_LINE not in body.splitlines() text = (subject + "\n" + body).lower() assert "a security event occurred on your account." not in text for claim in ("disabled", "deleted", "if this was you", "directory check", "a check against"): @@ -246,6 +260,84 @@ def test_a_directory_sessions_ended_notice_is_true_for_each_reason(reason: str) ) +#: vault BACKLOG #3121. The description each directory-driven kind carries, pinned whole. +_DIRECTORY_TEXT = { + ROLES_CHANGED: ( + "MessageFoundry read your groups in your organization's directory, changed your " + "account's roles to match them, and ended any sessions already open on your account." + ), + ACCOUNT_DISABLED: ( + "Your organization's directory reports your account as disabled, so MessageFoundry ended " + "your sessions." + ), +} +_DIRECTORY_CLOSING = "If you did not expect this change, contact your MessageFoundry administrator." +_ADMIN_TEXT = { + ROLES_CHANGED: "Your account's roles were changed by an administrator.", + ACCOUNT_DISABLED: "Your account was disabled by an administrator.", +} +_IF_THIS_WAS_YOU = ( + "If this was you, no action is needed. If not, contact your MessageFoundry administrator." +) + + +@pytest.mark.parametrize("kind", [ROLES_CHANGED, ACCOUNT_DISABLED]) +def test_a_directory_role_or_disable_notice_names_the_directory_and_no_administrator( + kind: str, +) -> None: + """vault BACKLOG #3121. The reconciler and the sign-in re-sync tag these ``source`` directory: + the holder did nothing, so neither the administrator wording nor "if this was you" may appear. + Nor the generic directory line: a role re-sync runs through the group-to-role map an + administrator edits in the console, so "not from the MessageFoundry console" could be false.""" + event = SecurityEvent( + kind, + username="jdoe", + email="j@example.org", + detail={"reason": "directory_disabled", "source": "directory"}, + ) + body = _build_body(event) + lines = body.splitlines() + assert _DIRECTORY_TEXT[kind] in lines + assert _DIRECTORY_LINE not in lines + assert _ADMIN_TEXT[kind] not in lines + assert "if this was you" not in body.lower() + assert body.endswith(_DIRECTORY_CLOSING) + # The administrator subject says the MessageFoundry account was disabled, which is false here. + assert "was disabled" not in _subject(event) + + +@pytest.mark.parametrize("kind", [ROLES_CHANGED, ACCOUNT_DISABLED]) +def test_an_administrator_role_or_disable_notice_keeps_its_wording(kind: str) -> None: + """vault BACKLOG #3121, closing item 2: the notice an administrator's own act sends keeps its + text. Kept by the row's instruction, not judged correct: "if this was you" on an administrator's + act is the shape BACKLOG #1141 fixed for the reset notice, and vault #3072 item 10 names.""" + body = _build_body(SecurityEvent(kind, username="jdoe", email="j@example.org")) + lines = body.splitlines() + assert _ADMIN_TEXT[kind] in lines + assert _DIRECTORY_TEXT[kind] not in lines + assert "directory" not in body.lower() + assert body.endswith(_IF_THIS_WAS_YOU) + + +def test_the_sign_in_role_re_sync_notice_says_where_the_sign_in_came_from() -> None: + """vault BACKLOG #3121. The sign-in re-sync passes the client address, and a directory notice + must not print it as the address of whoever made the change.""" + body = _build_body( + SecurityEvent( + ROLES_CHANGED, + username="jdoe", + email="j@example.org", + client_ip="203.0.113.7", + detail={"roles": ["viewer"], "source": "directory"}, + ) + ) + assert "Source IP" not in body + assert ( + "MessageFoundry picked up this change during a sign-in to your account from 203.0.113.7." + in body.splitlines() + ) + + def test_body_says_a_directory_repoint_came_from_the_directory() -> None: """BACKLOG #1139 (ASVS 6.3.7): where the change came from decides what the reader can DO. A directory-driven repoint is not editable in the console, so an unexplained one reads as a diff --git a/tests/test_self_target_refusal_audit.py b/tests/test_self_target_refusal_audit.py index 8c41395e6..02a9ba70a 100644 --- a/tests/test_self_target_refusal_audit.py +++ b/tests/test_self_target_refusal_audit.py @@ -137,3 +137,64 @@ async def test_another_account_writes_no_refusal_row( r = await c.request(method, f"/users/{other.id}{suffix}", json=body, headers=_auth(tok)) assert r.status_code == 200, r.text assert await engine.store.list_audit(action="auth.self_target_refused", limit=10) == [] + + +# --- vault BACKLOG #3260, limb 2: a self-service revoke that ends nothing -------------------------- + +#: A well-formed session id that names no session. +_UNKNOWN_SESSION = "0" * 64 + + +async def test_a_missed_session_revoke_writes_one_row_and_the_same_answer(engine: Engine) -> None: + """RED when: ``DELETE /me/sessions/{id}`` answers its 404 with no audit row of its own (vault + BACKLOG #3260, limb 2; owner ruling 2026-10-16), or the answer or the row tells an unknown id + from another account's. + + One row per miss, with the id cut to 12 characters, and nothing else in its detail.""" + service = await _service(engine) + await _add(service, "a", Role.VIEWER) + await _add(service, "b", Role.VIEWER) + async with _client(engine, service) as c: + ta = (await _login(c, "a")).json()["token"] + tb = (await _login(c, "b")).json()["token"] + foreign = (await c.get("/me/sessions", headers=_auth(tb))).json()["sessions"][0]["id"] + answers = [] + # The grant is single-use, so each attempt takes its own re-proof, as a caller's would. + for target in (_UNKNOWN_SESSION, foreign): + _r, ta = await _reauth(c, ta, purpose="session_terminate") + assert _r.status_code == 200 + r = await c.delete(f"/me/sessions/{target}", headers=_auth(ta)) + answers.append((r.status_code, r.json())) + still = await c.get("/auth/me", headers=_auth(tb)) + assert answers == [(404, {"detail": "no such session"})] * 2, answers + assert still.status_code == 200, "a missed revoke ended another account's session" + rows = await engine.store.list_audit(action="auth.session_revoke_missed", limit=10) + assert len(rows) == 2, "one miss, one row" + # Newest first: the foreign id's row, then the unknown id's. + assert [json.loads(str(r["detail"])) for r in rows] == [ + {"session": foreign[:12]}, + {"session": _UNKNOWN_SESSION[:12]}, + ] + for row in rows: + assert row["actor"] == "a" + assert row["client"] == _DEFAULT_PEER[0], "the miss row dropped the caller's address" + assert await engine.store.list_audit(action="auth.session_revoked", limit=10) == [] + + +async def test_a_revoke_of_your_own_session_writes_no_miss_row(engine: Engine) -> None: + """The control: a revoke that ends a session writes ``auth.session_revoked``, not a miss.""" + service = await _service(engine) + await _add(service, "u", Role.VIEWER) + async with _client(engine, service) as c: + t1 = (await _login(c, "u")).json()["token"] + t2 = (await _login(c, "u")).json()["token"] + _r, t2 = await _reauth(c, t2, purpose="session_terminate") + sessions = (await c.get("/me/sessions", headers=_auth(t2))).json()["sessions"] + other = next(s for s in sessions if not s["current"]) + r = await c.delete(f"/me/sessions/{other['id']}", headers=_auth(t2)) + gone = await c.get("/auth/me", headers=_auth(t1)) + assert r.status_code == 200, r.text + assert gone.status_code == 401 + assert await engine.store.list_audit(action="auth.session_revoke_missed", limit=10) == [] + revoked = await engine.store.list_audit(action="auth.session_revoked", limit=10) + assert len(revoked) == 1 and revoked[0]["client"] == _DEFAULT_PEER[0] diff --git a/tests/test_session_supersession.py b/tests/test_session_supersession.py index 79427ad62..6b8b01b43 100644 --- a/tests/test_session_supersession.py +++ b/tests/test_session_supersession.py @@ -364,13 +364,19 @@ async def test_a_rotation_racing_a_self_revoke_is_not_reported_as_a_revoke( caller = await service.identity_for_token(await _token(service), activity=False) assert caller is not None rotations = rotate_after_first_call(monkeypatch, store, service, target) - revoked = await service.revoke_own_session(caller, target_hash, actor="op") + revoked = await service.revoke_own_session(caller, target_hash, actor="op", client=None) assert rotations, "the injection never fired, so this measured nothing" rotated = rotations[0] assert rotated is not None and await _live(service, rotated), "the race was not driven" assert revoked is False, "revoke_own_session reported a revoke that did not happen" assert await _superseded(store, "self") == [], "an audit row recorded a revoke that missed" + # Vault BACKLOG #3260, limb 2: the miss answers the same 404 as an unknown id, so it writes the + # same miss row, naming the id the caller sent. + missed = await store.list_audit(action="auth.session_revoke_missed") + assert [(row["actor"], json.loads(str(row["detail"]))) for row in missed] == [ + ("op", {"session": target_hash[:12]}) + ], "the race's 404 left no miss row" async def test_a_self_revoke_that_lands_returns_true_and_audits_it(store: MessageStore) -> None: @@ -379,7 +385,10 @@ async def test_a_self_revoke_that_lands_returns_true_and_audits_it(store: Messag target = await _token(service) caller = await service.identity_for_token(await _token(service), activity=False) assert caller is not None - assert await service.revoke_own_session(caller, hash_token(target), actor="op") is True + assert ( + await service.revoke_own_session(caller, hash_token(target), actor="op", client=None) + is True + ) assert not await _live(service, target) assert await _superseded(store, "self") == [ {"scope": "self", "session": hash_token(target)[:12], "actor": "op"}