From 16c7d99963e0af88402cc215f847a97d04e44161 Mon Sep 17 00:00:00 2001 From: TCeason <33082201+TCeason@users.noreply.github.com> Date: Wed, 11 Sep 2024 00:30:21 +0800 Subject: [PATCH] refactor(query): first check privilege in SystemEngine get_full_data (#16421) * optimize(query): first check privilege in SystemEngine get_full_data * use mget_databases replace for --- Cargo.lock | 1 + src/query/catalog/src/catalog/interface.rs | 8 ++ .../src/catalogs/default/database_catalog.rs | 33 ++++++ .../src/catalogs/default/immutable_catalog.rs | 18 +++ .../src/catalogs/default/mutable_catalog.rs | 29 +++++ .../src/catalogs/default/session_catalog.rs | 9 ++ .../tests/it/sql/exec/get_table_bind_test.rs | 9 ++ .../it/storages/fuse/operations/commit.rs | 9 ++ .../storages/hive/hive/src/hive_catalog.rs | 11 ++ src/query/storages/iceberg/src/catalog.rs | 10 ++ .../storages/system/src/columns_table.rs | 63 ++++++++-- .../storages/system/src/databases_table.rs | 111 +++++++++++++----- src/query/storages/system/src/tables_table.rs | 54 +++++++-- src/query/users/Cargo.toml | 1 + src/query/users/src/visibility_checker.rs | 57 +++++++++ .../18_rbac/18_0013_column_privilege.result | 30 +++++ .../18_rbac/18_0013_column_privilege.sh | 52 ++++++++ 17 files changed, 454 insertions(+), 51 deletions(-) create mode 100644 tests/suites/0_stateless/18_rbac/18_0013_column_privilege.result create mode 100755 tests/suites/0_stateless/18_rbac/18_0013_column_privilege.sh diff --git a/Cargo.lock b/Cargo.lock index ebf72c82fba3..3c9cd71ff6e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4712,6 +4712,7 @@ dependencies = [ "databend-common-meta-store", "databend-common-meta-types", "enumflags2", + "itertools 0.13.0", "jwt-simple 0.11.9", "log", "p256 0.13.2", diff --git a/src/query/catalog/src/catalog/interface.rs b/src/query/catalog/src/catalog/interface.rs index 3dd5c050a6a0..ac4debc04eb8 100644 --- a/src/query/catalog/src/catalog/interface.rs +++ b/src/query/catalog/src/catalog/interface.rs @@ -19,6 +19,7 @@ use std::sync::Arc; use databend_common_config::InnerConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -223,6 +224,13 @@ pub trait Catalog: DynClone + Send + Sync + Debug { // Get the db name by meta id. async fn get_db_name_by_id(&self, db_ids: MetaId) -> Result; + // Mget dbs by DatabaseNameIdent. + async fn mget_databases( + &self, + tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>>; + // Mget the dbs name by meta ids. async fn mget_database_names_by_ids( &self, diff --git a/src/query/service/src/catalogs/default/database_catalog.rs b/src/query/service/src/catalogs/default/database_catalog.rs index c0f351168181..5cb096fb11dc 100644 --- a/src/query/service/src/catalogs/default/database_catalog.rs +++ b/src/query/service/src/catalogs/default/database_catalog.rs @@ -25,6 +25,7 @@ use databend_common_catalog::table_function::TableFunction; use databend_common_config::InnerConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -338,6 +339,38 @@ impl Catalog for DatabaseCatalog { } } + async fn mget_databases( + &self, + tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + let sys_dbs = self.immutable_catalog.list_databases(tenant).await?; + let sys_db_names: Vec<_> = sys_dbs + .iter() + .map(|sys_db| sys_db.get_db_info().name_ident.database_name()) + .collect(); + + let mut mut_db_names: Vec<_> = Vec::new(); + for db_name in db_names { + if !sys_db_names.contains(&db_name.database_name()) { + mut_db_names.push(db_name.clone()); + } + } + + let mut dbs = self + .immutable_catalog + .mget_databases(tenant, db_names) + .await?; + + let other = self + .mutable_catalog + .mget_databases(tenant, &mut_db_names) + .await?; + + dbs.extend(other); + Ok(dbs) + } + #[async_backtrace::framed] async fn mget_database_names_by_ids( &self, diff --git a/src/query/service/src/catalogs/default/immutable_catalog.rs b/src/query/service/src/catalogs/default/immutable_catalog.rs index f2800928756e..ae49d5f3690a 100644 --- a/src/query/service/src/catalogs/default/immutable_catalog.rs +++ b/src/query/service/src/catalogs/default/immutable_catalog.rs @@ -21,6 +21,7 @@ use databend_common_catalog::catalog::Catalog; use databend_common_config::InnerConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -232,6 +233,23 @@ impl Catalog for ImmutableCatalog { } } + async fn mget_databases( + &self, + _tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + let mut res: Vec> = vec![]; + for db_name in db_names { + let db_name = db_name.database_name(); + if db_name == "system" { + res.push(self.sys_db.clone()); + } else if db_name == "information_schema" { + res.push(self.info_schema_db.clone()); + } + } + Ok(res) + } + async fn mget_database_names_by_ids( &self, _tenant: &Tenant, diff --git a/src/query/service/src/catalogs/default/mutable_catalog.rs b/src/query/service/src/catalogs/default/mutable_catalog.rs index c7e3ad3de1f6..d1adbe2137c1 100644 --- a/src/query/service/src/catalogs/default/mutable_catalog.rs +++ b/src/query/service/src/catalogs/default/mutable_catalog.rs @@ -22,6 +22,7 @@ use databend_common_catalog::catalog::Catalog; use databend_common_config::InnerConfig; use databend_common_exception::Result; use databend_common_meta_api::kv_app_error::KVAppError; +use databend_common_meta_api::name_id_value_api::NameIdValueApiCompat; use databend_common_meta_api::SchemaApi; use databend_common_meta_api::SequenceApi; use databend_common_meta_app::app_error::AppError; @@ -422,6 +423,34 @@ impl Catalog for MutableCatalog { Ok(res) } + // Mget dbs by DatabaseNameIdent. + async fn mget_databases( + &self, + _tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + let res = self + .ctx + .meta + .mget_id_value_compat(db_names.iter().cloned()) + .await?; + let dbs = res + .map(|(name_ident, database_id, meta)| { + Arc::new(DatabaseInfo { + database_id, + name_ident, + meta, + }) + }) + .collect::>>(); + + dbs.iter().try_fold(vec![], |mut acc, item| { + let db = self.build_db_instance(item)?; + acc.push(db); + Ok(acc) + }) + } + async fn mget_database_names_by_ids( &self, _tenant: &Tenant, diff --git a/src/query/service/src/catalogs/default/session_catalog.rs b/src/query/service/src/catalogs/default/session_catalog.rs index e57b9d109f33..46f10275e701 100644 --- a/src/query/service/src/catalogs/default/session_catalog.rs +++ b/src/query/service/src/catalogs/default/session_catalog.rs @@ -23,6 +23,7 @@ use databend_common_catalog::table_args::TableArgs; use databend_common_catalog::table_function::TableFunction; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -305,6 +306,14 @@ impl Catalog for SessionCatalog { self.inner.get_db_name_by_id(db_id).await } + async fn mget_databases( + &self, + tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + self.inner.mget_databases(tenant, db_names).await + } + // Mget the dbs name by meta ids. async fn mget_database_names_by_ids( &self, diff --git a/src/query/service/tests/it/sql/exec/get_table_bind_test.rs b/src/query/service/tests/it/sql/exec/get_table_bind_test.rs index 873abc01fdf1..cd6f3cd9e7a4 100644 --- a/src/query/service/tests/it/sql/exec/get_table_bind_test.rs +++ b/src/query/service/tests/it/sql/exec/get_table_bind_test.rs @@ -55,6 +55,7 @@ use databend_common_meta_app::principal::RoleInfo; use databend_common_meta_app::principal::UserDefinedConnection; use databend_common_meta_app::principal::UserInfo; use databend_common_meta_app::principal::UserPrivilegeType; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -208,6 +209,14 @@ impl Catalog for FakedCatalog { self.cat.get_db_name_by_id(db_id).await } + async fn mget_databases( + &self, + tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + self.cat.mget_databases(tenant, db_names).await + } + async fn mget_database_names_by_ids( &self, tenant: &Tenant, diff --git a/src/query/service/tests/it/storages/fuse/operations/commit.rs b/src/query/service/tests/it/storages/fuse/operations/commit.rs index 735f0963741f..7acaf862daa2 100644 --- a/src/query/service/tests/it/storages/fuse/operations/commit.rs +++ b/src/query/service/tests/it/storages/fuse/operations/commit.rs @@ -54,6 +54,7 @@ use databend_common_meta_app::principal::RoleInfo; use databend_common_meta_app::principal::UserDefinedConnection; use databend_common_meta_app::principal::UserInfo; use databend_common_meta_app::principal::UserPrivilegeType; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CommitTableMetaReply; @@ -957,6 +958,14 @@ impl Catalog for FakedCatalog { self.cat.get_db_name_by_id(db_id).await } + async fn mget_databases( + &self, + tenant: &Tenant, + db_names: &[DatabaseNameIdent], + ) -> Result>> { + self.cat.mget_databases(tenant, db_names).await + } + #[async_backtrace::framed] async fn mget_database_names_by_ids( &self, diff --git a/src/query/storages/hive/hive/src/hive_catalog.rs b/src/query/storages/hive/hive/src/hive_catalog.rs index 944a03e8871a..d50db4340e97 100644 --- a/src/query/storages/hive/hive/src/hive_catalog.rs +++ b/src/query/storages/hive/hive/src/hive_catalog.rs @@ -28,6 +28,7 @@ use databend_common_catalog::table_function::TableFunction; use databend_common_config::InnerConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CatalogOption; @@ -385,6 +386,16 @@ impl Catalog for HiveCatalog { )) } + async fn mget_databases( + &self, + _tenant: &Tenant, + _db_names: &[DatabaseNameIdent], + ) -> Result>> { + Err(ErrorCode::Unimplemented( + "Cannot mget databases in HIVE catalog", + )) + } + async fn mget_database_names_by_ids( &self, _tenant: &Tenant, diff --git a/src/query/storages/iceberg/src/catalog.rs b/src/query/storages/iceberg/src/catalog.rs index 90ed0b3ac17e..837f64432a7f 100644 --- a/src/query/storages/iceberg/src/catalog.rs +++ b/src/query/storages/iceberg/src/catalog.rs @@ -26,6 +26,7 @@ use databend_common_catalog::table_function::TableFunction; use databend_common_config::InnerConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::dictionary_name_ident::DictionaryNameIdent; use databend_common_meta_app::schema::CatalogInfo; use databend_common_meta_app::schema::CatalogOption; @@ -295,6 +296,15 @@ impl Catalog for IcebergCatalog { "Cannot get db name by id in ICEBERG catalog", )) } + async fn mget_databases( + &self, + _tenant: &Tenant, + _db_names: &[DatabaseNameIdent], + ) -> Result>> { + Err(ErrorCode::Unimplemented( + "Cannot mget databases in ICEBERG catalog", + )) + } async fn mget_database_names_by_ids( &self, diff --git a/src/query/storages/system/src/columns_table.rs b/src/query/storages/system/src/columns_table.rs index e3357ba3b135..cecd2c0518df 100644 --- a/src/query/storages/system/src/columns_table.rs +++ b/src/query/storages/system/src/columns_table.rs @@ -28,6 +28,7 @@ use databend_common_expression::TableDataType; use databend_common_expression::TableField; use databend_common_expression::TableSchemaRefExt; use databend_common_functions::BUILTIN_FUNCTIONS; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::TableIdent; use databend_common_meta_app::schema::TableInfo; use databend_common_meta_app::schema::TableMeta; @@ -277,16 +278,7 @@ pub(crate) async fn dump_tables( let mut final_dbs: Vec<(String, u64)> = Vec::new(); - if databases.is_empty() { - let all_databases = catalog.list_databases(&tenant).await?; - for db in all_databases { - let db_id = db.get_db_info().database_id.db_id; - let db_name = db.name(); - if visibility_checker.check_database_visibility(CATALOG_DEFAULT, db_name, db_id) { - final_dbs.push((db_name.to_string(), db_id)); - } - } - } else { + if !databases.is_empty() { for db in databases { let db_id = catalog .get_database(&tenant, &db) @@ -298,12 +290,61 @@ pub(crate) async fn dump_tables( final_dbs.push((db.to_string(), db_id)); } } + } else { + let catalog_dbs = visibility_checker.get_visibility_database(); + // None means has global level privileges + if let Some(catalog_dbs) = catalog_dbs { + for (catalog_name, dbs) in catalog_dbs { + if catalog_name == CATALOG_DEFAULT { + let mut catalog_db_ids = vec![]; + let mut catalog_db_names = vec![]; + catalog_db_names.extend( + dbs.iter() + .filter_map(|(db_name, _)| *db_name) + .map(|db_name| db_name.to_string()), + ); + catalog_db_ids.extend(dbs.iter().filter_map(|(_, db_id)| *db_id)); + if let Ok(databases) = catalog + .mget_database_names_by_ids(&tenant, &catalog_db_ids) + .await + { + catalog_db_names.extend(databases.into_iter().flatten()); + } else { + let msg = format!("Failed to get database name by id: {}", catalog.name()); + warn!("{}", msg); + } + let db_idents = catalog_db_names + .iter() + .map(|name| DatabaseNameIdent::new(&tenant, name)) + .collect::>(); + let dbs: Vec<(String, u64)> = catalog + .mget_databases(&tenant, &db_idents) + .await? + .iter() + .map(|db| (db.name().to_string(), db.get_db_info().database_id.db_id)) + .collect(); + final_dbs.extend(dbs); + } + } + } else { + let all_databases = catalog.list_databases(&tenant).await?; + for db in all_databases { + let db_id = db.get_db_info().database_id.db_id; + let db_name = db.name(); + if visibility_checker.check_database_visibility(CATALOG_DEFAULT, db_name, db_id) { + final_dbs.push((db_name.to_string(), db_id)); + } + } + } } let mut final_tables: Vec<(String, Vec>)> = Vec::with_capacity(final_dbs.len()); for (database, db_id) in final_dbs { let tables = if tables.is_empty() { - (catalog.list_tables(&tenant, &database).await).unwrap_or_default() + catalog + .list_tables(&tenant, &database) + .await + .unwrap_or_default() } else { let mut res = Vec::new(); for table in &tables { diff --git a/src/query/storages/system/src/databases_table.rs b/src/query/storages/system/src/databases_table.rs index 64f8129bb02e..cfebe2c1b97f 100644 --- a/src/query/storages/system/src/databases_table.rs +++ b/src/query/storages/system/src/databases_table.rs @@ -29,10 +29,12 @@ use databend_common_expression::TableDataType; use databend_common_expression::TableField; use databend_common_expression::TableSchemaRefExt; use databend_common_meta_app::principal::OwnershipObject; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::TableIdent; use databend_common_meta_app::schema::TableInfo; use databend_common_meta_app::schema::TableMeta; use databend_common_users::UserApiProvider; +use log::warn; use crate::table::AsyncOneBlockSystemTable; use crate::table::AsyncSystemTable; @@ -68,47 +70,94 @@ impl AsyncSystemTable for DatabasesTable { let user_api = UserApiProvider::instance(); let mut catalog_names = vec![]; let mut db_names = vec![]; - let mut db_id = vec![]; + let mut db_ids = vec![]; let mut owners: Vec> = vec![]; let visibility_checker = ctx.get_visibility_checker().await?; - - for (ctl_name, catalog) in catalogs.into_iter() { - let databases = catalog.list_databases(&tenant).await?; - let final_dbs = databases - .into_iter() - .filter(|db| { - visibility_checker.check_database_visibility( - &ctl_name, - db.name(), - db.get_db_info().database_id.db_id, - ) - }) - .collect::>(); - - for db in final_dbs { - catalog_names.push(ctl_name.clone()); - let db_name = db.name().to_string(); - db_names.push(db_name); - let id = db.get_db_info().database_id.db_id; - db_id.push(id); - owners.push( - user_api - .get_ownership(&tenant, &OwnershipObject::Database { - catalog_name: ctl_name.to_string(), - db_id: id, - }) - .await - .ok() - .and_then(|ownership| ownership.map(|o| o.role.clone())), + let catalog_dbs = visibility_checker.get_visibility_database(); + // None means has global level privileges + if let Some(catalog_dbs) = catalog_dbs { + for (catalog, dbs) in catalog_dbs { + let mut catalog_db_ids = vec![]; + let mut catalog_db_names = vec![]; + let ctl = ctx.get_catalog(catalog).await?; + catalog_db_names.extend( + dbs.iter() + .filter_map(|(db_name, _)| *db_name) + .map(|db_name| db_name.to_string()), ); + catalog_db_ids.extend(dbs.iter().filter_map(|(_, db_id)| *db_id)); + + if let Ok(databases) = ctl + .mget_database_names_by_ids(&tenant, &catalog_db_ids) + .await + { + catalog_db_names.extend(databases.into_iter().flatten()); + } else { + let msg = format!("Failed to get database name by id: {}", ctl.name()); + warn!("{}", msg); + } + + let db_idents = catalog_db_names + .iter() + .map(|name| DatabaseNameIdent::new(&tenant, name)) + .collect::>(); + let dbs = ctl.mget_databases(&tenant, &db_idents).await?; + for db in dbs { + catalog_names.push(catalog.clone()); + db_names.push(db.get_db_info().name_ident.database_name().to_string()); + let db_id = db.get_db_info().database_id.db_id; + db_ids.push(db_id); + owners.push( + user_api + .get_ownership(&tenant, &OwnershipObject::Database { + catalog_name: catalog.to_string(), + db_id, + }) + .await + .ok() + .and_then(|ownership| ownership.map(|o| o.role.clone())), + ); + } + } + } else { + for (ctl_name, catalog) in catalogs.into_iter() { + let databases = catalog.list_databases(&tenant).await?; + let final_dbs = databases + .into_iter() + .filter(|db| { + visibility_checker.check_database_visibility( + &ctl_name, + db.name(), + db.get_db_info().database_id.db_id, + ) + }) + .collect::>(); + + for db in final_dbs { + catalog_names.push(ctl_name.clone()); + let db_name = db.name().to_string(); + db_names.push(db_name); + let id = db.get_db_info().database_id.db_id; + db_ids.push(id); + owners.push( + user_api + .get_ownership(&tenant, &OwnershipObject::Database { + catalog_name: ctl_name.to_string(), + db_id: id, + }) + .await + .ok() + .and_then(|ownership| ownership.map(|o| o.role.clone())), + ); + } } } Ok(DataBlock::new_from_columns(vec![ StringType::from_data(catalog_names), StringType::from_data(db_names), - UInt64Type::from_data(db_id), + UInt64Type::from_data(db_ids), StringType::from_opt_data(owners), ])) } diff --git a/src/query/storages/system/src/tables_table.rs b/src/query/storages/system/src/tables_table.rs index da2c547b02b1..0fd595579d54 100644 --- a/src/query/storages/system/src/tables_table.rs +++ b/src/query/storages/system/src/tables_table.rs @@ -39,6 +39,7 @@ use databend_common_expression::TableSchemaRef; use databend_common_expression::TableSchemaRefExt; use databend_common_functions::BUILTIN_FUNCTIONS; use databend_common_meta_app::principal::OwnershipObject; +use databend_common_meta_app::schema::database_name_ident::DatabaseNameIdent; use databend_common_meta_app::schema::TableIdent; use databend_common_meta_app::schema::TableInfo; use databend_common_meta_app::schema::TableMeta; @@ -323,6 +324,7 @@ where TablesTable: HistoryAware ); } } + let catalog_dbs = visibility_checker.get_visibility_database(); for (ctl_name, ctl) in ctls.iter() { if let Some(push_downs) = &push_downs { @@ -356,15 +358,49 @@ where TablesTable: HistoryAware } if dbs.is_empty() || invalid_optimize { - dbs = match ctl.list_databases(&tenant).await { - Ok(dbs) => dbs, - Err(err) => { - let msg = - format!("List databases failed on catalog {}: {}", ctl.name(), err); - warn!("{}", msg); - ctx.push_warning(msg); - - vec![] + // None means has global level privileges + dbs = if let Some(catalog_dbs) = &catalog_dbs { + let mut final_dbs = vec![]; + for (catalog_name, dbs) in catalog_dbs { + if ctl.name() == catalog_name.to_string() { + let mut catalog_db_ids = vec![]; + let mut catalog_db_names = vec![]; + catalog_db_names.extend( + dbs.iter() + .filter_map(|(db_name, _)| *db_name) + .map(|db_name| db_name.to_string()), + ); + catalog_db_ids.extend(dbs.iter().filter_map(|(_, db_id)| *db_id)); + if let Ok(databases) = ctl + .mget_database_names_by_ids(&tenant, &catalog_db_ids) + .await + { + catalog_db_names.extend(databases.into_iter().flatten()); + } else { + let msg = + format!("Failed to get database name by id: {}", ctl.name()); + warn!("{}", msg); + } + let db_idents = catalog_db_names + .iter() + .map(|name| DatabaseNameIdent::new(&tenant, name)) + .collect::>(); + let dbs = ctl.mget_databases(&tenant, &db_idents).await?; + final_dbs.extend(dbs); + } + } + final_dbs + } else { + match ctl.list_databases(&tenant).await { + Ok(dbs) => dbs, + Err(err) => { + let msg = + format!("List databases failed on catalog {}: {}", ctl.name(), err); + warn!("{}", msg); + ctx.push_warning(msg); + + vec![] + } } } } diff --git a/src/query/users/Cargo.toml b/src/query/users/Cargo.toml index 2061384c8be6..cb3743fe2ca7 100644 --- a/src/query/users/Cargo.toml +++ b/src/query/users/Cargo.toml @@ -30,6 +30,7 @@ databend-common-meta-kvapi = { workspace = true } databend-common-meta-store = { workspace = true } databend-common-meta-types = { workspace = true } enumflags2 = { workspace = true } +itertools = "0.13.0" jwt-simple = "0.11" log = { workspace = true } p256 = "0.13" diff --git a/src/query/users/src/visibility_checker.rs b/src/query/users/src/visibility_checker.rs index 22d6c6ab9ca1..898ccd839bd2 100644 --- a/src/query/users/src/visibility_checker.rs +++ b/src/query/users/src/visibility_checker.rs @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashMap; use std::collections::HashSet; +use std::string::ToString; use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OwnershipObject; @@ -22,6 +24,7 @@ use databend_common_meta_app::principal::UserInfo; use databend_common_meta_app::principal::UserPrivilegeSet; use databend_common_meta_app::principal::UserPrivilegeType; use enumflags2::BitFlags; +use itertools::Itertools; /// GrantObjectVisibilityChecker is used to check whether a user has the privilege to access a /// database or table. @@ -36,6 +39,7 @@ pub struct GrantObjectVisibilityChecker { granted_tables: HashSet<(String, String, String)>, granted_tables_id: HashSet<(String, u64, u64)>, extra_databases: HashSet<(String, String)>, + sys_databases: HashSet<(String, String)>, extra_databases_id: HashSet<(String, u64)>, granted_udfs: HashSet, granted_write_stages: HashSet, @@ -197,6 +201,10 @@ impl GrantObjectVisibilityChecker { granted_udfs, granted_write_stages, granted_read_stages, + sys_databases: HashSet::from([ + ("default".to_string(), "information_schema".to_string()), + ("default".to_string(), "system".to_string()), + ]), } } @@ -324,4 +332,53 @@ impl GrantObjectVisibilityChecker { false } + + #[allow(clippy::type_complexity)] + pub fn get_visibility_database( + &self, + ) -> Option, Option<&u64>)>>> { + if self.granted_global_db_table { + return None; + } + + let capacity = self.granted_databases.len() + + self.granted_databases_id.len() + + self.extra_databases.len() + + self.extra_databases_id.len() + + self.sys_databases.len(); + + let dbs = self + .granted_databases + .iter() + .map(|(catalog, db)| (catalog, (Some(db), None))) + .chain( + self.granted_databases_id + .iter() + .map(|(catalog, db_id)| (catalog, (None, Some(db_id)))), + ) + .chain( + self.extra_databases + .iter() + .map(|(catalog, db)| (catalog, (Some(db), None))), + ) + .chain( + self.sys_databases + .iter() + .map(|(catalog, db)| (catalog, (Some(db), None))), + ) + .chain( + self.extra_databases_id + .iter() + .map(|(catalog, db_id)| (catalog, (None, Some(db_id)))), + ) + .into_grouping_map() + .fold( + HashSet::with_capacity(capacity / 4), + |mut set, _key, value| { + set.insert(value); + set + }, + ); + Some(dbs) + } } diff --git a/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.result b/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.result new file mode 100644 index 000000000000..0e6403f6038f --- /dev/null +++ b/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.result @@ -0,0 +1,30 @@ +=== show grants for a === +SELECT default USER a GRANT SELECT ON 'default'.'default'.* TO 'a'@'%' +SELECT default.grant_db.t USER a GRANT SELECT ON 'default'.'grant_db'.'t' TO 'a'@'%' +=== show databases === +default +grant_db +information_schema +system +=== show tables === +Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for database 'system' +t +=== use db === +Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for database 'system' +=== show columns === +dummy TINYINT UNSIGNED NO NULL NULL +c1 INT NO NULL NULL +created_on TIMESTAMP NO NULL NULL +inherited_roles BIGINT UNSIGNED NO NULL NULL +inherited_roles_name VARCHAR NO NULL NULL +name VARCHAR NO NULL NULL +update_on TIMESTAMP NO NULL NULL +keywords VARCHAR NO NULL NULL +reserved TINYINT UNSIGNED NO NULL NULL +Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for database 'nogrant' +Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for table 'nogrant.t' +=== grant system to a === +0 +0 +1 +0 diff --git a/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.sh b/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.sh new file mode 100755 index 000000000000..482999a37cda --- /dev/null +++ b/tests/suites/0_stateless/18_rbac/18_0013_column_privilege.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. "$CURDIR"/../../../shell_env.sh + + +export TEST_USER_PASSWORD="password" +export USER_A_CONNECT="bendsql --user=a --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" + + +echo "drop user if exists a" | $BENDSQL_CLIENT_CONNECT +echo "create user a identified by '$TEST_USER_PASSWORD'" | $BENDSQL_CLIENT_CONNECT +echo "create or replace database grant_db" | $BENDSQL_CLIENT_CONNECT +echo "create table grant_db.t(c1 int not null)" | $BENDSQL_CLIENT_CONNECT +echo "create or replace database nogrant" | $BENDSQL_CLIENT_CONNECT +echo "create table nogrant.t(id int not null)" | $BENDSQL_CLIENT_CONNECT +echo "grant select on default.* to a" | $BENDSQL_CLIENT_CONNECT +echo "grant select on grant_db.t to a" | $BENDSQL_CLIENT_CONNECT +echo "create or replace table default.test_t(id int not null)" | $BENDSQL_CLIENT_CONNECT + +echo "=== show grants for a ===" +echo "show grants for a" | $BENDSQL_CLIENT_CONNECT | awk -F ' ' '{$3=""; print $0}' +echo "=== show databases ===" +echo "show databases" | $USER_A_CONNECT + +echo "=== show tables ===" +echo "show tables from system" | $USER_A_CONNECT +echo "show tables from grant_db" | $USER_A_CONNECT +echo "=== use db ===" +echo "use system" | $USER_A_CONNECT +echo "use information_schema" | $USER_A_CONNECT +echo "use grant_db" | $USER_A_CONNECT +echo "=== show columns ===" +echo "show columns from one from system" | $USER_A_CONNECT +echo "show columns from t from grant_db" | $USER_A_CONNECT +echo "show columns from roles from system" | $USER_A_CONNECT +echo "show columns from keywords from information_schema" | $USER_A_CONNECT +echo "show tables from nogrant" | $USER_A_CONNECT +echo "show columns from t from nogrant" | $USER_A_CONNECT + +echo "=== grant system to a ===" +echo "grant select on system.* to a" | $BENDSQL_CLIENT_CONNECT +echo "show tables from system" | $USER_A_CONNECT | echo $? +echo "use system" | $USER_A_CONNECT | echo $? + +echo "select count(1) from information_schema.columns where table_schema in ('grant_db');" | $USER_A_CONNECT +echo "select count(1) from information_schema.columns where table_schema in ('nogrant');" | $USER_A_CONNECT + +echo "drop database nogrant" | $BENDSQL_CLIENT_CONNECT +echo "drop database grant_db" | $BENDSQL_CLIENT_CONNECT +echo "drop table default.test_t" | $BENDSQL_CLIENT_CONNECT +echo "drop user a" | $BENDSQL_CLIENT_CONNECT