14262: Fix permissions so runtime_token can set container progress/output

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 0d8453174e205e85ab3f79e01a32cc530478a4a1..cd763a8e7e1eb0d851f08517b730ddf9f230a113 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -493,10 +493,14 @@ class Container < ArvadosModel
       return false
     end
 
-    if current_api_client_authorization.andand.uuid.andand == self.auth_uuid
-      # The contained process itself can update progress indicators,
-      # but can't change priority etc.
-      permitted = permitted & (progress_attrs + final_attrs + [:state] - [:log])
+    if self.state == Running &&
+       !current_api_client_authorization.nil? &&
+       (current_api_client_authorization.uuid == self.auth_uuid ||
+        current_api_client_authorization.token == self.runtime_token)
+      # The contained process itself can write final attrs but can't
+      # change priority or log.
+      permitted.push *final_attrs
+      permitted = permitted - [:log, :priority]
     elsif self.locked_by_uuid && self.locked_by_uuid != current_api_client_authorization.andand.uuid
       # When locked, progress fields cannot be updated by the wrong
       # dispatcher, even though it has admin privileges.

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 491022ad8d5a9cd6e47e1cf7727a5cba92d54ce4..90b4f13bf597b5b9ea306dec04b698e75fb98ae3 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -777,25 +777,41 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal [logpdh_time2], Collection.where(uuid: [cr1log_uuid, cr2log_uuid]).to_a.collect(&:portable_data_hash).uniq
   end
 
-  test "auth_uuid can set output, progress, runtime_status, state on running container -- but not log" do
-    set_user_from_auth :active
-    c, _ = minimal_new
-    set_user_from_auth :dispatch1
-    c.lock
-    c.update_attributes! state: Container::Running
-
-    auth = ApiClientAuthorization.find_by_uuid(c.auth_uuid)
-    Thread.current[:api_client_authorization] = auth
-    Thread.current[:api_client] = auth.api_client
-    Thread.current[:token] = auth.token
-    Thread.current[:user] = auth.user
+  ["auth_uuid", "runtime_token"].each do |tok|
+    test "#{tok} can set output, progress, runtime_status, state on running container -- but not log" do
+      if tok == "runtime_token"
+        set_user_from_auth :spectator
+        c, _ = minimal_new(container_image: "9ae44d5792468c58bcf85ce7353c7027+124",
+                           runtime_token: api_client_authorizations(:active).token)
+      else
+        set_user_from_auth :active
+        c, _ = minimal_new
+      end
+      set_user_from_auth :dispatch1
+      c.lock
+      c.update_attributes! state: Container::Running
+
+      if tok == "runtime_token"
+        auth = ApiClientAuthorization.validate(token: c.runtime_token)
+        Thread.current[:api_client_authorization] = auth
+        Thread.current[:api_client] = auth.api_client
+        Thread.current[:token] = auth.token
+        Thread.current[:user] = auth.user
+      else
+        auth = ApiClientAuthorization.find_by_uuid(c.auth_uuid)
+        Thread.current[:api_client_authorization] = auth
+        Thread.current[:api_client] = auth.api_client
+        Thread.current[:token] = auth.token
+        Thread.current[:user] = auth.user
+      end
 
-    assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash)
-    assert c.update_attributes(runtime_status: {'warning' => 'something happened'})
-    assert c.update_attributes(progress: 0.5)
-    refute c.update_attributes(log: collections(:real_log_collection).portable_data_hash)
-    c.reload
-    assert c.update_attributes(state: Container::Complete, exit_code: 0)
+      assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash)
+      assert c.update_attributes(runtime_status: {'warning' => 'something happened'})
+      assert c.update_attributes(progress: 0.5)
+      refute c.update_attributes(log: collections(:real_log_collection).portable_data_hash)
+      c.reload
+      assert c.update_attributes(state: Container::Complete, exit_code: 0)
+    end
   end
 
   test "not allowed to set output that is not readable by current user" do