[ARVADOS] created: 1.1.0-41-g0e65950
Git user
git at public.curoverse.com
Wed Oct 18 16:59:57 EDT 2017
at 0e659500b47d9bd4dc3e4c96167b3e55ac14082f (commit)
commit 0e659500b47d9bd4dc3e4c96167b3e55ac14082f
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Wed Oct 18 16:59:22 2017 -0400
11453: Refactor token checks. Use base64-looking "/" delimiter.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index b3718e1..57a187a 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -349,7 +349,9 @@ class ApplicationController < ActionController::Base
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])
+ auth = ApiClientAuthorization.
+ validate(token: Thread.current[:supplied_token],
+ remote: params[:remote_id])
if auth && auth.user
Thread.current[:user] = auth.user
@read_auths << auth
diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb
index 6dcadb0..dace944 100644
--- a/services/api/app/middlewares/arvados_api_token.rb
+++ b/services/api/app/middlewares/arvados_api_token.rb
@@ -15,64 +15,31 @@ class ArvadosApiToken
end
def call env
- # 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
- Thread.current[:api_client] = nil
- Thread.current[:user] = nil
-
request = Rack::Request.new(env)
params = request.params
remote_ip = env["action_dispatch.remote_ip"]
Thread.current[:request_starttime] = Time.now
- user = nil
- api_client = nil
- api_client_auth = nil
- supplied_token =
+ Thread.current[:supplied_token] =
params["api_token"] ||
params["oauth_token"] ||
- env["HTTP_AUTHORIZATION"].andand.match(/(OAuth2|Bearer) ([-,a-zA-Z0-9]+)/).andand[2]
- if supplied_token
- 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
- 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] = auth
- Thread.current[:api_client_uuid] = api_client.andand.uuid
- Thread.current[:api_client] = api_client
- Thread.current[:user] = user
+ env["HTTP_AUTHORIZATION"].andand.
+ match(/(OAuth2|Bearer) ([-\/a-zA-Z0-9]+)/).andand[2]
+
+ auth = ApiClientAuthorization.
+ validate(token: Thread.current[:supplied_token], remote: false)
if auth
auth.last_used_at = Time.now
auth.last_used_by_ip_address = remote_ip.to_s
auth.save validate: false
end
+ Thread.current[:api_client_ip_address] = remote_ip
+ 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[:user] = auth.andand.user
+
@app.call env if @app
end
end
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 3e90c0e..eb9a8dc 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -82,22 +82,34 @@ class ApiClientAuthorization < ArvadosModel
["#{table_name}.id desc"]
end
- def self.validate(remote_id:)
- token = Thread.current[:supplied_token]
+ def self.validate(token:, remote:)
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
+ remote ||= Rails.configuration.uuid_prefix
+
+ case token[0..2]
+ when 'v2/'
+ _, uuid, secret = token.split('/')
+ auth = ApiClientAuthorization.
+ includes(:user, :api_client).
+ where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', uuid).
+ first
+ if auth && auth.user &&
+ (secret == auth.api_token ||
+ secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote))
+ return auth
+ end
else
- return nil
+ auth = ApiClientAuthorization.
+ includes(:user, :api_client).
+ where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token).
+ first
+ if auth && auth.user
+ return auth
+ end
end
+ return nil
end
-
+
protected
def permission_to_create
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 64b6234..646b96c 100644
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -878,7 +878,6 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
"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
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 639b53b..e1f7542 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -132,7 +132,7 @@ class ActiveSupport::TestCase
uuid = auth.uuid
token = auth.api_token
hmac = OpenSSL::HMAC.hexdigest('sha1', token, remote_id)
- return "v2,#{uuid},#{hmac}"
+ return "v2/#{uuid}/#{hmac}"
end
def self.skip_slow_tests?
commit 01f52958d899c62b0b1f9a39fda960136835850c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Mon Oct 16 23:24:45 2017 -0400
11453: Accept salted tokens at /users/current and /groups.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index d09283d..b3718e1 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 b495290..6dcadb0 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 10c02cc..3e90c0e 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 ddc40a4..0fa8430 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 b75479f..64b6234 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 d874bc4..639b53b 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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list