From e6f3c9faab3f10d7efc7348be2ef85a6ea14766b Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 26 Oct 2020 22:55:46 -0400 Subject: [PATCH] 17040: Refactor trash check Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- services/api/app/models/arvados_model.rb | 26 +++++++++---------- ...200501150153_permission_table_constants.rb | 2 +- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 37f96c3ff0..86e9461529 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -296,21 +296,19 @@ class ArvadosModel < ApplicationRecord exclude_trashed_records = "AND (#{sql_table}.trash_at is NULL or #{sql_table}.trash_at > statement_timestamp())" end + trashed_check = "" + if !include_trash && sql_table != "api_client_authorizations" + trashed_check = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} " + + "where trash_at <= statement_timestamp()) #{exclude_trashed_records}" + end + if users_list.select { |u| u.is_admin }.any? # Admin skips most permission checks, but still want to filter on trashed items. - if !include_trash - if sql_table != "api_client_authorizations" - # Only include records where the owner is not trashed - sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} "+ - "where trash_at <= statement_timestamp()) #{exclude_trashed_records}" - end + if !include_trash && sql_table != "api_client_authorizations" + # Only include records where the owner is not trashed + sql_conds = trashed_check end else - trashed_check = "" - if !include_trash then - trashed_check = "AND target_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} where trash_at <= statement_timestamp())" - end - # The core of the permission check is a join against the # materialized_permissions table to determine if the user has at # least read permission to either the object itself or its @@ -333,7 +331,7 @@ class ArvadosModel < ApplicationRecord # Match a direct read permission link from the user to the record uuid direct_check = "#{sql_table}.uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+ - "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check})" + "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1)" # Match a read permission for the user to the record's # owner_uuid. This is so we can have a permissions table that @@ -354,7 +352,7 @@ class ArvadosModel < ApplicationRecord owner_check = "" if sql_table != "api_client_authorizations" and sql_table != "groups" then owner_check = "#{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+ - "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check} AND traverse_owned) " + "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 AND traverse_owned) " direct_check = " OR " + direct_check end @@ -367,7 +365,7 @@ class ArvadosModel < ApplicationRecord "(#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})))" end - sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) #{exclude_trashed_records}" + sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) #{trashed_check.empty? ? "" : "AND"} #{trashed_check}" end diff --git a/services/api/lib/20200501150153_permission_table_constants.rb b/services/api/lib/20200501150153_permission_table_constants.rb index 6e43a628c7..74c15bc2e9 100644 --- a/services/api/lib/20200501150153_permission_table_constants.rb +++ b/services/api/lib/20200501150153_permission_table_constants.rb @@ -63,7 +63,7 @@ def refresh_trashed INSERT INTO #{TRASHED_GROUPS} select ps.target_uuid as group_uuid, ps.trash_at from groups, lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps - where groups.owner_uuid like '_____-tpzed-_______________' + where groups.owner_uuid like '_____-tpzed-_______________' and ps.trash_at is not NULL }) end end -- 2.30.2