[ARVADOS] updated: d71020ac08d7b6e84d2d8f8d2c9b22d512144baa

Git user git at public.curoverse.com
Fri Oct 14 09:39:54 EDT 2016


Summary of changes:
 services/api/app/models/container.rb | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

       via  d71020ac08d7b6e84d2d8f8d2c9b22d512144baa (commit)
      from  23a37f77f36717f60884c2b8054a9670b35e611b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit d71020ac08d7b6e84d2d8f8d2c9b22d512144baa
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Oct 14 09:39:48 2016 -0400

    10172: More work giving auth_uuid limited permission to set progress and output
    on container.

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index d96af23..6b5bc4f 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -241,14 +241,12 @@ class Container < ArvadosModel
   end
 
   def validate_lock
-    # If the Container is already locked by someone other than the
-    # current api_client_auth, disallow all changes -- except
-    # priority, which needs to change to reflect max(priority) of
-    # relevant ContainerRequests.
-    if !locked_by_uuid_was.nil? and locked_by_uuid_was != Thread.current[:api_client_authorization].uuid
-      if auth_uuid_was == Thread.current[:api_client_authorization].uuid
-        check_update_whitelist [:priority, :output, :progress]
-      else
+    if locked_by_uuid_was
+      if not [locked_by_uuid_was, auth_uuid].include? Thread.current[:api_client_authorization].uuid
+        # The container is locked, but is being accessed by an API token that
+        # isn't associated with the container.  Only the priority field may be
+        # updated, needed when updating to max(priority) of relevant
+        # ContainerRequests.
         check_update_whitelist [:priority]
       end
     end
@@ -273,14 +271,16 @@ class Container < ArvadosModel
   end
 
   def validate_output
+    # Output must be exist and be readable by the current user.  This is so
+    # that a container cannot "claim" a collection that it doesn't otherwise
+    # have access to just by setting the output field to the collection PDH.
     if output_changed?
-      apiauth = ApiClientAuthorization.find_by_uuid(uuid: auth_uuid)
       c = Collection.
-          readable_by(User.find_by_id(apiauth.user_id)).
+          readable_by(current_user).
           where(portable_data_hash: self.output).
           first
       if !c
-        raise #ArvadosModel::UnresolvableContainerError.new "cannot mount collection #{uuid.inspect}: not found"
+        return errors.add :output, "collection must exist and be readable by current user."
       end
     end
   end
@@ -322,6 +322,17 @@ class Container < ArvadosModel
     end
   end
 
+  def ensure_owner_uuid_is_permitted
+    # Override base permission check to allow auth_uuid to set progress and
+    # output (only).  Whether it is legal to set progress and output in the current
+    # state has already been checked in validate_change.
+    if self.auth_uuid == Thread.current[:api_client_authorization].uuid
+      check_update_whitelist [:progress, :output]
+    else
+      super
+    end
+  end
+
   def handle_completed
     # This container is finished so finalize any associated container requests
     # that are associated with this container.

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list