diff --git a/docs/plans/2026-03-17-space-permission-cascading-design.md b/docs/plans/2026-03-17-space-permission-cascading-design.md index bec7ec9a..2f12516f 100644 --- a/docs/plans/2026-03-17-space-permission-cascading-design.md +++ b/docs/plans/2026-03-17-space-permission-cascading-design.md @@ -117,7 +117,7 @@ For every user in a child room of a Space: ### 3. Role revocation -When an `com.continuwuity.space.role.member` event is updated and a role is removed: +When a `com.continuwuity.space.role.member` event is updated and a role is removed: - Identify all child rooms that require the removed role. - Auto-kick the user from rooms they no longer qualify for. @@ -125,14 +125,14 @@ When an `com.continuwuity.space.role.member` event is updated and a role is remo ### 4. Room requirement change -When an `com.continuwuity.space.role.room` event is updated with new requirements: +When a `com.continuwuity.space.role.room` event is updated with new requirements: - Check all current members of the room. - Auto-kick members who do not hold all newly required roles. ### 5. Auto-join on role grant -When an `com.continuwuity.space.role.member` event is updated and a role is added: +When a `com.continuwuity.space.role.member` event is updated and a role is added: - Find all child rooms where the user now meets all required roles. - Auto-join the user to qualifying rooms they are not already in. diff --git a/src/service/rooms/roles/mod.rs b/src/service/rooms/roles/mod.rs index 55b29311..2bff986a 100644 --- a/src/service/rooms/roles/mod.rs +++ b/src/service/rooms/roles/mod.rs @@ -19,7 +19,6 @@ use conduwuit::{ }; use serde_json::value::to_raw_value; use conduwuit_core::{ - Err, matrix::space_roles::{ RoleDefinition, SpaceRoleMemberEventContent, SpaceRoleRoomEventContent, SpaceRolesEventContent, @@ -250,11 +249,18 @@ pub async fn populate_space(&self, space_id: &RoomId) { // 2. Read all com.continuwuity.space.role.member state events (state key: user ID) let member_event_type = StateEventType::from("com.continuwuity.space.role.member".to_owned()); - if let Ok(shortstatehash) = self + let shortstatehash = match self .services .state .get_room_shortstatehash(space_id) .await + { + | Ok(hash) => hash, + | Err(e) => { + debug_warn!("Failed to get shortstatehash for {space_id}, cache may be stale: {e}"); + return; + }, + }; { let mut user_roles_map: HashMap> = HashMap::new(); @@ -350,9 +356,15 @@ pub async fn populate_space(&self, space_id: &RoomId) { }) .await; - // Single write lock for all children + // Single write lock for all children — clear stale entries first { let mut room_to_space = self.room_to_space.write().await; + // Remove this space from all existing entries + room_to_space.retain(|_, parents| { + parents.remove(space_id); + !parents.is_empty() + }); + // Insert fresh children for child_room_id in child_rooms { room_to_space .entry(child_room_id) @@ -434,6 +446,18 @@ pub async fn get_parent_spaces(&self, room_id: &RoomId) -> Vec { .unwrap_or_default() } +/// Get all child rooms of a Space from the reverse index. +#[implement(Service)] +pub async fn get_child_rooms(&self, space_id: &RoomId) -> Vec { + self.room_to_space + .read() + .await + .iter() + .filter(|(_, parents)| parents.contains(space_id)) + .map(|(child, _)| child.clone()) + .collect() +} + /// Synchronize power levels in a child room based on Space roles. /// This overrides per-room power levels with Space-granted levels. #[implement(Service)] @@ -476,6 +500,9 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re // 3. For each member, check their space role power level let mut changed = false; for user_id in &members { + if user_id == server_user { + continue; + } if let Some(space_pl) = self.get_user_power_level(space_id, user_id).await { let space_pl_int = Int::new_saturating(space_pl); let current_pl = power_levels_content @@ -492,11 +519,21 @@ pub async fn sync_power_levels(&self, space_id: &RoomId, room_id: &RoomId) -> Re changed = true; } } else { - // User has no space role PL — ensure they're at default - if let Some(current) = power_levels_content.users.get(user_id) { - if *current != power_levels_content.users_default { - power_levels_content.users.remove(user_id); - changed = true; + // Check if any other parent space manages this user's PL + let parents = self.get_parent_spaces(room_id).await; + let mut managed_by_other = false; + for parent in &parents { + if self.get_user_power_level(parent, user_id).await.is_some() { + managed_by_other = true; + break; + } + } + if !managed_by_other { + if let Some(current) = power_levels_content.users.get(user_id) { + if *current != power_levels_content.users_default { + power_levels_content.users.remove(user_id); + changed = true; + } } } } @@ -536,15 +573,7 @@ pub async fn auto_join_qualifying_rooms( } // Get all child rooms from the room_to_space reverse index - // Filter to children of this space - let child_rooms: Vec = self - .room_to_space - .read() - .await - .iter() - .filter(|(_, parents)| parents.contains(space_id)) - .map(|(child, _)| child.clone()) - .collect(); + let child_rooms = self.get_child_rooms(space_id).await; let server_user = self.services.globals.server_user.as_ref(); @@ -649,14 +678,7 @@ impl Service { match event_type.as_str() { | "com.continuwuity.space.roles" => { // Role definitions changed — sync PLs in all child rooms - let child_rooms: Vec = this - .room_to_space - .read() - .await - .iter() - .filter(|(_, parents)| parents.contains(&space_id)) - .map(|(child, _)| child.clone()) - .collect(); + let child_rooms = this.get_child_rooms(&space_id).await; for child_room_id in &child_rooms { if let Err(e) = this.sync_power_levels(&space_id, child_room_id).await @@ -683,14 +705,7 @@ impl Service { ); } // Sync power levels in all child rooms - let child_rooms: Vec = this - .room_to_space - .read() - .await - .iter() - .filter(|(_, parents)| parents.contains(&space_id)) - .map(|(child, _)| child.clone()) - .collect(); + let child_rooms = this.get_child_rooms(&space_id).await; for child_room_id in &child_rooms { if let Err(e) = this.sync_power_levels(&space_id, child_room_id).await @@ -895,14 +910,7 @@ impl Service { debug_warn!("Auto-join on Space join failed for {user_id}: {e}"); } // Also sync their power levels - let child_rooms: Vec = this - .room_to_space - .read() - .await - .iter() - .filter(|(_, parents)| parents.contains(&space_id)) - .map(|(child, _)| child.clone()) - .collect(); + let child_rooms = this.get_child_rooms(&space_id).await; for child_room_id in &child_rooms { if let Err(e) = this.sync_power_levels(&space_id, &child_room_id).await @@ -931,6 +939,11 @@ pub async fn kick_unqualified_from_rooms( return Ok(()); } + let server_user = self.services.globals.server_user.as_ref(); + if user_id == server_user { + return Ok(()); + } + // Get child rooms that have requirements let child_rooms: Vec = self .room_requirements @@ -940,8 +953,6 @@ pub async fn kick_unqualified_from_rooms( .map(|reqs| reqs.keys().cloned().collect()) .unwrap_or_default(); - let server_user = self.services.globals.server_user.as_ref(); - for child_room_id in &child_rooms { // Check if server user is joined to the child room if !self diff --git a/src/service/rooms/timeline/append.rs b/src/service/rooms/timeline/append.rs index 20aaa350..c6add2ff 100644 --- a/src/service/rooms/timeline/append.rs +++ b/src/service/rooms/timeline/append.rs @@ -3,7 +3,7 @@ use std::{ sync::Arc, }; -use conduwuit::{debug_warn, trace}; +use conduwuit::trace; use conduwuit_core::{ Result, err, error, implement, matrix::{ @@ -14,7 +14,7 @@ use conduwuit_core::{ }; use futures::StreamExt; use ruma::{ - CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedUserId, RoomId, RoomVersionId, UserId, + CanonicalJsonObject, CanonicalJsonValue, EventId, RoomVersionId, UserId, events::{ GlobalAccountDataEventType, StateEventType, TimelineEventType, push_rules::PushRulesEvent, @@ -327,7 +327,7 @@ where } }, | TimelineEventType::SpaceChild => - if let Some(state_key) = pdu.state_key() { + if pdu.state_key().is_some() { self.services .spaces .roomid_spacehierarchy_cache diff --git a/src/service/rooms/timeline/build.rs b/src/service/rooms/timeline/build.rs index 8fffe766..63e0f974 100644 --- a/src/service/rooms/timeline/build.rs +++ b/src/service/rooms/timeline/build.rs @@ -121,32 +121,48 @@ pub async fn build_and_append_pdu( } } - // Also check that space-managed users aren't omitted + } + + // Also check that space-managed users aren't omitted + // Clone data out of guards to avoid holding locks across await + let space_data: Vec<(ruma::OwnedRoomId, Vec<(OwnedUserId, HashSet)>, std::collections::BTreeMap)> = { let user_roles_guard = self.services.roles.user_roles.read().await; - if let Some(space_users) = user_roles_guard.get(parent_space) { - let roles_guard = self.services.roles.roles.read().await; - if let Some(role_defs) = roles_guard.get(parent_space) { - for (user_id, assigned_roles) in space_users { - let space_pl = assigned_roles - .iter() - .filter_map(|r| role_defs.get(r)?.power_level) - .max(); - if let Some(space_pl) = space_pl { - // This user should be in the proposed event - match proposed.users.get(user_id) { - | None => { - return Err!(Request(Forbidden( - "Cannot omit a user whose power level is managed by Space roles" - ))); - }, - | Some(pl) if i64::from(*pl) != space_pl => { - return Err!(Request(Forbidden( - "Cannot change power level that is set by Space roles" - ))); - }, - | _ => {}, - } - } + let roles_guard = self.services.roles.roles.read().await; + parent_spaces.iter().filter_map(|ps| { + let space_users = user_roles_guard.get(ps)?; + let role_defs = roles_guard.get(ps)?; + Some(( + ps.clone(), + space_users.iter().map(|(u, r)| (u.clone(), r.clone())).collect(), + role_defs.clone(), + )) + }).collect() + }; + // Guards dropped here + + for (_parent_space, space_users, role_defs) in &space_data { + for (user_id, assigned_roles) in space_users { + // Only enforce for users who are actually members of this room + if !self.services.state_cache.is_joined(user_id, &room_id).await { + continue; + } + let space_pl = assigned_roles + .iter() + .filter_map(|r| role_defs.get(r)?.power_level) + .max(); + if let Some(space_pl) = space_pl { + match proposed.users.get(user_id) { + | None => { + return Err!(Request(Forbidden( + "Cannot omit a user whose power level is managed by Space roles" + ))); + }, + | Some(pl) if i64::from(*pl) != space_pl => { + return Err!(Request(Forbidden( + "Cannot change power level that is set by Space roles" + ))); + }, + | _ => {}, } } }