add config json to user table

server/db/auth.rs

@@ -4,6 +4,7 @@
 
 //! Authentication schema: users and sessions/cookies.
 
+use crate::json::UserConfig;
 use crate::schema::Permissions;
 use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt};
 use failure::{bail, format_err, Error};
@@ -13,7 +14,6 @@ use log::info;
 use parking_lot::Mutex;
 use protobuf::Message;
 use ring::rand::{SecureRandom, SystemRandom};
-use rusqlite::types::FromSqlError;
 use rusqlite::{named_params, params, Connection, Transaction};
 use std::collections::BTreeMap;
 use std::fmt;
@@ -35,47 +35,15 @@ pub(crate) fn set_test_config() {
     ));
 }
 
-#[derive(serde::Deserialize, serde::Serialize, Clone, Debug, Default, Eq, PartialEq)]
-pub struct UserPreferences(serde_json::Map<String, serde_json::Value>);
-
-impl rusqlite::types::FromSql for UserPreferences {
-    fn column_result(value: rusqlite::types::ValueRef) -> rusqlite::types::FromSqlResult<Self> {
-        Ok(Self(match value {
-            rusqlite::types::ValueRef::Null => serde_json::Map::default(),
-            rusqlite::types::ValueRef::Text(t) => {
-                serde_json::from_slice(t).map_err(|e| FromSqlError::Other(Box::new(e)))?
-            }
-            _ => return Err(FromSqlError::InvalidType),
-        }))
-    }
-}
-
-impl rusqlite::types::ToSql for UserPreferences {
-    fn to_sql(&self) -> rusqlite::Result<rusqlite::types::ToSqlOutput<'_>> {
-        if self.0.is_empty() {
-            return Ok(rusqlite::types::Null.into());
-        }
-        Ok(serde_json::to_string(&self.0)
-            .map_err(|e| rusqlite::Error::ToSqlConversionFailure(e.into()))?
-            .into())
-    }
-}
-
-enum UserFlag {
-    Disabled = 1,
-}
-
 #[derive(Debug)]
 pub struct User {
     pub id: i32,
     pub username: String,
-    pub flags: i32,
+    pub config: UserConfig,
     password_hash: Option<String>,
     pub password_id: i32,
     pub password_failure_count: i64,
-    pub unix_uid: Option<i32>,
     pub permissions: Permissions,
-    pub preferences: UserPreferences,
 
     /// True iff this `User` has changed since the last flush.
     /// Only a couple things are flushed lazily: `password_failure_count` and (on upgrade to a new
@@ -88,10 +56,8 @@ impl User {
         UserChange {
             id: Some(self.id),
             username: self.username.clone(),
-            flags: self.flags,
+            config: self.config.clone(),
             set_password_hash: None,
-            preferences: self.preferences.clone(),
-            unix_uid: self.unix_uid,
             permissions: self.permissions.clone(),
         }
     }
@@ -99,9 +65,6 @@ impl User {
     pub fn has_password(&self) -> bool {
         self.password_hash.is_some()
     }
-    fn disabled(&self) -> bool {
-        (self.flags & UserFlag::Disabled as i32) != 0
-    }
 }
 
 /// A change to a user.
@@ -114,10 +77,8 @@ impl User {
 pub struct UserChange {
     id: Option<i32>,
     pub username: String,
-    pub flags: i32,
+    pub config: UserConfig,
     set_password_hash: Option<Option<String>>,
-    pub preferences: UserPreferences,
-    pub unix_uid: Option<i32>,
     pub permissions: Permissions,
 }
 
@@ -126,10 +87,8 @@ impl UserChange {
         UserChange {
             id: None,
             username,
-            flags: 0,
+            config: UserConfig::default(),
             set_password_hash: None,
-            preferences: UserPreferences::default(),
-            unix_uid: None,
             permissions: Permissions::default(),
         }
     }
@@ -142,10 +101,6 @@ impl UserChange {
     pub fn clear_password(&mut self) {
         self.set_password_hash = Some(None);
     }
-
-    pub fn disable(&mut self) {
-        self.flags |= UserFlag::Disabled as i32;
-    }
 }
 
 #[derive(Clone, Debug, Default)]
@@ -385,13 +340,11 @@ impl State {
             select
                 id,
                 username,
-                flags,
+                config,
                 password_hash,
                 password_id,
                 password_failure_count,
-                unix_uid,
+                permissions
-                permissions,
-                preferences
             from
                 user
             "#,
@@ -401,20 +354,18 @@ impl State {
             let id = row.get(0)?;
             let name: String = row.get(1)?;
             let mut permissions = Permissions::new();
-            permissions.merge_from_bytes(row.get_ref(7)?.as_blob()?)?;
+            permissions.merge_from_bytes(row.get_ref(6)?.as_blob()?)?;
             state.users_by_id.insert(
                 id,
                 User {
                     id,
                     username: name.clone(),
-                    flags: row.get(2)?,
+                    config: row.get(2)?,
                     password_hash: row.get(3)?,
                     password_id: row.get(4)?,
                     password_failure_count: row.get(5)?,
-                    unix_uid: row.get(6)?,
                     dirty: false,
                     permissions,
-                    preferences: row.get(8)?,
                 },
             );
             state.users_by_name.insert(name, id);
@@ -448,10 +399,8 @@ impl State {
                 password_hash = :password_hash,
                 password_id = :password_id,
                 password_failure_count = :password_failure_count,
-                flags = :flags,
+                config = :config,
-                unix_uid = :unix_uid,
+                permissions = :permissions
-                permissions = :permissions,
-                preferences = :preferences
             where
                 id = :id
             "#,
@@ -478,11 +427,9 @@ impl State {
                 ":password_hash": phash,
                 ":password_id": &pid,
                 ":password_failure_count": &pcount,
-                ":flags": &change.flags,
+                ":config": &change.config,
-                ":unix_uid": &change.unix_uid,
                 ":id": &id,
                 ":permissions": &permissions,
-                ":preferences": &change.preferences,
             })?;
         }
         let u = e.into_mut();
@@ -492,20 +439,16 @@ impl State {
             u.password_id += 1;
             u.password_failure_count = 0;
         }
-        u.flags = change.flags;
+        u.config = change.config;
-        u.unix_uid = change.unix_uid;
         u.permissions = change.permissions;
-        u.preferences = change.preferences;
         Ok(u)
     }
 
     fn add_user(&mut self, conn: &Connection, change: UserChange) -> Result<&User, Error> {
         let mut stmt = conn.prepare_cached(
             r#"
-            insert into user (username,  password_hash,  flags,  unix_uid,  permissions,
+            insert into user (username,  password_hash,  config,  permissions)
-                              preferences)
+                      values (:username, :password_hash, :config, :permissions)
-                      values (:username, :password_hash, :flags, :unix_uid, :permissions,
-                              :preferences)
             "#,
         )?;
         let password_hash = change.set_password_hash.unwrap_or(None);
@@ -516,10 +459,8 @@ impl State {
         stmt.execute(named_params! {
             ":username": &change.username[..],
             ":password_hash": &password_hash,
-            ":flags": &change.flags,
+            ":config": &change.config,
-            ":unix_uid": &change.unix_uid,
             ":permissions": &permissions,
-            ":preferences": &change.preferences,
         })?;
         let id = conn.last_insert_rowid() as i32;
         self.users_by_name.insert(change.username.clone(), id);
@@ -531,14 +472,12 @@ impl State {
         Ok(e.insert(User {
             id,
             username: change.username,
-            flags: change.flags,
+            config: change.config,
             password_hash,
             password_id: 0,
             password_failure_count: 0,
-            unix_uid: change.unix_uid,
             dirty: false,
             permissions: change.permissions,
-            preferences: change.preferences,
         }))
     }
 
@@ -583,7 +522,7 @@ impl State {
             .users_by_id
             .get_mut(id)
             .expect("users_by_name implies users_by_id");
-        if u.disabled() {
+        if u.config.disabled {
             bail!("user {:?} is disabled", username);
         }
         let new_hash = {
@@ -633,7 +572,7 @@ impl State {
             .users_by_id
             .get_mut(&uid)
             .ok_or_else(|| format_err!("no such uid {:?}", uid))?;
-        if u.disabled() {
+        if u.config.disabled {
             bail!("user is disabled");
         }
         State::make_session_int(
@@ -739,7 +678,7 @@ impl State {
         s.last_use = req;
         s.use_count += 1;
         s.dirty = true;
-        if u.disabled() {
+        if u.config.disabled {
             bail_t!(Unauthenticated, "user {:?} is disabled", &u.username);
         }
         Ok((s, u))
@@ -1207,7 +1146,7 @@ mod tests {
         // Disable the user.
         {
             let mut c = state.users_by_id().get(&uid).unwrap().change();
-            c.disable();
+            c.config.disabled = true;
             state.apply(&conn, c).unwrap();
         }
 
@@ -1331,13 +1270,19 @@ mod tests {
         db::init(&mut conn).unwrap();
         let mut state = State::init(&conn).unwrap();
         let mut change = UserChange::add_user("slamb".to_owned());
-        change.preferences.0.insert("foo".to_string(), 42.into());
+        change
+            .config
+            .preferences
+            .insert("foo".to_string(), 42.into());
         let u = state.apply(&conn, change).unwrap();
         let mut change = u.change();
-        change.preferences.0.insert("bar".to_string(), 26.into());
+        change
+            .config
+            .preferences
+            .insert("bar".to_string(), 26.into());
         let u = state.apply(&conn, change).unwrap();
-        assert_eq!(u.preferences.0.get("foo"), Some(&42.into()));
+        assert_eq!(u.config.preferences.get("foo"), Some(&42.into()));
-        assert_eq!(u.preferences.0.get("bar"), Some(&26.into()));
+        assert_eq!(u.config.preferences.get("bar"), Some(&26.into()));
         let uid = u.id;
 
         {
@@ -1347,7 +1292,7 @@ mod tests {
         }
         let state = State::init(&conn).unwrap();
         let u = state.users_by_id().get(&uid).unwrap();
-        assert_eq!(u.preferences.0.get("foo"), Some(&42.into()));
+        assert_eq!(u.config.preferences.get("foo"), Some(&42.into()));
-        assert_eq!(u.preferences.0.get("bar"), Some(&26.into()));
+        assert_eq!(u.config.preferences.get("bar"), Some(&26.into()));
     }
 }

server/db/json.rs

@@ -35,7 +35,7 @@ use std::{collections::BTreeMap, path::PathBuf};
 
 use rusqlite::types::{FromSqlError, ValueRef};
 use serde::{Deserialize, Serialize};
-use serde_json::{Map, Value};
+use serde_json::Value;
 use url::Url;
 use uuid::Uuid;
 
@@ -74,6 +74,7 @@ pub struct GlobalConfig {
     ///
     /// If an update causes this to be exceeded, older times will be garbage
     /// collected to stay within the limit.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
     pub max_signal_changes: Option<u32>,
 
     /// Information about signal types.
@@ -85,7 +86,7 @@ pub struct GlobalConfig {
     pub signals: BTreeMap<u32, SignalConfig>,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(GlobalConfig);
 
@@ -96,7 +97,7 @@ pub struct SampleFileDirConfig {
     pub path: PathBuf,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(SampleFileDirConfig);
 
@@ -120,7 +121,7 @@ pub struct SignalTypeConfig {
     pub values: BTreeMap<u8, SignalTypeValueConfig>,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(SignalTypeConfig);
 
@@ -137,7 +138,7 @@ pub struct SignalTypeValueConfig {
     pub color: String,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 
 impl SignalTypeValueConfig {
@@ -172,7 +173,7 @@ pub struct CameraConfig {
     pub password: String,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(CameraConfig);
 
@@ -230,7 +231,7 @@ pub struct StreamConfig {
     pub flush_if_sec: u32,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(StreamConfig);
 
@@ -274,6 +275,35 @@ pub struct SignalConfig {
     pub camera_associations: BTreeMap<i32, String>,
 
     #[serde(flatten)]
-    pub unknown: Map<String, Value>,
+    pub unknown: BTreeMap<String, Value>,
 }
 sql!(SignalConfig);
+
+/// User configuration, used in the `config` column of the `user` table.
+#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct UserConfig {
+    /// If true, no method of authentication will succeed for this user.
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    pub disabled: bool,
+
+    /// If set, a Unix UID that is accepted for authentication when using HTTP over
+    /// a Unix domain socket.
+    ///
+    /// (Additionally, the UID running Moonfire NVR can authenticate as anyone;
+    /// there's no point in trying to do otherwise.) This might be an easy
+    /// bootstrap method once configuration happens through a web UI rather than
+    /// text UI.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub unix_uid: Option<u64>,
+
+    /// Preferences controlled by the user.
+    #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
+    pub preferences: UserPreferences,
+
+    #[serde(flatten)]
+    pub unknown: BTreeMap<String, Value>,
+}
+sql!(UserConfig);
+
+pub type UserPreferences = BTreeMap<String, Value>;

server/db/schema.sql

@@ -289,11 +289,18 @@ create table user (
   id integer primary key,
   username unique not null,
 
-  -- Bitwise mask of flags:
+  -- A json.UserConfig.
-  -- 1: disabled. If set, no method of authentication for this user will succeed.
+  config text,
-  flags integer not null,
 
-  -- If set, a hash for password authentication, as generated by `libpasta::hash_password`.
+  -- If set, a hash for password authentication, as generated by
+  -- `libpasta::hash_password`. This is separate from config for two reasons:
+  -- *   It should never be sent over the wire, because password hashes are
+  --     almost as sensitive as passwords themselves. Keeping it separate avoids
+  --     complicating the protocol for retrieving the config and updating it
+  --     with optimistic concurrency control.
+  -- *   It may be updated while authenticating to upgrade the password hash
+  --     format, and the conflicting writes again might complicate the update
+  --     protocol.
   password_hash text,
 
   -- A counter which increments with every password reset or clear.
@@ -303,19 +310,9 @@ create table user (
   -- This could be used to automatically disable the password on hitting a threshold.
   password_failure_count integer not null default 0,
 
-  -- If set, a Unix UID that is accepted for authentication when using HTTP over
-  -- a Unix domain socket. (Additionally, the UID running Moonfire NVR can authenticate
-  -- as anyone; there's no point in trying to do otherwise.) This might be an easy
-  -- bootstrap method once configuration happens through a web UI rather than text UI.
-  unix_uid integer,
-
   -- Permissions available for newly created tokens or when authenticating via
   -- unix_uid above. A serialized "Permissions" protobuf.
-  permissions blob not null default X'',
+  permissions blob not null default X''
-
-  -- Preferences controlled by the user. A JSON object, or null to represent
-  -- the empty object. Can be returned and modified through the API.
-  preferences text
 );
 
 -- A single session, whether for browser or robot use.

server/db/upgrade/v6_to_v7.rs

@@ -11,7 +11,9 @@ use url::Url;
 use uuid::Uuid;
 
 use crate::{
-    json::{CameraConfig, GlobalConfig, SampleFileDirConfig, SignalConfig, SignalTypeConfig},
+    json::{
+        CameraConfig, GlobalConfig, SampleFileDirConfig, SignalConfig, SignalTypeConfig, UserConfig,
+    },
     SqlUuid,
 };
 
@@ -69,6 +71,57 @@ fn copy_sample_file_dir(tx: &rusqlite::Transaction) -> Result<(), Error> {
     Ok(())
 }
 
+fn copy_users(tx: &rusqlite::Transaction) -> Result<(), Error> {
+    let mut stmt = tx.prepare(
+        r#"
+        select
+          id,
+          username,
+          flags,
+          password_hash,
+          password_id,
+          password_failure_count,
+          unix_uid,
+          permissions
+        from old_user
+        "#,
+    )?;
+    let mut insert = tx.prepare(
+        r#"
+        insert into user (id,  username,  config,  password_hash,  password_id,
+                          password_failure_count,  permissions)
+                  values (:id, :username, :config, :password_hash, :password_id,
+                          :password_failure_count, :permissions)
+        "#,
+    )?;
+    let mut rows = stmt.query(params![])?;
+    while let Some(row) = rows.next()? {
+        let id: i32 = row.get(0)?;
+        let username: String = row.get(1)?;
+        let flags: i32 = row.get(2)?;
+        let password_hash: String = row.get(3)?;
+        let password_id: i32 = row.get(4)?;
+        let password_failure_count: i32 = row.get(5)?;
+        let unix_uid: Option<i64> = row.get(6)?;
+        let permissions: Vec<u8> = row.get(7)?;
+        let config = UserConfig {
+            disabled: (flags & 1) != 0,
+            unix_uid: unix_uid.map(u64::try_from).transpose()?,
+            ..Default::default()
+        };
+        insert.execute(named_params! {
+            ":id": id,
+            ":username": username,
+            ":config": config,
+            ":password_hash": password_hash,
+            ":password_id": password_id,
+            ":password_failure_count": password_failure_count,
+            ":permissions": permissions,
+        })?;
+    }
+    Ok(())
+}
+
 fn copy_signal_types(tx: &rusqlite::Transaction) -> Result<(), Error> {
     let mut types_ = FnvHashMap::default();
     let mut stmt = tx.prepare("select type_uuid, value, name from signal_type_enum")?;
@@ -288,7 +341,8 @@ pub fn run(_args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
     tx.execute_batch(
         r#"
         alter table open add boot_uuid check (length(boot_uuid) = 16);
-        alter table user add preferences text;
+        alter table user rename to old_user;
+        alter table user_session rename to old_user_session;
         alter table camera rename to old_camera;
         alter table stream rename to old_stream;
         alter table signal rename to old_signal;
@@ -338,6 +392,42 @@ pub fn run(_args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
           uuid blob primary key check (length(uuid) = 16),
           config text
         ) without rowid;
+
+        create table user (
+          id integer primary key,
+          username unique not null,
+          config text,
+          password_hash text,
+          password_id integer not null default 0,
+          password_failure_count integer not null default 0,
+          permissions blob not null default X''
+        );
+
+        create table user_session (
+          session_id_hash blob primary key not null,
+          user_id integer references user (id) not null,
+          seed blob not null,
+          flags integer not null,
+          domain text,
+          description text,
+          creation_password_id integer,
+          creation_time_sec integer not null,
+          creation_user_agent text,
+          creation_peer_addr blob,
+          revocation_time_sec integer,
+          revocation_user_agent text,
+          revocation_peer_addr blob,
+          revocation_reason integer,
+          revocation_reason_detail text,
+          last_use_time_sec integer,
+          last_use_user_agent text,
+          last_use_peer_addr blob,
+          use_count not null default 0,
+          permissions blob not null default X''
+        ) without rowid;
+
+        drop index user_session_uid;
+        create index user_session_uid on user_session (user_id);
     "#,
     )?;
     copy_meta(tx)?;
@@ -346,8 +436,11 @@ pub fn run(_args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
     copy_signal_types(tx)?;
     copy_signals(tx)?;
     copy_streams(tx)?;
+    copy_users(tx)?;
     tx.execute_batch(
         r#"
+        insert into user_session select * from old_user_session;
+
         drop index recording_cover;
 
         alter table recording rename to old_recording;
@@ -416,6 +509,8 @@ pub fn run(_args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
         drop table old_sample_file_dir;
         drop table old_meta;
         drop table old_signal;
+        drop table old_user_session;
+        drop table old_user;
         drop table signal_type_enum;
         drop table signal_camera;
     "#,

server/src/json.rs

@@ -509,7 +509,7 @@ impl VideoSampleEntry {
 pub struct ToplevelUser {
     pub name: String,
     pub id: i32,
-    pub preferences: db::auth::UserPreferences,
+    pub preferences: db::json::UserPreferences,
     pub session: Option<Session>,
 }
 
@@ -523,5 +523,5 @@ pub struct PostUser {
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct UserSubset {
-    pub preferences: Option<db::auth::UserPreferences>,
+    pub preferences: Option<db::json::UserPreferences>,
 }

server/src/web.rs

@@ -1041,14 +1041,14 @@ impl Service {
             .get(&id)
             .ok_or_else(|| format_err_t!(Internal, "can't find currently authenticated user"))?;
         if let Some(precondition) = r.precondition {
-            if matches!(precondition.preferences, Some(p) if p != user.preferences) {
+            if matches!(precondition.preferences, Some(p) if p != user.config.preferences) {
                 bail_t!(FailedPrecondition, "preferences mismatch");
             }
         }
         if let Some(update) = r.update {
             let mut change = user.change();
             if let Some(preferences) = update.preferences {
-                change.preferences = preferences;
+                change.config.preferences = preferences;
             }
             db.apply_user_change(change).map_err(internal_server_err)?;
         }
@@ -1290,7 +1290,7 @@ impl Service {
                         user: Some(json::ToplevelUser {
                             id: s.user_id,
                             name: u.username.clone(),
-                            preferences: u.preferences.clone(),
+                            preferences: u.config.preferences.clone(),
                             session: Some(json::Session { csrf: s.csrf() }),
                         }),
                     })