Merge branch 'dont-give-guests-admin' into 'next'

fix(accounts): don't give guests admin

See merge request famedly/conduit!591

src/api/client_server/account.rs

@@ -239,7 +239,14 @@ pub async fn register_route(body: Ruma<register::v3::Request>) -> Result<registe
 
     // If this is the first real user, grant them admin privileges
     // Note: the server user, @conduit:servername, is generated first
-    if services().users.count()? == 2 {
+    if !is_guest {
+        if let Some(admin_room) = services().admin.get_admin_room()? {
+            if services()
+                .rooms
+                .state_cache
+                .room_joined_count(&admin_room)?
+                == Some(1)
+            {
                 services()
                     .admin
                     .make_user_admin(&user_id, displayname)
@@ -247,6 +254,8 @@ pub async fn register_route(body: Ruma<register::v3::Request>) -> Result<registe
 
                 warn!("Granting {} admin privileges as the first user", user_id);
             }
+        }
+    }
 
     Ok(register::v3::Response {
         access_token: Some(token),

src/service/admin/mod.rs

@@ -23,7 +23,7 @@ use ruma::{
         },
         TimelineEventType,
     },
-    EventId, OwnedRoomAliasId, RoomAliasId, RoomId, RoomVersionId, ServerName, UserId,
+    EventId, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId, RoomVersionId, ServerName, UserId,
 };
 use serde_json::value::to_raw_value;
 use tokio::sync::{mpsc, Mutex, MutexGuard};
@@ -214,19 +214,9 @@ impl Service {
         let conduit_user = UserId::parse(format!("@conduit:{}", services().globals.server_name()))
             .expect("@conduit:server_name is valid");
 
-        let conduit_room = services()
+        if let Ok(Some(conduit_room)) = services().admin.get_admin_room() {
-            .rooms
+            let send_message = |message: RoomMessageEventContent,
-            .alias
+                                mutex_lock: &MutexGuard<'_, ()>| {
-            .resolve_local_alias(
-                format!("#admins:{}", services().globals.server_name())
-                    .as_str()
-                    .try_into()
-                    .expect("#admins:server_name is a valid room alias"),
-            )
-            .expect("Database data for admin room alias must be valid")
-            .expect("Admin room must exist");
-
-        let send_message = |message: RoomMessageEventContent, mutex_lock: &MutexGuard<'_, ()>| {
                 services()
                     .rooms
                     .timeline
@@ -272,6 +262,7 @@ impl Service {
                 }
             }
         }
+    }
 
     pub fn process_message(&self, room_message: String) {
         self.sender
@@ -1105,6 +1096,21 @@ impl Service {
         Ok(())
     }
 
+    /// Gets the room ID of the admin room
+    ///
+    /// 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)
+    }
+
     /// Invite the user to the conduit admin room.
     ///
     /// In conduit, this is equivalent to granting admin privileges.
@@ -1113,16 +1119,7 @@ impl Service {
         user_id: &UserId,
         displayname: String,
     ) -> Result<()> {
-        let admin_room_alias: Box<RoomAliasId> =
+        if let Some(room_id) = services().admin.get_admin_room()? {
-            format!("#admins:{}", services().globals.server_name())
-                .try_into()
-                .expect("#admins:server_name is a valid alias name");
-        let room_id = services()
-            .rooms
-            .alias
-            .resolve_local_alias(&admin_room_alias)?
-            .expect("Admin room must exist");
-
             let mutex_state = Arc::clone(
                 services()
                     .globals
@@ -1224,7 +1221,7 @@ impl Service {
             &room_id,
             &state_lock,
         )?;
-
+        }
         Ok(())
     }
 }

src/service/rooms/timeline/mod.rs

@@ -28,7 +28,7 @@ use ruma::{
     state_res,
     state_res::{Event, RoomVersion},
     uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedEventId, OwnedRoomId,
-    OwnedServerName, RoomAliasId, RoomId, ServerName, UserId,
+    OwnedServerName, RoomId, ServerName, UserId,
 };
 use serde::Deserialize;
 use serde_json::value::{to_raw_value, RawValue as RawJsonValue};
@@ -448,12 +448,6 @@ impl Service {
                         .search
                         .index_pdu(shortroomid, &pdu_id, &body)?;
 
-                    let admin_room = services().rooms.alias.resolve_local_alias(
-                        <&RoomAliasId>::try_from(
-                            format!("#admins:{}", services().globals.server_name()).as_str(),
-                        )
-                        .expect("#admins:server_name is a valid room alias"),
-                    )?;
                     let server_user = format!("@conduit:{}", services().globals.server_name());
 
                     let to_conduit = body.starts_with(&format!("{server_user}: "))
@@ -466,11 +460,13 @@ impl Service {
                     let from_conduit = pdu.sender == server_user
                         && services().globals.emergency_password().is_none();
 
-                    if to_conduit && !from_conduit && admin_room.as_ref() == Some(&pdu.room_id) {
+                    if let Some(admin_room) = services().admin.get_admin_room()? {
+                        if to_conduit && !from_conduit && admin_room == pdu.room_id {
                             services().admin.process_message(body);
                         }
                     }
                 }
+            }
             _ => {}
         }
 
@@ -820,13 +816,8 @@ impl Service {
         let (pdu, pdu_json) =
             self.create_hash_and_sign_event(pdu_builder, sender, room_id, state_lock)?;
 
-        let admin_room = services().rooms.alias.resolve_local_alias(
+        if let Some(admin_room) = services().admin.get_admin_room()? {
-            <&RoomAliasId>::try_from(
+            if admin_room == room_id {
-                format!("#admins:{}", services().globals.server_name()).as_str(),
-            )
-            .expect("#admins:server_name is a valid room alias"),
-        )?;
-        if admin_room.filter(|v| v == room_id).is_some() {
                 match pdu.event_type() {
                     TimelineEventType::RoomEncryption => {
                         warn!("Encryption is not allowed in the admins room");
@@ -905,6 +896,7 @@ impl Service {
                     _ => {}
                 }
             }
+        }
 
         // We append to state before appending the pdu, so we don't have a moment in time with the
         // pdu without it's state. This is okay because append_pdu can't fail.