MyFavMirrors/moonfire-nvr
1
0
Fork 0
mirror of https://github.com/scottlamb/moonfire-nvr.git synced 2025-01-27 14:43:19 -05:00
Code Issues Packages Projects Releases Wiki Activity

small readability improvements to web.rs

Browse Source 
I think this has a minor behavior change: permission denied replies
change to HTTP 403 where they were HTTP 401. The new behavior seems
more correct, as these errors can occur when authentication has
succeeded but the session in question is not authorized for the given
operation. The UI currently doesn't care about this distinction.
This commit is contained in:
Scott Lamb 2021-03-10 16:18:47 -08:00
parent e66a88a591
commit 984989dc1c
2 changed files with 74 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
server/base/error.rs
View File

@ -158,10 +158,10 @@ where
#[macro_export] #[macro_export]
macro_rules! bail_t { macro_rules! bail_t {
($t:ident, $e:expr) => { ($t:ident, $e:expr) => {
return Err(failure::err_msg($e).context($crate::ErrorKind::$t).into()); return Err($crate::Error::from(failure::err_msg($e).context($crate::ErrorKind::$t)).into());
}; };
($t:ident, $fmt:expr, $($arg:tt)+) => { ($t:ident, $fmt:expr, $($arg:tt)+) => {
return Err(failure::err_msg(format!($fmt, $($arg)+)).context($crate::ErrorKind::$t).into()); return Err($crate::Error::from(failure::err_msg(format!($fmt, $($arg)+)).context($crate::ErrorKind::$t)).into());
}; };
} }

118
server/src/web.rs
View File

@ -5,8 +5,8 @@
use crate::body::Body; use crate::body::Body;
use crate::json; use crate::json;
use crate::mp4; use crate::mp4;
use base::clock::Clocks;
use base::{bail_t, ErrorKind}; use base::{bail_t, ErrorKind};
use base::{clock::Clocks, format_err_t};
use bytes::{BufMut, BytesMut}; use bytes::{BufMut, BytesMut};
use core::borrow::Borrow; use core::borrow::Borrow;
use core::str::FromStr; use core::str::FromStr;
@ -35,6 +35,26 @@ use tokio_tungstenite::tungstenite;
use url::form_urlencoded; use url::form_urlencoded;
use uuid::Uuid; use uuid::Uuid;
/// An HTTP error response.
/// This is a thin wrapper over the hyper response type; it doesn't even verify
/// that the response actually uses a non-2xx status code. Its purpose is to
/// allow automatic conversion from `base::Error`. Rust's orphan rule prevents
/// this crate from defining a direct conversion from `base::Error` to
/// `hyper::Response`.
struct HttpError(Response<Body>);
impl From<Response<Body>> for HttpError {
fn from(response: Response<Body>) -> Self {
HttpError(response)
}
}
impl From<base::Error> for HttpError {
fn from(err: base::Error) -> Self {
HttpError(from_base_error(err))
}
}
#[derive(Debug, Eq, PartialEq)] #[derive(Debug, Eq, PartialEq)]
enum Path { enum Path {
TopLevel, // "/api/" TopLevel, // "/api/"
@ -141,23 +161,28 @@ fn plain_response<B: Into<Body>>(status: http::StatusCode, body: B) -> Response<
.expect("hardcoded head should be valid") .expect("hardcoded head should be valid")
} }
fn not_found<B: Into<Body>>(body: B) -> Response<Body> { fn not_found<B: Into<Body>>(body: B) -> HttpError {
plain_response(StatusCode::NOT_FOUND, body) HttpError(plain_response(StatusCode::NOT_FOUND, body))
} }
fn bad_req<B: Into<Body>>(body: B) -> Response<Body> { fn bad_req<B: Into<Body>>(body: B) -> HttpError {
plain_response(StatusCode::BAD_REQUEST, body) HttpError(plain_response(StatusCode::BAD_REQUEST, body))
} }
fn internal_server_err<E: Into<Error>>(err: E) -> Response<Body> { fn internal_server_err<E: Into<Error>>(err: E) -> HttpError {
plain_response(StatusCode::INTERNAL_SERVER_ERROR, err.into().to_string()) HttpError(plain_response(
StatusCode::INTERNAL_SERVER_ERROR,
err.into().to_string(),
))
} }
fn from_base_error(err: base::Error) -> Response<Body> { fn from_base_error(err: base::Error) -> Response<Body> {
use ErrorKind::*;
let status_code = match err.kind() { let status_code = match err.kind() {
ErrorKind::PermissionDenied | ErrorKind::Unauthenticated => StatusCode::UNAUTHORIZED, Unauthenticated => StatusCode::UNAUTHORIZED,
ErrorKind::InvalidArgument => StatusCode::BAD_REQUEST, PermissionDenied => StatusCode::FORBIDDEN,
ErrorKind::NotFound => StatusCode::NOT_FOUND, InvalidArgument | FailedPrecondition => StatusCode::BAD_REQUEST,
NotFound => StatusCode::NOT_FOUND,
_ => StatusCode::INTERNAL_SERVER_ERROR, _ => StatusCode::INTERNAL_SERVER_ERROR,
}; };
plain_response(status_code, err.to_string()) plain_response(status_code, err.to_string())
@ -232,7 +257,7 @@ struct Caller {
session: Option<json::Session>, session: Option<json::Session>,
} }
type ResponseResult = Result<Response<Body>, Response<Body>>; type ResponseResult = Result<Response<Body>, HttpError>;
fn serve_json<T: serde::ser::Serialize>(req: &Request<hyper::Body>, out: &T) -> ResponseResult { fn serve_json<T: serde::ser::Serialize>(req: &Request<hyper::Body>, out: &T) -> ResponseResult {
let (mut resp, writer) = http_serve::streaming_body(&req).build(); let (mut resp, writer) = http_serve::streaming_body(&req).build();
@ -277,12 +302,9 @@ fn extract_sid(req: &Request<hyper::Body>) -> Option<auth::RawSessionId> {
/// This returns the request body as bytes rather than performing /// This returns the request body as bytes rather than performing
/// deserialization. Keeping the bytes allows the caller to use a `Deserialize` /// deserialization. Keeping the bytes allows the caller to use a `Deserialize`
/// that borrows from the bytes. /// that borrows from the bytes.
async fn extract_json_body(req: &mut Request<hyper::Body>) -> Result<Bytes, Response<Body>> { async fn extract_json_body(req: &mut Request<hyper::Body>) -> Result<Bytes, HttpError> {
if *req.method() != http::method::Method::POST { if *req.method() != http::method::Method::POST {
return Err(plain_response( return Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into());
StatusCode::METHOD_NOT_ALLOWED,
"POST expected",
));
} }
let correct_mime_type = match req.headers().get(header::CONTENT_TYPE) { let correct_mime_type = match req.headers().get(header::CONTENT_TYPE) {
Some(t) if t == "application/json" => true, Some(t) if t == "application/json" => true,
@ -376,10 +398,7 @@ impl Service {
stream_type: db::StreamType, stream_type: db::StreamType,
) -> ResponseResult { ) -> ResponseResult {
if !caller.permissions.view_video { if !caller.permissions.view_video {
return Err(plain_response( bail_t!(PermissionDenied, "view_video required");
StatusCode::UNAUTHORIZED,
"view_video required",
));
} }
let stream_id; let stream_id;
@ -389,10 +408,10 @@ impl Service {
let mut db = self.db.lock(); let mut db = self.db.lock();
open_id = match db.open { open_id = match db.open {
None => { None => {
return Err(plain_response( bail_t!(
StatusCode::PRECONDITION_FAILED, FailedPrecondition,
"database is read-only; there are no live streams", "database is read-only; there are no live streams"
)) );
} }
Some(o) => o.id, Some(o) => o.id,
}; };
@ -400,10 +419,7 @@ impl Service {
plain_response(StatusCode::NOT_FOUND, format!("no such camera {}", uuid)) plain_response(StatusCode::NOT_FOUND, format!("no such camera {}", uuid))
})?; })?;
stream_id = camera.streams[stream_type.index()].ok_or_else(|| { stream_id = camera.streams[stream_type.index()].ok_or_else(|| {
plain_response( format_err_t!(NotFound, "no such stream {}/{}", uuid, stream_type)
StatusCode::NOT_FOUND,
format!("no such stream {}/{}", uuid, stream_type),
)
})?; })?;
db.watch_live( db.watch_live(
stream_id, stream_id,
@ -525,10 +541,15 @@ impl Service {
_ => Err(plain_response( _ => Err(plain_response(
StatusCode::METHOD_NOT_ALLOWED, StatusCode::METHOD_NOT_ALLOWED,
"POST, GET, or HEAD expected", "POST, GET, or HEAD expected",
)), )
.into()),
} }
} }
/// 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.
async fn serve_inner( async fn serve_inner(
self: Arc<Self>, self: Arc<Self>,
req: Request<::hyper::Body>, req: Request<::hyper::Body>,
@ -586,6 +607,12 @@ impl Service {
Ok(response) Ok(response)
} }
/// Serves an HTTP request.
/// An error return from this method causes hyper to abruptly drop the
/// HTTP connection rather than respond. That's not terribly useful, so this
/// method always returns `Ok`. It delegates to a `serve_inner` which is
/// allowed to generate `Err` results with the `?` operator, but returns
/// them to hyper as `Ok` results.
pub async fn serve( pub async fn serve(
self: Arc<Self>, self: Arc<Self>,
req: Request<::hyper::Body>, req: Request<::hyper::Body>,
@ -600,7 +627,10 @@ impl Service {
Ok(c) => c, Ok(c) => c,
Err(e) => return Ok(from_base_error(e)), Err(e) => return Ok(from_base_error(e)),
}; };
Ok(self.serve_inner(req, p, caller).await.unwrap_or_else(|e| e)) Ok(self
.serve_inner(req, p, caller)
.await
.unwrap_or_else(|e| e.0))
} }
fn top_level(&self, req: &Request<::hyper::Body>, caller: Caller) -> ResponseResult { fn top_level(&self, req: &Request<::hyper::Body>, caller: Caller) -> ResponseResult {
@ -619,10 +649,7 @@ impl Service {
if camera_configs { if camera_configs {
if !caller.permissions.read_camera_configs { if !caller.permissions.read_camera_configs {
return Err(plain_response( bail_t!(PermissionDenied, "read_camera_configs required");
StatusCode::UNAUTHORIZED,
"read_camera_configs required",
));
} }
} }
@ -756,10 +783,7 @@ impl Service {
debug: bool, debug: bool,
) -> ResponseResult { ) -> ResponseResult {
if !caller.permissions.view_video { if !caller.permissions.view_video {
return Err(plain_response( bail_t!(PermissionDenied, "view_video required");
StatusCode::UNAUTHORIZED,
"view_video required",
));
} }
let (stream_id, camera_name); let (stream_id, camera_name);
{ {
@ -884,10 +908,11 @@ impl Service {
}; };
if let Some(end) = s.end_time { if let Some(end) = s.end_time {
if end > cur_off { if end > cur_off {
return Err(plain_response( bail_t!(
StatusCode::BAD_REQUEST, InvalidArgument,
format!("end time {} is beyond specified recordings", end), "end time {} is beyond specified recordings",
)); end
);
} }
} }
} }
@ -1019,6 +1044,10 @@ impl Service {
)) ))
} }
/// Returns true iff the client is connected over `https`.
/// Moonfire NVR currently doesn't directly serve `https`, but it supports
/// proxies which set the `X-Forwarded-Proto` header. See `guide/secure.md`
/// for more information.
fn is_secure(&self, req: &Request<::hyper::Body>) -> bool { fn is_secure(&self, req: &Request<::hyper::Body>) -> bool {
self.trust_forward_hdrs self.trust_forward_hdrs
&& req && req
@ -1124,10 +1153,7 @@ impl Service {
async fn post_signals(&self, mut req: Request<hyper::Body>, caller: Caller) -> ResponseResult { async fn post_signals(&self, mut req: Request<hyper::Body>, caller: Caller) -> ResponseResult {
if !caller.permissions.update_signals { if !caller.permissions.update_signals {
return Err(plain_response( bail_t!(PermissionDenied, "update_signals required");
StatusCode::UNAUTHORIZED,
"update_signals required",
));
} }
let r = extract_json_body(&mut req).await?; let r = extract_json_body(&mut req).await?;
let r: json::PostSignalsRequest = let r: json::PostSignalsRequest =