From 2e727c5d2d000faa6f1d9a566dc59568f1b276fe Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 10 Jun 2022 11:44:33 -0400 Subject: [PATCH] 19146: Remove unneeded special case checks, explain the needed one. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- services/api/app/models/arvados_model.rb | 16 ++++++++++------ services/api/app/models/user.rb | 8 ++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index e7ffe740b1..c2725506c0 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -275,18 +275,22 @@ class ArvadosModel < ApplicationRecord def can_write if respond_to?(:frozen_by_uuid) && frozen_by_uuid + # This special case is needed to return the correct value from a + # "freeze project" API, during which writable status changes + # from true to false. + # + # current_user.can?(write: self) returns true (which is correct + # in the context of permission-checking hooks) but the can_write + # value we're returning to the caller here represents the state + # _after_ the update, i.e., false. return false else - return owner_uuid == current_user.uuid || - current_user.is_admin || - current_user.can?(write: uuid) + return current_user.can?(write: self) end end def can_manage - return owner_uuid == current_user.uuid || - current_user.is_admin || - current_user.can?(manage: uuid) + return current_user.can?(manage: self) end # Return a query with read permissions restricted to the union of the diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index 4449466135..141bccef21 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -124,6 +124,14 @@ class User < ArvadosModel end next if target_uuid == self.uuid + if action == :write && target && !target.new_record? && + target.respond_to?(:frozen_by_uuid) && + target.frozen_by_uuid_was + # Just an optimization to skip the PERMISSION_VIEW and + # FrozenGroup queries below + return false + end + target_owner_uuid = target.owner_uuid if target.respond_to? :owner_uuid user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: "$1", perm_level: "$3"} -- 2.30.2