[ARVADOS] created: ee91e2ebc7759e1823edd376db938a785ad62f45

[git at public.curoverse.com]

        at  ee91e2ebc7759e1823edd376db938a785ad62f45 (commit)


commit ee91e2ebc7759e1823edd376db938a785ad62f45
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Thu Jul 31 22:45:09 2014 -0400

    Add check to JobsController#create that collection output of a job is readable
    by current user before attempting to re-use job.  Added unit test.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 3039433..6bd2c4d 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -67,18 +67,20 @@ class Arvados::V1::JobsController < ApplicationController
         if j.nondeterministic != true and
             ((j.success == true and j.output != nil) or j.running == true) and
             j.script_parameters == resource_attrs[:script_parameters]
-          if j.running
+          if j.running && j.owner_uuid == current_user.uuid
             # We'll use this if we don't find a job that has completed
             incomplete_job ||= j
           else
-            # Record the first job in the list
-            if !@object
-              @object = j
-            end
-            # Ensure that all candidate jobs actually did produce the same output
-            if @object.output != j.output
-              @object = nil
-              break
+            if Collection.readable_by(current_user).find_by_uuid(j.output)
+              # Record the first job in the list
+              if !@object
+                @object = j
+              end
+              # Ensure that all candidate jobs actually did produce the same output
+              if @object.output != j.output
+                @object = nil
+                break
+              end
             end
           end
         end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 4d465a6..4fa4162 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -156,3 +156,9 @@ anonymous:
   api_token: 4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi
   expires_at: 2038-01-01 00:00:00
   scopes: ["GET /"]
+
+job_reader:
+  api_client: untrusted
+  user: job_reader
+  api_token: e99512cdc0f3415c2428b9758f33bdfb07bc3561b00e86e7e6
+  expires_at: 2038-01-01 00:00:00
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index c221c7f..1d95599 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -633,3 +633,33 @@ anonymous_group_can_read_empty_collection:
   name: can_read
   tail_uuid: zzzzz-j7d0g-anonymouspublic
   head_uuid: d41d8cd98f00b204e9800998ecf8427e+0
+
+job_reader_can_read_previous_job_run:
+  # Permission link giving job_reader permission
+  # to read previous_job_run
+  uuid: zzzzz-o0j2j-8bbd851795ebafd
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-06-13 20:42:26 -0800
+  modified_by_client_uuid: zzzzz-tpzed-000000000000000
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-13 20:42:26 -0800
+  updated_at: 2014-06-13 20:42:26 -0800
+  link_class: permission
+  name: can_read
+  tail_uuid: zzzzz-tpzed-905b42d1dd4a354
+  head_uuid: zzzzz-8i9sb-cjs4pklxxjykqqq
+
+foo_repo_readable_by_job_reader:
+  # Permission link giving job_reader permission
+  # to read previous_job_run
+  uuid: zzzzz-o0j2j-072ec05dc9487f8
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-06-13 20:42:26 -0800
+  modified_by_client_uuid: zzzzz-tpzed-000000000000000
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-13 20:42:26 -0800
+  updated_at: 2014-06-13 20:42:26 -0800
+  link_class: permission
+  name: can_read
+  tail_uuid: zzzzz-tpzed-905b42d1dd4a354
+  head_uuid: zzzzz-s0uqq-382brsig8rp3666
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index acb67b4..0e02b7d 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -119,3 +119,14 @@ anonymous:
   is_active: false
   is_admin: false
   prefs: {}
+
+job_reader:
+  owner_uuid: zzzzz-tpzed-000000000000000
+  uuid: zzzzz-tpzed-905b42d1dd4a354
+  email: jobber at arvados.local
+  first_name: Job
+  last_name: Er
+  identity_url: https://spectator.openid.local
+  is_active: true
+  is_admin: false
+  prefs: {}
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index f17923d..01f2b9c 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -110,6 +110,27 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
   end
 
+  test "do not reuse job because output is not readable by user" do
+    authorize_with :job_reader
+    post :create, {
+      job: {
+        script: "hash",
+        script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+        repository: "foo",
+        script_parameters: {
+          input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+          an_integer: '1'
+        }
+      },
+      find_or_create: true
+    }
+    assert_response :success
+    assert_not_nil assigns(:object)
+    new_job = JSON.parse(@response.body)
+    assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+    assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
+  end
+
   test "test_cannot_reuse_job_no_output" do
     post :create, job: {
       no_reuse: false,

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


hooks/post-receive
--