From 3747204267e8b75bc77d6c0962b67bbe018dad15 Mon Sep 17 00:00:00 2001 From: Joel Klinghed Date: Thu, 9 Jan 2025 21:20:17 +0100 Subject: Add string id for project and reduce usage of numeric ids in general User: username must be unique, use as primary key and drop id. --- server/src/api_model.rs | 12 ++++----- server/src/auth.rs | 42 +++++++++++++++++-------------- server/src/main.rs | 67 +++++++++++++++++++++---------------------------- server/src/tests.rs | 35 +++++++++++++------------- 4 files changed, 75 insertions(+), 81 deletions(-) (limited to 'server/src') diff --git a/server/src/api_model.rs b/server/src/api_model.rs index 1b0d7b2..6c614e5 100644 --- a/server/src/api_model.rs +++ b/server/src/api_model.rs @@ -18,10 +18,8 @@ pub enum UserReviewRole { #[derive(Debug, Deserialize, Serialize, PartialEq, ToSchema)] pub struct User { - #[schema(example = 1337u64)] - pub id: u64, #[schema(example = "jsmith")] - pub username: String, + pub id: String, #[schema(example = "John Smith")] pub name: String, #[schema(example = true)] @@ -113,8 +111,8 @@ pub struct ProjectUserEntryData { #[derive(Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct Project { - #[schema(example = 1u64)] - pub id: u64, + #[schema(example = "fake")] + pub id: String, #[schema(example = "FAKE: Features All Kids Erase")] pub title: String, #[schema(example = "Example project")] @@ -140,8 +138,8 @@ pub struct ProjectData<'r> { #[derive(Deserialize, Serialize, ToSchema)] pub struct ProjectEntry { - #[schema(example = 1u64)] - pub id: u64, + #[schema(example = "fake")] + pub id: String, #[schema(example = "FAKE: Features All Kids Erase")] pub title: String, } diff --git a/server/src/auth.rs b/server/src/auth.rs index 4889f78..1b7ea89 100644 --- a/server/src/auth.rs +++ b/server/src/auth.rs @@ -74,7 +74,7 @@ struct LdapState { #[derive(Debug, Deserialize, Serialize)] pub struct Session { - pub user_id: u64, + pub user_id: String, session_id: u32, remote: String, } @@ -142,7 +142,7 @@ impl<'r> FromRequest<'r> for Session { fn new_session( sessions: &State, - user_id: u64, + user_id: String, remote: String, max_age: Duration, ) -> Session { @@ -209,17 +209,21 @@ async fn login( mut db: Connection, login: Form>, ) -> Result, Unauthorized<&'static str>> { - let (user_id, maybe_dn) = - sqlx::query!("SELECT id,dn FROM users WHERE username=?", login.username) - .fetch_one(&mut **db) - .map_ok(|r| (r.id, r.dn)) - .map_err(|_| Unauthorized("Unknown username or password")) - .await?; + let maybe_dn = sqlx::query!("SELECT dn FROM users WHERE id=?", login.username) + .fetch_one(&mut **db) + .map_ok(|r| r.dn) + .map_err(|_| Unauthorized("Unknown username or password")) + .await?; if let Some(dn) = maybe_dn { if authenticate(ldap_state, dn.as_str(), login.password).await { let max_age = Duration::days(i64::from(auth_config.session_max_age_days)); - let session = new_session(sessions, user_id, ipaddr.to_string(), max_age); + let session = new_session( + sessions, + login.username.to_string(), + ipaddr.to_string(), + max_age, + ); let cookie = Cookie::build((SESSION_COOKIE, json::to_string(&session).unwrap())) .path("/api") @@ -331,16 +335,16 @@ async fn sync_ldap( // TODO: Insert/Update name as well as dn. - let db_users = sqlx::query!("SELECT id,username,dn FROM users ORDER BY username") + let db_users = sqlx::query!("SELECT id,dn FROM users ORDER BY id") .fetch(&mut *tx) - .map_ok(|r| (r.id, r.username, r.dn)) + .map_ok(|r| (r.id, r.dn)) .try_collect::>() .await .unwrap(); let mut new_users: Vec<(String, String)> = Vec::new(); - let mut updated_users: Vec<(u64, String)> = Vec::new(); - let mut old_users: Vec = Vec::new(); + let mut updated_users: Vec<(String, String)> = Vec::new(); + let mut old_users: Vec = Vec::new(); let mut db_user = db_users.iter().peekable(); @@ -349,16 +353,16 @@ async fn sync_ldap( let uid = se.attrs.get("uid").unwrap().first().unwrap(); loop { if let Some(du) = db_user.peek() { - match du.1.cmp(uid) { + match du.0.cmp(uid) { Ordering::Equal => { - if du.2.as_ref().is_none_or(|x| *x != se.dn) { - updated_users.push((du.0, se.dn)); + if du.1.as_ref().is_none_or(|x| *x != se.dn) { + updated_users.push((du.0.clone(), se.dn)); } db_user.next(); break; } Ordering::Less => { - old_users.push(du.0); + old_users.push(du.0.clone()); db_user.next(); continue; } @@ -372,7 +376,7 @@ async fn sync_ldap( if !new_users.is_empty() { let mut query_builder: sqlx::QueryBuilder = - sqlx::QueryBuilder::new("INSERT INTO users (username,dn) VALUES"); + sqlx::QueryBuilder::new("INSERT INTO users (id,dn) VALUES"); let mut first = true; for pair in new_users { @@ -443,7 +447,7 @@ async fn run_import(rocket: Rocket) -> fairing::Result { async fn run_import(rocket: Rocket) -> fairing::Result { match Db::fetch(&rocket) { Some(db) => match sqlx::query!( - "INSERT IGNORE INTO users (username,dn) VALUES (?,?), (?,?)", + "INSERT IGNORE INTO users (id,dn) VALUES (?,?), (?,?)", "user", "user", "other", diff --git a/server/src/main.rs b/server/src/main.rs index 88546cb..e48cfb7 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -10,6 +10,7 @@ use rocket::serde::json::Json; use rocket::{futures, Build, Rocket}; use rocket_db_pools::{sqlx, Connection, Database}; use sqlx::Acquire; +use std::path::PathBuf; use utoipa::OpenApi; use utoipa_swagger_ui::SwaggerUi; @@ -132,16 +133,15 @@ async fn projects( async fn get_project( db: &mut Connection, - projectid: u64, + projectid: &str, ) -> Result, NotFound<&'static str>> { let users = sqlx::query!( - "SELECT id, username, name, dn, default_role, maintainer FROM users JOIN project_users ON project_users.user=users.id WHERE project_users.project=?", + "SELECT id, name, dn, default_role, maintainer FROM users JOIN project_users ON project_users.user=users.id WHERE project_users.project=?", projectid) .fetch(&mut ***db) .map_ok(|r| api_model::ProjectUserEntry { user: api_model::User { id: r.id, - username: r.username, name: r.name, active: r.dn.is_some(), }, @@ -184,7 +184,7 @@ async fn get_project( async fn project( mut db: Connection, _session: auth::Session, - projectid: u64, + projectid: &str, ) -> Result, NotFound<&'static str>> { get_project(&mut db, projectid).await } @@ -197,26 +197,25 @@ async fn project( ("session" = []), ), )] -#[post("/project/new", data = "")] +#[post("/project//new", data = "")] async fn project_new( mut db: Connection, session: auth::Session, + projectid: &str, data: Json>, ) -> Json { - let projectid: u64; - { let mut tx = db.begin().await.unwrap(); - projectid = sqlx::query!( - "INSERT INTO projects (title, description, remote, main_branch) VALUES (?, ?, ?, ?)", + sqlx::query!( + "INSERT INTO projects (id, title, description, remote, main_branch) VALUES (?, ?, ?, ?, ?)", + projectid, data.title.unwrap_or("Unnamed"), data.description.unwrap_or(""), data.remote.unwrap_or(""), data.main_branch.unwrap_or("main"), ) .execute(&mut *tx) - .map_ok(|r| r.last_insert_id()) .await .unwrap(); @@ -236,7 +235,7 @@ async fn project_new( async fn project_check_maintainer( db: &mut Connection, session: auth::Session, - projectid: u64, + projectid: &str, ) -> Result<&'static str, Custom<&'static str>> { let is_maintainer = sqlx::query!( "SELECT COUNT(user) AS count FROM project_users WHERE project=? AND user=? AND maintainer=TRUE", @@ -273,7 +272,7 @@ async fn project_check_maintainer( async fn project_update( mut db: Connection, session: auth::Session, - projectid: u64, + projectid: &str, data: Json>, ) -> Result<&'static str, Custom<&'static str>> { project_check_maintainer(&mut db, session, projectid).await?; @@ -316,12 +315,12 @@ async fn project_update( ("session" = []), ), )] -#[post("/project//user/new?", data = "")] +#[post("/project//user//new", data = "")] async fn project_user_add( mut db: Connection, session: auth::Session, - projectid: u64, - userid: u64, + projectid: &str, + userid: &str, data: Json, ) -> Result<&'static str, Custom<&'static str>> { project_check_maintainer(&mut db, session, projectid).await?; @@ -357,8 +356,8 @@ async fn project_user_add( async fn project_user_update( mut db: Connection, session: auth::Session, - projectid: u64, - userid: u64, + projectid: &str, + userid: &str, data: Json, ) -> Result<&'static str, Custom<&'static str>> { let need_maintainer = data.maintainer.is_some() || userid != session.user_id; @@ -404,8 +403,8 @@ async fn project_user_update( async fn project_user_del( mut db: Connection, session: auth::Session, - projectid: u64, - userid: u64, + projectid: &str, + userid: &str, ) -> Result<&'static str, Custom<&'static str>> { let need_maintainer = userid != session.user_id; @@ -437,14 +436,14 @@ async fn project_user_del( async fn reviews( mut db: Connection, _session: auth::Session, - projectid: u64, + projectid: &str, limit: Option, offset: Option, ) -> Json { let uw_offset = offset.unwrap_or(0); let uw_limit = limit.unwrap_or(10); let entries = sqlx::query!( - "SELECT reviews.id AS id,title,state,progress,users.id AS user_id,users.username AS username,users.name AS name,users.dn AS user_dn FROM reviews JOIN users ON users.id=owner WHERE project=? ORDER BY id DESC LIMIT ? OFFSET ?", + "SELECT reviews.id AS id,title,state,progress,users.id AS user_id,users.name AS name,users.dn AS user_dn FROM reviews JOIN users ON users.id=owner WHERE project=? ORDER BY id DESC LIMIT ? OFFSET ?", projectid, uw_limit, uw_offset) .fetch(&mut **db) .map_ok(|r| api_model::ReviewEntry { @@ -452,7 +451,6 @@ async fn reviews( title: r.title, owner: api_model::User { id: r.user_id, - username: r.username, name: r.name, active: r.user_dn.is_some(), }, @@ -492,28 +490,24 @@ async fn reviews( ("session" = []), ), )] -#[get("/review/")] +#[get("/review//")] async fn review( mut db: Connection, _session: auth::Session, - reviewid: u64, + projectid: &str, + branch: PathBuf, ) -> Result, NotFound<&'static str>> { - let mut projectid = 0; - let mut review = sqlx::query!( - "SELECT reviews.id AS id,project,title,description,state,progress,branch,archived,users.id AS user_id,users.username AS username,users.name AS name,users.dn AS user_dn FROM reviews JOIN users ON users.id=owner WHERE reviews.id=?", - reviewid) + "SELECT reviews.id AS id,title,description,state,progress,branch,archived,users.id AS user_id,users.name AS name,users.dn AS user_dn FROM reviews JOIN users ON users.id=owner WHERE project=? AND branch=?", + projectid, branch.as_path().to_str().unwrap()) .fetch_one(&mut **db) .map_ok(|r| { - projectid = r.project; - api_model::Review { id: r.id, title: r.title, description: r.description, owner: api_model::User { id: r.user_id, - username: r.username, name: r.name, active: r.user_dn.is_some(), }, @@ -528,13 +522,12 @@ async fn review( .await?; let mut users = sqlx::query!( - "SELECT id,username,name,dn,project_users.default_role AS role FROM users JOIN project_users ON project_users.user=id WHERE project_users.project=? ORDER BY role,username,id", + "SELECT id,name,dn,project_users.default_role AS role FROM users JOIN project_users ON project_users.user=id WHERE project_users.project=? ORDER BY role,id", projectid) .fetch(&mut **db) .map_ok(|r| api_model::ReviewUserEntry { user: api_model::User { id: r.id, - username: r.username, name: r.name, active: r.dn.is_some(), }, @@ -545,13 +538,12 @@ async fn review( .unwrap(); let override_users = sqlx::query!( - "SELECT id,username,name,dn,review_users.role AS role FROM users JOIN review_users ON review_users.user=id WHERE review_users.review=? ORDER BY role,username,id", - reviewid) + "SELECT id,name,dn,review_users.role AS role FROM users JOIN review_users ON review_users.user=id WHERE review_users.review=? ORDER BY role,id", + review.id) .fetch(&mut **db) .map_ok(|r| api_model::ReviewUserEntry { user: api_model::User { id: r.id, - username: r.username, name: r.name, active: r.dn.is_some(), }, @@ -595,14 +587,13 @@ async fn users( let uw_offset = offset.unwrap_or(0); let uw_limit = limit.unwrap_or(10); let entries = sqlx::query!( - "SELECT id,username,name,dn FROM users ORDER BY username LIMIT ? OFFSET ?", + "SELECT id,name,dn FROM users ORDER BY id LIMIT ? OFFSET ?", uw_limit, uw_offset ) .fetch(&mut **db) .map_ok(|r| api_model::User { id: r.id, - username: r.username, name: r.name, active: r.dn.is_some(), }) diff --git a/server/src/tests.rs b/server/src/tests.rs index ca7217b..72bdd78 100644 --- a/server/src/tests.rs +++ b/server/src/tests.rs @@ -180,7 +180,7 @@ async fn get_users<'a>(client: &Client) -> api_model::Users { async fn new_project(client: &Client) -> api_model::Project { get_project_from( client - .post("/api/v1/project/new") + .post("/api/v1/project/test/new") .json(&api_model::ProjectData { title: Some("foo"), description: Some("bar"), @@ -281,13 +281,14 @@ async fn test_project_new() { let project = new_project(&client).await; + assert_eq!(project.id, "test"); assert_eq!(project.title, "foo"); assert_eq!(project.description, "bar"); assert_eq!(project.remote, "fum"); assert_eq!(project.main_branch, "zod"); assert_eq!(project.users.len(), 1); let user = project.users.get(0).unwrap(); - assert_eq!(user.user.username, "user"); + assert_eq!(user.user.id, "user"); assert_eq!(user.default_role, api_model::UserReviewRole::Reviewer); assert_eq!(user.maintainer, true); @@ -309,7 +310,7 @@ async fn test_project_update() { login(&client).await; - let project = get_project_from(client.post("/api/v1/project/new").json( + let project = get_project_from(client.post("/api/v1/project/test/new").json( &api_model::ProjectData { title: Some("foo"), description: None, @@ -351,10 +352,10 @@ async fn test_project_new_user() { let project_url = format!("/api/v1/project/{}", project.id); let users = get_users(&client).await; - let other = users.users.iter().find(|u| u.username == "other").unwrap(); + let other = users.users.iter().find(|u| u.id == "other").unwrap(); let new = client - .post(format!("{project_url}/user/new?userid={}", other.id)) + .post(format!("{project_url}/user/{}/new", other.id)) .json(&api_model::ProjectUserEntryData { default_role: Some(api_model::UserReviewRole::Watcher), maintainer: Some(true), @@ -386,11 +387,11 @@ async fn test_project_change_user() { let project_url = format!("/api/v1/project/{}", project.id); let users = get_users(&client).await; - let user = users.users.iter().find(|u| u.username == "user").unwrap(); - let other = users.users.iter().find(|u| u.username == "other").unwrap(); + let user = users.users.iter().find(|u| u.id == "user").unwrap(); + let other = users.users.iter().find(|u| u.id == "other").unwrap(); let new = client - .post(format!("{project_url}/user/new?userid={}", other.id)) + .post(format!("{project_url}/user/{}/new", other.id)) .json(&api_model::ProjectUserEntryData { default_role: Some(api_model::UserReviewRole::Watcher), maintainer: Some(true), @@ -433,11 +434,11 @@ async fn test_project_check_maintainer() { let project_url = format!("/api/v1/project/{}", project.id); let users = get_users(&client).await; - let user = users.users.iter().find(|u| u.username == "user").unwrap(); - let other = users.users.iter().find(|u| u.username == "other").unwrap(); + let user = users.users.iter().find(|u| u.id == "user").unwrap(); + let other = users.users.iter().find(|u| u.id == "other").unwrap(); let new = client - .post(format!("{project_url}/user/new?userid={}", other.id)) + .post(format!("{project_url}/user/{}/new", other.id)) .json(&api_model::ProjectUserEntryData { default_role: Some(api_model::UserReviewRole::Watcher), maintainer: Some(true), @@ -485,11 +486,11 @@ async fn test_project_dont_check_maintainer() { let project_url = format!("/api/v1/project/{}", project.id); let users = get_users(&client).await; - let user = users.users.iter().find(|u| u.username == "user").unwrap(); - let other = users.users.iter().find(|u| u.username == "other").unwrap(); + let user = users.users.iter().find(|u| u.id == "user").unwrap(); + let other = users.users.iter().find(|u| u.id == "other").unwrap(); let new = client - .post(format!("{project_url}/user/new?userid={}", other.id)) + .post(format!("{project_url}/user/{}/new", other.id)) .json(&api_model::ProjectUserEntryData { default_role: Some(api_model::UserReviewRole::Watcher), maintainer: Some(true), @@ -545,11 +546,11 @@ async fn test_project_delete_user() { let project_url = format!("/api/v1/project/{}", project.id); let users = get_users(&client).await; - let user = users.users.iter().find(|u| u.username == "user").unwrap(); - let other = users.users.iter().find(|u| u.username == "other").unwrap(); + let user = users.users.iter().find(|u| u.id == "user").unwrap(); + let other = users.users.iter().find(|u| u.id == "other").unwrap(); let new = client - .post(format!("{project_url}/user/new?userid={}", other.id)) + .post(format!("{project_url}/user/{}/new", other.id)) .json(&api_model::ProjectUserEntryData { default_role: Some(api_model::UserReviewRole::Watcher), maintainer: Some(true), -- cgit v1.2.3-70-g09d2