Merge branch '19145-admin-write-frozen'

fixes #19145

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index bbb2378f5c56becac22646212beb343549da5170..1d1d83662c17f68b6611486624714555c6b854e2 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -110,7 +110,6 @@ class User < ArvadosModel
   end
 
   def can?(actions)
-    return true if is_admin
     actions.each do |action, target|
       unless target.nil?
         if target.respond_to? :uuid
@@ -126,7 +125,7 @@ class User < ArvadosModel
 
       user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: "$1", perm_level: "$3"}
 
-      unless ActiveRecord::Base.connection.
+      if !is_admin && !ActiveRecord::Base.connection.
         exec_query(%{
 SELECT 1 FROM #{PERMISSION_VIEW}
   WHERE user_uuid in (#{user_uuids_subquery}) and

diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 7a16962402c7a4609bfea6dddd5703526a4a2cbc..a3bcd4e3568acea466bc52a743cd108b59a8bcc0 100644 (file)
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -398,66 +398,70 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
         properties: {'frobity' => 'bar baz'})
       assert ok, proj.errors.messages.inspect
 
-      # Once project is frozen, cannot create new items inside it or
-      # its descendants
-      [proj, proj_inner].each do |frozen|
-        assert_raises do
-          collections(:collection_owned_by_active).update_attributes!(owner_uuid: frozen.uuid)
-        end
-        assert_raises do
-          Collection.create!(owner_uuid: frozen.uuid, name: 'inside-frozen-project')
-        end
-        assert_raises do
-          Group.create!(owner_uuid: frozen.uuid, group_class: 'project', name: 'inside-frozen-project')
-        end
-        cr = ContainerRequest.new(test_cr_attrs.merge(owner_uuid: frozen.uuid))
-        assert_raises ArvadosModel::PermissionDeniedError do
-          cr.save
-        end
-        assert_match /frozen/, cr.errors.inspect
-        # Check the frozen-parent condition is the only reason save failed.
-        cr.owner_uuid = users(:active).uuid
-        assert cr.save
-        cr.destroy
-      end
-
-      # Once project is frozen, cannot change name/contents, move,
-      # trash, or delete the project or anything beneath it
-      [proj, proj_inner, coll].each do |frozen|
-        assert_raises(StandardError, "should reject rename of #{frozen.uuid} (#{frozen.name}) with parent #{frozen.owner_uuid}") do
-          frozen.update_attributes!(name: 'foo2')
-        end
-        frozen.reload
-
-        if frozen.is_a?(Collection)
-          assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do
-            frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
+      [:active, :admin].each do |u|
+        act_as_user users(u) do
+          # Once project is frozen, cannot create new items inside it or
+          # its descendants
+          [proj, proj_inner].each do |frozen|
+            assert_raises do
+              collections(:collection_owned_by_active).update_attributes!(owner_uuid: frozen.uuid)
+            end
+            assert_raises do
+              Collection.create!(owner_uuid: frozen.uuid, name: 'inside-frozen-project')
+            end
+            assert_raises do
+              Group.create!(owner_uuid: frozen.uuid, group_class: 'project', name: 'inside-frozen-project')
+            end
+            cr = ContainerRequest.new(test_cr_attrs.merge(owner_uuid: frozen.uuid))
+            assert_raises ArvadosModel::PermissionDeniedError do
+              cr.save
+            end
+            assert_match /frozen/, cr.errors.inspect
+            # Check the frozen-parent condition is the only reason save failed.
+            cr.owner_uuid = users(u).uuid
+            assert cr.save
+            cr.destroy
           end
-        else
-          assert_raises(StandardError, "should reject moving a project into #{frozen.uuid}") do
-            groups(:private).update_attributes!(owner_uuid: frozen.uuid)
-          end
-        end
-        frozen.reload
 
-        assert_raises(StandardError, "should reject moving #{frozen.uuid} to a different parent project") do
-          frozen.update_attributes!(owner_uuid: groups(:private).uuid)
-        end
-        frozen.reload
-        assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do
-          frozen.update_attributes!(trash_at: db_current_time)
-        end
-        frozen.reload
-        assert_raises(StandardError, "should reject setting delete_at of #{frozen.uuid}") do
-          frozen.update_attributes!(delete_at: db_current_time)
-        end
-        frozen.reload
-        assert_raises(StandardError, "should reject delete of #{frozen.uuid}") do
-          frozen.destroy
-        end
-        frozen.reload
-        if frozen != proj
-          assert_equal [], frozen.writable_by
+          # Once project is frozen, cannot change name/contents, move,
+          # trash, or delete the project or anything beneath it
+          [proj, proj_inner, coll].each do |frozen|
+            assert_raises(StandardError, "should reject rename of #{frozen.uuid} (#{frozen.name}) with parent #{frozen.owner_uuid}") do
+              frozen.update_attributes!(name: 'foo2')
+            end
+            frozen.reload
+
+            if frozen.is_a?(Collection)
+              assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do
+                frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
+              end
+            else
+              assert_raises(StandardError, "should reject moving a project into #{frozen.uuid}") do
+                groups(:private).update_attributes!(owner_uuid: frozen.uuid)
+              end
+            end
+            frozen.reload
+
+            assert_raises(StandardError, "should reject moving #{frozen.uuid} to a different parent project") do
+              frozen.update_attributes!(owner_uuid: groups(:private).uuid)
+            end
+            frozen.reload
+            assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do
+              frozen.update_attributes!(trash_at: db_current_time)
+            end
+            frozen.reload
+            assert_raises(StandardError, "should reject setting delete_at of #{frozen.uuid}") do
+              frozen.update_attributes!(delete_at: db_current_time)
+            end
+            frozen.reload
+            assert_raises(StandardError, "should reject delete of #{frozen.uuid}") do
+              frozen.destroy
+            end
+            frozen.reload
+            if frozen != proj
+              assert_equal [], frozen.writable_by
+            end
+          end
         end
       end