Separate registering and linking accounts.

Can register an account with the bot to manage variables and stuff in
private room, and then separately "link" it with a password, which
makes it available to anything using the bot API (aka web app). Can
also unlink and unregister. Check command no longer validates
password. It just checks and reports your account status.

src/bin/dicebot-cmd.rs

@@ -4,7 +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 tenebrous_dicebot::models::Account;
 use url::Url;
 
 #[tokio::main]
@@ -27,7 +27,7 @@ async fn main() -> Result<(), BotError> {
 
     let context = Context {
         db: db,
-        user: User::default(),
+        account: Account::default(),
         matrix_client: &matrix_sdk::Client::new(homeserver)
             .expect("Could not create matrix client"),
         room: RoomContext {

src/bot/command_execution.rs

@@ -1,8 +1,8 @@
 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::logic;
 use crate::matrix;
 use futures::stream::{self, StreamExt};
 use matrix_sdk::{self, identifiers::EventId, room::Joined, Client};
@@ -78,7 +78,7 @@ async fn create_context<'a>(
         matrix_client: client,
         room: room_ctx,
         username: &sender,
-        user: db.get_or_create_user(&sender).await?,
+        account: logic::get_account(db, &sender).await?,
         message_body: &command,
     })
 }

src/cofd/dice.rs

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

src/commands/management.rs

@@ -1,13 +1,13 @@
-use super::{Command, Execution, ExecutionResult};
+use super::{Command, Execution, ExecutionError, ExecutionResult};
 use crate::db::Users;
-use crate::error::BotError::{AccountDoesNotExist, AuthenticationError, PasswordCreationError};
+use crate::error::BotError::{AccountDoesNotExist, PasswordCreationError};
 use crate::logic::hash_password;
 use crate::models::{AccountStatus, User};
 use crate::{context::Context, error::BotError};
 use async_trait::async_trait;
 use std::convert::{Into, TryFrom};
 
-pub struct RegisterCommand(pub String);
+pub struct RegisterCommand;
 
 impl From<RegisterCommand> for Box<dyn Command> {
     fn from(cmd: RegisterCommand) -> Self {
@@ -18,8 +18,8 @@ impl From<RegisterCommand> for Box<dyn Command> {
 impl TryFrom<&str> for RegisterCommand {
     type Error = BotError;
 
-    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        Ok(RegisterCommand(value.to_owned()))
+    fn try_from(_: &str) -> Result<Self, Self::Error> {
+        Ok(RegisterCommand)
     }
 }
 
@@ -34,24 +34,114 @@ impl Command for RegisterCommand {
     }
 
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
-        let pw_hash = hash_password(&self.0).map_err(|e| PasswordCreationError(e))?;
+        if let Some(_) = ctx.db.get_user(ctx.username).await? {
+            return Err(ExecutionError(BotError::AccountAlreadyExists));
+        }
+
         let user = User {
             username: ctx.username.to_owned(),
-            password: Some(pw_hash),
+            password: None,
             account_status: AccountStatus::Registered,
             ..Default::default()
         };
 
         ctx.db.upsert_user(&user).await?;
         Execution::success(format!(
-            "User account registered/updated. Please log in to external applications \
-             with username {} and the password you set.",
+            "User account {} registered for bot commands.",
             ctx.username
         ))
     }
 }
 
-pub struct CheckCommand(pub String);
+pub struct UnlinkCommand(pub String);
+
+impl From<UnlinkCommand> for Box<dyn Command> {
+    fn from(cmd: UnlinkCommand) -> Self {
+        Box::new(cmd)
+    }
+}
+
+impl TryFrom<&str> for UnlinkCommand {
+    type Error = BotError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Ok(UnlinkCommand(value.to_owned()))
+    }
+}
+
+#[async_trait]
+impl Command for UnlinkCommand {
+    fn name(&self) -> &'static str {
+        "unlink user accountx from external applications"
+    }
+
+    fn is_secure(&self) -> bool {
+        true
+    }
+
+    async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
+        let mut user = ctx
+            .db
+            .get_user(&ctx.username)
+            .await?
+            .ok_or(BotError::AccountDoesNotExist)?;
+
+        user.password = None;
+        ctx.db.upsert_user(&user).await?;
+
+        Execution::success(format!(
+            "Accounted {} is now inaccessible to external applications.",
+            ctx.username
+        ))
+    }
+}
+
+pub struct LinkCommand(pub String);
+
+impl From<LinkCommand> for Box<dyn Command> {
+    fn from(cmd: LinkCommand) -> Self {
+        Box::new(cmd)
+    }
+}
+
+impl TryFrom<&str> for LinkCommand {
+    type Error = BotError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Ok(LinkCommand(value.to_owned()))
+    }
+}
+
+#[async_trait]
+impl Command for LinkCommand {
+    fn name(&self) -> &'static str {
+        "link user account to external applications"
+    }
+
+    fn is_secure(&self) -> bool {
+        true
+    }
+
+    async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
+        let mut user = ctx
+            .db
+            .get_user(&ctx.username)
+            .await?
+            .ok_or(BotError::AccountDoesNotExist)?;
+
+        let pw_hash = hash_password(&self.0).map_err(|e| PasswordCreationError(e))?;
+        user.password = Some(pw_hash);
+        ctx.db.upsert_user(&user).await?;
+
+        Execution::success(format!(
+            "Accounted now available for external use. Please log in to \
+             external applications with username {} and the password you set.",
+            ctx.username
+        ))
+    }
+}
+
+pub struct CheckCommand;
 
 impl From<CheckCommand> for Box<dyn Command> {
     fn from(cmd: CheckCommand) -> Self {
@@ -62,15 +152,15 @@ impl From<CheckCommand> for Box<dyn Command> {
 impl TryFrom<&str> for CheckCommand {
     type Error = BotError;
 
-    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        Ok(CheckCommand(value.to_owned()))
+    fn try_from(_: &str) -> Result<Self, Self::Error> {
+        Ok(CheckCommand)
     }
 }
 
 #[async_trait]
 impl Command for CheckCommand {
     fn name(&self) -> &'static str {
-        "check user password"
+        "check user account status"
     }
 
     fn is_secure(&self) -> bool {
@@ -78,11 +168,20 @@ impl Command for CheckCommand {
     }
 
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
-        let user = ctx.db.authenticate_user(&ctx.username, &self.0).await?;
+        let user = ctx.db.get_user(&ctx.username).await?;
 
         match user {
-            Some(_) => Execution::success("Password is correct!".to_string()),
-            None => Err(AuthenticationError.into()),
+            Some(user) => match user.password {
+                Some(_) => Execution::success(
+                    "Account exists, and is available to external applications with a password. If you forgot your password, change it with !link.".to_string(),
+                ),
+                None => Execution::success(
+                    "Account exists, but is not available to external applications.".to_string(),
+                ),
+            },
+            None => Execution::success(
+                "No account registered. Only simple commands in public rooms are available.".to_string(),
+            ),
         }
     }
 }

src/commands/mod.rs

@@ -201,7 +201,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
-            user: crate::models::User::default(),
+            account: crate::models::Account::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: secure_room!(),
@@ -209,7 +209,7 @@ mod tests {
             message_body: "!notacommand",
         };
 
-        let cmd = RegisterCommand("".to_owned());
+        let cmd = RegisterCommand;
         assert_eq!(execution_allowed(&cmd, &ctx).is_ok(), true);
     }
 
@@ -223,7 +223,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
-            user: crate::models::User::default(),
+            account: crate::models::Account::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: secure_room!(),
@@ -245,7 +245,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
-            user: crate::models::User::default(),
+            account: crate::models::Account::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -267,7 +267,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
-            user: crate::models::User::default(),
+            account: crate::models::Account::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),
@@ -275,7 +275,7 @@ mod tests {
             message_body: "!notacommand",
         };
 
-        let cmd = RegisterCommand("".to_owned());
+        let cmd = RegisterCommand;
         assert_eq!(execution_allowed(&cmd, &ctx).is_err(), true);
     }
 
@@ -298,7 +298,7 @@ mod tests {
         let homeserver = Url::parse("http://example.com").unwrap();
 
         let ctx = Context {
-            user: crate::models::User::default(),
+            account: crate::models::Account::default(),
             db: db,
             matrix_client: &matrix_sdk::Client::new(homeserver).unwrap(),
             room: dummy_room!(),

src/commands/parser.rs

@@ -7,7 +7,7 @@ use crate::commands::{
     basic_rolling::RollCommand,
     cofd::PoolRollCommand,
     cthulhu::{CthAdvanceRoll, CthRoll},
-    management::{CheckCommand, RegisterCommand, UnregisterCommand},
+    management::{CheckCommand, LinkCommand, RegisterCommand, UnlinkCommand, UnregisterCommand},
     misc::HelpCommand,
     variables::{
         DeleteVariableCommand, GetAllVariablesCommand, GetVariableCommand, SetVariableCommand,
@@ -86,6 +86,8 @@ pub fn parse_command(input: &str) -> Result<Box<dyn Command>, BotError> {
             "cthadv" | "ctharoll" => convert_to!(CthAdvanceRoll, cmd_input),
             "help" => convert_to!(HelpCommand, cmd_input),
             "register" => convert_to!(RegisterCommand, cmd_input),
+            "link" => convert_to!(LinkCommand, cmd_input),
+            "unlink" => convert_to!(UnlinkCommand, cmd_input),
             "check" => convert_to!(CheckCommand, cmd_input),
             "unregister" => convert_to!(UnregisterCommand, cmd_input),
             _ => Err(CommandParsingError::UnrecognizedCommand(cmd).into()),

src/context.rs

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

src/cthulhu/dice.rs

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

src/db/mod.rs

@@ -16,8 +16,6 @@ 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/users.rs

@@ -3,7 +3,6 @@ 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 {
@@ -76,21 +75,6 @@ impl Users for Database {
         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(
         &self,
         username: &str,
@@ -119,48 +103,6 @@ mod tests {
             .unwrap()
     }
 
-    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-    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;

src/error.rs

@@ -87,6 +87,9 @@ pub enum BotError {
 
     #[error("user account does not exist, try registering")]
     AccountDoesNotExist,
+
+    #[error("user account already exists")]
+    AccountAlreadyExists,
 }
 
 #[derive(Error, Debug)]

src/logic.rs

@@ -1,7 +1,10 @@
-use crate::context::Context;
-use crate::db::Variables;
 use crate::error::{BotError, DiceRollingError};
 use crate::parser::dice::{Amount, Element};
+use crate::{context::Context, models::Account};
+use crate::{
+    db::{sqlite::Database, Users, Variables},
+    models::TransientUser,
+};
 use argon2::{self, Config, Error as ArgonError};
 use futures::stream::{self, StreamExt, TryStreamExt};
 use rand::Rng;
@@ -50,3 +53,71 @@ pub(crate) fn hash_password(raw_password: &str) -> Result<String, ArgonError> {
     let config = Config::default();
     argon2::hash_encoded(raw_password.as_bytes(), &salt, &config)
 }
+
+pub(crate) async fn get_account(db: &Database, username: &str) -> Result<Account, BotError> {
+    Ok(db
+        .get_user(username)
+        .await?
+        .map(|user| Account::Registered(user))
+        .unwrap_or_else(|| {
+            Account::Transient(TransientUser {
+                username: username.to_owned(),
+            })
+        }))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::db::Users;
+    use crate::models::{AccountStatus, User};
+
+    async fn create_db() -> Database {
+        let db_path = tempfile::NamedTempFile::new_in(".").unwrap();
+        crate::db::sqlite::migrator::migrate(db_path.path().to_str().unwrap())
+            .await
+            .unwrap();
+
+        Database::new(db_path.path().to_str().unwrap())
+            .await
+            .unwrap()
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    async fn get_account_no_user_exists() {
+        let db = create_db().await;
+
+        let account = get_account(&db, "@test:example.com")
+            .await
+            .expect("Account retrieval didn't work");
+
+        assert!(matches!(account, Account::Transient(_)));
+
+        let user = account.transient_user().unwrap();
+        assert_eq!(user.username, "@test:example.com");
+    }
+
+    #[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 account = get_account(&db, "myuser")
+            .await
+            .expect("Account retrieval did not work");
+
+        assert!(matches!(account, Account::Registered(_)));
+
+        let user_again = account.registered_user().unwrap();
+        assert_eq!(user, user_again);
+    }
+}

src/models.rs

@@ -10,9 +10,9 @@ pub struct RoomInfo {
 #[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.
+    /// Account is not registered, which means a transient "account"
+    /// with limited information exists only for the duration of the
+    /// command request.
     NotRegistered,
 
     /// User account is fully registered, either via Matrix directly,
@@ -30,6 +30,62 @@ impl Default for AccountStatus {
     }
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub enum Account {
+    /// A registered user account, stored in the database.
+    Registered(User),
+
+    /// A transient account. Not stored in the database. Represents a
+    /// user in a public channel that has not registered directly with
+    /// the bot yet.
+    Transient(TransientUser),
+}
+
+impl Account {
+    /// Gets the account status. For registered users, this is their
+    /// actual account status (fully registered or awaiting
+    /// activation). For transient users, this is
+    /// AccountStatus::NotRegistered.
+    pub fn account_status(&self) -> AccountStatus {
+        match self {
+            Self::Registered(user) => user.account_status,
+            Self::Transient(_) => AccountStatus::NotRegistered,
+        }
+    }
+
+    /// Consume self into an Option<User> instance, which will be Some
+    /// if this account has a registered user, and None otherwise.
+    pub fn registered_user(self) -> Option<User> {
+        match self {
+            Self::Registered(user) => Some(user),
+            _ => None,
+        }
+    }
+
+    /// Consume self into an Option<TransientUser> instance, which
+    /// will be Some if this account has a non-registered user, and
+    /// None otherwise.
+    pub fn transient_user(self) -> Option<TransientUser> {
+        match self {
+            Self::Transient(user) => Some(user),
+            _ => None,
+        }
+    }
+}
+
+impl Default for Account {
+    fn default() -> Self {
+        Account::Transient(TransientUser {
+            username: "".to_string(),
+        })
+    }
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct TransientUser {
+    pub username: String,
+}
+
 #[derive(Eq, PartialEq, Clone, Debug, Default, sqlx::FromRow)]
 pub struct User {
     pub username: String,