From 33c5afe050491988ee8224af25b9b06e892f4b50 Mon Sep 17 00:00:00 2001 From: cy Date: Wed, 19 Mar 2025 20:55:14 -0400 Subject: [PATCH] delete pushers created with different access token on password change --- src/api/client/account.rs | 23 ++++++++++++- src/api/client/push.rs | 2 +- src/database/maps.rs | 4 +++ src/service/pusher/mod.rs | 34 +++++++++++++------ .../complement/test_results.jsonl | 4 +-- 5 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/api/client/account.rs b/src/api/client/account.rs index 32438098..5dd622d7 100644 --- a/src/api/client/account.rs +++ b/src/api/client/account.rs @@ -4,7 +4,8 @@ use axum::extract::State; use axum_client_ip::InsecureClientIp; use conduwuit::{ Err, Error, PduBuilder, Result, debug_info, err, error, info, is_equal_to, utils, - utils::ReadyExt, warn, + utils::{ReadyExt, stream::BroadbandExt}, + warn, }; use futures::{FutureExt, StreamExt}; use register::RegistrationKind; @@ -627,6 +628,26 @@ pub(crate) async fn change_password_route( .ready_filter(|id| *id != sender_device) .for_each(|id| services.users.remove_device(sender_user, id)) .await; + + // Remove all pushers except the ones associated with this session + services + .pusher + .get_pushkeys(sender_user) + .map(ToOwned::to_owned) + .broad_filter_map(|pushkey| async move { + services + .pusher + .get_pusher_device(&pushkey) + .await + .ok() + .filter(|pusher_device| pusher_device != sender_device) + .is_some() + .then_some(pushkey) + }) + .for_each(|pushkey| async move { + services.pusher.delete_pusher(sender_user, &pushkey).await; + }) + .await; } info!("User {sender_user} changed their password."); diff --git a/src/api/client/push.rs b/src/api/client/push.rs index 384b9dbc..cc1d3be2 100644 --- a/src/api/client/push.rs +++ b/src/api/client/push.rs @@ -503,7 +503,7 @@ pub(crate) async fn set_pushers_route( services .pusher - .set_pusher(sender_user, &body.action) + .set_pusher(sender_user, body.sender_device(), &body.action) .await?; Ok(set_pusher::v3::Response::new()) diff --git a/src/database/maps.rs b/src/database/maps.rs index 138bb038..1da9acc0 100644 --- a/src/database/maps.rs +++ b/src/database/maps.rs @@ -219,6 +219,10 @@ pub(super) static MAPS: &[Descriptor] = &[ name: "senderkey_pusher", ..descriptor::RANDOM_SMALL }, + Descriptor { + name: "pushkey_deviceid", + ..descriptor::RANDOM_SMALL + }, Descriptor { name: "server_signingkeys", ..descriptor::RANDOM diff --git a/src/service/pusher/mod.rs b/src/service/pusher/mod.rs index 2b269b3d..27490fb8 100644 --- a/src/service/pusher/mod.rs +++ b/src/service/pusher/mod.rs @@ -10,7 +10,7 @@ use database::{Deserialized, Ignore, Interfix, Json, Map}; use futures::{Stream, StreamExt}; use ipaddress::IPAddress; use ruma::{ - RoomId, UInt, UserId, + DeviceId, OwnedDeviceId, RoomId, UInt, UserId, api::{ IncomingResponse, MatrixVersion, OutgoingRequest, SendAccessToken, client::push::{Pusher, PusherKind, set_pusher}, @@ -48,6 +48,7 @@ struct Services { struct Data { senderkey_pusher: Arc, + pushkey_deviceid: Arc, } impl crate::Service for Service { @@ -55,6 +56,7 @@ impl crate::Service for Service { Ok(Arc::new(Self { db: Data { senderkey_pusher: args.db["senderkey_pusher"].clone(), + pushkey_deviceid: args.db["pushkey_deviceid"].clone(), }, services: Services { globals: args.depend::("globals"), @@ -75,6 +77,7 @@ impl Service { pub async fn set_pusher( &self, sender: &UserId, + sender_device: &DeviceId, pusher: &set_pusher::v3::PusherAction, ) -> Result { match pusher { @@ -123,24 +126,35 @@ impl Service { } } - let key = (sender, data.pusher.ids.pushkey.as_str()); + let pushkey = data.pusher.ids.pushkey.as_str(); + let key = (sender, pushkey); self.db.senderkey_pusher.put(key, Json(pusher)); + self.db.pushkey_deviceid.insert(pushkey, sender_device); }, | set_pusher::v3::PusherAction::Delete(ids) => { - let key = (sender, ids.pushkey.as_str()); - self.db.senderkey_pusher.del(key); - - self.services - .sending - .cleanup_events(None, Some(sender), Some(ids.pushkey.as_str())) - .await - .ok(); + self.delete_pusher(sender, ids.pushkey.as_str()).await; }, } Ok(()) } + pub async fn delete_pusher(&self, sender: &UserId, pushkey: &str) { + let key = (sender, pushkey); + self.db.senderkey_pusher.del(key); + self.db.pushkey_deviceid.remove(pushkey); + + self.services + .sending + .cleanup_events(None, Some(sender), Some(pushkey)) + .await + .ok(); + } + + pub async fn get_pusher_device(&self, pushkey: &str) -> Result { + self.db.pushkey_deviceid.get(pushkey).await.deserialized() + } + pub async fn get_pusher(&self, sender: &UserId, pushkey: &str) -> Result { let senderkey = (sender, pushkey); self.db diff --git a/tests/test_results/complement/test_results.jsonl b/tests/test_results/complement/test_results.jsonl index 97170a5c..ac2733f8 100644 --- a/tests/test_results/complement/test_results.jsonl +++ b/tests/test_results/complement/test_results.jsonl @@ -69,8 +69,8 @@ {"Action":"pass","Test":"TestChangePassword/After_changing_password,_can_log_in_with_new_password"} {"Action":"pass","Test":"TestChangePassword/After_changing_password,_different_sessions_can_optionally_be_kept"} {"Action":"pass","Test":"TestChangePassword/After_changing_password,_existing_session_still_works"} -{"Action":"fail","Test":"TestChangePasswordPushers"} -{"Action":"fail","Test":"TestChangePasswordPushers/Pushers_created_with_a_different_access_token_are_deleted_on_password_change"} +{"Action":"pass","Test":"TestChangePasswordPushers"} +{"Action":"pass","Test":"TestChangePasswordPushers/Pushers_created_with_a_different_access_token_are_deleted_on_password_change"} {"Action":"pass","Test":"TestChangePasswordPushers/Pushers_created_with_the_same_access_token_are_not_deleted_on_password_change"} {"Action":"fail","Test":"TestClientSpacesSummary"} {"Action":"fail","Test":"TestClientSpacesSummary/max_depth"}