fix: permission checks for aliases

src/api/client_server/alias.rs

@@ -18,6 +18,8 @@ use ruma::{
 pub async fn create_alias_route(
     body: Ruma<create_alias::v3::Request>,
 ) -> Result<create_alias::v3::Response> {
+    let sender_user = body.sender_user.as_ref().expect("user is authenticated");
+
     if body.room_alias.server_name() != services().globals.server_name() {
         return Err(Error::BadRequest(
             ErrorKind::InvalidParam,
@@ -55,7 +57,7 @@ pub async fn create_alias_route(
     services()
         .rooms
         .alias
-        .set_alias(&body.room_alias, &body.room_id)?;
+        .set_alias(&body.room_alias, &body.room_id, sender_user)?;
 
     Ok(create_alias::v3::Response::new())
 }
@@ -64,11 +66,12 @@ pub async fn create_alias_route(
 ///
 /// Deletes a room alias from this server.
 ///
-/// - TODO: additional access control checks
 /// - TODO: Update canonical alias event
 pub async fn delete_alias_route(
     body: Ruma<delete_alias::v3::Request>,
 ) -> Result<delete_alias::v3::Response> {
+    let sender_user = body.sender_user.as_ref().expect("user is authenticated");
+
     if body.room_alias.server_name() != services().globals.server_name() {
         return Err(Error::BadRequest(
             ErrorKind::InvalidParam,
@@ -94,7 +97,10 @@ pub async fn delete_alias_route(
         ));
     }
 
-    services().rooms.alias.remove_alias(&body.room_alias)?;
+    services()
+        .rooms
+        .alias
+        .remove_alias(&body.room_alias, sender_user)?;
 
     // TODO: update alt_aliases?
 

src/api/client_server/room.rs

@@ -485,7 +485,10 @@ pub async fn create_room_route(
 
     // Homeserver specific stuff
     if let Some(alias) = alias {
-        services().rooms.alias.set_alias(&alias, &room_id)?;
+        services()
+            .rooms
+            .alias
+            .set_alias(&alias, &room_id, sender_user)?;
     }
 
     if body.visibility == room::Visibility::Public {
@@ -815,7 +818,7 @@ pub async fn upgrade_room_route(
         services()
             .rooms
             .alias
-            .set_alias(&alias, &replacement_room)?;
+            .set_alias(&alias, &replacement_room, sender_user)?;
     }
 
     // Get the old room power levels

src/database/key_value/rooms/alias.rs

@@ -1,9 +1,15 @@
-use ruma::{api::client::error::ErrorKind, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId};
+use ruma::{
+    api::client::error::ErrorKind, OwnedRoomAliasId, OwnedRoomId, OwnedUserId, RoomAliasId, RoomId,
+    UserId,
+};
 
 use crate::{database::KeyValueDatabase, service, services, utils, Error, Result};
 
 impl service::rooms::alias::Data for KeyValueDatabase {
-    fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId) -> Result<()> {
+    fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId, user_id: &UserId) -> Result<()> {
+        // Comes first as we don't want a stuck alias
+        self.alias_userid
+            .insert(alias.alias().as_bytes(), user_id.as_bytes())?;
         self.alias_roomid
             .insert(alias.alias().as_bytes(), room_id.as_bytes())?;
         let mut aliasid = room_id.as_bytes().to_vec();
@@ -22,13 +28,13 @@ impl service::rooms::alias::Data for KeyValueDatabase {
                 self.aliasid_alias.remove(&key)?;
             }
             self.alias_roomid.remove(alias.alias().as_bytes())?;
+            self.alias_userid.remove(alias.alias().as_bytes())
         } else {
-            return Err(Error::BadRequest(
+            Err(Error::BadRequest(
                 ErrorKind::NotFound,
                 "Alias does not exist.",
-            ));
+            ))
         }
-        Ok(())
     }
 
     fn resolve_local_alias(&self, alias: &RoomAliasId) -> Result<Option<OwnedRoomId>> {
@@ -57,4 +63,16 @@ impl service::rooms::alias::Data for KeyValueDatabase {
                 .map_err(|_| Error::bad_database("Invalid alias in aliasid_alias."))
         }))
     }
+
+    fn who_created_alias(&self, alias: &RoomAliasId) -> Result<Option<OwnedUserId>> {
+        self.alias_userid
+            .get(alias.alias().as_bytes())?
+            .map(|bytes| {
+                UserId::parse(utils::string_from_bytes(&bytes).map_err(|_| {
+                    Error::bad_database("User ID in alias_userid is invalid unicode.")
+                })?)
+                .map_err(|_| Error::bad_database("User ID in alias_roomid is invalid."))
+            })
+            .transpose()
+    }
 }

src/database/mod.rs

@@ -101,6 +101,8 @@ pub struct KeyValueDatabase {
     pub(super) userroomid_leftstate: Arc<dyn KvTree>,
     pub(super) roomuserid_leftcount: Arc<dyn KvTree>,
 
+    pub(super) alias_userid: Arc<dyn KvTree>, // User who created the alias
+
     pub(super) disabledroomids: Arc<dyn KvTree>, // Rooms where incoming federation handling is disabled
 
     pub(super) lazyloadedids: Arc<dyn KvTree>, // LazyLoadedIds = UserId + DeviceId + RoomId + LazyLoadedUserId
@@ -327,6 +329,8 @@ impl KeyValueDatabase {
             userroomid_leftstate: builder.open_tree("userroomid_leftstate")?,
             roomuserid_leftcount: builder.open_tree("roomuserid_leftcount")?,
 
+            alias_userid: builder.open_tree("alias_userid")?,
+
             disabledroomids: builder.open_tree("disabledroomids")?,
 
             lazyloadedids: builder.open_tree("lazyloadedids")?,

src/service/admin/mod.rs

@@ -1,9 +1,4 @@
-use std::{
+use std::{collections::BTreeMap, convert::TryFrom, sync::Arc, time::Instant};
-    collections::BTreeMap,
-    convert::{TryFrom, TryInto},
-    sync::Arc,
-    time::Instant,
-};
 
 use clap::Parser;
 use regex::Regex;
@@ -925,7 +920,15 @@ impl Service {
                 {
                     RoomMessageEventContent::text_plain("No such alias exists")
                 } else {
-                    services().rooms.alias.remove_alias(&alias)?;
+                    // We execute this as the server user for two reasons
+                    // 1. If the user can execute commands in the admin room, they can always remove the alias.
+                    // 2. In the future, we are likely going to be able to allow users to execute commands via
+                    //   other methods, such as IPC, which would lead to us not knowing their user id
+
+                    services()
+                        .rooms
+                        .alias
+                        .remove_alias(&alias, services().globals.server_user())?;
                     RoomMessageEventContent::text_plain("Alias removed sucessfully")
                 }
             }
@@ -1232,9 +1235,7 @@ impl Service {
             .await?;
 
         // 6. Room alias
-        let alias: OwnedRoomAliasId = format!("#admins:{}", services().globals.server_name())
+        let alias: OwnedRoomAliasId = services().globals.admin_alias().to_owned();
-            .try_into()
-            .expect("#admins:server_name is a valid alias name");
 
         services()
             .rooms
@@ -1257,7 +1258,10 @@ impl Service {
             )
             .await?;
 
-        services().rooms.alias.set_alias(&alias, &room_id)?;
+        services()
+            .rooms
+            .alias
+            .set_alias(&alias, &room_id, conduit_user)?;
 
         Ok(())
     }
@@ -1266,15 +1270,10 @@ impl Service {
     ///
     /// Errors are propagated from the database, and will have None if there is no admin room
     pub(crate) fn get_admin_room(&self) -> Result<Option<OwnedRoomId>> {
-        let admin_room_alias: Box<RoomAliasId> =
-            format!("#admins:{}", services().globals.server_name())
-                .try_into()
-                .expect("#admins:server_name is a valid alias name");
-
         services()
             .rooms
             .alias
-            .resolve_local_alias(&admin_room_alias)
+            .resolve_local_alias(services().globals.admin_alias())
     }
 
     /// Invite the user to the conduit admin room.
@@ -1400,6 +1399,15 @@ impl Service {
         }
         Ok(())
     }
+
+    /// Checks whether a given user is an admin of this server
+    pub fn user_is_admin(&self, user_id: &UserId) -> Result<bool> {
+        let Some(admin_room) = self.get_admin_room()? else {
+            return Ok(false);
+        };
+
+        services().rooms.state_cache.is_joined(user_id, &admin_room)
+    }
 }
 
 #[cfg(test)]

src/service/globals/mod.rs

@@ -4,6 +4,7 @@ use ruma::{
     serde::Base64, OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedServerName,
     OwnedServerSigningKeyId, OwnedUserId,
 };
+use ruma::{OwnedRoomAliasId, RoomAliasId};
 
 use crate::api::server_server::FedDest;
 
@@ -72,6 +73,7 @@ pub struct Service {
     pub roomid_mutex_federation: RwLock<HashMap<OwnedRoomId, Arc<Mutex<()>>>>, // this lock will be held longer
     pub roomid_federationhandletime: RwLock<HashMap<OwnedRoomId, (OwnedEventId, Instant)>>,
     server_user: OwnedUserId,
+    admin_alias: OwnedRoomAliasId,
     pub stateres_mutex: Arc<Mutex<()>>,
     pub rotate: RotationHandler,
 
@@ -194,6 +196,8 @@ impl Service {
 
         let mut s = Self {
             allow_registration: RwLock::new(config.allow_registration),
+            admin_alias: RoomAliasId::parse(format!("#admins:{}", &config.server_name))
+                .expect("#admins:server_name is a valid alias name"),
             server_user: UserId::parse(format!("@conduit:{}", &config.server_name))
                 .expect("@conduit:server_name is valid"),
             db,
@@ -293,6 +297,10 @@ impl Service {
         self.server_user.as_ref()
     }
 
+    pub fn admin_alias(&self) -> &RoomAliasId {
+        self.admin_alias.as_ref()
+    }
+
     pub fn max_request_size(&self) -> u32 {
         self.config.max_request_size
     }

src/service/rooms/alias/data.rs

@@ -1,9 +1,12 @@
 use crate::Result;
-use ruma::{OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId};
+use ruma::{OwnedRoomAliasId, OwnedRoomId, OwnedUserId, RoomAliasId, RoomId, UserId};
 
 pub trait Data: Send + Sync {
     /// Creates or updates the alias to the given room id.
-    fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId) -> Result<()>;
+    fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId, user_id: &UserId) -> Result<()>;
+
+    /// Finds the user who assigned the given alias to a room
+    fn who_created_alias(&self, alias: &RoomAliasId) -> Result<Option<OwnedUserId>>;
 
     /// Forgets about an alias. Returns an error if the alias did not exist.
     fn remove_alias(&self, alias: &RoomAliasId) -> Result<()>;

src/service/rooms/alias/mod.rs

@@ -1,9 +1,17 @@
 mod data;
 
 pub use data::Data;
+use tracing::error;
 
-use crate::Result;
+use crate::{services, Error, Result};
-use ruma::{OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId};
+use ruma::{
+    api::client::error::ErrorKind,
+    events::{
+        room::power_levels::{RoomPowerLevels, RoomPowerLevelsEventContent},
+        StateEventType,
+    },
+    OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId, UserId,
+};
 
 pub struct Service {
     pub db: &'static dyn Data,
@@ -11,13 +19,71 @@ pub struct Service {
 
 impl Service {
     #[tracing::instrument(skip(self))]
-    pub fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId) -> Result<()> {
+    pub fn set_alias(&self, alias: &RoomAliasId, room_id: &RoomId, user_id: &UserId) -> Result<()> {
-        self.db.set_alias(alias, room_id)
+        if alias == services().globals.admin_alias() && user_id != services().globals.server_user()
+        {
+            Err(Error::BadRequest(
+                ErrorKind::forbidden(),
+                "Only the server user can set this alias",
+            ))
+        } else {
+            self.db.set_alias(alias, room_id, user_id)
+        }
     }
 
     #[tracing::instrument(skip(self))]
-    pub fn remove_alias(&self, alias: &RoomAliasId) -> Result<()> {
+    fn user_can_remove_alias(&self, alias: &RoomAliasId, user_id: &UserId) -> Result<bool> {
-        self.db.remove_alias(alias)
+        let Some(room_id) = self.resolve_local_alias(alias)? else {
+            return Err(Error::BadRequest(ErrorKind::NotFound, "Alias not found."));
+        };
+
+        // The creator of an alias can remove it
+        if self
+            .db
+            .who_created_alias(alias)?
+            .map(|user| user == user_id)
+            .unwrap_or_default()
+            // Server admins can remove any local alias
+            || services().admin.user_is_admin(user_id)?
+            // Always allow the Conduit user to remove the alias, since there may not be an admin room
+            || services().globals.server_user ()== user_id
+        {
+            Ok(true)
+            // Checking whether the user is able to change canonical aliases of the room
+        } else if let Some(event) = services().rooms.state_accessor.room_state_get(
+            &room_id,
+            &StateEventType::RoomPowerLevels,
+            "",
+        )? {
+            serde_json::from_str(event.content.get())
+                .map_err(|_| Error::bad_database("Invalid event content for m.room.power_levels"))
+                .map(|content: RoomPowerLevelsEventContent| {
+                    RoomPowerLevels::from(content)
+                        .user_can_send_state(user_id, StateEventType::RoomCanonicalAlias)
+                })
+        // If there is no power levels event, only the room creator can change canonical aliases
+        } else if let Some(event) = services().rooms.state_accessor.room_state_get(
+            &room_id,
+            &StateEventType::RoomCreate,
+            "",
+        )? {
+            Ok(event.sender == user_id)
+        } else {
+            error!("Room {} has no m.room.create event (VERY BAD)!", room_id);
+            Err(Error::bad_database("Room has no m.room.create event"))
+        }
+    }
+
+    #[tracing::instrument(skip(self))]
+    pub fn remove_alias(&self, alias: &RoomAliasId, user_id: &UserId) -> Result<()> {
+        if self.user_can_remove_alias(alias, user_id)? {
+            self.db.remove_alias(alias)
+        } else {
+            Err(Error::BadRequest(
+                ErrorKind::forbidden(),
+                "User is not permitted to remove this alias.",
+            ))
+        }
     }
 
     #[tracing::instrument(skip(self))]

src/service/rooms/timeline/mod.rs

@@ -496,7 +496,14 @@ impl Service {
                         && services().globals.emergency_password().is_none();
 
                     if let Some(admin_room) = services().admin.get_admin_room()? {
-                        if to_conduit && !from_conduit && admin_room == pdu.room_id {
+                        if to_conduit
+                            && !from_conduit
+                            && admin_room == pdu.room_id
+                            && services()
+                                .rooms
+                                .state_cache
+                                .is_joined(server_user, &admin_room)?
+                        {
                             services().admin.process_message(body);
                         }
                     }

src/service/users/mod.rs

@@ -9,7 +9,6 @@ pub use data::Data;
 use ruma::{
     api::client::{
         device::Device,
-        error::ErrorKind,
         filter::FilterDefinition,
         sync::sync_events::{
             self,
@@ -20,7 +19,7 @@ use ruma::{
     events::AnyToDeviceEvent,
     serde::Raw,
     DeviceId, DeviceKeyAlgorithm, DeviceKeyId, OwnedDeviceId, OwnedDeviceKeyId, OwnedMxcUri,
-    OwnedRoomId, OwnedUserId, RoomAliasId, UInt, UserId,
+    OwnedRoomId, OwnedUserId, UInt, UserId,
 };
 
 use crate::{services, Error, Result};
@@ -262,19 +261,14 @@ impl Service {
 
     /// Check if a user is an admin
     pub fn is_admin(&self, user_id: &UserId) -> Result<bool> {
-        let admin_room_alias_id =
+        if let Some(admin_room_id) = services().admin.get_admin_room()? {
-            RoomAliasId::parse(format!("#admins:{}", services().globals.server_name()))
+            services()
-                .map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Invalid alias."))?;
+                .rooms
-        let admin_room_id = services()
+                .state_cache
-            .rooms
+                .is_joined(user_id, &admin_room_id)
-            .alias
+        } else {
-            .resolve_local_alias(&admin_room_alias_id)?
+            Ok(false)
-            .unwrap();
+        }
-
-        services()
-            .rooms
-            .state_cache
-            .is_joined(user_id, &admin_room_id)
     }
 
     /// Create a new user account on this homeserver.