From a29d2e39c4b677407719d28a564f843ab2ebd0cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 24 Dec 2020 16:35:32 +0100 Subject: [PATCH] base: Save profiles independently from membership events. The sender controls the content of the membership event, since the content contains profile data (display names, avatar urls) a sender might incorrectly change the profile of another member inside the room. This is allowed in the case where the sender is kicking or inviting the member, this it will self heal once the member re-joins. Still, to mitigate this a bit we're storing the profile data when we know that the member sent out the content on their own. --- matrix_sdk_base/examples/state_store.rs | 40 +++++++++++++++++++++++-- matrix_sdk_base/src/client.rs | 31 +++++++++++++++++-- matrix_sdk_base/src/rooms/members.rs | 11 +++++-- matrix_sdk_base/src/rooms/normal.rs | 8 +++-- matrix_sdk_base/src/store.rs | 31 +++++++++++++++++-- 5 files changed, 109 insertions(+), 12 deletions(-) diff --git a/matrix_sdk_base/examples/state_store.rs b/matrix_sdk_base/examples/state_store.rs index 731fc62e..0b785e73 100644 --- a/matrix_sdk_base/examples/state_store.rs +++ b/matrix_sdk_base/examples/state_store.rs @@ -114,7 +114,11 @@ impl Completer for InspectorHelper { let args: Vec<&str> = line.split_ascii_whitespace().collect(); let commands = vec![ - ("get-state", "get a state event in the give room"), + ("get-state", "get a state event in the given room"), + ( + "get-profiles", + "get all the stored profiles in the given room", + ), ("list-rooms", "list all rooms"), ( "get-members", @@ -131,7 +135,9 @@ impl Completer for InspectorHelper { if args.is_empty() { Ok((pos, commands)) } else if args.len() == 1 { - if (args[0] == "get-state" || args[0] == "get-members") && line.ends_with(' ') { + if (args[0] == "get-state" || args[0] == "get-members" || args[0] == "get-profiles") + && line.ends_with(' ') + { Ok((args[0].len() + 1, self.complete_rooms(args.get(1)))) } else { Ok(( @@ -152,7 +158,7 @@ impl Completer for InspectorHelper { } else { Ok((args[0].len() + 1, self.complete_rooms(args.get(1)))) } - } else if args[0] == "get-members" { + } else if args[0] == "get-members" || args[0] == "get-profiles" { Ok((args[0].len() + 1, self.complete_rooms(args.get(1)))) } else { Ok((pos, vec![])) @@ -222,6 +228,13 @@ impl Inspector { async fn run(&self, matches: ArgMatches<'_>) { match matches.subcommand() { + ("get-profiles", args) => { + let args = args.expect("No args provided for get-state"); + let room_id = RoomId::try_from(args.value_of("room-id").unwrap()).unwrap(); + + self.get_profiles(room_id).await; + } + ("get-members", args) => { let args = args.expect("No args provided for get-state"); let room_id = RoomId::try_from(args.value_of("room-id").unwrap()).unwrap(); @@ -244,6 +257,20 @@ impl Inspector { self.printer.pretty_print_struct(&rooms); } + async fn get_profiles(&self, room_id: RoomId) { + let joined: Vec = self + .store + .get_joined_user_ids(&room_id) + .await + .collect() + .await; + + for member in joined { + let event = self.store.get_profile(&room_id, &member).await; + self.printer.pretty_print_struct(&event); + } + } + async fn get_members(&self, room_id: RoomId) { let joined: Vec = self .store @@ -278,6 +305,13 @@ impl Inspector { .map_err(|_| "Invalid room id given".to_owned()) }), )) + .subcommand(SubCommand::with_name("get-profiles").arg( + Arg::with_name("room-id").required(true).validator(|r| { + RoomId::try_from(r) + .map(|_| ()) + .map_err(|_| "Invalid room id given".to_owned()) + }), + )) .subcommand( SubCommand::with_name("get-state") .arg(Arg::with_name("room-id").required(true).validator(|r| { diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 1b07335c..3ff27082 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -449,6 +449,18 @@ impl BaseClient { } } + // Senders can fake the profile easily so we keep track + // of profiles that the member set themselves to avoid + // having confusing profile changes when a member gets + // kicked/banned. + if member.state_key == member.sender { + changes + .profiles + .entry(room_id.clone()) + .or_insert_with(BTreeMap::new) + .insert(member.sender.clone(), member.content.clone()); + } + changes .members .entry(room_id.clone()) @@ -551,6 +563,7 @@ impl BaseClient { ) -> ( State, BTreeMap, + BTreeMap, BTreeMap>, BTreeSet, ) { @@ -558,6 +571,7 @@ impl BaseClient { let mut members = BTreeMap::new(); let mut state_events = BTreeMap::new(); let mut user_ids = BTreeSet::new(); + let mut profiles = BTreeMap::new(); let room_id = room_info.room_id.clone(); @@ -587,6 +601,15 @@ impl BaseClient { } _ => (), } + + // Senders can fake the profile easily so we keep track + // of profiles that the member set themselves to avoid + // having confusing profile changes when a member gets + // kicked/banned. + if m.state_key == m.sender { + profiles.insert(m.sender.clone(), m.content.clone()); + } + members.insert(m.state_key.clone(), m); } Err(e) => warn!( @@ -602,7 +625,7 @@ impl BaseClient { } } - (state, members, state_events, user_ids) + (state, members, profiles, state_events, user_ids) } async fn handle_room_account_data( @@ -665,10 +688,11 @@ impl BaseClient { room_info.update_summary(&new_info.summary); room_info.set_prev_batch(new_info.timeline.prev_batch.as_deref()); - let (state, members, state_events, mut user_ids) = + let (state, members, profiles, state_events, mut user_ids) = self.handle_state(new_info.state.events, &mut room_info); changes.members.insert(room_id.clone(), members); + changes.profiles.insert(room_id.clone(), profiles); changes.state.insert(room_id.clone(), state_events); if new_info.timeline.limited { @@ -737,10 +761,11 @@ impl BaseClient { let mut room_info = room.clone_info(); room_info.mark_as_left(); - let (state, members, state_events, mut user_ids) = + let (state, members, profiles, state_events, mut user_ids) = self.handle_state(new_info.state.events, &mut room_info); changes.members.insert(room_id.clone(), members); + changes.profiles.insert(room_id.clone(), profiles); changes.state.insert(room_id.clone(), state_events); let timeline = self diff --git a/matrix_sdk_base/src/rooms/members.rs b/matrix_sdk_base/src/rooms/members.rs index a31860e6..efb542cb 100644 --- a/matrix_sdk_base/src/rooms/members.rs +++ b/matrix_sdk_base/src/rooms/members.rs @@ -16,7 +16,9 @@ use std::sync::Arc; use matrix_sdk_common::{ events::{ - presence::PresenceEvent, room::power_levels::PowerLevelsEventContent, SyncStateEvent, + presence::PresenceEvent, + room::{member::MemberEventContent, power_levels::PowerLevelsEventContent}, + SyncStateEvent, }, identifiers::UserId, }; @@ -26,6 +28,7 @@ use crate::responses::MemberEvent; #[derive(Clone, Debug)] pub struct RoomMember { pub(crate) event: Arc, + pub(crate) profile: Arc>, pub(crate) presence: Arc>, pub(crate) power_levles: Arc>>, } @@ -36,7 +39,11 @@ impl RoomMember { } pub fn display_name(&self) -> Option<&str> { - self.event.content.displayname.as_deref() + if let Some(p) = self.profile.as_ref() { + p.displayname.as_deref() + } else { + self.event.content.displayname.as_deref() + } } pub fn power_level(&self) -> i64 { diff --git a/matrix_sdk_base/src/rooms/normal.rs b/matrix_sdk_base/src/rooms/normal.rs index 9e997dea..83e99a71 100644 --- a/matrix_sdk_base/src/rooms/normal.rs +++ b/matrix_sdk_base/src/rooms/normal.rs @@ -96,8 +96,9 @@ impl Room { let joined = self.store.get_joined_user_ids(self.room_id()).await; let invited = self.store.get_invited_user_ids(self.room_id()).await; - let x = move |u| async move { + let into_member = move |u| async move { let presence = self.store.get_presence_event(&u).await; + let profile = self.store.get_profile(self.room_id(), &u).await; let power = self .store .get_state_event(self.room_id(), EventType::RoomPowerLevels, "") @@ -116,12 +117,13 @@ impl Room { .await .map(|m| RoomMember { event: m.into(), + profile: profile.into(), presence: presence.into(), power_levles: power.into(), }) }; - joined.chain(invited).filter_map(x) + joined.chain(invited).filter_map(into_member) } /// Calculate the canonical display name of the room, taking into account @@ -234,6 +236,7 @@ impl Room { pub async fn get_member(&self, user_id: &UserId) -> Option { let presence = self.store.get_presence_event(user_id).await; + let profile = self.store.get_profile(self.room_id(), user_id).await; let power = self .store .get_state_event(self.room_id(), EventType::RoomPowerLevels, "") @@ -252,6 +255,7 @@ impl Room { .await .map(|e| RoomMember { event: e.into(), + profile: profile.into(), presence: presence.into(), power_levles: power.into(), }) diff --git a/matrix_sdk_base/src/store.rs b/matrix_sdk_base/src/store.rs index 924f78ca..18763322 100644 --- a/matrix_sdk_base/src/store.rs +++ b/matrix_sdk_base/src/store.rs @@ -6,8 +6,9 @@ use dashmap::DashMap; use futures::stream::{self, Stream}; use matrix_sdk_common::{ events::{ - presence::PresenceEvent, room::member::MembershipState, AnyBasicEvent, - AnyStrippedStateEvent, AnySyncStateEvent, EventContent, EventType, + presence::PresenceEvent, + room::member::{MemberEventContent, MembershipState}, + AnyBasicEvent, AnyStrippedStateEvent, AnySyncStateEvent, EventContent, EventType, }, identifiers::{RoomId, UserId}, locks::RwLock, @@ -126,6 +127,7 @@ pub struct SledStore { session: Tree, account_data: Tree, members: Tree, + profiles: Tree, joined_user_ids: Tree, invited_user_ids: Tree, room_info: Tree, @@ -144,6 +146,7 @@ pub struct StateChanges { pub presence: BTreeMap, pub members: BTreeMap>, + pub profiles: BTreeMap>, pub state: BTreeMap>>, pub room_account_data: BTreeMap>, pub room_infos: BTreeMap, @@ -217,6 +220,7 @@ impl SledStore { let account_data = db.open_tree("account_data").unwrap(); let members = db.open_tree("members").unwrap(); + let profiles = db.open_tree("profiles").unwrap(); let joined_user_ids = db.open_tree("joined_user_ids").unwrap(); let invited_user_ids = db.open_tree("invited_user_ids").unwrap(); @@ -234,6 +238,7 @@ impl SledStore { session, account_data, members, + profiles, joined_user_ids, invited_user_ids, room_account_data, @@ -279,6 +284,7 @@ impl SledStore { &self.session, &self.account_data, &self.members, + &self.profiles, &self.joined_user_ids, &self.invited_user_ids, &self.room_info, @@ -294,6 +300,7 @@ impl SledStore { session, account_data, members, + profiles, joined, invited, summaries, @@ -334,6 +341,15 @@ impl SledStore { } } + for (room, users) in &changes.profiles { + for (user_id, profile) in users { + profiles.insert( + format!("{}{}", room.as_str(), user_id.as_str()).as_str(), + serde_json::to_vec(&profile).unwrap(), + )?; + } + } + for (event_type, event) in &changes.account_data { account_data .insert(event_type.as_str(), serde_json::to_vec(&event).unwrap())?; @@ -435,6 +451,17 @@ impl SledStore { .map(|e| serde_json::from_slice(&e).unwrap()) } + pub async fn get_profile( + &self, + room_id: &RoomId, + user_id: &UserId, + ) -> Option { + self.profiles + .get(format!("{}{}", room_id.as_str(), user_id.as_str())) + .unwrap() + .map(|p| serde_json::from_slice(&p).unwrap()) + } + pub async fn get_member_event( &self, room_id: &RoomId,