diff --git a/server/base/error.rs b/server/base/error.rs index 2066fe5..04d70ad 100644 --- a/server/base/error.rs +++ b/server/base/error.rs @@ -29,6 +29,12 @@ pub struct Error { } impl Error { + pub fn wrap>(kind: ErrorKind, e: E) -> Self { + Self { + inner: e.into().context(kind), + } + } + pub fn kind(&self) -> ErrorKind { *self.inner.get_context() } @@ -146,7 +152,7 @@ where /// Like `failure::bail!`, but the first argument specifies a type as an `ErrorKind`. /// -/// Example: +/// Example with positional arguments: /// ``` /// use moonfire_base::bail_t; /// let e = || -> Result<(), moonfire_base::Error> { @@ -155,10 +161,21 @@ where /// assert_eq!(e.kind(), moonfire_base::ErrorKind::Unauthenticated); /// assert_eq!(e.to_string(), "Unauthenticated: unknown user: slamb"); /// ``` +/// +/// Example with named arguments: +/// ``` +/// use moonfire_base::bail_t; +/// let e = || -> Result<(), moonfire_base::Error> { +/// let user = "slamb"; +/// bail_t!(Unauthenticated, "unknown user: {user}"); +/// }().unwrap_err(); +/// assert_eq!(e.kind(), moonfire_base::ErrorKind::Unauthenticated); +/// assert_eq!(e.to_string(), "Unauthenticated: unknown user: slamb"); +/// ``` #[macro_export] macro_rules! bail_t { - ($t:ident, $e:expr) => { - return Err($crate::Error::from(failure::err_msg($e).context($crate::ErrorKind::$t)).into()); + ($t:ident, $fmt:expr) => { + return Err($crate::Error::from(failure::err_msg(format!($fmt)).context($crate::ErrorKind::$t)).into()); }; ($t:ident, $fmt:expr, $($arg:tt)+) => { return Err($crate::Error::from(failure::err_msg(format!($fmt, $($arg)+)).context($crate::ErrorKind::$t)).into()); diff --git a/server/db/auth.rs b/server/db/auth.rs index 1843a40..62eede6 100644 --- a/server/db/auth.rs +++ b/server/db/auth.rs @@ -6,8 +6,7 @@ use crate::json::UserConfig; use crate::schema::Permissions; -use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt as _}; -use failure::{bail, format_err, Error, Fail, ResultExt as _}; +use base::{bail_t, format_err_t, strutil, Error, ErrorKind, ResultExt as _}; use fnv::FnvHashMap; use protobuf::Message; use ring::rand::{SecureRandom, SystemRandom}; @@ -96,14 +95,14 @@ impl User { (Some(p), Some(h)) => (p, h), _ => return Ok(false), }; - let hash = PasswordHash::new(hash) - .with_context(|_| { - format!( - "bad stored password hash for user {:?}: {:?}", - self.username, hash - ) - }) - .context(ErrorKind::DataLoss)?; + let hash = PasswordHash::new(hash).map_err(|e| { + format_err_t!( + DataLoss, + "bad stored password hash for user {:?}: {}", + self.username, + e, + ) + })?; match scrypt::Scrypt.verify_password(password.as_bytes(), &hash) { Ok(()) => Ok(true), Err(scrypt::password_hash::errors::Error::Password) => { @@ -111,13 +110,12 @@ impl User { self.password_failure_count += 1; Ok(false) } - Err(e) => Err(e - .context(format!( - "unable to verify password for user {:?}", - self.username - )) - .context(ErrorKind::Internal) - .into()), + Err(e) => Err(format_err_t!( + Internal, + "unable to verify password for user {:?}: {}", + self.username, + e + )), } } } @@ -236,7 +234,7 @@ impl FromStr for SessionFlag { "secure" => Ok(Self::Secure), "same-site" => Ok(Self::SameSite), "same-site-strict" => Ok(Self::SameSiteStrict), - _ => bail!("No such session flag {:?}", s), + _ => bail_t!(InvalidArgument, "No such session flag {:?}", s), } } } @@ -286,9 +284,10 @@ pub struct RawSessionId([u8; 48]); impl RawSessionId { pub fn decode_base64(input: &[u8]) -> Result { let mut s = RawSessionId([0u8; 48]); - let l = ::base64::decode_config_slice(input, ::base64::STANDARD_NO_PAD, &mut s.0[..])?; + let l = ::base64::decode_config_slice(input, ::base64::STANDARD_NO_PAD, &mut s.0[..]) + .map_err(|e| format_err_t!(InvalidArgument, "bad session id: {e}"))?; if l != 48 { - bail!("session id must be 48 bytes"); + bail_t!(InvalidArgument, "session id must be 48 bytes"); } Ok(s) } @@ -334,9 +333,10 @@ impl SessionHash { pub fn decode_base64(input: &[u8]) -> Result { let mut h = SessionHash([0u8; 24]); - let l = ::base64::decode_config_slice(input, ::base64::STANDARD_NO_PAD, &mut h.0[..])?; + let l = ::base64::decode_config_slice(input, ::base64::STANDARD_NO_PAD, &mut h.0[..]) + .map_err(|e| format_err_t!(InvalidArgument, "invalid session hash: {e}"))?; if l != 24 { - bail!("session hash must be 24 bytes"); + bail_t!(InvalidArgument, "session hash must be 24 bytes"); } Ok(h) } @@ -362,7 +362,7 @@ impl rusqlite::types::FromSql for Seed { let b = value.as_blob()?; if b.len() != 32 { return Err(rusqlite::types::FromSqlError::Other(Box::new( - format_err!("expected a 32-byte seed").compat(), + format_err_t!(Internal, "expected a 32-byte seed").compat(), ))); } let mut s = Seed::default(); @@ -395,8 +395,9 @@ impl State { sessions: FnvHashMap::default(), rand: ring::rand::SystemRandom::new(), }; - let mut stmt = conn.prepare( - r#" + let mut stmt = conn + .prepare( + r#" select id, username, @@ -408,22 +409,30 @@ impl State { from user "#, - )?; - let mut rows = stmt.query(params![])?; - while let Some(row) = rows.next()? { - let id = row.get(0)?; - let name: String = row.get(1)?; + ) + .err_kind(ErrorKind::Unknown)?; + let mut rows = stmt.query(params![]).err_kind(ErrorKind::Unknown)?; + while let Some(row) = rows.next().err_kind(ErrorKind::Unknown)? { + let id = row.get(0).err_kind(ErrorKind::Unknown)?; + let name: String = row.get(1).err_kind(ErrorKind::Unknown)?; let mut permissions = Permissions::new(); - permissions.merge_from_bytes(row.get_ref(6)?.as_blob()?)?; + permissions + .merge_from_bytes( + row.get_ref(6) + .err_kind(ErrorKind::Unknown)? + .as_blob() + .err_kind(ErrorKind::Unknown)?, + ) + .err_kind(ErrorKind::Unknown)?; state.users_by_id.insert( id, User { id, username: name.clone(), - config: row.get(2)?, - password_hash: row.get(3)?, - password_id: row.get(4)?, - password_failure_count: row.get(5)?, + config: row.get(2).err_kind(ErrorKind::Unknown)?, + password_hash: row.get(3).err_kind(ErrorKind::Unknown)?, + password_id: row.get(4).err_kind(ErrorKind::Unknown)?, + password_failure_count: row.get(5).err_kind(ErrorKind::Unknown)?, dirty: false, permissions, }, @@ -470,7 +479,7 @@ impl State { id = :id "#, ) - .context(ErrorKind::Unknown)?; + .err_kind(ErrorKind::Unknown)?; let e = self.users_by_id.entry(id); let e = match e { ::std::collections::btree_map::Entry::Vacant(_) => panic!("missing uid {id}!"), @@ -497,7 +506,7 @@ impl State { ":id": &id, ":permissions": &permissions, }) - .context(ErrorKind::Unknown)?; + .err_kind(ErrorKind::Unknown)?; } let u = e.into_mut(); if u.username != change.username { @@ -523,7 +532,7 @@ impl State { values (:username, :password_hash, :config, :permissions) "#, ) - .context(ErrorKind::Unknown)?; + .err_kind(ErrorKind::Unknown)?; let password_hash = change.set_password_hash.unwrap_or(None); let permissions = change .permissions @@ -535,7 +544,7 @@ impl State { ":config": &change.config, ":permissions": &permissions, }) - .context(ErrorKind::Unknown)?; + .err_kind(ErrorKind::Unknown)?; let id = conn.last_insert_rowid() as i32; self.users_by_name.insert(change.username.clone(), id); let e = self.users_by_id.entry(id); @@ -556,18 +565,22 @@ impl State { } pub fn delete_user(&mut self, conn: &mut Connection, id: i32) -> Result<(), base::Error> { - let tx = conn.transaction().context(ErrorKind::Unknown)?; + let tx = conn.transaction().err_kind(ErrorKind::Unknown)?; tx.execute("delete from user_session where user_id = ?", params![id]) - .context(ErrorKind::Unknown)?; + .err_kind(ErrorKind::Unknown)?; { let mut user_stmt = tx .prepare_cached("delete from user where id = ?") - .context(ErrorKind::Unknown)?; - if user_stmt.execute(params![id]).context(ErrorKind::Unknown)? != 1 { + .err_kind(ErrorKind::Unknown)?; + if user_stmt + .execute(params![id]) + .err_kind(ErrorKind::Unknown)? + != 1 + { bail_t!(NotFound, "user {} not found", id); } } - tx.commit().context(ErrorKind::Unknown)?; + tx.commit().err_kind(ErrorKind::Unknown)?; let name = self.users_by_id.remove(&id).unwrap().username; self.users_by_name .remove(&name) @@ -592,17 +605,17 @@ impl State { password: String, domain: Option>, session_flags: i32, - ) -> Result<(RawSessionId, &Session), Error> { + ) -> Result<(RawSessionId, &Session), base::Error> { let id = self .users_by_name .get(username) - .ok_or_else(|| format_err!("no such user {:?}", username))?; + .ok_or_else(|| format_err_t!(Unauthenticated, "no such user {username:?}"))?; let u = self .users_by_id .get_mut(id) .expect("users_by_name implies users_by_id"); if u.config.disabled { - bail!("user {:?} is disabled", username); + bail_t!(Unauthenticated, "user {username:?} is disabled"); } if !u.check_password(Some(&password))? { bail_t!(Unauthenticated, "incorrect password"); @@ -630,13 +643,13 @@ impl State { domain: Option>, flags: i32, permissions: Permissions, - ) -> Result<(RawSessionId, &'s Session), Error> { + ) -> Result<(RawSessionId, &'s Session), base::Error> { let u = self .users_by_id .get_mut(&uid) - .ok_or_else(|| format_err!("no such uid {:?}", uid))?; + .ok_or_else(|| format_err_t!(NotFound, "no such uid {:?}", uid))?; if u.config.disabled { - bail!("user is disabled"); + bail_t!(FailedPrecondition, "user is disabled"); } State::make_session_int( &self.rand, @@ -662,14 +675,15 @@ impl State { flags: i32, sessions: &'s mut FnvHashMap, permissions: Permissions, - ) -> Result<(RawSessionId, &'s Session), Error> { + ) -> Result<(RawSessionId, &'s Session), base::Error> { let mut session_id = RawSessionId([0u8; 48]); rand.fill(&mut session_id.0).unwrap(); let mut seed = [0u8; 32]; rand.fill(&mut seed).unwrap(); let hash = session_id.hash(); - let mut stmt = conn.prepare_cached( - r#" + let mut stmt = conn + .prepare_cached( + r#" insert into user_session (session_id_hash, user_id, seed, flags, domain, creation_password_id, creation_time_sec, creation_user_agent, creation_peer_addr, @@ -679,7 +693,8 @@ impl State { :creation_user_agent, :creation_peer_addr, :permissions) "#, - )?; + ) + .err_kind(ErrorKind::Unknown)?; let addr = creation.addr_buf(); let addr: Option<&[u8]> = addr.as_ref().map(|a| a.as_ref()); let permissions_blob = permissions @@ -696,7 +711,8 @@ impl State { ":creation_user_agent": &creation.user_agent, ":creation_peer_addr": &addr, ":permissions": &permissions_blob, - })?; + }) + .err_kind(ErrorKind::Unknown)?; let e = match sessions.entry(hash) { ::std::collections::hash_map::Entry::Occupied(_) => panic!("duplicate session hash!"), ::std::collections::hash_map::Entry::Vacant(e) => e, @@ -761,8 +777,9 @@ impl State { ::std::collections::hash_map::Entry::Vacant(e) => e.insert(lookup_session(conn, hash)?), }; if s.revocation_reason.is_none() { - let mut stmt = conn.prepare( - r#" + let mut stmt = conn + .prepare( + r#" update user_session set revocation_time_sec = ?, @@ -773,7 +790,8 @@ impl State { where session_id_hash = ? "#, - )?; + ) + .err_kind(ErrorKind::Unknown)?; let addr = req.addr_buf(); let addr: Option<&[u8]> = addr.as_ref().map(|a| a.as_ref()); stmt.execute(params![ @@ -783,7 +801,8 @@ impl State { reason as i32, detail, &hash.0[..], - ])?; + ]) + .err_kind(ErrorKind::Unknown)?; s.revocation = req; s.revocation_reason = Some(reason as i32); } @@ -795,8 +814,9 @@ impl State { /// The caller is expected to call `post_flush` afterward if the transaction is /// successfully committed. pub fn flush(&self, tx: &Transaction) -> Result<(), Error> { - let mut u_stmt = tx.prepare( - r#" + let mut u_stmt = tx + .prepare( + r#" update user set password_failure_count = :password_failure_count, @@ -804,9 +824,11 @@ impl State { where id = :id "#, - )?; - let mut s_stmt = tx.prepare( - r#" + ) + .err_kind(ErrorKind::Unknown)?; + let mut s_stmt = tx + .prepare( + r#" update user_session set last_use_time_sec = :last_use_time_sec, @@ -816,7 +838,8 @@ impl State { where session_id_hash = :hash "#, - )?; + ) + .err_kind(ErrorKind::Unknown)?; for (&id, u) in &self.users_by_id { if !u.dirty { continue; @@ -825,11 +848,13 @@ impl State { "flushing user with hash: {}", u.password_hash.as_ref().unwrap() ); - u_stmt.execute(named_params! { - ":password_failure_count": &u.password_failure_count, - ":password_hash": &u.password_hash, - ":id": &id, - })?; + u_stmt + .execute(named_params! { + ":password_failure_count": &u.password_failure_count, + ":password_hash": &u.password_hash, + ":id": &id, + }) + .err_kind(ErrorKind::Unknown)?; } for (sh, s) in &self.sessions { if !s.dirty { @@ -837,13 +862,15 @@ impl State { } let addr = s.last_use.addr_buf(); let addr: Option<&[u8]> = addr.as_ref().map(|a| a.as_ref()); - let cnt = s_stmt.execute(named_params! { - ":last_use_time_sec": &s.last_use.when_sec, - ":last_use_user_agent": &s.last_use.user_agent, - ":last_use_peer_addr": &addr, - ":use_count": &s.use_count, - ":hash": &sh.0[..], - })?; + let cnt = s_stmt + .execute(named_params! { + ":last_use_time_sec": &s.last_use.when_sec, + ":last_use_user_agent": &s.last_use.user_agent, + ":last_use_peer_addr": &addr, + ":use_count": &s.use_count, + ":hash": &sh.0[..], + }) + .err_kind(ErrorKind::Unknown)?; debug_assert_eq!(cnt, 1); } Ok(()) @@ -892,51 +919,51 @@ fn lookup_session(conn: &Connection, hash: &SessionHash) -> Result Result, Error> { + pub fn get(&self) -> Result, base::Error> { Ok(self .dir .as_ref() - .ok_or_else(|| format_err!("sample file dir {} is closed", self.id))? + .ok_or_else(|| { + format_err_t!(FailedPrecondition, "sample file dir {} is closed", self.id) + })? .clone()) } @@ -2087,7 +2090,7 @@ impl LockedDatabase { password: String, domain: Option>, session_flags: i32, - ) -> Result<(RawSessionId, &Session), Error> { + ) -> Result<(RawSessionId, &Session), base::Error> { self.auth .login_by_password(&self.conn, req, username, password, domain, session_flags) } @@ -2099,7 +2102,7 @@ impl LockedDatabase { domain: Option>, flags: i32, permissions: schema::Permissions, - ) -> Result<(RawSessionId, &Session), Error> { + ) -> Result<(RawSessionId, &Session), base::Error> { self.auth .make_session(&self.conn, creation, uid, domain, flags, permissions) } @@ -2118,7 +2121,7 @@ impl LockedDatabase { detail: Option, req: auth::Request, hash: &auth::SessionHash, - ) -> Result<(), Error> { + ) -> Result<(), base::Error> { self.auth .revoke_session(&self.conn, reason, detail, req, hash) } diff --git a/server/src/cmds/login.rs b/server/src/cmds/login.rs index 65f5770..4c4096f 100644 --- a/server/src/cmds/login.rs +++ b/server/src/cmds/login.rs @@ -17,7 +17,7 @@ fn parse_perms(perms: String) -> Result Result, Error> { +fn parse_flags(flags: String) -> Result, base::Error> { flags .split(',') .map(|f| SessionFlag::from_str(f.trim())) diff --git a/server/src/web/mod.rs b/server/src/web/mod.rs index 3a09445..52c83e6 100644 --- a/server/src/web/mod.rs +++ b/server/src/web/mod.rs @@ -17,12 +17,14 @@ use self::path::Path; use crate::body::Body; use crate::json; use crate::mp4; +use base::format_err_t; +use base::Error; +use base::ResultExt; use base::{bail_t, clock::Clocks, ErrorKind}; use core::borrow::Borrow; use core::str::FromStr; use db::dir::SampleFileDir; use db::{auth, recording}; -use failure::{format_err, Error}; use fnv::FnvHashMap; use http::header::{self, HeaderValue}; use http::{status::StatusCode, Request, Response}; @@ -51,7 +53,7 @@ impl From> for HttpError { impl From for HttpError { fn from(err: base::Error) -> Self { - HttpError(from_base_error(err)) + HttpError(from_base_error(&err)) } } @@ -63,22 +65,7 @@ fn plain_response>(status: http::StatusCode, body: B) -> Response< .expect("hardcoded head should be valid") } -fn not_found>(body: B) -> HttpError { - HttpError(plain_response(StatusCode::NOT_FOUND, body)) -} - -fn bad_req>(body: B) -> HttpError { - HttpError(plain_response(StatusCode::BAD_REQUEST, body)) -} - -fn internal_server_err>(err: E) -> HttpError { - HttpError(plain_response( - StatusCode::INTERNAL_SERVER_ERROR, - err.into().to_string(), - )) -} - -fn from_base_error(err: base::Error) -> Response { +fn from_base_error(err: &base::Error) -> Response { use ErrorKind::*; let status_code = match err.kind() { Unauthenticated => StatusCode::UNAUTHORIZED, @@ -97,7 +84,7 @@ struct Caller { user: Option, } -type ResponseResult = Result, HttpError>; +type ResponseResult = Result, base::Error>; fn serve_json(req: &Request, out: &T) -> ResponseResult { let (mut resp, writer) = http_serve::streaming_body(req).build(); @@ -106,7 +93,7 @@ fn serve_json(req: &Request, out: &T) -> HeaderValue::from_static("application/json"), ); if let Some(mut w) = writer { - serde_json::to_writer(&mut w, out).map_err(internal_server_err)?; + serde_json::to_writer(&mut w, out).err_kind(ErrorKind::Internal)?; } Ok(resp) } @@ -140,19 +127,24 @@ fn extract_sid(req: &Request) -> Option { /// This returns the request body as bytes rather than performing /// deserialization. Keeping the bytes allows the caller to use a `Deserialize` /// that borrows from the bytes. -async fn extract_json_body(req: &mut Request) -> Result { +async fn extract_json_body(req: &mut Request) -> Result { let correct_mime_type = match req.headers().get(header::CONTENT_TYPE) { Some(t) if t == "application/json" => true, Some(t) if t == "application/json; charset=UTF-8" => true, _ => false, }; if !correct_mime_type { - return Err(bad_req("expected application/json request body")); + bail_t!(InvalidArgument, "expected application/json request body"); } let b = ::std::mem::replace(req.body_mut(), hyper::Body::empty()); hyper::body::to_bytes(b) .await - .map_err(|e| internal_server_err(format_err!("unable to read request body: {}", e))) + .map_err(|e| format_err_t!(Unavailable, "unable to read request body: {}", e)) +} + +fn parse_json_body<'a, T: serde::Deserialize<'a>>(body: &'a [u8]) -> Result { + serde_json::from_slice(body) + .map_err(|e| format_err_t!(InvalidArgument, "bad request body: {e}")) } fn require_csrf_if_session(caller: &Caller, csrf: Option<&str>) -> Result<(), base::Error> { @@ -240,18 +232,18 @@ impl Service { /// Serves an HTTP request. /// - /// Note that the `serve` wrapper handles responses the same whether they - /// are `Ok` or `Err`. But returning `Err` here with the `?` operator is - /// convenient for error paths. + /// The `Err` return path will cause the `serve` wrapper to log the error, + /// as well as returning it to the HTTP client. async fn serve_inner( self: Arc, req: Request<::hyper::Body>, authreq: auth::Request, conn_data: ConnData, ) -> ResponseResult { - let p = Path::decode(req.uri().path()); + let path = Path::decode(req.uri().path()); + tracing::trace!(?path, "path"); let always_allow_unauthenticated = matches!( - p, + path, Path::NotFound | Path::Request | Path::Login | Path::Logout | Path::Static ); let caller = self.authenticate(&req, &authreq, &conn_data, always_allow_unauthenticated); @@ -261,20 +253,20 @@ impl Service { .and_then(|c| c.user.as_ref()) .map(|u| &u.name) { - tracing::Span::current().record("auth.user", tracing::field::display(username)); + tracing::Span::current().record("enduser.id", tracing::field::display(username)); } // WebSocket stuff is handled separately, because most authentication // errors are returned as text messages over the protocol, rather than // HTTP-level errors. - if let Path::StreamLiveMp4Segments(uuid, type_) = p { + if let Path::StreamLiveMp4Segments(uuid, type_) = path { return websocket::upgrade(req, move |ws| { Box::pin(self.stream_live_m4s(ws, caller, uuid, type_)) }); } let caller = caller?; - let (cache, mut response) = match p { + let (cache, mut response) = match path { Path::InitSegment(sha1, debug) => ( CacheControl::PrivateStatic, self.init_segment(sha1, debug, &req)?, @@ -300,7 +292,7 @@ impl Service { Path::StreamLiveMp4Segments(..) => { unreachable!("StreamLiveMp4Segments should have already been handled") } - Path::NotFound => return Err(not_found("path not understood")), + Path::NotFound => return Err(format_err_t!(NotFound, "path not understood")), Path::Login => ( CacheControl::PrivateDynamic, self.login(req, authreq).await?, @@ -350,7 +342,7 @@ impl Service { req: Request<::hyper::Body>, conn_data: ConnData, ) -> Result, std::convert::Infallible> { - let id = ulid::Ulid::new(); + let request_id = ulid::Ulid::new(); let authreq = auth::Request { when_sec: Some(self.db.clocks().realtime().sec), addr: if self.trust_forward_hdrs { @@ -371,38 +363,44 @@ impl Service { // https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/ let span = tracing::info_span!( "request", - %id, + %request_id, net.sock.peer.uid = conn_data.client_unix_uid.map(tracing::field::display), http.client_ip = authreq.addr.map(tracing::field::display), http.method = %req.method(), http.target = %req.uri(), http.status_code = tracing::field::Empty, - auth.user = tracing::field::Empty, + enduser.id = tracing::field::Empty, ); tracing::debug!(parent: &span, "received request headers"); let response = self .serve_inner(req, authreq, conn_data) .instrument(span.clone()) - .await - .unwrap_or_else(|e| e.0); + .await; + let (response, error) = match response { + Ok(r) => (r, None), + Err(e) => (from_base_error(&e), Some(e)), + }; span.record("http.status_code", response.status().as_u16()); let latency = std::time::Instant::now().duration_since(start); if response.status().is_server_error() { tracing::error!( parent: &span, - latency = format_args!("{:.6}s", latency.as_secs_f32()), + latency = latency.as_secs_f32(), + error = error.map(tracing::field::display), "sending response headers", ); } else if response.status().is_client_error() { tracing::warn!( parent: &span, - latency = format_args!("{:.6}s", latency.as_secs_f32()), + latency = latency.as_secs_f32(), + error = error.map(tracing::field::display), "sending response headers", ); } else { tracing::info!( parent: &span, - latency = format_args!("{:.6}s", latency.as_secs_f32()), + latency = latency.as_secs_f32(), + error = error.map(tracing::field::display), "sending response headers", ); } @@ -446,10 +444,10 @@ impl Service { let db = self.db.lock(); let camera = db .get_camera(uuid) - .ok_or_else(|| not_found(format!("no such camera {uuid}")))?; + .ok_or_else(|| format_err_t!(NotFound, "no such camera {uuid}"))?; serve_json( req, - &json::Camera::wrap(camera, &db, true, false).map_err(internal_server_err)?, + &json::Camera::wrap(camera, &db, true, false).err_kind(ErrorKind::Internal)?, ) } @@ -467,18 +465,19 @@ impl Service { let (key, value) = (key.borrow(), value.borrow()); match key { "startTime90k" => { - time.start = recording::Time::parse(value) - .map_err(|_| bad_req("unparseable startTime90k"))? + time.start = recording::Time::parse(value).map_err(|_| { + format_err_t!(InvalidArgument, "unparseable startTime90k") + })? } "endTime90k" => { - time.end = recording::Time::parse(value) - .map_err(|_| bad_req("unparseable endTime90k"))? + time.end = recording::Time::parse(value).map_err(|_| { + format_err_t!(InvalidArgument, "unparseable endTime90k") + })? } "split90k" => { - split = recording::Duration( - i64::from_str(value) - .map_err(|_| bad_req("unparseable split90k"))?, - ) + split = recording::Duration(i64::from_str(value).map_err(|_| { + format_err_t!(InvalidArgument, "unparseable split90k") + })?) } _ => {} } @@ -491,15 +490,12 @@ impl Service { recordings: Vec::new(), video_sample_entries: (&db, Vec::new()), }; - let camera = db.get_camera(uuid).ok_or_else(|| { - plain_response(StatusCode::NOT_FOUND, format!("no such camera {uuid}")) - })?; - let stream_id = camera.streams[type_.index()].ok_or_else(|| { - plain_response( - StatusCode::NOT_FOUND, - format!("no such stream {uuid}/{type_}"), - ) - })?; + let Some(camera) = db.get_camera(uuid) else { + bail_t!(NotFound, "no such camera {uuid}"); + }; + let Some(stream_id) = camera.streams[type_.index()] else { + bail_t!(NotFound, "no such stream {uuid}/{type_}"); + }; db.list_aggregated_recordings(stream_id, r, split, &mut |row| { let end = row.ids.end - 1; // in api, ids are inclusive. out.recordings.push(json::Recording { @@ -528,21 +524,20 @@ impl Service { } Ok(()) }) - .map_err(internal_server_err)?; + .err_kind(ErrorKind::Internal)?; serve_json(req, &out) } fn init_segment(&self, id: i32, debug: bool, req: &Request<::hyper::Body>) -> ResponseResult { let mut builder = mp4::FileBuilder::new(mp4::Type::InitSegment); let db = self.db.lock(); - let ent = db - .video_sample_entries_by_id() - .get(&id) - .ok_or_else(|| not_found("not such init segment"))?; + let Some(ent) = db.video_sample_entries_by_id().get(&id) else { + bail_t!(NotFound, "no such init segment"); + }; builder.append_video_sample_entry(ent.clone()); let mp4 = builder .build(self.db.clone(), self.dirs_by_stream_id.clone()) - .map_err(from_base_error)?; + .err_kind(ErrorKind::Internal)?; if debug { Ok(plain_response(StatusCode::OK, format!("{mp4:#?}"))) } else { diff --git a/server/src/web/session.rs b/server/src/web/session.rs index 93c7940..8b8f783 100644 --- a/server/src/web/session.rs +++ b/server/src/web/session.rs @@ -4,16 +4,16 @@ //! Session management: `/api/login` and `/api/logout`. +use base::{bail_t, ErrorKind, ResultExt}; use db::auth; use http::{header, HeaderValue, Method, Request, Response, StatusCode}; use memchr::memchr; use tracing::{info, warn}; -use crate::json; +use crate::{json, web::parse_json_body}; use super::{ - bad_req, csrf_matches, extract_json_body, extract_sid, internal_server_err, plain_response, - ResponseResult, Service, + csrf_matches, extract_json_body, extract_sid, plain_response, ResponseResult, Service, }; use std::convert::TryFrom; @@ -24,15 +24,16 @@ impl Service { authreq: auth::Request, ) -> ResponseResult { if *req.method() != Method::POST { - return Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()); + return Ok(plain_response( + StatusCode::METHOD_NOT_ALLOWED, + "POST expected", + )); } let r = extract_json_body(&mut req).await?; - let r: json::LoginRequest = - serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; - let host = req - .headers() - .get(header::HOST) - .ok_or_else(|| bad_req("missing Host header!"))?; + let r: json::LoginRequest = parse_json_body(&r)?; + let Some(host) = req.headers().get(header::HOST) else { + bail_t!(InvalidArgument, "missing Host header"); + }; let host = host.as_bytes(); let domain = match memchr(b':', host) { Some(colon) => &host[0..colon], @@ -60,7 +61,7 @@ impl Service { }; let (sid, _) = l .login_by_password(authreq, r.username, r.password, Some(domain), flags) - .map_err(|e| plain_response(StatusCode::UNAUTHORIZED, e.to_string()))?; + .err_kind(ErrorKind::Unauthenticated)?; let cookie = encode_sid(sid, flags); Ok(Response::builder() .header( @@ -78,37 +79,33 @@ impl Service { authreq: auth::Request, ) -> ResponseResult { if *req.method() != Method::POST { - return Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()); + return Ok(plain_response( + StatusCode::METHOD_NOT_ALLOWED, + "POST expected", + )); } let r = extract_json_body(&mut req).await?; - let r: json::LogoutRequest = - serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + let r: json::LogoutRequest = parse_json_body(&r)?; let mut res = Response::new(b""[..].into()); if let Some(sid) = extract_sid(&req) { let mut l = self.db.lock(); let hash = sid.hash(); - let need_revoke = match l.authenticate_session(authreq.clone(), &hash) { + match l.authenticate_session(authreq.clone(), &hash) { Ok((s, _)) => { if !csrf_matches(r.csrf, s.csrf()) { - warn!("logout request with missing/incorrect csrf"); - return Err(bad_req("logout with incorrect csrf token")); + bail_t!(InvalidArgument, "logout with incorret csrf token"); } info!("revoking session"); - true + l.revoke_session(auth::RevocationReason::LoggedOut, None, authreq, &hash) + .err_kind(ErrorKind::Internal)?; } Err(e) => { // TODO: distinguish "no such session", "session is no longer valid", and // "user ... is disabled" (which are all client error / bad state) from database // errors. warn!("logout failed: {}", e); - false } - }; - if need_revoke { - // TODO: inline this above with non-lexical lifetimes. - l.revoke_session(auth::RevocationReason::LoggedOut, None, authreq, &hash) - .map_err(internal_server_err)?; } // By now the session is invalid (whether it was valid to start with or not). diff --git a/server/src/web/signals.rs b/server/src/web/signals.rs index ccee21a..1f56d13 100644 --- a/server/src/web/signals.rs +++ b/server/src/web/signals.rs @@ -4,7 +4,7 @@ //! `/api/signals` handling. -use base::{bail_t, clock::Clocks}; +use base::{bail_t, clock::Clocks, format_err_t}; use db::recording; use http::{Method, Request, StatusCode}; use url::form_urlencoded; @@ -12,8 +12,8 @@ use url::form_urlencoded; use crate::json; use super::{ - bad_req, extract_json_body, from_base_error, plain_response, require_csrf_if_session, - serve_json, Caller, ResponseResult, Service, + extract_json_body, parse_json_body, plain_response, require_csrf_if_session, serve_json, + Caller, ResponseResult, Service, }; use std::borrow::Borrow; @@ -27,11 +27,10 @@ impl Service { match *req.method() { Method::POST => self.post_signals(req, caller).await, Method::GET | Method::HEAD => self.get_signals(&req), - _ => Err(plain_response( + _ => Ok(plain_response( StatusCode::METHOD_NOT_ALLOWED, "POST, GET, or HEAD expected", - ) - .into()), + )), } } @@ -40,8 +39,7 @@ impl Service { bail_t!(PermissionDenied, "update_signals required"); } let r = extract_json_body(&mut req).await?; - let r: json::PostSignalsRequest = - serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + let r: json::PostSignalsRequest = parse_json_body(&r)?; require_csrf_if_session(&caller, r.csrf)?; let now = recording::Time::new(self.db.clocks().realtime()); let mut l = self.db.lock(); @@ -53,8 +51,7 @@ impl Service { json::PostSignalsTimeBase::Epoch(t) => t, json::PostSignalsTimeBase::Now(d) => now + d, }; - l.update_signals(start..end, &r.signal_ids, &r.states) - .map_err(from_base_error)?; + l.update_signals(start..end, &r.signal_ids, &r.states)?; serve_json(&req, &json::PostSignalsResponse { time_90k: now }) } @@ -65,12 +62,13 @@ impl Service { let (key, value) = (key.borrow(), value.borrow()); match key { "startTime90k" => { - time.start = recording::Time::parse(value) - .map_err(|_| bad_req("unparseable startTime90k"))? + time.start = recording::Time::parse(value).map_err(|_| { + format_err_t!(InvalidArgument, "unparseable startTime90k") + })? } "endTime90k" => { time.end = recording::Time::parse(value) - .map_err(|_| bad_req("unparseable endTime90k"))? + .map_err(|_| format_err_t!(InvalidArgument, "unparseable endTime90k"))? } _ => {} } diff --git a/server/src/web/static_file.rs b/server/src/web/static_file.rs index 7ddeaea..93a937f 100644 --- a/server/src/web/static_file.rs +++ b/server/src/web/static_file.rs @@ -4,26 +4,26 @@ //! Static file serving. +use base::{bail_t, format_err_t, Error, ErrorKind, ResultExt}; use http::{header, HeaderValue, Request}; -use super::{internal_server_err, not_found, ResponseResult, Service}; +use super::{ResponseResult, Service}; impl Service { /// Serves a static file if possible. pub(super) async fn static_file(&self, req: Request) -> ResponseResult { - let dir = self.ui_dir.clone().ok_or_else(|| { - not_found("ui dir not configured or missing; no static files available.") - })?; - let static_req = match StaticFileRequest::parse(req.uri().path()) { - None => return Err(not_found("static file not found")), - Some(r) => r, + let Some(dir) = self.ui_dir.clone() else { + bail_t!(NotFound, "ui dir not configured or missing; no static files available.") + }; + let Some(static_req) = StaticFileRequest::parse(req.uri().path()) else { + bail_t!(NotFound, "static file not found"); }; let f = dir.get(static_req.path, req.headers()); let node = f.await.map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { - not_found("no such static file") + format_err_t!(NotFound, "no such static file") } else { - internal_server_err(e) + Error::wrap(ErrorKind::Internal, e) } })?; let mut hdrs = http::HeaderMap::new(); @@ -41,7 +41,7 @@ impl Service { header::CONTENT_TYPE, HeaderValue::from_static(static_req.mime), ); - let e = node.into_file_entity(hdrs).map_err(internal_server_err)?; + let e = node.into_file_entity(hdrs).err_kind(ErrorKind::Internal)?; Ok(http_serve::serve(e, &req)) } } diff --git a/server/src/web/users.rs b/server/src/web/users.rs index a5d8404..f7c95b2 100644 --- a/server/src/web/users.rs +++ b/server/src/web/users.rs @@ -10,8 +10,8 @@ use http::{Method, Request, StatusCode}; use crate::json::{self, PutUsersResponse, UserSubset, UserWithId}; use super::{ - bad_req, extract_json_body, plain_response, require_csrf_if_session, serve_json, Caller, - ResponseResult, Service, + extract_json_body, parse_json_body, plain_response, require_csrf_if_session, serve_json, + Caller, ResponseResult, Service, }; impl Service { @@ -19,11 +19,10 @@ impl Service { match *req.method() { Method::GET | Method::HEAD => self.get_users(req, caller).await, Method::POST => self.post_users(req, caller).await, - _ => Err(plain_response( + _ => Ok(plain_response( StatusCode::METHOD_NOT_ALLOWED, "GET, HEAD, or POST expected", - ) - .into()), + )), } } @@ -48,8 +47,7 @@ impl Service { bail_t!(Unauthenticated, "must have admin_users permission"); } let r = extract_json_body(&mut req).await?; - let mut r: json::PutUsers = - serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + let mut r: json::PutUsers = parse_json_body(&r)?; require_csrf_if_session(&caller, r.csrf)?; let username = r .user @@ -84,11 +82,10 @@ impl Service { Method::GET | Method::HEAD => self.get_user(req, caller, id).await, Method::DELETE => self.delete_user(req, caller, id).await, Method::PATCH => self.patch_user(req, caller, id).await, - _ => Err(plain_response( + _ => Ok(plain_response( StatusCode::METHOD_NOT_ALLOWED, "GET, HEAD, DELETE, or PATCH expected", - ) - .into()), + )), } } @@ -112,7 +109,7 @@ impl Service { bail_t!(Unauthenticated, "must have admin_users permission"); } let r = extract_json_body(&mut req).await?; - let r: json::DeleteUser = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + let r: json::DeleteUser = parse_json_body(&r)?; require_csrf_if_session(&caller, r.csrf)?; let mut l = self.db.lock(); l.delete_user(id)?; @@ -127,7 +124,7 @@ impl Service { ) -> ResponseResult { require_same_or_admin(&caller, id)?; let r = extract_json_body(&mut req).await?; - let r: json::PostUser = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + let r: json::PostUser = parse_json_body(&r)?; let mut db = self.db.lock(); let user = db .get_user_by_id_mut(id) diff --git a/server/src/web/view.rs b/server/src/web/view.rs index 9fe98f0..0edd0c3 100644 --- a/server/src/web/view.rs +++ b/server/src/web/view.rs @@ -4,7 +4,7 @@ //! `/view.mp4` and `/view.m4s` handling. -use base::bail_t; +use base::{bail_t, format_err_t}; use db::recording::{self, rescale}; use http::{Request, StatusCode}; use nom::bytes::complete::{tag, take_while1}; @@ -20,11 +20,8 @@ use tracing::trace; use url::form_urlencoded; use uuid::Uuid; -use crate::web::{bad_req, from_base_error}; -use crate::{ - mp4, - web::{not_found, plain_response}, -}; +use crate::mp4; +use crate::web::plain_response; use super::{Caller, ResponseResult, Service}; @@ -46,10 +43,10 @@ impl Service { let db = self.db.lock(); let camera = db .get_camera(uuid) - .ok_or_else(|| not_found(format!("no such camera {uuid}")))?; + .ok_or_else(|| format_err_t!(NotFound, "no such camera {uuid}"))?; camera_name = camera.short_name.clone(); stream_id = camera.streams[stream_type.index()] - .ok_or_else(|| not_found(format!("no such stream {uuid}/{stream_type}")))?; + .ok_or_else(|| format_err_t!(NotFound, "no such stream {uuid}/{stream_type}"))?; }; let mut start_time_for_filename = None; let mut builder = mp4::FileBuilder::new(mp4_type); @@ -59,10 +56,7 @@ impl Service { match key { "s" => { let s = Segments::from_str(value).map_err(|()| { - plain_response( - StatusCode::BAD_REQUEST, - format!("invalid s parameter: {value}"), - ) + format_err_t!(InvalidArgument, "invalid s parameter: {value}") })?; trace!("stream_view_mp4: appending s={:?}", s); let mut est_segments = usize::try_from(s.ids.end - s.ids.start).unwrap(); @@ -150,17 +144,20 @@ impl Service { // Check for missing recordings. match prev { Some(id) if s.ids.end != id + 1 => { - return Err(not_found(format!( + bail_t!( + NotFound, "no such recording {}/{}", stream_id, s.ids.end - 1 - ))); + ); } None => { - return Err(not_found(format!( + bail_t!( + NotFound, "no such recording {}/{}", - stream_id, s.ids.start - ))); + stream_id, + s.ids.start + ); } _ => {} }; @@ -174,10 +171,8 @@ impl Service { } } } - "ts" => builder - .include_timestamp_subtitle_track(value == "true") - .map_err(from_base_error)?, - _ => return Err(bad_req(format!("parameter {key} not understood"))), + "ts" => builder.include_timestamp_subtitle_track(value == "true")?, + _ => bail_t!(InvalidArgument, "parameter {key} not understood"), } } } @@ -196,19 +191,15 @@ impl Service { } else { "m4s" }; - builder - .set_filename(&format!( - "{}-{}-{}.{}", - tm.strftime("%Y%m%d%H%M%S").unwrap(), - camera_name, - stream_abbrev, - suffix - )) - .map_err(from_base_error)?; + builder.set_filename(&format!( + "{}-{}-{}.{}", + tm.strftime("%Y%m%d%H%M%S").unwrap(), + camera_name, + stream_abbrev, + suffix + ))?; } - let mp4 = builder - .build(self.db.clone(), self.dirs_by_stream_id.clone()) - .map_err(from_base_error)?; + let mp4 = builder.build(self.db.clone(), self.dirs_by_stream_id.clone())?; if debug { return Ok(plain_response(StatusCode::OK, format!("{mp4:#?}"))); } diff --git a/server/src/web/websocket.rs b/server/src/web/websocket.rs index ae2ae49..005fde2 100644 --- a/server/src/web/websocket.rs +++ b/server/src/web/websocket.rs @@ -8,19 +8,20 @@ use std::pin::Pin; use crate::body::Body; -use base::bail_t; +use base::{bail_t, format_err_t}; use futures::{Future, SinkExt}; use http::{header, Request, Response}; use tokio_tungstenite::{tungstenite, WebSocketStream}; use tracing::Instrument; -use super::{bad_req, ResponseResult}; - /// Upgrades to WebSocket and runs the supplied stream handler in a separate tokio task. /// /// Fails on `Origin` mismatch with an HTTP-level error. If the handler returns /// an error, tries to send it to the client before dropping the stream. -pub(super) fn upgrade(req: Request<::hyper::Body>, handler: H) -> ResponseResult +pub(super) fn upgrade( + req: Request<::hyper::Body>, + handler: H, +) -> Result, base::Error> where for<'a> H: FnOnce( &'a mut WebSocketStream, @@ -36,7 +37,7 @@ where // Otherwise, upgrade and handle the rest in a separate task. let response = tungstenite::handshake::server::create_response_with_body(&req, hyper::Body::empty) - .map_err(|e| bad_req(e.to_string()))?; + .map_err(|e| format_err_t!(InvalidArgument, "{}", e.to_string()))?; let (parts, _) = response.into_parts(); let span = tracing::info_span!("websocket"); tokio::spawn( @@ -77,15 +78,17 @@ where /// If present, verify it. Chrome doesn't honor the `s=` cookie's /// `SameSite=Lax` setting for WebSocket requests, so this is the sole /// protection against [CSWSH](https://christian-schneider.net/CrossSiteWebSocketHijacking.html). -fn check_origin(headers: &header::HeaderMap) -> Result<(), super::HttpError> { +fn check_origin(headers: &header::HeaderMap) -> Result<(), base::Error> { let origin_hdr = match headers.get(http::header::ORIGIN) { None => return Ok(()), Some(o) => o, }; - let host_hdr = headers - .get(header::HOST) - .ok_or_else(|| bad_req("missing Host header"))?; - let host_str = host_hdr.to_str().map_err(|_| bad_req("bad Host header"))?; + let Some(host_hdr) = headers.get(header::HOST) else { + bail_t!(InvalidArgument, "missing Host header"); + }; + let host_str = host_hdr + .to_str() + .map_err(|_| format_err_t!(InvalidArgument, "bad Host header"))?; // Currently this ignores the port number. This is easiest and I think matches the browser's // rules for when it sends a cookie, so it probably doesn't cause great security problems. @@ -97,10 +100,10 @@ fn check_origin(headers: &header::HeaderMap) -> Result<(), super::HttpError> { .to_str() .ok() .and_then(|o| url::Url::parse(o).ok()) - .ok_or_else(|| bad_req("bad Origin header"))?; + .ok_or_else(|| format_err_t!(InvalidArgument, "bad Origin header"))?; let origin_host = origin_url .host_str() - .ok_or_else(|| bad_req("bad Origin header"))?; + .ok_or_else(|| format_err_t!(InvalidArgument, "bad Origin header"))?; if host != origin_host { bail_t!( PermissionDenied,