From f3dc89653597f7f6de480850231ea1f6b991c8aa Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 6 Dec 2017 11:39:57 -0500 Subject: [PATCH] 11453: Tests for invalid / future-proof / reanimated tokens. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../app/models/api_client_authorization.rb | 4 +++ .../api/test/integration/remote_user_test.rb | 36 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index 87505da874..55bd31742e 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -99,6 +99,10 @@ class ApiClientAuthorization < ArvadosModel case token[0..2] when 'v2/' _, uuid, secret = token.split('/') + unless uuid.andand.length == 27 && secret.andand.length.andand > 0 + return nil + end + auth = ApiClientAuthorization. includes(:user, :api_client). where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', uuid). diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb index 9b20f88d34..8af0e62728 100644 --- a/services/api/test/integration/remote_user_test.rb +++ b/services/api/test/integration/remote_user_test.rb @@ -86,7 +86,6 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest # revoke original token @stub_status = 401 - @stub_content = {error: 'not authorized'} # re-authorize before cache expires get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb') @@ -100,6 +99,11 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest # re-authorize after cache expires get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb') assert_response 401 + + # revive original token and re-authorize + @stub_status = 200 + get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb') + assert_response :success end test 'authenticate with remote token from misbhehaving remote cluster' do @@ -116,6 +120,36 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest assert_response 401 end + ['v2', + 'v2/', + 'v2//', + 'v2///', + "v2/'; delete from users where 1=1; commit; select '/lol", + 'v2/foo/bar', + 'v2/zzzzz-gj3su-077z32aux8dg2s1', + 'v2/zzzzz-gj3su-077z32aux8dg2s1/', + 'v2/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi', + 'v2/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi/zzzzz-gj3su-077z32aux8dg2s1', + 'v2//3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi', + 'v8/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi', + '/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi', + '"v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"', + '/', + '//', + '///', + ].each do |token| + test "authenticate with malformed remote token #{token}" do + get '/arvados/v1/users/current', {format: 'json'}, {"HTTP_AUTHORIZATION" => "Bearer #{token}"} + assert_response 401 + end + end + + test "ignore extra fields in remote token" do + token = salted_active_token(remote: 'zbbbb') + '/foo/bar/baz/*' + get '/arvados/v1/users/current', {format: 'json'}, {"HTTP_AUTHORIZATION" => "Bearer #{token}"} + assert_response :success + end + test 'remote api server is not an api server' do @stub_status = 200 @stub_content = 'bad' -- 2.30.2