diff --git a/src/meta/app/src/principal/user_grant.rs b/src/meta/app/src/principal/user_grant.rs index ece5570e2622c..42f073ea54d42 100644 --- a/src/meta/app/src/principal/user_grant.rs +++ b/src/meta/app/src/principal/user_grant.rs @@ -90,13 +90,21 @@ impl GrantEntry { &self.privileges } - pub fn verify_privilege(&self, object: &GrantObject, privilege: UserPrivilegeType) -> bool { + pub fn verify_privilege( + &self, + object: &GrantObject, + privileges: Vec, + ) -> bool { // the verified object should be smaller than the object inside my grant entry. if !self.object.contains(object) { return false; } - self.privileges.contains(privilege) + let mut priv_set = UserPrivilegeSet::empty(); + for privilege in privileges { + priv_set.set_privilege(privilege) + } + self.privileges.contains(BitFlags::from(priv_set)) } pub fn matches_entry(&self, object: &GrantObject) -> bool { @@ -156,10 +164,14 @@ impl UserGrantSet { self.roles.remove(role); } - pub fn verify_privilege(&self, object: &GrantObject, privilege: UserPrivilegeType) -> bool { + pub fn verify_privilege( + &self, + object: &GrantObject, + privilege: Vec, + ) -> bool { self.entries .iter() - .any(|e| e.verify_privilege(object, privilege)) + .any(|e| e.verify_privilege(object, privilege.clone())) } pub fn grant_privileges(&mut self, object: &GrantObject, privileges: UserPrivilegeSet) { diff --git a/src/meta/app/src/principal/user_privilege.rs b/src/meta/app/src/principal/user_privilege.rs index 75abdd1a07fc5..053f104b17fec 100644 --- a/src/meta/app/src/principal/user_privilege.rs +++ b/src/meta/app/src/principal/user_privilege.rs @@ -58,6 +58,10 @@ pub enum UserPrivilegeType { Grant = 1 << 12, // Privilege to Create Stage. CreateStage = 1 << 13, + // Privilege to Drop role. + DropRole = 1 << 14, + // Privilege to Drop user. + DropUser = 1 << 15, // TODO: remove this later Set = 1 << 4, } @@ -73,7 +77,9 @@ const ALL_PRIVILEGES: BitFlags = make_bitflags!( | Alter | Super | CreateUser + | DropUser | CreateRole + | DropRole | Grant | CreateStage | Set @@ -93,7 +99,9 @@ impl std::fmt::Display for UserPrivilegeType { UserPrivilegeType::Alter => "ALTER", UserPrivilegeType::Super => "SUPER", UserPrivilegeType::CreateUser => "CREATE USER", + UserPrivilegeType::DropUser => "DROP USER", UserPrivilegeType::CreateRole => "CREATE ROLE", + UserPrivilegeType::DropRole => "DROP ROLE", UserPrivilegeType::CreateStage => "CREATE STAGE", UserPrivilegeType::Grant => "GRANT", UserPrivilegeType::Set => "SET", @@ -121,8 +129,7 @@ impl UserPrivilegeSet { /// on databases and tables, and has some Global only privileges. pub fn available_privileges_on_global() -> Self { let database_privs = Self::available_privileges_on_database(); - let privs = - make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | CreateRole | Grant }); + let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | Grant }); (database_privs.privileges | privs).into() } diff --git a/src/meta/app/tests/it/user_grant.rs b/src/meta/app/tests/it/user_grant.rs index 94d0f40e373c8..2dd4d44420909 100644 --- a/src/meta/app/tests/it/user_grant.rs +++ b/src/meta/app/tests/it/user_grant.rs @@ -99,15 +99,15 @@ fn test_user_grant_entry() -> Result<()> { ); assert!(grant.verify_privilege( &GrantObject::Database("default".into(), "db1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grant.verify_privilege( &GrantObject::Database("default".into(), "db1".into()), - UserPrivilegeType::Insert + vec![UserPrivilegeType::Insert] )); assert!(grant.verify_privilege( &GrantObject::Database("default".into(), "db2".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); let grant = GrantEntry::new( @@ -116,15 +116,15 @@ fn test_user_grant_entry() -> Result<()> { ); assert!(grant.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grant.verify_privilege( &GrantObject::Table("default".into(), "db2".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(grant.verify_privilege( &GrantObject::Database("default".into(), "db1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); let grant = GrantEntry::new( @@ -133,19 +133,19 @@ fn test_user_grant_entry() -> Result<()> { ); assert!(grant.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grant.verify_privilege( &GrantObject::Table("default".into(), "db2".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grant.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Insert + vec![UserPrivilegeType::Insert] )); assert!(grant.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); Ok(()) @@ -180,23 +180,23 @@ fn test_user_grant_set() -> Result<()> { assert_eq!(2, grants.entries().len()); assert!(grants.verify_privilege( &GrantObject::Database("default".into(), "db1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grants.verify_privilege( &GrantObject::Database("default".into(), "db1".into()), - UserPrivilegeType::Select + vec![UserPrivilegeType::Select] )); assert!(grants.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Create + vec![UserPrivilegeType::Create] )); assert!(!grants.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Insert + vec![UserPrivilegeType::Insert] )); assert!(grants.verify_privilege( &GrantObject::Table("default".into(), "db1".into(), "table1".into()), - UserPrivilegeType::Select + vec![UserPrivilegeType::Select] )); Ok(()) } diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index 6f8f69b6ae3fa..8ce012b94473e 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -1359,14 +1359,16 @@ pub fn priv_type(i: Input) -> IResult { value(UserPrivilegeType::Insert, rule! { INSERT }), value(UserPrivilegeType::Update, rule! { UPDATE }), value(UserPrivilegeType::Delete, rule! { DELETE }), - value(UserPrivilegeType::Drop, rule! { DROP }), value(UserPrivilegeType::Alter, rule! { ALTER }), value(UserPrivilegeType::Super, rule! { SUPER }), value(UserPrivilegeType::CreateUser, rule! { CREATE ~ USER }), + value(UserPrivilegeType::DropUser, rule! { DROP ~ USER }), value(UserPrivilegeType::CreateRole, rule! { CREATE ~ ROLE }), + value(UserPrivilegeType::DropRole, rule! { DROP ~ ROLE }), value(UserPrivilegeType::Grant, rule! { GRANT }), value(UserPrivilegeType::CreateStage, rule! { CREATE ~ STAGE }), value(UserPrivilegeType::Set, rule! { SET }), + value(UserPrivilegeType::Drop, rule! { DROP }), value(UserPrivilegeType::Create, rule! { CREATE }), ))(i) } diff --git a/src/query/ast/tests/it/testdata/statement-error.txt b/src/query/ast/tests/it/testdata/statement-error.txt index 313bbc4ae425b..24df4773b9ad9 100644 --- a/src/query/ast/tests/it/testdata/statement-error.txt +++ b/src/query/ast/tests/it/testdata/statement-error.txt @@ -250,7 +250,7 @@ error: --> SQL:1:15 | 1 | GRANT SELECT, ALL PRIVILEGES, CREATE ON * TO 'test-grant'@'localhost'; - | ----- ------ ^^^ expected `USAGE`, `SELECT`, `INSERT`, `UPDATE`, `DELETE`, `DROP`, or 5 more ... + | ----- ------ ^^^ expected `USAGE`, `SELECT`, `INSERT`, `UPDATE`, `DELETE`, `ALTER`, or 5 more ... | | | | | while parsing ON | while parsing `GRANT { ROLE | schemaObjectPrivileges | ALL [ PRIVILEGES ] ON } TO { [ROLE ] | [USER] }` @@ -285,7 +285,7 @@ error: --> SQL:1:24 | 1 | REVOKE SELECT, CREATE, ALL PRIVILEGES ON * FROM 'test-grant'@'localhost'; - | ------ ------ ^^^ expected `USAGE`, `SELECT`, `INSERT`, `UPDATE`, `DELETE`, `DROP`, or 5 more ... + | ------ ------ ^^^ expected `USAGE`, `SELECT`, `INSERT`, `UPDATE`, `DELETE`, `ALTER`, or 5 more ... | | | | | while parsing ON | while parsing `REVOKE { ROLE | schemaObjectPrivileges | ALL [ PRIVILEGES ] ON } FROM { [ROLE ] | [USER] }` diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index 564cb228ef25a..7bceee9cf712d 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -14,9 +14,11 @@ use std::sync::Arc; +use common_catalog::table_context::TableContext; use common_exception::Result; use common_meta_app::principal::GrantObject; use common_meta_app::principal::UserPrivilegeType; +use common_sql::plans::CopyPlan; use crate::interpreters::access::AccessChecker; use crate::sessions::QueryContext; @@ -51,52 +53,72 @@ impl AccessChecker for PrivilegeAccess { table.database().to_string(), table.name().to_string(), ), - UserPrivilegeType::Select, + vec![UserPrivilegeType::Select], ) .await? } } - Plan::Explain { .. } => {} - Plan::ExplainAnalyze { .. } => {} - Plan::Copy(_) => {} - Plan::Call(_) => {} - // Catalog - Plan::ShowCreateCatalog(_) => {} - Plan::CreateCatalog(_) => {} - Plan::DropCatalog(_) => {} + Plan::ExplainAnalyze { plan } | Plan::Explain { plan, .. } => self.check(plan).await?, // Database. - Plan::ShowCreateDatabase(_) => {} - Plan::CreateDatabase(_) => { + Plan::ShowCreateDatabase(plan) => { session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Create) - .await?; + .validate_privilege( + &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), + vec![UserPrivilegeType::Select], + ) + .await? } - Plan::DropDatabase(_) => { + Plan::CreateUDF(_) | Plan::CreateDatabase(_) => { session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Drop) + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Create]) .await?; } - Plan::UndropDatabase(_) => { + Plan::DropDatabase(_) | Plan::UndropDatabase(_) | Plan::DropUDF(_) => { session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Drop) + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Drop]) .await?; } - Plan::RenameDatabase(_) => { + Plan::UseDatabase(plan) => { + let catalog = self.ctx.get_current_catalog(); session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Alter) - .await?; + .validate_privilege( + &GrantObject::Database(catalog, plan.database.clone()), + vec![UserPrivilegeType::Select], + ) + .await? } - Plan::UseDatabase(_) => {} // Table. - Plan::ShowCreateTable(_) => {} - Plan::DescribeTable(_) => {} + Plan::ShowCreateTable(plan) => { + session + .validate_privilege( + &GrantObject::Table( + plan.catalog.clone(), + plan.database.clone(), + plan.table.clone(), + ), + vec![UserPrivilegeType::Select], + ) + .await? + } + Plan::DescribeTable(plan) => { + session + .validate_privilege( + &GrantObject::Table( + plan.catalog.clone(), + plan.database.clone(), + plan.table.clone(), + ), + vec![UserPrivilegeType::Select], + ) + .await? + } Plan::CreateTable(plan) => { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Create, + vec![UserPrivilegeType::Create], ) .await?; } @@ -104,7 +126,7 @@ impl AccessChecker for PrivilegeAccess { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Drop, + vec![UserPrivilegeType::Drop], ) .await?; } @@ -112,11 +134,34 @@ impl AccessChecker for PrivilegeAccess { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Drop, + vec![UserPrivilegeType::Drop], + ) + .await?; + } + Plan::RenameTable(plan) => { + // You must have ALTER and DROP privileges for the original table, + // and CREATE and INSERT privileges for the new table. + session + .validate_privilege( + &GrantObject::Table( + plan.catalog.clone(), + plan.database.clone(), + plan.table.clone(), + ), + vec![UserPrivilegeType::Alter, UserPrivilegeType::Drop], + ) + .await?; + session + .validate_privilege( + &GrantObject::Table( + plan.catalog.clone(), + plan.new_database.clone(), + plan.new_table.clone(), + ), + vec![UserPrivilegeType::Create, UserPrivilegeType::Insert], ) .await?; } - Plan::RenameTable(_) => {} Plan::AddTableColumn(plan) => { session .validate_privilege( @@ -125,7 +170,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Alter], ) .await?; } @@ -137,7 +182,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Alter], ) .await?; } @@ -149,7 +194,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Alter], ) .await?; } @@ -161,7 +206,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Drop, + vec![UserPrivilegeType::Drop], ) .await?; } @@ -173,7 +218,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Alter], ) .await?; } @@ -185,7 +230,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Delete, + vec![UserPrivilegeType::Delete], ) .await?; } @@ -197,15 +242,11 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Super, + vec![UserPrivilegeType::Super], ) .await?; } - Plan::AnalyzeTable(_) => {} - Plan::ExistsTable(_) => {} - - // Others. - Plan::Insert(plan) => { + Plan::AnalyzeTable(plan) => { session .validate_privilege( &GrantObject::Table( @@ -213,11 +254,12 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Insert, + vec![UserPrivilegeType::Super], ) .await?; } - Plan::Replace(plan) => { + // Others. + Plan::Insert(plan) => { session .validate_privilege( &GrantObject::Table( @@ -225,10 +267,11 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Insert, + vec![UserPrivilegeType::Insert], ) .await?; - // TODO batch privilege checking api? + } + Plan::Replace(plan) => { session .validate_privilege( &GrantObject::Table( @@ -236,7 +279,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Delete, + vec![UserPrivilegeType::Insert, UserPrivilegeType::Delete], ) .await?; } @@ -248,7 +291,7 @@ impl AccessChecker for PrivilegeAccess { plan.database_name.clone(), plan.table_name.clone(), ), - UserPrivilegeType::Delete, + vec![UserPrivilegeType::Delete], ) .await?; } @@ -260,7 +303,7 @@ impl AccessChecker for PrivilegeAccess { plan.database.clone(), plan.table.clone(), ), - UserPrivilegeType::Update, + vec![UserPrivilegeType::Update], ) .await?; } @@ -268,7 +311,7 @@ impl AccessChecker for PrivilegeAccess { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Create], ) .await?; } @@ -276,7 +319,7 @@ impl AccessChecker for PrivilegeAccess { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Alter, + vec![UserPrivilegeType::Alter], ) .await?; } @@ -284,56 +327,125 @@ impl AccessChecker for PrivilegeAccess { session .validate_privilege( &GrantObject::Database(plan.catalog.clone(), plan.database.clone()), - UserPrivilegeType::Drop, + vec![UserPrivilegeType::Drop], ) .await?; } - Plan::AlterUser(_) => {} - Plan::CreateUser(_) => {} - Plan::DropUser(_) => {} - Plan::CreateUDF(_) => {} - Plan::AlterUDF(_) => {} - Plan::DropUDF(_) => {} - Plan::CreateRole(_) => {} - Plan::DropRole(_) => {} - Plan::GrantRole(_) => {} - Plan::GrantPriv(_) => {} - Plan::ShowGrants(_) => {} - Plan::ShowRoles(_) => {} - Plan::RevokePriv(_) => {} - Plan::RevokeRole(_) => {} - Plan::ListStage(_) => {} - Plan::CreateStage(_) => {} - Plan::DropStage(_) => {} - Plan::RemoveStage(_) => {} - Plan::CreateFileFormat(_) => {} - Plan::DropFileFormat(_) => {} - Plan::ShowFileFormats(_) => {} - Plan::Presign(_) => {} - Plan::SetVariable(_) => {} - Plan::UnSetVariable(_) => {} - Plan::SetRole(_) => {} - Plan::Kill(_) => { + Plan::CreateUser(_) => { session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Super) + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::CreateUser]) .await?; } - Plan::CreateShare(_) => {} - Plan::DropShare(_) => {} - Plan::GrantShareObject(_) => {} - Plan::RevokeShareObject(_) => {} - Plan::AlterShareTenants(_) => {} - Plan::DescShare(_) => {} - Plan::ShowShares(_) => {} - Plan::ShowObjectGrantPrivileges(_) => {} - Plan::ShowGrantTenantsOfShare(_) => {} - Plan::ExplainAst { .. } => {} - Plan::ExplainSyntax { .. } => {} - Plan::RevertTable(_) => { + Plan::DropUser(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::DropUser]) + .await?; + } + Plan::CreateRole(_) => { session - .validate_privilege(&GrantObject::Global, UserPrivilegeType::Alter) + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::CreateRole]) .await?; } + Plan::DropRole(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::DropRole]) + .await?; + } + Plan::GrantShareObject(_) + | Plan::RevokeShareObject(_) + | Plan::AlterShareTenants(_) + | Plan::ShowObjectGrantPrivileges(_) + | Plan::ShowGrantTenantsOfShare(_) + | Plan::SetRole(_) + | Plan::ShowGrants(_) + | Plan::ShowRoles(_) + | Plan::GrantRole(_) + | Plan::GrantPriv(_) + | Plan::RevokePriv(_) + | Plan::RevokeRole(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Grant]) + .await?; + } + Plan::SetVariable(_) | Plan::UnSetVariable(_) | Plan::Kill(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Super]) + .await?; + } + Plan::AlterUser(_) + | Plan::AlterUDF(_) + | Plan::RenameDatabase(_) + | Plan::RevertTable(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Alter]) + .await?; + } + Plan::Copy(plan) => match plan.as_ref() { + CopyPlan::IntoTable { + catalog_name, + database_name, + table_name, + .. + } => { + session + .validate_privilege( + &GrantObject::Table( + catalog_name.to_string(), + database_name.to_string(), + table_name.to_string(), + ), + vec![UserPrivilegeType::Insert], + ) + .await?; + } + CopyPlan::IntoTableWithTransform { + catalog_name, + database_name, + table_name, + .. + } => { + session + .validate_privilege( + &GrantObject::Table( + catalog_name.to_string(), + database_name.to_string(), + table_name.to_string(), + ), + vec![UserPrivilegeType::Insert], + ) + .await?; + } + CopyPlan::IntoStage { .. } => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Super]) + .await?; + } + }, + Plan::CreateShare(_) + | Plan::DropShare(_) + | Plan::DescShare(_) + | Plan::ShowShares(_) + | Plan::Call(_) + | Plan::ShowCreateCatalog(_) + | Plan::CreateCatalog(_) + | Plan::DropCatalog(_) + | Plan::ListStage(_) + | Plan::CreateStage(_) + | Plan::DropStage(_) + | Plan::RemoveStage(_) + | Plan::CreateFileFormat(_) + | Plan::DropFileFormat(_) + | Plan::ShowFileFormats(_) => { + session + .validate_privilege(&GrantObject::Global, vec![UserPrivilegeType::Super]) + .await?; + } + // Note: No need to check privileges + Plan::Presign(_) => {} + Plan::ExplainAst { .. } => {} + Plan::ExplainSyntax { .. } => {} + // just used in clickhouse-sqlalchemy, no need to check + Plan::ExistsTable(_) => {} } Ok(()) diff --git a/src/query/service/src/interpreters/interpreter_table_rename.rs b/src/query/service/src/interpreters/interpreter_table_rename.rs index 887f43bac5795..478823ed3966c 100644 --- a/src/query/service/src/interpreters/interpreter_table_rename.rs +++ b/src/query/service/src/interpreters/interpreter_table_rename.rs @@ -45,22 +45,19 @@ impl Interpreter for RenameTableInterpreter { // TODO check privileges // You must have ALTER and DROP privileges for the original table, // and CREATE and INSERT privileges for the new table. - for entity in &self.plan.entities { - let tenant = self.plan.tenant.clone(); - let catalog = self.ctx.get_catalog(&entity.catalog)?; - catalog - .rename_table(RenameTableReq { - if_exists: entity.if_exists, - name_ident: TableNameIdent { - tenant, - db_name: entity.database.clone(), - table_name: entity.table.clone(), - }, - new_db_name: entity.new_database.clone(), - new_table_name: entity.new_table.clone(), - }) - .await?; - } + let catalog = self.ctx.get_catalog(&self.plan.catalog)?; + catalog + .rename_table(RenameTableReq { + if_exists: self.plan.if_exists, + name_ident: TableNameIdent { + tenant: self.plan.tenant.clone(), + db_name: self.plan.database.clone(), + table_name: self.plan.table.clone(), + }, + new_db_name: self.plan.new_database.clone(), + new_table_name: self.plan.new_table.clone(), + }) + .await?; Ok(PipelineBuildResult::create()) } diff --git a/src/query/service/src/sessions/session.rs b/src/query/service/src/sessions/session.rs index 10fbaacd8b177..745161f3be89d 100644 --- a/src/query/service/src/sessions/session.rs +++ b/src/query/service/src/sessions/session.rs @@ -292,11 +292,13 @@ impl Session { pub async fn validate_privilege( self: &Arc, object: &GrantObject, - privilege: UserPrivilegeType, + privilege: Vec, ) -> Result<()> { // 1. check user's privilege set let current_user = self.get_current_user()?; - let user_verified = current_user.grants.verify_privilege(object, privilege); + let user_verified = current_user + .grants + .verify_privilege(object, privilege.clone()); if user_verified { return Ok(()); } @@ -305,16 +307,16 @@ impl Session { self.ensure_current_role().await?; let current_role = self.get_current_role(); let role_verified = current_role - .map(|r| r.grants.verify_privilege(object, privilege)) + .map(|r| r.grants.verify_privilege(object, privilege.clone())) .unwrap_or(false); if role_verified { return Ok(()); } Err(ErrorCode::PermissionDenied(format!( - "Permission denied, user {} requires {} privilege on {}", + "Permission denied, user {} requires {:?} privilege on {}", ¤t_user.identity(), - privilege, + privilege.clone(), object ))) } diff --git a/src/query/sql/src/planner/binder/ddl/table.rs b/src/query/sql/src/planner/binder/ddl/table.rs index a4db283f663a2..0c6f5a9393fc7 100644 --- a/src/query/sql/src/planner/binder/ddl/table.rs +++ b/src/query/sql/src/planner/binder/ddl/table.rs @@ -93,7 +93,6 @@ use crate::plans::OptimizeTableAction; use crate::plans::OptimizeTablePlan; use crate::plans::Plan; use crate::plans::ReclusterTablePlan; -use crate::plans::RenameTableEntity; use crate::plans::RenameTablePlan; use crate::plans::RevertTablePlan; use crate::plans::RewriteKind; @@ -588,18 +587,14 @@ impl Binder { match action { AlterTableAction::RenameTable { new_table } => { - let entities = vec![RenameTableEntity { + Ok(Plan::RenameTable(Box::new(RenameTablePlan { + tenant, if_exists: *if_exists, new_database: database.clone(), new_table: normalize_identifier(new_table, &self.name_resolution_ctx).name, catalog, database, table, - }]; - - Ok(Plan::RenameTable(Box::new(RenameTablePlan { - tenant, - entities, }))) } AlterTableAction::AddColumn { column } => { @@ -722,18 +717,14 @@ impl Binder { )); } - let entities = vec![RenameTableEntity { + Ok(Plan::RenameTable(Box::new(RenameTablePlan { + tenant, if_exists: *if_exists, catalog, database, table, new_database, new_table, - }]; - - Ok(Plan::RenameTable(Box::new(RenameTablePlan { - tenant, - entities, }))) } diff --git a/src/query/sql/src/planner/plans/ddl/table.rs b/src/query/sql/src/planner/plans/ddl/table.rs index 201ecaeb4cbaf..9bb7da0d5594a 100644 --- a/src/query/sql/src/planner/plans/ddl/table.rs +++ b/src/query/sql/src/planner/plans/ddl/table.rs @@ -132,11 +132,6 @@ impl AnalyzeTablePlan { #[derive(Clone, Debug, PartialEq, Eq)] pub struct RenameTablePlan { pub tenant: String, - pub entities: Vec, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct RenameTableEntity { pub if_exists: bool, pub catalog: String, pub database: String, diff --git a/src/query/users/tests/it/role_mgr.rs b/src/query/users/tests/it/role_mgr.rs index 8c64d788d676b..11cb0eb132abe 100644 --- a/src/query/users/tests/it/role_mgr.rs +++ b/src/query/users/tests/it/role_mgr.rs @@ -77,7 +77,7 @@ async fn test_role_manager() -> Result<()> { let role = role_mgr.get_role(tenant, role_name.clone()).await?; assert!( role.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Alter) + .verify_privilege(&GrantObject::Global, vec![UserPrivilegeType::Alter]) ); } diff --git a/src/query/users/tests/it/user_mgr.rs b/src/query/users/tests/it/user_mgr.rs index 9bf964b66d151..6d880cca545a3 100644 --- a/src/query/users/tests/it/user_mgr.rs +++ b/src/query/users/tests/it/user_mgr.rs @@ -135,12 +135,12 @@ async fn test_user_manager() -> Result<()> { assert!( new_user .grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Set) + .verify_privilege(&GrantObject::Global, vec![UserPrivilegeType::Set]) ); assert!( !new_user .grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Create) + .verify_privilege(&GrantObject::Global, vec![UserPrivilegeType::Create]) ); user_mgr .drop_user(tenant, new_user.identity(), true) @@ -261,22 +261,12 @@ async fn test_user_manager_with_root_user() -> Result<()> { .await?; assert_eq!(user.name, username1); assert_eq!(user.hostname, hostname1); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Create) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Select) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Insert) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super) - ); + assert!(user.grants.verify_privilege(&GrantObject::Global, vec![ + UserPrivilegeType::Create, + UserPrivilegeType::Select, + UserPrivilegeType::Insert, + UserPrivilegeType::Super + ])); } // Get user via username `default` and hostname `localhost`. @@ -286,22 +276,12 @@ async fn test_user_manager_with_root_user() -> Result<()> { .await?; assert_eq!(user.name, username1); assert_eq!(user.hostname, hostname2); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Create) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Select) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Insert) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super) - ); + assert!(user.grants.verify_privilege(&GrantObject::Global, vec![ + UserPrivilegeType::Create, + UserPrivilegeType::Select, + UserPrivilegeType::Insert, + UserPrivilegeType::Super + ])); } // Get user via username `default` and hostname `otherhost` will be denied. @@ -323,22 +303,12 @@ async fn test_user_manager_with_root_user() -> Result<()> { .await?; assert_eq!(user.name, username2); assert_eq!(user.hostname, hostname1); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Create) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Select) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Insert) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super) - ); + assert!(user.grants.verify_privilege(&GrantObject::Global, vec![ + UserPrivilegeType::Create, + UserPrivilegeType::Select, + UserPrivilegeType::Insert, + UserPrivilegeType::Super + ])); } // Get user via username `root` and hostname `localhost`. @@ -348,22 +318,12 @@ async fn test_user_manager_with_root_user() -> Result<()> { .await?; assert_eq!(user.name, username2); assert_eq!(user.hostname, hostname2); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Create) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Select) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Insert) - ); - assert!( - user.grants - .verify_privilege(&GrantObject::Global, UserPrivilegeType::Super) - ); + assert!(user.grants.verify_privilege(&GrantObject::Global, vec![ + UserPrivilegeType::Create, + UserPrivilegeType::Select, + UserPrivilegeType::Insert, + UserPrivilegeType::Super + ])); } // Get user via username `root` and hostname `otherhost` will be denied. diff --git a/tests/suites/0_stateless/20+_others/20_0012_privilege_access.result b/tests/suites/0_stateless/20+_others/20_0012_privilege_access.result index 3326d16e4f522..7a2884226c790 100644 --- a/tests/suites/0_stateless/20+_others/20_0012_privilege_access.result +++ b/tests/suites/0_stateless/20+_others/20_0012_privilege_access.result @@ -1,24 +1,24 @@ -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires INSERT privilege on 'default'.'default'.'t20_0012'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Insert] privilege on 'default'.'default'.'t20_0012'. 1 2 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires UPDATE privilege on 'default'.'default'.'t20_0012'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Update] privilege on 'default'.'default'.'t20_0012'. 2 3 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires DELETE privilege on 'default'.'default'.'t20_0012'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Delete] privilege on 'default'.'default'.'t20_0012'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SUPER privilege on 'default'.'default'.'t20_0012'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Super] privilege on 'default'.'default'.'t20_0012'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default'.'t20_0012_a'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'default'.'t20_0012_a'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default'.'t20_0012_b'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'default'.'t20_0012_b'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'default2'.'v_t20_0012'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'default2'.'v_t20_0012'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'clustering_information'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'system'.'clustering_information'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_snapshot'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'system'.'fuse_snapshot'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_segment'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'system'.'fuse_segment'. 1 -ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires SELECT privilege on 'default'.'system'.'fuse_block'. +ERROR 1105 (HY000) at line 1: Code: 1063, displayText = Permission denied, user 'test-user'@'127.0.0.1' requires [Select] privilege on 'default'.'system'.'fuse_block'. 1 diff --git a/tests/suites/0_stateless/20+_others/20_0012_privilege_access.sh b/tests/suites/0_stateless/20+_others/20_0012_privilege_access.sh index b2d1a3549a982..74f5400eef3b3 100755 --- a/tests/suites/0_stateless/20+_others/20_0012_privilege_access.sh +++ b/tests/suites/0_stateless/20+_others/20_0012_privilege_access.sh @@ -4,7 +4,7 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$CURDIR"/../../../shell_env.sh export TEST_USER_PASSWORD="password" -export TEST_USER_CONNECT="mysql --defaults-extra-file=password.out --port ${QUERY_MYSQL_HANDLER_PORT} ${MYSQL_DATABASE} -s" +export TEST_USER_CONNECT="mysql --defaults-extra-file=password.out --port ${QUERY_MYSQL_HANDLER_PORT} -s" echo -e "[mysql]\nhost=${QUERY_MYSQL_HANDLER_HOST}\nuser=test-user\npassword=${TEST_USER_PASSWORD}" >> password.out ## create user