diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index c2f60020c3b8e..d349cca48e3d6 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -143,8 +143,8 @@ impl AccessChecker for PrivilegeAccess { let user = self.ctx.get_current_user()?; let (identity, grant_set) = (user.identity().to_string(), user.grants); let tenant = self.ctx.get_tenant(); - let enable_stage_udf_priv_check = - self.ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_experimental_rbac = + self.ctx.get_settings().get_enable_new_rbac_check()?; match plan { Plan::Query { @@ -195,7 +195,7 @@ impl AccessChecker for PrivilegeAccess { } _ => {} }; - if enable_stage_udf_priv_check { + if enable_experimental_rbac { match s_expr.get_udfs() { Ok(udfs) => { if !udfs.is_empty() { @@ -218,7 +218,7 @@ impl AccessChecker for PrivilegeAccess { for table in metadata.tables() { - if enable_stage_udf_priv_check && table.is_source_of_stage() { + if enable_experimental_rbac && table.is_source_of_stage() { match table.table().get_data_source_info() { DataSourceInfo::StageSource(stage_info) => { if !stage_info.stage_info.is_from_uri { @@ -525,9 +525,11 @@ impl AccessChecker for PrivilegeAccess { .await?; } Plan::ReclusterTable(plan) => { - if let Some(scalar) = &plan.push_downs { - let udf = get_udf_names(scalar)?; - self.check_udf_priv(udf).await?; + if enable_experimental_rbac { + if let Some(scalar) = &plan.push_downs { + let udf = get_udf_names(scalar)?; + self.check_udf_priv(udf).await?; + } } self.validate_access( &GrantObject::Table( @@ -624,7 +626,7 @@ impl AccessChecker for PrivilegeAccess { .await?; } Plan::MergeInto(plan) => { - if enable_stage_udf_priv_check { + if enable_experimental_rbac { let s_expr = &plan.input; match s_expr.get_udfs() { Ok(udfs) => { @@ -679,7 +681,7 @@ impl AccessChecker for PrivilegeAccess { .await?; } Plan::Delete(plan) => { - if enable_stage_udf_priv_check { + if enable_experimental_rbac { if let Some(selection) = &plan.selection { let udf = get_udf_names(selection)?; self.check_udf_priv(udf).await?; @@ -715,29 +717,31 @@ impl AccessChecker for PrivilegeAccess { .await?; } Plan::Update(plan) => { - for scalar in plan.update_list.values() { - let udf = get_udf_names(scalar)?; - self.check_udf_priv(udf).await?; - } - if let Some(selection) = &plan.selection { - let udf = get_udf_names(selection)?; - self.check_udf_priv(udf).await?; - } - for subquery in &plan.subquery_desc { - match subquery.input_expr.get_udfs() { - Ok(udfs) => { - if !udfs.is_empty() { - for udf in udfs { - self.validate_access( - &GrantObject::UDF(udf.clone()), - vec![UserPrivilegeType::Usage], - false, - ).await? + if enable_experimental_rbac { + for scalar in plan.update_list.values() { + let udf = get_udf_names(scalar)?; + self.check_udf_priv(udf).await?; + } + if let Some(selection) = &plan.selection { + let udf = get_udf_names(selection)?; + self.check_udf_priv(udf).await?; + } + for subquery in &plan.subquery_desc { + match subquery.input_expr.get_udfs() { + Ok(udfs) => { + if !udfs.is_empty() { + for udf in udfs { + self.validate_access( + &GrantObject::UDF(udf.clone()), + vec![UserPrivilegeType::Usage], + false, + ).await? + } } } - } - Err(err) => { - return Err(err.add_message("get udf error on validating access")); + Err(err) => { + return Err(err.add_message("get udf error on validating access")); + } } } } @@ -851,19 +855,16 @@ impl AccessChecker for PrivilegeAccess { } Plan::CopyIntoTable(plan) => { // TODO(TCeason): need to check plan.query privileges. - - if enable_stage_udf_priv_check && !plan.stage_table_info.stage_info.is_from_uri { - let stage_name = &plan.stage_table_info.stage_info.stage_name; - self - .validate_access( - &GrantObject::Stage(stage_name.clone()), - vec![UserPrivilegeType::Read], - false, - ) - .await?; - } - - + if enable_experimental_rbac && !plan.stage_table_info.stage_info.is_from_uri { + let stage_name = &plan.stage_table_info.stage_info.stage_name; + self + .validate_access( + &GrantObject::Stage(stage_name.clone()), + vec![UserPrivilegeType::Read], + false, + ) + .await?; + } self .validate_access( &GrantObject::Table( @@ -877,34 +878,31 @@ impl AccessChecker for PrivilegeAccess { .await?; } Plan::CopyIntoLocation(plan) => { - - if enable_stage_udf_priv_check && !plan.stage.is_from_uri { - let stage_name = &plan.stage.stage_name; - self - .validate_access( - &GrantObject::Stage(stage_name.clone()), - vec![UserPrivilegeType::Write], - false, - ) - .await?; - } + if enable_experimental_rbac && !plan.stage.is_from_uri { + let stage_name = &plan.stage.stage_name; + self + .validate_access( + &GrantObject::Stage(stage_name.clone()), + vec![UserPrivilegeType::Write], + false, + ) + .await?; + } let from = plan.from.clone(); return self.check(ctx, &from).await; } Plan::RemoveStage(plan) => { - - if enable_stage_udf_priv_check && !plan.stage.is_from_uri { - let stage_name = &plan.stage.stage_name; - self - .validate_access( - &GrantObject::Stage(stage_name.clone()), - vec![UserPrivilegeType::Write], - false, - ) - .await?; - } - + if enable_experimental_rbac && !plan.stage.is_from_uri { + let stage_name = &plan.stage.stage_name; + self + .validate_access( + &GrantObject::Stage(stage_name.clone()), + vec![UserPrivilegeType::Write], + false, + ) + .await?; + } } Plan::CreateShareEndpoint(_) | Plan::ShowShareEndpoint(_) @@ -953,31 +951,30 @@ impl AccessChecker for PrivilegeAccess { Plan::SetSecondaryRoles(_) => {} Plan::ShowRoles(_) => {} Plan::Presign(plan) => { - if enable_stage_udf_priv_check && !plan.stage.is_from_uri { - let stage_name = &plan.stage.stage_name; - let action = &plan.action; - match action { - PresignAction::Upload => { - self - .validate_access( - &GrantObject::Stage(stage_name.clone()), - vec![UserPrivilegeType::Write], - false, - ) - .await? - } - PresignAction::Download => { - self - .validate_access( - &GrantObject::Stage(stage_name.clone()), - vec![UserPrivilegeType::Read], - false, - ) - .await? - } + if enable_experimental_rbac && !plan.stage.is_from_uri { + let stage_name = &plan.stage.stage_name; + let action = &plan.action; + match action { + PresignAction::Upload => { + self + .validate_access( + &GrantObject::Stage(stage_name.clone()), + vec![UserPrivilegeType::Write], + false, + ) + .await? + } + PresignAction::Download => { + self + .validate_access( + &GrantObject::Stage(stage_name.clone()), + vec![UserPrivilegeType::Read], + false, + ) + .await? } } - + } } Plan::ExplainAst { .. } => {} Plan::ExplainSyntax { .. } => {} diff --git a/src/query/service/src/table_functions/infer_schema/infer_schema_table.rs b/src/query/service/src/table_functions/infer_schema/infer_schema_table.rs index 7351f1c829cd7..69dcfc18c6192 100644 --- a/src/query/service/src/table_functions/infer_schema/infer_schema_table.rs +++ b/src/query/service/src/table_functions/infer_schema/infer_schema_table.rs @@ -181,8 +181,7 @@ impl AsyncSource for InferSchemaSource { let (stage_info, path) = resolve_stage_location(&self.ctx, &self.args_parsed.location).await?; - let enable_stage_udf_priv_check = - self.ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_stage_udf_priv_check = self.ctx.get_settings().get_enable_new_rbac_check()?; if enable_stage_udf_priv_check { let visibility_checker = self.ctx.get_visibility_checker().await?; if !stage_info.is_from_uri diff --git a/src/query/service/src/table_functions/inspect_parquet/inspect_parquet_table.rs b/src/query/service/src/table_functions/inspect_parquet/inspect_parquet_table.rs index 9088214f45d9f..3a692ed50eb88 100644 --- a/src/query/service/src/table_functions/inspect_parquet/inspect_parquet_table.rs +++ b/src/query/service/src/table_functions/inspect_parquet/inspect_parquet_table.rs @@ -210,8 +210,7 @@ impl AsyncSource for InspectParquetSource { self.is_finished = true; let uri = self.uri.strip_prefix('@').unwrap().to_string(); let (stage_info, path) = resolve_stage_location(&self.ctx, &uri).await?; - let enable_stage_udf_priv_check = - self.ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_stage_udf_priv_check = self.ctx.get_settings().get_enable_new_rbac_check()?; if enable_stage_udf_priv_check { let visibility_checker = self.ctx.get_visibility_checker().await?; if !stage_info.is_from_uri diff --git a/src/query/service/src/table_functions/list_stage/list_stage_table.rs b/src/query/service/src/table_functions/list_stage/list_stage_table.rs index 1c954d83ad3d4..9bfbc3656ddcd 100644 --- a/src/query/service/src/table_functions/list_stage/list_stage_table.rs +++ b/src/query/service/src/table_functions/list_stage/list_stage_table.rs @@ -185,8 +185,7 @@ impl AsyncSource for ListStagesSource { let (stage_info, path) = resolve_stage_location(&self.ctx, &self.args_parsed.location).await?; - let enable_stage_udf_priv_check = - self.ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_stage_udf_priv_check = self.ctx.get_settings().get_enable_new_rbac_check()?; if enable_stage_udf_priv_check { let visibility_checker = self.ctx.get_visibility_checker().await?; if !stage_info.is_from_uri diff --git a/src/query/service/tests/it/storages/testdata/settings_table.txt b/src/query/service/tests/it/storages/testdata/settings_table.txt index 0c15ff37646bc..8acb7ab6eadef 100644 --- a/src/query/service/tests/it/storages/testdata/settings_table.txt +++ b/src/query/service/tests/it/storages/testdata/settings_table.txt @@ -19,6 +19,7 @@ DB.Table: 'system'.'settings', Table: settings-table_id:1, ver:0, Engine: System | 'enable_distributed_replace_into' | '0' | '0' | 'SESSION' | 'Enable distributed execution of replace into.' | 'UInt64' | | 'enable_dphyp' | '1' | '1' | 'SESSION' | 'Enables dphyp join order algorithm.' | 'UInt64' | | 'enable_experimental_merge_into' | '0' | '0' | 'SESSION' | 'Enable experimental merge into.' | 'UInt64' | +| 'enable_experimental_rbac' | '0' | '0' | 'SESSION' | 'experiment setting disables stage and udf privilege check(disable by default).' | 'UInt64' | | 'enable_hive_parquet_predict_pushdown' | '1' | '1' | 'SESSION' | 'Enable hive parquet predict pushdown by setting this variable to 1, default value: 1' | 'UInt64' | | 'enable_parquet_page_index' | '1' | '1' | 'SESSION' | 'Enables parquet page index' | 'UInt64' | | 'enable_parquet_prewhere' | '0' | '0' | 'SESSION' | 'Enables parquet prewhere' | 'UInt64' | @@ -31,7 +32,6 @@ DB.Table: 'system'.'settings', Table: settings-table_id:1, ver:0, Engine: System | 'enable_replace_into_partitioning' | '1' | '1' | 'SESSION' | 'Enables partitioning for replace-into statement (if table has cluster keys).' | 'UInt64' | | 'enable_runtime_filter' | '0' | '0' | 'SESSION' | 'Enables runtime filter optimization for JOIN.' | 'UInt64' | | 'enable_table_lock' | '1' | '1' | 'SESSION' | 'Enables table lock if necessary (enabled by default).' | 'UInt64' | -| 'experiment_enable_stage_udf_priv_check' | '0' | '0' | 'SESSION' | 'experiment setting disables stage and udf privilege check(disable by default).' | 'UInt64' | | 'external_server_connect_timeout_secs' | '10' | '10' | 'SESSION' | 'Connection timeout to external server' | 'UInt64' | | 'external_server_request_timeout_secs' | '180' | '180' | 'SESSION' | 'Request timeout to external server' | 'UInt64' | | 'flight_client_timeout' | '60' | '60' | 'SESSION' | 'Sets the maximum time in seconds that a flight client request can be processed.' | 'UInt64' | diff --git a/src/query/settings/src/settings_default.rs b/src/query/settings/src/settings_default.rs index ac42022e8b3f0..d74c6883a2f0b 100644 --- a/src/query/settings/src/settings_default.rs +++ b/src/query/settings/src/settings_default.rs @@ -519,7 +519,7 @@ impl DefaultSettings { possible_values: Some(vec!["rounding", "truncating"]), mode: SettingMode::Both, }), - ("experiment_enable_stage_udf_priv_check", DefaultSettingValue { + ("enable_experimental_rbac", DefaultSettingValue { value: UserSettingValue::UInt64(0), desc: "experiment setting disables stage and udf privilege check(disable by default).", possible_values: None, diff --git a/src/query/settings/src/settings_getter_setter.rs b/src/query/settings/src/settings_getter_setter.rs index c657b2e9c623f..d219cd943da56 100644 --- a/src/query/settings/src/settings_getter_setter.rs +++ b/src/query/settings/src/settings_getter_setter.rs @@ -339,8 +339,8 @@ impl Settings { Ok(self.try_get_u64("enable_table_lock")? != 0) } - pub fn get_enable_stage_udf_priv_check(&self) -> Result { - Ok(self.try_get_u64("experiment_enable_stage_udf_priv_check")? != 0) + pub fn get_enable_new_rbac_check(&self) -> Result { + Ok(self.try_get_u64("enable_experimental_rbac")? != 0) } pub fn get_table_lock_expire_secs(&self) -> Result { diff --git a/src/query/storages/system/src/functions_table.rs b/src/query/storages/system/src/functions_table.rs index faefa9951ec8c..045485636a9f6 100644 --- a/src/query/storages/system/src/functions_table.rs +++ b/src/query/storages/system/src/functions_table.rs @@ -69,7 +69,7 @@ impl AsyncSystemTable for FunctionsTable { scalar_func_names.sort(); let aggregate_function_factory = AggregateFunctionFactory::instance(); let aggr_func_names = aggregate_function_factory.registered_names(); - let enable_stage_udf_priv_check = ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_stage_udf_priv_check = ctx.get_settings().get_enable_new_rbac_check()?; let udfs = if enable_stage_udf_priv_check { let visibility_checker = ctx.get_visibility_checker().await?; let udfs = FunctionsTable::get_udfs(ctx).await?; diff --git a/src/query/storages/system/src/stages_table.rs b/src/query/storages/system/src/stages_table.rs index 5f3e735d5f5b6..152bb67eb1ee7 100644 --- a/src/query/storages/system/src/stages_table.rs +++ b/src/query/storages/system/src/stages_table.rs @@ -56,7 +56,7 @@ impl AsyncSystemTable for StagesTable { ) -> Result { let tenant = ctx.get_tenant(); let stages = UserApiProvider::instance().get_stages(&tenant).await?; - let enable_stage_udf_priv_check = ctx.get_settings().get_enable_stage_udf_priv_check()?; + let enable_stage_udf_priv_check = ctx.get_settings().get_enable_new_rbac_check()?; let stages = if enable_stage_udf_priv_check { let visibility_checker = ctx.get_visibility_checker().await?; stages diff --git a/tests/suites/1_stateful/11_rbac/00_0012_stage_priv.sh b/tests/suites/1_stateful/11_rbac/00_0012_stage_priv.sh index e343845780b89..9518e7f278bd5 100755 --- a/tests/suites/1_stateful/11_rbac/00_0012_stage_priv.sh +++ b/tests/suites/1_stateful/11_rbac/00_0012_stage_priv.sh @@ -7,7 +7,7 @@ export TEST_USER_NAME="u1" export TEST_USER_PASSWORD="password" export TEST_USER_CONNECT="bendsql --user=u1 --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" -echo "set global experiment_enable_stage_udf_priv_check=1" | $BENDSQL_CLIENT_CONNECT +echo "set global enable_experimental_rbac=1" | $BENDSQL_CLIENT_CONNECT echo "drop table if exists test_table;" | $BENDSQL_CLIENT_CONNECT echo "drop user if exists u1;" | $BENDSQL_CLIENT_CONNECT @@ -118,5 +118,5 @@ echo "drop user u1" | $BENDSQL_CLIENT_CONNECT echo "drop table if exists t" | $BENDSQL_CLIENT_CONNECT rm -rf /tmp/00_0012 -echo "unset experiment_enable_stage_udf_priv_check" | $BENDSQL_CLIENT_CONNECT +echo "unset enable_experimental_rbac" | $BENDSQL_CLIENT_CONNECT diff --git a/tests/suites/1_stateful/11_rbac/18_0001_udf_priv.sh b/tests/suites/1_stateful/11_rbac/18_0001_udf_priv.sh index dea9833e0f1a5..885122bfd9db0 100755 --- a/tests/suites/1_stateful/11_rbac/18_0001_udf_priv.sh +++ b/tests/suites/1_stateful/11_rbac/18_0001_udf_priv.sh @@ -7,7 +7,7 @@ echo "=== test UDF priv" export TEST_USER_PASSWORD="password" export TEST_USER_CONNECT="bendsql --user=test-user --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" -echo "set global experiment_enable_stage_udf_priv_check=1" | $BENDSQL_CLIENT_CONNECT +echo "set global enable_experimental_rbac=1" | $BENDSQL_CLIENT_CONNECT echo "drop user if exists 'test-user'" | $BENDSQL_CLIENT_CONNECT echo "DROP FUNCTION IF EXISTS f1;" | $BENDSQL_CLIENT_CONNECT @@ -128,4 +128,4 @@ echo "DROP FUNCTION IF EXISTS f1;" | $BENDSQL_CLIENT_CONNECT echo "DROP FUNCTION IF EXISTS f2;" | $BENDSQL_CLIENT_CONNECT echo "drop table if exists default.t;" | $BENDSQL_CLIENT_CONNECT echo "drop table if exists default.t2;" | $BENDSQL_CLIENT_CONNECT -echo "unset experiment_enable_stage_udf_priv_check" | $BENDSQL_CLIENT_CONNECT +echo "unset enable_experimental_rbac" | $BENDSQL_CLIENT_CONNECT diff --git a/tests/suites/1_stateful/11_rbac/20_0012_privilege_access.sh b/tests/suites/1_stateful/11_rbac/20_0012_privilege_access.sh index 3d7fc06318847..41968aec071ff 100755 --- a/tests/suites/1_stateful/11_rbac/20_0012_privilege_access.sh +++ b/tests/suites/1_stateful/11_rbac/20_0012_privilege_access.sh @@ -6,7 +6,7 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) export TEST_USER_PASSWORD="password" export TEST_USER_CONNECT="bendsql --user=test-user --password=password --host=${QUERY_MYSQL_HANDLER_HOST} --port ${QUERY_HTTP_HANDLER_PORT}" -echo "set global experiment_enable_stage_udf_priv_check=1" | $BENDSQL_CLIENT_CONNECT +echo "set global enable_experimental_rbac=1" | $BENDSQL_CLIENT_CONNECT echo "drop user if exists 'test-user'" | $BENDSQL_CLIENT_CONNECT echo "drop role if exists 'test-role1'" | $BENDSQL_CLIENT_CONNECT @@ -179,4 +179,4 @@ echo "drop user a" | $BENDSQL_CLIENT_CONNECT echo "drop database if exists no_grant" | $BENDSQL_CLIENT_CONNECT echo "drop database grant_db" | $BENDSQL_CLIENT_CONNECT -echo "unset experiment_enable_stage_udf_priv_check" | $BENDSQL_CLIENT_CONNECT +echo "unset enable_experimental_rbac" | $BENDSQL_CLIENT_CONNECT