fix(query): show grants also need to check user roles

modify identify display
databendlabs · May 27, 2024 · cdf4002 · cdf4002
1 parent ce031db
commit cdf4002
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 7 deletions.
diff --git 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<Vec<RoleInfo>>;
+
+    async fn validate_privilege(
+        &self,
+        object: &GrantObject,
+        privilege: UserPrivilegeType,
+    ) -> Result<()>;
     async fn get_available_roles(&self) -> Result<Vec<RoleInfo>>;
     async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker>;
     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
@@ -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
@@ -265,11 +265,11 @@ async fn show_account_grants(
 ) -> Result<Option<DataBlock>> {
     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
@@ -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<GrantObjectVisibilityChecker> {
         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
@@ -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<GrantObjectVisibilityChecker> {
         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
@@ -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@test'@'%' 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
@@ -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