projects / arvados.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: d3340a0)
11453: Tests for invalid / future-proof / reanimated tokens.
authorTom Clegg <tclegg@veritasgenetics.com>
Wed, 6 Dec 2017 16:39:57 +0000 (11:39 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Wed, 6 Dec 2017 16:39:57 +0000 (11:39 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

services/api/app/models/api_client_authorization.rb patch | blob | history
services/api/test/integration/remote_user_test.rb patch | blob | history

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 87505da87457ae50be57cc72288d993caa2711bf..55bd31742e35d651ee581dfdaf4ced977f8de722 100644 (file)
--- 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 9b20f88d3459a2e4ce738ef90acf90d72a27f9a8..8af0e62728a4d33006b669da324f09c712f33e15 100644 (file)
--- 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 = '<html>bad</html>'