From e06e15d4ec614d284d494bac795299f05e5711fb Mon Sep 17 00:00:00 2001
From: Matthias Ahouansou <matthias@ahouansou.cz>
Date: Sun, 3 Mar 2024 11:26:18 +0000
Subject: [PATCH] fix(accounts): don't give guests admin

---
 src/api/client_server/account.rs  | 29 +++++++++++++--------
 src/service/admin/mod.rs          | 42 +++++++++++++++----------------
 src/service/rooms/timeline/mod.rs | 20 ++++-----------
 3 files changed, 44 insertions(+), 47 deletions(-)

diff --git a/src/api/client_server/account.rs b/src/api/client_server/account.rs
index 46551305..3a791215 100644
--- a/src/api/client_server/account.rs
+++ b/src/api/client_server/account.rs
@@ -149,17 +149,18 @@ pub async fn register_route(body: Ruma<register::v3::Request>) -> Result<registe
                 return Err(Error::Uiaa(uiaainfo));
             }
         // Success!
-        } else if let Some(json) = body.json_body {
-            uiaainfo.session = Some(utils::random_string(SESSION_ID_LENGTH));
-            services().uiaa.create(
-                &UserId::parse_with_server_name("", services().globals.server_name())
-                    .expect("we know this is valid"),
-                "".into(),
-                &uiaainfo,
-                &json,
-            )?;
-            return Err(Error::Uiaa(uiaainfo));
         } else {
+            if let Some(json) = body.json_body {
+                uiaainfo.session = Some(utils::random_string(SESSION_ID_LENGTH));
+                services().uiaa.create(
+                    &UserId::parse_with_server_name("", services().globals.server_name())
+                        .expect("we know this is valid"),
+                    "".into(),
+                    &uiaainfo,
+                    &json,
+                )?;
+                return Err(Error::Uiaa(uiaainfo));
+            }
             return Err(Error::BadRequest(ErrorKind::NotJson, "Not json."));
         }
     }
@@ -239,7 +240,13 @@ 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
+        && services()
+            .rooms
+            .state_cache
+            .room_joined_count(&services().admin.get_admin_room())?
+            == Some(1)
+    {
         services()
             .admin
             .make_user_admin(&user_id, displayname)
diff --git a/src/service/admin/mod.rs b/src/service/admin/mod.rs
index bd682550..b3c18d18 100644
--- a/src/service/admin/mod.rs
+++ b/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,17 +214,7 @@ impl Service {
         let conduit_user = UserId::parse(format!("@conduit:{}", services().globals.server_name()))
             .expect("@conduit:server_name is valid");
 
-        let conduit_room = services()
-            .rooms
-            .alias
-            .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 conduit_room = services().admin.get_admin_room();
 
         let send_message = |message: RoomMessageEventContent, mutex_lock: &MutexGuard<'_, ()>| {
             services()
@@ -1105,6 +1095,24 @@ impl Service {
         Ok(())
     }
 
+    /// Gets the room ID of the admin room
+    ///
+    /// If the room does not exist, this function panics, since it should have been created on first run
+    // ^ was the case before this function when the following code was re-used in multiple places
+    pub(crate) fn get_admin_room(&self) -> 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)
+            .expect("Room ID should be valid unicode, since this server created it")
+            .expect("Admin room must exist")
+    }
+
     /// Invite the user to the conduit admin room.
     ///
     /// In conduit, this is equivalent to granting admin privileges.
@@ -1113,15 +1121,7 @@ impl Service {
         user_id: &UserId,
         displayname: String,
     ) -> Result<()> {
-        let admin_room_alias: Box<RoomAliasId> =
-            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 room_id = services().admin.get_admin_room();
 
         let mutex_state = Arc::clone(
             services()
diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs
index ef09d061..13193e52 100644
--- a/src/service/rooms/timeline/mod.rs
+++ b/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,7 @@ 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 admin_room = services().admin.get_admin_room();
                     let server_user = format!("@conduit:{}", services().globals.server_name());
 
                     let to_conduit = body.starts_with(&format!("{server_user}: "))
@@ -466,7 +461,7 @@ 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 to_conduit && !from_conduit && admin_room == pdu.room_id {
                         services().admin.process_message(body);
                     }
                 }
@@ -820,13 +815,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(
-            <&RoomAliasId>::try_from(
-                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() {
+        let 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");