mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 08:09:27 +00:00
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"]
```
<details> <summary><h3> File Walkthrough</h3></summary>
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
<td>
<details>
<summary><strong>users.rs</strong><dd><code>Block root user org add; use
403 for denial</code>
</dd></summary>
<hr>
src/service/users.rs
<ul><li>Short-circuit if <code>existing_user.is_root</code>, return
400.<br> <li> Change unauthorized response from 401 to 403.<br> <li> Add
explanatory comments for root access.</ul>
</details>
</td>
<td><a
href="https://github.com/openobserve/openobserve/pull/8632/files#diff-98da2588a28d62bb2e301d989836125f89f89b76272cd7f75e08ec4deeea5f38">+13/-2</a>
</td>
</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
<td>
<details>
<summary><strong>integration_test.rs</strong><dd><code>Extend
integration tests for user/org behaviors</code>
</dd></summary>
<hr>
tests/integration_test.rs
<ul><li>Import <code>UserList</code> and validate list users
content.<br> <li> Add <code>e2e_add_root_user_to_org</code> expecting
failure.<br> <li> Add <code>e2e_add_user_to_org</code> to create and add
admin user.<br> <li> Update test sequence to include new cases.</ul>
</details>
</td>
<td><a
href="https://github.com/openobserve/openobserve/pull/8632/files#diff-7ff815b68943f08bef79f083c09e55ca8c18db8e25033034b12fa31f1d43dab4">+79/-2</a>
</td>
</tr>
</table></td></tr></tr></tbody></table>
</details>
___
This commit is contained in:
@@ -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),
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
+12
-3
@@ -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",
|
||||
)))
|
||||
}
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user