Add ability to store user active room, with skeleton accounts.
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details

- 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.
 - Bonus: converts user queries to compile-time checked macros.

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.
This commit is contained in:
projectmoon 2021-05-25 15:05:35 +00:00
parent 849a1b6a14
commit 5d002e5063
13 changed files with 367 additions and 115 deletions

View File

@ -18,6 +18,26 @@
]
}
},
"26903a92a7de34df3e227fe599e41ae1bb61612eb80befad398383af36df0ce4": {
"query": "DELETE FROM accounts WHERE user_id = ?",
"describe": {
"columns": [],
"parameters": {
"Right": 1
},
"nullable": []
}
},
"2d4a32735da04509c2e3c4f99bef79ef699964f58ae332b0611f3de088596e1e": {
"query": "INSERT INTO accounts (user_id, password, account_status)\n VALUES (?, ?, ?)\n ON CONFLICT(user_id) DO\n UPDATE SET password = ?, account_status = ?",
"describe": {
"columns": [],
"parameters": {
"Right": 5
},
"nullable": []
}
},
"59313c67900a1a9399389720b522e572f181ae503559cd2b49d6305acb9e2207": {
"query": "SELECT key, value as \"value: i32\" FROM user_variables\n WHERE room_id = ? AND user_id = ?",
"describe": {
@ -60,28 +80,14 @@
]
}
},
"64e137107139c56a43f7041db933671c210df4fa5110fe481d191fd63b2d3aeb": {
"query": "SELECT user_id, password FROM accounts\n WHERE user_id = ?",
"667b26343ce44e1c48ac689ce887ef6a0558a2ce199f7372a5dce58672499c5a": {
"query": "INSERT INTO user_state (user_id, active_room)\n VALUES (?, ?)\n ON CONFLICT(user_id) DO\n UPDATE SET active_room = ?",
"describe": {
"columns": [
{
"name": "user_id",
"ordinal": 0,
"type_info": "Text"
},
{
"name": "password",
"ordinal": 1,
"type_info": "Text"
}
],
"columns": [],
"parameters": {
"Right": 1
"Right": 3
},
"nullable": [
false,
false
]
"nullable": []
}
},
"711d222911c1258365a6a0de1fe00eeec4686fd3589e976e225ad599e7cfc75d": {
@ -102,66 +108,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": {
@ -179,5 +125,15 @@
false
]
}
},
"dce9bb45cf954054a920ee8b53852c6d562e3588d76bbfaa1433d8309d4e4921": {
"query": "DELETE FROM user_state WHERE user_id = ?",
"describe": {
"columns": [],
"parameters": {
"Right": 1
},
"nullable": []
}
}
}

View File

@ -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 {

View File

@ -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,
})
}

View File

@ -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!(),

View File

@ -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?;

View File

@ -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!(),

View File

@ -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<'_> {

View File

@ -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!(),

View File

@ -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>;

View File

@ -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>()
}

View File

@ -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()
}

View File

@ -3,45 +3,92 @@ 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> {
sqlx::query(
r#"INSERT INTO accounts (user_id, password) VALUES (?, ?)
ON CONFLICT(user_id) DO UPDATE SET password = ?"#,
let mut tx = self.conn.begin().await?;
sqlx::query!(
r#"INSERT INTO accounts (user_id, password, account_status)
VALUES (?, ?, ?)
ON CONFLICT(user_id) DO
UPDATE SET password = ?, account_status = ?"#,
user.username,
user.password,
user.account_status,
user.password,
user.account_status
)
.bind(&user.username)
.bind(&user.password)
.bind(&user.password)
.execute(&self.conn)
.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 = ?"#,
user.username,
user.active_room,
user.active_room
)
.execute(&mut tx)
.await?;
tx.commit().await?;
Ok(())
}
async fn delete_user(&self, username: &str) -> Result<(), DataError> {
sqlx::query(r#"DELETE FROM accounts WHERE user_id = ?"#)
.bind(&username)
.execute(&self.conn)
let mut tx = self.conn.begin().await?;
sqlx::query!(r#"DELETE FROM accounts WHERE user_id = ?"#, username)
.execute(&mut tx)
.await?;
sqlx::query!(r#"DELETE FROM user_state WHERE user_id = ?"#, 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 +106,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 +120,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 +184,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::AwaitingActivation,
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()));
assert_eq!(user.account_status, AccountStatus::AwaitingActivation);
//These should be default values because the state record is missing.
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 +281,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 +291,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 +307,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 +321,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 +358,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 +382,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;

View File

@ -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);