From bcdace5459800a7d8b26c521e22c0c55ebc988fc Mon Sep 17 00:00:00 2001 From: Subhra264 Date: Mon, 29 Sep 2025 11:53:15 +0530 Subject: [PATCH] fix: Root user should not be added to any organization (#8632) ### **User description** Root user by default can access all the organizations and does not need to be added to any organizations through api. ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Prevent adding root user to orgs - Return Forbidden for unauthorized add attempts - Expand integration tests for user/org flows - Validate user listing includes expected emails ___ ### Diagram Walkthrough ```mermaid flowchart LR addUser["add_user_to_org handler"] -- "user is root" --> rejectRoot["400 Bad Request: root no org needed"] addUser -- "not root & unauthorized" --> forbid["403 Forbidden"] tests["Integration tests"] -- "root add attempt" --> expect400["expect failure"] tests -- "create non-root & add to org" --> expect200["expect success"] tests -- "list users" --> assertUsers["assert expected emails present"] ```

File Walkthrough
Relevant files
Bug fix
users.rs
Block root user org add; use 403 for denial                           
src/service/users.rs
  • Short-circuit if existing_user.is_root, return 400.
  • Change unauthorized response from 401 to 403.
  • Add explanatory comments for root access.
 +13/-2   
Tests
integration_test.rs
Extend integration tests for user/org behaviors                   
tests/integration_test.rs
  • Import UserList and validate list users content.
  • Add e2e_add_root_user_to_org expecting failure.
  • Add e2e_add_user_to_org to create and add admin user.
  • Update test sequence to include new cases.
 +79/-2   
___ --- ...0001_update_enrichment_table_data_size.rs} | 0 src/infra/src/table/migration/mod.rs | 4 +- src/service/users.rs | 15 +++- tests/integration_test.rs | 84 ++++++++++++++++++- 4 files changed, 96 insertions(+), 7 deletions(-) rename src/infra/src/table/migration/{m20251023_000001_update_enrichment_table_data_size.rs => m20250923_000001_update_enrichment_table_data_size.rs} (100%) diff --git a/src/infra/src/table/migration/m20251023_000001_update_enrichment_table_data_size.rs b/src/infra/src/table/migration/m20250923_000001_update_enrichment_table_data_size.rs similarity index 100% rename from src/infra/src/table/migration/m20251023_000001_update_enrichment_table_data_size.rs rename to src/infra/src/table/migration/m20250923_000001_update_enrichment_table_data_size.rs diff --git a/src/infra/src/table/migration/mod.rs b/src/infra/src/table/migration/mod.rs index ed4df256e5..715d263cae 100644 --- a/src/infra/src/table/migration/mod.rs +++ b/src/infra/src/table/migration/mod.rs @@ -64,7 +64,7 @@ mod m20250716_000001_create_enrichment_table; mod m20250731_000001_create_compactor_manual_jobs; mod m20250801_000001_create_system_prompts_table; mod m20250822_093713_add_updated_at_for_file_list_table; -mod m20251023_000001_update_enrichment_table_data_size; +mod m20250923_000001_update_enrichment_table_data_size; pub struct Migrator; @@ -121,7 +121,7 @@ impl MigratorTrait for Migrator { Box::new(m20250731_000001_create_compactor_manual_jobs::Migration), Box::new(m20250801_000001_create_system_prompts_table::Migration), Box::new(m20250822_093713_add_updated_at_for_file_list_table::Migration), - Box::new(m20251023_000001_update_enrichment_table_data_size::Migration), + Box::new(m20250923_000001_update_enrichment_table_data_size::Migration), ] } } diff --git a/src/service/users.rs b/src/service/users.rs index b0a6d1e2ff..95a4febb6e 100644 --- a/src/service/users.rs +++ b/src/service/users.rs @@ -636,7 +636,16 @@ pub async fn add_user_to_org( let email = email.trim().to_lowercase(); let existing_user = db::user::get_user_record(&email).await; let root_user = ROOT_USER.clone(); - if existing_user.is_ok() { + if let Ok(existing_user) = existing_user { + // If the user is root, we don't need to add to the org, as root user + // already has access to all organizations. + if existing_user.is_root { + return Ok(HttpResponse::Conflict().json(MetaHttpResponse::message( + http::StatusCode::CONFLICT, + "User is root user, no need to add to organization.", + ))); + } + let initiating_user = if is_root_user(initiator_id) { let local_org = org_id.replace(' ', "_"); // If the org does not exist, create it @@ -718,8 +727,8 @@ pub async fn add_user_to_org( "User added to org successfully", ))) } else { - Ok(HttpResponse::Unauthorized().json(MetaHttpResponse::error( - http::StatusCode::UNAUTHORIZED, + Ok(HttpResponse::Forbidden().json(MetaHttpResponse::error( + http::StatusCode::FORBIDDEN, "Not Allowed", ))) } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 8df8388470..f47feb77fb 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -41,7 +41,10 @@ mod tests { }; use infra::schema::{STREAM_SCHEMAS, STREAM_SCHEMAS_LATEST, STREAM_SETTINGS}; use openobserve::{ - common::{infra::config::ENRICHMENT_TABLES, meta::ingestion::IngestionResponse}, + common::{ + infra::config::ENRICHMENT_TABLES, + meta::{ingestion::IngestionResponse, user::UserList}, + }, handler::{ grpc::{auth::check_auth, flight::FlightServiceImpl}, http::{ @@ -186,6 +189,7 @@ mod tests { e2e_update_user().await; e2e_update_user_with_empty().await; e2e_add_user_to_org().await; + e2e_add_root_user_to_org().await; e2e_list_users().await; e2e_get_organizations().await; e2e_get_user_passcode().await; @@ -787,6 +791,24 @@ mod tests { .to_request(); let resp = test::call_service(&app, req).await; assert!(resp.status().is_success()); + let body = test::read_body(resp).await; + let body_str = String::from_utf8_lossy(&body); + println!("list users resp: {body_str:?}"); + // deserialize the body into UserList + let user_list: UserList = serde_json::from_str(&body_str).unwrap(); + assert!(!user_list.data.is_empty()); + assert!( + user_list + .data + .iter() + .any(|user| user.email == "admin@example.com") + ); + assert!( + user_list + .data + .iter() + .any(|user| user.email == "nonadmin@example.com") + ); } async fn e2e_get_organizations() { @@ -986,7 +1008,7 @@ mod tests { assert!(!resp.status().is_success()); } - async fn e2e_add_user_to_org() { + async fn e2e_add_root_user_to_org() { let auth = setup(); let body_str = r#"{ "role":"member" @@ -1008,6 +1030,64 @@ mod tests { .set_payload(body_str) .to_request(); let resp = test::call_service(&app, req).await; + // It should fail as root user cannot be added to an organization + assert!(!resp.status().is_success()); + } + + async fn e2e_add_user_to_org() { + let auth = setup(); + let body_str = r#"{ + "role":"admin" + }"#; + let app = test::init_service( + App::new() + .app_data(web::JsonConfig::default().limit(get_config().limit.req_json_limit)) + .app_data(web::PayloadConfig::new( + get_config().limit.req_payload_limit, + )) + .configure(get_service_routes) + .configure(get_basic_routes), + ) + .await; + let req = test::TestRequest::post() + .uri(&format!("/api/{}/users/{}", "e2e", "admin@example.com")) + .insert_header(ContentType::json()) + .append_header(auth) + .set_payload(body_str) + .to_request(); + let resp = test::call_service(&app, req).await; + // Should fail as the user still does not exist + assert!(resp.status().is_client_error()); + + // Add the user + let body_str = r#"{ + "email": "admin@example.com", + "password": "Abcd12345", + "role": "admin" + }"#; + + let req = test::TestRequest::post() + .uri(&format!("/api/{}/users", "e2e")) + .insert_header(ContentType::json()) + .append_header(auth) + .set_payload(body_str) + .to_request(); + let resp = test::call_service(&app, req).await; + assert!(resp.status().is_success()); + + // Role in the default organization + let body_str = r#"{ + "role":"admin" + }"#; + + // Add the user to the default organization with role admin + let req = test::TestRequest::post() + .uri(&format!("/api/{}/users/{}", "default", "admin@example.com")) + .insert_header(ContentType::json()) + .append_header(auth) + .set_payload(body_str) + .to_request(); + let resp = test::call_service(&app, req).await; assert!(resp.status().is_success()); }