more user admin actions

CHANGELOG.md

@@ -12,10 +12,12 @@ Each release is tagged in Git and on the Docker repository
     and some TP-Link cameras. Fixes [#238](https://github.com/scottlamb/moonfire-nvr/issues/238).
 *   expanded API interface for examining and updating users:
     *   `admin_users` permission for operating on arbitrary users.
-    *   `GET /users/` endpoint
+    *   `GET /users/` endpoint to list users
-    *   `GET /users/<id>` endpoint
+    *   `PUT /users/` endpoint to add a user
+    *   `GET /users/<id>` endpoint to examine a user in detail
     *   expanded `POST /users/<id>` endpoint, including password and
         permissions.
+    *   `DELETE /users/<id>` endpoint to delete a user
 
 ## 0.7.5 (2022-05-09)
 

design/api.md

@@ -24,8 +24,10 @@ Status: **current**.
         * [Request 3](#request-3)
     * [User management](#user-management)
         * [`GET /api/users`](#get-apiusers)
+        * [`PUT /api/users`](#put-apiusers)
         * [`GET /api/users/<id>`](#get-apiusersid)
         * [`POST /api/users/<id>`](#post-apiusersid)
+        * [`DELETE /api/users/<id>`](#delete-apiusersid)
 
 ## Objective
 
@@ -834,6 +836,19 @@ Requires the `admin_users` permission.
 Lists all users. Currently there's no paging. Returns a JSON object with
 a `users` key with a map of id to username.
 
+#### `PUT /api/users`
+
+Requires the `admin_users` permission.
+
+Adds a user. Expects a JSON dictionary with the parameters for the user:
+
+*   `username`: a string, which must be unique.
+*   `permissions`: a JSON dictionary of permissions.
+*   `password` (optional): a string.
+*   `preferences` (optional): a JSON dictionary.
+
+Returns status 204 (No Content) on success.
+
 #### `GET /api/users/<id>`
 
 Retrieves the user. Requires the `admin_users` permission if the caller is
@@ -869,6 +884,12 @@ Currently the following fields are supported for `update` and `precondition`:
 
 Returns HTTP status 204 (No Content) on success.
 
+#### `DELETE /api/users/<id>`
+
+Deletes the given user. Requires the `admin_users` permission.
+
+Returns HTTP status 204 (No Content) on success.
+
 [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

server/db/auth.rs

@@ -411,7 +411,7 @@ impl State {
         Ok(state)
     }
 
-    pub fn apply(&mut self, conn: &Connection, change: UserChange) -> Result<&User, Error> {
+    pub fn apply(&mut self, conn: &Connection, change: UserChange) -> Result<&User, base::Error> {
         if let Some(id) = change.id {
             self.update_user(conn, id, change)
         } else {
@@ -432,8 +432,9 @@ impl State {
         conn: &Connection,
         id: i32,
         change: UserChange,
-    ) -> Result<&User, Error> {
+    ) -> Result<&User, base::Error> {
-        let mut stmt = conn.prepare_cached(
+        let mut stmt = conn
+            .prepare_cached(
                 r#"
             update user
             set
@@ -446,7 +447,8 @@ impl State {
             where
                 id = :id
             "#,
-        )?;
+            )
+            .context(ErrorKind::Unknown)?;
         let e = self.users_by_id.entry(id);
         let e = match e {
             ::std::collections::btree_map::Entry::Vacant(_) => panic!("missing uid {}!", id),
@@ -472,7 +474,8 @@ impl State {
                 ":config": &change.config,
                 ":id": &id,
                 ":permissions": &permissions,
-            })?;
+            })
+            .context(ErrorKind::Unknown)?;
         }
         let u = e.into_mut();
         u.username = change.username;
@@ -486,13 +489,15 @@ impl State {
         Ok(u)
     }
 
-    fn add_user(&mut self, conn: &Connection, change: UserChange) -> Result<&User, Error> {
+    fn add_user(&mut self, conn: &Connection, change: UserChange) -> Result<&User, base::Error> {
-        let mut stmt = conn.prepare_cached(
+        let mut stmt = conn
+            .prepare_cached(
                 r#"
             insert into user (username,  password_hash,  config,  permissions)
                       values (:username, :password_hash, :config, :permissions)
             "#,
-        )?;
+            )
+            .context(ErrorKind::Unknown)?;
         let password_hash = change.set_password_hash.unwrap_or(None);
         let permissions = change
             .permissions
@@ -503,7 +508,8 @@ impl State {
             ":password_hash": &password_hash,
             ":config": &change.config,
             ":permissions": &permissions,
-        })?;
+        })
+        .context(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);
@@ -523,16 +529,19 @@ impl State {
         }))
     }
 
-    pub fn delete_user(&mut self, conn: &mut Connection, id: i32) -> Result<(), Error> {
+    pub fn delete_user(&mut self, conn: &mut Connection, id: i32) -> Result<(), base::Error> {
-        let tx = conn.transaction()?;
+        let tx = conn.transaction().context(ErrorKind::Unknown)?;
-        tx.execute("delete from user_session where user_id = ?", params![id])?;
+        tx.execute("delete from user_session where user_id = ?", params![id])
+            .context(ErrorKind::Unknown)?;
         {
-            let mut user_stmt = tx.prepare_cached("delete from user where id = ?")?;
+            let mut user_stmt = tx
-            if user_stmt.execute(params![id])? != 1 {
+                .prepare_cached("delete from user where id = ?")
-                bail!("user {} not found", id);
+                .context(ErrorKind::Unknown)?;
+            if user_stmt.execute(params![id]).context(ErrorKind::Unknown)? != 1 {
+                bail_t!(NotFound, "user {} not found", id);
             }
         }
-        tx.commit()?;
+        tx.commit().context(ErrorKind::Unknown)?;
         let name = self.users_by_id.remove(&id).unwrap().username;
         self.users_by_name.remove(&name).unwrap();
         self.sessions.retain(|_k, ref mut v| v.user_id != id);

server/db/db.rs

@@ -2038,11 +2038,11 @@ impl LockedDatabase {
         self.auth.get_user_by_id_mut(id)
     }
 
-    pub fn apply_user_change(&mut self, change: UserChange) -> Result<&User, Error> {
+    pub fn apply_user_change(&mut self, change: UserChange) -> Result<&User, base::Error> {
         self.auth.apply(&self.conn, change)
     }
 
-    pub fn delete_user(&mut self, id: i32) -> Result<(), Error> {
+    pub fn delete_user(&mut self, id: i32) -> Result<(), base::Error> {
         self.auth.delete_user(&mut self.conn, id)
     }
 

server/src/json.rs

@@ -525,6 +525,9 @@ pub struct PostUser<'a> {
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct UserSubset<'a> {
+    #[serde(borrow)]
+    pub username: Option<&'a str>,
+
     pub preferences: Option<db::json::UserPreferences>,
 
     /// An optional password value.
@@ -587,8 +590,14 @@ impl From<&db::schema::Permissions> for Permissions {
     }
 }
 
-/// Response to `GET /users/`.
+/// Response to `GET /api/users/`.
 #[derive(Serialize)]
-pub struct UsersResponse {
+pub struct GetUsersResponse {
     pub users: BTreeMap<i32, String>,
 }
+
+/// Response to `PUT /api/users/`.
+#[derive(Serialize)]
+pub struct PutUsersResponse {
+    pub id: i32,
+}

server/src/web/users.rs

@@ -7,15 +7,23 @@
 use base::{bail_t, format_err_t};
 use http::{Method, Request, StatusCode};
 
-use crate::json::{self, UserSubset};
+use crate::json::{self, PutUsersResponse, UserSubset};
 
 use super::{
-    bad_req, csrf_matches, extract_json_body, internal_server_err, plain_response, serve_json,
+    bad_req, csrf_matches, extract_json_body, plain_response, serve_json, Caller, ResponseResult,
-    Caller, ResponseResult, Service,
+    Service,
 };
 
 impl Service {
     pub(super) async fn users(&self, req: Request<hyper::Body>, caller: Caller) -> ResponseResult {
+        match *req.method() {
+            Method::GET | Method::HEAD => self.get_users(req, caller).await,
+            Method::PUT => self.put_users(req, caller).await,
+            _ => Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()),
+        }
+    }
+
+    async fn get_users(&self, req: Request<hyper::Body>, caller: Caller) -> ResponseResult {
         if !caller.permissions.admin_users {
             bail_t!(Unauthenticated, "must have admin_users permission");
         }
@@ -26,7 +34,31 @@ impl Service {
             .iter()
             .map(|(&id, user)| (id, user.username.clone()))
             .collect();
-        serve_json(&req, &json::UsersResponse { users })
+        serve_json(&req, &json::GetUsersResponse { users })
+    }
+
+    async fn put_users(&self, mut req: Request<hyper::Body>, caller: Caller) -> 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::UserSubset = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?;
+        let username = r
+            .username
+            .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 {
+            change.set_password(pwd.to_owned());
+        }
+        if let Some(preferences) = r.preferences {
+            change.config.preferences = preferences;
+        }
+        if let Some(ref permissions) = r.permissions {
+            change.permissions = permissions.into();
+        }
+        let mut l = self.db.lock();
+        let user = l.apply_user_change(change)?;
+        serve_json(&req, &PutUsersResponse { id: user.id })
     }
 
     pub(super) async fn user(
@@ -35,26 +67,23 @@ impl Service {
         caller: Caller,
         id: i32,
     ) -> ResponseResult {
-        if caller.user.as_ref().map(|u| u.id) != Some(id) && !caller.permissions.admin_users {
-            bail_t!(
-                Unauthenticated,
-                "must be authenticated as supplied user or have admin_users permission"
-            );
-        }
         match *req.method() {
-            Method::GET | Method::HEAD => self.get_user(req, id).await,
+            Method::GET | Method::HEAD => self.get_user(req, caller, id).await,
+            Method::DELETE => self.delete_user(caller, id).await,
             Method::POST => self.post_user(req, caller, id).await,
             _ => Err(plain_response(StatusCode::METHOD_NOT_ALLOWED, "POST expected").into()),
         }
     }
 
-    async fn get_user(&self, req: Request<hyper::Body>, id: i32) -> ResponseResult {
+    async fn get_user(&self, req: Request<hyper::Body>, caller: Caller, id: i32) -> ResponseResult {
+        require_same_or_admin(&caller, id)?;
         let db = self.db.lock();
         let user = db
             .users_by_id()
             .get(&id)
             .ok_or_else(|| format_err_t!(NotFound, "can't find requested user"))?;
         let out = UserSubset {
+            username: Some(&user.username),
             preferences: Some(user.config.preferences.clone()),
             password: Some(if user.has_password() {
                 Some("(censored)")
@@ -66,12 +95,22 @@ impl Service {
         serve_json(&req, &out)
     }
 
+    async fn delete_user(&self, caller: Caller, id: i32) -> ResponseResult {
+        if !caller.permissions.admin_users {
+            bail_t!(Unauthenticated, "must have admin_users permission");
+        }
+        let mut l = self.db.lock();
+        l.delete_user(id)?;
+        Ok(plain_response(StatusCode::NO_CONTENT, &b""[..]))
+    }
+
     async fn post_user(
         &self,
         mut req: Request<hyper::Body>,
         caller: Caller,
         id: i32,
     ) -> 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 mut db = self.db.lock();
@@ -128,8 +167,18 @@ impl Service {
             if let Some(ref permissions) = update.permissions {
                 change.permissions = permissions.into();
             }
-            db.apply_user_change(change).map_err(internal_server_err)?;
+            db.apply_user_change(change)?;
         }
         Ok(plain_response(StatusCode::NO_CONTENT, &b""[..]))
     }
 }
+
+fn require_same_or_admin(caller: &Caller, id: i32) -> Result<(), base::Error> {
+    if caller.user.as_ref().map(|u| u.id) != Some(id) && !caller.permissions.admin_users {
+        bail_t!(
+            Unauthenticated,
+            "must be authenticated as supplied user or have admin_users permission"
+        );
+    }
+    Ok(())
+}