diff --git a/CHANGELOG.md b/CHANGELOG.md index eb702e7..44765ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,15 @@ changes, see Git history. Each release is tagged in Git and on the Docker repository [`scottlamb/moonfire-nvr`](https://hub.docker.com/r/scottlamb/moonfire-nvr). +Backwards-incompatible database schema changes happen on on major version +upgrades, e.g. `0.6.x` -> `0.7.x`. The config file format and +[API](design/api.md) currently have no stability guarantees, so they may change +even on minor releases, e.g. `0.7.5` -> `0.7.6`. + ## unreleased +* expect camelCase in `moonfire-nvr.toml` file, for consistency with the JSON + API. You'll need to adjust your config file when upgrading. * use Retina 0.4.3, which is newly compatible with rtsp-simple-server v0.19.3 and some TP-Link cameras. Fixes [#238](https://github.com/scottlamb/moonfire-nvr/issues/238). * expanded API interface for examining and updating users: @@ -18,6 +25,7 @@ Each release is tagged in Git and on the Docker repository * expanded `POST /users/` endpoint, including password and permissions. * `DELETE /users/` endpoint to delete a user +* improved API documentation in `design/api.md`. ## 0.7.5 (2022-05-09) diff --git a/design/api.md b/design/api.md index 0917b11..5b53379 100644 --- a/design/api.md +++ b/design/api.md @@ -2,8 +2,8 @@ Status: **current**. -* [Objective](#objective) -* [Detailed design](#detailed-design) +* [Summary](#summary) +* [Endpoints](#endpoints) * [Authentication](#authentication) * [`POST /api/login`](#post-apilogin) * [`POST /api/logout`](#post-apilogout) @@ -23,16 +23,20 @@ Status: **current**. * [Request 2](#request-2) * [Request 3](#request-3) * [User management](#user-management) - * [`GET /api/users`](#get-apiusers) - * [`PUT /api/users`](#put-apiusers) + * [`GET /api/users/`](#get-apiusers) + * [`PUT /api/users/`](#put-apiusers) * [`GET /api/users/`](#get-apiusersid) * [`POST /api/users/`](#post-apiusersid) * [`DELETE /api/users/`](#delete-apiusersid) +* [Types](#types) + * [UserSubset](#usersubset) + * [Permissions](#permissions) +* [Cross-site request forgery (CSRF) protection](#cross-site-request-forgery-csrf-protection) -## Objective +## Summary -Allow a JavaScript-based web interface to list cameras and view recordings. -Support external analytics. +A JavaScript-based web interface to list cameras and view recordings. +Supports external analytics. In the future, this is likely to be expanded: @@ -40,8 +44,6 @@ In the future, this is likely to be expanded: * commandline tool over a UNIX-domain socket (at least for bootstrapping web authentication) -## Detailed design - *Note:* italicized terms in this document are defined in the [glossary](glossary.md). Currently the API is considered an internal contract between the server and the @@ -56,6 +58,8 @@ developed tools. All requests for JSON data should be sent with the header `Accept: application/json` (exactly). +## Endpoints + ### Authentication #### `POST /api/login` @@ -89,8 +93,7 @@ request parameters: should be included. * `cameraConfigs`: a boolean indicating if the `camera.config` and `camera.stream[].config` parameters described below should be included. - This requires the `read_camera_configs` permission as described in - `schema.proto`. + This requires the `readCameraConfigs` permission. Example request URI (with added whitespace between parameters): @@ -184,13 +187,12 @@ The `application/json` response will have a JSON object as follows: considered to have motion when this signal is in this state. * `color` (optional): a recommended color to use in UIs to represent this state, as in the [HTML specification](https://html.spec.whatwg.org/#colours). +* `permissions`: the caller's current `Permissions` object (defined below). * `user`: an object, present only when authenticated: * `name`: a human-readable name * `id`: an integer * `preferences`: a JSON object * `session`: an object, present only if authenticated via session cookie. - (In the future, it will be possible to instead authenticate via uid over - a Unix domain socket.) * `csrf`: a cross-site request forgery token for use in `POST` requests. Example response: @@ -427,7 +429,7 @@ Example response: ### `GET /api/cameras///view.mp4` -Requires the `view_video` permission. +Requires the `viewVideo` permission. Returns a `.mp4` file, with an etag and support for range requests. The MIME type will be `video/mp4`, with a `codecs` parameter as specified in @@ -717,7 +719,7 @@ This represents the following observations: ### `POST /api/signals` -Requires the `update_signals` permission. +Requires the `updateSignals` permission. Alters the state of a signal. @@ -735,6 +737,7 @@ last ran. These will specify beginning and end times. The request should have an `application/json` body describing the change to make. It should be a JSON object with these attributes: +* `csrf`: a CSRF token, required when using session authentication. * `signalIds`: a list of signal ids to change. Must be sorted. * `states`: a list (one per `signalIds` entry) of states to set. * `start`: the starting time of the change, as a JSON object of the form @@ -829,69 +832,132 @@ Response: ### User management -#### `GET /api/users` +#### `GET /api/users/` -Requires the `admin_users` permission. +Requires the `adminUsers` permission. Lists all users. Currently there's no paging. Returns a JSON object with -a `users` key with a map of id to username. +a `users` key with an array of objects, each with the following keys: -#### `PUT /api/users` +* `id`: a number. +* `username`: a string. -Requires the `admin_users` permission. +#### `PUT /api/users/` -Adds a user. Expects a JSON dictionary with the parameters for the user: +Requires the `adminUsers` permission. -* `username`: a string, which must be unique. -* `permissions`: a JSON dictionary of permissions. -* `password` (optional): a string. -* `preferences` (optional): a JSON dictionary. +Adds a user. Expects a JSON object as follows: + +* `csrf`: a CSRF token, required when using session authentication. +* `user`: a `UserSubset` as defined below. Returns status 204 (No Content) on success. #### `GET /api/users/` -Retrieves the user. Requires the `admin_users` permission if the caller is +Retrieves the user. Requires the `adminUsers` permission if the caller is not authenticated as the user in question. -Returns a HTTP status 200 on success with a JSON dict: - -* `preferences`: a JSON dictionary. -* `password`: absent (no password set) or a placeholder string to indicate - the password is set. Passwords are stored hashed, so the cleartext can not - be retrieved. -* `permissions`. +Returns a HTTP status 200 on success with a JSON `UserSubset`. The `password` +will be absent (for no password) or a placeholder string to indicate the +password is set. Passwords are stored hashed, so the cleartext can not be +retrieved. #### `POST /api/users/` -Allows updating the given user. Requires the `admin_users` permission if the -caller is not authenticated as the user in question. +Updates the given user. Requires the `adminUsers` permission if the caller is +not authenticated as the user in question. Expects a JSON object: * `csrf`: a CSRF token, required when using session authentication. -* `update`: sets the provided fields -* `precondition`: forces the request to fail with HTTP status 412 - (Precondition failed) if the provided fields don't have the given value. - -Currently the following fields are supported for `update` and `precondition`: - -* `preferences`, a JSON dictionary. -* `password`, a cleartext string. When updating the password, the previous - password must be supplied as a precondition, unless the caller has - `admin_users` permission. -* `permissions`, which always requires `admin_users` permission to update. +* `update`: `UserSubset`, sets the provided fields. Field-specific notes: + * `password`: when updating the password, the previous password must + be supplied as a precondition, unless the caller has `admin_users` + permission. + * `permissions`: requires `adminUsers` permission. Note that updating a + user's permissions currently neither adds nor limits permissions of + existing sessions; it only changes what is available to newly created + sessions. +* `precondition`: `UserSubset`, forces the request to fail with HTTP status + 412 (Precondition failed) if the provided fields don't have the given + values. Returns HTTP status 204 (No Content) on success. #### `DELETE /api/users/` -Deletes the given user. Requires the `admin_users` permission. +Deletes the given user. Requires the `adminUsers` permission. + +Expects a JSON object body with the following parameters: + +* `csrf`: a CSRF token, required when using session authentication. Returns HTTP status 204 (No Content) on success. +## Types + +### UserSubset + +A JSON object with any of the following parameters: + +* `preferences`, a JSON object which the server stores without interpreting. + This field is meant for user-level preferences meaningful to the UI. +* `password`, a cleartext string. +* `permissions`, a `Permissions` as described below. + +### Permissions + +A JSON object of permissions to perform various actions: + +* `adminUsers`: bool +* `readCameraConfigs`: bool, read camera configs including credentials +* `updateSignals`: bool +* `viewVideo`: bool + +See endpoints above for more details on the contexts in which these are +required. + +## Cross-site request forgery (CSRF) protection + +The API includes several standard protections against [cross-site request +forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery) attacks, in +which a malicious third-party website convinces the user's browser to send +requests on its behalf. + +The following protections apply regardless of method of authentication (session, +no authentication + `allowUnauthenticatedPermissions`, or the browser proxying +to a Unix socket with `ownUidIsPrivileged`): + +* The `GET` method is always "safe". Actions that have significant side + effects require another method such as `DELETE`, `POST`, or `PUT`. This + prevents simple hyperlinks from causing damage. +* Mutations always require some non-default request header (e.g. + `Content-Type: application/json`) so that a `
` will be + rejected. +* The server does *not* override the default + [CORS](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing) policy. + Thus, cross-domain Ajax requests (via `XMLHTTPRequest` or `fetch`) should + fail. +* WebSocket upgrade requests are rejected unless the `Origin` and `Host` + headers match. + +The following additional protections apply only when using session +authentication: + +* Session cookies are set with the [`SameSite=Lax` attribute](samesite-lax), + so that sufficiently modern web browsers will never send the session cookie + on subrequests. (Note they still send session cookies when following links + and on WebSocket upgrade. In these cases, we rely on the protections + described above.) +* Mutations use a `csrf` token, requiring the caller to prove it is able + to read the `GET /api/` response. (This is subect to change. We may decide + to implement these tokens in a way that doesn't require session + authentication or decide they're entirely unnecessary.) + [media-segment]: https://w3c.github.io/media-source/isobmff-byte-stream-format.html#iso-media-segments [init-segment]: https://w3c.github.io/media-source/isobmff-byte-stream-format.html#iso-init-segments [rfc-6381]: https://tools.ietf.org/html/rfc6381 [rfc-6455]: https://tools.ietf.org/html/rfc6455 [multipart-mixed-js]: https://github.com/scottlamb/multipart-mixed-js +[samesite-lax]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#lax diff --git a/design/signal.md b/design/signal.md new file mode 100644 index 0000000..7f6da69 --- /dev/null +++ b/design/signal.md @@ -0,0 +1,35 @@ +# Moonfire NVR Signals + +Status: **draft**. + +"Signals" are what Moonfire NVR uses to describe non-video timeseries data +such as "was motion detected?" or "what mode was my burglar alarm in?" They are +intended to be displayed in the UI with the video scrub bar to aid in finding +a relevant portion of video. + +## Objective + +Goals: + +* represent simple results of on-camera and on-NVR motion detection, e.g.: + `true`, `false`, or `unknown`. +* represent external signals such as burglar alarm state, e.g.: + `off`, `stay`, `away`, `alarm`, or `unknown`. + +Non-goals: + +* provide meaningful data when the NVR has inaccurate system time. +* support internal state necessary for on-NVR motion detection. (This will + be considered separately.) +* support fine-grained outputs such as "what are the bounding boxes of all + detected faces?", "what cells have motion?", audio volume, or audio + spectograms. + +## Overview + +hmm, two ideas: + +* just use timestamps everywhere. allow adding/updating historical data. +* only allow updating the current open. initially, just support setting + current time. then support extending from a previous request. no ability + to fill in while NVR is down. diff --git a/docker/dev.bash b/docker/dev.bash index 91c8711..c8735cf 100755 --- a/docker/dev.bash +++ b/docker/dev.bash @@ -90,7 +90,7 @@ time apt-get install --assume-yes --no-install-recommends "${packages[@]}" # https://doc.rust-lang.org/cargo/guide/build-cache.html if [[ -n "${rust_target}" ]]; then su moonfire-nvr -lc "rustup target install ${rust_target} && - ln -s target/${rust_target}/release/moonfire-nvr ." + ln -s target/${rust_target}/release-lto/moonfire-nvr ." underscore_rust_target="${rust_target//-/_}" uppercase_underscore_rust_target="${underscore_rust_target^^}" cat >> /var/lib/moonfire-nvr/.buildrc < /docker-build-debug/dev/var-cache-apt-after diff --git a/guide/build.md b/guide/build.md index d9f700a..03e7d85 100644 --- a/guide/build.md +++ b/guide/build.md @@ -295,11 +295,11 @@ You'll also need a `/etc/moonfire-nvr.toml`: ```toml [[binds]] ipv4 = "0.0.0.0:8080" -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } [[binds]] unix = "/var/lib/moonfire-nvr/sock" -own_uid_is_privileged = true +ownUidIsPrivileged = true ``` Note this configuration is insecure. You can change that via replacing the diff --git a/guide/install.md b/guide/install.md index 4d403f7..a1b06ab 100644 --- a/guide/install.md +++ b/guide/install.md @@ -71,11 +71,11 @@ $ sudo chmod a+rx /usr/local/bin/nvr ```toml [[binds]] ipv4 = "0.0.0.0:8080" -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } [[binds]] unix = "/var/lib/moonfire-nvr/sock" -own_uid_is_privileged = true +ownUidIsPrivileged = true ``` `/usr/local/bin/nvr`: diff --git a/guide/secure.md b/guide/secure.md index fc9858c..2ac531e 100644 --- a/guide/secure.md +++ b/guide/secure.md @@ -165,13 +165,13 @@ If you follow the recommended Docker setup, your `/etc/moonfire-nvr.json` will contain this line: ```toml -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } ``` Replace it with the following: ```toml -trust_forward_headers = true +trustForwardHeaders = true ``` This change has two effects: diff --git a/server/db/proto/schema.proto b/server/db/proto/schema.proto index 8b14a4e..a24e783 100644 --- a/server/db/proto/schema.proto +++ b/server/db/proto/schema.proto @@ -2,6 +2,11 @@ // Copyright (C) 2018 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt. // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.'; +// Protobuf portion of the Moonfire NVR schema. In general Moonfire's schema +// uses a SQLite3 database with some fields in JSON representation. The protobuf +// stuff is just high-cardinality things that must be compact, e.g. permissions +// that can be stuffed into every user session. + syntax = "proto3"; // Metadata stored in sample file dirs as `/meta`. This is checked @@ -55,25 +60,12 @@ message DirMeta { Open in_progress_open = 4; } -// Permissions to perform actions, currently all simple bools. +// Permissions to perform actions. See description in design/api.md. // -// These indicate actions which may be unnecessary in some contexts. Some -// basic access - like listing the cameras - is currently always allowed. -// See design/api.md for a description of what requires these permissions. -// -// These are used in a few contexts: -// * a session - affects what can be done when using that session to -// authenticate. -// * a user - when a new session is created, it inherits these permissions. -// * on the commandline - to specify what permissions are available for -// unauthenticated access. +// This protobuf form is stored in user and session rows. message Permissions { bool view_video = 1; bool read_camera_configs = 2; - bool update_signals = 3; - - // Administrate user accounts: create, delete accounts; modify passwords of - // accounts other than the caller's own. bool admin_users = 4; } diff --git a/server/src/cmds/run/config.rs b/server/src/cmds/run/config.rs index c8619d2..6913c0e 100644 --- a/server/src/cmds/run/config.rs +++ b/server/src/cmds/run/config.rs @@ -21,6 +21,7 @@ fn default_ui_dir() -> PathBuf { /// Top-level configuration file object. #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct ConfigFile { pub binds: Vec, @@ -42,6 +43,7 @@ pub struct ConfigFile { /// Per-bind configuration. #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct BindConfig { /// The address to bind to. #[serde(flatten)] @@ -70,8 +72,8 @@ pub struct BindConfig { } #[derive(Debug, Deserialize)] -#[serde(rename_all = "lowercase")] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub enum AddressConfig { /// IPv4 address such as `0.0.0.0:8080` or `127.0.0.1:8080`. Ipv4(std::net::SocketAddrV4), diff --git a/server/src/cmds/run/mod.rs b/server/src/cmds/run/mod.rs index 6a1ad90..ba26ba5 100644 --- a/server/src/cmds/run/mod.rs +++ b/server/src/cmds/run/mod.rs @@ -367,7 +367,7 @@ async fn inner( ui_dir: Some(&config.ui_dir), allow_unauthenticated_permissions: b .allow_unauthenticated_permissions - .as_ref() + .clone() .map(db::Permissions::from), trust_forward_hdrs: b.trust_forward_headers, time_zone_name: time_zone_name.clone(), diff --git a/server/src/json.rs b/server/src/json.rs index 803f4df..cb5db23 100644 --- a/server/src/json.rs +++ b/server/src/json.rs @@ -9,7 +9,6 @@ use db::auth::SessionHash; use failure::{format_err, Error}; use serde::ser::{Error as _, SerializeMap, SerializeSeq, Serializer}; use serde::{Deserialize, Deserializer, Serialize}; -use std::collections::BTreeMap; use std::ops::Not; use uuid::Uuid; @@ -25,6 +24,8 @@ pub struct TopLevel<'a> { #[serde(serialize_with = "TopLevel::serialize_cameras")] pub cameras: (&'a db::LockedDatabase, bool, bool), + pub permissions: Permissions, + #[serde(skip_serializing_if = "Option::is_none")] pub user: Option, @@ -121,12 +122,15 @@ pub struct LoginRequest<'a> { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub struct LogoutRequest<'a> { + #[serde(borrow)] pub csrf: &'a str, } #[derive(Deserialize)] #[serde(rename_all = "camelCase")] -pub struct PostSignalsRequest { +pub struct PostSignalsRequest<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, pub signal_ids: Vec, pub states: Vec, pub start: PostSignalsTimeBase, @@ -514,15 +518,33 @@ pub struct ToplevelUser { pub session: Option, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub struct PutUsers<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, + pub user: UserSubset<'a>, +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] pub struct PostUser<'a> { + #[serde(borrow)] pub csrf: Option<&'a str>, pub update: Option>, pub precondition: Option>, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub struct DeleteUser<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, +} + #[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] @@ -553,8 +575,9 @@ where } /// API/config analog of `Permissions` defined in `db/proto/schema.proto`. -#[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct Permissions { #[serde(default)] view_video: bool, @@ -569,8 +592,8 @@ pub struct Permissions { admin_users: bool, } -impl From<&Permissions> for db::schema::Permissions { - fn from(p: &Permissions) -> Self { +impl From for db::schema::Permissions { + fn from(p: Permissions) -> Self { Self { view_video: p.view_video, read_camera_configs: p.read_camera_configs, @@ -581,8 +604,8 @@ impl From<&Permissions> for db::schema::Permissions { } } -impl From<&db::schema::Permissions> for Permissions { - fn from(p: &db::schema::Permissions) -> Self { +impl From for Permissions { + fn from(p: db::schema::Permissions) -> Self { Self { view_video: p.view_video, read_camera_configs: p.read_camera_configs, @@ -595,7 +618,13 @@ impl From<&db::schema::Permissions> for Permissions { /// Response to `GET /api/users/`. #[derive(Serialize)] pub struct GetUsersResponse { - pub users: BTreeMap, + pub users: Vec, +} + +#[derive(Serialize)] +pub struct UserSummary { + pub id: i32, + pub username: String, } /// Response to `PUT /api/users/`. diff --git a/server/src/web/mod.rs b/server/src/web/mod.rs index ec87481..e953e44 100644 --- a/server/src/web/mod.rs +++ b/server/src/web/mod.rs @@ -153,6 +153,16 @@ async fn extract_json_body(req: &mut Request) -> Result) -> Result<(), base::Error> { + match (csrf, caller.user.as_ref().and_then(|u| u.session.as_ref())) { + (None, Some(_)) => bail_t!(Unauthenticated, "csrf must be supplied"), + (Some(csrf), Some(session)) if !csrf_matches(csrf, session.csrf) => { + bail_t!(Unauthenticated, "incorrect csrf"); + } + (_, _) => Ok(()), + } +} + pub struct Config<'a> { pub db: Arc, pub ui_dir: Option<&'a std::path::Path>, @@ -354,6 +364,7 @@ impl Service { user: caller.user, signals: (&db, days), signal_types: &db, + permissions: caller.permissions.into(), }, ) } diff --git a/server/src/web/signals.rs b/server/src/web/signals.rs index b631454..ccee21a 100644 --- a/server/src/web/signals.rs +++ b/server/src/web/signals.rs @@ -12,8 +12,8 @@ use url::form_urlencoded; use crate::json; use super::{ - bad_req, extract_json_body, from_base_error, plain_response, serve_json, Caller, - ResponseResult, Service, + bad_req, extract_json_body, from_base_error, plain_response, require_csrf_if_session, + serve_json, Caller, ResponseResult, Service, }; use std::borrow::Borrow; @@ -42,6 +42,7 @@ impl Service { 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()))?; + require_csrf_if_session(&caller, r.csrf)?; let now = recording::Time::new(self.db.clocks().realtime()); let mut l = self.db.lock(); let start = match r.start { diff --git a/server/src/web/users.rs b/server/src/web/users.rs index a9fb0ce..8f8671e 100644 --- a/server/src/web/users.rs +++ b/server/src/web/users.rs @@ -7,11 +7,11 @@ use base::{bail_t, format_err_t}; use http::{Method, Request, StatusCode}; -use crate::json::{self, PutUsersResponse, UserSubset}; +use crate::json::{self, PutUsersResponse, UserSubset, UserSummary}; use super::{ - bad_req, csrf_matches, extract_json_body, plain_response, serve_json, Caller, ResponseResult, - Service, + bad_req, extract_json_body, plain_response, require_csrf_if_session, serve_json, Caller, + ResponseResult, Service, }; impl Service { @@ -34,7 +34,10 @@ impl Service { .lock() .users_by_id() .iter() - .map(|(&id, user)| (id, user.username.clone())) + .map(|(&id, user)| UserSummary { + id, + username: user.username.clone(), + }) .collect(); serve_json(&req, &json::GetUsersResponse { users }) } @@ -44,23 +47,25 @@ impl Service { bail_t!(Unauthenticated, "must have admin_users permission"); } let r = extract_json_body(&mut req).await?; - let mut r: json::UserSubset = + let mut r: json::PutUsers = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + require_csrf_if_session(&caller, r.csrf)?; let username = r + .user .username .take() .ok_or_else(|| format_err_t!(InvalidArgument, "username must be specified"))?; let mut change = db::UserChange::add_user(username.to_owned()); - if let Some(Some(pwd)) = r.password.take() { + if let Some(Some(pwd)) = r.user.password.take() { change.set_password(pwd.to_owned()); } - if let Some(preferences) = r.preferences.take() { + if let Some(preferences) = r.user.preferences.take() { change.config.preferences = preferences; } - if let Some(ref permissions) = r.permissions.take() { + if let Some(permissions) = r.user.permissions.take() { change.permissions = permissions.into(); } - if r != Default::default() { + if r.user != Default::default() { bail_t!(Unimplemented, "unsupported user fields: {:#?}", r); } let mut l = self.db.lock(); @@ -76,7 +81,7 @@ impl Service { ) -> ResponseResult { match *req.method() { Method::GET | Method::HEAD => self.get_user(req, caller, id).await, - Method::DELETE => self.delete_user(caller, id).await, + Method::DELETE => self.delete_user(req, caller, id).await, Method::POST => self.post_user(req, caller, id).await, _ => Err(plain_response( StatusCode::METHOD_NOT_ALLOWED, @@ -101,15 +106,23 @@ impl Service { } else { None }), - permissions: Some((&user.permissions).into()), + permissions: Some(user.permissions.clone().into()), }; serve_json(&req, &out) } - async fn delete_user(&self, caller: Caller, id: i32) -> ResponseResult { + async fn delete_user( + &self, + mut req: Request, + caller: Caller, + id: i32, + ) -> ResponseResult { if !caller.permissions.admin_users { 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()))?; + require_csrf_if_session(&caller, r.csrf)?; let mut l = self.db.lock(); l.delete_user(id)?; Ok(plain_response(StatusCode::NO_CONTENT, &b""[..])) @@ -137,15 +150,9 @@ impl Service { "to change password, must supply previous password or have admin_users permission" ); } - match (r.csrf, caller.user.and_then(|u| u.session)) { - (None, Some(_)) => bail_t!(Unauthenticated, "csrf must be supplied"), - (Some(csrf), Some(session)) if !csrf_matches(csrf, session.csrf) => { - bail_t!(Unauthenticated, "incorrect csrf"); - } - (_, _) => {} - } + require_csrf_if_session(&caller, r.csrf)?; if let Some(mut precondition) = r.precondition { - if matches!(precondition.username.take(), Some(n) if n != &user.username) { + if matches!(precondition.username.take(), Some(n) if n != user.username) { bail_t!(FailedPrecondition, "username mismatch"); } if matches!(precondition.preferences.take(), Some(ref p) if p != &user.config.preferences) @@ -158,7 +165,7 @@ impl Service { } } if let Some(p) = precondition.permissions.take() { - if user.permissions != db::Permissions::from(&p) { + if user.permissions != db::Permissions::from(p) { bail_t!(FailedPrecondition, "permissions mismatch"); } } @@ -193,7 +200,7 @@ impl Service { change.username = n.to_string(); } if let Some(permissions) = update.permissions.take() { - change.permissions = (&permissions).into(); + change.permissions = permissions.into(); } // Safety valve in case something is added to UserSubset and forgotten here.