From ad4a38cf43f6da0ddc5b7433721f4c506437f8ed Mon Sep 17 00:00:00 2001 From: oxalica Date: Sat, 12 Oct 2024 12:27:54 -0400 Subject: [PATCH] feat(blahd): impl non-self member removal --- blah-types/src/msg.rs | 5 ++++ blahd/src/database.rs | 27 ++++++++++++++++-- blahd/src/lib.rs | 18 ++++++++---- blahd/src/middleware.rs | 1 + blahd/tests/webapi.rs | 62 ++++++++++++++++++++++++++++++++++++++--- 5 files changed, 101 insertions(+), 12 deletions(-) diff --git a/blah-types/src/msg.rs b/blah-types/src/msg.rs index c04ce95..61cabf4 100644 --- a/blah-types/src/msg.rs +++ b/blah-types/src/msg.rs @@ -387,9 +387,14 @@ bitflags::bitflags! { pub struct MemberPermission: i32 { const POST_CHAT = 1 << 0; const ADD_MEMBER = 1 << 1; + // TODO: Group admin permissions together. const DELETE_ROOM = 1 << 2; const LIST_MEMBERS = 1 << 3; + // TODO: Should we have multiple levels of removal permission, so that admins + // may not remove all other admins? + const REMOVE_MEMBER = 1 << 4; + const MAX_SELF_ADD = Self::POST_CHAT.bits(); const MAX_PEER_CHAT = Self::POST_CHAT.bits() | Self::DELETE_ROOM.bits() | Self::LIST_MEMBERS.bits(); diff --git a/blahd/src/database.rs b/blahd/src/database.rs index 6e3a033..6829e69 100644 --- a/blahd/src/database.rs +++ b/blahd/src/database.rs @@ -220,6 +220,26 @@ pub trait TransactionOps { .and_then(|row| Ok((row.get(0)?, row.get(1)?, row.get(2)?))) } + fn get_room_member_by_id_key( + &self, + rid: Id, + id_key: &PubKey, + ) -> Result<(i64, MemberPermission, Id)> { + prepare_cached_and_bind!( + self.conn(), + r" + SELECT `uid`, `room_member`.`permission`, `last_seen_cid` + FROM `room_member` + JOIN `user` USING (`uid`) + WHERE (`rid`, `id_key`) = (:rid, :id_key) + " + ) + .raw_query() + .next()? + .ok_or(ApiError::MemberNotFound) + .and_then(|row| Ok((row.get(0)?, row.get(1)?, row.get(2)?))) + } + fn get_room_having(&self, rid: Id, filter: RoomAttrs) -> Result<(RoomAttrs, Option)> { prepare_cached_and_bind!( self.conn(), @@ -527,7 +547,7 @@ pub trait TransactionOps { Ok(()) } - fn remove_room_member(&self, rid: Id, uid: i64) -> Result { + fn remove_room_member(&self, rid: Id, uid: i64) -> Result<()> { // TODO: Check if it is the last member? let updated = prepare_cached_and_bind!( self.conn(), @@ -537,7 +557,10 @@ pub trait TransactionOps { " ) .raw_execute()?; - Ok(updated == 1) + if updated != 1 { + return Err(ApiError::UserNotFound); + } + Ok(()) } fn add_room_chat_msg(&self, rid: Id, uid: i64, cid: Id, chat: &SignedChatMsg) -> Result<()> { diff --git a/blahd/src/lib.rs b/blahd/src/lib.rs index e85ee86..6838499 100644 --- a/blahd/src/lib.rs +++ b/blahd/src/lib.rs @@ -539,17 +539,23 @@ async fn delete_room_member( ); let tgt_user = op.signee.payload.user; api_ensure!(id_key == tgt_user, "user id mismatch with URI"); - api_ensure!( - op.signee.user.id_key == tgt_user, - ApiError::NotImplemented("only self-removal is implemented yet"), - ); + let is_self_removal = op.signee.user.id_key == tgt_user; st.db.with_write(|txn| { - let (uid, ..) = txn.get_room_member(rid, &op.signee.user)?; + let (uid, perm, _) = txn.get_room_member(rid, &op.signee.user)?; + api_ensure!( + is_self_removal || perm.contains(MemberPermission::REMOVE_MEMBER), + ApiError::PermissionDenied("the user does not have permission to remove members") + ); let (attrs, _) = txn.get_room_having(rid, RoomAttrs::empty())?; // Sanity check. assert!(!attrs.contains(RoomAttrs::PEER_CHAT)); - txn.remove_room_member(rid, uid)?; + let tgt_uid = if is_self_removal { + uid + } else { + txn.get_room_member_by_id_key(rid, &tgt_user)?.0 + }; + txn.remove_room_member(rid, tgt_uid)?; Ok(NoContent) }) } diff --git a/blahd/src/middleware.rs b/blahd/src/middleware.rs index 3a6684a..737bc93 100644 --- a/blahd/src/middleware.rs +++ b/blahd/src/middleware.rs @@ -66,6 +66,7 @@ pub enum ApiError { UserNotFound = (StatusCode::NOT_FOUND, "the user does not exist"), RoomNotFound = (StatusCode::NOT_FOUND, "the room does not exist or the user is not a room member"), PeerUserNotFound = (StatusCode::NOT_FOUND, "peer user does not exist or disallows peer chat"), + MemberNotFound = (StatusCode::NOT_FOUND, "the operand user is not a room member"), Conflict(&'static str) = (StatusCode::CONFLICT), Exists(&'static str) = (StatusCode::CONFLICT), FetchIdDescription(Box) = (StatusCode::UNPROCESSABLE_ENTITY), diff --git a/blahd/tests/webapi.rs b/blahd/tests/webapi.rs index f85aab6..aae26df 100644 --- a/blahd/tests/webapi.rs +++ b/blahd/tests/webapi.rs @@ -320,17 +320,26 @@ impl Server { } fn leave_room(&self, rid: Id, user: &User) -> impl Future> + use<'_> { - let user_id = user.pubkeys.id_key.clone(); + self.remove_member(rid, user, user) + } + + fn remove_member( + &self, + rid: Id, + act_user: &User, + tgt_user: &User, + ) -> impl Future> + use<'_> { + let tgt_user_id = tgt_user.pubkeys.id_key.clone(); let req = self.sign( - user, + act_user, RemoveMemberPayload { room: rid, - user: user_id.clone(), + user: tgt_user_id.clone(), }, ); self.request::<_, NoContent>( Method::DELETE, - &format!("/room/{rid}/member/{user_id}"), + &format!("/room/{rid}/member/{tgt_user_id}"), None, Some(req), ) @@ -1627,3 +1636,48 @@ async fn room_member(server: Server) { }; assert_eq!(got, expect); } + +#[rstest] +#[tokio::test] +async fn room_management(server: Server) { + let rid = server + .create_room(&ALICE, RoomAttrs::PUBLIC_JOINABLE, "public") + .await + .unwrap(); + server + .join_room(rid, &BOB, MemberPermission::MAX_SELF_ADD) + .await + .unwrap(); + // Now bob can access the room. + server + .get::(&format!("/room/{rid}"), Some(&auth(&BOB))) + .await + .unwrap(); + + // No permission. + server + .remove_member(rid, &BOB, &ALICE) + .await + .expect_api_err(StatusCode::FORBIDDEN, "permission_denied"); + + // Invalid act user. + server + .remove_member(rid, &CAROL, &BOB) + .await + .expect_api_err(StatusCode::NOT_FOUND, "room_not_found"); + + // Invalid operand user. + server + .remove_member(rid, &ALICE, &CAROL) + .await + .expect_api_err(StatusCode::NOT_FOUND, "member_not_found"); + + // OK, removed. + server.remove_member(rid, &ALICE, &BOB).await.unwrap(); + + // Bob cannot access the room now. + server + .get::(&format!("/room/{rid}"), Some(&auth(&BOB))) + .await + .expect_api_err(StatusCode::NOT_FOUND, "room_not_found"); +}