13561: Avoid old versions to be updated, with test.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 61946f82b014b8d3a2308db45aeb0eec005cee40..f051fb34458b2e9253f3eea23c03a3ebc6c1a119 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -27,6 +27,7 @@ class Collection < ArvadosModel
   validate :ensure_pdh_matches_manifest_text
   validate :ensure_storage_classes_desired_is_not_empty
   validate :ensure_storage_classes_contain_non_empty_strings
+  validate :old_versions_cannot_be_updated, on: :update
   before_save :set_file_names
   before_create :set_current_version_uuid
 
@@ -252,7 +253,10 @@ class Collection < ArvadosModel
           end
         end
         Collection.where('current_version_uuid = ? AND uuid != ?', uuid, uuid).each do |c|
-          c.update_attributes!(updates)
+          c.attributes = updates
+          # Use a different validation context to skip the 'old_versions_cannot_be_updated'
+          # validator, as on this case it is legal to update some fields.
+          c.save(context: :update_old_versions)
         end
         # Also update current object just in case a new version will be created,
         # as it has to receive the same values for the synced attributes.
@@ -585,4 +589,12 @@ class Collection < ArvadosModel
       end
     end
   end
+
+  def old_versions_cannot_be_updated
+    # We check for the '_was' values just in case the update operation
+    # includes a change on current_version_uuid or uuid.
+    if current_version_uuid_was != uuid_was
+      raise ArvadosModel::PermissionDeniedError.new("previous versions cannot be updated")
+    end
+  end
 end

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index e3c2a999cb1cc4bbd8a36febb45c424ff8458b20..31c76efa3e73031f94c2dc4177ff079a326a0a11 100644 (file)
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -128,6 +128,42 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  test "older versions should no be directly updatable" do
+    Rails.configuration.collection_versioning = true
+    act_as_system_user do
+      # Set up initial collection
+      c = create_collection 'foo', Encoding::US_ASCII
+      assert c.valid?
+      # Make changes so that a new version is created
+      c.update_attributes!({'name' => 'bar'})
+      c.reload
+      assert_equal 2, c.version
+      # Get the old version
+      c_old = Collection.where(current_version_uuid: c.uuid, version: 1).first
+      assert_not_nil c_old
+      # With collection versioning still being enabled, try to update
+      assert_raises ArvadosModel::PermissionDeniedError do
+        c_old.update_attributes(name: 'this was foo')
+      end
+      c_old.reload
+      assert_equal 'foo', c_old.name
+      # Try to fool the validator attempting to make c_old to look like a
+      # current version, it should also fail.
+      assert_raises ArvadosModel::PermissionDeniedError do
+        c_old.update_attributes(current_version_uuid: c_old.uuid)
+      end
+      c_old.reload
+      assert_equal c.uuid, c_old.current_version_uuid
+      # Now disable collection versioning, it should behave the same way
+      Rails.configuration.collection_versioning = false
+      assert_raises ArvadosModel::PermissionDeniedError do
+        c_old.update_attributes(name: 'this was foo')
+      end
+      c_old.reload
+      assert_equal 'foo', c_old.name
+    end
+  end
+
   [
     ['owner_uuid', 'zzzzz-tpzed-d9tiejq69daie8f', 'zzzzz-tpzed-xurymjxw79nv3jz'],
     ['replication_desired', 2, 3],