refactor(webapi): split /room/:rid/admin endpoint and deprecate

Since we alraedy use `/room/:rid/member`, member CRUD can use this path
for better semantics.

The `admin` endpoint will be removed later.
This commit is contained in:
oxalica 2024-10-12 06:52:54 -04:00
parent b8921a5485
commit d1dfda51db
4 changed files with 235 additions and 97 deletions

View file

@ -339,24 +339,36 @@ pub struct RoomMember {
#[serde(tag = "typ", rename = "auth")] #[serde(tag = "typ", rename = "auth")]
pub struct AuthPayload {} pub struct AuthPayload {}
// FIXME: Remove this.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
// `typ` is provided by `RoomAdminOp`. // `typ` is provided by `RoomAdminOp`.
pub struct RoomAdminPayload { pub struct RoomAdminPayload {
pub room: Id,
#[serde(flatten)] #[serde(flatten)]
pub op: RoomAdminOp, pub op: RoomAdminOp,
} }
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "typ", rename_all = "snake_case", rename = "remove_member")]
pub struct RemoveMemberPayload {
pub room: Id,
// TODO: This field name collide with `Signee::user`.
pub user: PubKey,
}
// TODO: Maybe disallow adding other user without consent?
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "typ", rename_all = "snake_case")] #[serde(tag = "typ", rename_all = "snake_case")]
pub struct AddMemberPayload {
pub room: Id,
#[serde(flatten)]
pub member: RoomMember,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum RoomAdminOp { pub enum RoomAdminOp {
AddMember { AddMember(AddMemberPayload),
permission: MemberPermission, RemoveMember(RemoveMemberPayload),
user: PubKey,
},
RemoveMember {
user: PubKey,
},
// TODO: RU // TODO: RU
} }
@ -479,6 +491,7 @@ mod sql_impl {
mod tests { mod tests {
use ed25519_dalek::{SigningKey, PUBLIC_KEY_LENGTH}; use ed25519_dalek::{SigningKey, PUBLIC_KEY_LENGTH};
use expect_test::expect; use expect_test::expect;
use AddMemberPayload;
use crate::SignExt; use crate::SignExt;
@ -547,11 +560,13 @@ mod tests {
#[test] #[test]
fn room_admin_serde() { fn room_admin_serde() {
let data = RoomAdminPayload { let data = RoomAdminPayload {
room: Id(42), op: RoomAdminOp::AddMember(AddMemberPayload {
op: RoomAdminOp::AddMember { room: Id(42),
permission: MemberPermission::POST_CHAT, member: RoomMember {
user: PubKey([0x42; PUBLIC_KEY_LENGTH]), permission: MemberPermission::POST_CHAT,
}, user: PubKey([0x42; PUBLIC_KEY_LENGTH]),
},
}),
}; };
let raw = serde_jcs::to_string(&data).unwrap(); let raw = serde_jcs::to_string(&data).unwrap();

View file

@ -1,3 +1,4 @@
use std::marker::PhantomData;
use std::num::NonZero; use std::num::NonZero;
use std::sync::Arc; use std::sync::Arc;
use std::time::Duration; use std::time::Duration;
@ -7,24 +8,24 @@ use axum::body::Bytes;
use axum::extract::{Path, Query, State}; use axum::extract::{Path, Query, State};
use axum::http::{header, HeaderName, HeaderValue, StatusCode}; use axum::http::{header, HeaderName, HeaderValue, StatusCode};
use axum::response::{IntoResponse, Response}; use axum::response::{IntoResponse, Response};
use axum::routing::{get, post};
use axum::{Json, Router}; use axum::{Json, Router};
use axum_extra::extract::WithRejection as R; use axum_extra::extract::WithRejection as R;
use blah_types::msg::{ use blah_types::msg::{
ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload, AddMemberPayload, ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload,
MemberPermission, RoomAdminOp, RoomAdminPayload, RoomAttrs, ServerPermission, DeleteRoomPayload, MemberPermission, RemoveMemberPayload, RoomAdminOp, RoomAdminPayload,
SignedChatMsgWithId, WithMsgId, RoomAttrs, ServerPermission, SignedChatMsgWithId, WithMsgId,
}; };
use blah_types::server::{ use blah_types::server::{
ErrorResponseWithChallenge, RoomList, RoomMember, RoomMemberList, RoomMetadata, RoomMsgs, ErrorResponseWithChallenge, RoomList, RoomMember, RoomMemberList, RoomMetadata, RoomMsgs,
ServerCapabilities, ServerMetadata, ServerCapabilities, ServerMetadata,
}; };
use blah_types::{get_timestamp, Id, Signed, UserKey}; use blah_types::{get_timestamp, Id, PubKey, Signed, UserKey};
use data_encoding::BASE64_NOPAD; use data_encoding::BASE64_NOPAD;
use database::{Transaction, TransactionOps}; use database::{Transaction, TransactionOps};
use id::IdExt; use id::IdExt;
use middleware::{Auth, ETag, MaybeAuth, ResultExt as _, SignedJson}; use middleware::{Auth, ETag, MaybeAuth, ResultExt as _, SignedJson};
use parking_lot::Mutex; use parking_lot::Mutex;
use serde::de::DeserializeOwned;
use serde::{Deserialize, Deserializer, Serialize}; use serde::{Deserialize, Deserializer, Serialize};
use serde_inline_default::serde_inline_default; use serde_inline_default::serde_inline_default;
use sha2::Digest; use sha2::Digest;
@ -148,7 +149,9 @@ impl AppState {
type ArcState = State<Arc<AppState>>; type ArcState = State<Arc<AppState>>;
pub fn router(st: Arc<AppState>) -> Router { pub fn router(st: Arc<AppState>) -> Router {
// NB. User consistent handler naming: `<method>_<path>[_<details>]`. use axum::routing::{delete, get, post};
// NB. Use consistent handler naming: `<method>_<path>[_<details>]`.
// Use prefix `list` for GET with pagination. // Use prefix `list` for GET with pagination.
// //
// One route per line. // One route per line.
@ -165,8 +168,11 @@ pub fn router(st: Arc<AppState>) -> Router {
.route("/room/:rid/feed.atom", get(feed::get_room_feed::<feed::AtomFeed>)) .route("/room/:rid/feed.atom", get(feed::get_room_feed::<feed::AtomFeed>))
.route("/room/:rid/msg", get(list_room_msg).post(post_room_msg)) .route("/room/:rid/msg", get(list_room_msg).post(post_room_msg))
.route("/room/:rid/msg/:cid/seen", post(post_room_msg_seen)) .route("/room/:rid/msg/:cid/seen", post(post_room_msg_seen))
// TODO!: remove this.
.route("/room/:rid/admin", post(post_room_admin)) .route("/room/:rid/admin", post(post_room_admin))
.route("/room/:rid/member", get(list_room_member)); .route("/room/:rid/member", get(list_room_member).post(post_room_member))
.route("/room/:rid/member/:uid", delete(delete_room_member))
;
let router = router let router = router
.layer(tower_http::limit::RequestBodyLimitLayer::new( .layer(tower_http::limit::RequestBodyLimitLayer::new(
@ -470,58 +476,81 @@ async fn post_room_admin(
R(Path(rid), _): RE<Path<Id>>, R(Path(rid), _): RE<Path<Id>>,
SignedJson(op): SignedJson<RoomAdminPayload>, SignedJson(op): SignedJson<RoomAdminPayload>,
) -> Result<StatusCode, ApiError> { ) -> Result<StatusCode, ApiError> {
api_ensure!(rid == op.signee.payload.room, "room id mismatch with URI"); // TODO: This is a temporary endpoint so just reserialize them.
api_ensure!(!rid.is_peer_chat(), "cannot operate on a peer chat room"); fn transcode<T: Serialize, U: DeserializeOwned>(v: &T) -> SignedJson<U> {
let v = serde_json::to_value(v).expect("serialization cannot fail");
match op.signee.payload.op { SignedJson(serde_json::from_value(v).expect("format should be compatible"))
RoomAdminOp::AddMember { user, permission } => {
api_ensure!(
user == op.signee.user.id_key,
ApiError::NotImplemented("only self-adding is implemented yet"),
);
api_ensure!(
MemberPermission::MAX_SELF_ADD.contains(permission),
"invalid initial permission",
);
room_join(&st, rid, &op.signee.user, permission).await?;
}
RoomAdminOp::RemoveMember { user } => {
api_ensure!(
user == op.signee.user.id_key,
ApiError::NotImplemented("only self-removal is implemented yet"),
);
room_leave(&st, rid, &op.signee.user).await?;
}
} }
Ok(StatusCode::NO_CONTENT) match op.signee.payload.op {
RoomAdminOp::AddMember(_) => {
post_room_member(st, R(Path(rid), PhantomData), transcode(&op)).await
}
RoomAdminOp::RemoveMember(_) => {
delete_room_member(
st,
R(Path((rid, op.signee.user.id_key.clone())), PhantomData),
transcode(&op),
)
.await
}
}
} }
async fn room_join( async fn post_room_member(
st: &AppState, st: ArcState,
rid: Id, R(Path(rid), _): RE<Path<Id>>,
user: &UserKey, SignedJson(op): SignedJson<AddMemberPayload>,
permission: MemberPermission, ) -> Result<StatusCode, ApiError> {
) -> Result<(), ApiError> { api_ensure!(rid == op.signee.payload.room, "room id mismatch with URI");
api_ensure!(
!rid.is_peer_chat(),
"cannot add members to a peer chat room"
);
let member = op.signee.payload.member;
api_ensure!(
member.user == op.signee.user.id_key,
ApiError::NotImplemented("only self-adding is implemented yet"),
);
api_ensure!(
MemberPermission::MAX_SELF_ADD.contains(member.permission),
"invalid member permission",
);
st.db.with_write(|txn| { st.db.with_write(|txn| {
let (uid, _perm) = txn.get_user(user)?; let (uid, _perm) = txn.get_user(&op.signee.user)?;
let (attrs, _) = txn.get_room_having(rid, RoomAttrs::PUBLIC_JOINABLE)?; let (attrs, _) = txn.get_room_having(rid, RoomAttrs::PUBLIC_JOINABLE)?;
// Sanity check. // Sanity check.
assert!(!attrs.contains(RoomAttrs::PEER_CHAT)); assert!(!attrs.contains(RoomAttrs::PEER_CHAT));
txn.add_room_member(rid, uid, permission)?; txn.add_room_member(rid, uid, member.permission)?;
Ok(()) Ok(StatusCode::NO_CONTENT)
}) })
} }
async fn room_leave(st: &AppState, rid: Id, user: &UserKey) -> Result<(), ApiError> { async fn delete_room_member(
st: ArcState,
R(Path((rid, id_key)), _): RE<Path<(Id, PubKey)>>,
SignedJson(op): SignedJson<RemoveMemberPayload>,
) -> Result<StatusCode, ApiError> {
api_ensure!(rid == op.signee.payload.room, "room id mismatch with URI");
api_ensure!(
!rid.is_peer_chat(),
"cannot remove members from a peer chat room"
);
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"),
);
st.db.with_write(|txn| { st.db.with_write(|txn| {
api_ensure!(!rid.is_peer_chat(), "cannot leave a peer chat room"); let (uid, ..) = txn.get_room_member(rid, &op.signee.user)?;
let (uid, ..) = txn.get_room_member(rid, user)?;
let (attrs, _) = txn.get_room_having(rid, RoomAttrs::empty())?; let (attrs, _) = txn.get_room_having(rid, RoomAttrs::empty())?;
// Sanity check. // Sanity check.
assert!(!attrs.contains(RoomAttrs::PEER_CHAT)); assert!(!attrs.contains(RoomAttrs::PEER_CHAT));
txn.remove_room_member(rid, uid)?; txn.remove_room_member(rid, uid)?;
Ok(()) Ok(StatusCode::NO_CONTENT)
}) })
} }

View file

@ -10,10 +10,10 @@ use std::time::{Duration, Instant};
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use blah_types::identity::{IdUrl, UserActKeyDesc, UserIdentityDesc, UserProfile}; use blah_types::identity::{IdUrl, UserActKeyDesc, UserIdentityDesc, UserProfile};
use blah_types::msg::{ use blah_types::msg::{
AuthPayload, ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload, self, AddMemberPayload, AuthPayload, ChatPayload, CreateGroup, CreatePeerChat,
MemberPermission, RichText, RoomAdminOp, RoomAdminPayload, RoomAttrs, ServerPermission, CreateRoomPayload, DeleteRoomPayload, MemberPermission, RemoveMemberPayload, RichText,
SignedChatMsg, SignedChatMsgWithId, UserRegisterChallengeResponse, UserRegisterPayload, RoomAttrs, ServerPermission, SignedChatMsg, SignedChatMsgWithId, UserRegisterChallengeResponse,
WithMsgId, UserRegisterPayload, WithMsgId,
}; };
use blah_types::server::{ use blah_types::server::{
RoomList, RoomMember, RoomMemberList, RoomMetadata, RoomMsgs, ServerEvent, ServerMetadata, RoomList, RoomMember, RoomMemberList, RoomMetadata, RoomMsgs, ServerEvent, ServerMetadata,
@ -302,30 +302,39 @@ impl Server {
) -> impl Future<Output = Result<()>> + use<'_> { ) -> impl Future<Output = Result<()>> + use<'_> {
let req = self.sign( let req = self.sign(
user, user,
RoomAdminPayload { AddMemberPayload {
room: rid, room: rid,
op: RoomAdminOp::AddMember { member: msg::RoomMember {
permission, permission,
user: user.pubkeys.id_key.clone(), user: user.pubkeys.id_key.clone(),
}, },
}, },
); );
self.request::<_, NoContent>(Method::POST, &format!("/room/{rid}/admin"), None, Some(req)) self.request::<_, NoContent>(
.map_ok(|None| {}) Method::POST,
&format!("/room/{rid}/member"),
None,
Some(req),
)
.map_ok(|None| {})
} }
fn leave_room(&self, rid: Id, user: &User) -> impl Future<Output = Result<()>> + use<'_> { fn leave_room(&self, rid: Id, user: &User) -> impl Future<Output = Result<()>> + use<'_> {
let user_id = user.pubkeys.id_key.clone();
let req = self.sign( let req = self.sign(
user, user,
RoomAdminPayload { RemoveMemberPayload {
room: rid, room: rid,
op: RoomAdminOp::RemoveMember { user: user_id.clone(),
user: user.pubkeys.id_key.clone(),
},
}, },
); );
self.request::<_, NoContent>(Method::POST, &format!("/room/{rid}/admin"), None, Some(req)) self.request::<_, NoContent>(
.map_ok(|None| {}) Method::DELETE,
&format!("/room/{rid}/member/{user_id}"),
None,
Some(req),
)
.map_ok(|None| {})
} }
fn post_chat( fn post_chat(
@ -585,7 +594,7 @@ async fn room_join_leave(server: Server) {
server server
.join_room(rid_priv, &BOB, MemberPermission::ALL) .join_room(rid_priv, &BOB, MemberPermission::ALL)
.await .await
.expect_invalid_request("invalid initial permission"); .expect_invalid_request("invalid member permission");
// Bob is joined now. // Bob is joined now.
assert_eq!( assert_eq!(

View file

@ -327,7 +327,11 @@ paths:
/_blah/room/{rid}/admin: /_blah/room/{rid}/admin:
post: post:
summary: Room management summary: Room management (legacy)
deprecated: true
description: |
Use POST `/_blah/room/{rid}/member` or
DELETE `/_blah/room/{rid}/member/{member_id_key}` instead.
requestBody: requestBody:
content: content:
@ -561,6 +565,67 @@ paths:
schema: schema:
$ref: '#/components/schemas/ApiError' $ref: '#/components/schemas/ApiError'
post:
summary: Join a room
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Signed-AddMember'
responses:
204:
description: Operation completed.
404:
description: |
Room does not exist or the user does not have permission for the
operation.
content:
application/json:
schema:
$ref: '#/components/schemas/ApiError'
409:
description:
The user is already a room member.
content:
application/json:
schema:
$ref: '#/components/schemas/ApiError'
/_blah/room/{rid}/member/{target_id_key}:
delete:
summary: Remove a room member.
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Signed-RemoveMember'
responses:
204:
description: Operation completed.
403:
description: |
The user does not have permission to remove the operand member.
content:
application/json:
schema:
$ref: '#/components/schemas/ApiError'
404:
description: |
Room does not exist, the user does not have permission for the
operation, or the operand user is not a room member.
content:
application/json:
schema:
$ref: '#/components/schemas/ApiError'
# Ideally we should generate these from src, but we need to # Ideally we should generate these from src, but we need to
# WAIT: https://github.com/juhaku/utoipa/pull/1034 # WAIT: https://github.com/juhaku/utoipa/pull/1034
components: components:
@ -775,6 +840,11 @@ components:
const: 'auth' const: 'auth'
Signed-RoomAdmin: Signed-RoomAdmin:
oneOf:
- $ref: '#/components/schemas/Signed-AddMember'
- $ref: '#/components/schemas/Signed-RemoveMember'
Signed-AddMember:
type: object type: object
properties: properties:
sig: sig:
@ -793,32 +863,47 @@ components:
act_key: act_key:
type: string type: string
payload: payload:
oneOf: type: object
properties:
typ:
type: string
const: 'add_member'
room:
type: string
permission:
type: integer
format: int32
user:
type: string
- description: Add member to the room. Signed-RemoveMember:
type: object type: object
properties: properties:
typ: sig:
type: string type: string
const: 'add_member' signee:
room: type: object
type: string properties:
permission: nonce:
type: integer type: integer
format: int32 format: uint32
user: timestamp:
type: string type: integer
format: uint64
- description: Remove member from the room. id_key:
type: object type: string
properties: act_key:
typ: type: string
type: string payload:
const: 'remove_member' type: object
room: properties:
type: string typ:
user: type: string
type: string const: 'remove_member'
room:
type: string
user:
type: string
Signed-Chat: Signed-Chat:
type: object type: object