From f62a7a66c8768204e82e0efa8b04108f3a544ce9 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Sun, 29 Mar 2026 22:43:36 +0200 Subject: [PATCH] Rotate refresh-tokens on sstamp reset (#7031) When a security-stamp gets reset/rotated we should also rotate all device refresh-tokens to invalidate them. Else clients are still able to use old refresh tokens. Signed-off-by: BlackDex --- src/api/admin.rs | 4 ++-- src/api/core/accounts.rs | 24 ++++++++++++++++-------- src/api/core/emergency_access.rs | 2 +- src/api/core/organizations.rs | 3 ++- src/db/models/device.rs | 18 +++++++++++++++++- src/db/models/user.rs | 12 ++++++++---- 6 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/api/admin.rs b/src/api/admin.rs index 6cedc040..a4f53e0e 100644 --- a/src/api/admin.rs +++ b/src/api/admin.rs @@ -472,7 +472,7 @@ async fn deauth_user(user_id: UserId, _token: AdminToken, conn: DbConn, nt: Noti } Device::delete_all_by_user(&user.uuid, &conn).await?; - user.reset_security_stamp(); + user.reset_security_stamp(&conn).await?; user.save(&conn).await } @@ -481,7 +481,7 @@ async fn deauth_user(user_id: UserId, _token: AdminToken, conn: DbConn, nt: Noti async fn disable_user(user_id: UserId, _token: AdminToken, conn: DbConn, nt: Notify<'_>) -> EmptyResult { let mut user = get_user_or_404(&user_id, &conn).await?; Device::delete_all_by_user(&user.uuid, &conn).await?; - user.reset_security_stamp(); + user.reset_security_stamp(&conn).await?; user.enabled = false; let save_result = user.save(&conn).await; diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index d91eb4cd..e0869c63 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -296,7 +296,7 @@ pub async fn _register(data: Json, email_verification: bool, conn: set_kdf_data(&mut user, &data.kdf)?; - user.set_password(&data.master_password_hash, Some(data.key), true, None); + user.set_password(&data.master_password_hash, Some(data.key), true, None, &conn).await?; user.password_hint = password_hint; // Add extra fields if present @@ -364,7 +364,9 @@ async fn post_set_password(data: Json, headers: Headers, conn: Some(data.key), false, Some(vec![String::from("revision_date")]), // We need to allow revision-date to use the old security_timestamp - ); + &conn, + ) + .await?; user.password_hint = password_hint; if let Some(keys) = data.keys { @@ -532,7 +534,9 @@ async fn post_password(data: Json, headers: Headers, conn: DbCon String::from("get_public_keys"), String::from("get_api_webauthn"), ]), - ); + &conn, + ) + .await?; let save_result = user.save(&conn).await; @@ -633,7 +637,9 @@ async fn post_kdf(data: Json, headers: Headers, conn: DbConn, nt: Some(data.unlock_data.master_key_wrapped_user_key), true, None, - ); + &conn, + ) + .await?; let save_result = user.save(&conn).await; nt.send_logout(&user, Some(headers.device.uuid.clone()), &conn).await; @@ -900,7 +906,9 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: Some(data.account_unlock_data.master_password_unlock_data.master_key_encrypted_user_key), true, None, - ); + &conn, + ) + .await?; let save_result = user.save(&conn).await; @@ -920,7 +928,7 @@ async fn post_sstamp(data: Json, headers: Headers, conn: DbCo data.validate(&user, true, &conn).await?; Device::delete_all_by_user(&user.uuid, &conn).await?; - user.reset_security_stamp(); + user.reset_security_stamp(&conn).await?; let save_result = user.save(&conn).await; nt.send_logout(&user, None, &conn).await; @@ -1042,7 +1050,7 @@ async fn post_email(data: Json, headers: Headers, conn: DbConn, user.email_new = None; user.email_new_token = None; - user.set_password(&data.new_master_password_hash, Some(data.key), true, None); + user.set_password(&data.new_master_password_hash, Some(data.key), true, None, &conn).await?; let save_result = user.save(&conn).await; @@ -1254,7 +1262,7 @@ struct SecretVerificationRequest { pub async fn kdf_upgrade(user: &mut User, pwd_hash: &str, conn: &DbConn) -> ApiResult<()> { if user.password_iterations < CONFIG.password_iterations() { user.password_iterations = CONFIG.password_iterations(); - user.set_password(pwd_hash, None, false, None); + user.set_password(pwd_hash, None, false, None, conn).await?; if let Err(e) = user.save(conn).await { error!("Error updating user: {e:#?}"); diff --git a/src/api/core/emergency_access.rs b/src/api/core/emergency_access.rs index 1897f995..29a15c8d 100644 --- a/src/api/core/emergency_access.rs +++ b/src/api/core/emergency_access.rs @@ -653,7 +653,7 @@ async fn password_emergency_access( }; // change grantor_user password - grantor_user.set_password(new_master_password_hash, Some(data.key), true, None); + grantor_user.set_password(new_master_password_hash, Some(data.key), true, None, &conn).await?; grantor_user.save(&conn).await?; // Disable TwoFactor providers since they will otherwise block logins diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 0213a006..0beb7036 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -2858,7 +2858,8 @@ async fn put_reset_password( let reset_request = data.into_inner(); let mut user = user; - user.set_password(reset_request.new_master_password_hash.as_str(), Some(reset_request.key), true, None); + user.set_password(reset_request.new_master_password_hash.as_str(), Some(reset_request.key), true, None, &conn) + .await?; user.save(&conn).await?; nt.send_logout(&user, None, &conn).await; diff --git a/src/db/models/device.rs b/src/db/models/device.rs index d3c32213..1026574c 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -49,11 +49,16 @@ impl Device { push_uuid: Some(PushId(get_uuid())), push_token: None, - refresh_token: crypto::encode_random_bytes::<64>(&BASE64URL), + refresh_token: Device::generate_refresh_token(), twofactor_remember: None, } } + #[inline(always)] + pub fn generate_refresh_token() -> String { + crypto::encode_random_bytes::<64>(&BASE64URL) + } + pub fn to_json(&self) -> Value { json!({ "id": self.uuid, @@ -260,6 +265,17 @@ impl Device { .unwrap_or(0) != 0 }} } + + pub async fn rotate_refresh_tokens_by_user(user_uuid: &UserId, conn: &DbConn) -> EmptyResult { + // Generate a new token per device. + // We cannot do a single UPDATE with one value because each device needs a unique token. + let devices = Self::find_by_user(user_uuid, conn).await; + for mut device in devices { + device.refresh_token = Device::generate_refresh_token(); + device.save(false, conn).await?; + } + Ok(()) + } } #[derive(Display)] diff --git a/src/db/models/user.rs b/src/db/models/user.rs index e88c7296..ebc72101 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -185,13 +185,14 @@ impl User { /// These routes are able to use the previous stamp id for the next 2 minutes. /// After these 2 minutes this stamp will expire. /// - pub fn set_password( + pub async fn set_password( &mut self, password: &str, new_key: Option, reset_security_stamp: bool, allow_next_route: Option>, - ) { + conn: &DbConn, + ) -> EmptyResult { self.password_hash = crypto::hash_password(password.as_bytes(), &self.salt, self.password_iterations as u32); if let Some(route) = allow_next_route { @@ -203,12 +204,15 @@ impl User { } if reset_security_stamp { - self.reset_security_stamp() + self.reset_security_stamp(conn).await?; } + Ok(()) } - pub fn reset_security_stamp(&mut self) { + pub async fn reset_security_stamp(&mut self, conn: &DbConn) -> EmptyResult { self.security_stamp = get_uuid(); + Device::rotate_refresh_tokens_by_user(&self.uuid, conn).await?; + Ok(()) } /// Set the stamp_exception to only allow a subsequent request matching a specific route using the current security-stamp.