From b9fcf3da8fbe56d3bb9e438816359c4450de86e4 Mon Sep 17 00:00:00 2001 From: TCeason Date: Mon, 13 Jan 2025 09:38:49 +0800 Subject: [PATCH 1/7] 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 | 2 + .../it/v120_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 +- .../compat-logictest/rbac/fuse_compat_write | 6 + .../base/05_ddl/05_0017_ddl_grant_role.test | 2 +- 26 files changed, 493 insertions(+), 87 deletions(-) create mode 100644 src/meta/proto-conv/tests/it/v120_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 0baed0196fc8d..4bc2cdfd03cf1 100644 --- a/src/meta/proto-conv/src/util.rs +++ b/src/meta/proto-conv/src/util.rs @@ -149,6 +149,7 @@ const META_CHANGE_LOG: &[(u64, &str)] = &[ (117, "2025-01-21: Add: config.proto: add disable_list_batch in WebhdfsConfig"), (118, "2025-01-22: Add: config.proto: add user_name in WebhdfsConfig"), (119, "2025-01-25: Add: virtual_column add alias_names and auto_generated field"), + (120, "2025-02-11: 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 70009ddb71a65..ad5e0dc58547c 100644 --- a/src/meta/proto-conv/tests/it/main.rs +++ b/src/meta/proto-conv/tests/it/main.rs @@ -117,3 +117,5 @@ mod v116_marked_deleted_index_meta; mod v117_webhdfs_add_disable_list_batch; mod v118_webhdfs_add_user_name; mod v119_virtual_column; + +mod v120_warehouse_ownershipobject; diff --git a/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs b/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs new file mode 100644 index 0000000000000..87620298f1a33 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v120_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_v120_grant_object() -> anyhow::Result<()> { + let role_info_v120 = 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_v120.as_slice(), 117, want())?; + + Ok(()) +} + +#[test] +fn test_decode_v120_ownership() -> anyhow::Result<()> { + let ownership_info_v120 = 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_v120.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 c81c50b194ba7..f7988f995c59d 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -3304,6 +3304,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 }), @@ -3363,11 +3367,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) } @@ -3462,10 +3471,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 @@ -3474,13 +3485,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 db59ecdd1cb4c..d4389771323f4 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. @@ -1270,17 +1330,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(_) => {} @@ -1290,7 +1374,7 @@ impl AccessChecker for PrivilegeAccess { } } -fn check_ownership_access( +fn check_db_tb_ownership_access( identity: &String, catalog: &String, database: &String, @@ -1324,7 +1408,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/compat_fuse/compat-logictest/rbac/fuse_compat_write b/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write index 3de1201dd30dd..935a267fe6ddc 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'; 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 e727c40bc6083b564aa9a0ed0aef52bb1e963e71 Mon Sep 17 00:00:00 2001 From: TCeason Date: Fri, 14 Feb 2025 15:15:39 +0800 Subject: [PATCH 2/7] add WAREHOUSE as ident note: only support_forward_warehouse_request will apply warehouse rbac. Now only support SystemResourcesManagement --- src/query/ast/src/parser/statement.rs | 2 +- src/query/ast/src/parser/token.rs | 1 + src/query/ast/tests/it/parser.rs | 1 + src/query/ast/tests/it/testdata/stmt.txt | 22 ++++++++++++++++ .../interpreters/access/privilege_access.rs | 17 +++++++++--- .../service/src/interpreters/common/grant.rs | 26 ++++++++++++++++--- .../interpreter_privilege_grant.rs | 2 +- 7 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index f7988f995c59d..db739d08234d6 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -3476,7 +3476,7 @@ pub fn grant_ownership_level(i: Input) -> IResult { let object = alt(( value(Object::Udf, rule! { UDF }), value(Object::Stage, rule! { STAGE }), - value(Object::Warehouse, rule! { STAGE }), + value(Object::Warehouse, rule! { WAREHOUSE }), )); // Object object_name diff --git a/src/query/ast/src/parser/token.rs b/src/query/ast/src/parser/token.rs index 09f0f55fc9b0f..49cdda2ffc34f 100644 --- a/src/query/ast/src/parser/token.rs +++ b/src/query/ast/src/parser/token.rs @@ -1754,6 +1754,7 @@ impl TokenKind { // | TokenKind::RETURNING | TokenKind::STAGE | TokenKind::UDF + | TokenKind::WAREHOUSE | TokenKind::SHARE | TokenKind::SHARES | TokenKind::TO diff --git a/src/query/ast/tests/it/parser.rs b/src/query/ast/tests/it/parser.rs index b95ec76c808a5..a3966d08225eb 100644 --- a/src/query/ast/tests/it/parser.rs +++ b/src/query/ast/tests/it/parser.rs @@ -774,6 +774,7 @@ fn test_statement() { r#"GRANT OWNERSHIP ON d20_0014.* TO ROLE 'd20_0015_owner';"#, r#"GRANT OWNERSHIP ON d20_0014.t TO ROLE 'd20_0015_owner';"#, r#"GRANT OWNERSHIP ON STAGE s1 TO ROLE 'd20_0015_owner';"#, + r#"GRANT OWNERSHIP ON WAREHOUSE w1 TO ROLE 'd20_0015_owner';"#, r#"GRANT OWNERSHIP ON UDF f1 TO ROLE 'd20_0015_owner';"#, r#"attach table t 's3://a' connection=(access_key_id ='x' secret_access_key ='y' endpoint_url='http://127.0.0.1:9900')"#, r#"CREATE FUNCTION IF NOT EXISTS isnotempty AS(p) -> not(is_null(p));"#, diff --git a/src/query/ast/tests/it/testdata/stmt.txt b/src/query/ast/tests/it/testdata/stmt.txt index 1565362a45b5b..8163f360e1306 100644 --- a/src/query/ast/tests/it/testdata/stmt.txt +++ b/src/query/ast/tests/it/testdata/stmt.txt @@ -23659,6 +23659,28 @@ Grant( ) +---------- Input ---------- +GRANT OWNERSHIP ON WAREHOUSE w1 TO ROLE 'd20_0015_owner'; +---------- Output --------- +GRANT OWNERSHIP ON WAREHOUSE w1 TO ROLE 'd20_0015_owner' +---------- AST ------------ +Grant( + GrantStmt { + source: Privs { + privileges: [ + Ownership, + ], + level: Warehouse( + "w1", + ), + }, + principal: Role( + "d20_0015_owner", + ), + }, +) + + ---------- Input ---------- GRANT OWNERSHIP ON UDF f1 TO ROLE 'd20_0015_owner'; ---------- Output --------- diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index d4389771323f4..5365034590380 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -1361,13 +1361,24 @@ impl AccessChecker for PrivilegeAccess { self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; } Plan::CreateWarehouse(_) => { + let warehouse_mgr = GlobalInstance::get::>(); + // Only check support_forward_warehouse_request privileges + if !warehouse_mgr.support_forward_warehouse_request() { + return Ok(()); + } // 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(_) => {} + Plan::AddWarehouseCluster(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::AssignWarehouseNodes(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } + Plan::UnassignWarehouseNodes(plan) => { + self.validate_warehouse_ownership(plan.warehouse.clone(), identity).await.transpose()?; + } } Ok(()) diff --git a/src/query/service/src/interpreters/common/grant.rs b/src/query/service/src/interpreters/common/grant.rs index c0037dc2dad76..75563d8afb174 100644 --- a/src/query/service/src/interpreters/common/grant.rs +++ b/src/query/service/src/interpreters/common/grant.rs @@ -14,10 +14,13 @@ use std::sync::Arc; +use databend_common_base::base::GlobalInstance; use databend_common_catalog::table_context::TableContext; use databend_common_exception::Result; +use databend_common_management::WarehouseInfo; use databend_common_meta_app::principal::GrantObject; use databend_common_users::UserApiProvider; +use databend_enterprise_resources_management::ResourcesManagement; use crate::sessions::QueryContext; @@ -93,9 +96,26 @@ pub async fn validate_grant_object_exists( ))); } } - GrantObject::Warehouse(_w) => { - // TODO - return Ok(()); + GrantObject::Warehouse(w) => { + let warehouse_mgr = GlobalInstance::get::>(); + // Only check support_forward_warehouse_request + if !warehouse_mgr.support_forward_warehouse_request() { + return Ok(()); + } + let ws = warehouse_mgr.list_warehouses().await?; + return if ws.iter().any(|warehouse| { + if let WarehouseInfo::SystemManaged(sw) = warehouse { + &sw.id == w + } else { + false + } + }) { + Ok(()) + } else { + Err(databend_common_exception::ErrorCode::UnknownWarehouse( + format!("warehouse {w} not exists"), + )) + }; } GrantObject::Global => (), } diff --git a/src/query/service/src/interpreters/interpreter_privilege_grant.rs b/src/query/service/src/interpreters/interpreter_privilege_grant.rs index dad9d00dbe835..de45a8139fd10 100644 --- a/src/query/service/src/interpreters/interpreter_privilege_grant.rs +++ b/src/query/service/src/interpreters/interpreter_privilege_grant.rs @@ -131,7 +131,7 @@ impl GrantPrivilegeInterpreter { } let mut log_msg = format!( - "{}: grant ownership on {:?} to {}", + "{}: grant ownership on {:?} to {}", ctx.get_id(), owner_object, new_role From 7d2b5cf56102d5622d59f5d809190d4ea2e223cb Mon Sep 17 00:00:00 2001 From: TCeason Date: Fri, 14 Feb 2025 15:33:13 +0800 Subject: [PATCH 3/7] add new privilege type forward test --- tests/compat_fuse/compat-logictest/rbac/fuse_compat_write | 7 ++----- .../suites/base/05_ddl/05_0029_ddl_create_catalog.test | 2 +- tests/sqllogictests/suites/tpch_iceberg/prune.test | 2 +- tests/sqllogictests/suites/tpch_iceberg/queries.test | 2 +- .../3_stateful_iceberg/00_rest/00_0000_create_and_show.sh | 2 +- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write b/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write index 935a267fe6ddc..fd872a358dddf 100644 --- a/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write +++ b/tests/compat_fuse/compat-logictest/rbac/fuse_compat_write @@ -19,8 +19,5 @@ 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'; +statement ok +GRANT create warehouse on *.* to role 'role1'; diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test b/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test index 35169c9de556d..d98489c4b3495 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test @@ -10,7 +10,7 @@ SHOW CATALOGS; default statement error 1001 -CREATE CATALOG ctl TYPE=ICEBERG CONNECTION=(TYPE='REST' ADDRESS='http://127.0.0.1:1000' WAREHOUSE='default' ); +CREATE CATALOG ctl TYPE=ICEBERG CONNECTION=(TYPE='REST' ADDRESS='http://127.0.0.1:1000' `WAREHOUSE`='default' ); statement ok DROP CATALOG IF EXISTS ctl; diff --git a/tests/sqllogictests/suites/tpch_iceberg/prune.test b/tests/sqllogictests/suites/tpch_iceberg/prune.test index af392ee19c9a8..e9527dc410bfc 100644 --- a/tests/sqllogictests/suites/tpch_iceberg/prune.test +++ b/tests/sqllogictests/suites/tpch_iceberg/prune.test @@ -7,7 +7,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - WAREHOUSE='s3://iceberg-tpch' + `WAREHOUSE`='s3://iceberg-tpch' "s3.region"='us-east-1' "s3.endpoint"='http://127.0.0.1:9000' ); diff --git a/tests/sqllogictests/suites/tpch_iceberg/queries.test b/tests/sqllogictests/suites/tpch_iceberg/queries.test index bf656f0bbe1a4..a8d64f87bbf40 100644 --- a/tests/sqllogictests/suites/tpch_iceberg/queries.test +++ b/tests/sqllogictests/suites/tpch_iceberg/queries.test @@ -7,7 +7,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - WAREHOUSE='s3://iceberg-tpch' + `WAREHOUSE`='s3://iceberg-tpch' "s3.region"='us-east-1' "s3.endpoint"='http://127.0.0.1:9000' ); diff --git a/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh b/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh index 38c38f44659c9..a08e46b64bb78 100755 --- a/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh +++ b/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh @@ -12,7 +12,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - WAREHOUSE='s3://icebergdata/demo' + `WAREHOUSE`='s3://icebergdata/demo' ); EOF From ce9a14f9547a76aac61c942322a6f049a5d60773 Mon Sep 17 00:00:00 2001 From: TCeason Date: Fri, 14 Feb 2025 17:43:20 +0800 Subject: [PATCH 4/7] add new ident parse func: grant_ident --- src/query/ast/src/parser/common.rs | 4 ++++ src/query/ast/src/parser/stage.rs | 13 +++++++++++++ src/query/ast/src/parser/statement.rs | 6 +++--- src/query/ast/src/parser/token.rs | 8 +++++++- src/query/ast/tests/it/parser.rs | 2 +- src/query/ast/tests/it/testdata/stmt.txt | 5 +++-- .../base/05_ddl/05_0029_ddl_create_catalog.test | 2 +- .../suites/task/task_ddl_transaction_test.test | 3 --- tests/sqllogictests/suites/tpch_iceberg/prune.test | 2 +- .../sqllogictests/suites/tpch_iceberg/queries.test | 2 +- .../00_rest/00_0000_create_and_show.sh | 2 +- 11 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/query/ast/src/parser/common.rs b/src/query/ast/src/parser/common.rs index 48bf82389d90d..acfa4fff80ecb 100644 --- a/src/query/ast/src/parser/common.rs +++ b/src/query/ast/src/parser/common.rs @@ -88,6 +88,10 @@ pub fn ident(i: Input) -> IResult { non_reserved_identifier(|token| token.is_reserved_ident(false))(i) } +pub fn grant_ident(i: Input) -> IResult { + non_reserved_identifier(|token| token.is_grant_reserved_ident(false, true))(i) +} + pub fn plain_ident(i: Input) -> IResult { plain_identifier(|token| token.is_reserved_ident(false))(i) } diff --git a/src/query/ast/src/parser/stage.rs b/src/query/ast/src/parser/stage.rs index c545f38a3b066..e7fddfdfc5484 100644 --- a/src/query/ast/src/parser/stage.rs +++ b/src/query/ast/src/parser/stage.rs @@ -30,6 +30,19 @@ use crate::parser::input::Input; use crate::parser::token::*; use crate::parser::ErrorKind; +pub fn parameter_to_grant_string(i: Input) -> IResult { + let ident_to_string = |i| map_res(grant_ident, |ident| Ok(ident.name))(i); + let u64_to_string = |i| map(literal_u64, |v| v.to_string())(i); + let boolean_to_string = |i| map(literal_bool, |v| v.to_string())(i); + + rule!( + #literal_string + | #ident_to_string + | #u64_to_string + | #boolean_to_string + )(i) +} + pub fn parameter_to_string(i: Input) -> IResult { let ident_to_string = |i| map_res(ident, |ident| Ok(ident.name))(i); let u64_to_string = |i| map(literal_u64, |v| v.to_string())(i); diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index db739d08234d6..314d6d96fd9e5 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -3452,7 +3452,7 @@ pub fn grant_ownership_level(i: Input) -> IResult { // "*": as current db or "table" with current db let db = map( rule! { - ( #ident ~ "." )? ~ "*" + ( #grant_ident ~ "." )? ~ "*" }, |(database, _)| AccountMgrLevel::Database(database.map(|(database, _)| database.name)), ); @@ -3460,7 +3460,7 @@ pub fn grant_ownership_level(i: Input) -> IResult { // `db01`.'tb1' or `db01`.`tb1` or `db01`.tb1 let table = map( rule! { - ( #ident ~ "." )? ~ #parameter_to_string + ( #grant_ident ~ "." )? ~ #parameter_to_grant_string }, |(database, table)| { AccountMgrLevel::Table(database.map(|(database, _)| database.name), table) @@ -3481,7 +3481,7 @@ pub fn grant_ownership_level(i: Input) -> IResult { // Object object_name let object = map( - rule! { #object ~ #ident}, + rule! { #object ~ #grant_ident }, |(object, object_name)| match object { Object::Stage => AccountMgrLevel::Stage(object_name.to_string()), Object::Udf => AccountMgrLevel::UDF(object_name.to_string()), diff --git a/src/query/ast/src/parser/token.rs b/src/query/ast/src/parser/token.rs index 49cdda2ffc34f..b65615a4c2c5d 100644 --- a/src/query/ast/src/parser/token.rs +++ b/src/query/ast/src/parser/token.rs @@ -1754,7 +1754,6 @@ impl TokenKind { // | TokenKind::RETURNING | TokenKind::STAGE | TokenKind::UDF - | TokenKind::WAREHOUSE | TokenKind::SHARE | TokenKind::SHARES | TokenKind::TO @@ -1773,6 +1772,13 @@ impl TokenKind { _ => false } } + + pub(crate) fn is_grant_reserved_ident(&self, after_as: bool, in_grant: bool) -> bool { + match self { + TokenKind::WAREHOUSE if in_grant => true, + _ => self.is_reserved_ident(after_as), + } + } } pub fn all_reserved_keywords() -> Vec { diff --git a/src/query/ast/tests/it/parser.rs b/src/query/ast/tests/it/parser.rs index a3966d08225eb..9b238044af454 100644 --- a/src/query/ast/tests/it/parser.rs +++ b/src/query/ast/tests/it/parser.rs @@ -137,7 +137,7 @@ fn test_statement() { r#"drop table a;"#, r#"drop table if exists a."b";"#, r#"use "a";"#, - r#"create catalog ctl type=hive connection=(url='' thrift_protocol='binary');"#, + r#"create catalog ctl type=hive connection=(url='' thrift_protocol='binary' warehouse='default');"#, r#"select current_catalog();"#, r#"use catalog ctl;"#, r#"create database if not exists a;"#, diff --git a/src/query/ast/tests/it/testdata/stmt.txt b/src/query/ast/tests/it/testdata/stmt.txt index 8163f360e1306..338727c522450 100644 --- a/src/query/ast/tests/it/testdata/stmt.txt +++ b/src/query/ast/tests/it/testdata/stmt.txt @@ -2937,9 +2937,9 @@ UseDatabase { ---------- Input ---------- -create catalog ctl type=hive connection=(url='' thrift_protocol='binary'); +create catalog ctl type=hive connection=(url='' thrift_protocol='binary' warehouse='default'); ---------- Output --------- -CREATE CATALOG ctl TYPE=HIVE CONNECTION = ( thrift_protocol = 'binary', url = '' ) +CREATE CATALOG ctl TYPE=HIVE CONNECTION = ( thrift_protocol = 'binary', url = '', warehouse = 'default' ) ---------- AST ------------ CreateCatalog( CreateCatalogStmt { @@ -2949,6 +2949,7 @@ CreateCatalog( catalog_options: { "thrift_protocol": "binary", "url": "", + "warehouse": "default", }, }, ) diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test b/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test index d98489c4b3495..35169c9de556d 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0029_ddl_create_catalog.test @@ -10,7 +10,7 @@ SHOW CATALOGS; default statement error 1001 -CREATE CATALOG ctl TYPE=ICEBERG CONNECTION=(TYPE='REST' ADDRESS='http://127.0.0.1:1000' `WAREHOUSE`='default' ); +CREATE CATALOG ctl TYPE=ICEBERG CONNECTION=(TYPE='REST' ADDRESS='http://127.0.0.1:1000' WAREHOUSE='default' ); statement ok DROP CATALOG IF EXISTS ctl; diff --git a/tests/sqllogictests/suites/task/task_ddl_transaction_test.test b/tests/sqllogictests/suites/task/task_ddl_transaction_test.test index 5e38c539ec183..850a487cf0b73 100644 --- a/tests/sqllogictests/suites/task/task_ddl_transaction_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_transaction_test.test @@ -61,8 +61,5 @@ select SPLIT(definition, '\n') from system.tasks where name = 'transactionTask' ---- ['BEGIN','SELECT 2024;','BEGIN;','MERGE INTO t USING s ON t.id = s.id WHEN MATCHED THEN UPDATE *;','DELETE FROM t WHERE c = '';'';','ABORT;','END;'] - - - statement ok DROP TASK transactionTask diff --git a/tests/sqllogictests/suites/tpch_iceberg/prune.test b/tests/sqllogictests/suites/tpch_iceberg/prune.test index e9527dc410bfc..af392ee19c9a8 100644 --- a/tests/sqllogictests/suites/tpch_iceberg/prune.test +++ b/tests/sqllogictests/suites/tpch_iceberg/prune.test @@ -7,7 +7,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - `WAREHOUSE`='s3://iceberg-tpch' + WAREHOUSE='s3://iceberg-tpch' "s3.region"='us-east-1' "s3.endpoint"='http://127.0.0.1:9000' ); diff --git a/tests/sqllogictests/suites/tpch_iceberg/queries.test b/tests/sqllogictests/suites/tpch_iceberg/queries.test index a8d64f87bbf40..bf656f0bbe1a4 100644 --- a/tests/sqllogictests/suites/tpch_iceberg/queries.test +++ b/tests/sqllogictests/suites/tpch_iceberg/queries.test @@ -7,7 +7,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - `WAREHOUSE`='s3://iceberg-tpch' + WAREHOUSE='s3://iceberg-tpch' "s3.region"='us-east-1' "s3.endpoint"='http://127.0.0.1:9000' ); diff --git a/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh b/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh index a08e46b64bb78..38c38f44659c9 100755 --- a/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh +++ b/tests/suites/3_stateful_iceberg/00_rest/00_0000_create_and_show.sh @@ -12,7 +12,7 @@ TYPE=ICEBERG CONNECTION=( TYPE='rest' ADDRESS='http://127.0.0.1:8181' - `WAREHOUSE`='s3://icebergdata/demo' + WAREHOUSE='s3://icebergdata/demo' ); EOF From 1c41631d30eb61edcffa5f81060514115eb6e65f Mon Sep 17 00:00:00 2001 From: TCeason Date: Wed, 19 Feb 2025 10:50:55 +0800 Subject: [PATCH 5/7] 1. Normalize the code 2. grant ownership on warehouse object only supported for warehouses managed by the system --- src/meta/app/src/principal/ownership_object.rs | 8 ++++---- .../principal/tenant_ownership_object_ident.rs | 2 +- src/meta/app/src/principal/user_privilege.rs | 4 +++- .../src/ownership_from_to_protobuf_impl.rs | 4 ++-- .../tests/it/v120_warehouse_ownershipobject.rs | 2 +- src/query/management/src/role/role_mgr.rs | 2 +- .../src/interpreters/access/privilege_access.rs | 2 +- .../interpreter_create_warehouses.rs | 4 ++-- .../interpreters/interpreter_drop_warehouses.rs | 2 +- .../interpreters/interpreter_privilege_grant.rs | 15 ++++++++++++++- .../show_grants/show_grants_table.rs | 16 +++++++--------- src/query/users/src/user_mgr.rs | 2 +- src/query/users/src/visibility_checker.rs | 2 +- .../base/05_ddl/05_0017_ddl_grant_role.test | 3 +++ 14 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/meta/app/src/principal/ownership_object.rs b/src/meta/app/src/principal/ownership_object.rs index 4db4fb9524922..0bd733edaa97a 100644 --- a/src/meta/app/src/principal/ownership_object.rs +++ b/src/meta/app/src/principal/ownership_object.rs @@ -54,7 +54,7 @@ pub enum OwnershipObject { }, Warehouse { - uid: String, + id: String, }, } @@ -84,7 +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}"), + OwnershipObject::Warehouse { id: uid } => write!(f, "Warehouse {uid}"), } } } @@ -124,7 +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), + OwnershipObject::Warehouse { id: uid } => b.push_raw("warehouse-by-uid").push_str(uid), } } @@ -173,7 +173,7 @@ impl KeyCodec for OwnershipObject { } "warehouse-by-uid" => { let uid = p.next_str()?; - Ok(OwnershipObject::Warehouse { uid }) + Ok(OwnershipObject::Warehouse { id: uid }) } _ => Err(kvapi::KeyError::InvalidSegment { i: p.index(), 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 1bc371b4b25bc..622972be70a35 100644 --- a/src/meta/app/src/principal/tenant_ownership_object_ident.rs +++ b/src/meta/app/src/principal/tenant_ownership_object_ident.rs @@ -260,7 +260,7 @@ mod tests { let role_grantee = TenantOwnershipObjectIdent::new_unchecked( Tenant::new_literal("test"), OwnershipObject::Warehouse { - uid: "n87s".to_string(), + id: "n87s".to_string(), }, ); diff --git a/src/meta/app/src/principal/user_privilege.rs b/src/meta/app/src/principal/user_privilege.rs index 3dd691f695dfc..9944a43424baf 100644 --- a/src/meta/app/src/principal/user_privilege.rs +++ b/src/meta/app/src/principal/user_privilege.rs @@ -207,7 +207,9 @@ impl UserPrivilegeSet { 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(false); - let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask | CreateWarehouse }); + // TODO : The warehouse functionality is not yet fully integrated. Therefore, the CreateWarehouse permission must be granted separately + // If self-created user or configured user wants to create warehouse in system-manage cluster must execute: grant create warehouse on *.* to ; + let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask }); (database_privs.privileges | privs | stage_privs_without_ownership.privileges 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 358f991eb5695..b837cb5444ecb 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 @@ -90,7 +90,7 @@ impl FromToProto for mt::principal::OwnershipObject { }) => Ok(mt::principal::OwnershipObject::Stage { name: stage }), pb::ownership_object::Object::Warehouse( pb::ownership_object::OwnershipWarehouseObject { uid }, - ) => Ok(mt::principal::OwnershipObject::Warehouse { uid }), + ) => Ok(mt::principal::OwnershipObject::Warehouse { id: uid }), } } @@ -126,7 +126,7 @@ impl FromToProto for mt::principal::OwnershipObject { stage: name.clone(), }), ), - mt::principal::OwnershipObject::Warehouse { uid } => { + mt::principal::OwnershipObject::Warehouse { id: uid } => { Some(pb::ownership_object::Object::Warehouse( pb::ownership_object::OwnershipWarehouseObject { uid: uid.clone() }, )) diff --git a/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs b/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs index 87620298f1a33..a95e50c773271 100644 --- a/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs +++ b/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs @@ -106,7 +106,7 @@ fn test_decode_v120_ownership() -> anyhow::Result<()> { let want = || mt::principal::OwnershipInfo { role: "r1".to_string(), object: OwnershipObject::Warehouse { - uid: "auniqueid".to_string(), + id: "auniqueid".to_string(), }, }; common::test_pb_from_to(func_name!(), want())?; diff --git a/src/query/management/src/role/role_mgr.rs b/src/query/management/src/role/role_mgr.rs index 6bd6bfaa48266..09ed1497cf7da 100644 --- a/src/query/management/src/role/role_mgr.rs +++ b/src/query/management/src/role/role_mgr.rs @@ -504,7 +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()), + OwnershipObject::Warehouse { id: 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 5365034590380..a1cae5769d237 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -155,7 +155,7 @@ impl PrivilegeAccess { name: name.to_string(), }, GrantObject::Warehouse(uid) => OwnershipObject::Warehouse { - uid: uid.to_string(), + id: uid.to_string(), }, GrantObject::Global => return Ok(None), }; diff --git a/src/query/service/src/interpreters/interpreter_create_warehouses.rs b/src/query/service/src/interpreters/interpreter_create_warehouses.rs index 3aef5edf1bcbe..3cb1cf4930bf6 100644 --- a/src/query/service/src/interpreters/interpreter_create_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_create_warehouses.rs @@ -87,7 +87,7 @@ impl Interpreter for CreateWarehouseInterpreter { if let Some(current_role) = self.ctx.get_current_role() { role_api .grant_ownership( - &OwnershipObject::Warehouse { uid: sw.role_id }, + &OwnershipObject::Warehouse { id: sw.role_id }, ¤t_role.name, ) .await?; @@ -118,7 +118,7 @@ impl Interpreter for CreateWarehouseInterpreter { if let Some(current_role) = self.ctx.get_current_role() { role_api .grant_ownership( - &OwnershipObject::Warehouse { uid: sw.role_id }, + &OwnershipObject::Warehouse { id: sw.role_id }, ¤t_role.name, ) .await?; diff --git a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs index 15570871f37a8..dd3f9253e66a8 100644 --- a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs @@ -76,7 +76,7 @@ impl Interpreter for DropWarehouseInterpreter { if let Some(current_role) = self.ctx.get_current_role() { role_api .grant_ownership( - &OwnershipObject::Warehouse { uid: sw.role_id }, + &OwnershipObject::Warehouse { id: sw.role_id }, ¤t_role.name, ) .await?; diff --git a/src/query/service/src/interpreters/interpreter_privilege_grant.rs b/src/query/service/src/interpreters/interpreter_privilege_grant.rs index de45a8139fd10..a81c465386efb 100644 --- a/src/query/service/src/interpreters/interpreter_privilege_grant.rs +++ b/src/query/service/src/interpreters/interpreter_privilege_grant.rs @@ -14,6 +14,7 @@ use std::sync::Arc; +use databend_common_base::base::GlobalInstance; use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_meta_app::principal::GrantObject; @@ -25,6 +26,7 @@ use databend_common_meta_app::tenant::Tenant; use databend_common_sql::plans::GrantPrivilegePlan; use databend_common_users::RoleCacheManager; use databend_common_users::UserApiProvider; +use databend_enterprise_resources_management::ResourcesManagement; use log::debug; use log::error; use log::info; @@ -102,7 +104,7 @@ impl GrantPrivilegeInterpreter { name: name.to_string(), }), GrantObject::Warehouse(uid) => Ok(OwnershipObject::Warehouse { - uid: uid.to_string(), + id: uid.to_string(), }), GrantObject::Global => Err(ErrorCode::IllegalGrant( "Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used", @@ -203,6 +205,17 @@ impl Interpreter for GrantPrivilegeInterpreter { .convert_to_ownerobject(&tenant, &plan.on, plan.on.catalog()) .await?; if self.ctx.get_current_role().is_some() { + if let OwnershipObject::Warehouse { .. } = owner_object { + let warehouse_mgr = + GlobalInstance::get::>(); + + // Only support grant ownership when support_forward_warehouse_request is true + if !warehouse_mgr.support_forward_warehouse_request() { + return Err(ErrorCode::IllegalGrant( + "Illegal GRANT/REVOKE command; only supported for warehouses managed by the system", + )); + } + } self.grant_ownership(&self.ctx, &tenant, &owner_object, &role) .await?; } else { 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 4aa2de5bbad52..e569060d3699a 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 @@ -516,7 +516,7 @@ async fn show_account_grants( privileges.push("OWNERSHIP".to_string()); grant_list.push(format!("GRANT OWNERSHIP ON UDF {} TO {}", name, identity)); } - OwnershipObject::Warehouse { uid } => { + OwnershipObject::Warehouse { id: uid } => { if let Some(sw) = warehouses .iter() .filter_map(|w| { @@ -739,27 +739,25 @@ async fn show_object_grant( 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(); + let mut id = String::new(); for w in warehouses { if let WarehouseInfo::SystemManaged(rw) = w { if rw.id == name { - uid = rw.role_id.to_string(); + id = rw.role_id.to_string(); break; } } } - if !visibility_checker.check_warehouse_visibility(&uid) { + if !visibility_checker.check_warehouse_visibility(&id) { 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), + GrantObject::Warehouse(id.to_string()), + OwnershipObject::Warehouse { id: id.to_string() }, + Some(id), name, ) } diff --git a/src/query/users/src/user_mgr.rs b/src/query/users/src/user_mgr.rs index 86520e38b7a07..42b44c5ba3671 100644 --- a/src/query/users/src/user_mgr.rs +++ b/src/query/users/src/user_mgr.rs @@ -233,7 +233,7 @@ impl UserApiProvider { } if let GrantObject::Warehouse(_) = object { return Err(ErrorCode::IllegalUser(format!( - "Cannot grant warehouse privileges to user `{}`", + "Cannot revoke warehouse privileges to user `{}`", user.username ))); } diff --git a/src/query/users/src/visibility_checker.rs b/src/query/users/src/visibility_checker.rs index 1bc2935ccf04e..a51633a99cff5 100644 --- a/src/query/users/src/visibility_checker.rs +++ b/src/query/users/src/visibility_checker.rs @@ -200,7 +200,7 @@ impl GrantObjectVisibilityChecker { OwnershipObject::UDF { name } => { granted_udfs.insert(name.to_string()); } - OwnershipObject::Warehouse { uid } => { + OwnershipObject::Warehouse { id: uid } => { granted_ws.insert(uid.to_string()); } } 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 0518b2fc74c24..ea1b4652c48e0 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 @@ -264,3 +264,6 @@ drop role if exists role1; statement ok drop role if exists role2; + +statement error 1061 +GRANT ownership on warehouse w1 to role 'role1'; From 8db8cc66cf510dc6a2bc60d3a00b2c7505961628 Mon Sep 17 00:00:00 2001 From: TCeason Date: Thu, 20 Feb 2025 14:53:56 +0800 Subject: [PATCH 6/7] fix warehouse uid modify to id --- .../app/src/principal/ownership_object.rs | 12 +++++----- .../tenant_ownership_object_ident.rs | 2 +- .../src/ownership_from_to_protobuf_impl.rs | 8 +++---- src/meta/proto-conv/tests/it/main.rs | 1 - .../it/v120_warehouse_ownershipobject.rs | 22 +++++++++---------- src/meta/protos/proto/ownership.proto | 2 +- src/query/management/src/role/role_mgr.rs | 2 +- .../interpreters/access/privilege_access.rs | 8 +++---- .../interpreter_privilege_grant.rs | 4 ++-- .../show_grants/show_grants_table.rs | 14 ++++++------ src/query/users/src/visibility_checker.rs | 8 +++---- 11 files changed, 40 insertions(+), 43 deletions(-) diff --git a/src/meta/app/src/principal/ownership_object.rs b/src/meta/app/src/principal/ownership_object.rs index 0bd733edaa97a..d3d55858bcbe5 100644 --- a/src/meta/app/src/principal/ownership_object.rs +++ b/src/meta/app/src/principal/ownership_object.rs @@ -84,7 +84,7 @@ impl fmt::Display for OwnershipObject { } OwnershipObject::UDF { name } => write!(f, "UDF {name}"), OwnershipObject::Stage { name } => write!(f, "STAGE {name}"), - OwnershipObject::Warehouse { id: uid } => write!(f, "Warehouse {uid}"), + OwnershipObject::Warehouse { id } => write!(f, "Warehouse {id}"), } } } @@ -124,7 +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 { id: uid } => b.push_raw("warehouse-by-uid").push_str(uid), + OwnershipObject::Warehouse { id } => b.push_raw("warehouse-by-id").push_str(id), } } @@ -171,13 +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 { id: uid }) + "warehouse-by-id" => { + let id = p.next_str()?; + Ok(OwnershipObject::Warehouse { id }) } _ => 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|warehouse-by-uid" + expect: "database-by-id|database-by-catalog-id|table-by-id|table-by-catalog-id|stage-by-name|udf-by-name|warehouse-by-id" .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 622972be70a35..ee4a874643fc5 100644 --- a/src/meta/app/src/principal/tenant_ownership_object_ident.rs +++ b/src/meta/app/src/principal/tenant_ownership_object_ident.rs @@ -265,7 +265,7 @@ mod tests { ); let key = role_grantee.to_string_key(); - assert_eq!("__fd_object_owners/test/warehouse-by-uid/n87s", key); + assert_eq!("__fd_object_owners/test/warehouse-by-id/n87s", key); let parsed = TenantOwnershipObjectIdent::from_str_key(&key).unwrap(); assert_eq!(role_grantee, parsed); 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 b837cb5444ecb..f711d7041e96d 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 @@ -89,8 +89,8 @@ impl FromToProto for mt::principal::OwnershipObject { stage, }) => Ok(mt::principal::OwnershipObject::Stage { name: stage }), pb::ownership_object::Object::Warehouse( - pb::ownership_object::OwnershipWarehouseObject { uid }, - ) => Ok(mt::principal::OwnershipObject::Warehouse { id: uid }), + pb::ownership_object::OwnershipWarehouseObject { id }, + ) => Ok(mt::principal::OwnershipObject::Warehouse { id }), } } @@ -126,9 +126,9 @@ impl FromToProto for mt::principal::OwnershipObject { stage: name.clone(), }), ), - mt::principal::OwnershipObject::Warehouse { id: uid } => { + mt::principal::OwnershipObject::Warehouse { id } => { Some(pb::ownership_object::Object::Warehouse( - pb::ownership_object::OwnershipWarehouseObject { uid: uid.clone() }, + pb::ownership_object::OwnershipWarehouseObject { id: id.clone() }, )) } }; diff --git a/src/meta/proto-conv/tests/it/main.rs b/src/meta/proto-conv/tests/it/main.rs index ad5e0dc58547c..fca400817084b 100644 --- a/src/meta/proto-conv/tests/it/main.rs +++ b/src/meta/proto-conv/tests/it/main.rs @@ -117,5 +117,4 @@ mod v116_marked_deleted_index_meta; mod v117_webhdfs_add_disable_list_batch; mod v118_webhdfs_add_user_name; mod v119_virtual_column; - mod v120_warehouse_ownershipobject; diff --git a/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs b/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs index a95e50c773271..997fc1160ec50 100644 --- a/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs +++ b/src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs @@ -38,17 +38,17 @@ use crate::common; #[test] fn test_decode_v120_grant_object() -> anyhow::Result<()> { let role_info_v120 = 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, 2, 114, 49, 18, 172, 1, 10, 21, 10, 8, 10, 0, 160, 6, 120, 168, 6, 24, 16, 128, 128, + 128, 1, 160, 6, 120, 168, 6, 24, 10, 31, 10, 21, 18, 13, 10, 7, 100, 101, 102, 97, 117, + 108, 116, 18, 2, 100, 98, 160, 6, 120, 168, 6, 24, 16, 2, 160, 6, 120, 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, + 160, 6, 120, 168, 6, 24, 16, 2, 160, 6, 120, 168, 6, 24, 10, 22, 10, 12, 34, 4, 10, 2, 102, + 49, 160, 6, 120, 168, 6, 24, 16, 1, 160, 6, 120, 168, 6, 24, 10, 24, 10, 12, 42, 4, 10, 2, + 115, 49, 160, 6, 120, 168, 6, 24, 16, 128, 128, 32, 160, 6, 120, 168, 6, 24, 10, 21, 10, 8, + 10, 0, 160, 6, 120, 168, 6, 24, 16, 254, 255, 191, 1, 160, 6, 120, 168, 6, 24, 160, 6, 120, 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, + 58, 48, 48, 32, 85, 84, 67, 160, 6, 120, 168, 6, 24, ]; let want = || mt::principal::RoleInfo { name: "r1".to_string(), @@ -91,7 +91,7 @@ fn test_decode_v120_grant_object() -> anyhow::Result<()> { }; common::test_pb_from_to(func_name!(), want())?; - common::test_load_old(func_name!(), role_info_v120.as_slice(), 117, want())?; + common::test_load_old(func_name!(), role_info_v120.as_slice(), 120, want())?; Ok(()) } @@ -100,7 +100,7 @@ fn test_decode_v120_grant_object() -> anyhow::Result<()> { fn test_decode_v120_ownership() -> anyhow::Result<()> { let ownership_info_v120 = 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, + 120, 168, 6, 24, 160, 6, 120, 168, 6, 24, ]; let want = || mt::principal::OwnershipInfo { @@ -110,7 +110,7 @@ fn test_decode_v120_ownership() -> anyhow::Result<()> { }, }; common::test_pb_from_to(func_name!(), want())?; - common::test_load_old(func_name!(), ownership_info_v120.as_slice(), 117, want())?; + common::test_load_old(func_name!(), ownership_info_v120.as_slice(), 120, want())?; Ok(()) } diff --git a/src/meta/protos/proto/ownership.proto b/src/meta/protos/proto/ownership.proto index 5512daaf30c5c..e21e6300b48cd 100644 --- a/src/meta/protos/proto/ownership.proto +++ b/src/meta/protos/proto/ownership.proto @@ -47,7 +47,7 @@ message OwnershipObject { } message OwnershipWarehouseObject { - string uid = 1; + string id = 1; } oneof object { diff --git a/src/query/management/src/role/role_mgr.rs b/src/query/management/src/role/role_mgr.rs index 09ed1497cf7da..01d0c52cb9e0b 100644 --- a/src/query/management/src/role/role_mgr.rs +++ b/src/query/management/src/role/role_mgr.rs @@ -504,7 +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 { id: uid } => GrantObject::Warehouse(uid.to_string()), + OwnershipObject::Warehouse { id } => GrantObject::Warehouse(id.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 a1cae5769d237..02ca47216188a 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -154,9 +154,7 @@ impl PrivilegeAccess { GrantObject::UDF(name) => OwnershipObject::UDF { name: name.to_string(), }, - GrantObject::Warehouse(uid) => OwnershipObject::Warehouse { - id: uid.to_string(), - }, + GrantObject::Warehouse(id) => OwnershipObject::Warehouse { id: id.to_string() }, GrantObject::Global => return Ok(None), }; @@ -409,8 +407,8 @@ impl PrivilegeAccess { }) .find(|sw| sw.id == warehouse.clone()) { - let uid = sw.role_id.to_string(); - let grant_object = GrantObject::Warehouse(uid); + let id = sw.role_id.to_string(); + let grant_object = GrantObject::Warehouse(id); match self .has_ownership(&session, &grant_object, false, false) .await diff --git a/src/query/service/src/interpreters/interpreter_privilege_grant.rs b/src/query/service/src/interpreters/interpreter_privilege_grant.rs index a81c465386efb..0047852b203af 100644 --- a/src/query/service/src/interpreters/interpreter_privilege_grant.rs +++ b/src/query/service/src/interpreters/interpreter_privilege_grant.rs @@ -103,8 +103,8 @@ impl GrantPrivilegeInterpreter { GrantObject::UDF(name) => Ok(OwnershipObject::UDF { name: name.to_string(), }), - GrantObject::Warehouse(uid) => Ok(OwnershipObject::Warehouse { - id: uid.to_string(), + GrantObject::Warehouse(id) => Ok(OwnershipObject::Warehouse { + id: id.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/table_functions/show_grants/show_grants_table.rs b/src/query/service/src/table_functions/show_grants/show_grants_table.rs index e569060d3699a..c65a791180836 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 @@ -439,7 +439,7 @@ async fn show_account_grants( privileges.push(get_priv_str(&grant_entry)); grant_list.push(format!("{} TO {}", grant_entry, identity)); } - GrantObject::Warehouse(uid) => { + GrantObject::Warehouse(id) => { if let Some(sw) = warehouses .iter() .filter_map(|w| { @@ -449,10 +449,10 @@ async fn show_account_grants( None } }) - .find(|sw| sw.role_id == *uid) + .find(|sw| sw.role_id == *id) { object_name.push(sw.id.to_string()); - object_id.push(Some(uid.to_string())); + object_id.push(Some(id.to_string())); privileges.push(get_priv_str(&grant_entry)); grant_list.push(format!("{} TO {}", grant_entry, identity)); } @@ -516,7 +516,7 @@ async fn show_account_grants( privileges.push("OWNERSHIP".to_string()); grant_list.push(format!("GRANT OWNERSHIP ON UDF {} TO {}", name, identity)); } - OwnershipObject::Warehouse { id: uid } => { + OwnershipObject::Warehouse { id } => { if let Some(sw) = warehouses .iter() .filter_map(|w| { @@ -526,14 +526,14 @@ async fn show_account_grants( None } }) - .find(|sw| sw.role_id == uid) + .find(|sw| sw.role_id == id) { object_name.push(sw.id.to_string()); - object_id.push(Some(uid.to_string())); + object_id.push(Some(id.to_string())); privileges.push("OWNERSHIP".to_string()); grant_list.push(format!( "GRANT OWNERSHIP ON WAREHOUSE {} TO {}", - uid, identity + id, identity )); } } diff --git a/src/query/users/src/visibility_checker.rs b/src/query/users/src/visibility_checker.rs index a51633a99cff5..cce5c5fde03dd 100644 --- a/src/query/users/src/visibility_checker.rs +++ b/src/query/users/src/visibility_checker.rs @@ -200,8 +200,8 @@ impl GrantObjectVisibilityChecker { OwnershipObject::UDF { name } => { granted_udfs.insert(name.to_string()); } - OwnershipObject::Warehouse { id: uid } => { - granted_ws.insert(uid.to_string()); + OwnershipObject::Warehouse { id } => { + granted_ws.insert(id.to_string()); } } } @@ -262,12 +262,12 @@ impl GrantObjectVisibilityChecker { false } - pub fn check_warehouse_visibility(&self, uid: &str) -> bool { + pub fn check_warehouse_visibility(&self, id: &str) -> bool { if self.granted_global_ws { return true; } - if self.granted_ws.contains(uid) { + if self.granted_ws.contains(id) { return true; } false From a6c818b6ed0be91c394141de528617a2958f51f3 Mon Sep 17 00:00:00 2001 From: TCeason Date: Thu, 20 Feb 2025 15:04:45 +0800 Subject: [PATCH 7/7] add CreateWarehouse in available_privileges_on_global --- src/meta/app/src/principal/user_privilege.rs | 4 +--- .../suites/base/05_ddl/05_0017_ddl_grant_role.test | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/meta/app/src/principal/user_privilege.rs b/src/meta/app/src/principal/user_privilege.rs index 9944a43424baf..3dd691f695dfc 100644 --- a/src/meta/app/src/principal/user_privilege.rs +++ b/src/meta/app/src/principal/user_privilege.rs @@ -207,9 +207,7 @@ impl UserPrivilegeSet { 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(false); - // TODO : The warehouse functionality is not yet fully integrated. Therefore, the CreateWarehouse permission must be granted separately - // If self-created user or configured user wants to create warehouse in system-manage cluster must execute: grant create warehouse on *.* to ; - let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask }); + let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask | CreateWarehouse }); (database_privs.privileges | privs | stage_privs_without_ownership.privileges 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 ea1b4652c48e0..5a8b5cfe9a4b9 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 @@ -256,6 +256,9 @@ create user u1 identified by '123'; statement error 2218 grant usage on warehouse a to u1; +statement ok +GRANT create warehouse on *.* to role 'role1'; + statement ok drop user if exists u1;