All tests pass!

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 6932dc77a593e2262ec17ecc91c75d19ccb773e1..e1b85aa89108a9bdf6897c69cd62c6c6f8cad120 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -44,11 +44,8 @@ class ApplicationController < ActionController::Base
     attrs_to_update = resource_attrs.reject { |k,v|
       [:kind, :etag, :href].index k
     }
-    if @object.update_attributes attrs_to_update
-      show
-    else
-      raise "Update failed"
-    end
+    @object.update_attributes! attrs_to_update
+    show
   end
 
   def destroy

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index b7b0e677d63d5b5d2e5389a27a42ab1f709f34ad..46dd0ecf8f368e7b6c71309800054f4d74b4be5b 100644 (file)
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -44,7 +44,7 @@ class Arvados::V1::JobsController < ApplicationController
 
   def cancel
     reload_object_before_update
-    @object.update_attributes cancelled_at: Time.now
+    @object.update_attributes! cancelled_at: Time.now
     show
   end
 

diff --git a/services/api/app/controllers/arvados/v1/keep_disks_controller.rb b/services/api/app/controllers/arvados/v1/keep_disks_controller.rb
index 7db295dbb2250be51f524969227bd3b7af086fc7..3d9191641ee5a1d0f92a65b6e76cb7cd7b99c86a 100644 (file)
--- a/services/api/app/controllers/arvados/v1/keep_disks_controller.rb
+++ b/services/api/app/controllers/arvados/v1/keep_disks_controller.rb
@@ -12,15 +12,18 @@ class Arvados::V1::KeepDisksController < ApplicationController
       service_ssl_flag: true
     }
   end
+
   def ping
     params[:service_host] ||= request.env['REMOTE_ADDR']
-    if not @object.ping params
-      return render_not_found "object not found"
+    act_as_system_user do
+      if not @object.ping params
+        return render_not_found "object not found"
+      end
+      # Render the :superuser view (i.e., include the ping_secret) even
+      # if !current_user.is_admin. This is safe because @object.ping's
+      # success implies the ping_secret was already known by the client.
+      render json: @object.as_api_response(:superuser)
     end
-    # Render the :superuser view (i.e., include the ping_secret) even
-    # if !current_user.is_admin. This is safe because @object.ping's
-    # success implies the ping_secret was already known by the client.
-    render json: @object.as_api_response(:superuser)
   end
 
   def find_objects_for_index

diff --git a/services/api/app/controllers/arvados/v1/nodes_controller.rb b/services/api/app/controllers/arvados/v1/nodes_controller.rb
index 1461eeccaa1481fc568eb2a0a8d91a8be8b18562..4415a511631df1b4646b974c7595816b9f9db71d 100644 (file)
--- a/services/api/app/controllers/arvados/v1/nodes_controller.rb
+++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb
@@ -13,18 +13,21 @@ class Arvados::V1::NodesController < ApplicationController
   def self._ping_requires_parameters
     { ping_secret: true }
   end
+
   def ping
-    @object = Node.where(uuid: (params[:id] || params[:uuid])).first
-    if !@object
-      return render_not_found
-    end
-    @object.ping({ ip: params[:local_ipv4] || request.env['REMOTE_ADDR'],
-                   ping_secret: params[:ping_secret],
-                   ec2_instance_id: params[:instance_id] })
-    if @object.info[:ping_secret] == params[:ping_secret]
-      render json: @object.as_api_response(:superuser)
-    else
-      raise "Invalid ping_secret after ping"
+    act_as_system_user do 
+      @object = Node.where(uuid: (params[:id] || params[:uuid])).first
+      if !@object
+        return render_not_found
+      end
+      @object.ping({ ip: params[:local_ipv4] || request.env['REMOTE_ADDR'],
+                     ping_secret: params[:ping_secret],
+                     ec2_instance_id: params[:instance_id] })
+      if @object.info[:ping_secret] == params[:ping_secret]
+        render json: @object.as_api_response(:superuser)
+      else
+        raise "Invalid ping_secret after ping"
+      end
     end
   end
 

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index ed19636fcb20953a84c2f9413402eca24c885137..e6f5b369181c4cb33ccca5c1f0265bfb28b36a5b 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -167,6 +167,10 @@ class ArvadosModel < ActiveRecord::Base
     attributes.keys.select { |a| a.match /_uuid$/ }
   end
 
+  def skip_uuid_read_permission_check
+    %w(modified_by_client_uuid)
+  end
+
   def normalize_collection_uuids
     foreign_key_attributes.each do |attr|
       attr_value = send attr
@@ -187,15 +191,18 @@ class ArvadosModel < ActiveRecord::Base
     specials = [system_user_uuid, 'd41d8cd98f00b204e9800998ecf8427e+0']
 
     foreign_key_attributes.each do |attr|
-      next if attr == "modified_by_client_uuid"
       begin
-        attr_value = send attr
-        r = ArvadosModel::resource_class_for_uuid attr_value if attr_value
-        if r and r.readable_by(current_user).where(uuid: attr_value).count == 0 and not specials.include? attr_value
-          errors.add(attr, "'#{attr_value}' not found")
+        if new_record? or send (attr + "_changed?")
+          attr_value = send attr
+          r = ArvadosModel::resource_class_for_uuid attr_value if attr_value
+          r = r.readable_by(current_user) if r and not skip_uuid_read_permission_check.include? attr
+          if r and r.where(uuid: attr_value).count == 0 and not specials.include? attr_value
+            errors.add(attr, "'#{attr_value}' not found")
+          end
         end
       rescue Exception => e
-          errors.add(attr, "'#{attr_value}' error #{e}")
+        bt = e.backtrace.join("\n")
+        errors.add(attr, "'#{attr_value}' error '#{e}'\n#{bt}\n")
       end
     end
   end

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 1f0ef758d97a893664e9e82fb9131b90de688995..e9ec10d53ea7808305b468cc0308c94af1e93230 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -62,6 +62,10 @@ class Job < ArvadosModel
     super + %w(output log)
   end
 
+  def skip_uuid_read_permission_check
+    super + %w(cancelled_by_client_uuid)
+  end
+
   def ensure_script_version_is_commit
     if self.is_locked_by_uuid and self.started_at
       # Apparently client has already decided to go for it. This is

diff --git a/services/api/app/models/keep_disk.rb b/services/api/app/models/keep_disk.rb
index 0998fcd84a1b5f9ba855aebbcf4440426f598487..77fc6278eba531f6baa1acf997044aaf893121c6 100644 (file)
--- a/services/api/app/models/keep_disk.rb
+++ b/services/api/app/models/keep_disk.rb
@@ -22,6 +22,10 @@ class KeepDisk < ArvadosModel
     t.add :ping_secret
   end
 
+  def foreign_key_attributes
+    super.reject { |a| a == "filesystem_uuid" }
+  end
+
   def ping(o)
     raise "must have :service_host and :ping_secret" unless o[:service_host] and o[:ping_secret]
 
@@ -31,7 +35,7 @@ class KeepDisk < ArvadosModel
     end
 
     @bypass_arvados_authorization = true
-    self.update_attributes(o.select { |k,v|
+    self.update_attributes!(o.select { |k,v|
                              [:service_host,
                               :service_port,
                               :service_ssl_flag,