Skip to content

Commit

Permalink
fix execute/desc task access bug & tasks table bug
Browse files Browse the repository at this point in the history
  • Loading branch information
TCeason committed May 20, 2024
1 parent 59eee4a commit 8dd2f82
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 7 deletions.
10 changes: 4 additions & 6 deletions src/query/service/src/interpreters/access/privilege_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,21 +722,19 @@ impl AccessChecker for PrivilegeAccess {
}
Plan::DescribeTask(plan) => {
let session = self.ctx.get_current_session();
if self
if !self
.has_ownership(&session, &GrantObject::Task(plan.task_name.to_owned()))
.await
.is_err()
.await?
{
self.validate_access(&GrantObject::Global, UserPrivilegeType::Super)
.await?;
}
}
Plan::ExecuteTask(plan) => {
let session = self.ctx.get_current_session();
if self
if !self
.has_ownership(&session, &GrantObject::Task(plan.task_name.to_owned()))
.await
.is_err()
.await?
{
self.validate_access(&GrantObject::Global, UserPrivilegeType::Super)
.await?;
Expand Down
4 changes: 4 additions & 0 deletions src/query/storages/system/src/task_history_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::sync::Arc;

use chrono_tz::Tz::UTC;
use log::info;
use databend_common_catalog::plan::PushDownInfo;
use databend_common_catalog::table::Table;
use databend_common_catalog::table_context::TableContext;
Expand Down Expand Up @@ -194,10 +195,13 @@ impl AsyncSystemTable for TaskHistoryTable {
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN)
&& !owned_tasks_names.contains(task_name)
{
info!("--task_history:198 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}", all_effective_roles.clone(), owned_tasks_names.clone(), task_name.clone());
return parse_task_runs_to_datablock(vec![]);
}
}

info!("--task_history:203 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}", all_effective_roles.clone(), owned_tasks_names.clone(), task_name.clone());

let req = ShowTaskRunsRequest {
tenant_id: tenant.tenant_name().to_string(),
scheduled_time_start: scheduled_time_start.unwrap_or("".to_string()),
Expand Down
34 changes: 33 additions & 1 deletion src/query/storages/system/src/tasks_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

use std::sync::Arc;
use log::info;

use databend_common_catalog::plan::PushDownInfo;
use databend_common_catalog::table::Table;
Expand All @@ -32,14 +33,19 @@ use databend_common_expression::types::UInt64Type;
use databend_common_expression::types::VariantType;
use databend_common_expression::DataBlock;
use databend_common_expression::FromData;
use databend_common_expression::Scalar;
use databend_common_functions::BUILTIN_FUNCTIONS;
use databend_common_meta_app::schema::TableIdent;
use databend_common_meta_app::schema::TableInfo;
use databend_common_meta_app::schema::TableMeta;
use databend_common_sql::plans::task_schema;
use databend_common_users::UserApiProvider;
use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN;

use crate::parse_task_runs_to_datablock;
use crate::table::AsyncOneBlockSystemTable;
use crate::table::AsyncSystemTable;
use crate::util::find_eq_filter;
use crate::util::get_owned_task_names;

pub fn parse_tasks_to_datablock(tasks: Vec<Task>) -> Result<DataBlock> {
Expand Down Expand Up @@ -120,7 +126,7 @@ impl AsyncSystemTable for TasksTable {
async fn get_full_data(
&self,
ctx: Arc<dyn TableContext>,
_push_downs: Option<PushDownInfo>,
push_downs: Option<PushDownInfo>,
) -> Result<DataBlock> {
let user_api = UserApiProvider::instance();
let config = GlobalConfig::instance();
Expand All @@ -140,7 +146,33 @@ impl AsyncSystemTable for TasksTable {
.map(|x| x.identity().to_string())
.collect();

let mut task_name = None;
if let Some(push_downs) = push_downs {
if let Some(filter) = push_downs.filters.as_ref().map(|f| &f.filter) {
let expr = filter.as_expr(&BUILTIN_FUNCTIONS);
find_eq_filter(&expr, &mut |col_name, scalar| {
if col_name == "name" {
if let Scalar::String(s) = scalar {
task_name = Some(s.clone());
}
}
});
}
}
let owned_tasks_names = get_owned_task_names(user_api, &tenant, &all_effective_roles).await;
if let Some(task_name) = &task_name {
// The user does not have admin role and not own the task_name
// Need directly return empty block
if !all_effective_roles
.iter()
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN)
&& !owned_tasks_names.contains(task_name)
{
info!("--tasks:171 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}", all_effective_roles.clone(), owned_tasks_names.clone(), task_name.clone());
return parse_task_runs_to_datablock(vec![]);
}
}
info!("--tasks:175 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}", all_effective_roles.clone(), owned_tasks_names.clone(), task_name.clone());

let req = ShowTasksRequest {
tenant_id: tenant.tenant_name().to_string(),
Expand Down

0 comments on commit 8dd2f82

Please sign in to comment.