From 2af8ee49ba3ea5d6b5686165994f7a1da4148249 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Thu, 11 Jun 2026 20:35:02 +0200 Subject: [PATCH] fix(audit-2026-06-11/H-E1): password change invalidates sessions + API keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PATCH /api/v1/admin/admins/{id} handler's password branch updated the password hash but explicitly preserved every live session and API key for the target user. That made password rotation a non-event for credential compromise: a hijacked session that triggered a password change kept its grip after the rotation, and a leaked API key survived its owner's "I'm rotating because I think I'm compromised" reaction. Now the password branch mirrors the deactivation branch: 1. update the password hash (unchanged), 2. `state.sessions.delete_for_user(id)` — wipes every live session including the caller's, which logs them out and forces re-login under the new password, 3. `state.keys.expire_all_for_user(id)` — revokes every API key. This matches the `cmd_reset_password` CLI flow already in the codebase. The dashboard's 401 handler redirects to the login screen, so the UX on self-change is "you're logged out; sign back in". Audit ref: security_audit/01_authn_session.md (H-E1). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/admin_users_api.rs | 26 +++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/crates/manager-core/src/admin_users_api.rs b/crates/manager-core/src/admin_users_api.rs index d86bd2e..2c8d51d 100644 --- a/crates/manager-core/src/admin_users_api.rs +++ b/crates/manager-core/src/admin_users_api.rs @@ -233,11 +233,27 @@ async fn patch_admin( validate_password(new_password)?; let hash = hash_password(new_password).map_err(|e| AdminApiError::Hash(e.to_string()))?; latest = Some(state.users.update_password_hash(id, &hash).await?); - // Best practice: rotating your own password should still keep - // your session alive, so we don't wipe sessions here. (If we - // wanted "log everyone else out on password change", that'd be - // a `delete_for_user` + re-issue current session. Out of scope - // for the initial cut.) + // Audit 2026-06-11 H-E1 — a password change invalidates both + // credential surfaces for the target user (sessions + API keys), + // matching the CLI `reset-password` flow and the deactivation + // path below. A self-change therefore logs the caller out, and + // the dashboard handles the subsequent 401 by redirecting to the + // login screen. The previous behavior kept all sessions live, + // which meant a hijacked session that triggered a password + // change retained its grip after the rotation. + if let Err(err) = state.sessions.delete_for_user(id).await { + tracing::error!(?err, "failed to delete sessions on password change"); + } + match state.keys.expire_all_for_user(id).await { + Ok(n) => { + if n > 0 { + tracing::info!(user_id = %id, expired = n, "expired api keys on password change"); + } + } + Err(err) => { + tracing::error!(?err, "failed to expire api keys on password change"); + } + } } if let Some(email_patch) = input.email.as_ref() {