From a35fe1672ec3623a21f8e6e92043d0365c505384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADcio=20Bracht?= Date: Fri, 22 May 2026 17:52:54 -0700 Subject: [PATCH 1/4] invalidate other sessions on password change --- CHANGELOG.md | 7 ++ Cargo.lock | 2 +- crates/mqdb-agent/Cargo.toml | 2 +- crates/mqdb-agent/src/http/handlers.rs | 24 +++++- crates/mqdb-agent/src/http/session_store.rs | 82 +++++++++++++++++++++ 5 files changed, 114 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38a4da6..528b7e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. Each entry lists the date and the crate versions that were released. +## 2026-05-23 — mqdb-agent 0.8.2 + +### Fixed + +- Password changes left every other HTTP session for the same user valid until the 24h TTL expired (`SessionStore` only exposed `destroy(session_id)`, so `POST /auth/password/change` updated `_credentials` and returned 200 with the attacker's session still live). Added `SessionStore::destroy_others_by_canonical_id(canonical_id, keep_session_id)` returning the JWTs of removed sessions so the caller can revoke their JTIs. `handle_password_change` now keeps the caller's session and destroys the rest; `handle_password_reset_submit` destroys all sessions for the target canonical_id (the user has no live session at reset time). Each destroyed session's JWT is decoded with `verify_jwt_ignore_expiry` and its `jti` added to `JtiRevocationStore`, matching the pattern already used by `handle_logout` so reissued MQTT tickets bound to those JWTs are rejected. The MQTT password-change path (`$DB/_auth/password/change`) intentionally still does not invalidate HTTP sessions — `AdminContext` has no reference to `SessionStore`; tracked as follow-up to issue #37. +- Test coverage: 3 unit tests for `destroy_others_by_canonical_id` covering keep-current-session, destroy-all (`None` keep), and unknown-user (empty result). + ## 2026-05-22 — mqdb-agent 0.8.1, mqdb-cluster 0.3.6, mqdb-cli 0.7.7 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index 7c07747..f10b63e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1349,7 +1349,7 @@ dependencies = [ [[package]] name = "mqdb-agent" -version = "0.8.1" +version = "0.8.2" dependencies = [ "arc-swap", "argon2", diff --git a/crates/mqdb-agent/Cargo.toml b/crates/mqdb-agent/Cargo.toml index 38bd3a5..fdec550 100644 --- a/crates/mqdb-agent/Cargo.toml +++ b/crates/mqdb-agent/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mqdb-agent" -version = "0.8.1" +version = "0.8.2" edition.workspace = true license = "Apache-2.0" authors.workspace = true diff --git a/crates/mqdb-agent/src/http/handlers.rs b/crates/mqdb-agent/src/http/handlers.rs index a29a1fa..195a307 100644 --- a/crates/mqdb-agent/src/http/handlers.rs +++ b/crates/mqdb-agent/src/http/handlers.rs @@ -2298,6 +2298,17 @@ pub async fn handle_verify_status(state: &ServerState, headers: &HeaderMap) -> H ) } +fn revoke_jwt_jtis(state: &ServerState, jwts: &[String]) { + for jwt in jwts { + if let Some(payload) = verify_jwt_ignore_expiry(jwt, &state.jwt_config) + && let Some(jti) = payload.get("jti").and_then(|v| v.as_str()) + { + state.jti_revocation.revoke(jti); + } + } +} + +#[allow(clippy::too_many_lines)] pub async fn handle_password_change( state: &ServerState, headers: &HeaderMap, @@ -2309,12 +2320,13 @@ pub async fn handle_password_change( return json_response_with_credentials(404, &json!({"error": "not found"}), cors); } - let (_, session) = match require_session(state, headers) { + let (current_session_id, session) = match require_session(state, headers) { Ok(r) => r, Err(resp) => return *resp, }; let canonical_id = session.canonical_id.clone(); + let current_session_id = current_session_id.to_string(); let body_value: serde_json::Value = match serde_json::from_slice(body) { Ok(v) => v, @@ -2415,6 +2427,11 @@ pub async fn handle_password_change( ); } + let revoked_jwts = state + .session_store + .destroy_others_by_canonical_id(&canonical_id, Some(¤t_session_id)); + revoke_jwt_jtis(state, &revoked_jwts); + json_response_with_credentials(200, &json!({"status": "password changed"}), cors) } @@ -2763,6 +2780,11 @@ pub async fn handle_password_reset_submit( ) .await; + let revoked_jwts = state + .session_store + .destroy_others_by_canonical_id(canonical_id, None); + revoke_jwt_jtis(state, &revoked_jwts); + json_response_with_credentials(200, &json!({"status": "password_reset"}), cors) } diff --git a/crates/mqdb-agent/src/http/session_store.rs b/crates/mqdb-agent/src/http/session_store.rs index ab065eb..5601225 100644 --- a/crates/mqdb-agent/src/http/session_store.rs +++ b/crates/mqdb-agent/src/http/session_store.rs @@ -103,6 +103,31 @@ impl SessionStore { sessions.remove(session_id).is_some() } + /// Destroys every session whose `canonical_id` matches, except `keep_session_id` + /// (pass `None` to destroy all). Returns the JWTs of destroyed sessions so the + /// caller can decode and revoke their JTIs. + pub fn destroy_others_by_canonical_id( + &self, + canonical_id: &str, + keep_session_id: Option<&str>, + ) -> Vec { + let Ok(mut sessions) = self.sessions.write() else { + return Vec::new(); + }; + let mut removed_jwts = Vec::new(); + sessions.retain(|sid, session| { + let same_user = session.canonical_id == canonical_id; + let is_kept = keep_session_id.is_some_and(|keep| sid.as_str() == keep); + if same_user && !is_kept { + removed_jwts.push(session.jwt.clone()); + false + } else { + true + } + }); + removed_jwts + } + pub fn set_vault_unlocked(&self, session_id: &str, unlocked: bool) -> bool { let Ok(mut sessions) = self.sessions.write() else { return false; @@ -278,4 +303,61 @@ mod tests { let store = SessionStore::new(); assert!(store.get("nonexistent").is_none()); } + + fn make_session(store: &SessionStore, canonical_id: &str, jwt: &str) -> String { + store + .create(NewSession { + jwt: jwt.into(), + canonical_id: canonical_id.into(), + provider: "email".into(), + provider_sub: canonical_id.into(), + email: None, + name: None, + picture: None, + }) + .expect("create should succeed") + } + + #[test] + fn destroy_others_keeps_target_session_and_returns_other_jwts() { + let store = SessionStore::new(); + let keep = make_session(&store, "user-a", "jwt-keep"); + let other_a = make_session(&store, "user-a", "jwt-a1"); + let other_b = make_session(&store, "user-a", "jwt-a2"); + let untouched = make_session(&store, "user-b", "jwt-b"); + + let removed = store.destroy_others_by_canonical_id("user-a", Some(&keep)); + assert_eq!(removed.len(), 2); + assert!(removed.contains(&"jwt-a1".to_string())); + assert!(removed.contains(&"jwt-a2".to_string())); + + assert!(store.get(&keep).is_some()); + assert!(store.get(&other_a).is_none()); + assert!(store.get(&other_b).is_none()); + assert!(store.get(&untouched).is_some()); + } + + #[test] + fn destroy_others_with_none_destroys_all_sessions_for_user() { + let store = SessionStore::new(); + let a1 = make_session(&store, "user-a", "jwt-a1"); + let a2 = make_session(&store, "user-a", "jwt-a2"); + let b = make_session(&store, "user-b", "jwt-b"); + + let removed = store.destroy_others_by_canonical_id("user-a", None); + assert_eq!(removed.len(), 2); + + assert!(store.get(&a1).is_none()); + assert!(store.get(&a2).is_none()); + assert!(store.get(&b).is_some()); + } + + #[test] + fn destroy_others_with_unknown_user_returns_empty() { + let store = SessionStore::new(); + make_session(&store, "user-a", "jwt-a"); + + let removed = store.destroy_others_by_canonical_id("user-x", None); + assert!(removed.is_empty()); + } } From 24315e92c2455a021c6a35fdc1ae9088559ea27d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADcio=20Bracht?= Date: Fri, 22 May 2026 17:57:12 -0700 Subject: [PATCH 2/4] extract verify_stored_password helper from handle_password_change --- crates/mqdb-agent/src/http/handlers.rs | 86 +++++++++++++++----------- 1 file changed, 49 insertions(+), 37 deletions(-) diff --git a/crates/mqdb-agent/src/http/handlers.rs b/crates/mqdb-agent/src/http/handlers.rs index 195a307..219ef29 100644 --- a/crates/mqdb-agent/src/http/handlers.rs +++ b/crates/mqdb-agent/src/http/handlers.rs @@ -2308,7 +2308,53 @@ fn revoke_jwt_jtis(state: &ServerState, jwts: &[String]) { } } -#[allow(clippy::too_many_lines)] +async fn verify_stored_password( + state: &ServerState, + canonical_id: &str, + current_password: &str, + cors: Option<&str>, +) -> Result<(), HttpResponse> { + let identity = read_entity(&state.mqtt_client, "_identities", canonical_id).await; + let email_verified = identity + .as_ref() + .and_then(|i| i.get("email_verified")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + if !email_verified { + return Err(json_response_with_credentials( + 403, + &json!({"error": "email must be verified before changing password"}), + cors, + )); + } + + let Some(cred) = read_entity(&state.mqtt_client, "_credentials", canonical_id).await else { + return Err(json_response_with_credentials( + 404, + &json!({"error": "no credentials found (OAuth-only account)"}), + cors, + )); + }; + + let Some(stored_hash) = cred.get("password_hash").and_then(|v| v.as_str()) else { + return Err(json_response_with_credentials( + 500, + &json!({"error": "credential record is corrupt"}), + cors, + )); + }; + + if !credentials::verify_password(stored_hash, current_password) { + return Err(json_response_with_credentials( + 401, + &json!({"error": "incorrect current password"}), + cors, + )); + } + + Ok(()) +} + pub async fn handle_password_change( state: &ServerState, headers: &HeaderMap, @@ -2366,42 +2412,8 @@ pub async fn handle_password_change( ); } - let identity = read_entity(&state.mqtt_client, "_identities", &canonical_id).await; - let email_verified = identity - .as_ref() - .and_then(|i| i.get("email_verified")) - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - if !email_verified { - return json_response_with_credentials( - 403, - &json!({"error": "email must be verified before changing password"}), - cors, - ); - } - - let Some(cred) = read_entity(&state.mqtt_client, "_credentials", &canonical_id).await else { - return json_response_with_credentials( - 404, - &json!({"error": "no credentials found (OAuth-only account)"}), - cors, - ); - }; - - let Some(stored_hash) = cred.get("password_hash").and_then(|v| v.as_str()) else { - return json_response_with_credentials( - 500, - &json!({"error": "credential record is corrupt"}), - cors, - ); - }; - - if !credentials::verify_password(stored_hash, current_password) { - return json_response_with_credentials( - 401, - &json!({"error": "incorrect current password"}), - cors, - ); + if let Err(resp) = verify_stored_password(state, &canonical_id, current_password, cors).await { + return resp; } let new_hash = match credentials::hash_password(new_password) { From 0076b72ca14626ef23bf4080700cbf3a7987983f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADcio=20Bracht?= Date: Fri, 22 May 2026 18:09:33 -0700 Subject: [PATCH 3/4] drop unneeded to_string clone in handle_password_change --- crates/mqdb-agent/src/http/handlers.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/mqdb-agent/src/http/handlers.rs b/crates/mqdb-agent/src/http/handlers.rs index 219ef29..5c38250 100644 --- a/crates/mqdb-agent/src/http/handlers.rs +++ b/crates/mqdb-agent/src/http/handlers.rs @@ -2372,7 +2372,6 @@ pub async fn handle_password_change( }; let canonical_id = session.canonical_id.clone(); - let current_session_id = current_session_id.to_string(); let body_value: serde_json::Value = match serde_json::from_slice(body) { Ok(v) => v, @@ -2441,7 +2440,7 @@ pub async fn handle_password_change( let revoked_jwts = state .session_store - .destroy_others_by_canonical_id(&canonical_id, Some(¤t_session_id)); + .destroy_others_by_canonical_id(&canonical_id, Some(current_session_id)); revoke_jwt_jtis(state, &revoked_jwts); json_response_with_credentials(200, &json!({"status": "password changed"}), cors) From eaafd087a6a12cb60f35c0649d15052f8fa1d226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADcio=20Bracht?= Date: Sat, 23 May 2026 11:04:47 -0700 Subject: [PATCH 4/4] log silent failures along password-change session invalidation --- CHANGELOG.md | 1 + README.md | 4 +++- crates/mqdb-agent/src/http/handlers.rs | 25 +++++++++++++-------- crates/mqdb-agent/src/http/session_store.rs | 5 +++++ docs/testing/13-http-api.md | 9 ++++++-- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 528b7e5..1e1cc93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Each entry lists the date and the crate versions that were released. - Password changes left every other HTTP session for the same user valid until the 24h TTL expired (`SessionStore` only exposed `destroy(session_id)`, so `POST /auth/password/change` updated `_credentials` and returned 200 with the attacker's session still live). Added `SessionStore::destroy_others_by_canonical_id(canonical_id, keep_session_id)` returning the JWTs of removed sessions so the caller can revoke their JTIs. `handle_password_change` now keeps the caller's session and destroys the rest; `handle_password_reset_submit` destroys all sessions for the target canonical_id (the user has no live session at reset time). Each destroyed session's JWT is decoded with `verify_jwt_ignore_expiry` and its `jti` added to `JtiRevocationStore`, matching the pattern already used by `handle_logout` so reissued MQTT tickets bound to those JWTs are rejected. The MQTT password-change path (`$DB/_auth/password/change`) intentionally still does not invalidate HTTP sessions — `AdminContext` has no reference to `SessionStore`; tracked as follow-up to issue #37. - Test coverage: 3 unit tests for `destroy_others_by_canonical_id` covering keep-current-session, destroy-all (`None` keep), and unknown-user (empty result). +- Observability for security-sensitive silent failures along the new path: `destroy_others_by_canonical_id` now `warn!`s on a poisoned `SessionStore` lock (was a silent empty return); `revoke_jwt_jtis` `warn!`s when a destroyed session's JWT fails to decode or is missing the `jti` claim (was a silent skip); `handle_password_reset_submit` now `error!`s and returns 500 when the verified challenge record is missing `canonical_id` (was `.unwrap_or("")` → silent no-op on `destroy_others_by_canonical_id`). ## 2026-05-22 — mqdb-agent 0.8.1, mqdb-cluster 0.3.6, mqdb-cli 0.7.7 diff --git a/README.md b/README.md index a463b6b..b36749e 100644 --- a/README.md +++ b/README.md @@ -521,9 +521,11 @@ When `--http-bind` is set, the following HTTP endpoints are available: All auth endpoints use cookie-based sessions. The `/auth/ticket` endpoint exchanges a valid session for a JWT that can be used to authenticate MQTT connections. +A successful `POST /auth/password/change` keeps the caller's session and destroys every other HTTP session for the same user; their JWTs' `jti` claims are added to the revocation store so any reissued MQTT ticket is rejected. `POST /auth/password/reset/submit` does the same for every session of the target user (no caller session at reset time). + ### Password Change & Reset MQTT API -Password change and reset are also available over MQTT 5.0 request-response for JWT-authenticated users: +Password change and reset are also available over MQTT 5.0 request-response for JWT-authenticated users. The MQTT path currently updates `_credentials` but does not invalidate HTTP sessions for the same user — tracked in issue #69. | Topic | Payload | Description | |-------|---------|-------------| diff --git a/crates/mqdb-agent/src/http/handlers.rs b/crates/mqdb-agent/src/http/handlers.rs index 5c38250..6fa99eb 100644 --- a/crates/mqdb-agent/src/http/handlers.rs +++ b/crates/mqdb-agent/src/http/handlers.rs @@ -2300,11 +2300,15 @@ pub async fn handle_verify_status(state: &ServerState, headers: &HeaderMap) -> H fn revoke_jwt_jtis(state: &ServerState, jwts: &[String]) { for jwt in jwts { - if let Some(payload) = verify_jwt_ignore_expiry(jwt, &state.jwt_config) - && let Some(jti) = payload.get("jti").and_then(|v| v.as_str()) - { - state.jti_revocation.revoke(jti); - } + let Some(payload) = verify_jwt_ignore_expiry(jwt, &state.jwt_config) else { + warn!("failed to decode destroyed session JWT; JTI not revoked"); + continue; + }; + let Some(jti) = payload.get("jti").and_then(|v| v.as_str()) else { + warn!("destroyed session JWT missing jti claim; not revoked"); + continue; + }; + state.jti_revocation.revoke(jti); } } @@ -2743,10 +2747,13 @@ pub async fn handle_password_reset_submit( ) .await; - let canonical_id = challenge - .get("canonical_id") - .and_then(|v| v.as_str()) - .unwrap_or(""); + let Some(canonical_id) = challenge.get("canonical_id").and_then(|v| v.as_str()) else { + error!( + challenge_id, + "password reset challenge missing canonical_id" + ); + return json_response_with_credentials(500, &json!({"error": "internal error"}), cors); + }; let new_hash = match credentials::hash_password(new_password) { Ok(h) => h, diff --git a/crates/mqdb-agent/src/http/session_store.rs b/crates/mqdb-agent/src/http/session_store.rs index 5601225..d8cd9fa 100644 --- a/crates/mqdb-agent/src/http/session_store.rs +++ b/crates/mqdb-agent/src/http/session_store.rs @@ -5,6 +5,7 @@ use ring::rand::{SecureRandom, SystemRandom}; use std::collections::HashMap; use std::sync::RwLock; use std::time::{Duration, SystemTime}; +use tracing::warn; const SESSION_ID_BYTES: usize = 32; const SESSION_TTL_SECS: u64 = 86400; @@ -112,6 +113,10 @@ impl SessionStore { keep_session_id: Option<&str>, ) -> Vec { let Ok(mut sessions) = self.sessions.write() else { + warn!( + canonical_id, + "session store lock poisoned; password-change session invalidation skipped" + ); return Vec::new(); }; let mut removed_jwts = Vec::new(); diff --git a/docs/testing/13-http-api.md b/docs/testing/13-http-api.md index f4da0f9..39813a7 100644 --- a/docs/testing/13-http-api.md +++ b/docs/testing/13-http-api.md @@ -380,7 +380,10 @@ Expected: successful login with session cookie. - [ ] Rate limited after 5 attempts per minute - [ ] Login works with new password after change - [ ] Login fails with old password after change -- [ ] MQTT `$DB/_auth/password/change` works with same logic +- [ ] Caller's session cookie still works after the change (other sessions for the same user are destroyed, caller's is kept) +- [ ] A second HTTP session for the same user logged in before the change returns 401 on any authenticated endpoint after the change +- [ ] An MQTT ticket bound to a destroyed session's JWT is rejected (`jti` is in `JtiRevocationStore`) +- [ ] MQTT `$DB/_auth/password/change` works with same logic (note: MQTT path does NOT yet invalidate HTTP sessions — tracked in issue #69) - [ ] Cluster mode rejects with "auth endpoints not supported in cluster mode" --- @@ -518,9 +521,11 @@ MQTT: shares the vault unlock rate limiter. - [ ] `email_verified` set to `true` after successful reset - [ ] Login works with new password after reset - [ ] Login fails with old password after reset +- [ ] Any HTTP session for the target user that existed before the reset returns 401 on any authenticated endpoint after the reset +- [ ] An MQTT ticket bound to a destroyed session's JWT is rejected after the reset - [ ] Rate limited after 3 attempts per minute (HTTP) - [ ] MQTT `$DB/_auth/password/reset/start` works with same logic -- [ ] MQTT `$DB/_auth/password/reset/submit` works with same logic +- [ ] MQTT `$DB/_auth/password/reset/submit` works with same logic (note: MQTT path does NOT yet invalidate HTTP sessions — tracked in issue #69) - [ ] MQTT handler verifies email matches authenticated user's identity - [ ] Cluster mode rejects with "auth endpoints not supported in cluster mode"