From a6a143f238c97e1382e367cf91b5c9cd29cfda88 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 12:39:23 +0200 Subject: [PATCH 1/2] user-role-cluster (#761) Reviewed-on: https://git.hofer.link/Ruderverein-Donau-Linz/rowt/pulls/761 --- doc/db/README.md | 1 + doc/db/user.mermaid | 1 + doc/db/user.svg | 2 +- migration.sql | 20 +++++- src/model/family.rs | 13 +++- src/model/role.rs | 42 ++++++++++-- src/model/user.rs | 95 +++++++++++++++++++++++----- src/tera/admin/user.rs | 22 +++++-- staging-diff.sql | 32 ++++++++++ templates/admin/user/index.html.tera | 3 + 10 files changed, 199 insertions(+), 32 deletions(-) diff --git a/doc/db/README.md b/doc/db/README.md index dc2ac23..90ffc4d 100644 --- a/doc/db/README.md +++ b/doc/db/README.md @@ -9,6 +9,7 @@ Thus, here is the current (October '24) model and the reasoning behind it: - All user-relevant fields are stored in `User`. - `Role` (and its associative table `UserRole`) map current roles the user has. This is used for e.g. permissions (`Vorstand`, `Admin`, `cox`, ... roles) and fee calculation (`Donau Linz`, `scheckbuch`, `Rennjugend`). - `Family` specifies, well, a family. Currently only used for fee calculation. +- `cluster` in `Role` groups roles together. There is a db check to only allow for at most 1 role of the same cluster (e.g. either `cox` or `bootsfuehrer`, but not both). ## Planned rowing adventures :-) ![](./planned.svg) diff --git a/doc/db/user.mermaid b/doc/db/user.mermaid index 85dbac6..31d3200 100644 --- a/doc/db/user.mermaid +++ b/doc/db/user.mermaid @@ -29,6 +29,7 @@ classDiagram class Role { +int id +string name + +string cluster } class UserRole { diff --git a/doc/db/user.svg b/doc/db/user.svg index f635599..b1c18f8 100644 --- a/doc/db/user.svg +++ b/doc/db/user.svg @@ -1 +1 @@ -
1
*
1
*
1
0..1
User
+int id
+string name
+string pw
+bool deleted
+datetime last_access
+string dob
+string weight
+string sex
+string dirty_thirty
+string dirty_dozen
+string member_since_date
+string birthdate
+string mail
+string nickname
+string notes
+string phone
+string address
+int family_id
+blob membership_pdf
+string user_token
Family
+int id
Role
+int id
+string name
UserRole
+int user_id
+int role_id
 \ No newline at end of file +
1
*
1
*
1
0..1
User
+int id
+string name
+string pw
+bool deleted
+datetime last_access
+string dob
+string weight
+string sex
+string dirty_thirty
+string dirty_dozen
+string member_since_date
+string birthdate
+string mail
+string nickname
+string notes
+string phone
+string address
+int family_id
+blob membership_pdf
+string user_token
Family
+int id
Role
+int id
+string name
+string cluster
UserRole
+int user_id
+int role_id
 \ No newline at end of file diff --git a/migration.sql b/migration.sql index ad652b5..365aa72 100644 --- a/migration.sql +++ b/migration.sql @@ -27,7 +27,8 @@ CREATE TABLE IF NOT EXISTS "family" ( CREATE TABLE IF NOT EXISTS "role" ( "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, - "name" text NOT NULL UNIQUE + "name" text NOT NULL UNIQUE, + "cluster" text ); CREATE TABLE IF NOT EXISTS "user_role" ( @@ -220,3 +221,20 @@ CREATE TABLE IF NOT EXISTS "distance" ( "distance_in_km" integer NOT NULL ); + +CREATE TRIGGER IF NOT EXISTS prevent_multiple_roles_same_cluster +BEFORE INSERT ON user_role +BEGIN + SELECT CASE + WHEN EXISTS ( + SELECT 1 + FROM user_role ur + JOIN role r1 ON ur.role_id = r1.id + JOIN role r2 ON r1."cluster" = r2."cluster" + WHERE ur.user_id = NEW.user_id + AND r2.id = NEW.role_id + AND r1.id != NEW.role_id + ) + THEN RAISE(ABORT, 'User already has a role in this cluster') + END; +END; diff --git a/src/model/family.rs b/src/model/family.rs index 894b082..6821d33 100644 --- a/src/model/family.rs +++ b/src/model/family.rs @@ -1,5 +1,7 @@ +use std::ops::DerefMut; + use serde::Serialize; -use sqlx::{sqlite::SqliteQueryResult, FromRow, SqlitePool}; +use sqlx::{sqlite::SqliteQueryResult, FromRow, Sqlite, SqlitePool, Transaction}; use super::user::User; @@ -22,6 +24,15 @@ impl Family { .unwrap() } + pub async fn insert_tx(db: &mut Transaction<'_, Sqlite>) -> i64 { + let result: SqliteQueryResult = sqlx::query("INSERT INTO family DEFAULT VALUES") + .execute(db.deref_mut()) + .await + .unwrap(); + + result.last_insert_rowid() + } + pub async fn insert(db: &SqlitePool) -> i64 { let result: SqliteQueryResult = sqlx::query("INSERT INTO family DEFAULT VALUES") .execute(db) diff --git a/src/model/role.rs b/src/model/role.rs index 8904860..c8accad 100644 --- a/src/model/role.rs +++ b/src/model/role.rs @@ -1,17 +1,18 @@ use std::ops::DerefMut; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use sqlx::{FromRow, Sqlite, SqlitePool, Transaction}; -#[derive(FromRow, Serialize, Clone)] +#[derive(FromRow, Serialize, Clone, Deserialize, Debug)] pub struct Role { pub(crate) id: i64, pub(crate) name: String, + pub(crate) cluster: Option, } impl Role { pub async fn all(db: &SqlitePool) -> Vec { - sqlx::query_as!(Role, "SELECT id, name FROM role") + sqlx::query_as!(Role, "SELECT id, name, cluster FROM role") .fetch_all(db) .await .unwrap() @@ -21,7 +22,7 @@ impl Role { sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE id like ? ", @@ -31,12 +32,41 @@ WHERE id like ? .await .ok() } + pub async fn find_by_id_tx(db: &mut Transaction<'_, Sqlite>, name: i32) -> Option { + sqlx::query_as!( + Self, + " +SELECT id, name, cluster +FROM role +WHERE id like ? + ", + name + ) + .fetch_one(db.deref_mut()) + .await + .ok() + } + + pub async fn find_by_cluster_tx(db: &mut Transaction<'_, Sqlite>, name: i32) -> Option { + sqlx::query_as!( + Self, + " +SELECT id, name, cluster +FROM role +WHERE cluster = ? + ", + name + ) + .fetch_one(db.deref_mut()) + .await + .ok() + } pub async fn find_by_name(db: &SqlitePool, name: &str) -> Option { sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE name like ? ", @@ -51,7 +81,7 @@ WHERE name like ? sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE name like ? ", diff --git a/src/model/user.rs b/src/model/user.rs index c2bc150..24127a9 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -490,6 +490,17 @@ ASKÖ Ruderverein Donau Linz", self.name), _ => true, } } + pub async fn has_membership_pdf_tx(&self, db: &mut Transaction<'_, Sqlite>) -> bool { + match sqlx::query_scalar!("SELECT membership_pdf FROM user WHERE id = ?", self.id) + .fetch_one(db.deref_mut()) + .await + .unwrap() + { + Some(a) if a.is_empty() => false, + None => false, + _ => true, + } + } pub async fn roles(&self, db: &SqlitePool) -> Vec { sqlx::query!( @@ -502,6 +513,21 @@ ASKÖ Ruderverein Donau Linz", self.name), .into_iter().map(|r| r.name).collect() } + pub async fn real_roles(&self, db: &SqlitePool) -> Vec { + sqlx::query_as!( + Role, + "SELECT r.id, r.name, r.cluster + FROM role r + JOIN user_role ur ON r.id = ur.role_id + JOIN user u ON u.id = ur.user_id + WHERE ur.user_id = ? AND u.deleted = 0;", + self.id + ) + .fetch_all(db) + .await + .unwrap() + } + pub async fn has_role_tx(&self, db: &mut Transaction<'_, Sqlite>, role: &str) -> bool { if sqlx::query!( "SELECT * FROM user_role WHERE user_id=? AND role_id = (SELECT id FROM role WHERE name = ?)", @@ -697,14 +723,16 @@ ORDER BY last_access DESC .is_ok() } - pub async fn update(&self, db: &SqlitePool, data: UserEditForm<'_>) { + pub async fn update(&self, db: &SqlitePool, data: UserEditForm<'_>) -> Result<(), String> { + let mut db = db.begin().await.map_err(|e| e.to_string())?; + let mut family_id = data.family_id; if family_id.is_some_and(|x| x == -1) { - family_id = Some(Family::insert(db).await) + family_id = Some(Family::insert_tx(&mut db).await) } - if !self.has_membership_pdf(db).await { + if !self.has_membership_pdf_tx(&mut db).await { if let Some(membership_pdf) = data.membership_pdf { let mut stream = membership_pdf.open().await.unwrap(); let mut buffer = Vec::new(); @@ -714,7 +742,7 @@ ORDER BY last_access DESC buffer, self.id ) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); //Okay, because we can only create a User of a valid id } @@ -735,28 +763,29 @@ ORDER BY last_access DESC family_id, self.id ) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); //Okay, because we can only create a User of a valid id // handle roles sqlx::query!("DELETE FROM user_role WHERE user_id = ?", self.id) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); for role_id in data.roles.into_keys() { - self.add_role( - db, - &Role::find_by_id(db, role_id.parse::().unwrap()) - .await - .unwrap(), - ) - .await; + let role = Role::find_by_id_tx(&mut db, role_id.parse::().unwrap()) + .await + .unwrap(); + self.add_role_tx(&mut db, &role).await?; } + + db.commit().await.map_err(|e| e.to_string())?; + + Ok(()) } - pub async fn add_role(&self, db: &SqlitePool, role: &Role) { + pub async fn add_role(&self, db: &SqlitePool, role: &Role) -> Result<(), String> { sqlx::query!( "INSERT INTO user_role(user_id, role_id) VALUES (?, ?)", self.id, @@ -764,7 +793,40 @@ ORDER BY last_access DESC ) .execute(db) .await - .unwrap(); + .map_err(|_| { + format!( + "User already has a role in the cluster '{}'", + role.cluster + .clone() + .expect("db trigger can't activate on empty string") + ) + })?; + + Ok(()) + } + + pub async fn add_role_tx( + &self, + db: &mut Transaction<'_, Sqlite>, + role: &Role, + ) -> Result<(), String> { + sqlx::query!( + "INSERT INTO user_role(user_id, role_id) VALUES (?, ?)", + self.id, + role.id + ) + .execute(db.deref_mut()) + .await + .map_err(|_| { + format!( + "User already has a role in the cluster '{}'", + role.cluster + .clone() + .expect("db trigger can't activate on empty string") + ) + })?; + + Ok(()) } pub async fn remove_role(&self, db: &SqlitePool, role: &Role) { @@ -1234,7 +1296,8 @@ mod test { membership_pdf: None, }, ) - .await; + .await + .unwrap(); let user = User::find_by_id(&pool, 1).await.unwrap(); diff --git a/src/tera/admin/user.rs b/src/tera/admin/user.rs index 9731054..3c7564d 100644 --- a/src/tera/admin/user.rs +++ b/src/tera/admin/user.rs @@ -200,7 +200,8 @@ async fn fees_paid( ) .await; user.add_role(db, &Role::find_by_name(db, "paid").await.unwrap()) - .await; + .await + .expect("paid role has no group"); } } @@ -305,9 +306,10 @@ async fn update( ); }; - user.update(db, data.into_inner()).await; - - Flash::success(Redirect::to("/admin/user"), "Successfully updated user") + match user.update(db, data.into_inner()).await { + Ok(_) => Flash::success(Redirect::to("/admin/user"), "Successfully updated user"), + Err(e) => Flash::error(Redirect::to("/admin/user"), e), + } } #[get("/user//membership")] @@ -394,7 +396,9 @@ async fn create_scheckbuch( // 4. Add 'scheckbuch' role let scheckbuch = Role::find_by_name(db, "scheckbuch").await.unwrap(); - user.add_role(db, &scheckbuch).await; + user.add_role(db, &scheckbuch) + .await + .expect("new user has no roles yet"); // 4. Send welcome mail (+ notification) user.send_welcome_email(db, &config.smtp_pw).await.unwrap(); @@ -434,10 +438,14 @@ async fn schnupper_to_scheckbuch( user.remove_role(db, &paid).await; let scheckbuch = Role::find_by_name(db, "scheckbuch").await.unwrap(); - user.add_role(db, &scheckbuch).await; + user.add_role(db, &scheckbuch) + .await + .expect("just removed 'schnupperant' thus can't have a role with that group"); if let Some(no_einschreibgebuehr) = Role::find_by_name(db, "no-einschreibgebuehr").await { - user.add_role(db, &no_einschreibgebuehr).await; + user.add_role(db, &no_einschreibgebuehr) + .await + .expect("role doesn't have a group"); } user.send_welcome_email(db, &config.smtp_pw).await.unwrap(); diff --git a/staging-diff.sql b/staging-diff.sql index 6fb21fc..e4387f2 100644 --- a/staging-diff.sql +++ b/staging-diff.sql @@ -3,3 +3,35 @@ INSERT INTO user(name) VALUES('Marie'); INSERT INTO "user_role" (user_id, role_id) VALUES((SELECT id from user where name = 'Marie'),(SELECT id FROM role where name = 'Donau Linz')); INSERT INTO user(name) VALUES('Philipp'); INSERT INTO "user_role" (user_id, role_id) VALUES((SELECT id from user where name = 'Philipp'),(SELECT id FROM role where name = 'Donau Linz')); + +ALTER TABLE "role" ADD COLUMN "cluster" text; +CREATE TRIGGER IF NOT EXISTS prevent_multiple_roles_same_cluster +BEFORE INSERT ON user_role +BEGIN + SELECT CASE + WHEN EXISTS ( + SELECT 1 + FROM user_role ur + JOIN role r1 ON ur.role_id = r1.id + JOIN role r2 ON r1."cluster" = r2."cluster" + WHERE ur.user_id = NEW.user_id + AND r2.id = NEW.role_id + AND r1.id != NEW.role_id + ) + THEN RAISE(ABORT, 'User already has a role in this cluster') + END; +END; + + +UPDATE role SET 'cluster'='skill' WHERE id=2; +UPDATE role SET 'cluster'='membership_type' WHERE id=3; +UPDATE role SET 'cluster'='skill' WHERE id=5; +UPDATE role SET 'cluster'='skill' WHERE id=6; +UPDATE role SET 'cluster'='membership_type' WHERE id=7; +UPDATE role SET 'cluster'='financial' WHERE id=8; +UPDATE role SET 'cluster'='membership_type' WHERE id=9; +UPDATE role SET 'cluster'='membership_type' WHERE id=14; +UPDATE role SET 'cluster'='financial' WHERE id=17; +UPDATE role SET 'cluster'='financial' WHERE id=18; +UPDATE role SET 'cluster'='membership_type' WHERE id=20; +UPDATE role SET 'cluster'='membership_type' WHERE id=22; diff --git a/templates/admin/user/index.html.tera b/templates/admin/user/index.html.tera index adf1026..05cd9e5 100644 --- a/templates/admin/user/index.html.tera +++ b/templates/admin/user/index.html.tera @@ -79,9 +79,12 @@
+ {# for cluster, r in roles | group_by(attribute="cluster") #} + {# cluster #} {% for role in roles %} {{ macros::checkbox(label=role.name, name="roles[" ~ role.id ~ "]", id=loop.index , checked=role.name in user.roles, disabled=allowed_to_edit == false) }} {% endfor %} + {# endfor #}
{% if user.membership_pdf %} Date: Sun, 13 Oct 2024 14:51:11 +0200 Subject: [PATCH 2/2] only allow people with access rights to login via wordpress --- seeds.sql | 1 + src/tera/mod.rs | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/seeds.sql b/seeds.sql index f1e1acd..caa32a2 100644 --- a/seeds.sql +++ b/seeds.sql @@ -13,6 +13,7 @@ INSERT INTO "role" (name) VALUES ('kassier'); INSERT INTO "role" (name) VALUES ('schriftfuehrer'); INSERT INTO "role" (name) VALUES ('no-einschreibgebuehr'); INSERT INTO "role" (name) VALUES ('schnupper-betreuer'); +INSERT INTO "role" (name) VALUES ('allow_website_login'); INSERT INTO "user" (name, pw) VALUES('admin', '$argon2id$v=19$m=19456,t=2,p=1$dS/X5/sPEKTj4Rzs/CuvzQ$4P4NCw4Ukhv80/eQYTsarHhnw61JuL1KMx/L9dm82YM'); INSERT INTO "user_role" (user_id, role_id) VALUES(1,1); INSERT INTO "user_role" (user_id, role_id) VALUES(1,2); diff --git a/src/tera/mod.rs b/src/tera/mod.rs index e49e053..314483b 100644 --- a/src/tera/mod.rs +++ b/src/tera/mod.rs @@ -106,10 +106,18 @@ async fn steering(db: &State, user: User, flash: Option, login: Form>) -> String { - match User::login(db, login.name, login.password).await { - Ok(_) => "SUCC".into(), - Err(_) => "FAIL".into(), + if let Ok(user) = User::login(db, login.name, login.password).await { + if user.has_role(db, "allow_website_login").await { + return String::from("SUCC"); + } + if user.has_role(db, "admin").await { + return String::from("SUCC"); + } + if user.has_role(db, "Vorstand").await { + return String::from("SUCC"); + } } + "FAIL".into() } #[catch(401)] //Unauthorized