Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/query/catalog/src/table_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions src/query/service/src/sessions/query_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" => {
Expand Down
10 changes: 10 additions & 0 deletions src/query/service/tests/it/sql/exec/get_table_bind_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!()
}
Expand Down
10 changes: 10 additions & 0 deletions src/query/service/tests/it/storages/fuse/operations/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ===
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading