diff --git a/Cargo.lock b/Cargo.lock index af64954..7ad9561 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -322,6 +322,7 @@ dependencies = [ "humantime", "nix", "parking_lot", + "paste", "rand", "reqwest", "rstest", @@ -1466,6 +1467,12 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "pem-rfc7468" version = "0.7.0" diff --git a/blahd/Cargo.toml b/blahd/Cargo.toml index b5185f0..b2c9cca 100644 --- a/blahd/Cargo.toml +++ b/blahd/Cargo.toml @@ -15,6 +15,7 @@ html-escape = "0.2" http-body-util = "0.1" humantime = "2" parking_lot = "0.12" # Maybe no better performance, just that we hate poisoning. ¯\_(ツ)_/¯ +paste = "1.0.15" rand = "0.8" reqwest = "0.12" rusqlite = { version = "0.32", features = ["rusqlite-macros"] } diff --git a/blahd/src/database.rs b/blahd/src/database.rs index 73e1584..244ec89 100644 --- a/blahd/src/database.rs +++ b/blahd/src/database.rs @@ -2,7 +2,6 @@ use std::num::NonZero; use std::path::PathBuf; use anyhow::{ensure, Context}; -use axum::http::StatusCode; use blah_types::identity::UserIdentityDesc; use blah_types::{ ChatPayload, Id, MemberPermission, PubKey, RoomAttrs, RoomMetadata, ServerPermission, @@ -13,7 +12,7 @@ use rusqlite::{named_params, params, prepare_cached_and_bind, Connection, OpenFl use serde::Deserialize; use serde_inline_default::serde_inline_default; -use crate::ApiError; +use crate::middleware::ApiError; #[cfg(test)] mod tests; @@ -182,13 +181,7 @@ pub trait TransactionOps { ) .raw_query() .next()? - .ok_or_else(|| { - error_response!( - StatusCode::NOT_FOUND, - "not_found", - "the user does not exist", - ) - }) + .ok_or(ApiError::UserNotFound) .and_then(|row| Ok((row.get(0)?, row.get(1)?))) } @@ -203,13 +196,7 @@ pub trait TransactionOps { ) .raw_query() .next()? - .ok_or_else(|| { - error_response!( - StatusCode::NOT_FOUND, - "user_not_found", - "the user does not exists", - ) - }) + .ok_or(ApiError::UserNotFound) .and_then(|row| Ok((row.get(0)?, row.get(1)?))) } @@ -229,13 +216,7 @@ pub trait TransactionOps { ) .raw_query() .next()? - .ok_or_else(|| { - error_response!( - StatusCode::NOT_FOUND, - "room_not_found", - "the room does not exist or user is not a room member", - ) - }) + .ok_or(ApiError::RoomNotFound) .and_then(|row| Ok((row.get(0)?, row.get(1)?, row.get(2)?))) } @@ -258,13 +239,7 @@ pub trait TransactionOps { }) .transpose()? .filter(|(attrs, _)| attrs.contains(filter)) - .ok_or_else(|| { - error_response!( - StatusCode::NOT_FOUND, - "room_not_found", - "the room does not exist" - ) - }) + .ok_or(ApiError::RoomNotFound) } // FIXME: Eliminate this. @@ -432,13 +407,9 @@ pub trait TransactionOps { ) .raw_query() .next()? - .ok_or_else(|| { - error_response!( - StatusCode::CONFLICT, - "conflict", - "racing register, please try again later", - ) - }) + .ok_or(ApiError::Conflict( + "racing registration, please try again later", + )) .and_then(|row| Ok(row.get::<_, i64>(0)?))?; // Delete existing act_keys. @@ -504,13 +475,10 @@ pub trait TransactionOps { " ) .raw_execute()?; - if updated == 0 { - return Err(error_response!( - StatusCode::CONFLICT, - "exists", - "room already exists" - )); - } + api_ensure!( + updated != 0, + ApiError::Exists("peer chat room already exists") + ); // TODO: Limit permission of the src user? let perm = MemberPermission::MAX_PEER_CHAT; @@ -548,11 +516,7 @@ pub trait TransactionOps { ) .raw_execute()?; if updated != 1 { - return Err(error_response!( - StatusCode::CONFLICT, - "exists", - "the user already joined the room", - )); + return Err(ApiError::Exists("the user already joined the room")); } Ok(()) } @@ -602,13 +566,8 @@ pub trait TransactionOps { .map(|row| row.get(0)) .transpose()? .unwrap_or(Id(0)); - if max_cid_in_room < cid { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_request", - "invalid cid", - )); - } + // FIXME: This leaks room existence information. + api_ensure!(cid <= max_cid_in_room, "invalid cid"); let updated = prepare_cached_and_bind!( self.conn(), r" @@ -619,11 +578,7 @@ pub trait TransactionOps { ) .raw_execute()?; if updated != 1 { - return Err(error_response!( - StatusCode::NOT_FOUND, - "room_not_found", - "the room does not exist or the user is not a room member", - )); + return Err(ApiError::RoomNotFound); } Ok(()) diff --git a/blahd/src/event.rs b/blahd/src/event.rs index 0e99253..b2657b6 100644 --- a/blahd/src/event.rs +++ b/blahd/src/event.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use std::task::{Context, Poll}; use std::time::Duration; -use anyhow::{anyhow, bail, Context as _, Result}; +use anyhow::{bail, Context as _, Result}; use axum::extract::ws::{Message, WebSocket}; use blah_types::{AuthPayload, Signed, SignedChatMsg}; use futures_util::future::Either; @@ -143,10 +143,7 @@ pub async fn handle_ws(st: Arc, ws: &mut WebSocket) -> Result>(&payload)?; st.verify_signed_data(&auth)?; - let (uid, _) = st - .db - .with_read(|txn| txn.get_user(&auth.signee.user)) - .map_err(|err| anyhow!("{}", err.message))?; + let (uid, _) = st.db.with_read(|txn| txn.get_user(&auth.signee.user))?; uid }; diff --git a/blahd/src/lib.rs b/blahd/src/lib.rs index be55ea6..712a027 100644 --- a/blahd/src/lib.rs +++ b/blahd/src/lib.rs @@ -102,32 +102,20 @@ impl AppState { } fn verify_signed_data(&self, data: &Signed) -> Result<(), ApiError> { - let Ok(()) = data.verify() else { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_signature", - "signature verification failed" - )); - }; + api_ensure!(data.verify().is_ok(), "signature verification failed"); let timestamp_diff = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .expect("after UNIX epoch") .as_secs() .abs_diff(data.signee.timestamp); - if timestamp_diff > self.config.timestamp_tolerance_secs { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_timestamp", - "invalid timestamp, off by {timestamp_diff}s" - )); - } - if !self.used_nonces.lock().try_insert(data.signee.nonce) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "duplicated_nonce", - "duplicated nonce", - )); - } + api_ensure!( + timestamp_diff <= self.config.timestamp_tolerance_secs, + "invalid timestamp", + ); + api_ensure!( + self.used_nonces.lock().try_insert(data.signee.nonce), + "used nonce", + ); Ok(()) } } @@ -196,7 +184,7 @@ async fn user_get( None => None, Some(user) => st.db.with_read(|txn| txn.get_user(&user)).ok(), } - .ok_or_else(|| error_response!(StatusCode::NOT_FOUND, "not_found", "user does not exist")) + .ok_or(ApiError::UserNotFound) })(); match ret { @@ -288,23 +276,17 @@ async fn room_create_group( user: &UserKey, op: CreateGroup, ) -> Result, ApiError> { - if !RoomAttrs::GROUP_ATTRS.contains(op.attrs) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "deserialization", - "invalid room attributes", - )); - } + api_ensure!( + RoomAttrs::GROUP_ATTRS.contains(op.attrs), + "invalid group attributes", + ); let rid = st.db.with_write(|conn| { let (uid, perm) = conn.get_user(user)?; - if !perm.contains(ServerPermission::CREATE_ROOM) { - return Err(error_response!( - StatusCode::FORBIDDEN, - "permission_denied", - "the user does not have permission to create room", - )); - } + api_ensure!( + perm.contains(ServerPermission::CREATE_ROOM), + ApiError::PermissionDenied("the user does not have permission to create room"), + ); let rid = Id::gen(); conn.create_group(rid, &op.title, op.attrs)?; conn.add_room_member(rid, uid, MemberPermission::ALL)?; @@ -320,13 +302,10 @@ async fn room_create_peer_chat( op: CreatePeerChat, ) -> Result, ApiError> { let tgt_user_id_key = op.peer; - if tgt_user_id_key == src_user.id_key { - return Err(error_response!( - StatusCode::NOT_IMPLEMENTED, - "not_implemented", - "self-chat is not implemented yet", - )); - } + api_ensure!( + tgt_user_id_key != src_user.id_key, + ApiError::NotImplemented("self-chat is not implemented yet"), + ); // TODO: Access control and throttling. let rid = st.db.with_write(|txn| { @@ -335,13 +314,7 @@ async fn room_create_peer_chat( .get_user_by_id_key(&tgt_user_id_key) .ok() .filter(|(_, perm)| perm.contains(ServerPermission::ACCEPT_PEER_CHAT)) - .ok_or_else(|| { - error_response!( - StatusCode::NOT_FOUND, - "peer_user_not_found", - "peer user does not exist or disallows peer chat", - ) - })?; + .ok_or(ApiError::PeerUserNotFound)?; let rid = Id::gen_peer_chat_rid(); txn.create_peer_room_with_members(rid, RoomAttrs::PEER_CHAT, src_uid, tgt_uid)?; Ok(rid) @@ -509,23 +482,14 @@ async fn room_msg_post( R(Path(rid), _): RE>, SignedJson(chat): SignedJson, ) -> Result, ApiError> { - if rid != chat.signee.payload.room { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_request", - "URI and payload room id mismatch", - )); - } + api_ensure!(rid == chat.signee.payload.room, "room id mismatch with URI"); let (cid, members) = st.db.with_write(|txn| { let (uid, perm, ..) = txn.get_room_member(rid, &chat.signee.user)?; - if !perm.contains(MemberPermission::POST_CHAT) { - return Err(error_response!( - StatusCode::FORBIDDEN, - "permission_denied", - "the user does not have permission to post in the room", - )); - } + api_ensure!( + perm.contains(MemberPermission::POST_CHAT), + ApiError::PermissionDenied("the user does not have permission to post in the room"), + ); let cid = Id::gen(); txn.add_room_chat_msg(rid, uid, cid, &chat)?; @@ -556,47 +520,26 @@ async fn room_admin( R(Path(rid), _): RE>, SignedJson(op): SignedJson, ) -> Result { - if rid != op.signee.payload.room { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_request", - "URI and payload room id mismatch", - )); - } - if rid.is_peer_chat() { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_request", - "operation not permitted on peer chat rooms", - )); - } + api_ensure!(rid == op.signee.payload.room, "room id mismatch with URI"); + api_ensure!(!rid.is_peer_chat(), "cannot operate on a peer chat room"); match op.signee.payload.op { RoomAdminOp::AddMember { user, permission } => { - if user != op.signee.user.id_key { - return Err(error_response!( - StatusCode::NOT_IMPLEMENTED, - "not_implemented", - "only self-adding is implemented yet", - )); - } - if !MemberPermission::MAX_SELF_ADD.contains(permission) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "deserialization", - "invalid 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 } => { - if user != op.signee.user.id_key { - return Err(error_response!( - StatusCode::NOT_IMPLEMENTED, - "not_implemented", - "only self-removing is implemented yet", - )); - } + api_ensure!( + user == op.signee.user.id_key, + ApiError::NotImplemented("only self-removal is implemented yet"), + ); room_leave(&st, rid, &op.signee.user).await?; } } @@ -622,15 +565,11 @@ async fn room_join( async fn room_leave(st: &AppState, rid: Id, user: &UserKey) -> Result<(), ApiError> { st.db.with_write(|txn| { + api_ensure!(!rid.is_peer_chat(), "cannot leave a peer chat room"); let (uid, ..) = txn.get_room_member(rid, user)?; let (attrs, _) = txn.get_room_having(rid, RoomAttrs::empty())?; - if attrs.contains(RoomAttrs::PEER_CHAT) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_operation", - "cannot leave a peer chat room without deleting it", - )); - } + // Sanity check. + assert!(!attrs.contains(RoomAttrs::PEER_CHAT)); txn.remove_room_member(rid, uid)?; Ok(()) }) @@ -641,23 +580,14 @@ async fn room_delete( R(Path(rid), _): RE>, SignedJson(op): SignedJson, ) -> Result { - if rid != op.signee.payload.room { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_request", - "URI and payload room id mismatch", - )); - } + api_ensure!(rid == op.signee.payload.room, "room id mismatch with URI"); st.db.with_write(|txn| { // TODO: Should we only shadow delete here? let (_uid, perm, ..) = txn.get_room_member(rid, &op.signee.user)?; - if !perm.contains(MemberPermission::DELETE_ROOM) { - return Err(error_response!( - StatusCode::FORBIDDEN, - "permission_denied", - "the user does not have permission to delete the room", - )); - } + api_ensure!( + perm.contains(MemberPermission::DELETE_ROOM), + ApiError::PermissionDenied("the user does not have permission to delete the room") + ); txn.delete_room(rid)?; Ok(StatusCode::NO_CONTENT) }) diff --git a/blahd/src/middleware.rs b/blahd/src/middleware.rs index d79b77e..8ab8316 100644 --- a/blahd/src/middleware.rs +++ b/blahd/src/middleware.rs @@ -10,56 +10,116 @@ use axum::response::{IntoResponse, Response}; use axum::{async_trait, Json}; use blah_types::{AuthPayload, Signed, UserKey}; use serde::de::DeserializeOwned; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use crate::AppState; +macro_rules! define_api_error { + ( + $(#[$meta:meta])* + $vis:vis enum $name:ident { + $( + $variant:ident + $(= ($status1:expr, $message1:expr))? + $(($(;$marker2:tt)? $ty:ty) = ($status2:expr))? + , + )* + } + ) => { + $(#[$meta])* + $vis enum $name { + $($variant $(($ty))?,)* + } + + impl $name { + fn to_response_tuple(&self) -> (StatusCode, &'static str, &str) { + paste::paste! { + match self { + $( + Self::$variant + $(=> ($status1, stringify!([<$variant:snake>]), $message1))? + $((message) => ($status2, stringify!([<$variant:snake>]), message))? + , + )* + } + } + } + } + + }; +} + +define_api_error! { + /// Error response body for json endpoints. -/// -/// Mostly following: -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq)] #[must_use] -pub struct ApiError { - #[serde(skip, default)] - pub status: StatusCode, - pub code: String, - pub message: String, +pub enum ApiError { + InvalidRequest(Box) = (StatusCode::BAD_REQUEST), + Unauthorized(&'static str) = (StatusCode::UNAUTHORIZED), + PermissionDenied(&'static str) = (StatusCode::FORBIDDEN), + Disabled(&'static str) = (StatusCode::FORBIDDEN), + 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"), + Conflict(&'static str) = (StatusCode::CONFLICT), + Exists(&'static str) = (StatusCode::CONFLICT), + FetchIdDescription(Box) = (StatusCode::UNPROCESSABLE_ENTITY), + InvalidIdDescription(Box) = (StatusCode::UNPROCESSABLE_ENTITY), + + ServerError = (StatusCode::INTERNAL_SERVER_ERROR, "internal server error"), + NotImplemented(&'static str) = (StatusCode::NOT_IMPLEMENTED), } -impl fmt::Display for ApiError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "api error status={} code={}: {}", - self.status, self.code, self.message, - ) - } } -impl std::error::Error for ApiError {} - -macro_rules! error_response { - ($status:expr, $code:literal, $msg:literal $(, $msg_args:expr)* $(,)?) => { - $crate::middleware::ApiError { - status: $status, - code: $code.to_owned(), - message: ::std::format!($msg $(, $msg_args)*), +macro_rules! api_ensure { + ($assertion:expr, $msg:literal $(,)?) => { + if !$assertion { + return Err($crate::middleware::ApiError::InvalidRequest($msg.into())); + } + }; + ($assertion:expr, $err:expr $(,)?) => { + if !$assertion { + return Err($err); } }; } +/// Response structure mostly follows: +/// +/// Only `error/{code,message}` are provided and are always available. impl IntoResponse for ApiError { fn into_response(self) -> Response { #[derive(Serialize)] struct Resp<'a> { - error: &'a ApiError, + error: Error<'a>, } - let mut resp = Json(Resp { error: &self }).into_response(); - *resp.status_mut() = self.status; + #[derive(Serialize)] + struct Error<'a> { + code: &'a str, + message: &'a str, + } + + let (status, code, message) = self.to_response_tuple(); + let mut resp = Json(Resp { + error: Error { code, message }, + }) + .into_response(); + *resp.status_mut() = status; resp } } +impl fmt::Display for ApiError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let (_, code, message) = self.to_response_tuple(); + write!(f, "({code}) {message}") + } +} + +impl std::error::Error for ApiError {} + // For infallible extractors. impl From for ApiError { fn from(v: Infallible) -> Self { @@ -72,14 +132,7 @@ macro_rules! define_from_deser_rejection { $( impl From<$ty> for ApiError { fn from(rej: $ty) -> Self { - tracing::debug!(?rej, "rejected"); - error_response!( - StatusCode::BAD_REQUEST, - "deserialization", - "invalid {}: {}", - $name, - rej, - ) + ApiError::InvalidRequest(format!(concat!("invalid ", $name, "{}"), rej).into()) } } )* @@ -95,11 +148,7 @@ define_from_deser_rejection! { impl From for ApiError { fn from(err: rusqlite::Error) -> Self { tracing::error!(%err, backtrace = %Backtrace::force_capture(), "database error"); - error_response!( - StatusCode::INTERNAL_SERVER_ERROR, - "server_error", - "internal server error", - ) + ApiError::ServerError } } @@ -133,11 +182,7 @@ pub enum AuthRejection { impl From for ApiError { fn from(rej: AuthRejection) -> Self { match rej { - AuthRejection::None => error_response!( - StatusCode::UNAUTHORIZED, - "unauthorized", - "missing authorization header" - ), + AuthRejection::None => ApiError::Unauthorized("missing authorization header"), AuthRejection::Invalid(err) => err, } } @@ -188,11 +233,9 @@ where let st = >::from_ref(state); let data = - serde_json::from_slice::>(auth.as_bytes()).map_err(|err| { - AuthRejection::Invalid(error_response!( - StatusCode::BAD_REQUEST, - "deserialization", - "invalid authorization header: {err}", + serde_json::from_slice::>(auth.as_bytes()).map_err(|_err| { + AuthRejection::Invalid(ApiError::InvalidRequest( + "invalid authorization header".into(), )) })?; st.verify_signed_data(&data) diff --git a/blahd/src/register.rs b/blahd/src/register.rs index 15a99f6..0127be0 100644 --- a/blahd/src/register.rs +++ b/blahd/src/register.rs @@ -151,37 +151,20 @@ pub async fn user_register( msg: Signed, ) -> Result { if !st.config.register.enable_public { - return Err(error_response!( - StatusCode::FORBIDDEN, - "disabled", - "public registration is disabled", - )); + return Err(ApiError::Disabled("public registration is disabled")); } let reg = &msg.signee.payload; // Basic validity check. - if reg.server_url != st.config.base_url { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_server_url", - "unexpected server url in payload", - )); - } + api_ensure!(reg.server_url == st.config.base_url, "server url mismatch"); if let Err(err) = st.config.register.validate_id_url(®.id_url) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_id_url", - "{err}", - )); - } - if !st.register.nonce().contains(®.challenge_nonce) { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_challenge_nonce", - "invalid or outdated challenge nonce", - )); + return Err(ApiError::Disabled(err)); } + api_ensure!( + st.register.nonce().contains(®.challenge_nonce), + "invalid challenge nonce", + ); // Challenge verification. let expect_bits = st.register.config.difficulty; @@ -197,13 +180,7 @@ pub async fn user_register( let (bytes, bits) = (expect_bits as usize / 8, expect_bits as usize % 8); // NB. Shift by 8 would overflow and wrap around for u8. Convert it to u32 first. let ok = hash[..bytes].iter().all(|&b| b == 0) && (hash[bytes] as u32) >> (8 - bits) == 0; - if !ok { - return Err(error_response!( - StatusCode::BAD_REQUEST, - "invalid_challenge_hash", - "challenge failed", - )); - } + api_ensure!(ok, "hash challenge failed"); } // TODO: Limit concurrency for the same domain and/or id_key? @@ -235,13 +212,13 @@ pub async fn user_register( let id_desc = match fut.await { Ok(id_desc) => id_desc, Err(err) => { - return Err(error_response!( - StatusCode::UNAUTHORIZED, - "fetch_id_description", - "failed to fetch identity description from {}: {}", - reg.id_url, - err, - )) + return Err(ApiError::FetchIdDescription( + format!( + "failed to fetch identity description from {}: {}", + reg.id_url, err, + ) + .into(), + )); } }; @@ -250,11 +227,7 @@ pub async fn user_register( id_desc.verify(Some(®.id_url), fetch_time)?; Ok(()) })() { - return Err(error_response!( - StatusCode::UNAUTHORIZED, - "invalid_id_description", - "{err}", - )); + return Err(ApiError::InvalidIdDescription(err.to_string().into())); } // Now the identity is verified. diff --git a/blahd/tests/webapi.rs b/blahd/tests/webapi.rs index 17eecbb..7682f84 100644 --- a/blahd/tests/webapi.rs +++ b/blahd/tests/webapi.rs @@ -16,7 +16,7 @@ use blah_types::{ ServerPermission, SignExt, Signed, SignedChatMsg, UserKey, UserRegisterPayload, WithMsgId, X_BLAH_DIFFICULTY, X_BLAH_NONCE, }; -use blahd::{ApiError, AppState, Database, RoomList, RoomMsgs}; +use blahd::{AppState, Database, RoomList, RoomMsgs}; use ed25519_dalek::SigningKey; use expect_test::expect; use futures_util::future::BoxFuture; @@ -91,33 +91,46 @@ enum NoContent {} trait ResultExt { fn expect_api_err(self, status: StatusCode, code: &str); + fn expect_invalid_request(self, message: &str); } impl ResultExt for Result { #[track_caller] fn expect_api_err(self, status: StatusCode, code: &str) { - let err = self - .unwrap_err() - .downcast::() - .unwrap() - .error; + let err = self.unwrap_err().downcast::().unwrap(); assert_eq!( (err.status, &*err.code), (status, code), - "unexpecteed API error: {err:?}", + "unexpecteed API error: {err}", + ); + } + + #[track_caller] + fn expect_invalid_request(self, message: &str) { + let err = self.unwrap_err().downcast::().unwrap(); + assert_eq!( + (err.status, &*err.code, &*err.message), + (StatusCode::BAD_REQUEST, "invalid_request", message), + "unexpected API error: {err}" ); } } #[derive(Debug)] pub struct ApiErrorWithHeaders { - error: ApiError, + status: StatusCode, + code: String, + message: String, headers: HeaderMap, } impl fmt::Display for ApiErrorWithHeaders { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.error.fmt(f) + write!( + f, + "status={} code={}: {}", + self.status, self.code, self.message, + ) } } @@ -193,12 +206,23 @@ impl Server { if !status.is_success() { #[derive(Deserialize)] struct Resp { - error: ApiError, + error: RespErr, } - let Resp { mut error } = serde_json::from_str(&resp_str) + #[derive(Deserialize)] + struct RespErr { + code: String, + message: String, + } + + let resp = serde_json::from_str::(&resp_str) .with_context(|| format!("failed to parse response {resp_str:?}"))?; - error.status = status; - Err(ApiErrorWithHeaders { error, headers }.into()) + Err(ApiErrorWithHeaders { + status, + code: resp.error.code, + message: resp.error.message, + headers, + } + .into()) } else if resp_str.is_empty() { Ok(None) } else { @@ -336,7 +360,7 @@ impl Server { Ok(None) => Ok(()), Err(err) => { let err = err.downcast::().unwrap(); - assert_eq!(err.error.status, StatusCode::NOT_FOUND); + assert_eq!(err.status, StatusCode::NOT_FOUND); if !err.headers.contains_key(X_BLAH_NONCE) { return Err(None); } @@ -555,7 +579,7 @@ async fn room_join_leave(server: Server) { server .join_room(rid_priv, &BOB, MemberPermission::ALL) .await - .expect_api_err(StatusCode::BAD_REQUEST, "deserialization"); + .expect_invalid_request("invalid initial permission"); // Bob is joined now. assert_eq!( @@ -626,12 +650,12 @@ async fn room_chat_post_read(server: Server) { // Duplicated chat. post(rid_pub, chat2.clone()) .await - .expect_api_err(StatusCode::BAD_REQUEST, "duplicated_nonce"); + .expect_invalid_request("used nonce"); // Wrong room. post(rid_pub, chat(rid_priv, &ALICE, "wrong room")) .await - .expect_api_err(StatusCode::BAD_REQUEST, "invalid_request"); + .expect_invalid_request("room id mismatch with URI"); // Not a member. post(rid_pub, chat(rid_pub, &BOB, "not a member")) @@ -1066,12 +1090,14 @@ async fn register_flow(server: Server) { register_fast(&req) .await - .expect_api_err(StatusCode::BAD_REQUEST, "invalid_server_url"); + .expect_invalid_request("server url mismatch"); req.server_url = BASE_URL.parse().unwrap(); + // Trailing dot in id_url. + // TODO: Rule this out in `IdUrl` parser? register_fast(&req) .await - .expect_api_err(StatusCode::BAD_REQUEST, "invalid_id_url"); + .expect_api_err(StatusCode::FORBIDDEN, "disabled"); // Test identity server. type DynHandler = Box BoxFuture<'static, (StatusCode, String)> + Send>; @@ -1108,19 +1134,19 @@ async fn register_flow(server: Server) { register_fast(&req) .await - .expect_api_err(StatusCode::BAD_REQUEST, "invalid_challenge_nonce"); + .expect_invalid_request("invalid challenge nonce"); req.challenge_nonce += 1; register(sign_with_difficulty(&req, false)) .await - .expect_api_err(StatusCode::BAD_REQUEST, "invalid_challenge_hash"); + .expect_invalid_request("hash challenge failed"); //// Starting here, early validation passed. //// // id_url 404 register(sign_with_difficulty(&req, true)) .await - .expect_api_err(StatusCode::UNAUTHORIZED, "fetch_id_description"); + .expect_api_err(StatusCode::UNPROCESSABLE_ENTITY, "fetch_id_description"); // Timeout set_handler! {{ @@ -1130,7 +1156,7 @@ async fn register_flow(server: Server) { let inst = Instant::now(); register(sign_with_difficulty(&req, true)) .await - .expect_api_err(StatusCode::UNAUTHORIZED, "fetch_id_description"); + .expect_api_err(StatusCode::UNPROCESSABLE_ENTITY, "fetch_id_description"); let elapsed = inst.elapsed(); assert!( elapsed.abs_diff(Duration::from_secs(1)) < TIME_TOLERANCE, @@ -1143,7 +1169,7 @@ async fn register_flow(server: Server) { }} register(sign_with_difficulty(&req, true)) .await - .expect_api_err(StatusCode::UNAUTHORIZED, "fetch_id_description"); + .expect_api_err(StatusCode::UNPROCESSABLE_ENTITY, "fetch_id_description"); let set_id_desc = |desc: &UserIdentityDesc| { let desc = serde_json::to_string(&desc).unwrap(); @@ -1182,14 +1208,14 @@ async fn register_flow(server: Server) { set_id_desc(&id_desc); register(sign_with_difficulty(&req, true)) .await - .expect_api_err(StatusCode::UNAUTHORIZED, "invalid_id_description"); + .expect_api_err(StatusCode::UNPROCESSABLE_ENTITY, "invalid_id_description"); // Still not registered. server.get_me(Some(&CAROL)).await.unwrap_err(); server .join_room(rid, &CAROL, MemberPermission::MAX_SELF_ADD) .await - .expect_api_err(StatusCode::NOT_FOUND, "not_found"); + .expect_api_err(StatusCode::NOT_FOUND, "user_not_found"); // Finally pass. id_desc.profile = sign_profile(req.id_url.clone()); @@ -1257,11 +1283,8 @@ unsafe_allow_id_url_single_label = {allow_single_label} let ret = server .request::<_, ()>(Method::POST, "/user/me", None, Some(req)) .await; - if !enabled { - ret.expect_api_err(StatusCode::FORBIDDEN, "disabled"); - } else { - ret.expect_api_err(StatusCode::BAD_REQUEST, "invalid_id_url"); - } + // Unpermitted due to server restriction. + ret.expect_api_err(StatusCode::FORBIDDEN, "disabled"); } #[rstest] diff --git a/docs/webapi.yaml b/docs/webapi.yaml index 0b16f71..f95a7dc 100644 --- a/docs/webapi.yaml +++ b/docs/webapi.yaml @@ -108,7 +108,7 @@ paths: description: User successfully registered. 400: - description: Invalid request format, or invalid challenge. + description: Invalid request format or any invalid fields in the request. content: application/json: schema: @@ -123,6 +123,15 @@ paths: schema: $ref: '#/components/schemas/ApiError' + 403: + description: | + Server disallows registration, either due to server restriction or + unacceptable id_url. + content: + application/json: + schema: + $ref: '#/components/schemas/ApiError' + 409: description: | User state changed during the operation. Could retry later. @@ -131,6 +140,16 @@ paths: schema: $ref: '#/components/schemas/ApiError' + 422: + description: | + Fail to process identity description. Could be failure to fetch + remote description, unacceptable result from id_url, or any fields + (eg. signatures) in the returned description being invalid. + content: + application/json: + schema: + $ref: '#/components/schemas/ApiError' + /_blah/room: get: summary: List rooms @@ -283,6 +302,13 @@ paths: schema: $ref: '#/components/schemas/ApiError' + 404: + description: | + Room does not exist or the user does not have permission to access it. + content: + application/json: + schema: + $ref: '#/components/schemas/ApiError' /_blah/room/{rid}/admin: post: