From feee99b3ec0018e7941549103d0ede3a44078b85 Mon Sep 17 00:00:00 2001 From: TCeason Date: Mon, 13 Jan 2025 09:38:49 +0800 Subject: [PATCH 1/3] feat: object warehouse support rbac --- .../app/src/principal/ownership_object.rs | 12 +- .../tenant_ownership_object_ident.rs | 16 +++ src/meta/app/src/principal/user_grant.rs | 4 +- src/meta/app/src/principal/user_privilege.rs | 19 ++- .../src/ownership_from_to_protobuf_impl.rs | 8 ++ src/meta/proto-conv/src/util.rs | 1 + src/meta/proto-conv/tests/it/main.rs | 1 + .../it/v117_warehouse_ownershipobject.rs | 116 +++++++++++++++++ src/meta/protos/proto/ownership.proto | 5 + src/query/ast/src/ast/statements/principal.rs | 3 + src/query/ast/src/ast/statements/user.rs | 2 + src/query/ast/src/parser/statement.rs | 14 +- .../ast/tests/it/testdata/stmt-error.txt | 2 +- src/query/management/src/role/role_mgr.rs | 1 + .../interpreters/access/privilege_access.rs | 122 +++++++++++++++--- .../interpreter_create_warehouses.rs | 36 +++++- .../interpreter_drop_warehouses.rs | 21 ++- .../interpreter_privilege_grant.rs | 5 +- .../interpreter_show_warehouses.rs | 10 +- .../src/sessions/session_privilege_mgr.rs | 30 ++++- .../show_grants/show_grants_table.rs | 106 ++++++++++++--- .../sql/src/planner/binder/ddl/account.rs | 3 + src/query/users/src/role_mgr.rs | 25 ---- src/query/users/src/visibility_checker.rs | 9 +- .../base/05_ddl/05_0017_ddl_grant_role.test | 2 +- 25 files changed, 486 insertions(+), 87 deletions(-) create mode 100644 src/meta/proto-conv/tests/it/v117_warehouse_ownershipobject.rs diff --git a/src/meta/app/src/principal/ownership_object.rs b/src/meta/app/src/principal/ownership_object.rs index 372773c95d1b9..4db4fb9524922 100644 --- a/src/meta/app/src/principal/ownership_object.rs +++ b/src/meta/app/src/principal/ownership_object.rs @@ -52,6 +52,10 @@ pub enum OwnershipObject { UDF { name: String, }, + + Warehouse { + uid: String, + }, } impl OwnershipObject { @@ -80,6 +84,7 @@ impl fmt::Display for OwnershipObject { } OwnershipObject::UDF { name } => write!(f, "UDF {name}"), OwnershipObject::Stage { name } => write!(f, "STAGE {name}"), + OwnershipObject::Warehouse { uid } => write!(f, "Warehouse {uid}"), } } } @@ -119,6 +124,7 @@ impl KeyCodec for OwnershipObject { } OwnershipObject::Stage { name } => b.push_raw("stage-by-name").push_str(name), OwnershipObject::UDF { name } => b.push_raw("udf-by-name").push_str(name), + OwnershipObject::Warehouse { uid } => b.push_raw("warehouse-by-uid").push_str(uid), } } @@ -165,9 +171,13 @@ impl KeyCodec for OwnershipObject { let name = p.next_str()?; Ok(OwnershipObject::UDF { name }) } + "warehouse-by-uid" => { + let uid = p.next_str()?; + Ok(OwnershipObject::Warehouse { uid }) + } _ => Err(kvapi::KeyError::InvalidSegment { i: p.index(), - expect: "database-by-id|database-by-catalog-id|table-by-id|table-by-catalog-id|stage-by-name|udf-by-name" + expect: "database-by-id|database-by-catalog-id|table-by-id|table-by-catalog-id|stage-by-name|udf-by-name|warehouse-by-uid" .to_string(), got: q.to_string(), }), diff --git a/src/meta/app/src/principal/tenant_ownership_object_ident.rs b/src/meta/app/src/principal/tenant_ownership_object_ident.rs index 5276ecc4e5f52..1bc371b4b25bc 100644 --- a/src/meta/app/src/principal/tenant_ownership_object_ident.rs +++ b/src/meta/app/src/principal/tenant_ownership_object_ident.rs @@ -254,6 +254,22 @@ mod tests { let parsed = TenantOwnershipObjectIdent::from_str_key(&key).unwrap(); assert_eq!(role_grantee, parsed); } + + // warehouse + { + let role_grantee = TenantOwnershipObjectIdent::new_unchecked( + Tenant::new_literal("test"), + OwnershipObject::Warehouse { + uid: "n87s".to_string(), + }, + ); + + let key = role_grantee.to_string_key(); + assert_eq!("__fd_object_owners/test/warehouse-by-uid/n87s", key); + + let parsed = TenantOwnershipObjectIdent::from_str_key(&key).unwrap(); + assert_eq!(role_grantee, parsed); + } } #[test] diff --git a/src/meta/app/src/principal/user_grant.rs b/src/meta/app/src/principal/user_grant.rs index 6fb2c38796e35..38cdc67e518eb 100644 --- a/src/meta/app/src/principal/user_grant.rs +++ b/src/meta/app/src/principal/user_grant.rs @@ -111,7 +111,9 @@ impl GrantObject { GrantObject::Stage(_) => { UserPrivilegeSet::available_privileges_on_stage(available_ownership) } - GrantObject::Warehouse(_) => UserPrivilegeSet::available_privileges_on_warehouse(), + GrantObject::Warehouse(_) => { + UserPrivilegeSet::available_privileges_on_warehouse(available_ownership) + } } } diff --git a/src/meta/app/src/principal/user_privilege.rs b/src/meta/app/src/principal/user_privilege.rs index 6d87ea58f26bc..3dd691f695dfc 100644 --- a/src/meta/app/src/principal/user_privilege.rs +++ b/src/meta/app/src/principal/user_privilege.rs @@ -76,6 +76,8 @@ pub enum UserPrivilegeType { Write = 1 << 19, // Privilege to Create database CreateDatabase = 1 << 20, + // Privilege to Create warehouse + CreateWarehouse = 1 << 21, // Discard Privilege Type Set = 1 << 4, } @@ -102,6 +104,7 @@ const ALL_PRIVILEGES: BitFlags = make_bitflags!( | Read | Write | CreateDatabase + | CreateWarehouse } ); @@ -129,6 +132,7 @@ impl Display for UserPrivilegeType { UserPrivilegeType::Read => "Read", UserPrivilegeType::Write => "Write", UserPrivilegeType::CreateDatabase => "CREATE DATABASE", + UserPrivilegeType::CreateWarehouse => "CREATE WAREHOUSE", }) } } @@ -166,6 +170,9 @@ impl From for UserPrivilegeType { databend_common_ast::ast::UserPrivilegeType::CreateDatabase => { UserPrivilegeType::CreateDatabase } + databend_common_ast::ast::UserPrivilegeType::CreateWarehouse => { + UserPrivilegeType::CreateWarehouse + } databend_common_ast::ast::UserPrivilegeType::Set => UserPrivilegeType::Set, } } @@ -199,8 +206,8 @@ impl UserPrivilegeSet { let database_privs = Self::available_privileges_on_database(false); let stage_privs_without_ownership = Self::available_privileges_on_stage(false); let udf_privs_without_ownership = Self::available_privileges_on_udf(false); - let wh_privs_without_ownership = Self::available_privileges_on_warehouse(); - let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask }); + let wh_privs_without_ownership = Self::available_privileges_on_warehouse(false); + let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask | CreateWarehouse }); (database_privs.privileges | privs | stage_privs_without_ownership.privileges @@ -234,8 +241,12 @@ impl UserPrivilegeSet { } } - pub fn available_privileges_on_warehouse() -> Self { - make_bitflags!(UserPrivilegeType::{ Usage }).into() + pub fn available_privileges_on_warehouse(available_ownership: bool) -> Self { + if available_ownership { + make_bitflags!(UserPrivilegeType::{ Usage | Ownership }).into() + } else { + make_bitflags!(UserPrivilegeType::{ Usage }).into() + } } pub fn available_privileges_on_udf(available_ownership: bool) -> Self { diff --git a/src/meta/proto-conv/src/ownership_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/ownership_from_to_protobuf_impl.rs index 998acdbee995c..358f991eb5695 100644 --- a/src/meta/proto-conv/src/ownership_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/ownership_from_to_protobuf_impl.rs @@ -88,6 +88,9 @@ impl FromToProto for mt::principal::OwnershipObject { pb::ownership_object::Object::Stage(pb::ownership_object::OwnershipStageObject { stage, }) => Ok(mt::principal::OwnershipObject::Stage { name: stage }), + pb::ownership_object::Object::Warehouse( + pb::ownership_object::OwnershipWarehouseObject { uid }, + ) => Ok(mt::principal::OwnershipObject::Warehouse { uid }), } } @@ -123,6 +126,11 @@ impl FromToProto for mt::principal::OwnershipObject { stage: name.clone(), }), ), + mt::principal::OwnershipObject::Warehouse { uid } => { + Some(pb::ownership_object::Object::Warehouse( + pb::ownership_object::OwnershipWarehouseObject { uid: uid.clone() }, + )) + } }; Ok(pb::OwnershipObject { ver: VER, diff --git a/src/meta/proto-conv/src/util.rs b/src/meta/proto-conv/src/util.rs index a9ef7e617502d..34922ec4d08f0 100644 --- a/src/meta/proto-conv/src/util.rs +++ b/src/meta/proto-conv/src/util.rs @@ -146,6 +146,7 @@ const META_CHANGE_LOG: &[(u64, &str)] = &[ (114, "2024-12-12: Add: New DataType Interval."), (115, "2024-12-16: Add: udf.proto: add UDAFScript and UDAFServer"), (116, "2025-01-09: Add: MarkedDeletedIndexMeta"), + (117, "2025-01-13: Add: Add new UserPrivilege CreateWarehouse and new OwnershipObject::Warehouse"), // Dear developer: // If you're gonna add a new metadata version, you'll have to add a test for it. // You could just copy an existing test file(e.g., `../tests/it/v024_table_meta.rs`) diff --git a/src/meta/proto-conv/tests/it/main.rs b/src/meta/proto-conv/tests/it/main.rs index 432c9c0e087da..a4783cc0ad8b6 100644 --- a/src/meta/proto-conv/tests/it/main.rs +++ b/src/meta/proto-conv/tests/it/main.rs @@ -114,3 +114,4 @@ mod v113_warehouse_grantobject; mod v114_interval_datatype; mod v115_add_udaf_script; mod v116_marked_deleted_index_meta; +mod v117_warehouse_ownershipobject; diff --git a/src/meta/proto-conv/tests/it/v117_warehouse_ownershipobject.rs b/src/meta/proto-conv/tests/it/v117_warehouse_ownershipobject.rs new file mode 100644 index 0000000000000..f3dc11f497d44 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v117_warehouse_ownershipobject.rs @@ -0,0 +1,116 @@ +// Copyright 2023 Datafuse Labs. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::collections::HashSet; + +use chrono::DateTime; +use chrono::Utc; +use databend_common_meta_app as mt; +use databend_common_meta_app::principal::OwnershipObject; +use databend_common_meta_app::principal::UserGrantSet; +use databend_common_meta_app::principal::UserPrivilegeType; +use enumflags2::make_bitflags; +use fastrace::func_name; + +use crate::common; + +// These bytes are built when a new version in introduced, +// and are kept for backward compatibility test. +// +// ************************************************************* +// * These messages should never be updated, * +// * only be added when a new version is added, * +// * or be removed when an old version is no longer supported. * +// ************************************************************* +// + +#[test] +fn test_decode_v117_grant_object() -> anyhow::Result<()> { + let role_info_v117 = vec![ + 10, 2, 114, 49, 18, 172, 1, 10, 21, 10, 8, 10, 0, 160, 6, 117, 168, 6, 24, 16, 128, 128, + 128, 1, 160, 6, 117, 168, 6, 24, 10, 31, 10, 21, 18, 13, 10, 7, 100, 101, 102, 97, 117, + 108, 116, 18, 2, 100, 98, 160, 6, 117, 168, 6, 24, 16, 2, 160, 6, 117, 168, 6, 24, 10, 35, + 10, 25, 26, 17, 10, 7, 100, 101, 102, 97, 117, 108, 116, 18, 2, 100, 98, 26, 2, 116, 98, + 160, 6, 117, 168, 6, 24, 16, 2, 160, 6, 117, 168, 6, 24, 10, 22, 10, 12, 34, 4, 10, 2, 102, + 49, 160, 6, 117, 168, 6, 24, 16, 1, 160, 6, 117, 168, 6, 24, 10, 24, 10, 12, 42, 4, 10, 2, + 115, 49, 160, 6, 117, 168, 6, 24, 16, 128, 128, 32, 160, 6, 117, 168, 6, 24, 10, 21, 10, 8, + 10, 0, 160, 6, 117, 168, 6, 24, 16, 254, 255, 191, 1, 160, 6, 117, 168, 6, 24, 160, 6, 117, + 168, 6, 24, 26, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 48, + 48, 32, 85, 84, 67, 34, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, + 58, 48, 48, 32, 85, 84, 67, 160, 6, 117, 168, 6, 24, + ]; + let want = || mt::principal::RoleInfo { + name: "r1".to_string(), + grants: UserGrantSet::new( + vec![ + mt::principal::GrantEntry::new( + mt::principal::GrantObject::Global, + make_bitflags!(UserPrivilegeType::{CreateWarehouse}), + ), + mt::principal::GrantEntry::new( + mt::principal::GrantObject::Database("default".to_string(), "db".to_string()), + make_bitflags!(UserPrivilegeType::{Create}), + ), + mt::principal::GrantEntry::new( + mt::principal::GrantObject::Table( + "default".to_string(), + "db".to_string(), + "tb".to_string(), + ), + make_bitflags!(UserPrivilegeType::{Create}), + ), + mt::principal::GrantEntry::new( + mt::principal::GrantObject::UDF("f1".to_string()), + make_bitflags!(UserPrivilegeType::{Usage}), + ), + mt::principal::GrantEntry::new( + mt::principal::GrantObject::Stage("s1".to_string()), + make_bitflags!(UserPrivilegeType::{Write}), + ), + // test new global privilege CreateWarehouse + mt::principal::GrantEntry::new( + mt::principal::GrantObject::Global, + make_bitflags!(UserPrivilegeType::{Create | Select | Insert | Update | Delete | Drop | Alter | Super | CreateUser | DropUser | CreateRole | DropRole | Grant | CreateStage | Set | CreateDataMask | Ownership | Read | Write | CreateWarehouse }), + ), + ], + HashSet::new(), + ), + created_on: DateTime::::default(), + update_on: DateTime::::default(), + }; + + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old(func_name!(), role_info_v117.as_slice(), 117, want())?; + + Ok(()) +} + +#[test] +fn test_decode_v117_ownership() -> anyhow::Result<()> { + let ownership_info_v117 = vec![ + 10, 2, 114, 49, 18, 19, 42, 11, 10, 9, 97, 117, 110, 105, 113, 117, 101, 105, 100, 160, 6, + 117, 168, 6, 24, 160, 6, 117, 168, 6, 24, + ]; + + let want = || mt::principal::OwnershipInfo { + role: "r1".to_string(), + object: OwnershipObject::Warehouse { + uid: "auniqueid".to_string(), + }, + }; + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old(func_name!(), ownership_info_v117.as_slice(), 117, want())?; + + Ok(()) +} diff --git a/src/meta/protos/proto/ownership.proto b/src/meta/protos/proto/ownership.proto index 227b7239cd1d4..5512daaf30c5c 100644 --- a/src/meta/protos/proto/ownership.proto +++ b/src/meta/protos/proto/ownership.proto @@ -46,10 +46,15 @@ message OwnershipObject { string stage = 1; } + message OwnershipWarehouseObject { + string uid = 1; + } + oneof object { OwnershipDatabaseObject database = 1; OwnershipTableObject table = 2; OwnershipUdfObject udf = 3; OwnershipStageObject stage = 4; + OwnershipWarehouseObject warehouse = 5; } } diff --git a/src/query/ast/src/ast/statements/principal.rs b/src/query/ast/src/ast/statements/principal.rs index fe574ebb70c9b..10112185d6408 100644 --- a/src/query/ast/src/ast/statements/principal.rs +++ b/src/query/ast/src/ast/statements/principal.rs @@ -150,6 +150,8 @@ pub enum UserPrivilegeType { Write, // Privilege to Create database CreateDatabase, + // Privilege to Create warehouse + CreateWarehouse, // Discard Privilege Type Set, } @@ -178,6 +180,7 @@ impl Display for UserPrivilegeType { UserPrivilegeType::Read => "Read", UserPrivilegeType::Write => "Write", UserPrivilegeType::CreateDatabase => "CREATE DATABASE", + UserPrivilegeType::CreateWarehouse => "CREATE WAREHOUSE", }) } } diff --git a/src/query/ast/src/ast/statements/user.rs b/src/query/ast/src/ast/statements/user.rs index f0f729baf1067..692dfc250959f 100644 --- a/src/query/ast/src/ast/statements/user.rs +++ b/src/query/ast/src/ast/statements/user.rs @@ -147,6 +147,7 @@ pub enum GrantObjectName { Table(Option, String), UDF(String), Stage(String), + Warehouse(String), } impl Display for GrantObjectName { @@ -164,6 +165,7 @@ impl Display for GrantObjectName { } GrantObjectName::UDF(udf) => write!(f, " UDF {udf}"), GrantObjectName::Stage(stage) => write!(f, " STAGE {stage}"), + GrantObjectName::Warehouse(w) => write!(f, " WAREHOUSE {w}"), } } } diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index 1fdb17af71838..6d4da8ba8ce8e 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -3278,6 +3278,10 @@ pub fn priv_type(i: Input) -> IResult { UserPrivilegeType::CreateDatabase, rule! { CREATE ~ DATABASE }, ), + value( + UserPrivilegeType::CreateWarehouse, + rule! { CREATE ~ WAREHOUSE }, + ), value(UserPrivilegeType::DropUser, rule! { DROP ~ USER }), value(UserPrivilegeType::CreateRole, rule! { CREATE ~ ROLE }), value(UserPrivilegeType::DropRole, rule! { DROP ~ ROLE }), @@ -3337,11 +3341,16 @@ pub fn on_object_name(i: Input) -> IResult { GrantObjectName::UDF(udf_name.to_string()) }); + let warehouse = map(rule! { WAREHOUSE ~ #ident}, |(_, w)| { + GrantObjectName::Warehouse(w.to_string()) + }); + rule!( #database : "DATABASE " | #table : "TABLE ." | #stage : "STAGE " | #udf : "UDF " + | #warehouse : "WAREHOUSE " )(i) } @@ -3436,10 +3445,12 @@ pub fn grant_ownership_level(i: Input) -> IResult { enum Object { Stage, Udf, + Warehouse, } let object = alt(( value(Object::Udf, rule! { UDF }), value(Object::Stage, rule! { STAGE }), + value(Object::Warehouse, rule! { STAGE }), )); // Object object_name @@ -3448,13 +3459,14 @@ pub fn grant_ownership_level(i: Input) -> IResult { |(object, object_name)| match object { Object::Stage => AccountMgrLevel::Stage(object_name.to_string()), Object::Udf => AccountMgrLevel::UDF(object_name.to_string()), + Object::Warehouse => AccountMgrLevel::Warehouse(object_name.to_string()), }, ); rule!( #db : ".*" | #table : ".
" - | #object : "STAGE | UDF " + | #object : "STAGE | UDF | WAREHOUSE " )(i) } diff --git a/src/query/ast/tests/it/testdata/stmt-error.txt b/src/query/ast/tests/it/testdata/stmt-error.txt index 1375fdc05611f..84018fd636c53 100644 --- a/src/query/ast/tests/it/testdata/stmt-error.txt +++ b/src/query/ast/tests/it/testdata/stmt-error.txt @@ -942,7 +942,7 @@ error: --> SQL:1:16 | 1 | SHOW GRANTS ON task t1; - | ^^^^ unexpected `task`, expecting `STAGE`, `TABLE`, `DATABASE`, or `UDF` + | ^^^^ unexpected `task`, expecting `STAGE`, `TABLE`, `WAREHOUSE`, `DATABASE`, or `UDF` ---------- Input ---------- diff --git a/src/query/management/src/role/role_mgr.rs b/src/query/management/src/role/role_mgr.rs index 43c4666bd4e48..6bd6bfaa48266 100644 --- a/src/query/management/src/role/role_mgr.rs +++ b/src/query/management/src/role/role_mgr.rs @@ -504,6 +504,7 @@ fn convert_to_grant_obj(owner_obj: &OwnershipObject) -> GrantObject { } => GrantObject::TableById(catalog_name.to_string(), *db_id, *table_id), OwnershipObject::Stage { name } => GrantObject::Stage(name.to_string()), OwnershipObject::UDF { name } => GrantObject::UDF(name.to_string()), + OwnershipObject::Warehouse { uid } => GrantObject::Warehouse(uid.to_string()), } } diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index 6c15c2369f58c..ac03252419b4c 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -15,12 +15,14 @@ use std::collections::HashSet; use std::sync::Arc; +use databend_common_base::base::GlobalInstance; use databend_common_catalog::catalog::Catalog; use databend_common_catalog::plan::DataSourceInfo; use databend_common_catalog::table_context::TableContext; use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_management::RoleApi; +use databend_common_management::WarehouseInfo; use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OwnershipInfo; use databend_common_meta_app::principal::OwnershipObject; @@ -43,6 +45,7 @@ use databend_common_sql::plans::RewriteKind; use databend_common_sql::Planner; use databend_common_users::RoleCacheManager; use databend_common_users::UserApiProvider; +use databend_enterprise_resources_management::ResourcesManagement; use databend_storages_common_table_meta::table::OPT_KEY_TEMP_PREFIX; use crate::interpreters::access::AccessChecker; @@ -151,7 +154,10 @@ impl PrivilegeAccess { GrantObject::UDF(name) => OwnershipObject::UDF { name: name.to_string(), }, - GrantObject::Global | GrantObject::Warehouse(_) => return Ok(None), + GrantObject::Warehouse(uid) => OwnershipObject::Warehouse { + uid: uid.to_string(), + }, + GrantObject::Global => return Ok(None), }; Ok(Some(object)) @@ -377,6 +383,59 @@ impl PrivilegeAccess { Ok(()) } + async fn validate_warehouse_ownership( + &self, + warehouse: String, + current_user: String, + ) -> Option> { + let session = self.ctx.get_current_session(); + let warehouse_mgr = GlobalInstance::get::>(); + + // Only check support_forward_warehouse_request privileges + if !warehouse_mgr.support_forward_warehouse_request() { + return Some(Ok(())); + } + + match warehouse_mgr.list_warehouses().await { + Ok(warehouses) => { + if let Some(sw) = warehouses + .iter() + .filter_map(|w| { + if let WarehouseInfo::SystemManaged(sw) = w { + Some(sw) + } else { + None + } + }) + .find(|sw| sw.id == warehouse.clone()) + { + let uid = sw.role_id.to_string(); + let grant_object = GrantObject::Warehouse(uid); + match self + .has_ownership(&session, &grant_object, false, false) + .await + { + Ok(has) => { + if has { + Some(Ok(())) + } else { + Some(Err(ErrorCode::PermissionDenied(format!( + "Permission denied: Ownership is required on WAREHOUSE '{}' for user {}", + warehouse, + current_user + )))) + } + } + Err(e) => Some(Err(e.add_message("error on checking warehouse ownership"))), + } + } else { + None + } + } + Err(e) => Some(Err(e.add_message("error on validating warehouse ownership"))), + } + } + async fn has_ownership( &self, session: &Arc, @@ -440,8 +499,9 @@ impl PrivilegeAccess { | GrantObject::DatabaseById(_, _) | GrantObject::UDF(_) | GrantObject::Stage(_) + | GrantObject::Warehouse(_) | GrantObject::TableById(_, _, _) => true, - GrantObject::Global | GrantObject::Warehouse(_) => false, + GrantObject::Global => false, }; if verify_ownership @@ -712,7 +772,7 @@ impl AccessChecker for PrivilegeAccess { .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); - check_ownership_access(&identity, catalog, database, show_db_id, &ownerships, &roles_name)?; + check_db_tb_ownership_access(&identity, catalog, database, show_db_id, &ownerships, &roles_name)?; } Some(RewriteKind::ShowStreams(database)) => { let ctl = self.ctx.get_catalog(&ctl_name).await?; @@ -730,7 +790,7 @@ impl AccessChecker for PrivilegeAccess { .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); - check_ownership_access(&identity, &ctl_name, database, show_db_id, &ownerships, &roles_name)?; + check_db_tb_ownership_access(&identity, &ctl_name, database, show_db_id, &ownerships, &roles_name)?; } Some(RewriteKind::ShowColumns(catalog_name, database, table)) => { if self.ctx.is_temp_table(catalog_name,database,table){ @@ -885,7 +945,7 @@ impl AccessChecker for PrivilegeAccess { .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); - check_ownership_access(&identity, &ctl_name, &plan.database, show_db_id, &ownerships, &roles_name)?; + check_db_tb_ownership_access(&identity, &ctl_name, &plan.database, show_db_id, &ownerships, &roles_name)?; } // Virtual Column. @@ -1269,17 +1329,41 @@ impl AccessChecker for PrivilegeAccess { } Plan::Commit => {} Plan::Abort => {} - Plan::ShowWarehouses => {} - Plan::ShowOnlineNodes => {} - Plan::DropWarehouse(_) => {} - Plan::ResumeWarehouse(_) => {} - Plan::SuspendWarehouse(_) => {} - Plan::RenameWarehouse(_) => {} - Plan::InspectWarehouse(_) => {} - Plan::DropWarehouseCluster(_) => {} - Plan::RenameWarehouseCluster(_) => {} - Plan::UseWarehouse(_) => {} - Plan::CreateWarehouse(_) => {} + Plan::ShowWarehouses => { + // check privilege in interpreter + } + Plan::ShowOnlineNodes => { + // todo: now no limit + } + Plan::DropWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::ResumeWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::SuspendWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::RenameWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::InspectWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::DropWarehouseCluster(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::RenameWarehouseCluster(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::UseWarehouse(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::CreateWarehouse(_) => { + // only current role has global level create warehouse privilege, it will pass + self.validate_access(&GrantObject::Global, UserPrivilegeType::CreateWarehouse, true, false) + .await?; + } Plan::AddWarehouseCluster(_) => {} Plan::AssignWarehouseNodes(_) => {} Plan::UnassignWarehouseNodes(_) => {} @@ -1289,7 +1373,7 @@ impl AccessChecker for PrivilegeAccess { } } -fn check_ownership_access( +fn check_db_tb_ownership_access( identity: &String, catalog: &String, database: &String, @@ -1323,7 +1407,9 @@ fn check_ownership_access( return Ok(()); } } - OwnershipObject::UDF { .. } | OwnershipObject::Stage { .. } => {} + OwnershipObject::UDF { .. } + | OwnershipObject::Stage { .. } + | OwnershipObject::Warehouse { .. } => {} } } } diff --git a/src/query/service/src/interpreters/interpreter_create_warehouses.rs b/src/query/service/src/interpreters/interpreter_create_warehouses.rs index aafa1554c29bb..3aef5edf1bcbe 100644 --- a/src/query/service/src/interpreters/interpreter_create_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_create_warehouses.rs @@ -20,8 +20,13 @@ use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_license::license::Feature; use databend_common_license::license_manager::LicenseManagerSwitch; +use databend_common_management::RoleApi; use databend_common_management::SelectedNode; +use databend_common_management::WarehouseInfo; +use databend_common_meta_app::principal::OwnershipObject; use databend_common_sql::plans::CreateWarehousePlan; +use databend_common_users::RoleCacheManager; +use databend_common_users::UserApiProvider; use databend_enterprise_resources_management::ResourcesManagement; use crate::interpreters::util::AuditElement; @@ -56,6 +61,7 @@ impl Interpreter for CreateWarehouseInterpreter { LicenseManagerSwitch::instance() .check_enterprise_enabled(self.ctx.get_license_key(), Feature::SystemManagement)?; + let tenant = self.ctx.get_tenant(); if let Some(warehouse_size) = self.plan.options.get("warehouse_size") { if !self.plan.nodes.is_empty() { return Err(ErrorCode::InvalidArgument( @@ -69,13 +75,26 @@ impl Interpreter for CreateWarehouseInterpreter { )); }; - GlobalInstance::get::>() + let warehouse = GlobalInstance::get::>() .create_warehouse(self.plan.warehouse.clone(), vec![ SelectedNode::Random(None); warehouse_size ]) .await?; + if let WarehouseInfo::SystemManaged(sw) = warehouse { + let role_api = UserApiProvider::instance().role_api(&tenant); + if let Some(current_role) = self.ctx.get_current_role() { + role_api + .grant_ownership( + &OwnershipObject::Warehouse { uid: sw.role_id }, + ¤t_role.name, + ) + .await?; + RoleCacheManager::instance().invalidate_cache(&tenant); + } + } + return Ok(PipelineBuildResult::create()); } @@ -90,10 +109,23 @@ impl Interpreter for CreateWarehouseInterpreter { } } - GlobalInstance::get::>() + let warehouse = GlobalInstance::get::>() .create_warehouse(self.plan.warehouse.clone(), selected_nodes) .await?; + if let WarehouseInfo::SystemManaged(sw) = warehouse { + let role_api = UserApiProvider::instance().role_api(&tenant); + if let Some(current_role) = self.ctx.get_current_role() { + role_api + .grant_ownership( + &OwnershipObject::Warehouse { uid: sw.role_id }, + ¤t_role.name, + ) + .await?; + RoleCacheManager::instance().invalidate_cache(&tenant); + } + } + let user_info = self.ctx.get_current_user()?; log::info!( target: "databend::log::audit", diff --git a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs index 46276c39750ee..15570871f37a8 100644 --- a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs @@ -19,7 +19,12 @@ use databend_common_catalog::table_context::TableContext; use databend_common_exception::Result; use databend_common_license::license::Feature; use databend_common_license::license_manager::LicenseManagerSwitch; +use databend_common_management::RoleApi; +use databend_common_management::WarehouseInfo; +use databend_common_meta_app::principal::OwnershipObject; use databend_common_sql::plans::DropWarehousePlan; +use databend_common_users::RoleCacheManager; +use databend_common_users::UserApiProvider; use databend_enterprise_resources_management::ResourcesManagement; use crate::interpreters::util::AuditElement; @@ -54,7 +59,8 @@ impl Interpreter for DropWarehouseInterpreter { LicenseManagerSwitch::instance() .check_enterprise_enabled(self.ctx.get_license_key(), Feature::SystemManagement)?; - GlobalInstance::get::>() + let tenant = self.ctx.get_tenant(); + let warehouse = GlobalInstance::get::>() .drop_warehouse(self.plan.warehouse.clone()) .await?; @@ -65,6 +71,19 @@ impl Interpreter for DropWarehouseInterpreter { serde_json::to_string(&AuditElement::create(&user_info, "drop_warehouse", &self.plan))? ); + if let WarehouseInfo::SystemManaged(sw) = warehouse { + let role_api = UserApiProvider::instance().role_api(&tenant); + if let Some(current_role) = self.ctx.get_current_role() { + role_api + .grant_ownership( + &OwnershipObject::Warehouse { uid: sw.role_id }, + ¤t_role.name, + ) + .await?; + RoleCacheManager::instance().invalidate_cache(&tenant); + } + } + Ok(PipelineBuildResult::create()) } } diff --git a/src/query/service/src/interpreters/interpreter_privilege_grant.rs b/src/query/service/src/interpreters/interpreter_privilege_grant.rs index 91694329d21ce..dad9d00dbe835 100644 --- a/src/query/service/src/interpreters/interpreter_privilege_grant.rs +++ b/src/query/service/src/interpreters/interpreter_privilege_grant.rs @@ -101,7 +101,10 @@ impl GrantPrivilegeInterpreter { GrantObject::UDF(name) => Ok(OwnershipObject::UDF { name: name.to_string(), }), - GrantObject::Global | GrantObject::Warehouse(_) => Err(ErrorCode::IllegalGrant( + GrantObject::Warehouse(uid) => Ok(OwnershipObject::Warehouse { + uid: uid.to_string(), + }), + GrantObject::Global => Err(ErrorCode::IllegalGrant( "Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used", )), } diff --git a/src/query/service/src/interpreters/interpreter_show_warehouses.rs b/src/query/service/src/interpreters/interpreter_show_warehouses.rs index be44d4993c717..f915ee9a7a733 100644 --- a/src/query/service/src/interpreters/interpreter_show_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_show_warehouses.rs @@ -63,6 +63,7 @@ impl Interpreter for ShowWarehousesInterpreter { let mut warehouses_status = ColumnBuilder::with_capacity(&DataType::String, warehouses.len()); + let visibility_checker = self.ctx.get_visibility_checker(false).await?; for warehouse in warehouses { match warehouse { WarehouseInfo::SelfManaged(name) => { @@ -71,9 +72,12 @@ impl Interpreter for ShowWarehousesInterpreter { warehouses_status.push(Scalar::String(String::from("Running")).as_ref()); } WarehouseInfo::SystemManaged(v) => { - warehouses_name.push(Scalar::String(v.id.clone()).as_ref()); - warehouses_type.push(Scalar::String(String::from("System-Managed")).as_ref()); - warehouses_status.push(Scalar::String(v.status.clone()).as_ref()); + if visibility_checker.check_warehouse_visibility(&v.role_id) { + warehouses_name.push(Scalar::String(v.id.clone()).as_ref()); + warehouses_type + .push(Scalar::String(String::from("System-Managed")).as_ref()); + warehouses_status.push(Scalar::String(v.status.clone()).as_ref()); + } } } } diff --git a/src/query/service/src/sessions/session_privilege_mgr.rs b/src/query/service/src/sessions/session_privilege_mgr.rs index 07f01c1314f26..381f2036db26a 100644 --- a/src/query/service/src/sessions/session_privilege_mgr.rs +++ b/src/query/service/src/sessions/session_privilege_mgr.rs @@ -12,9 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashMap; +use std::sync::Arc; + +use databend_common_base::base::GlobalInstance; use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_management::RoleApi; +use databend_common_management::WarehouseInfo; use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OwnershipObject; use databend_common_meta_app::principal::RoleInfo; @@ -25,6 +30,7 @@ use databend_common_users::RoleCacheManager; use databend_common_users::UserApiProvider; use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN; use databend_common_users::BUILTIN_ROLE_PUBLIC; +use databend_enterprise_resources_management::ResourcesManagement; use crate::sessions::SessionContext; @@ -229,15 +235,27 @@ impl SessionPrivilegeManager for SessionPrivilegeManagerImpl<'_> { async fn set_current_warehouse(&self, warehouse: Option) -> Result<()> { let warehouse = match &warehouse { Some(warehouse) => { + let warehouse_mgr = GlobalInstance::get::>(); + let warehouses = warehouse_mgr.list_warehouses().await?; + let user_warehouse: HashMap = warehouses + .into_iter() + .filter_map(|w| match w { + WarehouseInfo::SystemManaged(sw) => Some((sw.role_id, sw.id)), + _ => None, + }) + .collect(); let effective_roles = self.get_all_effective_roles().await?; effective_roles.iter().find_map(|role| { role.grants.entries().iter().find_map(|grant| { - if let GrantObject::Warehouse(rw) = grant.object() { - if warehouse == rw { - Some(warehouse.to_string()) - } else { - None - } + let obj = grant.object(); + if let GrantObject::Warehouse(rw) = obj { + user_warehouse.get(warehouse).and_then(|w| { + if w == rw { + Some(warehouse.to_string()) + } else { + None + } + }) } else { None } diff --git a/src/query/service/src/table_functions/show_grants/show_grants_table.rs b/src/query/service/src/table_functions/show_grants/show_grants_table.rs index 79b77dc8b5be6..4aa2de5bbad52 100644 --- a/src/query/service/src/table_functions/show_grants/show_grants_table.rs +++ b/src/query/service/src/table_functions/show_grants/show_grants_table.rs @@ -17,6 +17,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; +use databend_common_base::base::GlobalInstance; use databend_common_catalog::plan::DataSourcePlan; use databend_common_catalog::plan::PartStatistics; use databend_common_catalog::plan::Partitions; @@ -27,9 +28,7 @@ use databend_common_catalog::table_context::TableContext; use databend_common_catalog::table_function::TableFunction; use databend_common_exception::ErrorCode; use databend_common_exception::Result; -use databend_common_expression::types::NumberDataType; use databend_common_expression::types::StringType; -use databend_common_expression::types::UInt64Type; use databend_common_expression::DataBlock; use databend_common_expression::FromData; use databend_common_expression::Scalar; @@ -38,6 +37,7 @@ use databend_common_expression::TableField; use databend_common_expression::TableSchema; use databend_common_expression::TableSchemaRefExt; use databend_common_management::RoleApi; +use databend_common_management::WarehouseInfo; use databend_common_meta_app::principal::GrantEntry; use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OwnershipObject; @@ -55,6 +55,7 @@ use databend_common_pipeline_sources::AsyncSourcer; use databend_common_sql::validate_function_arg; use databend_common_users::RoleCacheManager; use databend_common_users::UserApiProvider; +use databend_enterprise_resources_management::ResourcesManagement; use itertools::Itertools; const SHOW_GRANTS: &str = "show_grants"; @@ -128,7 +129,7 @@ impl ShowGrants { TableField::new("object_name", TableDataType::String), TableField::new( "object_id", - TableDataType::Nullable(Box::from(TableDataType::Number(NumberDataType::UInt64))), + TableDataType::Nullable(Box::from(TableDataType::String)), ), TableField::new("grant_to", TableDataType::String), TableField::new("name", TableDataType::String), @@ -237,7 +238,7 @@ impl AsyncSource for ShowGrantsSource { "role" | "user" => { show_account_grants(self.ctx.clone(), &self.grant_type, &self.name).await? } - "table" | "database" | "udf" | "stage" => { + "table" | "database" | "udf" | "stage" | "warehouse" => { show_object_grant( self.ctx.clone(), &self.grant_type, @@ -275,6 +276,13 @@ async fn show_account_grants( let user_api = UserApiProvider::instance(); + let warehouse_mgr = GlobalInstance::get::>(); + let warehouses = if warehouse_mgr.support_forward_warehouse_request() { + warehouse_mgr.list_warehouses().await? + } else { + vec![] + }; + // TODO: add permission check on reading user grants let (grant_to, name, identity, grant_set, roles) = match grant_type { "user" => { @@ -431,12 +439,23 @@ async fn show_account_grants( privileges.push(get_priv_str(&grant_entry)); grant_list.push(format!("{} TO {}", grant_entry, identity)); } - GrantObject::Warehouse(w_name) => { - // grant all on *.* to a - object_name.push(w_name.to_string()); - object_id.push(None); - privileges.push(get_priv_str(&grant_entry)); - grant_list.push(format!("{} TO {}", grant_entry, identity)); + GrantObject::Warehouse(uid) => { + if let Some(sw) = warehouses + .iter() + .filter_map(|w| { + if let WarehouseInfo::SystemManaged(sw) = w { + Some(sw) + } else { + None + } + }) + .find(|sw| sw.role_id == *uid) + { + object_name.push(sw.id.to_string()); + object_id.push(Some(uid.to_string())); + privileges.push(get_priv_str(&grant_entry)); + grant_list.push(format!("{} TO {}", grant_entry, identity)); + } } GrantObject::Global => { // grant all on *.* to a @@ -453,8 +472,6 @@ async fn show_account_grants( // No need to display ownership. if !roles.contains(&"account_admin".to_string()) { let ownerships = user_api.role_api(&tenant).get_ownerships().await?; - // let mut catalog_db_ids: HashMap> = HashMap::new(); - // let mut catalog_table_ids: HashMap> = HashMap::new(); for ownership in ownerships { if roles.contains(&ownership.data.role) { match ownership.data.object { @@ -499,6 +516,27 @@ async fn show_account_grants( privileges.push("OWNERSHIP".to_string()); grant_list.push(format!("GRANT OWNERSHIP ON UDF {} TO {}", name, identity)); } + OwnershipObject::Warehouse { uid } => { + if let Some(sw) = warehouses + .iter() + .filter_map(|w| { + if let WarehouseInfo::SystemManaged(sw) = w { + Some(sw) + } else { + None + } + }) + .find(|sw| sw.role_id == uid) + { + object_name.push(sw.id.to_string()); + object_id.push(Some(uid.to_string())); + privileges.push("OWNERSHIP".to_string()); + grant_list.push(format!( + "GRANT OWNERSHIP ON WAREHOUSE {} TO {}", + uid, identity + )); + } + } } } } @@ -526,7 +564,7 @@ async fn show_account_grants( privilege_str, catalog_name, db_name, identity ); object_name.push(db_name.to_string()); - object_id.push(Some(db_id)); + object_id.push(Some(db_id.to_string())); privileges.push(privilege_str); grant_list.push(grant_str); } @@ -573,7 +611,7 @@ async fn show_account_grants( &privilege_str, catalog_name, db_name, table_name, identity ); object_name.push(format!("{}.{}.{}", catalog_name, db_name, table_name)); - object_id.push(Some(table_id)); + object_id.push(Some(table_id.to_string())); privileges.push(privilege_str); grant_list.push(grant_str); } @@ -586,7 +624,7 @@ async fn show_account_grants( Ok(Some(DataBlock::new_from_columns(vec![ StringType::from_data(privileges), StringType::from_data(object_name), - UInt64Type::from_opt_data(object_id), + StringType::from_opt_data(object_id), StringType::from_data(grant_tos), StringType::from_data(names), StringType::from_data(grant_list), @@ -634,7 +672,7 @@ async fn show_object_grant( db_id, table_id, }, - Some(table_id), + Some(table_id.to_string()), name, ) } @@ -659,7 +697,7 @@ async fn show_object_grant( catalog_name: catalog_name.to_string(), db_id, }, - Some(db_id), + Some(db_id.to_string()), name, ) } @@ -695,9 +733,39 @@ async fn show_object_grant( name, ) } + "warehouse" => { + let warehouse_mgr = GlobalInstance::get::>(); + if !warehouse_mgr.support_forward_warehouse_request() { + return Err(ErrorCode::InvalidArgument("The 'SHOW GRANTS ON ' only supported for warehouses managed by the system. Please verify that you are using a system-managed warehouse".to_string())); + } + let warehouses = warehouse_mgr.list_warehouses().await?; + let mut uid = String::new(); + for w in warehouses { + if let WarehouseInfo::SystemManaged(rw) = w { + if rw.id == name { + uid = rw.role_id.to_string(); + break; + } + } + } + if !visibility_checker.check_warehouse_visibility(&uid) { + return Err(ErrorCode::PermissionDenied(format!( + "Permission denied: No privilege on warehouse {} for user {}.", + name, current_user + ))); + } + ( + GrantObject::Warehouse(uid.to_string()), + OwnershipObject::Warehouse { + uid: uid.to_string(), + }, + Some(uid), + name, + ) + } _ => { return Err(ErrorCode::InvalidArgument(format!( - "Expected 'table|database|udf|stage', but got {:?}", + "Expected 'table|database|udf|stage|warehouse', but got {:?}", grant_type ))); } @@ -730,7 +798,7 @@ async fn show_object_grant( Ok(Some(DataBlock::new_from_columns(vec![ StringType::from_data(privileges), StringType::from_data(object_names), - UInt64Type::from_opt_data(object_ids), + StringType::from_opt_data(object_ids), StringType::from_data(grant_tos), StringType::from_data(names), StringType::from_data(grant_list), diff --git a/src/query/sql/src/planner/binder/ddl/account.rs b/src/query/sql/src/planner/binder/ddl/account.rs index 38c02666e959c..50bcf8eaaa7a8 100644 --- a/src/query/sql/src/planner/binder/ddl/account.rs +++ b/src/query/sql/src/planner/binder/ddl/account.rs @@ -448,6 +448,9 @@ impl Binder { GrantObjectName::Stage(name) => { format!("SELECT * FROM show_grants('stage', '{}')", name) } + GrantObjectName::Warehouse(name) => { + format!("SELECT * FROM show_grants('warehouse', '{}')", name) + } }; let (show_limit, limit_str) = get_show_options(show_option, Some("name".to_string())); diff --git a/src/query/users/src/role_mgr.rs b/src/query/users/src/role_mgr.rs index ab12fc206d888..4f3ca63ff630d 100644 --- a/src/query/users/src/role_mgr.rs +++ b/src/query/users/src/role_mgr.rs @@ -198,31 +198,6 @@ impl UserApiProvider { privileges: UserPrivilegeSet, ) -> Result> { let client = self.role_api(tenant); - // Enforces warehouse exclusivity: a warehouse’s specific permission can only be granted to a single role. - // This logic verifies that no other role within the given tenant currently holds the target permission on the specified warehouse. - if let GrantObject::Warehouse(w) = &object { - let roles = self.get_roles(tenant).await?; - let mut illegal_role = String::new(); - let has_same_warehouse_grant = roles.iter().any(|r| { - let has_grant = r.grants.entries().iter().any(|grant| match grant.object() { - GrantObject::Warehouse(rw) => { - if w.to_lowercase() == rw.to_lowercase() { - illegal_role = r.identity().to_string(); - true - } else { - false - } - } - _ => false, - }); - has_grant - }); - if has_same_warehouse_grant { - return Err(ErrorCode::IllegalRole(format!( - "Error granting '{privileges}' access on data warehouse '{w}' to role '{role}'. Role '{illegal_role}' already possesses this access" - ))); - } - } client .update_role_with(role, MatchSeq::GE(1), |ri: &mut RoleInfo| { ri.update_role_time(); diff --git a/src/query/users/src/visibility_checker.rs b/src/query/users/src/visibility_checker.rs index a5face0632f15..1bc2935ccf04e 100644 --- a/src/query/users/src/visibility_checker.rs +++ b/src/query/users/src/visibility_checker.rs @@ -103,7 +103,7 @@ impl GrantObjectVisibilityChecker { &mut granted_global_ws, ent.privileges().iter(), |privilege| { - UserPrivilegeSet::available_privileges_on_warehouse() + UserPrivilegeSet::available_privileges_on_warehouse(false) .has_privilege(privilege) }, ); @@ -200,6 +200,9 @@ impl GrantObjectVisibilityChecker { OwnershipObject::UDF { name } => { granted_udfs.insert(name.to_string()); } + OwnershipObject::Warehouse { uid } => { + granted_ws.insert(uid.to_string()); + } } } @@ -259,12 +262,12 @@ impl GrantObjectVisibilityChecker { false } - pub fn check_warehouse_visibility(&self, udf: &str) -> bool { + pub fn check_warehouse_visibility(&self, uid: &str) -> bool { if self.granted_global_ws { return true; } - if self.granted_ws.contains(udf) { + if self.granted_ws.contains(uid) { return true; } false diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test b/tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test index 4ad34d0048c15..0518b2fc74c24 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test @@ -241,7 +241,7 @@ grant usage on warehouse a to role role1; statement ok grant usage on warehouse b to role role1; -statement error 2217 +statement ok grant usage on warehouse a to role role2; statement ok From 11f3a5e153ba9aff376c76bf004b741ac8d9b168 Mon Sep 17 00:00:00 2001 From: TCeason Date: Wed, 15 Jan 2025 11:48:43 +0800 Subject: [PATCH 2/3] databend-test print more std err --- tests/databend-test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/databend-test b/tests/databend-test index b4b5dc41688b6..f439858093e5d 100755 --- a/tests/databend-test +++ b/tests/databend-test @@ -326,7 +326,7 @@ def run_tests_array(all_tests_with_params): if os.path.isfile(stdout_file): status += ", result:\n\n" status += '\n'.join( - open(stdout_file).read().split('\n')[:100]) + open(stdout_file).read().split('\n')[:300]) status += '\n' elif stderr: @@ -336,7 +336,7 @@ def run_tests_array(all_tests_with_params): status += MSG_FAIL status += print_test_time(total_time) status += " - having stderror:\n{}\n".format( - '\n'.join(stderr.split('\n')[:100])) + '\n'.join(stderr.split('\n')[:300])) elif 'Exception' in stdout: failures += 1 failure_cases.append(case_file_full_path) @@ -344,7 +344,7 @@ def run_tests_array(all_tests_with_params): status += MSG_FAIL status += print_test_time(total_time) status += " - having exception:\n{}\n".format( - '\n'.join(stdout.split('\n')[:100])) + '\n'.join(stdout.split('\n')[:300])) elif not os.path.isfile(result_file): status += MSG_UNKNOWN status += print_test_time(total_time) From 02cd206a444fbb7ad4d17522c6c188c240bd6654 Mon Sep 17 00:00:00 2001 From: taichong Date: Thu, 16 Jan 2025 09:26:46 +0800 Subject: [PATCH 3/3] try add compat test --- tests/compat_fuse/compat-logictest/rbac/fuse_compat_write | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write b/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write index 3de1201dd30dd..c993ad44e3016 100644 --- a/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write +++ b/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write @@ -18,3 +18,9 @@ create role 'role2'; statement ok GRANT OWNERSHIP on udf a to role 'role1'; + +statement ok +create warehouse w1 with warehouse_size=1; + +statement ok +GRANT OWNERSHIP on warehouse w1 to role 'role1';