Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More redaction improvements #652

Merged
merged 7 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ zeroize = { version = "1.3.0", features = ["zeroize_derive"] }
ruma = { version = "0.6.1", features = ["client-api-c", "js", "signatures"] }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
ruma = { version = "0.6.1", features = ["client-api-c", "signatures"] }
ruma = { version = "0.6.2", features = ["client-api-c", "signatures"] }

[dev-dependencies]
futures = { version = "0.3.21", default-features = false, features = ["executor"] }
Expand Down
69 changes: 37 additions & 32 deletions crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ use ruma::{
api::client::{self as api, push::get_notifications::v3::Notification},
events::{
push_rules::PushRulesEvent,
room::member::{MembershipState, OriginalSyncRoomMemberEvent, RoomMemberEvent},
room::member::{MembershipState, SyncRoomMemberEvent},
AnyGlobalAccountDataEvent, AnyRoomAccountDataEvent, AnyStrippedStateEvent,
AnySyncEphemeralRoomEvent, AnySyncRoomEvent, AnySyncStateEvent, GlobalAccountDataEventType,
StateEventType, SyncStateEvent,
StateEventType,
},
push::{Action, PushConditionRoomCtx, Ruleset},
serde::Raw,
MilliSecondsSinceUnixEpoch, OwnedUserId, RoomId, UInt, UserId,
};
use tracing::{info, trace, warn};
use tracing::{debug, info, trace, warn};

#[cfg(feature = "e2e-encryption")]
use crate::error::Error;
Expand Down Expand Up @@ -281,35 +281,35 @@ impl BaseClient {
#[allow(clippy::single_match)]
match &e {
AnySyncRoomEvent::State(s) => match s {
AnySyncStateEvent::RoomMember(SyncStateEvent::Original(member)) => {
AnySyncStateEvent::RoomMember(member) => {
ambiguity_cache.handle_event(changes, room_id, member).await?;

match member.content.membership {
match member.membership() {
MembershipState::Join | MembershipState::Invite => {
user_ids.insert(member.state_key.clone());
user_ids.insert(member.state_key().to_owned());
}
_ => {
user_ids.remove(&member.state_key);
user_ids.remove(member.state_key());
}
}

// 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 {
if member.state_key() == member.sender() {
changes
.profiles
.entry(room_id.to_owned())
.or_default()
.insert(member.sender.clone(), member.content.clone());
.insert(member.sender().to_owned(), member.into());
}

changes
.members
.entry(room_id.to_owned())
.or_default()
.insert(member.state_key.clone(), member.to_owned());
.insert(member.state_key().to_owned(), member.clone());
}
_ => {
room_info.handle_state_event(s);
Expand Down Expand Up @@ -470,12 +470,12 @@ impl BaseClient {

room_info.handle_state_event(&event);

if let AnySyncStateEvent::RoomMember(SyncStateEvent::Original(member)) = event {
if let AnySyncStateEvent::RoomMember(member) = event {
ambiguity_cache.handle_event(changes, &room_id, &member).await?;

match member.content.membership {
match member.membership() {
MembershipState::Join | MembershipState::Invite => {
user_ids.insert(member.state_key.clone());
user_ids.insert(member.state_key().to_owned());
}
_ => (),
}
Expand All @@ -484,11 +484,11 @@ impl BaseClient {
// 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 {
profiles.insert(member.sender.clone(), member.content.clone());
if member.state_key() == member.sender() {
profiles.insert(member.sender().to_owned(), (&member).into());
}

members.insert(member.state_key.clone(), member);
members.insert(member.state_key().to_owned(), member);
} else {
state_events
.entry(event.event_type())
Expand Down Expand Up @@ -859,12 +859,12 @@ impl BaseClient {
let members: Vec<_> = response
.chunk
.iter()
.filter_map(|e| e.deserialize().ok())
.filter_map(|ev| match ev {
RoomMemberEvent::Original(ev) => Some(ev),
// FIXME: don't filter these out
// https://github.com/matrix-org/matrix-rust-sdk/issues/607
RoomMemberEvent::Redacted(_) => None,
.filter_map(|event| match event.deserialize() {
Ok(ev) => Some(ev),
Err(e) => {
debug!(?event, "Failed to deserialize m.room.member event: {}", e);
None
}
})
.collect();

Expand All @@ -880,32 +880,32 @@ impl BaseClient {
let mut user_ids = BTreeSet::new();

for member in &members {
let member: OriginalSyncRoomMemberEvent = member.clone().into();
let member: SyncRoomMemberEvent = member.clone().into();

if self.store.get_member_event(room_id, &member.state_key).await?.is_none() {
if self.store.get_member_event(room_id, member.state_key()).await?.is_none() {
#[cfg(feature = "e2e-encryption")]
match member.content.membership {
match member.membership() {
MembershipState::Join | MembershipState::Invite => {
user_ids.insert(member.state_key.clone());
user_ids.insert(member.state_key().to_owned());
}
_ => (),
}

ambiguity_cache.handle_event(&changes, room_id, &member).await?;

if member.state_key == member.sender {
if member.state_key() == member.sender() {
changes
.profiles
.entry(room_id.to_owned())
.or_default()
.insert(member.sender.clone(), member.content.clone());
.insert(member.sender().to_owned(), (&member).into());
}

changes
.members
.entry(room_id.to_owned())
.or_default()
.insert(member.state_key.clone(), member);
.insert(member.state_key().to_owned(), member);
}
}

Expand Down Expand Up @@ -1238,7 +1238,10 @@ impl BaseClient {
let user_display_name = if let Some(member) =
changes.members.get(room_id).and_then(|members| members.get(user_id))
{
member.content.displayname.clone().unwrap_or_else(|| user_id.localpart().to_owned())
member
.as_original()
.and_then(|ev| ev.content.displayname.clone())
.unwrap_or_else(|| user_id.localpart().to_owned())
} else if let Some(member) = room.get_member(user_id).await? {
member.name().to_owned()
} else {
Expand Down Expand Up @@ -1291,8 +1294,10 @@ impl BaseClient {
if let Some(member) =
changes.members.get(&**room_id).and_then(|members| members.get(user_id))
{
push_rules.user_display_name =
member.content.displayname.clone().unwrap_or_else(|| user_id.localpart().to_owned())
push_rules.user_display_name = member
.as_original()
.and_then(|ev| ev.content.displayname.clone())
.unwrap_or_else(|| user_id.localpart().to_owned())
}

if let Some(AnySyncStateEvent::RoomPowerLevels(event)) = changes
Expand Down
4 changes: 4 additions & 0 deletions crates/matrix-sdk-base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ mod session;
pub mod store;
#[cfg(feature = "experimental-timeline")]
mod timeline_stream;
mod utils;

pub use client::BaseClient;
#[cfg(any(test, feature = "testing"))]
Expand All @@ -42,3 +43,6 @@ pub use http;
pub use matrix_sdk_crypto as crypto;
pub use rooms::{DisplayName, Room, RoomInfo, RoomMember, RoomType};
pub use store::{StateChanges, StateStore, Store, StoreError};
pub use utils::{
MinimalRoomMemberEvent, MinimalStateEvent, OriginalMinimalStateEvent, RedactedMinimalStateEvent,
};
35 changes: 15 additions & 20 deletions crates/matrix-sdk-base/src/rooms/members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ use std::sync::Arc;
use ruma::{
events::{
presence::PresenceEvent,
room::{
member::{MembershipState, RoomMemberEventContent},
power_levels::SyncRoomPowerLevelsEvent,
},
room::{member::MembershipState, power_levels::SyncRoomPowerLevelsEvent},
},
MxcUri, UserId,
};

use crate::deserialized_responses::MemberEvent;
use crate::{deserialized_responses::MemberEvent, MinimalRoomMemberEvent};

/// A member of a room.
#[derive(Clone, Debug)]
pub struct RoomMember {
pub(crate) event: Arc<MemberEvent>,
pub(crate) profile: Arc<Option<RoomMemberEventContent>>,
// The latest member event sent by the member themselves.
// Stored in addition to the latest member event overall to get displayname
// and avatar from, which should be ignored on events sent by others.
pub(crate) profile: Arc<Option<MinimalRoomMemberEvent>>,
#[allow(dead_code)]
pub(crate) presence: Arc<Option<PresenceEvent>>,
pub(crate) power_levels: Arc<Option<SyncRoomPowerLevelsEvent>>,
Expand All @@ -49,9 +49,9 @@ impl RoomMember {
/// Get the display name of the member if there is one.
pub fn display_name(&self) -> Option<&str> {
if let Some(p) = self.profile.as_ref() {
p.displayname.as_deref()
p.as_original().and_then(|e| e.content.displayname.as_deref())
} else {
self.event.content().displayname.as_deref()
self.event.original_content()?.displayname.as_deref()
}
}

Expand All @@ -69,9 +69,10 @@ impl RoomMember {

/// Get the avatar url of the member, if there is one.
pub fn avatar_url(&self) -> Option<&MxcUri> {
match self.profile.as_ref() {
Some(p) => p.avatar_url.as_deref(),
None => self.event.content().avatar_url.as_deref(),
if let Some(p) = self.profile.as_ref() {
p.as_original().and_then(|e| e.content.avatar_url.as_deref())
} else {
self.event.original_content()?.avatar_url.as_deref()
}
}

Expand All @@ -91,13 +92,7 @@ impl RoomMember {
pub fn power_level(&self) -> i64 {
(*self.power_levels)
.as_ref()
.map(|e| {
let pls = e.power_levels();
pls.users
.get(self.user_id())
.map(|p| (*p).into())
.unwrap_or_else(|| pls.users_default.into())
})
.map(|e| e.power_levels().for_user(self.user_id()).into())
.unwrap_or_else(|| if self.is_room_creator { 100 } else { 0 })
}

Expand All @@ -112,9 +107,9 @@ impl RoomMember {
/// Get the membership state of this member.
pub fn membership(&self) -> &MembershipState {
if let Some(p) = self.profile.as_ref() {
&p.membership
p.membership()
} else {
&self.event.content().membership
self.event.membership()
}
}
}
Loading