From 945dd1588b84a3d19248aff4b9bd32c2ca8766eb Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 17 Sep 2018 10:07:35 -0400 Subject: [PATCH] 13994: Use entire token for blob signatures. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../api/app/middlewares/arvados_api_token.rb | 3 ++ .../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(-) diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb index 4098fd72ca..acdc485811 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 8ea9f7bd88..12ef8eb3eb 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 85b12a377b..ddb85862a2 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 e6ecea219b..98c4bd11e4 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 6dbaa7550f..73b45f95ec 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 -- 2.30.2