From 3fdf3536db7cce34b82790cf2325c40314c697aa Mon Sep 17 00:00:00 2001 From: flaneur Date: Thu, 28 Dec 2023 22:48:15 +0800 Subject: [PATCH] feat(rbac): treat the empty ownership as owned by ACCOUNT_ADMIN instead of PUBLIC (#14112) * default as account_admin * fix cargo check * allow list in system tables * tune comments * remove verify_ownership parameter in validate_access_db and _table * fix typo * fix lint * add test for rewritten * add stateless tests * fix stateless error * fix set_role * add result file * rename validate_ownership to has_ownership * fix typo --- .../interpreters/access/privilege_access.rs | 197 ++++++++---------- src/query/service/src/sessions/session.rs | 6 +- .../src/sessions/session_privilege_mgr.rs | 24 +-- .../18_rbac/20_0012_privilege_access.result | 5 +- .../18_rbac/20_0012_privilege_access.sh | 13 +- .../0_stateless/18_rbac/20_0015_set_role.sh | 8 +- .../18_rbac/20_0016_rewrite_statements.result | 3 + .../18_rbac/20_0016_rewrite_statements.sh | 23 ++ 8 files changed, 139 insertions(+), 140 deletions(-) create mode 100644 tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.result create mode 100755 tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.sh diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index 2fffe7142c64..19b20f071960 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -44,6 +44,25 @@ enum ObjectId { Table(u64, u64), } +// some statements like `SELECT 1`, `SHOW USERS`, `SHOW ROLES`, `SHOW TABLES` will be +// rewritten to the queries on the system tables, we need to skip the privilege check on +// these tables. +const SYSTEM_TABLES_ALLOW_LIST: [&str; 13] = [ + "catalogs", + "columns", + "databases", + "tables", + "tables_with_history", + "password_policies", + "streams", + "virtual_columns", + "users", + "roles", + "stages", + "one", + "processes", +]; + impl PrivilegeAccess { pub fn create(ctx: Arc) -> Box { Box::new(PrivilegeAccess { ctx }) @@ -117,7 +136,6 @@ impl PrivilegeAccess { catalog_name: &str, db_name: &str, privileges: Vec, - verify_ownership: bool, ) -> Result<()> { let tenant = self.ctx.get_tenant(); @@ -125,7 +143,7 @@ impl PrivilegeAccess { .validate_access( &GrantObject::Database(catalog_name.to_string(), db_name.to_string()), privileges.clone(), - verify_ownership, + true, ) .await { @@ -143,7 +161,7 @@ impl PrivilegeAccess { self.validate_access( &GrantObject::DatabaseById(catalog_name.to_string(), db_id), privileges, - verify_ownership, + true, ) .await? } @@ -157,7 +175,6 @@ impl PrivilegeAccess { db_name: &str, table_name: &str, privileges: Vec, - verify_ownership: bool, ) -> Result<()> { let tenant = self.ctx.get_tenant(); @@ -176,7 +193,7 @@ impl PrivilegeAccess { table_name.to_string(), ), privileges.clone(), - verify_ownership, + true, ) .await { @@ -194,7 +211,7 @@ impl PrivilegeAccess { self.validate_access( &GrantObject::TableById(catalog_name.to_string(), db_id, table_id.unwrap()), privileges, - verify_ownership, + true, ) .await? } @@ -209,57 +226,37 @@ impl PrivilegeAccess { verify_ownership: bool, ) -> Result<()> { let session = self.ctx.get_current_session(); - let len = privileges.len(); - let mut db_name = String::new(); - let mut table_name = String::new(); - - match object { - GrantObject::Database(_, db_name) => { - if db_name.to_lowercase() == "information_schema" - && privileges.contains(&UserPrivilegeType::Select) - && len == 1 - { - return Ok(()); - } - } + // translate the db_id and table_id to db_name and table_name, used on + // skipping the privilege check on system tables, and on the error message. + let (db_name, table_name) = match object { + GrantObject::Database(_, db_name) => (db_name.to_lowercase(), "".to_string()), GrantObject::Table(_, db_name, table_name) => { - let db_name = db_name.to_lowercase(); - let table_name = table_name.to_lowercase(); - if (db_name.to_lowercase() == "information_schema" - || (db_name == "system" && table_name == "one")) - && privileges.contains(&UserPrivilegeType::Select) - && len == 1 - { - return Ok(()); - } + (db_name.to_lowercase(), table_name.to_lowercase()) } GrantObject::DatabaseById(catalog_name, db_id) => { let catalog = self.ctx.get_catalog(catalog_name).await?; - db_name = catalog.get_db_name_by_id(*db_id).await?.to_lowercase(); - if db_name == "information_schema" - && privileges.contains(&UserPrivilegeType::Select) - && len == 1 - { - return Ok(()); - } + let db_name = catalog.get_db_name_by_id(*db_id).await?; + (db_name.to_lowercase(), "".to_string()) } GrantObject::TableById(catalog_name, db_id, table_id) => { let catalog = self.ctx.get_catalog(catalog_name).await?; - db_name = catalog.get_db_name_by_id(*db_id).await?.to_lowercase(); - table_name = catalog - .get_table_name_by_id(*table_id) - .await? - .to_lowercase(); - if (db_name == "information_schema" || (db_name == "system" && table_name == "one")) - && privileges.contains(&UserPrivilegeType::Select) - && len == 1 - { - return Ok(()); - } + let db_name = catalog.get_db_name_by_id(*db_id).await?; + let table_name = catalog.get_table_name_by_id(*table_id).await?; + (db_name.to_lowercase(), table_name.to_lowercase()) } - GrantObject::Global | GrantObject::Stage(_) | GrantObject::UDF(_) => {} + _ => ("".to_string(), "".to_string()), + }; + + // skip checking the privilege on system tables. + if ((db_name == "system" && SYSTEM_TABLES_ALLOW_LIST.iter().any(|x| x == &table_name)) + || db_name == "information_schema") + && privileges == [UserPrivilegeType::Select] + { + return Ok(()); } + + // TODO: remove the verify_ownership parameter if verify_ownership { let object_by_id = self.convert_grant_object_by_id(object) @@ -271,13 +268,14 @@ impl PrivilegeAccess { _ => Err(e.add_message("error on validating access")), })?; if let Some(object_by_id) = &object_by_id { - let result = session.validate_ownership(object_by_id).await; - if result.is_ok() { + let ok = session.has_ownership(object_by_id).await?; + if ok { return Ok(()); } } } - // wrap an user-facing error message on privilege validations on cases like TableByID / DatabaseByID + + // wrap an user-facing error message with table/db names on cases like TableByID / DatabaseByID match session.validate_privilege(object, privileges.clone()).await { Ok(_) => Ok(()), Err(err) => { @@ -519,7 +517,7 @@ impl AccessChecker for PrivilegeAccess { // like this sql: copy into t from (select * from @s3); will bind a mock table with name `system.read_parquet(s3)` // this is no means to check table `system.read_parquet(s3)` privilege if !table.is_source_of_stage() { - self.validate_table_access(catalog_name, table.database(), table.name(), vec![UserPrivilegeType::Select], true).await? + self.validate_table_access(catalog_name, table.database(), table.name(), vec![UserPrivilegeType::Select]).await? } } } @@ -529,8 +527,7 @@ impl AccessChecker for PrivilegeAccess { // Database. Plan::ShowCreateDatabase(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Select], - true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Select]).await? } Plan::CreateUDF(_) | Plan::CreateDatabase(_) | Plan::CreateIndex(_) => { self.validate_access(&GrantObject::Global, vec![UserPrivilegeType::Create], true) @@ -564,76 +561,64 @@ impl AccessChecker for PrivilegeAccess { // Virtual Column. Plan::CreateVirtualColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Create], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Create]).await? } Plan::AlterVirtualColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::DropVirtualColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Drop], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Drop]).await? } Plan::RefreshVirtualColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super]).await? } // Table. Plan::ShowCreateTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Select], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Select]).await? } Plan::DescribeTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Select], false).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Select]).await? } Plan::CreateTable(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create], - true).await?; + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create]).await?; if let Some(query) = &plan.as_select { self.check(ctx, query).await?; } } Plan::DropTable(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop], - true).await?; + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop]).await?; } Plan::UndropTable(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop], - true).await?; + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop]).await?; } Plan::RenameTable(plan) => { // You must have ALTER and DROP privileges for the original table, // and CREATE for the new db. - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter, UserPrivilegeType::Drop], - true).await?; - self.validate_db_access(&plan.catalog, &plan.new_database, vec![UserPrivilegeType::Create], - true).await?; + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter, UserPrivilegeType::Drop]).await?; + self.validate_db_access(&plan.catalog, &plan.new_database, vec![UserPrivilegeType::Create]).await?; } Plan::SetOptions(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::AddTableColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::RenameTableColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::ModifyTableColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::DropTableColumn(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::AlterTableClusterKey(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::DropTableClusterKey(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Drop], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Drop]).await? } Plan::ReclusterTable(plan) => { if enable_experimental_rbac_check { @@ -642,33 +627,26 @@ impl AccessChecker for PrivilegeAccess { self.check_udf_priv(udf).await?; } } - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Alter]).await? } Plan::TruncateTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Delete], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Delete]).await? } Plan::OptimizeTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super]).await? } Plan::VacuumTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super]).await? } Plan::VacuumDropTable(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Super], - true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Super]).await? } Plan::AnalyzeTable(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super], - true).await? + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Super]).await? } // Others. Plan::Insert(plan) => { - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert], - true).await?; + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert]).await?; match &plan.source { InsertInputSource::SelectPlan(plan) => { self.check(ctx, plan).await?; @@ -683,8 +661,7 @@ impl AccessChecker for PrivilegeAccess { } Plan::Replace(plan) => { //plan.delete_when is Expr no need to check privileges. - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert, UserPrivilegeType::Delete], - true).await?; + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert, UserPrivilegeType::Delete]).await?; match &plan.source { InsertInputSource::SelectPlan(plan) => { self.check(ctx, plan).await?; @@ -741,8 +718,7 @@ impl AccessChecker for PrivilegeAccess { } } } - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert, UserPrivilegeType::Delete], - true).await?; + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Insert, UserPrivilegeType::Delete]).await?; } Plan::Delete(plan) => { if enable_experimental_rbac_check { @@ -769,8 +745,7 @@ impl AccessChecker for PrivilegeAccess { } } } - self.validate_table_access(&plan.catalog_name, &plan.database_name, &plan.table_name, vec![UserPrivilegeType::Delete], - true).await? + self.validate_table_access(&plan.catalog_name, &plan.database_name, &plan.table_name, vec![UserPrivilegeType::Delete]).await? } Plan::Update(plan) => { if enable_experimental_rbac_check { @@ -801,23 +776,22 @@ impl AccessChecker for PrivilegeAccess { } } } - self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Update], - true).await?; + self.validate_table_access(&plan.catalog, &plan.database, &plan.table, vec![UserPrivilegeType::Update]).await?; } Plan::CreateView(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create], true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create]).await? } Plan::AlterView(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Alter], true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Alter]).await? } Plan::DropView(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop], true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop]).await? } Plan::CreateStream(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create], true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Create]).await? } Plan::DropStream(plan) => { - self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop], true).await? + self.validate_db_access(&plan.catalog, &plan.database, vec![UserPrivilegeType::Drop]).await? } Plan::CreateUser(_) => { self.validate_access( @@ -878,8 +852,7 @@ impl AccessChecker for PrivilegeAccess { } Plan::CopyIntoTable(plan) => { self.validate_access_stage(&plan.stage_table_info.stage_info, UserPrivilegeType::Read).await?; - self.validate_table_access(plan.catalog_info.catalog_name(), &plan.database_name, &plan.table_name, vec![UserPrivilegeType::Insert], - true).await?; + self.validate_table_access(plan.catalog_info.catalog_name(), &plan.database_name, &plan.table_name, vec![UserPrivilegeType::Insert]).await?; if let Some(query) = &plan.query { self.check(ctx, query).await?; } diff --git a/src/query/service/src/sessions/session.rs b/src/query/service/src/sessions/session.rs index ede848f4d35f..cb3f2571f741 100644 --- a/src/query/service/src/sessions/session.rs +++ b/src/query/service/src/sessions/session.rs @@ -260,11 +260,11 @@ impl Session { } #[async_backtrace::framed] - pub async fn validate_ownership(self: &Arc, object: &GrantObjectByID) -> Result<()> { + pub async fn has_ownership(self: &Arc, object: &GrantObjectByID) -> Result { if matches!(self.get_type(), SessionType::Local) { - return Ok(()); + return Ok(true); } - self.privilege_mgr.validate_ownership(object).await + self.privilege_mgr.has_ownership(object).await } #[async_backtrace::framed] diff --git a/src/query/service/src/sessions/session_privilege_mgr.rs b/src/query/service/src/sessions/session_privilege_mgr.rs index e763db9d0d9e..6ae669da5d96 100644 --- a/src/query/service/src/sessions/session_privilege_mgr.rs +++ b/src/query/service/src/sessions/session_privilege_mgr.rs @@ -23,6 +23,7 @@ use databend_common_meta_app::principal::UserInfo; use databend_common_meta_app::principal::UserPrivilegeType; use databend_common_users::GrantObjectVisibilityChecker; use databend_common_users::RoleCacheManager; +use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN; use databend_common_users::BUILTIN_ROLE_PUBLIC; use crate::sessions::SessionContext; @@ -63,7 +64,7 @@ pub trait SessionPrivilegeManager { privilege: Vec, ) -> Result<()>; - async fn validate_ownership(&self, object: &GrantObjectByID) -> Result<()>; + async fn has_ownership(&self, object: &GrantObjectByID) -> Result; async fn validate_available_role(&self, role_name: &str) -> Result; @@ -265,25 +266,20 @@ impl SessionPrivilegeManager for SessionPrivilegeManagerImpl { } #[async_backtrace::framed] - async fn validate_ownership(&self, object: &GrantObjectByID) -> Result<()> { + async fn has_ownership(&self, object: &GrantObjectByID) -> Result { let role_mgr = RoleCacheManager::instance(); let tenant = self.session_ctx.get_current_tenant(); - // if the object is not owned by any role, then considered as PUBLIC, which is always true - let owner_role = match role_mgr.find_object_owner(&tenant, object).await? { - Some(owner_role) => owner_role, - None => return Ok(()), + // if the object is not owned by any role, then considered as ACCOUNT_ADMIN, the normal users + // can not access it unless ACCOUNT_ADMIN grant relevant privileges to them. + let owner_role_name = match role_mgr.find_object_owner(&tenant, object).await? { + Some(owner_role) => owner_role.name, + None => BUILTIN_ROLE_ACCOUNT_ADMIN.to_string(), }; let available_roles = self.get_all_available_roles().await?; - if !available_roles.iter().any(|r| r.name == owner_role.name) { - return Err(ErrorCode::PermissionDenied( - "Permission denied, current session do not have the ownership of this object" - .to_string(), - )); - } - - Ok(()) + let exists = available_roles.iter().any(|r| r.name == owner_role_name); + return Ok(exists); } #[async_backtrace::framed] diff --git a/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.result b/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.result index 2bf521980db3..1b56ef121428 100644 --- a/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.result +++ b/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.result @@ -18,7 +18,8 @@ test -- select 1 1 1 -1 +test -- select view +Error: APIError: ResponseError with 1063: Permission denied, privilege [Select] is required on 'default'.'default2'.'v_t20_0012' for user 'test-user'@'%' with roles [public,test-role1,test-role2] 1 test -- clustering_information true @@ -30,8 +31,6 @@ GRANT SELECT ON 'default'.'grant_db'.'t' TO 'a'@'%' default grant_db information_schema -test -- show tables -test_t test -- show tables from system Error: APIError: ResponseError with 1063: Permission denied: User 'a'@'%' does not have the required privileges for database 'system'. test -- show tables from grant_db diff --git a/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.sh b/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.sh index edade616c987..e1a6e0c977ab 100755 --- a/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.sh +++ b/tests/suites/0_stateless/18_rbac/20_0012_privilege_access.sh @@ -86,13 +86,16 @@ echo "GRANT SELECT ON default.t20_0012_b TO 'test-user'" | $BENDSQL_CLIENT_CONNE echo "select * from default.t20_0012_b order by c" | $TEST_USER_CONNECT ## Create view table -## TODO(liyz): view is not covered with ownership yet, so the created views are owned by PUBLIC, which -## is accessible by all users. This need change. +## View is not covered with ownership yet, the privilge checks is bound to the database +## the view table is created in. +echo "select 'test -- select view'" | $TEST_USER_CONNECT echo "create database default2" | $BENDSQL_CLIENT_CONNECT + echo "create view default2.v_t20_0012 as select * from default.t20_0012_a" | $BENDSQL_CLIENT_CONNECT -## Verify view table privilege echo "select * from default2.v_t20_0012" | $TEST_USER_CONNECT -## Only grant privilege for view table +## Only grant privilege for view table, now this user can access the view under default2 db, +## but can not access the tables under the `default` database, stil raises permission error +## on SELECT default2.v_t20_0012 echo "GRANT SELECT ON default2.v_t20_0012 TO 'test-user'" | $BENDSQL_CLIENT_CONNECT echo "REVOKE SELECT ON default.t20_0012_a FROM 'test-user'" | $BENDSQL_CLIENT_CONNECT echo "REVOKE SELECT ON default.t20_0012_b FROM 'test-user'" | $BENDSQL_CLIENT_CONNECT @@ -140,8 +143,6 @@ echo "drop table if exists default.test_t" | $BENDSQL_CLIENT_CONNECT echo "create table default.test_t(id int not null)" | $BENDSQL_CLIENT_CONNECT echo "show grants for a" | $BENDSQL_CLIENT_CONNECT echo "show databases" | $USER_A_CONNECT -echo "select 'test -- show tables'" | $BENDSQL_CLIENT_CONNECT -echo "show tables" | $USER_A_CONNECT echo "select 'test -- show tables from system'" | $BENDSQL_CLIENT_CONNECT echo "show tables from system" | $USER_A_CONNECT echo "select 'test -- show tables from grant_db'" | $BENDSQL_CLIENT_CONNECT diff --git a/tests/suites/0_stateless/18_rbac/20_0015_set_role.sh b/tests/suites/0_stateless/18_rbac/20_0015_set_role.sh index 3c0d2260ac49..777cf148c216 100755 --- a/tests/suites/0_stateless/18_rbac/20_0015_set_role.sh +++ b/tests/suites/0_stateless/18_rbac/20_0015_set_role.sh @@ -10,6 +10,10 @@ echo '-- reset user, roles, and tables' echo "DROP USER IF EXISTS 'testuser1'" | $BENDSQL_CLIENT_CONNECT echo "DROP ROLE IF EXISTS 'testrole1'" | $BENDSQL_CLIENT_CONNECT echo "DROP ROLE IF EXISTS 'testrole2'" | $BENDSQL_CLIENT_CONNECT +echo "DROP ROLE IF EXISTS 'testrole3'" | $BENDSQL_CLIENT_CONNECT +echo "DROP ROLE IF EXISTS 'testrole4'" | $BENDSQL_CLIENT_CONNECT +echo "DROP TABLE IF EXISTS t20_0015_table1" | $BENDSQL_CLIENT_CONNECT +echo "DROP TABLE IF EXISTS t20_0015_table2" | $BENDSQL_CLIENT_CONNECT echo '-- prepare user, roles, and tables for tests' echo "CREATE USER 'testuser1' IDENTIFIED BY '$TEST_USER_PASSWORD'" | $BENDSQL_CLIENT_CONNECT @@ -21,8 +25,8 @@ echo 'GRANT ROLE testrole2 to ROLE testrole3' | $BENDSQL_CLIENT_CONNECT echo 'GRANT ROLE testrole1 to testuser1' | $BENDSQL_CLIENT_CONNECT echo 'GRANT ROLE testrole2 to testuser1' | $BENDSQL_CLIENT_CONNECT echo 'GRANT ROLE testrole3 to testuser1' | $BENDSQL_CLIENT_CONNECT -echo "CREATE TABLE t20_0015_table1(c int not null)" | $BENDSQL_CLIENT_CONNECT -echo "CREATE TABLE t20_0015_table2(c int not null)" | $BENDSQL_CLIENT_CONNECT +echo "CREATE TABLE t20_0015_table1(c int not null) ENGINE = MEMORY" | $BENDSQL_CLIENT_CONNECT +echo "CREATE TABLE t20_0015_table2(c int not null) ENGINE = MEMORY" | $BENDSQL_CLIENT_CONNECT echo '-- grant privilege to roles' echo 'GRANT SELECT, INSERT ON default.t20_0015_table1 TO ROLE testrole1' | $BENDSQL_CLIENT_CONNECT diff --git a/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.result b/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.result new file mode 100644 index 000000000000..82e927c88008 --- /dev/null +++ b/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.result @@ -0,0 +1,3 @@ +-- reset users +-- prepare user and tables for tests +-- ensure the statements not break with PUBLIC role diff --git a/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.sh b/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.sh new file mode 100755 index 000000000000..8028671c79c3 --- /dev/null +++ b/tests/suites/0_stateless/18_rbac/20_0016_rewrite_statements.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. "$CURDIR"/../../../shell_env.sh + +export TEST_USER_PASSWORD="password" +export TEST_USER_CONNECT="bendsql --user=testuser1 --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" + +echo '-- reset users' +echo "DROP USER IF EXISTS 'testuser1'" | $BENDSQL_CLIENT_CONNECT + +echo '-- prepare user and tables for tests' +echo "CREATE USER 'testuser1' IDENTIFIED BY '$TEST_USER_PASSWORD'" | $BENDSQL_CLIENT_CONNECT +echo "CREATE TABLE IF NOT EXISTS t20_0016_table1(c int not null)" | $BENDSQL_CLIENT_CONNECT +echo "GRANT SELECT ON default.t20_0016_table1 TO testuser1" | $BENDSQL_CLIENT_CONNECT + +echo '-- ensure the statements not break with PUBLIC role' +echo "SHOW TABLES;" | $TEST_USER_CONNECT > /dev/null +echo "SHOW DATABASES;" | $TEST_USER_CONNECT > /dev/null +echo "SHOW USERS;" | $TEST_USER_CONNECT > /dev/null +echo "SHOW ROLES;" | $TEST_USER_CONNECT > /dev/null +echo "SHOW STAGES;" | $TEST_USER_CONNECT > /dev/null +echo "SHOW PROCESSLIST;" | $TEST_USER_CONNECT > /dev/null \ No newline at end of file