Sign-out audit rows record the client address, and the no-local-Administrator WARNING keeps every name (BACKLOG #3339, #3282)

changelog.d/3282.fixed.md

@@ -0,0 +1,6 @@
+- **The start WARNING about no usable local Administrator no longer loses its first name to the
+  credential scrub.** It listed the accounts right after "no usable password:". The log's credential
+  scrub read that as a password and masked the first name, from the day the WARNING was added. The
+  list now follows "Accounts that need one:", ahead of the word "password". A name that itself looks
+  like a credential label or a date can still be rewritten by the log filters. The
+  `auth.no_local_administrator` audit row holds the complete list. (`BACKLOG #3282`)

changelog.d/3339.security.md

@@ -0,0 +1,9 @@
+- **A sign-out's `auth.logout` audit row records the client address it came from (ASVS 16.2.1).**
+  Neither sign-out route passed an address. On a first deployment, an investigator could not have
+  told from the audit log where a sign-out came from. `POST /auth/logout` and the console's
+  `/ui/logout` now pass the `client_ip` address, the extractor other audited auth routes use.
+  `AuthService.logout` takes it as a required `client` keyword, so a new caller cannot omit it.
+  The row's actor is unchanged. A console sign-out that ends nothing, including one that loses a
+  rotation race, still records no actor. A sign-out that revokes nothing keeps its own recorded
+  `detail.outcome`, `nothing_to_revoke`. The web console seam digest moved, so a console and an
+  engine built on either side of this change refuse each other at mount. (`BACKLOG #3339`)

messagefoundry/api/_ui_seam.py

@@ -594,12 +594,18 @@
 #: 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.
 #:
+#: BACKLOG #3339: ``AuthService.logout`` gained a required keyword ``client``, the caller's
+#: address, which ``/ui/logout`` now passes so the ``auth.logout`` row records it. The digest moved
+#: from ``5d9c162790077dfc`` because the discovered signature changed. A console on an engine from
+#: the other side of this change would fail its sign-out with a ``TypeError``; the pinned digest
+#: refuses that pair at mount. Re-derived with ``--write``.
+#:
 #: 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 = "5d9c162790077dfc"
+ENGINE_UI_SEAM: str = "1f2d208818510f18"
 
 
 @dataclass(frozen=True, slots=True)

messagefoundry/api/auth_routes.py

@@ -652,7 +652,9 @@ async def logout(
         service: AuthService = Depends(_service),
         identity: Identity = Depends(require()),
     ) -> SimpleMessage:
-        await service.logout(bearer_token(request), actor=identity.username)
+        await service.logout(
+            bearer_token(request), actor=identity.username, client=client_ip(request)
+        )
         return SimpleMessage(detail="logged out")
 
     @app.get("/auth/me", response_model=CurrentUser)

messagefoundry/auth/service.py

@@ -9920,7 +9920,9 @@ async def identity_for_user_id(self, user_id: str) -> Identity | None:
             return None
         return await self._build_identity(user)
 
-    async def logout(self, token: str | None, *, actor: str | None = None) -> LogoutOutcome:
+    async def logout(
+        self, token: str | None, *, client: str | None, actor: str | None = None
+    ) -> LogoutOutcome:
         """End the session ``token`` names, audit ``auth.logout`` saying whether one ended, and
         return that same outcome.
 
@@ -9944,7 +9946,16 @@ async def logout(self, token: str | None, *, actor: str | None = None) -> Logout
         The row's actor is ``actor`` when given (``POST /auth/logout`` passes the signed-in
         caller). Otherwise it is the OWNER of the row the revoke ended, so a console sign-out, which
         has no auth dependency to name a caller, still lands in that user's ``/me/security-events``
-        feed; with nothing ended there is no owner, and the actor stays NULL."""
+        feed; with nothing ended there is no owner, and the actor stays NULL. That includes a
+        console sign-out that loses a rotation race (engine PR 2384, open finding 3). A
+        read before the revoke would name the owner only through :meth:`_load_session`, the one
+        sanctioned session reader, and that read ends and reports a row whose seal fails, so the
+        revoke would then record ``nothing_to_revoke`` for a session this call ended.
+
+        ``client`` is the caller's address, from ``client_ip`` on a request (ADR 0150). It is
+        ``None`` where ``client_ip`` finds no address, so on this row a NULL client does not by
+        itself mean no request was in scope. It has no default, so a new sign-out caller cannot
+        omit it silently (BACKLOG #3339)."""
         if not token:
             return "nothing_to_revoke"
         ended = await self._store.supersede_session(hash_token(token), now=time.time())
@@ -9954,7 +9965,9 @@ async def logout(self, token: str | None, *, actor: str | None = None) -> Logout
             actor = owner.username if owner is not None else None
         # Emit the documented auth.logout event (SECURITY.md, ASVS 16.3.3) even when nothing
         # ended, so the trail still records the sign-out request.
-        await self._audit("auth.logout", actor=actor, detail=_json({"outcome": outcome}))
+        await self._audit(
+            "auth.logout", actor=actor, detail=_json({"outcome": outcome}), client=client
+        )
         return outcome
 
     async def _supersede_session_hash(self, prior_hash: str, *, client: str | None) -> bool:
@@ -11567,11 +11580,15 @@ async def report_administrators_needing_an_outside_service(self) -> tuple[str, .
         names = await self.administrators_needing_an_outside_service()
         if not names:
             return names
+        # The names come right after a neutral label and before the word "password". The shipped
+        # CredentialScrubFilter reads "password: <value>" as a credential, so the old wording, with
+        # the list after "no usable password:", masked the first name (BACKLOG #3282).
         _log.warning(
             "no enabled Administrator can sign in without an outside identity service (the "
-            "directory or a federated identity provider); each is a directory account or a local "
-            "one with no usable password: %s. If that service is down, no Administrator can sign "
-            "in to manage this engine. Create a local Administrator now, before an outage, from "
+            "directory or a federated identity provider). Accounts that need one: %s. Each is a "
+            "directory account, or a local account with no usable password. If that service is "
+            "down, no Administrator can sign in to manage this engine. Create a local "
+            "Administrator now, before an outage, from "
             "the web console. During an outage, `messagefoundry provision-admin`, run at the host "
             "with the engine stopped, is not blocked by these accounts "
             "(docs/SECURITY.md, 'Keep a local Administrator').",

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({"5d9c162790077dfc"})
+SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"1f2d208818510f18"})
 
 #: The vendored static assets shipped in THIS wheel (mounted at /ui/static by :func:`mount_ui`).
 STATIC_DIR = Path(__file__).parent / "static"

messagefoundry_webconsole/routes/core.py

@@ -735,7 +735,7 @@ async def ui_logout(request: Request) -> Response:
         auth = get_auth(request)
         token = session_token(request)
         if auth is not None and token:
-            await auth.logout(token)
+            await auth.logout(token, client=client_ip(request))
         resp = RedirectResponse("/ui/login?e=loggedout", status_code=303)
         # ASVS 14.3.1: revoking server-side and deleting the cookie leaves the browser's CACHED
         # representations of the operator's PHI pages behind, which Back / bfcache can resurrect.

packaging/messagefoundry-webconsole/CHANGELOG.md

@@ -85,6 +85,10 @@ this line.**
 - **Seam only: `/status` names the FHIR lookups that failed to start.** `EngineInfo` gains
   `fhir_lookups_failed` (vault `BACKLOG #3008`). The console does not show it yet, but the field
   is on the seam it pins, so the supported digest moved.
+- **Sign-out records where it came from.** `/ui/logout` passes the caller's `client_ip` address to
+  `AuthService.logout`, so the `auth.logout` audit row records it. Seam change: `logout` takes a
+  required `client` keyword, so the supported digest moved. A console from either side of this
+  change refuses an engine from the other at mount. (`BACKLOG #3339`)
 - **Seam only: the posture reports the deny-by-default refusal count.** `SecurityPosture` gains
   `undeclared_route_refusals` (vault `BACKLOG #2954`). The console does not show it yet, but the
   field is on the seam it pins, so the supported digest moved.

packaging/messagefoundry-webconsole/tests/test_ui_logout_affordance.py

@@ -171,12 +171,27 @@ def test_every_nav_suppressing_page_builder_is_accounted_for() -> None:
 # --- end-to-end: the confined must-change session can actually sign out ---------------------------
 
 
+#: Non-loopback peers, pinned so an assertion on the audited ``client`` compares a value this file
+#: chose rather than a transport default. The sign-in comes from a different address than the
+#: sign-out, so the row cannot pass by recording the address the session was minted from.
+LOGIN_PEER = ("198.51.100.10", 40000)
+PEER_ADDRESS = "198.51.100.23"
+
+
 async def _service(engine: Engine) -> AuthService:
     service = AuthService(engine.store, AuthSettings())
     await service.initialize()
     return service
 
 
+async def _logout_rows(engine: Engine) -> list[tuple[object, object, object]]:
+    """Each ``auth.logout`` row as its actor, client address and ``outcome``."""
+    return [
+        (r["actor"], r["client"], json.loads(str(r["detail"] or "{}")).get("outcome"))
+        for r in await engine.store.list_audit(action="auth.logout")
+    ]
+
+
 async def test_must_change_session_can_see_and_use_sign_out(engine: Engine) -> None:
     """A must_change_password account is confined to /ui/account/password. The page now renders the
     Sign-out form, and POST /ui/logout — which has no Depends gate precisely so this works — really
@@ -194,7 +209,8 @@ async def test_must_change_session_can_see_and_use_sign_out(engine: Engine) -> N
         roles=[Role.OPERATOR.value],
         actor="test",
     )
-    transport = httpx.ASGITransport(app=create_app(engine, auth=service, serve_ui=True))
+    app = create_app(engine, auth=service, serve_ui=True)
+    transport = httpx.ASGITransport(app=app, client=LOGIN_PEER)
     async with httpx.AsyncClient(transport=transport, base_url="http://t") as c:
         login = await c.post("/ui/login", data={"username": "rotate", "password": PW})
         assert login.status_code == 303
@@ -204,13 +220,36 @@ async def test_must_change_session_can_see_and_use_sign_out(engine: Engine) -> N
         confined = await c.get("/ui/account/password")
         assert confined.status_code == 200
         assert LOGOUT_FORM in confined.text
-        out = await c.post("/ui/logout")
+        # BACKLOG #3339: the same session signs out from another address.
+        elsewhere = httpx.ASGITransport(app=app, client=(PEER_ADDRESS, 51234))
+        async with httpx.AsyncClient(transport=elsewhere, base_url="http://t") as away:
+            away.cookies.set("mf_session", str(c.cookies.get("mf_session")))
+            out = await away.post("/ui/logout")
         assert out.status_code == 303
         # the session is really gone: the change-password page now bounces to login
         assert (await c.get("/ui/account/password")).status_code in (302, 303, 401)
     # BACKLOG #3029: the route passes no actor, so the auth.logout row names the owner of the
-    # session it ended rather than carrying a NULL actor.
-    rows = await engine.store.list_audit(action="auth.logout")
-    assert [(r["actor"], json.loads(str(r["detail"]))["outcome"]) for r in rows] == [
-        ("rotate", "revoked")
-    ]
+    # session it ended rather than carrying a NULL actor. BACKLOG #3339: it also records the
+    # address the sign-out came from.
+    assert await _logout_rows(engine) == [("rotate", PEER_ADDRESS, "revoked")]
+
+
+async def test_ui_logout_with_a_cookie_naming_no_session_still_records_the_address(
+    engine: Engine,
+) -> None:
+    """BACKLOG #3339, the control. A cookie that names no session ends nothing, so the row has no
+    owner to name and says ``nothing_to_revoke``; the client address is still recorded. Read beside
+    the test above, it shows the actor there comes from the session, not from the request.
+
+    It pins the CURRENT behaviour, which is still open: an unauthenticated sign-out can append a
+    row for a cookie naming no session (engine PR 2384, open finding 6, held by vault BACKLOG
+    #3749). A fix that writes no row here, or bounds the rows, is expected to change this test."""
+    service = await _service(engine)
+    transport = httpx.ASGITransport(
+        app=create_app(engine, auth=service, serve_ui=True), client=(PEER_ADDRESS, 51234)
+    )
+    async with httpx.AsyncClient(transport=transport, base_url="http://t") as c:
+        c.cookies.set("mf_session", "a-token-that-names-no-session")
+        out = await c.post("/ui/logout")
+        assert out.status_code == 303
+    assert await _logout_rows(engine) == [(None, PEER_ADDRESS, "nothing_to_revoke")]

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
-5d9c162790077dfc
+1f2d208818510f18
 
 ## dataclass messagefoundry.api._ui_seam.UiDeps
 engine_seam
@@ -186,7 +186,7 @@ identity_for_token: (self, token: 'str | None', *, activity: 'bool' = True) -> '
 kerberos_available: property -> 'bool'
 list_sessions: (self, user_id: 'str') -> 'list[SessionRecord]'
 login: (self, username: 'str', password: 'str', *, provider: 'AuthProvider' = <AuthProvider.LOCAL: 'local'>, client: 'str | None' = None, supersedes: 'str | None' = None, totp_code: 'str | None' = None) -> 'LoginOutcome'
-logout: (self, token: 'str | None', *, actor: 'str | None' = None) -> 'LogoutOutcome'
+logout: (self, token: 'str | None', *, client: 'str | None', actor: 'str | None' = None) -> 'LogoutOutcome'
 mfa_satisfied: (self, token: 'str | None') -> 'bool'
 mfa_status: (self, identity: 'Identity') -> 'MfaStatus'
 must_enrol_before_rotating: (self, identity: 'Identity') -> 'bool'

tests/test_api_auth.py

@@ -647,8 +647,10 @@ async def test_a_logout_raced_by_a_rotation_does_not_audit_a_revoke(
     service = await _service(engine)
     await _add(service, "u", Role.VIEWER)
     store = engine.store
-    async with _client(engine, service) as c:
+    # Signed in from another address, so the row's client is the sign-out's own (BACKLOG #3339).
+    async with _client(engine, service, peer=("192.0.2.10", 40000)) as c:
         token = (await _login(c, "u")).json()["token"]
+    async with _client(engine, service) as c:
         rotations = rotate_after_first_call(
             monkeypatch, store, service, token, names=("get_session",)
         )
@@ -658,11 +660,34 @@ async def test_a_logout_raced_by_a_rotation_does_not_audit_a_revoke(
     rotated = rotations[0]
     assert rotated is not None, "the rotation failed closed, so the race was not driven"
     assert await service.identity_for_token(rotated, activity=False) is not None
-    outcomes = [
-        json.loads(str(row["detail"] or "{}")).get("outcome")
+    # BACKLOG #3339: a sign-out that ended nothing still names the caller and the address.
+    assert await _logout_rows(store) == [("u", _DEFAULT_PEER[0], "nothing_to_revoke")], (
+        "the auth.logout row claimed a revoke that missed, or lost its actor or address"
+    )
+
+
+async def _logout_rows(store: Any) -> list[tuple[object, object, object]]:
+    """Each ``auth.logout`` row as its actor, client address and ``detail.outcome``."""
+    return [
+        (row["actor"], row["client"], json.loads(str(row["detail"] or "{}")).get("outcome"))
         for row in await store.list_audit(action="auth.logout")
     ]
-    assert outcomes == ["nothing_to_revoke"], "the auth.logout row claimed a revoke that missed"
+
+
+async def test_a_logout_audit_row_names_the_caller_and_the_client_address(engine: Engine) -> None:
+    """BACKLOG #3339. ``POST /auth/logout`` passes the signed-in caller and the address from
+    ``client_ip``, and the ``auth.logout`` row records both. The peer is pinned, so the ``client``
+    assertion compares a value this test named (BACKLOG #1644). The sign-in comes from another
+    address, so the row cannot pass by recording the address the session was minted from. The
+    route's control, a token that names no session, is the rotation race test above: its dependency
+    refuses any other dead token."""
+    service = await _service(engine)
+    await _add(service, "u", Role.VIEWER)
+    async with _client(engine, service, peer=("192.0.2.10", 40000)) as c:
+        token = (await _login(c, "u")).json()["token"]
+    async with _client(engine, service, peer=("192.0.2.44", 40001)) as c:
+        assert (await c.post("/auth/logout", headers=_auth(token))).status_code == 200
+    assert await _logout_rows(engine.store) == [("u", "192.0.2.44", "revoked")]
 
 
 async def test_cluster_endpoints_gated_by_monitoring_read(engine: Engine) -> None:

tests/test_asvs_phase0.py

@@ -158,7 +158,7 @@ async def test_logout_emits_audit_event() -> None:
     try:
         service = AuthService(store, AuthSettings())
         identity, token, _ = await login_admin(service)
-        await service.logout(token, actor=identity.username)
+        await service.logout(token, actor=identity.username, client=None)
         actions = [row["action"] for row in await store.list_audit()]
         assert "auth.logout" in actions
     finally:

tests/test_auth_before_body.py

@@ -879,7 +879,9 @@ async def test_a_session_revoked_while_the_body_arrives_is_refused_by_the_gate(
     # The early step passes, the session ends, and then the gate runs.
     request = _request(app, bearer)
     await found.step(request)
-    await service.logout(bearer["Authorization"].removeprefix("Bearer "), actor="admin")
+    await service.logout(
+        bearer["Authorization"].removeprefix("Bearer "), actor="admin", client=None
+    )
     for gate in gates:
         with pytest.raises(HTTPException) as refused:
             await gate(request)