[ARVADOS] created: 2.1.0-743-g90c505485
Git user
git at public.arvados.org
Mon May 3 14:29:57 UTC 2021
at 90c505485d9185923b45a5b9562abeb9329982c3 (commit)
commit 90c505485d9185923b45a5b9562abeb9329982c3
Author: Tom Clegg <tom at curii.com>
Date: Mon May 3 10:29:40 2021 -0400
17610: Check scopes when using a remote token.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
index 99446688d..22bcb6c1d 100644
--- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
+++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
@@ -49,7 +49,12 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
end
def current
- @object = Thread.current[:api_client_authorization]
+ @object = Thread.current[:api_client_authorization].dup
+ if params[:remote]
+ # Client is validating a salted token. Don't return the unsalted
+ # secret!
+ @object.api_token = nil
+ end
show
end
diff --git a/services/api/app/middlewares/arvados_api_token.rb b/services/api/app/middlewares/arvados_api_token.rb
index be4e8bb0b..2c240984c 100644
--- a/services/api/app/middlewares/arvados_api_token.rb
+++ b/services/api/app/middlewares/arvados_api_token.rb
@@ -25,6 +25,7 @@ class ArvadosApiToken
reader_tokens = nil
if params["remote"] && request.get? && (
request.path.start_with?('/arvados/v1/groups') ||
+ request.path.start_with?('/arvados/v1/api_client_authorizations/current') ||
request.path.start_with?('/arvados/v1/users/current'))
# Request from a remote API server, asking to validate a salted
# token.
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index ee63c4d93..96e04d8ae 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -249,21 +249,28 @@ class ApiClientAuthorization < ArvadosModel
remote_user_prefix = remote_user['uuid'][0..4]
- if token_uuid == ''
- # Use the same UUID as the remote when caching the token.
- begin
- remote_token = SafeJSON.load(
- clnt.get_content('https://' + host + '/arvados/v1/api_client_authorizations/current',
- {'remote' => Rails.configuration.ClusterID},
- {'Authorization' => 'Bearer ' + token}))
- token_uuid = remote_token['uuid']
- if !token_uuid.match(HasUuid::UUID_REGEX) || token_uuid[0..4] != upstream_cluster_id
- raise "remote cluster #{upstream_cluster_id} returned invalid token uuid #{token_uuid.inspect}"
- end
- rescue => e
- Rails.logger.warn "error getting remote token details for #{token.inspect}: #{e}"
- return nil
+ # Get token scope, and make sure we use the same UUID as the
+ # remote when caching the token.
+ remote_token = nil
+ begin
+ remote_token = SafeJSON.load(
+ clnt.get_content('https://' + host + '/arvados/v1/api_client_authorizations/current',
+ {'remote' => Rails.configuration.ClusterID},
+ {'Authorization' => 'Bearer ' + token}))
+ Rails.logger.debug "retrieved remote token #{remote_token.inspect}"
+ if !token_uuid.match(HasUuid::UUID_REGEX) || token_uuid[0..4] != upstream_cluster_id
+ raise "remote cluster #{upstream_cluster_id} returned invalid token uuid #{token_uuid.inspect}"
end
+ token_uuid = remote_token['uuid']
+ rescue HTTPClient::BadResponseError => e
+ if e.res.status == 401
+ Rails.logger.warn "remote cluster #{upstream_cluster_id} at #{host} refused to provide token expiry and scopes - assuming it is an old version, using scopes=['all']"
+ else
+ raise
+ end
+ rescue => e
+ Rails.logger.warn "error getting remote token details for #{token.inspect}: #{e}"
+ return nil
end
# Clusters can only authenticate for their own users.
@@ -339,26 +346,31 @@ class ApiClientAuthorization < ArvadosModel
end
end
- # We will accept this token (and avoid reloading the user
- # record) for 'RemoteTokenRefresh' (default 5 minutes).
- # Possible todo:
- # Request the actual api_client_auth record from the remote
- # server in case it wants the token to expire sooner.
- auth = ApiClientAuthorization.find_or_create_by(uuid: token_uuid) do |auth|
- auth.user = user
- auth.api_client_id = 0
- end
# If stored_secret is set, we save stored_secret in the database
# but return the real secret to the caller. This way, if we end
# up returning the auth record to the client, they see the same
# secret they supplied, instead of the HMAC we saved in the
# database.
stored_secret = stored_secret || secret
+
+ # We will accept this token (and avoid reloading the user
+ # record) for 'RemoteTokenRefresh' (default 5 minutes).
+ exp = [db_current_time + Rails.configuration.Login.RemoteTokenRefresh,
+ remote_token.andand['expires_at']].compact.min
+ scopes = remote_token.andand['scopes'] || ['all']
+ auth = ApiClientAuthorization.find_or_create_by(uuid: token_uuid) do |auth|
+ auth.user = user
+ auth.api_token = stored_secret
+ auth.api_client_id = 0
+ auth.scopes = scopes
+ auth.expires_at = exp
+ end
auth.update_attributes!(user: user,
api_token: stored_secret,
api_client_id: 0,
- expires_at: db_current_time + Rails.configuration.Login.RemoteTokenRefresh)
- Rails.logger.debug "cached remote token #{token_uuid} with secret #{stored_secret} in local db"
+ scopes: scopes,
+ expires_at: exp)
+ Rails.logger.debug "cached remote token #{token_uuid} with secret #{stored_secret} and scopes #{scopes} in local db"
auth.api_token = secret
return auth
end
diff --git a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
index c2c71ca54..bf407afcd 100644
--- a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb
@@ -81,6 +81,15 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
end
end
+ [:admin, :active].each do |token|
+ test "using '#{token}', get token details via 'current'" do
+ authorize_with token
+ get :current
+ assert_response 200
+ assert_equal json_response['scopes'], ['all']
+ end
+ end
+
[# anyone can look up the token they're currently using
[:admin, :admin, 200, 200, 1],
[:active, :active, 200, 200, 1],
diff --git a/services/api/test/integration/api_client_authorizations_api_test.rb b/services/api/test/integration/api_client_authorizations_api_test.rb
index ce79fc557..14e3bb361 100644
--- a/services/api/test/integration/api_client_authorizations_api_test.rb
+++ b/services/api/test/integration/api_client_authorizations_api_test.rb
@@ -167,4 +167,16 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
end
end
end
+
+ test "get current token using salted token" do
+ salted = salt_token(fixture: :active, remote: 'abcde')
+ get('/arvados/v1/api_client_authorizations/current',
+ params: {remote: 'abcde'},
+ headers: {'HTTP_AUTHORIZATION' => "Bearer #{salted}"})
+ assert_response :success
+ assert_equal(json_response['uuid'], api_client_authorizations(:active).uuid)
+ assert_equal(json_response['scopes'], ['all'])
+ assert_not_nil(json_response['expires_at'])
+ assert_nil(json_response['api_token'])
+ end
end
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 432388452..568d5088d 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -67,6 +67,20 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
res.status = @stub_status
res.body = @stub_content.is_a?(String) ? @stub_content : @stub_content.to_json
end
+ srv.mount_proc '/arvados/v1/api_client_authorizations/current' do |req, res|
+ if clusterid == 'zbbbb' and req.header['authorization'][0][10..14] == 'zbork'
+ # asking zbbbb about zbork should yield an error, zbbbb doesn't trust zbork
+ res.status = 401
+ return
+ end
+ res.status = @stub_token_status
+ if res.status == 200
+ res.body = {
+ uuid: api_client_authorizations(:active).uuid.sub('zzzzz', clusterid),
+ scopes: @stub_token_scopes,
+ }.to_json
+ end
+ end
Thread.new do
srv.start
end
@@ -86,6 +100,8 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
is_active: true,
is_invited: true,
}
+ @stub_token_status = 200
+ @stub_token_scopes = ["all"]
end
teardown do
@@ -94,6 +110,31 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
end
end
+ test 'authenticate with remote token that has limited scope' do
+ get '/arvados/v1/collections',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+
+ @stub_token_scopes = ["GET /arvados/v1/users/current"]
+
+ # re-authorize before cache expires
+ get '/arvados/v1/collections',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+
+ # simulate cache expiry
+ ApiClientAuthorization.where('uuid like ?', 'zbbbb-%').
+ update_all(expires_at: db_current_time - 1.minute)
+
+ # re-authorize after cache expires
+ get '/arvados/v1/collections',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response 403
+ end
+
test 'authenticate with remote token' do
get '/arvados/v1/users/current',
params: {format: 'json'},
@@ -115,8 +156,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
assert_response :success
# simulate cache expiry
- ApiClientAuthorization.where(
- uuid: salted_active_token(remote: 'zbbbb').split('/')[1]).
+ ApiClientAuthorization.where('uuid like ?', 'zbbbb-%').
update_all(expires_at: db_current_time - 1.minute)
# re-authorize after cache expires
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list