[ARVADOS] updated: afe466027816e2adf997969099ef307f0591509e
Git user
git at public.curoverse.com
Mon Mar 7 02:43:01 EST 2016
Summary of changes:
.../arvados/v1/api_client_authorizations_controller.rb | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
discards 5e6671aba2106fb616192606fa6b4be11af2d999 (commit)
discards 227589de13afa707bf19aea4db4a99fdc2b24d95 (commit)
via afe466027816e2adf997969099ef307f0591509e (commit)
via 3ba3d4b7c9fef59f73eae59aa79195e2e51eb2e1 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (5e6671aba2106fb616192606fa6b4be11af2d999)
\
N -- N -- N (afe466027816e2adf997969099ef307f0591509e)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit afe466027816e2adf997969099ef307f0591509e
Author: Tom Clegg <tom at curoverse.com>
Date: Sun Mar 6 14:50:22 2016 -0500
8079: Prevent users from changing their own token UUIDs.
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index c587e58..499a61b 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -82,8 +82,9 @@ class ApiClientAuthorization < ArvadosModel
def permission_to_update
(permission_to_create and
- not self.user_id_changed? and
- not self.owner_uuid_changed?)
+ not uuid_changed? and
+ not user_id_changed? and
+ not owner_uuid_changed?)
end
def log_update
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 e45bdc4..1fb94ab 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
@@ -126,4 +126,22 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
end
end
end
+
+ test "scoped token cannot change its own scopes" do
+ authorize_with :admin_vm
+ put :update, {
+ id: api_client_authorizations(:admin_vm).uuid,
+ api_client_authorization: {scopes: ['all']},
+ }
+ assert_response 403
+ end
+
+ test "token cannot change its own uuid" do
+ authorize_with :admin
+ put :update, {
+ id: api_client_authorizations(:admin).uuid,
+ api_client_authorization: {uuid: 'zzzzz-gj3su-zzzzzzzzzzzzzzz'},
+ }
+ assert_response 403
+ end
end
commit 3ba3d4b7c9fef59f73eae59aa79195e2e51eb2e1
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Mar 4 17:30:53 2016 -0500
8079: Prevent users from looking up other users' tokens by UUID.
Previous code was allowing any user logging in through a "trusted
client" (typically Workbench) to retrieve the secret token for any
ApiClientAuthorization whose UUID is known. This won't be acceptable
when Container records start including those UUIDs.
Also added permission for any user to update (e.g., change expiration)
and delete their current token, even if the token wasn't assigned
through a "trusted client".
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 2eb79c0..cb232f0 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,14 +49,14 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
def find_objects_for_index
# Here we are deliberately less helpful about searching for client
# authorizations. We look up tokens belonging to the current user
- # and filter by exact matches on api_token and scopes.
+ # and filter by exact matches on uuid, api_token, and scopes.
wanted_scopes = []
if @filters
wanted_scopes.concat(@filters.map { |attr, operator, operand|
((attr == 'scopes') and (operator == '=')) ? operand : nil
})
@filters.select! { |attr, operator, operand|
- ((attr == 'uuid') and (operator == '=')) || ((attr == 'api_token') and (operator == '='))
+ operator == '=' && (attr == 'uuid' || attr == 'api_token')
}
end
if @where
@@ -74,21 +74,26 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
end
def find_object_by_uuid
- @object = model_class.where(uuid: (params[:uuid] || params[:id])).first
+ conditions = {
+ uuid: (params[:uuid] || params[:id]),
+ user_id: current_user.id,
+ }
+ unless Thread.current[:api_client].andand.is_trusted
+ conditions[:api_token] = current_api_client_authorization.andand.api_token
+ end
+ @object = model_class.where(conditions).first
end
def current_api_client_is_trusted
unless Thread.current[:api_client].andand.is_trusted
- if params["action"] == "show"
- if @object and @object['api_token'] == current_api_client_authorization.andand.api_token
+ if %w[show update destroy].include? params['action']
+ if @object.andand['api_token'] == current_api_client_authorization.andand.api_token
return true
end
elsif params["action"] == "index" and @objects.andand.size == 1
filters = @filters.map{|f|f.first}.uniq
- if ['uuid'] == filters
+ if [['uuid'], ['api_token']].include? filters
return true if @objects.first['api_token'] == current_api_client_authorization.andand.api_token
- elsif ['api_token'] == filters
- return true if @objects.first[:user_id] = current_user.id
end
end
send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.',
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 5da9145..e45bdc4 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
@@ -68,46 +68,62 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
end
end
- [
- [:admin, :admin, 200],
- [:admin, :active, 403],
- [:admin, :admin_vm, 403], # this belongs to the user of current session, but we can't get it by uuid
- [:admin_trustedclient, :active, 200],
- ].each do |user, token, status|
- test "as user #{user} get #{token} token and expect #{status}" do
+ [# anyone can look up the token they're currently using
+ [:admin, :admin, 200, 200, 1],
+ [:active, :active, 200, 200, 1],
+ # cannot look up other tokens (even for same user) if not trustedclient
+ [:admin, :active, 404, 403],
+ [:admin, :admin_vm, 404, 403],
+ [:active, :admin, 404, 403],
+ # cannot look up other tokens for other users, regardless of trustedclient
+ [:admin_trustedclient, :active, 404, 200, 0],
+ [:active_trustedclient, :admin, 404, 200, 0],
+ ].each do |user, token, expect_get_response, expect_list_response, expect_list_items|
+ test "using '#{user}', get '#{token}' by uuid" do
authorize_with user
- get :show, {id: api_client_authorizations(token).uuid}
- assert_response status
+ get :show, {
+ id: api_client_authorizations(token).uuid,
+ }
+ assert_response expect_get_response
+ end
+
+ test "using '#{user}', update '#{token}' by uuid" do
+ authorize_with user
+ put :update, {
+ id: api_client_authorizations(token).uuid,
+ api_client_authorization: {},
+ }
+ assert_response expect_get_response
end
- end
- [
- [:admin, :admin, 200],
- [:admin, :active, 403],
- [:admin, :admin_vm, 403], # this belongs to the user of current session, but we can't list it by uuid
- [:admin_trustedclient, :active, 200],
- ].each do |user, token, status|
- test "as user #{user} list #{token} token using uuid and expect #{status}" do
+ test "using '#{user}', delete '#{token}' by uuid" do
+ authorize_with user
+ post :destroy, {
+ id: api_client_authorizations(token).uuid,
+ }
+ assert_response expect_get_response
+ end
+
+ test "using '#{user}', list '#{token}' by uuid" do
authorize_with user
get :index, {
- filters: [['uuid','=',api_client_authorizations(token).uuid]]
+ filters: [['uuid','=',api_client_authorizations(token).uuid]],
}
- assert_response status
+ assert_response expect_list_response
+ if expect_list_items
+ assert_equal assigns(:objects).length, expect_list_items
+ end
end
- end
- [
- [:admin, :admin, 200],
- [:admin, :active, 403],
- [:admin, :admin_vm, 200], # this belongs to the user of current session, and can be listed by token
- [:admin_trustedclient, :active, 200],
- ].each do |user, token, status|
- test "as user #{user} list #{token} token using token and expect #{status}" do
+ test "using '#{user}', list '#{token}' by token" do
authorize_with user
get :index, {
- filters: [['api_token','=',api_client_authorizations(token).api_token]]
+ filters: [['api_token','=',api_client_authorizations(token).api_token]],
}
- assert_response status
+ assert_response expect_list_response
+ if expect_list_items
+ assert_equal assigns(:objects).length, expect_list_items
+ end
end
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list