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::Account;
+use tenebrous_dicebot::models::User;
 use url::Url;
 
 #[tokio::main]
@@ -27,7 +27,7 @@ async fn main() -> Result<(), BotError> {
 
     let context = Context {
         db: db,
-        account: Account::default(),
+        user: User::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,
-        account: logic::get_account(db, &sender).await?,
+        user: db.get_or_create_user(&sender).await?,
         message_body: &command,
     })
 }

src/cofd/dice.rs

@@ -483,7 +483,7 @@ mod tests {
             .unwrap();
 
         let ctx = Context {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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, ExecutionError, ExecutionResult};
+use super::{Command, Execution, ExecutionResult};
 use crate::db::Users;
-use crate::error::BotError::{AccountDoesNotExist, PasswordCreationError};
+use crate::error::BotError::{AccountDoesNotExist, AuthenticationError, 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 struct RegisterCommand(pub String);
 
 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(_: &str) -> Result<Self, Self::Error> {
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        Ok(RegisterCommand)
+        Ok(RegisterCommand(value.to_owned()))
     }
 }
 
@@ -34,114 +34,24 @@ impl Command for RegisterCommand {
     }
 
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
-        if let Some(_) = ctx.db.get_user(ctx.username).await? {
+        let pw_hash = hash_password(&self.0).map_err(|e| PasswordCreationError(e))?;
-            return Err(ExecutionError(BotError::AccountAlreadyExists));
-        }
-
         let user = User {
             username: ctx.username.to_owned(),
-            password: None,
+            password: Some(pw_hash),
             account_status: AccountStatus::Registered,
             ..Default::default()
         };
 
         ctx.db.upsert_user(&user).await?;
         Execution::success(format!(
-            "User account {} registered for bot commands.",
+            "User account registered/updated. Please log in to external applications \
+             with username {} and the password you set.",
             ctx.username
         ))
     }
 }
 
-pub struct UnlinkCommand(pub String);
+pub struct CheckCommand(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 {
@@ -152,15 +62,15 @@ impl From<CheckCommand> for Box<dyn Command> {
 impl TryFrom<&str> for CheckCommand {
     type Error = BotError;
 
-    fn try_from(_: &str) -> Result<Self, Self::Error> {
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        Ok(CheckCommand)
+        Ok(CheckCommand(value.to_owned()))
     }
 }
 
 #[async_trait]
 impl Command for CheckCommand {
     fn name(&self) -> &'static str {
-        "check user account status"
+        "check user password"
     }
 
     fn is_secure(&self) -> bool {
@@ -168,20 +78,11 @@ impl Command for CheckCommand {
     }
 
     async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult {
-        let user = ctx.db.get_user(&ctx.username).await?;
+        let user = ctx.db.authenticate_user(&ctx.username, &self.0).await?;
 
         match user {
-            Some(user) => match user.password {
+            Some(_) => Execution::success("Password is correct!".to_string()),
-                Some(_) => Execution::success(
+            None => Err(AuthenticationError.into()),
-                    "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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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;
+        let cmd = RegisterCommand("".to_owned());
         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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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;
+        let cmd = RegisterCommand("".to_owned());
         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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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, LinkCommand, RegisterCommand, UnlinkCommand, UnregisterCommand},
+    management::{CheckCommand, RegisterCommand, UnregisterCommand},
     misc::HelpCommand,
     variables::{
         DeleteVariableCommand, GetAllVariablesCommand, GetVariableCommand, SetVariableCommand,
@@ -86,8 +86,6 @@ 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::Account;
+use crate::models::User;
 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 account: Account,
+    pub user: User,
 }
 
 impl Context<'_> {

src/cthulhu/dice.rs

@@ -504,7 +504,7 @@ mod tests {
 
         let homeserver = Url::parse("http://example.com").unwrap();
         let ctx = Context {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            user: crate::models::User::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 {
-            account: crate::models::Account::default(),
+            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/users.rs

@@ -3,6 +3,7 @@ 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 {
@@ -75,6 +76,21 @@ 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,
@@ -103,6 +119,48 @@ 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,9 +87,6 @@ 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,10 +1,7 @@
+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;
@@ -53,71 +50,3 @@ 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 {
-    /// Account is not registered, which means a transient "account"
+    /// User is not registered, which means the "account" only exists
-    /// with limited information exists only for the duration of the
+    /// for state management in the bot. No privileged actions
-    /// command request.
+    /// possible.
     NotRegistered,
 
     /// User account is fully registered, either via Matrix directly,
@@ -30,62 +30,6 @@ 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,