13994: Use entire token for blob signatures.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb
index 4098fd72ca436bdf3ee806b5a0dfb938fe7a5a9b..acdc4858118fcb4c3fd5be1a1a65208ed72ff530 100644 (file)
--- a/services/api/app/middlewares/arvados_api_token.rb
+++ b/services/api/app/middlewares/arvados_api_token.rb
@@ -39,6 +39,7 @@ class ArvadosApiToken
     # Set current_user etc. based on the primary session token if a
     # valid one is present. Otherwise, use the first valid token in
     # reader_tokens.
+    accepted = false
     auth = nil
     [params["api_token"],
      params["oauth_token"],
@@ -50,6 +51,7 @@ class ArvadosApiToken
                  validate(token: supplied, remote: remote)
       if try_auth.andand.user
         auth = try_auth
+        accepted = supplied
         break
       end
     end
@@ -58,6 +60,7 @@ class ArvadosApiToken
     Thread.current[:api_client_authorization] = auth
     Thread.current[:api_client_uuid] = auth.andand.api_client.andand.uuid
     Thread.current[:api_client] = auth.andand.api_client
+    Thread.current[:token] = accepted
     Thread.current[:user] = auth.andand.user
 
     @app.call env if @app

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 8ea9f7bd885a396541b2e1db9f6c9c55688ba870..12ef8eb3eb5a2abede54c35919a8b72c815a357c 100644 (file)
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -204,6 +204,18 @@ class ApiClientAuthorization < ArvadosModel
     return nil
   end
 
+  def token
+    v2token
+  end
+
+  def v1token
+    api_token
+  end
+
+  def v2token
+    'v2/' + uuid + '/' + api_token
+  end
+
   protected
 
   def permission_to_create

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 85b12a377b15d5445fa241375b8d6a422977fbee..ddb85862a26c6af6751680a8358147ce4fe841f6 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -94,7 +94,7 @@ class Collection < ArvadosModel
       # Check permissions on the collection manifest.
       # If any signature cannot be verified, raise PermissionDeniedError
       # which will return 403 Permission denied to the client.
-      api_token = current_api_client_authorization.andand.api_token
+      api_token = Thread.current[:token]
       signing_opts = {
         api_token: api_token,
         now: @validation_timestamp.to_i,
@@ -244,7 +244,7 @@ class Collection < ArvadosModel
     elsif is_trashed
       return manifest_text
     else
-      token = current_api_client_authorization.andand.api_token
+      token = Thread.current[:token]
       exp = [db_current_time.to_i + Rails.configuration.blob_signature_ttl,
              trash_at].compact.map(&:to_i).min
       self.class.sign_manifest manifest_text, token, exp

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index e6ecea219b9da1ea4c71fee07dddf025aa78aab3..98c4bd11e44db845e99a881d1239c6d9f0ddb87e 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -14,11 +14,21 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
     Rails.configuration.permit_create_collection_with_unsigned_manifest = isok
   end
 
-  def assert_signed_manifest manifest_text, label=''
+  def assert_signed_manifest manifest_text, label='', token: false
     assert_not_nil manifest_text, "#{label} manifest_text was nil"
     manifest_text.scan(/ [[:xdigit:]]{32}\S*/) do |tok|
       assert_match(PERM_TOKEN_RE, tok,
                    "Locator in #{label} manifest_text was not signed")
+      if token
+        bare = tok.gsub(/\+A[^\+]*/, '').sub(/^ /, '')
+        exp = tok[/\+A[[:xdigit:]]+@([[:xdigit:]]+)/, 1].to_i(16)
+        sig = Blob.sign_locator(
+          bare,
+          key: Rails.configuration.blob_signing_key,
+          expire: exp,
+          api_token: token)[/\+A[^\+]*/, 0]
+        assert_includes tok, sig
+      end
     end
   end
 
@@ -52,6 +62,33 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
     refute_includes json_response, 'unsigned_manifest_text'
   end
 
+  ['v1token', 'v2token'].each do |token_method|
+    test "correct signatures are given for #{token_method}" do
+      token = api_client_authorizations(:active).send(token_method)
+      authorize_with_token token
+      get :show, {id: collections(:foo_file).uuid}
+      assert_response :success
+      assert_signed_manifest json_response['manifest_text'], 'foo_file', token: token
+    end
+
+    test "signatures with #{token_method} are accepted" do
+      token = api_client_authorizations(:active).send(token_method)
+      signed = Blob.sign_locator(
+        'acbd18db4cc2f85cedef654fccc4a4d8+3',
+        key: Rails.configuration.blob_signing_key,
+        api_token: token)
+      authorize_with_token token
+      put :update, {
+            id: collections(:collection_owned_by_active).uuid,
+            collection: {
+              manifest_text: ". #{signed} 0:3:foo.txt\n",
+            },
+          }
+      assert_response :success
+      assert_signed_manifest json_response['manifest_text'], 'updated', token: token
+    end
+  end
+
   test "index with manifest_text selected returns signed locators" do
     columns = %w(uuid owner_uuid manifest_text)
     authorize_with :active

diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 6dbaa7550f55a8e49b035e6092c331304c6e4edb..73b45f95ec71a7b28564c8a5767eb48503ec5465 100644 (file)
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -65,6 +65,7 @@ class ActiveSupport::TestCase
     Thread.current[:api_client_authorization] = nil
     Thread.current[:api_client_uuid] = nil
     Thread.current[:api_client] = nil
+    Thread.current[:token] = nil
     Thread.current[:user] = nil
     restore_configuration
   end
@@ -110,6 +111,7 @@ class ActiveSupport::TestCase
     Thread.current[:api_client_authorization] = client_auth
     Thread.current[:api_client] = client_auth.api_client
     Thread.current[:user] = client_auth.user
+    Thread.current[:token] = client_auth.token
   end
 
   def expect_json
@@ -188,6 +190,7 @@ class ActionDispatch::IntegrationTest
     Thread.current[:api_client_authorization] = nil
     Thread.current[:api_client_uuid] = nil
     Thread.current[:api_client] = nil
+    Thread.current[:token] = nil
     Thread.current[:user] = nil
   end
 end