Merge branch 'charles/cfg-admin-bot-localpart' into 'main'

allow configuring admin bot localpart

See merge request matrix/grapevine!118

src/config.rs

@@ -10,7 +10,7 @@ use std::{
 use reqwest::Url;
 use ruma::{
     api::federation::discovery::OldVerifyKey, OwnedServerName,
-    OwnedServerSigningKeyId, RoomVersionId,
+    OwnedServerSigningKeyId, RoomVersionId, UserId,
 };
 use serde::Deserialize;
 use strum::{Display, EnumIter, IntoEnumIterator};
@@ -71,6 +71,7 @@ pub(crate) struct Config {
     #[serde(default)]
     pub(crate) turn: TurnConfig,
 
+    pub(crate) admin_bot_localpart: String,
     pub(crate) emergency_password: Option<String>,
 }
 
@@ -518,5 +519,15 @@ where
         return Err(Error::RegistrationTokenEmpty);
     }
 
+    let admin_bot_id = UserId::parse(format!(
+        "@{}:{}",
+        config.admin_bot_localpart, config.server_name
+    ))
+    .map_err(Error::InvalidAdminBotId)?;
+
+    if admin_bot_id.is_historical() {
+        return Err(Error::HistoricalAdminBotLocalpart);
+    }
+
     Ok(config)
 }

src/database.rs

@@ -997,6 +997,13 @@ impl KeyValueDatabase {
                 Ok(())
             })?;
 
+            // Ensure the admin bot localpart has been written once
+            if services().globals.saved_admin_bot_localpart()?.is_none() {
+                services().globals.save_admin_bot_localpart(
+                    services().globals.admin_bot_user_id.localpart(),
+                )?;
+            }
+
             assert_eq!(
                 services().globals.database_version().unwrap(),
                 latest_database_version,
@@ -1008,6 +1015,21 @@ impl KeyValueDatabase {
                 version = latest_database_version,
                 "Loaded database",
             );
+
+            // TODO: Return this error directly instead of map_err hacks
+            services().admin.apply_admin_bot_localpart().await.map_err(
+                |e| {
+                    error!(
+                        error = %crate::error::DisplayWithSources{
+                            error: &e,
+                            infix: ": ",
+                        },
+                        "failed to apply admin bot localpart"
+                    );
+
+                    Error::BadConfig("failed to apply admin bot localpart")
+                },
+            )?;
         } else {
             services()
                 .globals

src/database/key_value/globals.rs

@@ -3,7 +3,7 @@ use futures_util::{stream::FuturesUnordered, StreamExt};
 use ruma::{
     api::federation::discovery::{OldVerifyKey, ServerSigningKeys},
     signatures::Ed25519KeyPair,
-    DeviceId, ServerName, UserId,
+    DeviceId, OwnedServerName, ServerName, UserId,
 };
 
 use crate::{
@@ -291,4 +291,50 @@ impl service::globals::Data for KeyValueDatabase {
         self.global.insert(b"version", &new_version.to_be_bytes())?;
         Ok(())
     }
+
+    fn set_server_name(&self, server_name: &ServerName) -> Result<()> {
+        self.global.insert(b"server_name", server_name.as_bytes())
+    }
+
+    fn server_name(&self) -> Result<Option<OwnedServerName>> {
+        let opt_bytes = self
+            .global
+            .get(b"server_name")
+            .map_err(|_| Error::bad_database("Failed to read from globals"))?;
+
+        // `server_name` has not been set yet
+        let Some(bytes) = opt_bytes else {
+            return Ok(None);
+        };
+
+        let utf8 = String::from_utf8(bytes)
+            .map_err(|_| Error::bad_database("Invalid UTF-8 in server_name"))?;
+
+        let server_name = OwnedServerName::try_from(utf8)
+            .map_err(|_| Error::bad_database("Invalid server_name"))?;
+
+        Ok(Some(server_name))
+    }
+
+    fn set_admin_bot_localpart(&self, localpart: &str) -> Result<()> {
+        self.global.insert(b"admin_bot_localpart", localpart.as_bytes())
+    }
+
+    fn admin_bot_localpart(&self) -> Result<Option<String>> {
+        let opt_bytes = self
+            .global
+            .get(b"admin_bot_localpart")
+            .map_err(|_| Error::bad_database("Failed to read from globals"))?;
+
+        // `admin_bot_localpart` has not been set yet
+        let Some(bytes) = opt_bytes else {
+            return Ok(None);
+        };
+
+        let localpart = String::from_utf8(bytes).map_err(|_| {
+            Error::bad_database("Invalid UTF-8 in admin_bot_localpart")
+        })?;
+
+        Ok(Some(localpart))
+    }
 }

src/error.rs

@@ -2,6 +2,7 @@
 
 use std::{fmt, iter, path::PathBuf};
 
+use ruma::{OwnedServerName, OwnedUserId};
 use thiserror::Error;
 
 use crate::config::ListenConfig;
@@ -91,14 +92,29 @@ pub(crate) enum CheckConfigCommand {
 #[allow(missing_docs)]
 #[derive(Error, Debug)]
 pub(crate) enum ServerNameChanged {
+    #[error("failed to read saved server_name")]
+    ReadSavedServerName(#[source] crate::utils::error::Error),
+
     #[error("failed to check if there are any users")]
     NonZeroUsers(#[source] crate::utils::error::Error),
 
     #[error("failed to check if the admin bot exists")]
     AdminBotExists(#[source] crate::utils::error::Error),
 
-    #[error("`server_name` in the database and config file differ")]
-    Renamed,
+    #[error(
+        "`server_name` in the database ({0}) and config file ({1}) differ"
+    )]
+    Renamed(OwnedServerName, OwnedServerName),
+
+    #[error(
+        "couldn't find {0} in the database, either the `server_name` changed \
+         or the admin bot localpart was reconfigured without running the \
+         server at least once with the original localpart"
+    )]
+    MissingAdminBot(OwnedUserId),
+
+    #[error("failed to save the configured server_name")]
+    SaveServerName(#[source] crate::utils::error::Error),
 }
 
 /// Observability initialization errors
@@ -136,6 +152,14 @@ pub(crate) enum Config {
 
     #[error("registration token must not be empty")]
     RegistrationTokenEmpty,
+
+    #[error("invalid admin bot ID")]
+    InvalidAdminBotId(#[source] ruma::IdParseError),
+
+    #[error(
+        "admin bot ID is historical (i.e. contains deprecated characters)"
+    )]
+    HistoricalAdminBotLocalpart,
 }
 
 /// Errors that can occur while searching for a config file
@@ -183,3 +207,50 @@ pub(crate) enum Serve {
     )]
     FederationSelfTestFailed(#[source] crate::Error),
 }
+
+/// Errors applying the admin bot user localpart
+// Missing docs are allowed here since that kind of information should be
+// encoded in the error messages themselves anyway.
+#[allow(missing_docs)]
+#[derive(Error, Debug)]
+pub(crate) enum ApplyAdminBotLocalpart {
+    #[error("failed to resolve the admin room ID")]
+    ResolveAdminRoomId(#[source] crate::utils::error::Error),
+
+    #[error("admin room alias does not exist")]
+    MissingAdminRoomAlias,
+
+    #[error("failed to check if {1} is a member of the room")]
+    CheckMembership(#[source] crate::utils::error::Error, OwnedUserId),
+
+    #[error("failed to read saved admin bot localpart")]
+    ReadSavedAdminBotLocalpart(#[source] crate::utils::error::Error),
+
+    #[error("failed to create {1}")]
+    CreateUser(#[source] crate::utils::error::Error, OwnedUserId),
+
+    #[error("{1} failed to invite {2} to the room")]
+    PreviousInviteNew(
+        #[source] crate::utils::error::Error,
+        OwnedUserId,
+        OwnedUserId,
+    ),
+
+    #[error("{1} failed to join the room")]
+    NewJoin(#[source] crate::utils::error::Error, OwnedUserId),
+
+    #[error("failed to get the room state")]
+    GetState(#[source] crate::utils::error::Error),
+
+    #[error("failed parse power level state")]
+    ParsePowerLevels(#[source] serde_json::Error),
+
+    #[error("{1} failed to set the room state")]
+    SetState(#[source] crate::utils::error::Error, OwnedUserId),
+
+    #[error("{1} failed to leave the room")]
+    PreviousLeave(#[source] crate::utils::error::Error, OwnedUserId),
+
+    #[error("failed to save the new admin bot localpart")]
+    SaveAdminBotLocalpart(#[source] crate::utils::error::Error),
+}

src/service/admin.rs

@@ -20,7 +20,7 @@ use ruma::{
             power_levels::RoomPowerLevelsEventContent,
             topic::RoomTopicEventContent,
         },
-        TimelineEventType,
+        StateEventType, TimelineEventType,
     },
     signatures::verify_json,
     EventId, MilliSecondsSinceUnixEpoch, OwnedMxcUri, OwnedRoomId,
@@ -1330,6 +1330,8 @@ impl Service {
 
     /// Create the admin room.
     ///
+    /// This function should only be run on a fresh database.
+    ///
     /// Users in this room are considered admins by grapevine, and the room can
     /// be used to issue admin commands by talking to the server user inside
     /// it.
@@ -1347,6 +1349,9 @@ impl Service {
             .await;
 
         services().users.create(&services().globals.admin_bot_user_id, None)?;
+        services().globals.save_admin_bot_localpart(
+            services().globals.admin_bot_user_id.localpart(),
+        )?;
 
         let room_version_id = services().globals.default_room_version();
         let room_version = RoomVersion::try_from(&room_version_id)?;
@@ -1570,6 +1575,200 @@ impl Service {
         Ok(())
     }
 
+    #[allow(clippy::too_many_lines)]
+    pub(crate) async fn apply_admin_bot_localpart(
+        &self,
+    ) -> Result<(), crate::error::ApplyAdminBotLocalpart> {
+        use crate::error::ApplyAdminBotLocalpart as Error;
+
+        let cur_user_id = &services().globals.admin_bot_user_id;
+
+        let admin_room_id = self
+            .get_admin_room()
+            .map_err(Error::ResolveAdminRoomId)?
+            .ok_or(Error::MissingAdminRoomAlias)?;
+
+        let cur_in_room = services()
+            .rooms
+            .state_cache
+            .is_joined(cur_user_id, &admin_room_id)
+            .map_err(|e| Error::CheckMembership(e, cur_user_id.clone()))?;
+
+        let previous_user_id = {
+            let localpart = services()
+                .globals
+                .saved_admin_bot_localpart()
+                .map_err(Error::ReadSavedAdminBotLocalpart)?
+                .expect("admin bot localpart should have been saved by now");
+
+            let server_name = services().globals.server_name();
+
+            UserId::parse(format!("@{localpart}:{server_name}")).expect(
+                "localpart and server_name should have been validated by now",
+            )
+        };
+
+        if cur_in_room {
+            // Don't need to make any changes
+            return Ok(());
+        }
+
+        // Rename variable because now we know it's a new value
+        let new_user_id = cur_user_id;
+
+        // Create new
+        services()
+            .users
+            .create(new_user_id, None)
+            .map_err(|e| Error::CreateUser(e, new_user_id.clone()))?;
+
+        // Acquire lock
+        let room_token = services()
+            .globals
+            .roomid_mutex_state
+            .lock_key(admin_room_id.clone())
+            .await;
+
+        // Make previous invite new
+        services()
+            .rooms
+            .timeline
+            .build_and_append_pdu(
+                PduBuilder {
+                    event_type: TimelineEventType::RoomMember,
+                    content: to_raw_value(&RoomMemberEventContent {
+                        membership: MembershipState::Invite,
+                        displayname: None,
+                        avatar_url: None,
+                        is_direct: None,
+                        third_party_invite: None,
+                        blurhash: None,
+                        reason: None,
+                        join_authorized_via_users_server: None,
+                    })
+                    .expect("event is valid, we just created it"),
+                    unsigned: None,
+                    state_key: Some(new_user_id.to_string()),
+                    redacts: None,
+                },
+                &previous_user_id,
+                &room_token,
+            )
+            .await
+            .map_err(|e| {
+                Error::PreviousInviteNew(
+                    e,
+                    previous_user_id.clone(),
+                    new_user_id.clone(),
+                )
+            })?;
+
+        // Make new join
+        services()
+            .rooms
+            .timeline
+            .build_and_append_pdu(
+                PduBuilder {
+                    event_type: TimelineEventType::RoomMember,
+                    content: to_raw_value(&RoomMemberEventContent {
+                        membership: MembershipState::Join,
+                        displayname: None,
+                        avatar_url: None,
+                        is_direct: None,
+                        third_party_invite: None,
+                        blurhash: None,
+                        reason: None,
+                        join_authorized_via_users_server: None,
+                    })
+                    .expect("event is valid, we just created it"),
+                    unsigned: None,
+                    state_key: Some(new_user_id.to_string()),
+                    redacts: None,
+                },
+                new_user_id,
+                &room_token,
+            )
+            .await
+            .map_err(|e| Error::NewJoin(e, new_user_id.clone()))?;
+
+        // Get current power_levels
+        let mut power_levels: RoomPowerLevelsEventContent = services()
+            .rooms
+            .state_accessor
+            .room_state_get(
+                &admin_room_id,
+                &StateEventType::RoomPowerLevels,
+                "",
+            )
+            .map_err(Error::GetState)?
+            .map(|ev| serde_json::from_str(ev.content.get()))
+            .transpose()
+            .map_err(Error::ParsePowerLevels)?
+            .unwrap_or_default();
+
+        // Make previous promote new and demote self
+        services()
+            .rooms
+            .timeline
+            .build_and_append_pdu(
+                PduBuilder {
+                    event_type: TimelineEventType::RoomPowerLevels,
+                    content: to_raw_value({
+                        power_levels.users.remove(&previous_user_id);
+                        power_levels
+                            .users
+                            .insert(new_user_id.clone(), 100.into());
+
+                        &power_levels
+                    })
+                    .expect("event is valid, we just created it"),
+                    unsigned: None,
+                    state_key: Some(String::new()),
+                    redacts: None,
+                },
+                &previous_user_id,
+                &room_token,
+            )
+            .await
+            .map_err(|e| Error::SetState(e, previous_user_id.clone()))?;
+
+        // Make previous leave
+        services()
+            .rooms
+            .timeline
+            .build_and_append_pdu(
+                PduBuilder {
+                    event_type: TimelineEventType::RoomMember,
+                    content: to_raw_value(&RoomMemberEventContent {
+                        membership: MembershipState::Leave,
+                        displayname: None,
+                        avatar_url: None,
+                        is_direct: None,
+                        third_party_invite: None,
+                        blurhash: None,
+                        reason: None,
+                        join_authorized_via_users_server: None,
+                    })
+                    .expect("event is valid, we just created it"),
+                    unsigned: None,
+                    state_key: Some(previous_user_id.to_string()),
+                    redacts: None,
+                },
+                &previous_user_id,
+                &room_token,
+            )
+            .await
+            .map_err(|e| Error::PreviousLeave(e, previous_user_id.clone()))?;
+
+        // Save the new localpart
+        services()
+            .globals
+            .save_admin_bot_localpart(new_user_id.localpart())
+            .map_err(Error::SaveAdminBotLocalpart)?;
+
+        Ok(())
+    }
+
     /// Gets the room ID of the admin room
     ///
     /// Errors are propagated from the database, and will have None if there is

src/service/globals.rs

@@ -236,14 +236,9 @@ impl Service {
 
         let admin_bot_user_id = UserId::parse(format!(
             "@{}:{}",
-            if config.conduit_compat {
-                "conduit"
-            } else {
-                "grapevine"
-            },
-            config.server_name,
+            config.admin_bot_localpart, config.server_name,
         ))
-        .expect("admin bot user ID should be valid");
+        .map_err(|_| Error::BadConfig("Invalid admin bot localpart"))?;
 
         let admin_bot_room_alias_id =
             RoomAliasId::parse(format!("#admins:{}", config.server_name))
@@ -309,6 +304,10 @@ impl Service {
 
     /// Check if `server_name` in the DB and config differ, return error if so
     ///
+    /// This function will save the currently configured `server_name` if the
+    /// check passes, so that future calls to this function will continue to
+    /// check against the first-configured value.
+    ///
     /// Matrix resource ownership is based on the server name; changing it
     /// requires recreating the database from scratch. This check needs to be
     /// done before background tasks are started to avoid data races.
@@ -319,22 +318,43 @@ impl Service {
     ) -> Result<(), crate::error::ServerNameChanged> {
         use crate::error::ServerNameChanged as Error;
 
-        if services()
+        let config = &*services().globals.config.server_name;
+
+        let opt_saved =
+            self.saved_server_name().map_err(Error::ReadSavedServerName)?;
+
+        // Check against saved server name
+        if let Some(saved) = opt_saved {
+            if saved == config {
+                return Ok(());
+            }
+
+            return Err(Error::Renamed(saved.clone(), config.to_owned()));
+        }
+
+        let non_zero_users = services()
             .users
             .count()
             .map(|x| x > 0)
-            .map_err(Error::NonZeroUsers)?
-        {
-            let admin_bot = self.admin_bot_user_id.as_ref();
-            if !services()
-                .users
-                .exists(admin_bot)
-                .map_err(Error::AdminBotExists)?
-            {
-                return Err(Error::Renamed);
-            }
+            .map_err(Error::NonZeroUsers)?;
+
+        let admin_bot_exists = services()
+            .users
+            .exists(&self.admin_bot_user_id)
+            .map_err(Error::AdminBotExists)?;
+
+        // Fall back to checking against the admin bot user ID
+        if non_zero_users && !admin_bot_exists {
+            return Err(Error::MissingAdminBot(self.admin_bot_user_id.clone()));
         }
 
+        // If the server_name wasn't saved and the admin bot user ID check
+        // didn't fail, save the current server_name
+        services()
+            .globals
+            .save_server_name(config)
+            .map_err(Error::SaveServerName)?;
+
         Ok(())
     }
 
@@ -621,6 +641,29 @@ impl Service {
         self.shutdown.store(true, atomic::Ordering::Relaxed);
         self.rotate.fire();
     }
+
+    pub(crate) fn save_server_name(
+        &self,
+        server_name: &ServerName,
+    ) -> Result<()> {
+        self.db.set_server_name(server_name)
+    }
+
+    // Named this way to avoid conflicts with the existing `fn server_name`
+    pub(crate) fn saved_server_name(&self) -> Result<Option<OwnedServerName>> {
+        self.db.server_name()
+    }
+
+    pub(crate) fn save_admin_bot_localpart(
+        &self,
+        localpart: &str,
+    ) -> Result<()> {
+        self.db.set_admin_bot_localpart(localpart)
+    }
+
+    pub(crate) fn saved_admin_bot_localpart(&self) -> Result<Option<String>> {
+        self.db.admin_bot_localpart()
+    }
 }
 
 fn reqwest_client_builder(config: &Config) -> Result<reqwest::ClientBuilder> {

src/service/globals/data.rs

@@ -8,7 +8,7 @@ use ruma::{
     api::federation::discovery::{OldVerifyKey, ServerSigningKeys, VerifyKey},
     serde::Base64,
     signatures::Ed25519KeyPair,
-    DeviceId, MilliSecondsSinceUnixEpoch, ServerName, UserId,
+    DeviceId, MilliSecondsSinceUnixEpoch, OwnedServerName, ServerName, UserId,
 };
 use serde::Deserialize;
 
@@ -117,4 +117,8 @@ pub(crate) trait Data: Send + Sync {
     ) -> Result<Option<SigningKeys>>;
     fn database_version(&self) -> Result<u64>;
     fn bump_database_version(&self, new_version: u64) -> Result<()>;
+    fn set_server_name(&self, server_name: &ServerName) -> Result<()>;
+    fn server_name(&self) -> Result<Option<OwnedServerName>>;
+    fn set_admin_bot_localpart(&self, localpart: &str) -> Result<()>;
+    fn admin_bot_localpart(&self) -> Result<Option<String>>;
 }

src/service/rooms/timeline.rs

@@ -957,114 +957,14 @@ impl Service {
         let (pdu, pdu_json) =
             self.create_hash_and_sign_event(pdu_builder, sender, room_id)?;
 
-        if let Some(admin_room) = services().admin.get_admin_room()? {
-            if admin_room == **room_id {
-                match pdu.event_type() {
-                    TimelineEventType::RoomEncryption => {
-                        warn!("Encryption is not allowed in the admins room");
-                        return Err(Error::BadRequest(
-                            ErrorKind::forbidden(),
-                            "Encryption is not allowed in the admins room.",
-                        ));
-                    }
-                    TimelineEventType::RoomMember => {
-                        #[derive(Deserialize)]
-                        struct ExtractMembership {
-                            membership: MembershipState,
-                        }
-
-                        let target = pdu
-                            .state_key()
-                            .filter(|v| v.starts_with('@'))
-                            .unwrap_or(sender.as_str());
-                        let server_name = services().globals.server_name();
-                        let server_user = format!(
-                            "@{}:{server_name}",
-                            if services().globals.config.conduit_compat {
-                                "conduit"
-                            } else {
-                                "grapevine"
-                            },
-                        );
-                        let content =
-                            serde_json::from_str::<ExtractMembership>(
-                                pdu.content.get(),
-                            )
-                            .map_err(|_| {
-                                Error::bad_database("Invalid content in pdu.")
-                            })?;
-
-                        if content.membership == MembershipState::Leave {
-                            if target == server_user {
-                                warn!(
-                                    "Grapevine user cannot leave from admins \
-                                     room"
-                                );
-                                return Err(Error::BadRequest(
-                                    ErrorKind::forbidden(),
-                                    "Grapevine user cannot leave from admins \
-                                     room.",
-                                ));
-                            }
-
-                            let count = services()
-                                .rooms
-                                .state_cache
-                                .room_members(room_id)
-                                .filter_map(Result::ok)
-                                .filter(|m| m.server_name() == server_name)
-                                .filter(|m| m != target)
-                                .count();
-                            if count < 2 {
-                                warn!(
-                                    "Last admin cannot leave from admins room"
-                                );
-                                return Err(Error::BadRequest(
-                                    ErrorKind::forbidden(),
-                                    "Last admin cannot leave from admins room.",
-                                ));
-                            }
-                        }
-
-                        if content.membership == MembershipState::Ban
-                            && pdu.state_key().is_some()
-                        {
-                            if target == server_user {
-                                warn!(
-                                    "Grapevine user cannot be banned in \
-                                     admins room"
-                                );
-                                return Err(Error::BadRequest(
-                                    ErrorKind::forbidden(),
-                                    "Grapevine user cannot be banned in \
-                                     admins room.",
-                                ));
-                            }
-
-                            let count = services()
-                                .rooms
-                                .state_cache
-                                .room_members(room_id)
-                                .filter_map(Result::ok)
-                                .filter(|m| m.server_name() == server_name)
-                                .filter(|m| m != target)
-                                .count();
-                            if count < 2 {
-                                warn!(
-                                    "Last admin cannot be banned in admins \
-                                     room"
-                                );
-                                return Err(Error::BadRequest(
-                                    ErrorKind::forbidden(),
-                                    "Last admin cannot be banned in admins \
-                                     room.",
-                                ));
-                            }
-                        }
-                    }
-                    _ => {}
-                }
-            }
+        if services().admin.get_admin_room()?.is_some_and(|x| x == **room_id)
+            && pdu.event_type() == &TimelineEventType::RoomEncryption
+        {
+            warn!("Encryption is not allowed in the admins room");
+            return Err(Error::BadRequest(
+                ErrorKind::forbidden(),
+                "Encryption is not allowed in the admins room.",
+            ));
         }
 
         // If redaction event is not authorized, do not append it to the