From 7066011c5e06af4da9f90e0b14c519e7db5af0ba Mon Sep 17 00:00:00 2001 From: anant Date: Mon, 14 Jul 2025 17:10:03 +0530 Subject: [PATCH 1/2] Bugfixes - RBAC fix for internal datasets --- src/rbac/map.rs | 11 ++++++++++- src/utils/mod.rs | 13 ++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/rbac/map.rs b/src/rbac/map.rs index 8da51db7a..23ee06440 100644 --- a/src/rbac/map.rs +++ b/src/rbac/map.rs @@ -239,7 +239,16 @@ impl Sessions { | ParseableResourceType::Llm(resource_id) => { let ok_resource = if let Some(context_resource_id) = context_resource { - resource_id == context_resource_id || resource_id == "*" + let is_internal = PARSEABLE + .get_stream(context_resource_id) + .is_ok_and(|stream| { + stream + .get_stream_type() + .eq(&crate::storage::StreamType::Internal) + }); + resource_id == context_resource_id + || resource_id == "*" + || is_internal } else { // if no resource to match then resource check is not needed // WHEN IS THIS VALID?? diff --git a/src/utils/mod.rs b/src/utils/mod.rs index e4edb0692..da7f78f13 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -26,6 +26,7 @@ pub mod uid; pub mod update; use crate::handlers::http::rbac::RBACError; +use crate::parseable::PARSEABLE; use crate::query::{TableScanVisitor, QUERY_SESSION}; use crate::rbac::map::SessionKey; use crate::rbac::role::{Action, Permission}; @@ -117,10 +118,20 @@ pub fn user_auth_for_datasets( Action::Query, crate::rbac::role::ParseableResourceType::Stream(stream), ) => { - if stream == table_name || stream == "*" { + let is_internal = PARSEABLE + .get_stream(&table_name) + .is_ok_and(|stream|stream.get_stream_type().eq(&crate::storage::StreamType::Internal)); + + if stream == table_name + || stream == "*" + || is_internal + { authorized = true; } } + Permission::Resource(_, crate::rbac::role::ParseableResourceType::All) => { + authorized = true; + } _ => (), } } From 66e7a794317f2cb673c1ad775a58566569ad4b97 Mon Sep 17 00:00:00 2001 From: anant Date: Tue, 15 Jul 2025 00:16:56 +0530 Subject: [PATCH 2/2] Coderabbit suggestions --- src/utils/mod.rs | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/utils/mod.rs b/src/utils/mod.rs index da7f78f13..ee7583ba7 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -29,7 +29,7 @@ use crate::handlers::http::rbac::RBACError; use crate::parseable::PARSEABLE; use crate::query::{TableScanVisitor, QUERY_SESSION}; use crate::rbac::map::SessionKey; -use crate::rbac::role::{Action, Permission}; +use crate::rbac::role::{Action, ParseableResourceType, Permission}; use crate::rbac::Users; use actix::extract_session_key_from_req; use actix_web::HttpRequest; @@ -114,22 +114,31 @@ pub fn user_auth_for_datasets( authorized = true; break; } - Permission::Resource( - Action::Query, - crate::rbac::role::ParseableResourceType::Stream(stream), - ) => { - let is_internal = PARSEABLE - .get_stream(&table_name) - .is_ok_and(|stream|stream.get_stream_type().eq(&crate::storage::StreamType::Internal)); - - if stream == table_name - || stream == "*" - || is_internal - { + Permission::Resource(Action::Query, ParseableResourceType::Stream(stream)) => { + let is_internal = PARSEABLE.get_stream(table_name).is_ok_and(|stream| { + stream + .get_stream_type() + .eq(&crate::storage::StreamType::Internal) + }); + + if stream == table_name || stream == "*" || is_internal { authorized = true; } } - Permission::Resource(_, crate::rbac::role::ParseableResourceType::All) => { + Permission::Resource(action, ParseableResourceType::All) + if ![ + Action::All, + Action::PutUser, + Action::PutRole, + Action::DeleteUser, + Action::DeleteRole, + Action::ModifyUserGroup, + Action::CreateUserGroup, + Action::DeleteUserGroup, + Action::DeleteNode, + ] + .contains(action) => + { authorized = true; } _ => (),