From fd5a7c21a66ab6faec8f7a35cef41354696fdbea Mon Sep 17 00:00:00 2001 From: TCeason <33082201+TCeason@users.noreply.github.com> Date: Tue, 28 May 2024 10:07:52 +0800 Subject: [PATCH] fix(query): show grants also need to check user roles (#15650) modify identify display --- src/query/catalog/src/table_context.rs | 8 ++++++++ src/query/service/src/sessions/query_ctx.rs | 12 ++++++++++++ .../table_functions/show_grants/show_grants_table.rs | 10 +++++----- .../service/tests/it/sql/exec/get_table_bind_test.rs | 10 ++++++++++ .../tests/it/storages/fuse/operations/commit.rs | 10 ++++++++++ .../18_rbac/18_0005_show_grants_with_dropped.result | 7 ++++++- .../18_rbac/18_0005_show_grants_with_dropped.sh | 6 +++++- 7 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/query/catalog/src/table_context.rs b/src/query/catalog/src/table_context.rs index c640b8309f37..afcefabeadd3 100644 --- a/src/query/catalog/src/table_context.rs +++ b/src/query/catalog/src/table_context.rs @@ -35,10 +35,12 @@ use databend_common_expression::Expr; use databend_common_expression::FunctionContext; use databend_common_io::prelude::FormatSettings; use databend_common_meta_app::principal::FileFormatParams; +use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OnErrorMode; 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::tenant::Tenant; use databend_common_pipeline_core::processors::PlanProfile; use databend_common_pipeline_core::InputError; @@ -202,6 +204,12 @@ pub trait TableContext: Send + Sync { unimplemented!() } async fn get_all_effective_roles(&self) -> Result>; + + async fn validate_privilege( + &self, + object: &GrantObject, + privilege: UserPrivilegeType, + ) -> Result<()>; async fn get_available_roles(&self) -> Result>; async fn get_visibility_checker(&self) -> Result; fn get_fuse_version(&self) -> String; diff --git a/src/query/service/src/sessions/query_ctx.rs b/src/query/service/src/sessions/query_ctx.rs index ab66f2483d53..44972f338911 100644 --- a/src/query/service/src/sessions/query_ctx.rs +++ b/src/query/service/src/sessions/query_ctx.rs @@ -61,11 +61,13 @@ use databend_common_expression::Expr; use databend_common_expression::FunctionContext; use databend_common_io::prelude::FormatSettings; use databend_common_meta_app::principal::FileFormatParams; +use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OnErrorMode; use databend_common_meta_app::principal::RoleInfo; use databend_common_meta_app::principal::StageFileFormatType; 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::principal::COPY_MAX_FILES_COMMIT_MSG; use databend_common_meta_app::principal::COPY_MAX_FILES_PER_COMMIT; use databend_common_meta_app::schema::CatalogInfo; @@ -596,6 +598,16 @@ impl TableContext for QueryContext { self.get_current_session().get_all_effective_roles().await } + async fn validate_privilege( + &self, + object: &GrantObject, + privilege: UserPrivilegeType, + ) -> Result<()> { + self.get_current_session() + .validate_privilege(object, privilege) + .await + } + fn get_current_session_id(&self) -> String { self.get_current_session().get_id() } 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 73f5a35e6887..4fa3a9b7e490 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 @@ -265,11 +265,11 @@ async fn show_account_grants( ) -> Result> { let tenant = ctx.get_tenant(); let current_user = ctx.get_current_user()?; - let has_grant_priv = current_user - .grants - .entries() - .iter() - .any(|entry| entry.verify_privilege(&GrantObject::Global, UserPrivilegeType::Grant)); + let has_grant_priv = ctx + .validate_privilege(&GrantObject::Global, UserPrivilegeType::Grant) + .await + .is_ok(); + // TODO: add permission check on reading user grants let (grant_to, name, identity, grant_set) = match grant_type { "user" => { 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 6859550c3890..8f6009a4de0d 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 @@ -47,10 +47,12 @@ use databend_common_expression::Expr; use databend_common_expression::FunctionContext; use databend_common_io::prelude::FormatSettings; use databend_common_meta_app::principal::FileFormatParams; +use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OnErrorMode; 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::CatalogInfo; use databend_common_meta_app::schema::CreateDatabaseReply; use databend_common_meta_app::schema::CreateDatabaseReq; @@ -622,6 +624,14 @@ impl TableContext for CtxDelegation { todo!() } + async fn validate_privilege( + &self, + _object: &GrantObject, + _privilege: UserPrivilegeType, + ) -> Result<()> { + todo!() + } + async fn get_visibility_checker(&self) -> Result { todo!() } 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 ec32edd33d36..038730d90095 100644 --- a/src/query/service/tests/it/storages/fuse/operations/commit.rs +++ b/src/query/service/tests/it/storages/fuse/operations/commit.rs @@ -46,10 +46,12 @@ use databend_common_expression::Expr; use databend_common_expression::FunctionContext; use databend_common_io::prelude::FormatSettings; use databend_common_meta_app::principal::FileFormatParams; +use databend_common_meta_app::principal::GrantObject; use databend_common_meta_app::principal::OnErrorMode; 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::CatalogInfo; use databend_common_meta_app::schema::CreateDatabaseReply; use databend_common_meta_app::schema::CreateDatabaseReq; @@ -564,6 +566,14 @@ impl TableContext for CtxDelegation { todo!() } + async fn validate_privilege( + &self, + _object: &GrantObject, + _privilege: UserPrivilegeType, + ) -> Result<()> { + todo!() + } + async fn get_visibility_checker(&self) -> Result { todo!() } diff --git a/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.result b/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.result index aff8c8e172fc..e95b20016fe2 100644 --- a/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.result +++ b/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.result @@ -71,9 +71,14 @@ UPDATE,DELETE default.c_r1.t1 ROLE role3 GRANT UPDATE,DELETE ON 'default'.'c_r1 SELECT,INSERT *.* NULL ROLE role1 GRANT SELECT,INSERT ON *.* TO ROLE `role1` SELECT,INSERT *.* NULL USER u1 GRANT SELECT,INSERT ON *.* TO 'u1'@'%' SELECT,INSERT *.* NULL USER u1 GRANT SELECT,INSERT ON *.* TO 'u1'@'%' +INSERT c_r1 ROLE role3 SELECT,INSERT *.* NULL USER u1 GRANT SELECT,INSERT ON *.* TO 'u1'@'%' Need Err: Error: APIError: ResponseError with 1063: Permission denied: privilege [Grant] is required on *.* for user 'u1'@'%' with roles [role1,role2] Error: APIError: ResponseError with 1063: Permission denied: privilege [Grant] is required on *.* for user 'u1'@'%' with roles [role1,role2] -Error: APIError: ResponseError with 1063: Permission denied: privilege [Grant] is required on *.* for user 'u1'@'%' with roles [role1,role2] +UPDATE information_schema ROLE role3 GRANT UPDATE ON 'default'.'information_schema'.* TO ROLE `role3` +INSERT c_r1 ROLE role3 GRANT INSERT ON 'default'.'c_r1'.* TO ROLE `role3` +SELECT default.system.one ROLE role3 GRANT SELECT ON 'default'.'system'.'one' TO ROLE `role3` +UPDATE,DELETE default.c_r1.t1 ROLE role3 GRANT UPDATE,DELETE ON 'default'.'c_r1'.'t1' TO ROLE `role3` +Error: APIError: ResponseError with 2201: User 'role3%40test'@'%' does not exist. === clean up === diff --git a/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.sh b/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.sh index 937f9a6576e8..93273c498737 100755 --- a/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.sh +++ b/tests/suites/0_stateless/18_rbac/18_0005_show_grants_with_dropped.sh @@ -88,11 +88,15 @@ echo "show grants for role role2" | $USER_U1_CONNECT echo "show grants for user u1" | $USER_U1_CONNECT echo "show grants for user u1 where name='u1' limit 1;" | $USER_U1_CONNECT echo "show grants for user u1 where name!='u1' limit 1;" | $USER_U1_CONNECT +echo "show grants on database c_r1" | $USER_U1_CONNECT | awk -F ' ' '{$3=""; print $0}' echo "show grants" | $USER_U1_CONNECT echo "Need Err:" echo "show grants for user root" | $USER_U1_CONNECT echo "show grants for role account_admin" | $USER_U1_CONNECT -echo "show grants for role c_r2" | $USER_U1_CONNECT + +echo "grant grant on *.* to role role1;" | $BENDSQL_CLIENT_CONNECT +echo "show grants for role role3;" | $USER_U1_CONNECT | awk -F ' ' '{$3=""; print $0}' +echo "show grants for user 'role3@test';" | $USER_U1_CONNECT echo "=== clean up ===" echo "drop user if exists u1" | $BENDSQL_CLIENT_CONNECT