From 3cc141d77864412f72ea77405071eb995847f5cc Mon Sep 17 00:00:00 2001 From: Ashish Kolhe <35160958+oasisk@users.noreply.github.com> Date: Sat, 23 Dec 2023 22:48:59 +0530 Subject: [PATCH] fix: user api checks (#2294) - [x] Check role during User Creation - [x] Added checks during User Deletion/removal from org --- src/cli/basic/cli.rs | 1 + src/common/utils/auth.rs | 2 +- src/handler/http/auth/validator.rs | 2 + src/handler/http/request/users/mod.rs | 21 ++- src/job/mod.rs | 2 +- src/service/db/user.rs | 2 +- src/service/organization.rs | 1 + src/service/users.rs | 203 +++++++++++++++----------- 8 files changed, 142 insertions(+), 92 deletions(-) diff --git a/src/cli/basic/cli.rs b/src/cli/basic/cli.rs index 9dc229af4c..9fc051d64f 100644 --- a/src/cli/basic/cli.rs +++ b/src/cli/basic/cli.rs @@ -125,6 +125,7 @@ pub async fn cli() -> Result { last_name: "".to_owned(), is_external: false, }, + &CONFIG.auth.root_user_email.clone(), ) .await?; } diff --git a/src/common/utils/auth.rs b/src/common/utils/auth.rs index 2a1615dc77..8f696837ad 100644 --- a/src/common/utils/auth.rs +++ b/src/common/utils/auth.rs @@ -115,7 +115,7 @@ mod tests { #[actix_web::test] async fn test_is_root_user2() { infra_db::create_table().await.unwrap(); - let _ = users::post_user( + let _ = users::create_root_user( DEFAULT_ORG, UserRequest { email: "root@example.com".to_string(), diff --git a/src/handler/http/auth/validator.rs b/src/handler/http/auth/validator.rs index 52ccce833e..d22c87a7b1 100644 --- a/src/handler/http/auth/validator.rs +++ b/src/handler/http/auth/validator.rs @@ -404,6 +404,7 @@ mod tests { last_name: "".to_owned(), is_external: false, }, + "root@example.com", ) .await; let _ = users::post_user( @@ -416,6 +417,7 @@ mod tests { last_name: "".to_owned(), is_external: false, }, + "root@example.com", ) .await; diff --git a/src/handler/http/request/users/mod.rs b/src/handler/http/request/users/mod.rs index 6df6b6ce12..75dd577af1 100644 --- a/src/handler/http/request/users/mod.rs +++ b/src/handler/http/request/users/mod.rs @@ -67,11 +67,22 @@ pub async fn list(org_id: web::Path) -> Result { pub async fn save( org_id: web::Path, user: web::Json, + user_email: UserEmail, ) -> Result { let org_id = org_id.into_inner(); + let initiator_id = user_email.user_id; let mut user = user.into_inner(); user.email = user.email.trim().to_string(); - users::post_user(&org_id, user).await + if user.role.eq(&meta::user::UserRole::Root) { + return Ok( + HttpResponse::BadRequest().json(meta::http::HttpResponse::error( + http::StatusCode::BAD_REQUEST.into(), + "Not allowed".to_string(), + )), + ); + } + + users::post_user(&org_id, user, &initiator_id).await } /// UpdateUser @@ -160,9 +171,13 @@ pub async fn add_user_to_org( ) )] #[delete("/{org_id}/users/{email_id}")] -pub async fn delete(path: web::Path<(String, String)>) -> Result { +pub async fn delete( + path: web::Path<(String, String)>, + user_email: UserEmail, +) -> Result { let (org_id, email_id) = path.into_inner(); - users::remove_user_from_org(&org_id, &email_id).await + let initiator_id = user_email.user_id; + users::remove_user_from_org(&org_id, &email_id, &initiator_id).await } /// AuthenticateUser diff --git a/src/job/mod.rs b/src/job/mod.rs index 801390d2e0..47f23ca691 100644 --- a/src/job/mod.rs +++ b/src/job/mod.rs @@ -55,7 +55,7 @@ pub async fn init() -> Result<(), anyhow::Error> { "Please set root user email-id & password using ZO_ROOT_USER_EMAIL & ZO_ROOT_USER_PASSWORD environment variables. This can also indicate an invalid email ID. Email ID must comply with ([a-z0-9_+]([a-z0-9_+.-]*[a-z0-9_+])?)@([a-z0-9]+([\\-\\.]{{1}}[a-z0-9]+)*\\.[a-z]{{2,6}})" ); } - let _ = users::post_user( + let _ = users::create_root_user( DEFAULT_ORG, UserRequest { email: CONFIG.auth.root_user_email.clone(), diff --git a/src/service/db/user.rs b/src/service/db/user.rs index f98ca89f76..30fe4a226d 100644 --- a/src/service/db/user.rs +++ b/src/service/db/user.rs @@ -215,7 +215,7 @@ pub async fn cache() -> Result<(), anyhow::Error> { pub async fn root_user_exists() -> bool { let db = infra_db::get_db().await; let key = "/user/"; - let mut ret = db.list_values(key).await.unwrap(); + let mut ret = db.list_values(key).await.unwrap_or_default(); ret.retain(|item| { let user: DBUser = json::from_slice(item).unwrap(); user.organizations diff --git a/src/service/organization.rs b/src/service/organization.rs index 454db7991a..7450fe7699 100644 --- a/src/service/organization.rs +++ b/src/service/organization.rs @@ -181,6 +181,7 @@ mod tests { last_name: "".to_owned(), is_external: false, }, + user_id, ) .await; assert!(resp.is_ok()); diff --git a/src/service/users.rs b/src/service/users.rs index 712b76ea82..6ca350f8c0 100644 --- a/src/service/users.rs +++ b/src/service/users.rs @@ -39,34 +39,52 @@ use crate::{ service::db, }; -pub async fn post_user(org_id: &str, usr_req: UserRequest) -> Result { - let existing_user = if is_root_user(&usr_req.email) { - db::user::get(None, &usr_req.email).await +pub async fn post_user( + org_id: &str, + usr_req: UserRequest, + initiator_id: &str, +) -> Result { + if is_root_user(initiator_id) + || db::user::get(Some(org_id), initiator_id) + .await + .unwrap() + .unwrap() + .role + .eq(&UserRole::Admin) + { + let existing_user = if is_root_user(&usr_req.email) { + db::user::get(None, &usr_req.email).await + } else { + db::user::get(Some(org_id), &usr_req.email).await + }; + if existing_user.is_err() { + let salt = Uuid::new_v4().to_string(); + let password = get_hash(&usr_req.password, &salt); + let token = generate_random_string(16); + let rum_token = format!("rum{}", generate_random_string(16)); + let user = usr_req.to_new_dbuser( + password, + salt, + org_id.replace(' ', "_"), + token, + rum_token, + usr_req.is_external, + ); + db::user::set(user).await.unwrap(); + Ok(HttpResponse::Ok().json(MetaHttpResponse::message( + http::StatusCode::OK.into(), + "User saved successfully".to_string(), + ))) + } else { + Ok(HttpResponse::BadRequest().json(MetaHttpResponse::message( + http::StatusCode::BAD_REQUEST.into(), + "Unable to process your request".to_string(), + ))) + } } else { - db::user::get(Some(org_id), &usr_req.email).await - }; - if existing_user.is_err() { - let salt = Uuid::new_v4().to_string(); - let password = get_hash(&usr_req.password, &salt); - let token = generate_random_string(16); - let rum_token = format!("rum{}", generate_random_string(16)); - let user = usr_req.to_new_dbuser( - password, - salt, - org_id.replace(' ', "_"), - token, - rum_token, - usr_req.is_external, - ); - db::user::set(user).await.unwrap(); - Ok(HttpResponse::Ok().json(MetaHttpResponse::message( - http::StatusCode::OK.into(), - "User saved successfully".to_string(), - ))) - } else { - Ok(HttpResponse::BadRequest().json(MetaHttpResponse::message( - http::StatusCode::BAD_REQUEST.into(), - "Unable to process your request".to_string(), + Ok(HttpResponse::Unauthorized().json(MetaHttpResponse::error( + StatusCode::UNAUTHORIZED.into(), + "Not Allowed".to_string(), ))) } } @@ -365,38 +383,57 @@ pub async fn list_users(org_id: &str) -> Result { Ok(HttpResponse::Ok().json(UserList { data: user_list })) } -pub async fn remove_user_from_org(org_id: &str, email_id: &str) -> Result { - let ret_user = db::user::get_db_user(email_id).await; - match ret_user { - Ok(mut user) => { - if !user.organizations.is_empty() { - let mut orgs = user.clone().organizations; - if orgs.len() == 1 { - let _ = db::user::delete(email_id).await; - } else { - orgs.retain(|x| !x.name.eq(&org_id.to_string())); - user.organizations = orgs; - let resp = db::user::set(user).await; - // special case as we cache flattened user struct - if resp.is_ok() { - USERS.remove(&format!("{org_id}/{email_id}")); +pub async fn remove_user_from_org( + org_id: &str, + email_id: &str, + initiator_id: &str, +) -> Result { + let initiating_user = if is_root_user(initiator_id) { + ROOT_USER.get("root").unwrap().clone() + } else { + db::user::get(Some(org_id), initiator_id) + .await + .unwrap() + .unwrap() + }; + if initiating_user.role.eq(&UserRole::Root) || initiating_user.role.eq(&UserRole::Admin) { + let ret_user = db::user::get_db_user(email_id).await; + match ret_user { + Ok(mut user) => { + if !user.organizations.is_empty() { + let mut orgs = user.clone().organizations; + if orgs.len() == 1 { + let _ = db::user::delete(email_id).await; + } else { + orgs.retain(|x| !x.name.eq(&org_id.to_string())); + user.organizations = orgs; + let resp = db::user::set(user).await; + // special case as we cache flattened user struct + if resp.is_ok() { + USERS.remove(&format!("{org_id}/{email_id}")); + } } + Ok(HttpResponse::Ok().json(MetaHttpResponse::message( + http::StatusCode::OK.into(), + "User removed from organization".to_string(), + ))) + } else { + Ok(HttpResponse::NotFound().json(MetaHttpResponse::error( + StatusCode::NOT_FOUND.into(), + "User for the organization not found".to_string(), + ))) } - Ok(HttpResponse::Ok().json(MetaHttpResponse::message( - http::StatusCode::OK.into(), - "User removed from organization".to_string(), - ))) - } else { - Ok(HttpResponse::NotFound().json(MetaHttpResponse::error( - StatusCode::NOT_FOUND.into(), - "User for the organization not found".to_string(), - ))) } + Err(_) => Ok(HttpResponse::NotFound().json(MetaHttpResponse::error( + StatusCode::NOT_FOUND.into(), + "User for the organization not found".to_string(), + ))), } - Err(_) => Ok(HttpResponse::NotFound().json(MetaHttpResponse::error( - StatusCode::NOT_FOUND.into(), - "User for the organization not found".to_string(), - ))), + } else { + Ok(HttpResponse::Unauthorized().json(MetaHttpResponse::error( + StatusCode::UNAUTHORIZED.into(), + "Not Allowed".to_string(), + ))) } } @@ -437,6 +474,23 @@ pub fn is_user_from_org(orgs: Vec, org_id: &str) -> (bool, UserOrg) { } } +pub(crate) async fn create_root_user(org_id: &str, usr_req: UserRequest) -> Result<(), Error> { + let salt = Uuid::new_v4().to_string(); + let password = get_hash(&usr_req.password, &salt); + let token = generate_random_string(16); + let rum_token = format!("rum{}", generate_random_string(16)); + let user = usr_req.to_new_dbuser( + password, + salt, + org_id.replace(' ', "_"), + token, + rum_token, + usr_req.is_external, + ); + db::user::set(user).await.unwrap(); + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -481,52 +535,29 @@ mod tests { #[actix_web::test] async fn test_post_user() { infra_db::create_table().await.unwrap(); + set_up().await; + let resp = post_user( "dummy", UserRequest { - email: "admin@zo.dev".to_string(), + email: "user@zo.dev".to_string(), password: "pass#123".to_string(), role: crate::common::meta::user::UserRole::Admin, - first_name: "admin".to_owned(), + first_name: "user".to_owned(), last_name: "".to_owned(), is_external: false, }, + "admin@zo.dev", ) .await; + println!("resp: {:?}", resp); assert!(resp.is_ok()); } #[actix_web::test] async fn test_user() { infra_db::create_table().await.unwrap(); - let _ = post_user( - "test_org", - UserRequest { - email: "user2@example.com".to_string(), - password: "pass#123".to_string(), - role: crate::common::meta::user::UserRole::Admin, - first_name: "admin".to_owned(), - last_name: "".to_owned(), - is_external: false, - }, - ) - .await; - - USERS.insert( - "dummy/admin@zo.dev".to_string(), - User { - email: "admin@zo.dev".to_string(), - password: "pass#123".to_string(), - role: crate::common::meta::user::UserRole::Admin, - salt: String::new(), - token: "token".to_string(), - rum_token: Some("rum_token".to_string()), - first_name: "admin".to_owned(), - last_name: "".to_owned(), - org: "dummy".to_string(), - is_external: false, - }, - ); + set_up().await; let resp = update_user( "dummy", @@ -576,7 +607,7 @@ mod tests { assert!(resp.is_ok()); - let resp = remove_user_from_org("dummy", "user2@example.com").await; + let resp = remove_user_from_org("dummy", "user2@example.com", "admin@zo.dev").await; assert!(resp.is_ok()); }