From 01f52958d899c62b0b1f9a39fda960136835850c Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 16 Oct 2017 23:24:45 -0400 Subject: [PATCH] 11453: Accept salted tokens at /users/current and /groups. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../app/controllers/application_controller.rb | 14 +++++- .../api/app/middlewares/arvados_api_token.rb | 43 ++++++++++++------- .../app/models/api_client_authorization.rb | 16 +++++++ .../arvados/v1/groups_controller_test.rb | 11 +++++ .../arvados/v1/users_controller_test.rb | 16 +++++++ services/api/test/test_helper.rb | 8 ++++ 6 files changed, 92 insertions(+), 16 deletions(-) diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb index d09283d928..b3718e1021 100644 --- a/services/api/app/controllers/application_controller.rb +++ b/services/api/app/controllers/application_controller.rb @@ -344,7 +344,19 @@ class ApplicationController < ActionController::Base .all end @read_auths.select! { |auth| auth.scopes_allow_request? request } - @read_users = @read_auths.map { |auth| auth.user }.uniq + + # Use a salted token as a reader token for /groups/ and /users/current + if params[:remote_id] && ( + request.path.start_with?('/arvados/v1/groups') || + request.path.start_with?('/arvados/v1/users/current')) + auth = ApiClientAuthorization.validate(remote_id: params[:remote_id]) + if auth && auth.user + Thread.current[:user] = auth.user + @read_auths << auth + end + end + + @read_users = @read_auths.map(&:user).uniq end def require_login diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb index b495290912..6dcadb01e2 100644 --- a/services/api/app/middlewares/arvados_api_token.rb +++ b/services/api/app/middlewares/arvados_api_token.rb @@ -18,6 +18,7 @@ class ArvadosApiToken # First, clean up just in case we have a multithreaded server and thread # local variables are still set from a prior request. Also useful for # tests that call this code to set up the environment. + Thread.current[:supplied_token] = nil Thread.current[:api_client_ip_address] = nil Thread.current[:api_client_authorization] = nil Thread.current[:api_client_uuid] = nil @@ -35,29 +36,41 @@ class ArvadosApiToken supplied_token = params["api_token"] || params["oauth_token"] || - env["HTTP_AUTHORIZATION"].andand.match(/OAuth2 ([a-zA-Z0-9]+)/).andand[1] + env["HTTP_AUTHORIZATION"].andand.match(/(OAuth2|Bearer) ([-,a-zA-Z0-9]+)/).andand[2] if supplied_token - api_client_auth = ApiClientAuthorization. - includes(:api_client, :user). - where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', supplied_token). - first - if api_client_auth.andand.user - user = api_client_auth.user - api_client = api_client_auth.api_client + case supplied_token[0..2] + when 'v2,' + _, uuid, secret = supplied_token.split(',') + auth = ApiClientAuthorization. + includes(:api_client, :user). + where('uuid=? and api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', + uuid, secret). + first else - # Token seems valid, but points to a non-existent (deleted?) user. - api_client_auth = nil + auth = ApiClientAuthorization. + includes(:api_client, :user). + where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', + supplied_token). + first + end + if auth.andand.user + user = auth.user + api_client = auth.api_client + else + # Token is invalid, or belongs to a non-existent (deleted?) user. + auth = nil end end + Thread.current[:supplied_token] = supplied_token Thread.current[:api_client_ip_address] = remote_ip - Thread.current[:api_client_authorization] = api_client_auth + Thread.current[:api_client_authorization] = auth Thread.current[:api_client_uuid] = api_client.andand.uuid Thread.current[:api_client] = api_client Thread.current[:user] = user - if api_client_auth - api_client_auth.last_used_at = Time.now - api_client_auth.last_used_by_ip_address = remote_ip.to_s - api_client_auth.save validate: false + if auth + auth.last_used_at = Time.now + auth.last_used_by_ip_address = remote_ip.to_s + auth.save validate: false end @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 10c02cca25..3e90c0eb30 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -82,6 +82,22 @@ class ApiClientAuthorization < ArvadosModel ["#{table_name}.id desc"] end + def self.validate(remote_id:) + token = Thread.current[:supplied_token] + return nil if !token + version, uuid, secret = token.split(',') + return nil if version != 'v2' + auth = ApiClientAuthorization. + includes(:user). + where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', uuid). + first + if auth && secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote_id) + return auth + else + return nil + end + end + protected def permission_to_create diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb index ddc40a4844..0fa8430718 100644 --- a/services/api/test/functional/arvados/v1/groups_controller_test.rb +++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb @@ -706,4 +706,15 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase end end + + test "list readable groups with salted token" do + salted_token = salt_token(fixture: :active, remote_id: 'zbbbb') + ArvadosApiToken.new.call("rack.input" => "", + "HTTP_AUTHORIZATION" => "Bearer #{salted_token}") + get :index, {remote_id: 'zbbbb', limit: 10000} + assert_response 200 + group_uuids = json_response['items'].collect { |i| i['uuid'] } + assert_includes(group_uuids, 'zzzzz-j7d0g-fffffffffffffff') + refute_includes(group_uuids, 'zzzzz-j7d0g-000000000000000') + end end diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb index b75479ff8d..64b6234f56 100644 --- a/services/api/test/functional/arvados/v1/users_controller_test.rb +++ b/services/api/test/functional/arvados/v1/users_controller_test.rb @@ -870,4 +870,20 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase } return return_obj end + + ['zbbbb', 'z0000'].each do |token_valid_for| + test "validate #{token_valid_for}-salted token for remote cluster zbbbb" do + salted_token = salt_token(fixture: :active, remote_id: token_valid_for) + ArvadosApiToken.new.call("rack.input" => "", + "HTTP_AUTHORIZATION" => "Bearer #{salted_token}") + get :current, {remote_id: 'zbbbb'} + if token_valid_for == 'zbbbb' + STDERR.puts json_response.inspect + assert_equal(users(:active).uuid, json_response['uuid']) + assert_response 200 + else + assert_response 401 + end + end + end end diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb index d874bc4f1f..639b53bfa7 100644 --- a/services/api/test/test_helper.rb +++ b/services/api/test/test_helper.rb @@ -127,6 +127,14 @@ class ActiveSupport::TestCase "HTTP_AUTHORIZATION" => "OAuth2 #{t}") end + def salt_token(fixture:, remote_id:) + auth = api_client_authorizations(fixture) + uuid = auth.uuid + token = auth.api_token + hmac = OpenSSL::HMAC.hexdigest('sha1', token, remote_id) + return "v2,#{uuid},#{hmac}" + end + def self.skip_slow_tests? !(ENV['RAILS_TEST_SHORT'] || '').empty? end -- 2.30.2