diff --git a/src/api/client/membership/join.rs b/src/api/client/membership/join.rs index 16ce81fa..b2002dbb 100644 --- a/src/api/client/membership/join.rs +++ b/src/api/client/membership/join.rs @@ -553,20 +553,16 @@ async fn join_room_by_id_helper_remote( .iter() .stream() .then(|pdu| { - debug!(?pdu, "Validating send_join response room_state event"); services .server_keys .validate_and_add_event_id_no_fetch(pdu, &room_version_id) .inspect_err(|e| { - debug_warn!( - "Could not validate send_join response room_state event: {e:?}" - ); + debug_warn!("Could not validate send_join response room_state event: {e:?}"); }) .inspect(|_| debug!("Completed validating send_join response room_state event")) }) .ready_filter_map(Result::ok) .fold(HashMap::new(), |mut state, (event_id, value)| async move { - debug!(?event_id, "Processing send_join response room_state event"); let pdu = match PduEvent::from_id_val(&event_id, value.clone()) { | Ok(pdu) => pdu, | Err(e) => { @@ -574,10 +570,8 @@ async fn join_room_by_id_helper_remote( return state; }, }; - debug!(event_id = ?event_id.clone(), "Adding PDU outlier for send_join response room_state event"); services.rooms.outlier.add_pdu_outlier(&event_id, &value); if let Some(state_key) = &pdu.state_key { - debug!(?state_key, "Creating shortstatekey for state event in send_join response"); let shortstatekey = services .rooms .short @@ -586,7 +580,6 @@ async fn join_room_by_id_helper_remote( state.insert(shortstatekey, pdu.event_id.clone()); } - debug!("Completed send_join response"); state }) .await; diff --git a/src/api/client/membership/mod.rs b/src/api/client/membership/mod.rs index 4f3850e3..5e742a69 100644 --- a/src/api/client/membership/mod.rs +++ b/src/api/client/membership/mod.rs @@ -69,12 +69,11 @@ pub(crate) async fn banned_room_check( } if let Some(room_id) = room_id { - if services.rooms.metadata.is_banned(room_id).await - || (room_id.server_name().is_some() - && services - .moderation - .is_remote_server_forbidden(room_id.server_name().expect("legacy room mxid"))) - { + let room_banned = services.rooms.metadata.is_banned(room_id).await; + let server_banned = room_id.server_name().is_some_and(|server_name| { + services.moderation.is_remote_server_forbidden(server_name) + }); + if room_banned || server_banned { warn!( "User {user_id} who is not an admin attempted to send an invite for or \ attempted to join a banned room or banned room server name: {room_id}" @@ -107,7 +106,6 @@ pub(crate) async fn banned_room_check( .boxed() .await?; } - return Err!(Request(Forbidden("This room is banned on this homeserver."))); } } else if let Some(server_name) = server_name { diff --git a/src/api/client/room/create.rs b/src/api/client/room/create.rs index a4402b0e..0fe74b1b 100644 --- a/src/api/client/room/create.rs +++ b/src/api/client/room/create.rs @@ -4,7 +4,7 @@ use axum::extract::State; use conduwuit::{ Err, Result, RoomVersion, debug, debug_info, debug_warn, err, info, matrix::{StateKey, pdu::PduBuilder}, - warn, + trace, warn, }; use conduwuit_service::{Services, appservice::RegistrationInfo}; use futures::FutureExt; @@ -82,12 +82,13 @@ pub(crate) async fn create_room_route( }; let room_features = RoomVersion::new(&room_version)?; - let room_id: Option = match room_features.room_ids_as_hashes { - | true => None, - | false => match &body.room_id { + let room_id: Option = if !room_features.room_ids_as_hashes { + match &body.room_id { | Some(custom_room_id) => Some(custom_room_id_check(&services, custom_room_id)?), | None => Some(RoomId::new(services.globals.server_name())), - }, + } + } else { + None }; // check if room ID doesn't already exist instead of erroring on auth check @@ -179,7 +180,7 @@ pub(crate) async fn create_room_route( | Some(room_id) => services.rooms.state.mutex.lock(&room_id).await, | None => { let temp_room_id = RoomId::new(services.globals.server_name()); - debug_info!("Locking temporary room state mutex for {temp_room_id}"); + trace!("Locking temporary room state mutex for {temp_room_id}"); services.rooms.state.mutex.lock(&temp_room_id).await }, }; @@ -202,12 +203,12 @@ pub(crate) async fn create_room_route( ) .boxed() .await?; - debug!("Created room create event with ID {}", create_event_id); + trace!("Created room create event with ID {}", create_event_id); let room_id = match room_id { | Some(room_id) => room_id, | None => { let as_room_id = create_event_id.as_str().replace('$', "!"); - debug_info!("Creating room with v12 room ID {as_room_id}"); + trace!("Creating room with v12 room ID {as_room_id}"); RoomId::parse(&as_room_id)?.to_owned() }, }; @@ -260,21 +261,26 @@ pub(crate) async fn create_room_route( } let mut creators: Vec = vec![sender_user.to_owned()]; - if let Some(additional_creators) = create_content.get("additional_creators") { - if let Some(additional_creators) = additional_creators.as_array() { - for creator in additional_creators { - if let Some(creator) = creator.as_str() { - if let Ok(creator) = OwnedUserId::parse(creator) { - creators.push(creator.clone()); - users.insert(creator.clone(), int!(100)); + // Do we care about additional_creators? + if room_features.explicitly_privilege_room_creators { + // Have they been specified? + if let Some(additional_creators) = create_content.get("additional_creators") { + // Are they a real array? + if let Some(additional_creators) = additional_creators.as_array() { + // Iterate through them + for creator in additional_creators { + // Are they a string? + if let Some(creator) = creator.as_str() { + // Do they parse into a real user ID? + if let Ok(creator) = OwnedUserId::parse(creator) { + // Add them to the power levels and creators + creators.push(creator.clone()); + } } } } } } - if !(RoomVersion::new(&room_version)?).explicitly_privilege_room_creators { - creators.clear(); - } let power_levels_content = default_power_levels_content( body.power_level_content_override.as_ref(), diff --git a/src/api/server/send.rs b/src/api/server/send.rs index b228fa98..9c5bfd2b 100644 --- a/src/api/server/send.rs +++ b/src/api/server/send.rs @@ -138,7 +138,6 @@ async fn handle( pdus: impl Stream + Send, edus: impl Stream + Send, ) -> Result { - // TODO(hydra): Does having no room ID break this? // group pdus by room let pdus = pdus .collect() @@ -187,7 +186,6 @@ async fn handle_room( .lock(&room_id) .await; - // TODO(hydra): We might be missing a room ID let room_id = &room_id; pdus.try_stream() .and_then(|(_, event_id, value)| async move { diff --git a/src/core/matrix/event/filter.rs b/src/core/matrix/event/filter.rs index e698a90f..aa537d68 100644 --- a/src/core/matrix/event/filter.rs +++ b/src/core/matrix/event/filter.rs @@ -35,13 +35,16 @@ fn matches_room(event: &E, filter: &RoomEventFilter) -> bool { if filter .not_rooms .iter() - .any(is_equal_to!(event.room_id().unwrap())) + .any(is_equal_to!(event.room_id().expect("event has a room ID"))) { return false; } if let Some(rooms) = filter.rooms.as_ref() { - if !rooms.iter().any(is_equal_to!(event.room_id().unwrap())) { + if !rooms + .iter() + .any(is_equal_to!(event.room_id().expect("event has a room ID"))) + { return false; } } diff --git a/src/core/matrix/pdu.rs b/src/core/matrix/pdu.rs index c7fb0d65..dcde90ff 100644 --- a/src/core/matrix/pdu.rs +++ b/src/core/matrix/pdu.rs @@ -115,12 +115,20 @@ impl Event for Pdu { #[inline] fn room_id_or_hash(&self) -> OwnedRoomId { + if *self.event_type() != TimelineEventType::RoomCreate { + return self + .room_id() + .expect("Event must have a room ID") + .to_owned(); + } if let Some(room_id) = &self.room_id { + // v1-v11 room_id.clone() } else { + // v12+ let constructed_hash = self.event_id.as_str().replace('$', "!"); RoomId::parse(&constructed_hash) - .expect("event ID can be indexed") + .expect("event ID can be parsed") .to_owned() } } @@ -180,12 +188,20 @@ impl Event for &Pdu { #[inline] fn room_id_or_hash(&self) -> OwnedRoomId { + if *self.event_type() != TimelineEventType::RoomCreate { + return self + .room_id() + .expect("Event must have a room ID") + .to_owned(); + } if let Some(room_id) = &self.room_id { + // v1-v11 room_id.clone() } else { + // v12+ let constructed_hash = self.event_id.as_str().replace('$', "!"); RoomId::parse(&constructed_hash) - .expect("event ID can be indexed") + .expect("event ID can be parsed") .to_owned() } } diff --git a/src/core/matrix/state_res/event_auth.rs b/src/core/matrix/state_res/event_auth.rs index 90902640..8ab23abd 100644 --- a/src/core/matrix/state_res/event_auth.rs +++ b/src/core/matrix/state_res/event_auth.rs @@ -5,7 +5,7 @@ use futures::{ future::{OptionFuture, join, join3}, }; use ruma::{ - Int, OwnedRoomId, OwnedUserId, RoomVersionId, UserId, + Int, OwnedUserId, RoomVersionId, UserId, events::room::{ create::RoomCreateEventContent, join_rules::{JoinRule, RoomJoinRulesEventContent}, @@ -219,9 +219,8 @@ where return Ok(false); } - // TODO(hydra): If the create event has a room_id, reject if room_version.room_ids_as_hashes && incoming_event.room_id().is_some() { - warn!("this room version does not support room IDs in m.room.create"); + warn!("room create event incorrectly claims a room ID"); return Ok(false); } @@ -239,7 +238,7 @@ where return Ok(true); } - // NOTE(hydra): We must have an event ID from this point forward. + // NOTE(hydra): We always have a room ID from this point forward. /* // TODO: In the past this code was commented as it caused problems with Synapse. This is no @@ -274,21 +273,6 @@ where ) .await; - // TODO(hydra): Re-enable { - // // Room was either v11 with no create event, or v12+ room - // if incoming_event.room_id().is_some() { - // // invalid v11 - // warn!("no m.room.create event found in claimed state"); - // return Ok(false); - // } - // // v12 room - // debug!("no m.room.create event found, assuming v12 room"); - // create_event.clone() - // }, - // | Some(e) => e, - // }; let room_create_event = create_event.clone(); // Get the content of the room create event, used later. @@ -301,14 +285,7 @@ where warn!("invalid room version found in m.room.create event"); return Ok(false); } - let expected_room_id = match room_version.room_ids_as_hashes { - // If the room version uses hashes, we replace the create event's event ID leading sigil - // with ! - | true => OwnedRoomId::try_from(room_create_event.event_id().as_str().replace('$', "!")) - .expect("Failed to convert event ID to room ID") - .clone(), - | false => room_create_event.room_id().unwrap().to_owned(), - }; + let expected_room_id = room_create_event.room_id_or_hash(); if incoming_event.room_id().unwrap() != expected_room_id { warn!( @@ -347,17 +324,6 @@ where } } - // 3. If event does not have m.room.create in auth_events reject - // removed as part of Hydra. - // TODO: reintroduce this for &'a E: Event + Send, { + fn is_creator(v: &RoomVersion, c: &BTreeSet, user_id: &UserId) -> bool { + c.contains(user_id) && v.explicitly_privilege_room_creators + } #[derive(Deserialize)] struct GetThirdPartyInvite { third_party_invite: Option>, @@ -719,7 +690,7 @@ where let user_for_join_auth_is_valid = if let Some(user_for_join_auth) = user_for_join_auth { // Is the authorised user allowed to invite users into this room - let (mut auth_user_pl, invite_level) = if let Some(pl) = &power_levels_event { + let (auth_user_pl, invite_level) = if let Some(pl) = &power_levels_event { // TODO Refactor all powerlevel parsing let invite = deserialize_power_levels_content_invite(pl.content().get(), room_version)?.invite; @@ -735,19 +706,21 @@ where } else { (int!(0), int!(0)) }; - if creators.contains(user_for_join_auth) { - auth_user_pl = Int::MAX; - } - (user_for_join_auth_membership == &MembershipState::Join) - && (auth_user_pl >= invite_level) + let user_joined = user_for_join_auth_membership == &MembershipState::Join; + let okay_power = is_creator(room_version, &creators, user_for_join_auth) + || auth_user_pl >= invite_level; + user_joined && okay_power } else { // No auth user was given false }; + let sender_creator = is_creator(room_version, &creators, sender); + let target_creator = is_creator(room_version, &creators, target_user); + Ok(match target_membership { | MembershipState::Join => { - debug!("starting target_membership=join check"); + trace!("starting target_membership=join check"); // 1. If the only previous event is an m.room.create and the state_key is the // creator, // allow @@ -759,14 +732,8 @@ where let no_more_prev_events = prev_events.next().is_none(); if prev_event_is_create_event && no_more_prev_events { - debug!("checking if sender is a room creator for initial membership event"); - let is_creator = if room_version.explicitly_privilege_room_creators { - creators.contains(target_user) && creators.contains(sender) - } else if room_version.use_room_create_sender { - let creator = create_room.sender(); - - creator == sender && creator == target_user - } else { + trace!("checking if sender is a room creator for initial membership event"); + let is_creator = (sender_creator && target_creator) || { #[allow(deprecated)] let creator = from_json_str::(create_room.content().get())? .creator @@ -779,7 +746,7 @@ where debug!("sender is room creator, allowing join"); return Ok(true); } - debug!("sender is not room creator, proceeding with normal auth checks"); + trace!("sender is not room creator, proceeding with normal auth checks"); } let membership_allows_join = matches!( target_user_current_membership, @@ -834,7 +801,7 @@ where }, | MembershipState::Invite => { // If content has third_party_invite key - debug!("starting target_membership=invite check"); + trace!("starting target_membership=invite check"); match third_party_invite.and_then(|i| i.deserialize().ok()) { | Some(tp_id) => if target_user_current_membership == MembershipState::Ban { @@ -865,9 +832,10 @@ where ); false } else { - let allow = sender_power - .filter(|&p| p >= &power_levels.invite) - .is_some(); + let allow = sender_creator + || sender_power + .filter(|&p| p >= &power_levels.invite) + .is_some(); if !allow { warn!( ?target_user_membership_event_id, @@ -895,7 +863,8 @@ where allow } else if !sender_is_joined || target_user_current_membership == MembershipState::Ban - && sender_power.filter(|&p| p < &power_levels.ban).is_some() + && (sender_creator + || sender_power.filter(|&p| p < &power_levels.ban).is_some()) { warn!( ?target_user_membership_event_id, @@ -904,8 +873,9 @@ where ); false } else { - let allow = sender_power.filter(|&p| p >= &power_levels.kick).is_some() - && target_power < sender_power; + let allow = sender_creator + || (sender_power.filter(|&p| p >= &power_levels.kick).is_some() + && target_power < sender_power); if !allow { warn!( ?target_user_membership_event_id, @@ -920,8 +890,9 @@ where warn!(?sender_membership_event_id, "Can't ban user if sender is not joined"); false } else { - let allow = sender_power.filter(|&p| p >= &power_levels.ban).is_some() - && target_power < sender_power; + let allow = sender_creator + || (sender_power.filter(|&p| p >= &power_levels.ban).is_some() + && target_power < sender_power); if !allow { warn!( ?target_user_membership_event_id, @@ -986,6 +957,7 @@ where /// Does the event have the correct userId as its state_key if it's not the "" /// state_key. fn can_send_event(event: &impl Event, ple: Option<&impl Event>, user_level: Int) -> bool { + // TODO(hydra): This function does not care about creators! let event_type_power_level = get_send_level(event.event_type(), event.state_key(), ple); debug!( @@ -1016,6 +988,7 @@ fn check_power_levels( previous_power_event: Option<&impl Event>, user_level: Int, ) -> Option { + // TODO(hydra): This function does not care about creators! match power_event.state_key() { | Some("") => {}, | Some(key) => { diff --git a/src/service/rooms/auth_chain/mod.rs b/src/service/rooms/auth_chain/mod.rs index fe15549c..729b038e 100644 --- a/src/service/rooms/auth_chain/mod.rs +++ b/src/service/rooms/auth_chain/mod.rs @@ -195,7 +195,7 @@ async fn get_auth_chain_inner( debug_error!(?event_id, ?e, "Could not find pdu mentioned in auth events"); }, | Ok(pdu) => { - if pdu.room_id.is_some() && pdu.room_id != Some(room_id.to_owned()) { + if pdu.room_id.as_ref().is_some_and(|r| r == room_id) { return Err!(Request(Forbidden(error!( ?event_id, ?room_id, diff --git a/src/service/rooms/event_handler/handle_incoming_pdu.rs b/src/service/rooms/event_handler/handle_incoming_pdu.rs index fab4d295..5299e8d4 100644 --- a/src/service/rooms/event_handler/handle_incoming_pdu.rs +++ b/src/service/rooms/event_handler/handle_incoming_pdu.rs @@ -58,8 +58,6 @@ pub async fn handle_incoming_pdu<'a>( value: BTreeMap, is_timeline_event: bool, ) -> Result> { - // TODO(hydra): Room IDs should be calculated before this function is called - assert!(!room_id.is_empty(), "room ID cannot be empty"); // 1. Skip the PDU if we already have it as a timeline event if let Ok(pdu_id) = self.services.timeline.get_pdu_id(event_id).await { return Ok(Some(pdu_id)); diff --git a/src/service/rooms/timeline/build.rs b/src/service/rooms/timeline/build.rs index 6bee3769..8c34e64c 100644 --- a/src/service/rooms/timeline/build.rs +++ b/src/service/rooms/timeline/build.rs @@ -1,6 +1,6 @@ use std::{collections::HashSet, iter::once}; -use conduwuit::{RoomVersion, debug_warn, trace}; +use conduwuit::{RoomVersion, trace}; use conduwuit_core::{ Err, Result, implement, matrix::{event::Event, pdu::PduBuilder}, @@ -38,9 +38,7 @@ pub async fn build_and_append_pdu( .await?; let room_id = pdu.room_id_or_hash(); - trace!("Checking if room {room_id} is an admin room"); if self.services.admin.is_admin_room(&room_id).await { - trace!("Room {room_id} is an admin room, checking PDU for admin room restrictions"); self.check_pdu_for_admin_room(&pdu, sender).boxed().await?; } @@ -106,7 +104,6 @@ pub async fn build_and_append_pdu( let room_features = RoomVersion::new(&content.room_version)?; if room_features.room_ids_as_hashes { // bootstrap shortid for room - debug_warn!(%room_id, "Bootstrapping shortid for room"); self.services .short .get_or_create_shortroomid(&room_id) diff --git a/src/service/rooms/timeline/create.rs b/src/service/rooms/timeline/create.rs index 9c8abe63..c161a23a 100644 --- a/src/service/rooms/timeline/create.rs +++ b/src/service/rooms/timeline/create.rs @@ -72,7 +72,6 @@ pub async fn create_hash_and_sign_event( }; let room_version = RoomVersion::new(&room_version_id).expect("room version is supported"); - // TODO(hydra): Only create events can lack a room ID. let prev_events: Vec = match room_id { | Some(room_id) => diff --git a/src/service/server_keys/get.rs b/src/service/server_keys/get.rs index 98b3ff35..e2f742a5 100644 --- a/src/service/server_keys/get.rs +++ b/src/service/server_keys/get.rs @@ -1,6 +1,6 @@ use std::borrow::Borrow; -use conduwuit::{Err, Result, debug, debug_error, implement}; +use conduwuit::{Err, Result, debug_error, implement, trace}; use ruma::{ CanonicalJsonObject, RoomVersionId, ServerName, ServerSigningKeyId, api::federation::discovery::VerifyKey, @@ -23,7 +23,7 @@ pub async fn get_event_keys( return Err!(BadServerResponse("Failed to determine keys required to verify: {e}")); }, }; - debug!(?required, "Keys required to verify event"); + trace!(?required, "Keys required to verify event"); let batch = required .iter() @@ -73,7 +73,7 @@ pub async fn get_verify_key( let notary_only = self.services.server.config.only_query_trusted_key_servers; if let Some(result) = self.verify_keys_for(origin).await.remove(key_id) { - debug!("Found key in cache"); + trace!("Found key in cache"); return Ok(result); } diff --git a/src/service/server_keys/mod.rs b/src/service/server_keys/mod.rs index 0bc06e8a..f99cf711 100644 --- a/src/service/server_keys/mod.rs +++ b/src/service/server_keys/mod.rs @@ -8,7 +8,7 @@ mod verify; use std::{collections::BTreeMap, sync::Arc, time::Duration}; use conduwuit::{ - Result, Server, debug, debug_error, debug_warn, implement, + Result, Server, debug_error, debug_warn, implement, trace, utils::{IterStream, timepoint_from_now}, }; use database::{Deserialized, Json, Map}; @@ -120,12 +120,12 @@ pub async fn required_keys_exist( ) -> bool { use ruma::signatures::required_keys; - debug!(?object, "Checking required keys exist"); + trace!(?object, "Checking required keys exist"); let Ok(required_keys) = required_keys(object, version) else { debug_error!("Failed to determine required keys"); return false; }; - debug!(?required_keys, "Required keys to verify event"); + trace!(?required_keys, "Required keys to verify event"); required_keys .iter() .flat_map(|(server, key_ids)| key_ids.iter().map(move |key_id| (server, key_id))) diff --git a/src/service/server_keys/verify.rs b/src/service/server_keys/verify.rs index f1027e54..1be38405 100644 --- a/src/service/server_keys/verify.rs +++ b/src/service/server_keys/verify.rs @@ -1,5 +1,5 @@ use conduwuit::{ - Err, Result, debug, debug_warn, implement, matrix::event::gen_event_id_canonical_json, + Err, Result, debug_warn, implement, matrix::event::gen_event_id_canonical_json, trace, }; use ruma::{ CanonicalJsonObject, CanonicalJsonValue, OwnedEventId, RoomVersionId, signatures::Verified, @@ -30,9 +30,9 @@ pub async fn validate_and_add_event_id_no_fetch( pdu: &RawJsonValue, room_version: &RoomVersionId, ) -> Result<(OwnedEventId, CanonicalJsonObject)> { - debug!(?pdu, "Validating PDU without fetching keys"); + trace!(?pdu, "Validating PDU without fetching keys"); let (event_id, mut value) = gen_event_id_canonical_json(pdu, room_version)?; - debug!(event_id = event_id.as_str(), "Generated event ID, checking required keys"); + trace!(event_id = event_id.as_str(), "Generated event ID, checking required keys"); if !self.required_keys_exist(&value, room_version).await { debug_warn!( "Event {event_id} is missing required keys, cannot verify without fetching keys" @@ -41,14 +41,14 @@ pub async fn validate_and_add_event_id_no_fetch( "Event {event_id} cannot be verified: missing keys." ))); } - debug!("All required keys exist, verifying event"); + trace!("All required keys exist, verifying event"); if let Err(e) = self.verify_event(&value, Some(room_version)).await { debug_warn!("Event verification failed"); return Err!(BadServerResponse(debug_error!( "Event {event_id} failed verification: {e:?}" ))); } - debug!("Event verified successfully"); + trace!("Event verified successfully"); value.insert("event_id".into(), CanonicalJsonValue::String(event_id.as_str().into()));