[ARVADOS] updated: 1.2.0-42-g945dd1588

Git user git at public.curoverse.com
Mon Sep 17 10:10:33 EDT 2018


Summary of changes:
 services/api/app/middlewares/arvados_api_token.rb  |  3 ++
 .../api/app/models/api_client_authorization.rb     | 12 +++++++
 services/api/app/models/collection.rb              |  4 +--
 .../arvados/v1/collections_controller_test.rb      | 39 +++++++++++++++++++++-
 services/api/test/test_helper.rb                   |  3 ++
 5 files changed, 58 insertions(+), 3 deletions(-)

       via  945dd1588b84a3d19248aff4b9bd32c2ca8766eb (commit)
      from  21fe33427ce1e726e527c37716b30a519e1ebb94 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 945dd1588b84a3d19248aff4b9bd32c2ca8766eb
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Sep 17 10:07:35 2018 -0400

    13994: Use entire token for blob signatures.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb
index 4098fd72c..acdc48581 100644
--- 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 8ea9f7bd8..12ef8eb3e 100644
--- 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 85b12a377..ddb85862a 100644
--- 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 e6ecea219..98c4bd11e 100644
--- 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 6dbaa7550..73b45f95e 100644
--- 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

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list