diff --git a/crates/core/database/fixtures/server_with_roles.json b/crates/core/database/fixtures/server_with_roles.json index 01ec436b..0a76489d 100644 --- a/crates/core/database/fixtures/server_with_roles.json +++ b/crates/core/database/fixtures/server_with_roles.json @@ -52,7 +52,7 @@ "a": 545270208, "d": 0 }, - "rank": 3 + "rank": 1 }, "__ID:6__": { "name": "Owner", diff --git a/crates/core/database/src/events/client.rs b/crates/core/database/src/events/client.rs index af480d5a..a887e9f1 100644 --- a/crates/core/database/src/events/client.rs +++ b/crates/core/database/src/events/client.rs @@ -165,6 +165,9 @@ pub enum EventV1 { /// Server role deleted ServerRoleDelete { id: String, role_id: String }, + /// Server roles ranks updated + ServerRoleRanksUpdate { id: String, ranks: Vec }, + /// Update existing user UserUpdate { id: String, diff --git a/crates/core/database/src/models/admin_migrations/ops/mongodb/scripts.rs b/crates/core/database/src/models/admin_migrations/ops/mongodb/scripts.rs index 0f58082a..d164cab1 100644 --- a/crates/core/database/src/models/admin_migrations/ops/mongodb/scripts.rs +++ b/crates/core/database/src/models/admin_migrations/ops/mongodb/scripts.rs @@ -1,4 +1,8 @@ -use std::{collections::HashSet, ops::BitXor, time::Duration}; +use std::{ + collections::{HashMap, HashSet}, + ops::BitXor, + time::Duration, +}; use crate::{ mongodb::{ @@ -22,7 +26,7 @@ struct MigrationInfo { revision: i32, } -pub const LATEST_REVISION: i32 = 41; // MUST BE +1 to last migration +pub const LATEST_REVISION: i32 = 42; // MUST BE +1 to last migration pub async fn migrate_database(db: &MongoDb) { let migrations = db.col::("migrations"); @@ -1165,6 +1169,63 @@ pub async fn run_migrations(db: &MongoDb, revision: i32) -> i32 { .expect("failed to update users"); } + if revision <= 41 { + info!( + "Running migration [revision 41 / 05-06-2025]: convert role ranks to uniform numbers." + ); + + #[derive(Serialize, Deserialize, Clone)] + struct Role { + pub rank: i64, + } + + #[derive(Serialize, Deserialize, Clone)] + struct Server { + #[serde(rename = "_id")] + pub id: String, + #[serde(default = "HashMap::::new")] + pub roles: HashMap, + } + + let mut servers = db + .db() + .collection::("servers") + .find(doc! { + "roles": { + "$exists": true, + "$ne": [] + } + }) + .await + .unwrap() + .filter_map(|s| async { s.ok() }) + .boxed(); + + while let Some(server) = servers.next().await { + let mut ordered_roles = server.roles.clone().into_iter().collect::>(); + ordered_roles.sort_by(|(_, role_a), (_, role_b)| role_a.rank.cmp(&role_b.rank)); + let ordered_roles = ordered_roles + .into_iter() + .map(|(id, _)| id) + .collect::>(); + + let mut doc = doc! {}; + + for id in server.roles.keys() { + doc.insert( + format!("roles.{id}.rank"), + ordered_roles.iter().position(|x| id == x).unwrap() as i64, + ); + } + + db.db() + .collection::("servers") + .update_one(doc! { "_id": &server.id }, doc! { "$set": doc }) + .await + .unwrap(); + } + } + // Reminder to update LATEST_REVISION when adding new migrations. LATEST_REVISION.max(revision) } diff --git a/crates/core/database/src/models/servers/model.rs b/crates/core/database/src/models/servers/model.rs index c69b1196..eea8f392 100644 --- a/crates/core/database/src/models/servers/model.rs +++ b/crates/core/database/src/models/servers/model.rs @@ -228,6 +228,13 @@ impl Server { } } + /// Ordered roles list + pub fn ordered_roles(&self) -> Vec<(String, Role)> { + let mut ordered_roles = self.roles.clone().into_iter().collect::>(); + ordered_roles.sort_by(|(_, role_a), (_, role_b)| role_a.rank.cmp(&role_b.rank)); + ordered_roles + } + /// Set role permission on a server pub async fn set_role_permission( &mut self, @@ -253,6 +260,37 @@ impl Server { Err(create_error!(NotFound)) } } + + /// Reorders the server's roles rankings + pub async fn set_role_ordering(&mut self, db: &Database, new_order: Vec) -> Result<()> { + // New order must always contain every role + debug_assert_eq!(self.roles.len(), new_order.len()); + + // Set the role's ranks to the positions in the vec + for (rank, id) in new_order.iter().enumerate() { + self.roles.get_mut(id).unwrap().rank = rank as i64; + } + + db.update_server( + &self.id, + &PartialServer { + roles: Some(self.roles.clone()), + ..Default::default() + }, + Vec::new(), + ) + .await?; + + // Publish bulk update event + EventV1::ServerRoleRanksUpdate { + id: self.id.clone(), + ranks: new_order, + } + .p(self.id.clone()) + .await; + + Ok(()) + } } impl Role { diff --git a/crates/core/models/src/v0/servers.rs b/crates/core/models/src/v0/servers.rs index ae5a50a2..4c7e1ee8 100644 --- a/crates/core/models/src/v0/servers.rs +++ b/crates/core/models/src/v0/servers.rs @@ -267,7 +267,7 @@ auto_derived!( pub hoist: Option, /// Ranking position /// - /// Smaller values take priority. + /// **Removed** - no effect, use the edit server role positions route pub rank: Option, /// Fields to remove from role object #[cfg_attr(feature = "validator", validate(length(min = 1)))] @@ -286,4 +286,9 @@ auto_derived!( /// Whether to not send a leave message pub leave_silently: Option, } + + /// New role positions + pub struct DataEditRoleRanks { + pub ranks: Vec, + } ); diff --git a/crates/delta/fixtures/server_with_many_roles.json b/crates/delta/fixtures/server_with_many_roles.json new file mode 100644 index 00000000..49cfe276 --- /dev/null +++ b/crates/delta/fixtures/server_with_many_roles.json @@ -0,0 +1,114 @@ +[ + { + "_object_type": "User", + "_id": "__ID:0__", + "username": "Owner", + "last_acknowledged_policy_change": "2025-06-07T04:04:48+0000", + "discriminator": "0001" + }, + { + "_object_type": "User", + "_id": "__ID:1__", + "username": "Moderator", + "last_acknowledged_policy_change": "2025-06-07T04:04:48+0000", + "discriminator": "0001" + }, + { + "_object_type": "User", + "_id": "__ID:2__", + "username": "User", + "last_acknowledged_policy_change": "2025-06-07T04:04:48+0000", + "discriminator": "0001" + }, + { + "_object_type": "Channel", + "_id": "__ID:3__", + "channel_type": "TextChannel", + "name": "General", + "server": "__ID:4__", + "default_permissions": { + "a": 0, + "d": 1048576 + }, + "role_permissions": { + "__ID:5__": { + "a": 1048576, + "d": 0 + } + } + }, + { + "_object_type": "Server", + "_id": "__ID:4__", + "owner": "__ID:0__", + "name": "Server", + "channels": [ + "__ID:3__" + ], + "roles": { + "__ID:5__": { + "name": "Moderator", + "permissions": { + "a": 545270216, + "d": 0 + }, + "rank": 1 + }, + "__ID:6__": { + "name": "Owner", + "permissions": { + "a": 0, + "d": 0 + }, + "rank": 0 + }, + "__ID:7__": { + "name": "Lower Rank 1", + "permissions": { + "a": 0, + "d": 0 + }, + "rank": 2 + }, + "__ID:8__": { + "name": "Lower Rank 2", + "permissions": { + "a": 0, + "d": 0 + }, + "rank": 2 + } + }, + "default_permissions": 4000322560 + }, + { + "_object_type": "ServerMember", + "_id": { + "user": "__ID:0__", + "server": "__ID:4__" + }, + "roles": [ + "__ID:6__" + ], + "joined_at": 1698318340195 + }, + { + "_object_type": "ServerMember", + "_id": { + "user": "__ID:1__", + "server": "__ID:4__" + }, + "roles": [ + "__ID:5__" + ], + "joined_at": 1698318340195 + }, + { + "_object_type": "ServerMember", + "_id": { + "user": "__ID:2__", + "server": "__ID:4__" + }, + "joined_at": 1698318340195 + } +] \ No newline at end of file diff --git a/crates/delta/src/routes/servers/mod.rs b/crates/delta/src/routes/servers/mod.rs index a7553625..066fd32a 100644 --- a/crates/delta/src/routes/servers/mod.rs +++ b/crates/delta/src/routes/servers/mod.rs @@ -18,6 +18,7 @@ mod roles_create; mod roles_delete; mod roles_edit; mod roles_fetch; +mod roles_edit_positions; mod server_ack; mod server_create; mod server_delete; @@ -47,6 +48,7 @@ pub fn routes() -> (Vec, OpenApi) { roles_delete::delete, permissions_set::set_role_permission, permissions_set_default::set_default_permissions, - emoji_list::list_emoji + emoji_list::list_emoji, + roles_edit_positions::edit_role_ranks ] } diff --git a/crates/delta/src/routes/servers/roles_edit.rs b/crates/delta/src/routes/servers/roles_edit.rs index 72ec6635..f8152efe 100644 --- a/crates/delta/src/routes/servers/roles_edit.rs +++ b/crates/delta/src/routes/servers/roles_edit.rs @@ -12,7 +12,7 @@ use validator::Validate; /// /// Edit a role by its id. #[openapi(tag = "Server Permissions")] -#[patch("//roles/", data = "")] +#[patch("//roles/", data = "", rank = 1)] pub async fn edit( db: &State, user: User, @@ -45,22 +45,14 @@ pub async fn edit( name, colour, hoist, - rank, remove, + .. } = data; - // Prevent us from moving a role above other roles - if let Some(rank) = &rank { - if rank <= &member_rank { - return Err(create_error!(NotElevated)); - } - } - let partial = PartialRole { name, colour, hoist, - rank, ..Default::default() }; diff --git a/crates/delta/src/routes/servers/roles_edit_positions.rs b/crates/delta/src/routes/servers/roles_edit_positions.rs new file mode 100644 index 00000000..59614162 --- /dev/null +++ b/crates/delta/src/routes/servers/roles_edit_positions.rs @@ -0,0 +1,177 @@ +use revolt_database::{ + util::{permissions::DatabasePermissionQuery, reference::Reference}, + Database, User, +}; +use revolt_models::v0; +use revolt_permissions::{calculate_server_permissions, ChannelPermission}; +use revolt_result::{create_error, Result}; +use rocket::{serde::json::Json, State}; + +/// # Edits server roles ranks +/// +/// Edit's server role's ranks. +#[openapi(tag = "Server Permissions")] +#[patch("//roles/ranks", data = "")] +pub async fn edit_role_ranks( + db: &State, + user: User, + target: Reference, + data: Json, +) -> Result> { + let data = data.into_inner(); + + let mut server = target.as_server(db).await?; + let mut query = DatabasePermissionQuery::new(db, &user).server(&server); + calculate_server_permissions(&mut query) + .await + .throw_if_lacking_channel_permission(ChannelPermission::ManageRole)?; + + let existing_order = server + .ordered_roles() + .into_iter() + .map(|(id, _)| id) + .collect::>(); + + let new_order = data.ranks.clone().into_iter().collect::>(); + + // Verify all roles are in the new ordering + if data.ranks.len() != server.roles.len() + || !server.roles.iter().all(|(id, _)| data.ranks.contains(id)) + { + return Err(create_error!(InvalidOperation)); + } + + // Don't have to check what the user can't modify if they are the server owner + if server.owner != user.id { + let member_top_rank = query.get_member_rank(); + + if server + .roles + .iter() + // Find all roles above the member which we should not be able to reorder + .filter(|(_, role)| { + if let Some(top_rank) = member_top_rank { + role.rank <= top_rank + } else { + true + } + }) + // Check if user is trying to reorder roles they can't reorder (as found previously) + .any(|(id, _)| { + existing_order + .iter() + .position(|existing_id| id == existing_id) + != new_order.iter().position(|new_id| id == new_id) + }) + { + return Err(create_error!(NotElevated)); + } + } + + server.set_role_ordering(db, new_order).await?; + + Ok(Json(server.into())) +} + +#[cfg(test)] +mod test { + use revolt_database::fixture; + use revolt_models::v0; + use rocket::http::{ContentType, Header, Status}; + + use crate::util::test::TestHarness; + + #[rocket::async_test] + async fn edit_role_rankings() { + let harness = TestHarness::new().await; + + fixture!(harness.db, "server_with_many_roles", + owner user 0 + moderator user 1 + server server 4); + + // Moderator can re-order the roles below them + let (_, moderator_session) = harness.account_from_user(moderator.id).await; + let mut target_order: Vec = server + .ordered_roles() + .into_iter() + .map(|(id, _)| id) + .collect(); + + // Swap the two lower ranked roles + target_order.swap(2, 3); + + let response = harness + .client + .patch(format!("/servers/{}/roles/ranks", server.id)) + .header(ContentType::JSON) + .body( + json!(v0::DataEditRoleRanks { + ranks: target_order.clone() + }) + .to_string(), + ) + .header(Header::new( + "x-session-token", + moderator_session.token.to_string(), + )) + .dispatch() + .await; + + assert_eq!(response.status(), Status::Ok); + drop(response); + + // ... but not above them + let mut target_order: Vec = server + .ordered_roles() + .into_iter() + .map(|(id, _)| id) + .collect(); + + // Swap the two lower ranked roles + target_order.swap(0, 1); + + let response = harness + .client + .patch(format!("/servers/{}/roles/ranks", server.id)) + .header(ContentType::JSON) + .body( + json!(v0::DataEditRoleRanks { + ranks: target_order.clone() + }) + .to_string(), + ) + .header(Header::new( + "x-session-token", + moderator_session.token.to_string(), + )) + .dispatch() + .await; + + assert_eq!(response.status(), Status::Forbidden); + drop(response); + + // The owner can set any order they want + let (_, owner_session) = harness.account_from_user(owner.id).await; + + let response = harness + .client + .patch(format!("/servers/{}/roles/ranks", server.id)) + .header(ContentType::JSON) + .body( + json!(v0::DataEditRoleRanks { + ranks: target_order.clone() + }) + .to_string(), + ) + .header(Header::new( + "x-session-token", + owner_session.token.to_string(), + )) + .dispatch() + .await; + + assert_eq!(response.status(), Status::Ok); + drop(response); + } +} diff --git a/crates/delta/src/util/test.rs b/crates/delta/src/util/test.rs index e600ac15..067f8bd0 100644 --- a/crates/delta/src/util/test.rs +++ b/crates/delta/src/util/test.rs @@ -1,5 +1,5 @@ use authifier::{ - models::{Account, Session}, + models::{Account, EmailVerification, Session}, Authifier, }; use futures::StreamExt; @@ -83,30 +83,41 @@ impl TestHarness { } pub async fn new_user(&self) -> (Account, Session, User) { - let account = Account::new( - &self.authifier, - format!("{}@revolt.chat", TestHarness::rand_string()), - "password".to_string(), - false, - ) - .await - .expect("`Account`"); + let user = User::create(&self.db, TestHarness::rand_string(), None, None) + .await + .expect("`User`"); + + let (account, session) = self.account_from_user(user.id.clone()).await; + + (account, session, user) + } + + pub async fn account_from_user(&self, id: String) -> (Account, Session) { + let account = Account { + id, + email: format!("{}@revolt.chat", TestHarness::rand_string()), + password: Default::default(), + email_normalised: Default::default(), + deletion: None, + disabled: false, + lockout: None, + mfa: Default::default(), + password_reset: None, + verification: EmailVerification::Verified, + }; + + self.authifier + .database + .save_account(&account) + .await + .expect("`Account`"); let session = account .create_session(&self.authifier, String::new()) .await .expect("`Session`"); - let user = User::create( - &self.db, - TestHarness::rand_string(), - account.id.to_string(), - None, - ) - .await - .expect("`User`"); - - (account, session, user) + (account, session) } pub async fn new_server(&self, user: &User) -> (Server, Vec) {