extend POST /users/:id

Now you can set a password for a user while the server is running,
e.g. via the following command:

```shell
curl \
    -H 'Content-Type: application/json' \
    -d '{"update": {"password": "asdf"}}' \
    --unix-socket /var/lib/moonfire-nvr/sock \
    http://nvr/api/users/1
```

design/api.md

@@ -4,6 +4,7 @@ Status: **current**.
 
 * [Objective](#objective)
 * [Detailed design](#detailed-design)
+    * [Authentication](#authentication)
         * [`POST /api/login`](#post-apilogin)
         * [`POST /api/logout`](#post-apilogout)
     * [`GET /api/`](#get-api)
@@ -50,7 +51,9 @@ developed tools.
 All requests for JSON data should be sent with the header
 `Accept: application/json` (exactly).
 
-### `POST /api/login`
+### Authentication
+
+#### `POST /api/login`
 
 The request should have an `application/json` body containing a JSON object with
 `username` and `password` keys.
@@ -63,7 +66,7 @@ If authentication or authorization fails, the server will return a HTTP 403
 (forbidden) response. Currently the body will be a `text/plain` error message;
 future versions will likely be more sophisticated.
 
-### `POST /api/logout`
+#### `POST /api/logout`
 
 The request should have an `application/json` body containing
 a `csrf` parameter copied from the `session.csrf` of the
@@ -821,17 +824,22 @@ Response:
 
 ### `POST /api/users/<id>`
 
-Currently this request only allows updating the preferences for the
+Allows updating the given user. Requires the `admin_users` permission if the
-currently-authenticated user. This is likely to change.
+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 both objects support a single field, `preferences`, which should be
+Currently the following fields are supported for `update` and `precondition`:
-a JSON dictionary.
+
+* `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.
 
 Returns HTTP status 204 (No Content) on success.
 

server/db/auth.rs

@@ -62,6 +62,42 @@ impl User {
     pub fn has_password(&self) -> bool {
         self.password_hash.is_some()
     }
+
+    /// Checks if the user's password hash matches the supplied password.
+    ///
+    /// As a side effect, increments `password_failure_count` and sets `dirty`
+    /// if `password` is incorrect.
+    pub fn check_password(&mut self, password: Option<&str>) -> Result<bool, base::Error> {
+        let hash = self.password_hash.as_ref();
+        let (password, hash) = match (password, hash) {
+            (None, None) => return Ok(true),
+            (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)?;
+        match scrypt::Scrypt.verify_password(password.as_bytes(), &hash) {
+            Ok(()) => Ok(true),
+            Err(scrypt::password_hash::errors::Error::Password) => {
+                self.dirty = true;
+                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()),
+        }
+    }
 }
 
 /// A change to a user.
@@ -387,6 +423,10 @@ impl State {
         &self.users_by_id
     }
 
+    pub fn get_user_by_id_mut(&mut self, id: i32) -> Option<&mut User> {
+        self.users_by_id.get_mut(&id)
+    }
+
     fn update_user(
         &mut self,
         conn: &Connection,
@@ -527,26 +567,9 @@ impl State {
         if u.config.disabled {
             bail!("user {:?} is disabled", username);
         }
-        let hash = u
+        if !u.check_password(Some(&password))? {
-            .password_hash
+            bail_t!(Unauthenticated, "incorrect password");
-            .as_ref()
-            .ok_or_else(|| format_err!("no password set for user {:?}", username))?;
-        let hash = PasswordHash::new(hash)
-            .with_context(|_| format!("bad stored password hash for user {:?}", username))?;
-        match scrypt::Scrypt.verify_password(password.as_bytes(), &hash) {
-            Ok(()) => {}
-            Err(scrypt::password_hash::errors::Error::Password) => {
-                u.dirty = true;
-                u.password_failure_count += 1;
-                bail!("incorrect password for user {:?}", username);
         }
-            Err(e) => {
-                return Err(e
-                    .context(format!("unable to verify password for user {:?}", username))
-                    .into());
-            }
-        }
-
         let password_id = u.password_id;
         State::make_session_int(
             &self.rand,
@@ -924,7 +947,7 @@ mod tests {
                 0,
             )
             .unwrap_err();
-        assert_eq!(format!("{}", e), "no password set for user \"slamb\"");
+        assert_eq!(format!("{}", e), "Unauthenticated: incorrect password");
         c.set_password("hunter2".to_owned());
         state.apply(&conn, c).unwrap();
         let e = state
@@ -937,7 +960,7 @@ mod tests {
                 0,
             )
             .unwrap_err();
-        assert_eq!(format!("{}", e), "incorrect password for user \"slamb\"");
+        assert_eq!(format!("{}", e), "Unauthenticated: incorrect password");
         let sid = {
             let (sid, s) = state
                 .login_by_password(

server/db/db.rs

@@ -2034,6 +2034,10 @@ impl LockedDatabase {
         self.auth.users_by_id()
     }
 
+    pub fn get_user_by_id_mut(&mut self, id: i32) -> Option<&mut User> {
+        self.auth.get_user_by_id_mut(id)
+    }
+
     pub fn apply_user_change(&mut self, change: UserChange) -> Result<&User, Error> {
         self.auth.apply(&self.conn, change)
     }

server/db/proto/schema.proto

@@ -72,4 +72,8 @@ message Permissions {
   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;
 }

server/src/cmds/run/config.rs

@@ -95,6 +95,9 @@ pub struct Permissions {
 
     #[serde(default)]
     update_signals: bool,
+
+    #[serde(default)]
+    admin_users: bool,
 }
 
 impl Permissions {
@@ -103,6 +106,7 @@ impl Permissions {
             view_video: self.view_video,
             read_camera_configs: self.read_camera_configs,
             update_signals: self.update_signals,
+            admin_users: self.admin_users,
             ..Default::default()
         }
     }

server/src/json.rs

@@ -6,7 +6,7 @@ use base::time::{Duration, Time};
 use db::auth::SessionHash;
 use failure::{format_err, Error};
 use serde::ser::{Error as _, SerializeMap, SerializeSeq, Serializer};
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Deserializer, Serialize};
 use std::ops::Not;
 use uuid::Uuid;
 
@@ -513,13 +513,31 @@ pub struct ToplevelUser {
 
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct PostUser {
+pub struct PostUser<'a> {
-    pub update: Option<UserSubset>,
+    pub csrf: Option<&'a str>,
-    pub precondition: Option<UserSubset>,
+    pub update: Option<UserSubset<'a>>,
+    pub precondition: Option<UserSubset<'a>>,
 }
 
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct UserSubset {
+pub struct UserSubset<'a> {
     pub preferences: Option<db::json::UserPreferences>,
+
+    /// An optional password value.
+    ///
+    /// `None` indicates the password does not wish to check/update the password.
+    /// `Some(None)` indicates the password should be absent.
+    #[serde(borrow, default, deserialize_with = "deserialize_some")]
+    pub password: Option<Option<&'a str>>,
+}
+
+// Any value that is present is considered Some value, including null.
+// https://github.com/serde-rs/serde/issues/984#issuecomment-314143738
+fn deserialize_some<'de, T, D>(deserializer: D) -> Result<Option<T>, D::Error>
+where
+    T: Deserialize<'de>,
+    D: Deserializer<'de>,
+{
+    Deserialize::deserialize(deserializer).map(Some)
 }

server/src/web/mod.rs

@@ -15,8 +15,7 @@ use self::path::Path;
 use crate::body::Body;
 use crate::json;
 use crate::mp4;
-use base::{bail_t, ErrorKind};
+use base::{bail_t, clock::Clocks, format_err_t, ErrorKind};
-use base::{clock::Clocks, format_err_t};
 use core::borrow::Borrow;
 use core::str::FromStr;
 use db::dir::SampleFileDir;
@@ -82,7 +81,8 @@ fn from_base_error(err: base::Error) -> Response<Body> {
     let status_code = match err.kind() {
         Unauthenticated => StatusCode::UNAUTHORIZED,
         PermissionDenied => StatusCode::FORBIDDEN,
-        InvalidArgument | FailedPrecondition => StatusCode::BAD_REQUEST,
+        InvalidArgument => StatusCode::BAD_REQUEST,
+        FailedPrecondition => StatusCode::PRECONDITION_FAILED,
         NotFound => StatusCode::NOT_FOUND,
         _ => StatusCode::INTERNAL_SERVER_ERROR,
     };
@@ -462,33 +462,66 @@ impl Service {
     }
 
     async fn user(&self, req: Request<hyper::Body>, caller: Caller, id: i32) -> ResponseResult {
-        if caller.user.map(|u| u.id) != Some(id) {
+        if caller.user.as_ref().map(|u| u.id) != Some(id) && !caller.permissions.admin_users {
-            bail_t!(Unauthenticated, "must be authenticated as supplied user");
+            bail_t!(
+                Unauthenticated,
+                "must be authenticated as supplied user or have admin_users permission"
+            );
         }
         match *req.method() {
-            Method::POST => self.post_user(req, id).await,
+            Method::POST => self.post_user(req, caller, id).await,
             _ => Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()),
         }
     }
 
-    async fn post_user(&self, mut req: Request<hyper::Body>, id: i32) -> ResponseResult {
+    async fn post_user(
+        &self,
+        mut req: Request<hyper::Body>,
+        caller: Caller,
+        id: i32,
+    ) -> ResponseResult {
         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 mut db = self.db.lock();
         let user = db
-            .users_by_id()
+            .get_user_by_id_mut(id)
-            .get(&id)
+            .ok_or_else(|| format_err_t!(Internal, "can't find requested user"))?;
-            .ok_or_else(|| format_err_t!(Internal, "can't find currently authenticated user"))?;
+        if r.update.as_ref().map(|u| u.password).is_some()
-        if let Some(precondition) = r.precondition {
+            && r.precondition.as_ref().map(|p| p.password).is_none()
-            if matches!(precondition.preferences, Some(p) if p != user.config.preferences) {
+            && !caller.permissions.admin_users
+        {
+            bail_t!(
+                Unauthenticated,
+                "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");
+            }
+            (_, _) => {}
+        }
+        if let Some(ref precondition) = r.precondition {
+            if matches!(precondition.preferences, Some(ref p) if p != &user.config.preferences) {
                 bail_t!(FailedPrecondition, "preferences mismatch");
             }
+            if let Some(p) = precondition.password {
+                if !user.check_password(p)? {
+                    bail_t!(FailedPrecondition, "password mismatch"); // or Unauthenticated?
+                }
+            }
         }
         if let Some(update) = r.update {
             let mut change = user.change();
             if let Some(preferences) = update.preferences {
                 change.config.preferences = preferences;
             }
+            match update.password {
+                None => {}
+                Some(None) => change.clear_password(),
+                Some(Some(p)) => change.set_password(p.to_owned()),
+            }
             db.apply_user_change(change).map_err(internal_server_err)?;
         }
         Ok(plain_response(StatusCode::NO_CONTENT, &b""[..]))
@@ -611,6 +644,7 @@ impl Service {
                     view_video: true,
                     read_camera_configs: true,
                     update_signals: true,
+                    admin_users: true,
                     ..Default::default()
                 },
                 user: None,