From f2fab27d819f7b33f246491e8d4d4ba0b12eea72 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Mon, 27 Jul 2020 20:48:51 -0400 Subject: [PATCH 1/6] Implement filtering invites if sender is ignored by receiver --- src/client_server.rs | 22 ++++++++++++++++++++++ src/database/rooms.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/client_server.rs b/src/client_server.rs index 17218dfc..cf3f67ca 100644 --- a/src/client_server.rs +++ b/src/client_server.rs @@ -487,6 +487,7 @@ pub fn deactivate_route( redacts: None, }, &db.globals, + &db.account_data, )?; } @@ -695,6 +696,7 @@ pub fn set_displayname_route( redacts: None, }, &db.globals, + &db.account_data, )?; // Presence update @@ -797,6 +799,7 @@ pub fn set_avatar_url_route( redacts: None, }, &db.globals, + &db.account_data, )?; // Presence update @@ -1318,6 +1321,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // 2. Let the room creator join @@ -1339,6 +1343,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // Figure out preset. We need it for power levels and preset specific events @@ -1387,6 +1392,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // 4. Events set by preset @@ -1412,6 +1418,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // 4.2 History Visibility @@ -1429,6 +1436,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // 4.3 Guest Access @@ -1454,6 +1462,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; // 5. Events listed in initial_state @@ -1481,6 +1490,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; } @@ -1502,6 +1512,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; } @@ -1520,6 +1531,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; } @@ -1543,6 +1555,7 @@ pub fn create_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; } @@ -1602,6 +1615,7 @@ pub fn redact_event_route( redacts: Some(body.event_id.clone()), }, &db.globals, + &db.account_data, )?; Ok(redact_event::Response { event_id }.into()) @@ -1696,6 +1710,7 @@ pub fn join_room_by_id_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(join_room_by_id::Response { @@ -1775,6 +1790,7 @@ pub fn leave_room_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(leave_room::Response.into()) @@ -1821,6 +1837,7 @@ pub fn kick_user_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(kick_user::Response.into()) @@ -1913,6 +1930,7 @@ pub fn ban_user_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(ban_user::Response.into()) @@ -1958,6 +1976,7 @@ pub fn unban_user_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(unban_user::Response.into()) @@ -2007,6 +2026,7 @@ pub fn invite_user_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(invite_user::Response.into()) @@ -2396,6 +2416,7 @@ pub fn create_message_event_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(create_message_event::Response { event_id }.into()) @@ -2461,6 +2482,7 @@ pub fn create_state_event_for_key_route( redacts: None, }, &db.globals, + &db.account_data, )?; Ok(create_state_event_for_key::Response { event_id }.into()) diff --git a/src/database/rooms.rs b/src/database/rooms.rs index 9ff11b89..f30ce14a 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -7,6 +7,7 @@ use log::error; use ruma::{ api::client::error::ErrorKind, events::{ + ignored_user_list, room::{ join_rules, member, power_levels::{self, PowerLevelsEventContent}, @@ -255,6 +256,7 @@ impl Rooms { &self, pdu_builder: PduBuilder, globals: &super::globals::Globals, + account_data: &super::account_data::AccountData, ) -> Result { let PduBuilder { room_id, @@ -411,6 +413,43 @@ impl Rooms { || join_rules == join_rules::JoinRule::Public } } else if target_membership == member::MembershipState::Invite { + // we want to know if the sender is ignored by the receiver + let is_ignored = if let Ok(Some(ignored)) = + account_data.get::( + None, // we cannot use the provided room_id it's the invite room + &target_user_id, // receiver + EventType::IgnoredUserList, + ) { + ignored.ignored_users.contains(&sender) + } else { + false + }; + + if is_ignored { + let mut event = + serde_json::from_value::>(content) + .expect("from_value::> cannot fail") + .deserialize() + .map_err(|_| { + Error::bad_database("Invalid member event in database.") + })?; + + event.membership = member::MembershipState::Leave; + + return self.append_pdu( + room_id, + target_user_id.clone(), + EventType::RoomMember, + serde_json::to_value(event) + .expect("event is valid, we just created it"), + None, + Some(target_user_id.to_string()), + None, + globals, + account_data, + ); + } + if let Some(third_party_invite_json) = content.get("third_party_invite") { if current_membership == member::MembershipState::Ban { false From 8aac332b3ab023f723df08dd052c02ae16f26b32 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Wed, 29 Jul 2020 13:47:50 -0400 Subject: [PATCH 2/6] Move ignore invite logic into update_member --- src/database/rooms.rs | 86 ++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/src/database/rooms.rs b/src/database/rooms.rs index f30ce14a..59ebc720 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -355,12 +355,13 @@ impl Rooms { .membership) })?; - let target_membership = + let member_content = serde_json::from_value::>(content.clone()) .expect("Raw::from_value always works.") .deserialize() - .map_err(|_| Error::bad_database("Invalid Member event in db."))? - .membership; + .map_err(|_| Error::bad_database("Invalid Member event in db."))?; + + let target_membership = member_content.membership; let target_power = power_levels.users.get(&target_user_id).map_or_else( || { @@ -413,43 +414,6 @@ impl Rooms { || join_rules == join_rules::JoinRule::Public } } else if target_membership == member::MembershipState::Invite { - // we want to know if the sender is ignored by the receiver - let is_ignored = if let Ok(Some(ignored)) = - account_data.get::( - None, // we cannot use the provided room_id it's the invite room - &target_user_id, // receiver - EventType::IgnoredUserList, - ) { - ignored.ignored_users.contains(&sender) - } else { - false - }; - - if is_ignored { - let mut event = - serde_json::from_value::>(content) - .expect("from_value::> cannot fail") - .deserialize() - .map_err(|_| { - Error::bad_database("Invalid member event in database.") - })?; - - event.membership = member::MembershipState::Leave; - - return self.append_pdu( - room_id, - target_user_id.clone(), - EventType::RoomMember, - serde_json::to_value(event) - .expect("event is valid, we just created it"), - None, - Some(target_user_id.to_string()), - None, - globals, - account_data, - ); - } - if let Some(third_party_invite_json) = content.get("third_party_invite") { if current_membership == member::MembershipState::Ban { false @@ -502,7 +466,14 @@ impl Rooms { if authorized { // Update our membership info - self.update_membership(&room_id, &target_user_id, &target_membership)?; + self.update_membership( + &room_id, + &target_user_id, + member_content, + &sender, + account_data, + globals, + )?; } authorized @@ -780,8 +751,12 @@ impl Rooms { &self, room_id: &RoomId, user_id: &UserId, - membership: &member::MembershipState, + mut member_event: member::MemberEventContent, + sender: &UserId, + account_data: &super::account_data::AccountData, + globals: &super::globals::Globals, ) -> Result<()> { + let membership = member_event.membership; let mut userroom_id = user_id.to_string().as_bytes().to_vec(); userroom_id.push(0xff); userroom_id.extend_from_slice(room_id.to_string().as_bytes()); @@ -799,6 +774,33 @@ impl Rooms { self.userroomid_left.remove(&userroom_id)?; } member::MembershipState::Invite => { + // We want to know if the sender is ignored by the receiver + let is_ignored = account_data + .get::( + None, // Ignored users are in global account data + &user_id, // Receiver + EventType::IgnoredUserList, + )? + .map_or(false, |ignored| ignored.ignored_users.contains(&sender)); + + if is_ignored { + member_event.membership = member::MembershipState::Leave; + + return self + .append_pdu( + room_id.clone(), + user_id.clone(), + EventType::RoomMember, + serde_json::to_value(member_event) + .expect("event is valid, we just created it"), + None, + Some(user_id.to_string()), + None, + globals, + account_data, + ) + .map(|_| ()); + } self.userroomid_invited.insert(&userroom_id, &[])?; self.roomuserid_invited.insert(&roomuser_id, &[])?; self.userroomid_joined.remove(&userroom_id)?; From 25c0e75f29b60d8c9104e51e26f83bdc98cb2a71 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Wed, 29 Jul 2020 14:28:40 -0400 Subject: [PATCH 3/6] Cargo fmt --- src/database/account_data.rs | 3 +-- src/database/key_backups.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/database/account_data.rs b/src/database/account_data.rs index 99e0d5c7..a9171235 100644 --- a/src/database/account_data.rs +++ b/src/database/account_data.rs @@ -4,8 +4,7 @@ use ruma::{ events::{AnyEvent as EduEvent, EventType}, Raw, RoomId, UserId, }; -use serde::de::DeserializeOwned; -use serde::Serialize; +use serde::{de::DeserializeOwned, Serialize}; use sled::IVec; use std::{collections::HashMap, convert::TryFrom}; diff --git a/src/database/key_backups.rs b/src/database/key_backups.rs index a5065647..5b37f1b0 100644 --- a/src/database/key_backups.rs +++ b/src/database/key_backups.rs @@ -4,7 +4,7 @@ use ruma::{ error::ErrorKind, r0::backup::{BackupAlgorithm, KeyData, Sessions}, }, - {RoomId, UserId}, + RoomId, UserId, }; use std::{collections::BTreeMap, convert::TryFrom}; From 99220565d4f71b734f88d2dc9c341a3ac72903c1 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Wed, 29 Jul 2020 17:07:12 -0400 Subject: [PATCH 4/6] Fix invite leave auth error by moving update_membership --- src/client_server.rs | 4 +-- src/database/rooms.rs | 64 +++++++++++++++++++++++++++---------------- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/client_server.rs b/src/client_server.rs index cf3f67ca..aad0b637 100644 --- a/src/client_server.rs +++ b/src/client_server.rs @@ -3243,8 +3243,8 @@ pub fn get_message_events_route( .collect::>(); Ok(get_message_events::Response { - start: Some(body.from.clone()), - end: start_token, + start: start_token, + end: Some(body.from.clone()), chunk: events_before, state: Vec::new(), } diff --git a/src/database/rooms.rs b/src/database/rooms.rs index 59ebc720..c44eb5a5 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -355,13 +355,12 @@ impl Rooms { .membership) })?; - let member_content = + let target_membership = serde_json::from_value::>(content.clone()) .expect("Raw::from_value always works.") .deserialize() - .map_err(|_| Error::bad_database("Invalid Member event in db."))?; - - let target_membership = member_content.membership; + .map_err(|_| Error::bad_database("Invalid Member event in db."))? + .membership; let target_power = power_levels.users.get(&target_user_id).map_or_else( || { @@ -464,18 +463,6 @@ impl Rooms { false }; - if authorized { - // Update our membership info - self.update_membership( - &room_id, - &target_user_id, - member_content, - &sender, - account_data, - globals, - )?; - } - authorized } EventType::RoomCreate => prev_events.is_empty(), @@ -533,7 +520,7 @@ impl Rooms { .expect("time is valid"), kind: event_type.clone(), content: content.clone(), - state_key, + state_key: state_key.clone(), prev_events, depth: depth .try_into() @@ -606,6 +593,35 @@ impl Rooms { self.redact_pdu(&redact_id)?; } + EventType::RoomMember => { + if let Some(state_key) = state_key { + // if the state_key fails + let target_user_id = UserId::try_from(state_key).map_err(|_| { + Error::BadRequest( + ErrorKind::InvalidParam, + "State key of member event does not contain user id.", + ) + })?; + // Update our membership info, we do this here incase a user is invited + // and imediatly leaves we need the DB to record the invite event for auth + self.update_membership( + &room_id, + &target_user_id, + serde_json::from_value::(content).map_err( + |_| { + Error::BadRequest( + ErrorKind::InvalidParam, + "Invalid redaction event content.", + ) + }, + )?, + &sender, + account_data, + globals, + )?; + } + } + _ => {} } self.edus.room_read_set(&room_id, &sender, index)?; @@ -751,12 +767,12 @@ impl Rooms { &self, room_id: &RoomId, user_id: &UserId, - mut member_event: member::MemberEventContent, + mut member_content: member::MemberEventContent, sender: &UserId, account_data: &super::account_data::AccountData, globals: &super::globals::Globals, ) -> Result<()> { - let membership = member_event.membership; + let membership = member_content.membership; let mut userroom_id = user_id.to_string().as_bytes().to_vec(); userroom_id.push(0xff); userroom_id.extend_from_slice(room_id.to_string().as_bytes()); @@ -776,22 +792,24 @@ impl Rooms { member::MembershipState::Invite => { // We want to know if the sender is ignored by the receiver let is_ignored = account_data - .get::( + .get::( None, // Ignored users are in global account data &user_id, // Receiver EventType::IgnoredUserList, )? - .map_or(false, |ignored| ignored.ignored_users.contains(&sender)); + .map_or(false, |ignored| { + ignored.content.ignored_users.contains(&sender) + }); if is_ignored { - member_event.membership = member::MembershipState::Leave; + member_content.membership = member::MembershipState::Leave; return self .append_pdu( room_id.clone(), user_id.clone(), EventType::RoomMember, - serde_json::to_value(member_event) + serde_json::to_value(member_content) .expect("event is valid, we just created it"), None, Some(user_id.to_string()), From c8d7d80eb24acf2a62304a87b3e08b7cd2a2f86f Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Thu, 30 Jul 2020 07:50:09 -0400 Subject: [PATCH 5/6] Fix start/end token swap left from dropped commits --- src/client_server.rs | 4 ++-- src/database/rooms.rs | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/client_server.rs b/src/client_server.rs index aad0b637..cf3f67ca 100644 --- a/src/client_server.rs +++ b/src/client_server.rs @@ -3243,8 +3243,8 @@ pub fn get_message_events_route( .collect::>(); Ok(get_message_events::Response { - start: start_token, - end: Some(body.from.clone()), + start: Some(body.from.clone()), + end: start_token, chunk: events_before, state: Vec::new(), } diff --git a/src/database/rooms.rs b/src/database/rooms.rs index c44eb5a5..5c17f27e 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -596,14 +596,10 @@ impl Rooms { EventType::RoomMember => { if let Some(state_key) = state_key { // if the state_key fails - let target_user_id = UserId::try_from(state_key).map_err(|_| { - Error::BadRequest( - ErrorKind::InvalidParam, - "State key of member event does not contain user id.", - ) - })?; + let target_user_id = UserId::try_from(state_key) + .expect("This state_key was previously validated"); // Update our membership info, we do this here incase a user is invited - // and imediatly leaves we need the DB to record the invite event for auth + // and immediately leaves we need the DB to record the invite event for auth self.update_membership( &room_id, &target_user_id, From 7a70d8488f0a4e6b286bc151520efd2dd85ecb0e Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Thu, 30 Jul 2020 08:43:51 -0400 Subject: [PATCH 6/6] Rebase with master and update append_pdu call --- src/database/rooms.rs | 67 ++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/src/database/rooms.rs b/src/database/rooms.rs index 5c17f27e..fe633180 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -388,7 +388,7 @@ impl Rooms { .join_rule) })?; - let authorized = if target_membership == member::MembershipState::Join { + if target_membership == member::MembershipState::Join { let mut prev_events = prev_events.iter(); let prev_event = self .get_pdu(prev_events.next().ok_or(Error::BadRequest( @@ -461,14 +461,11 @@ impl Rooms { } } else { false - }; - - authorized + } } EventType::RoomCreate => prev_events.is_empty(), // Not allow any of the following events if the sender is not joined. _ if sender_membership != member::MembershipState::Join => false, - _ => { // TODO sender_power.unwrap_or(&power_levels.users_default) @@ -576,22 +573,24 @@ impl Rooms { self.roomstateid_pdu.insert(key, &*pdu_json.to_string())?; } - if let EventType::RoomRedaction = event_type { - if let Some(redact_id) = &redacts { - // TODO: Reason - let _reason = - serde_json::from_value::>(content) - .expect("Raw::from_value always works.") - .deserialize() - .map_err(|_| { - Error::BadRequest( - ErrorKind::InvalidParam, - "Invalid redaction event content.", - ) - })? - .reason; + match event_type { + EventType::RoomRedaction => { + if let Some(redact_id) = &redacts { + // TODO: Reason + let _reason = + serde_json::from_value::>(content) + .expect("Raw::from_value always works.") + .deserialize() + .map_err(|_| { + Error::BadRequest( + ErrorKind::InvalidParam, + "Invalid redaction event content.", + ) + })? + .reason; - self.redact_pdu(&redact_id)?; + self.redact_pdu(&redact_id)?; + } } EventType::RoomMember => { if let Some(state_key) = state_key { @@ -800,20 +799,22 @@ impl Rooms { if is_ignored { member_content.membership = member::MembershipState::Leave; - return self - .append_pdu( - room_id.clone(), - user_id.clone(), - EventType::RoomMember, - serde_json::to_value(member_content) + self.append_pdu( + PduBuilder { + room_id: room_id.clone(), + sender: user_id.clone(), + event_type: EventType::RoomMember, + content: serde_json::to_value(member_content) .expect("event is valid, we just created it"), - None, - Some(user_id.to_string()), - None, - globals, - account_data, - ) - .map(|_| ()); + unsigned: None, + state_key: Some(user_id.to_string()), + redacts: None, + }, + globals, + account_data, + )?; + + return Ok(()); } self.userroomid_invited.insert(&userroom_id, &[])?; self.roomuserid_invited.insert(&roomuser_id, &[])?;