Add ability to store user active room, with skeleton accounts.

- Adds a user_state table, currently only with active_room.
 - A user must have an account to take advantage of state.
 - Now, all users will get an 'account' even if they don't explicitly register.

To support these automatically created "accounts," the accounts table
now also has an account_status column, indicating if the user is
registered or not (or pending activation--future use).

The User model has been updated with extra properties from the state,
and the user is now carrried in the Context during command execution.
A user is ensured to be created before executing the command.

sqlx-data.json

@@ -60,30 +60,6 @@
       ]
     }
   },
-  "64e137107139c56a43f7041db933671c210df4fa5110fe481d191fd63b2d3aeb": {
-    "query": "SELECT user_id, password FROM accounts\n               WHERE user_id = ?",
-    "describe": {
-      "columns": [
-        {
-          "name": "user_id",
-          "ordinal": 0,
-          "type_info": "Text"
-        },
-        {
-          "name": "password",
-          "ordinal": 1,
-          "type_info": "Text"
-        }
-      ],
-      "parameters": {
-        "Right": 1
-      },
-      "nullable": [
-        false,
-        false
-      ]
-    }
-  },
   "711d222911c1258365a6a0de1fe00eeec4686fd3589e976e225ad599e7cfc75d": {
     "query": "SELECT count(*) as \"count: i32\" FROM user_variables\n               WHERE room_id = ? and user_id = ?",
     "describe": {
@@ -102,66 +78,6 @@
       ]
     }
   },
-  "7248c8ae30bbe4bc5866e80cc277312c7f8cb9af5a8801fd8eaf178fd99eae18": {
-    "query": "SELECT room_id FROM room_users\n               WHERE username = ?",
-    "describe": {
-      "columns": [
-        {
-          "name": "room_id",
-          "ordinal": 0,
-          "type_info": "Text"
-        }
-      ],
-      "parameters": {
-        "Right": 1
-      },
-      "nullable": [
-        false
-      ]
-    }
-  },
-  "97f5d58f62baca51efd8c295ca6737d1240923c69c973621cd0a718ac9eed99f": {
-    "query": "SELECT room_id, room_name FROM room_info\n               WHERE room_id = ?",
-    "describe": {
-      "columns": [
-        {
-          "name": "room_id",
-          "ordinal": 0,
-          "type_info": "Text"
-        },
-        {
-          "name": "room_name",
-          "ordinal": 1,
-          "type_info": "Text"
-        }
-      ],
-      "parameters": {
-        "Right": 1
-      },
-      "nullable": [
-        false,
-        false
-      ]
-    }
-  },
-  "b302d586e5ac4c72c2970361ea5a5936c0b8c6dad10033c626a0ce0404cadb25": {
-    "query": "SELECT username FROM room_users\n                        WHERE room_id = ?",
-    "describe": {
-      "columns": [
-        {
-          "name": "username",
-          "ordinal": 0,
-          "type_info": "Text"
-        }
-      ],
-      "parameters": {
-        "Right": 1
-      },
-      "nullable": [
-        false
-      ]
-    }
-  },
   "bba0fc255e7c30d1d2d9468c68ba38db6e8a13be035aa1152933ba9247b14f8c": {
     "query": "SELECT event_id FROM room_events\n               WHERE room_id = ? AND event_id = ?",
     "describe": {

src/bin/dicebot-cmd.rs

@@ -4,6 +4,7 @@ use tenebrous_dicebot::commands::ResponseExtractor;
 use tenebrous_dicebot::context::{Context, RoomContext};
 use tenebrous_dicebot::db::sqlite::Database;
 use tenebrous_dicebot::error::BotError;
+use tenebrous_dicebot::models::User;
 use url::Url;
 
 #[tokio::main]
@@ -26,6 +27,7 @@ async fn main() -> Result<(), BotError> {
 
     let context = Context {
         db: db,
+        user: User::default(),
         matrix_client: &matrix_sdk::Client::new(homeserver)
             .expect("Could not create matrix client"),
         room: RoomContext {

src/bot/command_execution.rs

@@ -1,6 +1,7 @@
 use crate::commands::{execute_command, ExecutionError, ExecutionResult, ResponseExtractor};
 use crate::context::{Context, RoomContext};
 use crate::db::sqlite::Database;
+use crate::db::Users;
 use crate::error::BotError;
 use crate::matrix;
 use futures::stream::{self, StreamExt};
@@ -77,6 +78,7 @@ async fn create_context<'a>(
         matrix_client: client,
         room: room_ctx,
         username: &sender,
+        user: db.get_or_create_user(&sender).await?,
         message_body: &command,
     })
 }

src/cofd/dice.rs

@@ -483,6 +483,7 @@ mod tests {
             .unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -523,6 +524,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -560,6 +562,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db.clone(),
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),

src/commands/management.rs

@@ -3,7 +3,7 @@ use crate::context::Context;
 use crate::db::Users;
 use crate::error::BotError::{AccountDoesNotExist, AuthenticationError, PasswordCreationError};
 use crate::logic::hash_password;
-use crate::models::User;
+use crate::models::{AccountStatus, User};
 use async_trait::async_trait;
 
 pub struct RegisterCommand(pub String);
@@ -22,7 +22,9 @@ impl Command for RegisterCommand {
         let pw_hash = hash_password(&self.0).map_err(|e| PasswordCreationError(e))?;
         let user = User {
             username: ctx.username.to_owned(),
-            password: pw_hash,
+            password: Some(pw_hash),
+            account_status: AccountStatus::Registered,
+            ..Default::default()
         };
 
         ctx.db.upsert_user(&user).await?;

src/commands/mod.rs

@@ -201,6 +201,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: secure_room!(),
@@ -222,6 +223,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: secure_room!(),
@@ -243,6 +245,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -264,6 +267,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -294,6 +298,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),

src/context.rs

@@ -1,5 +1,6 @@
 use crate::db::sqlite::Database;
 use crate::error::BotError;
+use crate::models::User;
 use matrix_sdk::identifiers::{RoomId, UserId};
 use matrix_sdk::room::Joined;
 use matrix_sdk::Client;
@@ -14,6 +15,7 @@ pub struct Context<'a> {
     pub room: RoomContext<'a>,
     pub username: &'a str,
     pub message_body: &'a str,
+    pub user: User,
 }
 
 impl Context<'_> {

src/cthulhu/dice.rs

@@ -504,6 +504,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -540,6 +541,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -576,6 +578,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
+            user: crate::models::User::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),

src/db/mod.rs

@@ -16,6 +16,8 @@ pub(crate) trait DbState {
 
 #[async_trait]
 pub(crate) trait Users {
+    async fn get_or_create_user(&self, username: &str) -> Result<User, DataError>;
+
     async fn upsert_user(&self, user: &User) -> Result<(), DataError>;
 
     async fn get_user(&self, username: &str) -> Result<Option<User>, DataError>;

src/db/sqlite/migrator/migrations/V8__user_state.rs

@@ -0,0 +1,18 @@
+use barrel::backend::Sqlite;
+use barrel::{types, types::Type, Migration};
+
+fn primary_uuid() -> Type {
+    types::text().unique(true).primary(true).nullable(false)
+}
+
+pub fn migration() -> String {
+    let mut m = Migration::new();
+
+    // Keep track of contextual user state.
+    m.create_table("user_state", move |t| {
+        t.add_column("user_id", primary_uuid());
+        t.add_column("active_room", types::text().nullable(true));
+    });
+
+    m.make::<Sqlite>()
+}

src/db/sqlite/migrator/migrations/V9__nullable_password_and_account_status.rs

@@ -0,0 +1,17 @@
+pub fn migration() -> String {
+    // sqlite does really support alter column, and barrel does not
+    // implement the required workaround, so we do it ourselves!
+    r#"
+      CREATE TABLE IF NOT EXISTS "accounts2" (
+         "user_id" TEXT PRIMARY KEY NOT NULL UNIQUE,
+         "password" TEXT NULL,
+         "account_status" TEXT NOT NULL CHECK(
+             account_status IN ('not_registered', 'registered', 'awaiting_activation'
+         ))
+      );
+      INSERT INTO accounts2 select *, 'registered' FROM accounts;
+      DROP TABLE accounts;
+      ALTER TABLE accounts2 RENAME TO accounts;
+    "#
+    .to_string()
+}

src/db/sqlite/users.rs

@@ -3,45 +3,94 @@ use crate::db::{errors::DataError, Users};
 use crate::error::BotError;
 use crate::models::User;
 use async_trait::async_trait;
+use log::info;
 
 #[async_trait]
 impl Users for Database {
     async fn upsert_user(&self, user: &User) -> Result<(), DataError> {
+        let mut tx = self.conn.begin().await?;
+
         sqlx::query(
-            r#"INSERT INTO accounts (user_id, password) VALUES (?, ?)
-               ON CONFLICT(user_id) DO UPDATE SET password = ?"#,
+            r#"INSERT INTO accounts (user_id, password, account_status)
+               VALUES (?, ?, ?)
+               ON CONFLICT(user_id) DO
+               UPDATE SET password = ?, account_status = ?"#,
         )
         .bind(&user.username)
         .bind(&user.password)
+        .bind(&user.account_status)
         .bind(&user.password)
-        .execute(&self.conn)
+        .bind(&user.account_status)
+        .execute(&mut tx)
         .await?;
 
+        sqlx::query(
+            r#"INSERT INTO user_state (user_id, active_room)
+               VALUES (?, ?)
+               ON CONFLICT(user_id) DO
+               UPDATE SET active_room = ?"#,
+        )
+        .bind(&user.username)
+        .bind(&user.active_room)
+        .bind(&user.active_room)
+        .execute(&mut tx)
+        .await?;
+
+        tx.commit().await?;
         Ok(())
     }
 
     async fn delete_user(&self, username: &str) -> Result<(), DataError> {
+        let mut tx = self.conn.begin().await?;
+
         sqlx::query(r#"DELETE FROM accounts WHERE user_id = ?"#)
             .bind(&username)
-            .execute(&self.conn)
+            .execute(&mut tx)
             .await?;
 
+        sqlx::query(r#"DELETE FROM user_state WHERE user_id = ?"#)
+            .bind(&username)
+            .execute(&mut tx)
+            .await?;
+
+        tx.commit().await?;
+
         Ok(())
     }
 
     async fn get_user(&self, username: &str) -> Result<Option<User>, DataError> {
-        let user_row = sqlx::query!(
-            r#"SELECT user_id, password FROM accounts
-               WHERE user_id = ?"#,
-            username
+        // Should be query_as! macro, but the left join breaks it with a
+        // non existing error message.
+        let user_row: Option<User> = sqlx::query_as(
+            r#"SELECT
+               a.user_id as "username",
+               a.password,
+               s.active_room,
+               COALESCE(a.account_status, 'not_registered') as "account_status"
+               FROM accounts a
+               LEFT JOIN user_state s on a.user_id = s.user_id
+               WHERE a.user_id = ?"#,
         )
+        .bind(username)
         .fetch_optional(&self.conn)
         .await?;
 
-        Ok(user_row.map(|u| User {
-            username: u.user_id,
-            password: u.password,
-        }))
+        Ok(user_row)
+    }
+
+    //TODO should this logic be moved further up into logic.rs maybe?
+    async fn get_or_create_user(&self, username: &str) -> Result<User, DataError> {
+        let maybe_user = self.get_user(username).await?;
+
+        match maybe_user {
+            Some(user) => Ok(user),
+            None => {
+                info!("Creating unregistered account for {}", username);
+                let user = User::unregistered(&username);
+                self.upsert_user(&user).await?;
+                Ok(user)
+            }
+        }
     }
 
     async fn authenticate_user(
@@ -59,6 +108,7 @@ mod tests {
     use super::*;
     use crate::db::sqlite::Database;
     use crate::db::Users;
+    use crate::models::AccountStatus;
 
     async fn create_db() -> Database {
         let db_path = tempfile::NamedTempFile::new_in(".").unwrap();
@@ -72,13 +122,57 @@ mod tests {
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-    async fn create_and_get_user_test() {
+    async fn get_or_create_user_no_user_exists() {
+        let db = create_db().await;
+
+        let user = db
+            .get_or_create_user("@test:example.com")
+            .await
+            .expect("User creation didn't work.");
+
+        assert_eq!(user.username, "@test:example.com");
+
+        let user_again = db
+            .get_user("@test:example.com")
+            .await
+            .expect("User retrieval didn't work.")
+            .expect("No user returned from option.");
+
+        assert_eq!(user, user_again);
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn get_or_create_user_when_user_exists() {
+        let db = create_db().await;
+
+        let user = User {
+            username: "myuser".to_string(),
+            password: Some("abc".to_string()),
+            account_status: AccountStatus::Registered,
+            active_room: Some("myroom".to_string()),
+        };
+
+        let insert_result = db.upsert_user(&user).await;
+        assert!(insert_result.is_ok());
+
+        let user_again = db
+            .get_or_create_user("myuser")
+            .await
+            .expect("User retrieval didn't work.");
+
+        assert_eq!(user, user_again);
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn create_and_get_full_user_test() {
         let db = create_db().await;
 
         let insert_result = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: "abc".to_string(),
+                password: Some("abc".to_string()),
+                account_status: AccountStatus::Registered,
+                active_room: Some("myroom".to_string()),
             })
             .await;
 
@@ -92,7 +186,94 @@ mod tests {
         assert!(user.is_some());
         let user = user.unwrap();
         assert_eq!(user.username, "myuser");
-        assert_eq!(user.password, "abc");
+        assert_eq!(user.password, Some("abc".to_string()));
+        assert_eq!(user.account_status, AccountStatus::Registered);
+        assert_eq!(user.active_room, Some("myroom".to_string()));
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn can_get_user_with_no_state_record() {
+        let db = create_db().await;
+
+        let insert_result = db
+            .upsert_user(&User {
+                username: "myuser".to_string(),
+                password: Some("abc".to_string()),
+                account_status: AccountStatus::Registered,
+                active_room: Some("myroom".to_string()),
+            })
+            .await;
+
+        assert!(insert_result.is_ok());
+
+        sqlx::query("DELETE FROM user_state")
+            .execute(&db.conn)
+            .await
+            .expect("Could not delete from user_state table.");
+
+        let user = db
+            .get_user("myuser")
+            .await
+            .expect("User retrieval query failed");
+
+        assert!(user.is_some());
+        let user = user.unwrap();
+        assert_eq!(user.username, "myuser");
+        assert_eq!(user.password, Some("abc".to_string()));
+
+        //These should be default values because the state record is missing.
+        assert_eq!(user.account_status, AccountStatus::NotRegistered);
+        assert_eq!(user.active_room, None);
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn can_insert_without_password() {
+        let db = create_db().await;
+
+        let insert_result = db
+            .upsert_user(&User {
+                username: "myuser".to_string(),
+                password: None,
+                ..Default::default()
+            })
+            .await;
+
+        assert!(insert_result.is_ok());
+
+        let user = db
+            .get_user("myuser")
+            .await
+            .expect("User retrieval query failed");
+
+        assert!(user.is_some());
+        let user = user.unwrap();
+        assert_eq!(user.username, "myuser");
+        assert_eq!(user.password, None);
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn can_insert_without_active_room() {
+        let db = create_db().await;
+
+        let insert_result = db
+            .upsert_user(&User {
+                username: "myuser".to_string(),
+                active_room: None,
+                ..Default::default()
+            })
+            .await;
+
+        assert!(insert_result.is_ok());
+
+        let user = db
+            .get_user("myuser")
+            .await
+            .expect("User retrieval query failed");
+
+        assert!(user.is_some());
+        let user = user.unwrap();
+        assert_eq!(user.username, "myuser");
+        assert_eq!(user.active_room, None);
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -102,7 +283,8 @@ mod tests {
         let insert_result1 = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: "abc".to_string(),
+                password: Some("abc".to_string()),
+                ..Default::default()
             })
             .await;
 
@@ -111,7 +293,9 @@ mod tests {
         let insert_result2 = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: "123".to_string(),
+                password: Some("123".to_string()),
+                active_room: Some("room".to_string()),
+                account_status: AccountStatus::AwaitingActivation,
             })
             .await;
 
@@ -125,7 +309,11 @@ mod tests {
         assert!(user.is_some());
         let user = user.unwrap();
         assert_eq!(user.username, "myuser");
-        assert_eq!(user.password, "123"); //From second upsert
+
+        //From second upsert
+        assert_eq!(user.password, Some("123".to_string()));
+        assert_eq!(user.active_room, Some("room".to_string()));
+        assert_eq!(user.account_status, AccountStatus::AwaitingActivation);
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -135,7 +323,8 @@ mod tests {
         let insert_result = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: "abc".to_string(),
+                password: Some("abc".to_string()),
+                ..Default::default()
             })
             .await;
 
@@ -171,7 +360,8 @@ mod tests {
         let insert_result = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: crate::logic::hash_password("abc").expect("password hash error!"),
+                password: Some(crate::logic::hash_password("abc").expect("password hash error!")),
+                ..Default::default()
             })
             .await;
 
@@ -194,7 +384,8 @@ mod tests {
         let insert_result = db
             .upsert_user(&User {
                 username: "myuser".to_string(),
-                password: crate::logic::hash_password("abc").expect("password hash error!"),
+                password: Some(crate::logic::hash_password("abc").expect("password hash error!")),
+                ..Default::default()
             })
             .await;
 

src/models.rs

@@ -7,15 +7,52 @@ pub struct RoomInfo {
     pub room_name: String,
 }
 
-#[derive(Eq, PartialEq, Debug)]
+#[derive(Eq, PartialEq, Clone, Copy, Debug, sqlx::Type)]
+#[sqlx(rename_all = "snake_case")]
+pub enum AccountStatus {
+    /// User is not registered, which means the "account" only exists
+    /// for state management in the bot. No privileged actions
+    /// possible.
+    NotRegistered,
+
+    /// User account is fully registered, either via Matrix directly,
+    /// or a web UI sign-up.
+    Registered,
+
+    /// Account is awaiting activation with a registration
+    /// code. Account cannot do privileged actions yet.
+    AwaitingActivation,
+}
+
+impl Default for AccountStatus {
+    fn default() -> Self {
+        AccountStatus::NotRegistered
+    }
+}
+
+#[derive(Eq, PartialEq, Clone, Debug, Default, sqlx::FromRow)]
 pub struct User {
     pub username: String,
-    pub password: String,
+    pub password: Option<String>,
+    pub active_room: Option<String>,
+    pub account_status: AccountStatus,
 }
 
 impl User {
+    /// Create a new unregistered skeleton marker account for a
+    /// username.
+    pub fn unregistered(username: &str) -> User {
+        User {
+            username: username.to_owned(),
+            ..Default::default()
+        }
+    }
+
     pub fn verify_password(&self, raw_password: &str) -> bool {
-        argon2::verify_encoded(&self.password, raw_password.as_bytes()).unwrap_or(false)
+        self.password
+            .as_ref()
+            .and_then(|p| argon2::verify_encoded(p, raw_password.as_bytes()).ok())
+            .unwrap_or(false)
     }
 }
 
@@ -26,8 +63,10 @@ mod tests {
     #[test]
     fn verify_password_passes_with_correct_password() {
         let user = User {
-            username: "myuser".to_string(),
-            password: crate::logic::hash_password("mypassword").expect("Password hashing error!"),
+            password: Some(
+                crate::logic::hash_password("mypassword").expect("Password hashing error!"),
+            ),
+            ..Default::default()
         };
 
         assert_eq!(user.verify_password("mypassword"), true);
@@ -36,8 +75,20 @@ mod tests {
     #[test]
     fn verify_password_fails_with_wrong_password() {
         let user = User {
-            username: "myuser".to_string(),
-            password: crate::logic::hash_password("mypassword").expect("Password hashing error!"),
+            password: Some(
+                crate::logic::hash_password("mypassword").expect("Password hashing error!"),
+            ),
+            ..Default::default()
+        };
+
+        assert_eq!(user.verify_password("wrong-password"), false);
+    }
+
+    #[test]
+    fn verify_password_fails_with_no_password() {
+        let user = User {
+            password: None,
+            ..Default::default()
         };
 
         assert_eq!(user.verify_password("wrong-password"), false);