From d95b279b9569037f4ce40e6d8625f22e499a2c76 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Wed, 7 Oct 2026 15:45:49 +0530 Subject: [PATCH 01/10] feat: accept caller-provided API keys --- src/apikeys.rs | 26 +++++++++++++++++++++++++- src/handlers/http/apikeys.rs | 16 ++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index d139b351a..b1db6879f 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -30,12 +30,27 @@ use serde::Deserialize; /// Request body for creating a new API key. `roles` is a set of role names /// that must already exist in the tenant; permissions for the backing user /// are derived from these roles (same flow as native/OAuth users). +/// `providedApiKey` lets a caller register their own key; omitting it preserves +/// the existing server-generated key behavior. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CreateApiKeyRequest { pub key_name: String, #[serde(default)] pub roles: HashSet, + pub provided_api_key: Option, +} + +/// Reject short or malformed caller-supplied credentials before persisting them. +pub fn validate_provided_api_key(value: &str) -> Result<(), ApiKeyError> { + if !(32..=256).contains(&value.len()) + || !value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') + { + return Err(ApiKeyError::InvalidProvidedApiKey); + } + Ok(()) } #[derive(Debug, thiserror::Error)] @@ -46,6 +61,12 @@ pub enum ApiKeyError { #[error("Duplicate key name: {0}")] DuplicateKeyName(String), + #[error("API key value is already registered")] + DuplicateApiKey, + + #[error("providedApiKey must be 32-256 ASCII letters, digits, hyphens, or underscores")] + InvalidProvidedApiKey, + #[error("Unauthorized: {0}")] Unauthorized(String), @@ -66,7 +87,10 @@ impl actix_web::ResponseError for ApiKeyError { fn status_code(&self) -> actix_web::http::StatusCode { match self { ApiKeyError::KeyNotFound(_) => actix_web::http::StatusCode::NOT_FOUND, - ApiKeyError::DuplicateKeyName(_) => actix_web::http::StatusCode::CONFLICT, + ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { + actix_web::http::StatusCode::CONFLICT + } + ApiKeyError::InvalidProvidedApiKey => actix_web::http::StatusCode::BAD_REQUEST, ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, ApiKeyError::RevocationSyncFailed(_) => { actix_web::http::StatusCode::SERVICE_UNAVAILABLE diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 9b226db7f..848f8c3be 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Deserializer, Serialize, de}; use ulid::Ulid; use crate::{ - apikeys::{ApiKeyError, CreateApiKeyRequest}, + apikeys::{ApiKeyError, CreateApiKeyRequest, validate_provided_api_key}, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, rbac::{RBACError, UPDATE_LOCK}, @@ -227,7 +227,19 @@ pub async fn create_api_key( } let key_id = Ulid::new(); - let api_key_value = generate_api_key_value(); + let api_key_value = if let Some(provided_api_key) = body.provided_api_key { + validate_provided_api_key(&provided_api_key)?; + provided_api_key + } else { + generate_api_key_value() + }; + if existing + .iter() + .filter_map(|user: &User| user.as_api_key()) + .any(|key| key.api_key == api_key_value) + { + return Err(ApiKeyError::DuplicateApiKey); + } let user = User::new_api_key( key_id, api_key_value, From 511cf20a8c15d7410b6d37596bdeaade8b5e86f7 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Wed, 21 Oct 2026 15:15:20 +0530 Subject: [PATCH 02/10] feat: add ingest-only mode for caller-provided API keys --- src/apikeys.rs | 33 +++++++++++++++++++-------------- src/handlers/http/apikeys.rs | 27 ++++++++++++++++++--------- src/rbac/mod.rs | 21 +++++++++++++++++++++ 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index b1db6879f..36be6c9e3 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -30,25 +30,25 @@ use serde::Deserialize; /// Request body for creating a new API key. `roles` is a set of role names /// that must already exist in the tenant; permissions for the backing user /// are derived from these roles (same flow as native/OAuth users). -/// `providedApiKey` lets a caller register their own key; omitting it preserves +/// `apiKey` lets a caller register their own key; omitting it preserves /// the existing server-generated key behavior. #[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct CreateApiKeyRequest { pub key_name: String, #[serde(default)] pub roles: HashSet, - pub provided_api_key: Option, + pub api_key: Option, + /// Helm uses this mode to request the server-managed global ingest role. + #[serde(default)] + pub ingestor_only: bool, } -/// Reject short or malformed caller-supplied credentials before persisting them. -pub fn validate_provided_api_key(value: &str) -> Result<(), ApiKeyError> { - if !(32..=256).contains(&value.len()) - || !value - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') - { - return Err(ApiKeyError::InvalidProvidedApiKey); +/// Require a hyphenated UUID v4, allowing either letter case. +pub fn validate_api_key(value: &str) -> Result<(), ApiKeyError> { + let uuid = uuid::Uuid::parse_str(value).map_err(|_| ApiKeyError::InvalidApiKey)?; + if value.len() != 36 || uuid.get_version_num() != 4 { + return Err(ApiKeyError::InvalidApiKey); } Ok(()) } @@ -64,8 +64,11 @@ pub enum ApiKeyError { #[error("API key value is already registered")] DuplicateApiKey, - #[error("providedApiKey must be 32-256 ASCII letters, digits, hyphens, or underscores")] - InvalidProvidedApiKey, + #[error("apiKey must be a UUID v4 in 8-4-4-4-12 format")] + InvalidApiKey, + + #[error("ingestorOnly cannot be combined with roles")] + IngestorOnlyWithRoles, #[error("Unauthorized: {0}")] Unauthorized(String), @@ -90,7 +93,9 @@ impl actix_web::ResponseError for ApiKeyError { ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { actix_web::http::StatusCode::CONFLICT } - ApiKeyError::InvalidProvidedApiKey => actix_web::http::StatusCode::BAD_REQUEST, + ApiKeyError::InvalidApiKey | ApiKeyError::IngestorOnlyWithRoles => { + actix_web::http::StatusCode::BAD_REQUEST + } ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, ApiKeyError::RevocationSyncFailed(_) => { actix_web::http::StatusCode::SERVICE_UNAVAILABLE diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 848f8c3be..26966abc8 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -6,14 +6,14 @@ use serde::{Deserialize, Deserializer, Serialize, de}; use ulid::Ulid; use crate::{ - apikeys::{ApiKeyError, CreateApiKeyRequest, validate_provided_api_key}, + apikeys::{ApiKeyError, CreateApiKeyRequest, validate_api_key as validate_api_key_value}, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, rbac::{RBACError, UPDATE_LOCK}, }, parseable::DEFAULT_TENANT, rbac::{ - Users, + GLOBAL_INGESTOR_ROLE, Users, map::{roles, users}, role::model::{DefaultPrivilege, Role}, user::{User, UserType}, @@ -212,9 +212,18 @@ pub async fn create_api_key( let guard = UPDATE_LOCK.lock().await; - // Reject any role names that don't exist in this tenant while holding the - // update lock so a concurrent role update cannot invalidate the check. - validate_roles(&body.roles, &tenant_id)?; + // Helm's ingest-only mode uses an internal role. Explicit roles retain + // the existing UI/API behavior and cannot be mixed with this mode. + let assigned_roles = if body.ingestor_only { + if !body.roles.is_empty() { + return Err(ApiKeyError::IngestorOnlyWithRoles); + } + HashSet::from([GLOBAL_INGESTOR_ROLE.to_string()]) + } else { + // Hold the update lock while checking user-defined roles. + validate_roles(&body.roles, &tenant_id)?; + body.roles + }; // Duplicate key-name detection inside the same tenant. let existing = collect_tenant_api_keys(&tenant_id); @@ -227,9 +236,9 @@ pub async fn create_api_key( } let key_id = Ulid::new(); - let api_key_value = if let Some(provided_api_key) = body.provided_api_key { - validate_provided_api_key(&provided_api_key)?; - provided_api_key + let api_key_value = if let Some(api_key) = body.api_key { + validate_api_key_value(&api_key)?; + api_key } else { generate_api_key_value() }; @@ -244,7 +253,7 @@ pub async fn create_api_key( key_id, api_key_value, body.key_name, - body.roles, + assigned_roles, created_by, tenant_id.clone(), ); diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index 0e7dda57d..6898a86fc 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -45,6 +45,9 @@ use self::role::{Permission, RoleBuilder}; use self::user::UserType; pub const EXPIRY_DURATION: Duration = Duration::hours(1); +/// Reserved, server-managed role for Helm's ingest-only API keys. +/// This name is outside the user-defined role-name grammar. +pub const GLOBAL_INGESTOR_ROLE: &str = "__global_ingestor__"; #[derive(PartialEq, Debug)] pub enum Response { @@ -397,6 +400,10 @@ pub struct UsersPrism { pub fn roles_to_permission(roles: Vec, tenant_id: &str) -> Vec { let mut perms = HashSet::new(); for role in &roles { + if role == GLOBAL_INGESTOR_ROLE { + perms.insert(Permission::Resource(Action::Ingest, None)); + continue; + } let role_map = &map::roles(); if let Some(roles) = role_map.get(tenant_id) && let Some(privilege_list) = roles.get(role) @@ -410,3 +417,17 @@ pub fn roles_to_permission(roles: Vec, tenant_id: &str) -> Vec Date: Wed, 21 Oct 2026 21:31:32 +0530 Subject: [PATCH 03/10] feat: support typed API keys with global reader access --- src/apikeys.rs | 46 +++++++++++++++++++++++++++++++----- src/handlers/http/apikeys.rs | 33 ++++++++++++++++---------- src/rbac/mod.rs | 28 ++++++++++++++++++++-- 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index 36be6c9e3..cc0030b74 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -32,6 +32,13 @@ use serde::Deserialize; /// are derived from these roles (same flow as native/OAuth users). /// `apiKey` lets a caller register their own key; omitting it preserves /// the existing server-generated key behavior. +#[derive(Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum ApiKeyType { + Ingestor, + Query, +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct CreateApiKeyRequest { @@ -39,9 +46,8 @@ pub struct CreateApiKeyRequest { #[serde(default)] pub roles: HashSet, pub api_key: Option, - /// Helm uses this mode to request the server-managed global ingest role. - #[serde(default)] - pub ingestor_only: bool, + /// Omit for the existing UI/API flow that assigns explicit roles. + pub api_key_type: Option, } /// Require a hyphenated UUID v4, allowing either letter case. @@ -67,8 +73,8 @@ pub enum ApiKeyError { #[error("apiKey must be a UUID v4 in 8-4-4-4-12 format")] InvalidApiKey, - #[error("ingestorOnly cannot be combined with roles")] - IngestorOnlyWithRoles, + #[error("apiKeyType cannot be combined with roles")] + ApiKeyTypeWithRoles, #[error("Unauthorized: {0}")] Unauthorized(String), @@ -93,7 +99,7 @@ impl actix_web::ResponseError for ApiKeyError { ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { actix_web::http::StatusCode::CONFLICT } - ApiKeyError::InvalidApiKey | ApiKeyError::IngestorOnlyWithRoles => { + ApiKeyError::InvalidApiKey | ApiKeyError::ApiKeyTypeWithRoles => { actix_web::http::StatusCode::BAD_REQUEST } ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, @@ -107,3 +113,31 @@ impl actix_web::ResponseError for ApiKeyError { } } } + +#[cfg(test)] +mod request_tests { + use super::{ApiKeyType, CreateApiKeyRequest}; + + #[test] + fn typed_keys_and_existing_role_requests_deserialize() { + let ingestor: CreateApiKeyRequest = + serde_json::from_str(r#"{"keyName":"ingest-key","apiKeyType":"ingestor"}"#).unwrap(); + assert_eq!(ingestor.api_key_type, Some(ApiKeyType::Ingestor)); + + let query: CreateApiKeyRequest = + serde_json::from_str(r#"{"keyName":"query-key","apiKeyType":"query"}"#).unwrap(); + assert_eq!(query.api_key_type, Some(ApiKeyType::Query)); + + let ui: CreateApiKeyRequest = + serde_json::from_str(r#"{"keyName":"ui-key","roles":["reader-role"]}"#).unwrap(); + assert_eq!(ui.api_key_type, None); + assert!(ui.roles.contains("reader-role")); + + assert!( + serde_json::from_str::( + r#"{"keyName":"old-key","ingestorOnly":true}"# + ) + .is_err() + ); + } +} diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 26966abc8..7cb0d74fc 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -6,14 +6,16 @@ use serde::{Deserialize, Deserializer, Serialize, de}; use ulid::Ulid; use crate::{ - apikeys::{ApiKeyError, CreateApiKeyRequest, validate_api_key as validate_api_key_value}, + apikeys::{ + ApiKeyError, ApiKeyType, CreateApiKeyRequest, validate_api_key as validate_api_key_value, + }, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, rbac::{RBACError, UPDATE_LOCK}, }, parseable::DEFAULT_TENANT, rbac::{ - GLOBAL_INGESTOR_ROLE, Users, + GLOBAL_INGESTOR_ROLE, GLOBAL_QUERIER_ROLE, Users, map::{roles, users}, role::model::{DefaultPrivilege, Role}, user::{User, UserType}, @@ -212,17 +214,24 @@ pub async fn create_api_key( let guard = UPDATE_LOCK.lock().await; - // Helm's ingest-only mode uses an internal role. Explicit roles retain - // the existing UI/API behavior and cannot be mixed with this mode. - let assigned_roles = if body.ingestor_only { - if !body.roles.is_empty() { - return Err(ApiKeyError::IngestorOnlyWithRoles); + // Typed keys get server-managed roles; the existing UI/API flow keeps + // assigning explicit roles when apiKeyType is omitted. + let assigned_roles = match body.api_key_type { + Some(api_key_type) => { + if !body.roles.is_empty() { + return Err(ApiKeyError::ApiKeyTypeWithRoles); + } + let role = match api_key_type { + ApiKeyType::Ingestor => GLOBAL_INGESTOR_ROLE, + ApiKeyType::Query => GLOBAL_QUERIER_ROLE, + }; + HashSet::from([role.to_string()]) + } + None => { + // Hold the update lock while checking user-defined roles. + validate_roles(&body.roles, &tenant_id)?; + body.roles } - HashSet::from([GLOBAL_INGESTOR_ROLE.to_string()]) - } else { - // Hold the update lock while checking user-defined roles. - validate_roles(&body.roles, &tenant_id)?; - body.roles }; // Duplicate key-name detection inside the same tenant. diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index 6898a86fc..275360a03 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -41,13 +41,15 @@ use crate::rbac::user::User; use crate::utils::get_tenant_id_from_key; use self::map::SessionKey; -use self::role::{Permission, RoleBuilder}; +use self::role::{ParseableResourceType, Permission, RoleBuilder, model::DefaultPrivilege}; use self::user::UserType; pub const EXPIRY_DURATION: Duration = Duration::hours(1); /// Reserved, server-managed role for Helm's ingest-only API keys. /// This name is outside the user-defined role-name grammar. pub const GLOBAL_INGESTOR_ROLE: &str = "__global_ingestor__"; +/// Reserved, server-managed role with the existing reader privilege on all datasets. +pub const GLOBAL_QUERIER_ROLE: &str = "__global_querier__"; #[derive(PartialEq, Debug)] pub enum Response { @@ -404,6 +406,15 @@ pub fn roles_to_permission(roles: Vec, tenant_id: &str) -> Vec, tenant_id: &str) -> Vec Date: Wed, 21 Oct 2026 22:16:29 +0530 Subject: [PATCH 04/10] chore: remove API key tests --- src/apikeys.rs | 28 ---------------------------- src/rbac/mod.rs | 27 --------------------------- 2 files changed, 55 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index cc0030b74..5a0682135 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -113,31 +113,3 @@ impl actix_web::ResponseError for ApiKeyError { } } } - -#[cfg(test)] -mod request_tests { - use super::{ApiKeyType, CreateApiKeyRequest}; - - #[test] - fn typed_keys_and_existing_role_requests_deserialize() { - let ingestor: CreateApiKeyRequest = - serde_json::from_str(r#"{"keyName":"ingest-key","apiKeyType":"ingestor"}"#).unwrap(); - assert_eq!(ingestor.api_key_type, Some(ApiKeyType::Ingestor)); - - let query: CreateApiKeyRequest = - serde_json::from_str(r#"{"keyName":"query-key","apiKeyType":"query"}"#).unwrap(); - assert_eq!(query.api_key_type, Some(ApiKeyType::Query)); - - let ui: CreateApiKeyRequest = - serde_json::from_str(r#"{"keyName":"ui-key","roles":["reader-role"]}"#).unwrap(); - assert_eq!(ui.api_key_type, None); - assert!(ui.roles.contains("reader-role")); - - assert!( - serde_json::from_str::( - r#"{"keyName":"old-key","ingestorOnly":true}"# - ) - .is_err() - ); - } -} diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index 275360a03..cee0da54b 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -428,30 +428,3 @@ pub fn roles_to_permission(roles: Vec, tenant_id: &str) -> Vec Date: Thu, 22 Oct 2026 09:29:23 +0530 Subject: [PATCH 05/10] fix: reject non-RFC UUID variants in provided API keys --- src/apikeys.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index 5a0682135..903647d4f 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -53,7 +53,10 @@ pub struct CreateApiKeyRequest { /// Require a hyphenated UUID v4, allowing either letter case. pub fn validate_api_key(value: &str) -> Result<(), ApiKeyError> { let uuid = uuid::Uuid::parse_str(value).map_err(|_| ApiKeyError::InvalidApiKey)?; - if value.len() != 36 || uuid.get_version_num() != 4 { + if value.len() != 36 + || uuid.get_version_num() != 4 + || uuid.get_variant() != uuid::Variant::RFC4122 + { return Err(ApiKeyError::InvalidApiKey); } Ok(()) From b7848d0589e78ea706926503b677e556883ef825 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Fri, 23 Oct 2026 12:01:41 +0530 Subject: [PATCH 06/10] feat: support API key name and value updates --- src/apikeys.rs | 25 +++++++-- src/handlers/http/apikeys.rs | 88 ++++++++++++++++++++++++++++++- src/handlers/http/cluster/mod.rs | 6 +++ src/handlers/http/modal/server.rs | 5 ++ 4 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index 903647d4f..8dc1c1c6e 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -50,6 +50,13 @@ pub struct CreateApiKeyRequest { pub api_key_type: Option, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct PatchApiKeyRequest { + pub key_name: Option, + pub api_key: Option, +} + /// Require a hyphenated UUID v4, allowing either letter case. pub fn validate_api_key(value: &str) -> Result<(), ApiKeyError> { let uuid = uuid::Uuid::parse_str(value).map_err(|_| ApiKeyError::InvalidApiKey)?; @@ -73,6 +80,12 @@ pub enum ApiKeyError { #[error("API key value is already registered")] DuplicateApiKey, + #[error("API key update must include keyName or apiKey")] + EmptyApiKeyUpdate, + + #[error("keyName cannot be empty")] + InvalidApiKeyName, + #[error("apiKey must be a UUID v4 in 8-4-4-4-12 format")] InvalidApiKey, @@ -85,6 +98,9 @@ pub enum ApiKeyError { #[error("API key revocation could not be synced to all live nodes: {0}")] RevocationSyncFailed(String), + #[error("API key update could not be synced to all live nodes: {0}")] + UpdateSyncFailed(String), + #[error("{0}")] Storage(#[from] crate::storage::ObjectStorageError), @@ -102,11 +118,12 @@ impl actix_web::ResponseError for ApiKeyError { ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { actix_web::http::StatusCode::CONFLICT } - ApiKeyError::InvalidApiKey | ApiKeyError::ApiKeyTypeWithRoles => { - actix_web::http::StatusCode::BAD_REQUEST - } + ApiKeyError::InvalidApiKey + | ApiKeyError::ApiKeyTypeWithRoles + | ApiKeyError::EmptyApiKeyUpdate + | ApiKeyError::InvalidApiKeyName => actix_web::http::StatusCode::BAD_REQUEST, ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, - ApiKeyError::RevocationSyncFailed(_) => { + ApiKeyError::RevocationSyncFailed(_) | ApiKeyError::UpdateSyncFailed(_) => { actix_web::http::StatusCode::SERVICE_UNAVAILABLE } ApiKeyError::Rbac(err) => actix_web::ResponseError::status_code(err), diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 7cb0d74fc..e7224360b 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -7,7 +7,8 @@ use ulid::Ulid; use crate::{ apikeys::{ - ApiKeyError, ApiKeyType, CreateApiKeyRequest, validate_api_key as validate_api_key_value, + ApiKeyError, ApiKeyType, CreateApiKeyRequest, PatchApiKeyRequest, + validate_api_key as validate_api_key_value, }, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, @@ -298,6 +299,91 @@ pub async fn create_api_key( Ok(HttpResponse::Ok().json(response)) } +/// PATCH /api/prism/v1/apikeys/{key_id} +/// +/// Update a key's name or value without changing its identity or roles. +pub async fn patch_api_key( + req: HttpRequest, + path: web::Path, + web::Json(body): web::Json, +) -> Result { + let (caller_userid, tenant_id) = verify_admin(&req)?; + let key_id_str = path.into_inner(); + let key_id = + Ulid::from_string(&key_id_str).map_err(|_| ApiKeyError::KeyNotFound(key_id_str.clone()))?; + if body.key_name.is_none() && body.api_key.is_none() { + return Err(ApiKeyError::EmptyApiKeyUpdate); + } + + let guard = UPDATE_LOCK.lock().await; + let existing = collect_tenant_api_keys(&tenant_id); + let mut user = existing + .iter() + .find(|user| user.as_api_key().is_some_and(|key| key.key_id == key_id)) + .cloned() + .ok_or_else(|| ApiKeyError::KeyNotFound(key_id_str))?; + let current = user.as_api_key().expect("selected API key user"); + let key_name = body.key_name.unwrap_or_else(|| current.key_name.clone()); + let api_key_value = body.api_key.unwrap_or_else(|| current.api_key.clone()); + + if key_name.trim().is_empty() { + return Err(ApiKeyError::InvalidApiKeyName); + } + if api_key_value != current.api_key { + validate_api_key_value(&api_key_value)?; + } + if existing.iter().any(|other| { + other + .as_api_key() + .is_some_and(|key| key.key_id != key_id && key.key_name == key_name) + }) { + return Err(ApiKeyError::DuplicateKeyName(key_name.clone())); + } + if existing.iter().any(|other| { + other + .as_api_key() + .is_some_and(|key| key.key_id != key_id && key.api_key == api_key_value) + }) { + return Err(ApiKeyError::DuplicateApiKey); + } + + let changed = key_name != current.key_name || api_key_value != current.api_key; + if changed { + let key = match &mut user.ty { + UserType::ApiKey(key) => key, + _ => unreachable!("selected API key user"), + }; + key.key_name.clone_from(&key_name); + key.api_key.clone_from(&api_key_value); + key.modified_at = Utc::now(); + + let mut metadata = get_metadata(&tenant_id).await?; + let stored = metadata + .users + .iter_mut() + .find(|stored| stored.userid() == user.userid()) + .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))?; + *stored = user.clone(); + put_metadata(&metadata, &tenant_id).await?; + Users.put_user(user.clone()); + } + drop(guard); + + // A retry also re-sends an already applied update to any lagging nodes. + crate::handlers::http::cluster::sync_user_creation( + &req, + user.clone(), + &None, + &tenant_id, + &caller_userid, + ) + .await + .map_err(|error: RBACError| ApiKeyError::UpdateSyncFailed(error.to_string()))?; + + let response = ApiKeyResponse::try_from(&user)?; + Ok(HttpResponse::Ok().json(response)) +} + /// DELETE /api/prism/v1/apikeys/{key_id} /// /// Delete an API key by key_id. Only admins can delete keys. diff --git a/src/handlers/http/cluster/mod.rs b/src/handlers/http/cluster/mod.rs index 8ecc5538f..99d354ffc 100644 --- a/src/handlers/http/cluster/mod.rs +++ b/src/handlers/http/cluster/mod.rs @@ -618,11 +618,17 @@ pub async fn sync_user_creation( })?; if !res.status().is_success() { + let status = res.status(); error!( "failed to forward request to node: {}\nResponse Returned: {:?}", node.domain_name, res.text().await ); + return Err(RBACError::Anyhow(anyhow::anyhow!( + "user sync to {} failed with HTTP {}", + node.domain_name, + status + ))); } Ok(()) diff --git a/src/handlers/http/modal/server.rs b/src/handlers/http/modal/server.rs index 7b58e91da..6d1fee80a 100644 --- a/src/handlers/http/modal/server.rs +++ b/src/handlers/http/modal/server.rs @@ -270,6 +270,11 @@ impl Server { .service( resource("/{key_id}") .route(web::get().to(apikeys::get_api_key).authorize(Action::All)) + .route( + web::patch() + .to(apikeys::patch_api_key) + .authorize(Action::All), + ) .route( web::delete() .to(apikeys::delete_api_key) From dbfa506072164663df7ed3d3c4d7534e8a0264d7 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Fri, 23 Oct 2026 13:27:55 +0530 Subject: [PATCH 07/10] fix: use clone_from for API key metadata update --- src/handlers/http/apikeys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index e7224360b..ca7f5bfe6 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -363,7 +363,7 @@ pub async fn patch_api_key( .iter_mut() .find(|stored| stored.userid() == user.userid()) .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))?; - *stored = user.clone(); + stored.clone_from(&user); put_metadata(&metadata, &tenant_id).await?; Users.put_user(user.clone()); } From 510a667ba9eecdbfe586c365729e2595acd9a6eb Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Thu, 29 Oct 2026 18:54:25 +0530 Subject: [PATCH 08/10] Expose protected global API key roles to role APIs --- src/handlers/http/apikeys.rs | 12 ++----- .../http/modal/ingest/ingestor_role.rs | 11 +++++- src/handlers/http/modal/query/querier_rbac.rs | 18 +++------- src/handlers/http/modal/query/querier_role.rs | 7 ++++ src/handlers/http/rbac.rs | 20 +++-------- src/handlers/http/role.rs | 16 +++++++-- src/rbac/mod.rs | 34 +++++++++++++++++-- src/rbac/utils.rs | 16 ++------- 8 files changed, 77 insertions(+), 57 deletions(-) diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index ca7f5bfe6..b022cea06 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -19,6 +19,7 @@ use crate::{ GLOBAL_INGESTOR_ROLE, GLOBAL_QUERIER_ROLE, Users, map::{roles, users}, role::model::{DefaultPrivilege, Role}, + role_exists, user::{User, UserType}, }, utils::{get_user_and_tenant_from_request, is_admin}, @@ -186,15 +187,9 @@ fn validate_roles( tenant_id: &Option, ) -> Result<(), ApiKeyError> { let tenant = tenant_or_default(tenant_id); - let tenant_roles = roles(); let missing: Vec = role_names .iter() - .filter(|r| { - tenant_roles - .get(tenant) - .map(|m| !m.contains_key(*r)) - .unwrap_or(true) - }) + .filter(|r| !role_exists(tenant, r)) .cloned() .collect(); if missing.is_empty() { @@ -367,8 +362,6 @@ pub async fn patch_api_key( put_metadata(&metadata, &tenant_id).await?; Users.put_user(user.clone()); } - drop(guard); - // A retry also re-sends an already applied update to any lagging nodes. crate::handlers::http::cluster::sync_user_creation( &req, @@ -379,6 +372,7 @@ pub async fn patch_api_key( ) .await .map_err(|error: RBACError| ApiKeyError::UpdateSyncFailed(error.to_string()))?; + drop(guard); let response = ApiKeyResponse::try_from(&user)?; Ok(HttpResponse::Ok().json(response)) diff --git a/src/handlers/http/modal/ingest/ingestor_role.rs b/src/handlers/http/modal/ingest/ingestor_role.rs index 43c947250..6721a9ccb 100644 --- a/src/handlers/http/modal/ingest/ingestor_role.rs +++ b/src/handlers/http/modal/ingest/ingestor_role.rs @@ -29,7 +29,10 @@ use crate::{ role::RoleError, }, parseable::DEFAULT_TENANT, - rbac::map::{mut_roles, mut_sessions, read_user_groups, users}, + rbac::{ + is_system_role, + map::{mut_roles, mut_sessions, read_user_groups, users}, + }, storage, utils::get_tenant_id_from_request, }; @@ -42,6 +45,9 @@ pub async fn put( Json(sync_req): Json, ) -> Result { let name = name.into_inner(); + if is_system_role(&name) { + return Err(RoleError::ProtectedRole); + } let req_tenant_id = get_tenant_id_from_request(&req); let req_tenant = req_tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); if req_tenant.ne(DEFAULT_TENANT) && (req_tenant_id.ne(&sync_req.tenant_id)) { @@ -100,6 +106,9 @@ pub async fn delete( name: web::Path, ) -> Result { let name = name.into_inner(); + if is_system_role(&name) { + return Err(RoleError::ProtectedRole); + } let req_tenant_id = get_tenant_id_from_request(&req); let mut metadata = get_metadata(&req_tenant_id).await?; diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 576a500b8..97384fb04 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -31,8 +31,8 @@ use crate::{ }, parseable::{DEFAULT_TENANT, PARSEABLE}, rbac::{ - Users, - map::{roles, users, write_user_groups}, + self, Users, + map::{users, write_user_groups}, user::{self, UserType}, }, tenants::TENANT_METADATA, @@ -62,11 +62,7 @@ pub async fn post_user( let mut non_existent_roles = Vec::new(); for role in &user_roles { let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); - let role_exists = roles() - .get(tenant) - .map(|tenant_roles| tenant_roles.contains_key(role)) - .unwrap_or(false); - if !role_exists { + if !rbac::role_exists(tenant, role) { non_existent_roles.push(role.clone()); } } @@ -203,9 +199,7 @@ pub async fn add_roles_to_user( // check if the role exists roles_to_add.iter().for_each(|r| { - if let Some(tenant_roles) = roles().get(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT)) - && tenant_roles.get(r).is_none() - { + if !rbac::role_exists(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT), r) { non_existent_roles.push(r.clone()); } }); @@ -279,9 +273,7 @@ pub async fn remove_roles_from_user( // check if the role exists roles_to_remove.iter().for_each(|r| { - if let Some(tenant_roles) = roles().get(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT)) - && tenant_roles.get(r).is_none() - { + if !rbac::role_exists(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT), r) { non_existent_roles.push(r.clone()); } }); diff --git a/src/handlers/http/modal/query/querier_role.rs b/src/handlers/http/modal/query/querier_role.rs index 321efa9b0..263023076 100644 --- a/src/handlers/http/modal/query/querier_role.rs +++ b/src/handlers/http/modal/query/querier_role.rs @@ -31,6 +31,7 @@ use crate::{ }, parseable::DEFAULT_TENANT, rbac::{ + is_system_role, map::{mut_roles, mut_sessions, read_user_groups, roles, users}, role::model::{Role, RoleType}, roles_to_permission, @@ -47,6 +48,9 @@ pub async fn put( Json(body): Json, ) -> Result { let name = name.into_inner(); + if is_system_role(&name) { + return Err(RoleError::ProtectedRole); + } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); // extract caller userid early, before any session mutations @@ -130,6 +134,9 @@ pub async fn delete( name: web::Path, ) -> Result { let name = name.into_inner(); + if is_system_role(&name) { + return Err(RoleError::ProtectedRole); + } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); // extract caller userid early, before any session mutations diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 6d5bf1896..bc746ff36 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -150,9 +150,7 @@ pub async fn post_user( let mut non_existent_roles = Vec::new(); for role in &user_roles { - if let Some(tenant_roles) = roles().get(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT)) - && !tenant_roles.contains_key(role) - { + if !rbac::role_exists(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT), role) { non_existent_roles.push(role.clone()); } } @@ -294,13 +292,7 @@ pub fn get_role_internal( .get_role(userid, tenant_id) .iter() .filter_map(|role_name| { - if let Some(roles) = roles().get(tenant) - && let Some(role) = roles.get(role_name) - { - Some((role_name.to_owned(), role.clone())) - } else { - None - } + rbac::role_for(tenant, role_name).map(|role| (role_name.clone(), role)) }) .collect(); @@ -424,9 +416,7 @@ pub async fn add_roles_to_user( // check if the role exists for role in &roles_to_add { - if let Some(tenant_roles) = roles().get(tenant_str) - && !tenant_roles.contains_key(role) - { + if !rbac::role_exists(tenant_str, role) { non_existent_roles.push(role.clone()); } } @@ -488,9 +478,7 @@ pub async fn remove_roles_from_user( // check if the role exists for role in &roles_to_remove { - if let Some(tenant_roles) = roles().get(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT)) - && !tenant_roles.contains_key(role) - { + if !rbac::role_exists(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT), role) { non_existent_roles.push(role.clone()); } } diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index 969020c24..ceaba2805 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -27,6 +27,7 @@ use actix_web::{ use crate::rbac::map::roles; use crate::rbac::role::model::{Role, RoleType, RoleUI}; +use crate::rbac::{GLOBAL_INGESTOR_ROLE, GLOBAL_QUERIER_ROLE, is_system_role, system_role}; use crate::{ parseable::{DEFAULT_TENANT, PARSEABLE}, rbac::map::{DEFAULT_ROLE, mut_roles, mut_sessions, read_user_groups, users}, @@ -42,14 +43,14 @@ pub async fn put( name: web::Path, Json(role): Json, ) -> Result { + let name = name.into_inner(); // internal role manipulation not allowed - if role.role_type().eq(&RoleType::Internal) { + if is_system_role(&name) || role.role_type().eq(&RoleType::Internal) { return Err(RoleError::ProtectedRole); } if role.deny_super_admin() { return Err(RoleError::SuperAdminPrivilege); } - let name = name.into_inner(); let tenant_id = get_tenant_id_from_request(&req); // validate the role name @@ -104,6 +105,9 @@ pub async fn get(req: HttpRequest, name: web::Path) -> Result) -> Result { + if let Some(role) = system_role(name) { + return Ok(role); + } let metadata = get_metadata(tenant_id).await?; let role = metadata.roles.get(name).cloned().unwrap_or_default(); if role.role_type().eq(&RoleType::Internal) { @@ -130,6 +134,11 @@ pub async fn list_internal( roles.insert(k, RoleUI(r)); } } + for name in [GLOBAL_INGESTOR_ROLE, GLOBAL_QUERIER_ROLE] { + if let Some(role) = system_role(name) { + roles.insert(name.to_string(), RoleUI(role)); + } + } Ok(roles) } @@ -141,6 +150,9 @@ pub async fn delete( name: web::Path, ) -> Result { let name = name.into_inner(); + if is_system_role(&name) { + return Err(RoleError::ProtectedRole); + } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); if let Some(tenant_roles) = roles().get(tenant) diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index cee0da54b..1e5b4969e 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -36,7 +36,7 @@ use crate::handlers::http::rbac::RBACError; use crate::parseable::DEFAULT_TENANT; use crate::rbac::map::{mut_sessions, mut_users, read_user_groups, roles, sessions, users}; use crate::rbac::role::Action; -use crate::rbac::role::model::RoleUI; +use crate::rbac::role::model::{Role, RoleUI}; use crate::rbac::user::User; use crate::utils::get_tenant_id_from_key; @@ -45,12 +45,42 @@ use self::role::{ParseableResourceType, Permission, RoleBuilder, model::DefaultP use self::user::UserType; pub const EXPIRY_DURATION: Duration = Duration::hours(1); -/// Reserved, server-managed role for Helm's ingest-only API keys. +/// Reserved, server-managed role for global ingestion. /// This name is outside the user-defined role-name grammar. pub const GLOBAL_INGESTOR_ROLE: &str = "__global_ingestor__"; /// Reserved, server-managed role with the existing reader privilege on all datasets. pub const GLOBAL_QUERIER_ROLE: &str = "__global_querier__"; +pub fn system_role(name: &str) -> Option { + let privilege = match name { + GLOBAL_INGESTOR_ROLE => DefaultPrivilege::Ingestor { resource: None }, + GLOBAL_QUERIER_ROLE => DefaultPrivilege::Reader { + resource: Some(ParseableResourceType::Stream("*".to_string())), + }, + _ => return None, + }; + Some(Role::create_internal_role(vec![privilege])) +} + +pub fn is_system_role(name: &str) -> bool { + matches!(name, GLOBAL_INGESTOR_ROLE | GLOBAL_QUERIER_ROLE) +} + +pub fn role_for(tenant_id: &str, name: &str) -> Option { + system_role(name).or_else(|| { + roles() + .get(tenant_id) + .and_then(|roles| roles.get(name).cloned()) + }) +} + +pub fn role_exists(tenant_id: &str, name: &str) -> bool { + is_system_role(name) + || roles() + .get(tenant_id) + .is_some_and(|roles| roles.contains_key(name)) +} + #[derive(PartialEq, Debug)] pub enum Response { Authorized, diff --git a/src/rbac/utils.rs b/src/rbac/utils.rs index 435fc4b1e..c2eda9fb8 100644 --- a/src/rbac/utils.rs +++ b/src/rbac/utils.rs @@ -21,7 +21,7 @@ use url::Url; use crate::{ parseable::{DEFAULT_TENANT, PARSEABLE}, - rbac::{map::read_user_groups, role::model::RoleUI}, + rbac::{map::read_user_groups, role::model::RoleUI, role_for}, }; use super::{ @@ -63,19 +63,7 @@ pub fn to_prism_user(user: &User) -> UsersPrism { .get_role(id, &user.tenant) .iter() .filter_map(|role_name| { - roles() - .get(tenant_id) - .filter(|roles| roles.get(role_name).is_some()) - // .map(|roles| { - // if let Some(role) = roles.get(role_name) { - // (role_name.to_owned(), role.clone()) - // } - // }) - // .get(role_name) - .map(|roles| { - let role = roles.get(role_name).unwrap(); - (role_name.to_owned(), RoleUI(role.clone())) - }) + role_for(tenant_id, role_name).map(|role| (role_name.clone(), RoleUI(role))) }) .collect(); From 4c6d11a6fdbc46c6e2a79a008be91e599d4ea635 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Fri, 30 Oct 2026 08:39:55 +0530 Subject: [PATCH 09/10] fix: scope global ingestor and clarify system role errors --- src/handlers/http/modal/ingest/ingestor_role.rs | 4 ++-- src/handlers/http/modal/query/querier_role.rs | 4 ++-- src/handlers/http/role.rs | 14 ++++++++++++-- src/rbac/mod.rs | 9 +++++++-- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_role.rs b/src/handlers/http/modal/ingest/ingestor_role.rs index 6721a9ccb..70ff3467d 100644 --- a/src/handlers/http/modal/ingest/ingestor_role.rs +++ b/src/handlers/http/modal/ingest/ingestor_role.rs @@ -46,7 +46,7 @@ pub async fn put( ) -> Result { let name = name.into_inner(); if is_system_role(&name) { - return Err(RoleError::ProtectedRole); + return Err(RoleError::SystemRolePrivilegesLocked(name)); } let req_tenant_id = get_tenant_id_from_request(&req); let req_tenant = req_tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); @@ -107,7 +107,7 @@ pub async fn delete( ) -> Result { let name = name.into_inner(); if is_system_role(&name) { - return Err(RoleError::ProtectedRole); + return Err(RoleError::SystemRoleDeletion(name)); } let req_tenant_id = get_tenant_id_from_request(&req); diff --git a/src/handlers/http/modal/query/querier_role.rs b/src/handlers/http/modal/query/querier_role.rs index 263023076..b9b9afa67 100644 --- a/src/handlers/http/modal/query/querier_role.rs +++ b/src/handlers/http/modal/query/querier_role.rs @@ -49,7 +49,7 @@ pub async fn put( ) -> Result { let name = name.into_inner(); if is_system_role(&name) { - return Err(RoleError::ProtectedRole); + return Err(RoleError::SystemRolePrivilegesLocked(name)); } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); @@ -135,7 +135,7 @@ pub async fn delete( ) -> Result { let name = name.into_inner(); if is_system_role(&name) { - return Err(RoleError::ProtectedRole); + return Err(RoleError::SystemRoleDeletion(name)); } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index ceaba2805..ba6d90779 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -45,7 +45,10 @@ pub async fn put( ) -> Result { let name = name.into_inner(); // internal role manipulation not allowed - if is_system_role(&name) || role.role_type().eq(&RoleType::Internal) { + if is_system_role(&name) { + return Err(RoleError::SystemRolePrivilegesLocked(name)); + } + if role.role_type().eq(&RoleType::Internal) { return Err(RoleError::ProtectedRole); } if role.deny_super_admin() { @@ -151,7 +154,7 @@ pub async fn delete( ) -> Result { let name = name.into_inner(); if is_system_role(&name) { - return Err(RoleError::ProtectedRole); + return Err(RoleError::SystemRoleDeletion(name)); } let tenant_id = get_tenant_id_from_request(&req); let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); @@ -269,6 +272,10 @@ pub enum RoleError { RoleInUse, #[error("Cannot perform this operation as role is assigned to a protected user.")] ProtectedRole, + #[error("System role '{0}' cannot be deleted.")] + SystemRoleDeletion(String), + #[error("Privileges of system role '{0}' are fixed; you cannot add or remove them.")] + SystemRolePrivilegesLocked(String), #[error("Error: {0}")] Anyhow(#[from] anyhow::Error), #[error("{0}")] @@ -288,6 +295,9 @@ impl actix_web::ResponseError for RoleError { Self::RoleInUse => StatusCode::BAD_REQUEST, Self::SuperAdminPrivilege => StatusCode::BAD_REQUEST, Self::ProtectedRole => StatusCode::BAD_REQUEST, + Self::SystemRoleDeletion(_) | Self::SystemRolePrivilegesLocked(_) => { + StatusCode::BAD_REQUEST + } Self::Anyhow(_) => StatusCode::INTERNAL_SERVER_ERROR, Self::SerdeError(_) => StatusCode::BAD_REQUEST, Self::Network(_) => StatusCode::BAD_GATEWAY, diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index 1e5b4969e..773fcc94f 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -53,7 +53,9 @@ pub const GLOBAL_QUERIER_ROLE: &str = "__global_querier__"; pub fn system_role(name: &str) -> Option { let privilege = match name { - GLOBAL_INGESTOR_ROLE => DefaultPrivilege::Ingestor { resource: None }, + GLOBAL_INGESTOR_ROLE => DefaultPrivilege::Ingestor { + resource: Some(ParseableResourceType::Stream("*".to_string())), + }, GLOBAL_QUERIER_ROLE => DefaultPrivilege::Reader { resource: Some(ParseableResourceType::Stream("*".to_string())), }, @@ -433,7 +435,10 @@ pub fn roles_to_permission(roles: Vec, tenant_id: &str) -> Vec Date: Fri, 30 Oct 2026 10:28:34 +0530 Subject: [PATCH 10/10] fix: propagate role sync failures to ingestors --- src/handlers/http/cluster/mod.rs | 6 ++++++ src/handlers/http/modal/ingest/ingestor_rbac.rs | 16 +++++----------- src/handlers/http/modal/query/querier_rbac.rs | 16 ++++------------ 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/handlers/http/cluster/mod.rs b/src/handlers/http/cluster/mod.rs index 99d354ffc..f6d2b90e6 100644 --- a/src/handlers/http/cluster/mod.rs +++ b/src/handlers/http/cluster/mod.rs @@ -507,11 +507,17 @@ pub async fn sync_users_with_roles_with_ingestors( })?; if !res.status().is_success() { + let status = res.status(); error!( "failed to forward request to ingestor: {}\nResponse Returned: {:?}", ingestor.domain_name, res.text().await ); + return Err(RBACError::Anyhow(anyhow::anyhow!( + "role sync to {} failed with HTTP {}", + ingestor.domain_name, + status + ))); } Ok(()) diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index 7f1bc4f73..7fc0a2987 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -27,8 +27,7 @@ use crate::{ }, parseable::DEFAULT_TENANT, rbac::{ - Users, - map::roles, + Users, role_exists, user::{self, User as ParseableUser}, }, storage, @@ -107,10 +106,7 @@ pub async fn add_roles_to_user( let mut non_existent_roles = Vec::new(); roles_to_add.iter().for_each(|r| { let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT); - let role_exists = roles() - .get(tenant) - .is_some_and(|tenant_roles| tenant_roles.contains_key(r)); - if !role_exists { + if !role_exists(tenant, r) { non_existent_roles.push(r.clone()); } }); @@ -153,11 +149,9 @@ pub async fn remove_roles_from_user( // check if all roles exist let mut non_existent_roles = Vec::new(); - roles_to_remove.iter().for_each(|r| { - if let Some(tenant_roles) = roles().get(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT)) - && tenant_roles.get(r).is_none() - { - non_existent_roles.push(r.clone()); + roles_to_remove.iter().for_each(|role| { + if !role_exists(tenant_id.as_deref().unwrap_or(DEFAULT_TENANT), role) { + non_existent_roles.push(role.clone()); } }); diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 97384fb04..8aabf2e74 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -223,8 +223,7 @@ pub async fn add_roles_to_user( put_metadata(&metadata, &tenant_id).await?; - // sync to other nodes before updating in-memory (which invalidates sessions) - if let Err(e) = sync_users_with_roles_with_ingestors( + sync_users_with_roles_with_ingestors( &req, &userid, &roles_to_add, @@ -232,10 +231,7 @@ pub async fn add_roles_to_user( &tenant_id, &caller_userid, ) - .await - { - tracing::error!("Failed to sync role addition to cluster nodes: {e}"); - } + .await?; // update in mem table (this invalidates the user's session) Users.add_roles(&userid.clone(), roles_to_add.clone(), &tenant_id); @@ -326,8 +322,7 @@ pub async fn remove_roles_from_user( put_metadata(&metadata, &tenant_id).await?; - // sync to other nodes before updating in-memory (which invalidates sessions) - if let Err(e) = sync_users_with_roles_with_ingestors( + sync_users_with_roles_with_ingestors( &req, &userid, &roles_to_remove, @@ -335,10 +330,7 @@ pub async fn remove_roles_from_user( &tenant_id, &caller_userid, ) - .await - { - tracing::error!("Failed to sync role removal to cluster nodes: {e}"); - } + .await?; // update in mem table (this invalidates the user's session) Users.remove_roles(&userid.clone(), roles_to_remove.clone(), &tenant_id);