From 317952f20cb0d979d35b66bc735d02c2ea69050e Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curoverse.com>
Date: Thu, 14 Aug 2014 22:53:24 -0400
Subject: [PATCH] 3036: ensure_owner_uuid_is_permitted checks that owner_uuid
 is a User or Group. Added comments on permission logic.

---
 services/api/app/models/arvados_model.rb | 28 +++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 7f1f1e1e42..52fe56d5a2 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -175,7 +175,7 @@ class ArvadosModel < ActiveRecord::Base
     if new_record? or owner_uuid_changed?
       uuid_in_path = {owner_uuid => true, uuid => true}
       x = owner_uuid
-      while (owner_class = self.class.resource_class_for_uuid(x)) != User
+      while (owner_class = ArvadosModel::resource_class_for_uuid(x)) != User
         begin
           if x == uuid
             # Test for cycles with the new version, not the DB contents
@@ -205,12 +205,24 @@ class ArvadosModel < ActiveRecord::Base
 
   def ensure_owner_uuid_is_permitted
     raise PermissionDeniedError if !current_user
+
     if new_record? and respond_to? :owner_uuid=
       self.owner_uuid ||= current_user.uuid
     end
-    # Verify permission to write to old owner (unless owner_uuid was
-    # nil -- or hasn't changed, in which case the following
-    # "permission to write to new owner" block will take care of us)
+
+    rsc_class = ArvadosModel::resource_class_for_uuid owner_uuid
+    unless rsc_class == User or rsc_class == Group
+      errors.add :owner_uuid, "can only be set to User or Group"
+      raise PermissionDeniedError
+    end
+
+    # Verify "write" permission on old owner
+    # default fail unless one of:
+    # owner_uuid did not change
+    # previous owner_uuid is nil
+    # current user is the old owner
+    # current user is this object
+    # current user can_write old owner
     unless !owner_uuid_changed? or
         owner_uuid_was.nil? or
         current_user.uuid == self.owner_uuid_was or
@@ -220,12 +232,18 @@ class ArvadosModel < ActiveRecord::Base
       errors.add :owner_uuid, "cannot be changed without write permission on old owner"
       raise PermissionDeniedError
     end
-    # Verify permission to write to new owner
+
+    # Verify "write" permission on new owner
+    # default fail unless one of:
+    # current_user is this object
+    # current user can_write new owner
     unless current_user == self or current_user.can? write: owner_uuid
       logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{uuid} but does not have permission to write new owner_uuid #{owner_uuid}"
       errors.add :owner_uuid, "cannot be changed without write permission on new owner"
       raise PermissionDeniedError
     end
+
+    true
   end
 
   def ensure_permission_to_save
-- 
2.30.2