[ARVADOS] updated: be1c59b0135072dccbbbdb845a32783109e5d2e4
Git user
git at public.curoverse.com
Sat Mar 12 17:30:36 EST 2016
Summary of changes:
sdk/cwl/arvados_cwl/__init__.py | 13 +++-
sdk/cwl/setup.py | 4 +-
.../cwl/tests/__init__.py | 0
sdk/cwl/tests/test_job.py | 77 ++++++++++++++++++++
.../v1/api_client_authorizations_controller.rb | 23 ++++--
.../arvados/v1/repositories_controller.rb | 4 +-
.../api_client_authorizations_controller_test.rb | 6 +-
.../arvados/v1/repositories_controller_test.rb | 4 +-
services/arv-git-httpd/gitolite_test.go | 14 +++-
services/datamanager/datamanager.go | 2 +-
services/datamanager/datamanager_test.go | 26 ++++++-
services/nodemanager/arvnodeman/baseactor.py | 85 ++++++++++++++++++++++
services/nodemanager/arvnodeman/clientactor.py | 2 +-
.../arvnodeman/computenode/dispatch/__init__.py | 11 ++-
services/nodemanager/arvnodeman/config.py | 4 +-
services/nodemanager/arvnodeman/daemon.py | 25 +++----
services/nodemanager/arvnodeman/fullstopactor.py | 17 -----
services/nodemanager/arvnodeman/launcher.py | 12 +--
services/nodemanager/arvnodeman/timedcallback.py | 2 +-
services/nodemanager/tests/test_daemon.py | 1 +
services/nodemanager/tests/test_failure.py | 29 +++-----
services/nodemanager/tests/testutil.py | 5 +-
22 files changed, 277 insertions(+), 89 deletions(-)
copy apps/workbench/app/views/application/_breadcrumb_page_name.html.erb => sdk/cwl/tests/__init__.py (100%)
create mode 100644 sdk/cwl/tests/test_job.py
create mode 100644 services/nodemanager/arvnodeman/baseactor.py
delete mode 100644 services/nodemanager/arvnodeman/fullstopactor.py
discards d21a4bb4c979f87177e0972d6421f2d33d9273fd (commit)
discards afe466027816e2adf997969099ef307f0591509e (commit)
discards 3ba3d4b7c9fef59f73eae59aa79195e2e51eb2e1 (commit)
via be1c59b0135072dccbbbdb845a32783109e5d2e4 (commit)
via 6248cc853b5be6f6a876d85d60b40e6a562379f1 (commit)
via 54342985a3f511988005b51133630346bfdd5788 (commit)
via 341066e5326e35c303993d365dfbbccf97b1c8b2 (commit)
via a2d5d6b24dde2de391831aa122cfda8e2cb759e0 (commit)
via a677caec8b7bc236d9558ff39913237a3e4ed8fc (commit)
via d1de3281f023bfdbb62a172dec058caf2496224f (commit)
via b4c4d4c3229b1f00463968943d6f3f24eaf7b6f9 (commit)
via 16ed3c023655eba037012eb3e046f0ed333f33b6 (commit)
via 071cbfb5b1b9328d0db5d4e4c07fd0c8d604c39e (commit)
via 68b6917bd994ac608fa70f160643a070b7ead159 (commit)
via 648aaa04080e11b0a793d6100260770b878b24c0 (commit)
via c6df16d2af30e989bcfb04f6ef730cde658a9dc9 (commit)
via d54cd5298bb6e043205995c6e5d414a841d9c389 (commit)
via 2dbbaaefc6a4a46a7f17b9e7799fc455cd722113 (commit)
via 054f95044461c08fd5fb6cd983d1e8ea1dc62ea8 (commit)
via f6aee8a6a829c60015506d89a4e87eb9dc96a07a (commit)
via e5c99ebf68f31d630f2a35f7e4e79e93143a3607 (commit)
via 60b10d07802263e1f141dc52b96e0fb9623e5417 (commit)
via 67f5a1a4d8dde5bac538e7e8d102ca83dc3335d0 (commit)
via a26464d38bbc004c2949152a6617ed0fb1b5353e (commit)
via e6d8eb6d2415f15439ab6b7ab715ca962e7e7763 (commit)
via e5ee153a56578d13a025cde47fd0c07e21fd975f (commit)
via 0e97dd2d9fe31a6c1cf73471e6e5ca1f33500850 (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 (d21a4bb4c979f87177e0972d6421f2d33d9273fd)
\
N -- N -- N (be1c59b0135072dccbbbdb845a32783109e5d2e4)
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 be1c59b0135072dccbbbdb845a32783109e5d2e4
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Mar 12 05:55:01 2016 -0500
8079: Tidy up and document current_api_client_is_trusted.
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 cb232f0..fcbccc9 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
@@ -74,30 +74,46 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
end
def find_object_by_uuid
- 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
+ _uuid = params[:uuid] || params[:id]
+ if (_uuid != current_api_client_authorization.andand.uuid and
+ not Thread.current[:api_client].andand.is_trusted)
+ return forbidden
end
- @object = model_class.where(conditions).first
+ @limit = 1
+ @offset = 0
+ @orders = []
+ @where = {}
+ @filters = [['uuid', '=', _uuid]]
+ find_objects_for_index
+ @object = @objects.first
end
def current_api_client_is_trusted
- unless Thread.current[:api_client].andand.is_trusted
- 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'], ['api_token']].include? filters
- return true if @objects.first['api_token'] == current_api_client_authorization.andand.api_token
- end
- end
- send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.',
- status: 403)
+ if Thread.current[:api_client].andand.is_trusted
+ return true
+ end
+ # A non-trusted client can do a search for its own token if it
+ # explicitly restricts the search to its own UUID or api_token.
+ # Any other kind of query must return 403, even if it matches only
+ # the current token, because that's currently how Workbench knows
+ # (after searching on scopes) the difference between "the token
+ # I'm using now *is* the only sharing token for this collection"
+ # (403) and "my token is trusted, and there is one sharing token
+ # for this collection" (200).
+ #
+ # The @filters test here also prevents a non-trusted token from
+ # filtering on its own scopes, and discovering whether any _other_
+ # equally scoped tokens exist (403=yes, 200=no).
+ if (@objects.andand.count == 1 and
+ @objects.first.uuid == current_api_client_authorization.andand.uuid and
+ (@filters.map(&:first) & %w(uuid api_token)).any?)
+ return true
end
+ forbidden
+ end
+
+ def forbidden
+ send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.',
+ status: 403)
end
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 1fb94ab..192e6b9 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
@@ -72,9 +72,9 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes
[: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],
+ [:admin, :active, 403, 403],
+ [:admin, :admin_vm, 403, 403],
+ [:active, :admin, 403, 403],
# cannot look up other tokens for other users, regardless of trustedclient
[:admin_trustedclient, :active, 404, 200, 0],
[:active_trustedclient, :admin, 404, 200, 0],
commit 6248cc853b5be6f6a876d85d60b40e6a562379f1
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 54342985a3f511988005b51133630346bfdd5788
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