[ARVADOS] created: 2.1.0-2151-g66ed4f9af
Git user
git at public.arvados.org
Fri Mar 25 06:30:57 UTC 2022
at 66ed4f9aff314ab3f65d6481a0e28a645d597430 (commit)
commit 66ed4f9aff314ab3f65d6481a0e28a645d597430
Author: Tom Clegg <tom at curii.com>
Date: Fri Mar 25 02:29:32 2022 -0400
18865: Ensure can_manage reveals permission links to descendants.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index 384ffd64b..e20b8a41e 100644
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -26,8 +26,8 @@ class Arvados::V1::LinksController < ApplicationController
def get_permissions
if current_user.andand.can?(manage: @object)
# find all links and return them
- @objects = Link.where(link_class: "permission",
- head_uuid: params[:uuid])
+ @objects = Link.unscoped.where(link_class: "permission",
+ head_uuid: params[:uuid])
@offset = 0
@limit = @objects.count
render_list
@@ -39,14 +39,29 @@ class Arvados::V1::LinksController < ApplicationController
protected
def find_object_by_uuid
+ if params[:id] && params[:id].match(/\D/)
+ params[:uuid] = params.delete :id
+ end
if action_name == 'get_permissions'
# get_permissions accepts a UUID for any kind of object.
@object = ArvadosModel::resource_class_for_uuid(params[:uuid])
.readable_by(*@read_users)
.where(uuid: params[:uuid])
.first
+ elsif !current_user
+ @object = nil
else
- super
+ # The usual permission-filtering index query is unnecessarily
+ # inefficient, and doesn't match all links that should be
+ # visible (see #18865). Instead, we look up the link by UUID,
+ # then check whether (a) its tail_uuid is the current user or
+ # (b) its head_uuid is an object the current_user can_manage.
+ @object = Link.unscoped.where(uuid: params[:uuid]).first
+ if @object &&
+ current_user.uuid != @object.tail_uuid &&
+ !current_user.can?(manage: @object.head_uuid)
+ @object = nil
+ end
end
end
@@ -86,6 +101,23 @@ class Arvados::V1::LinksController < ApplicationController
k
end
end
+
+ # If filtering by one or more head_uuid, and the current user has
+ # manage permission on those uuids, bypass the normal readable_by
+ # query (which doesn't match all can_manage-able items, see
+ # #18865) -- just rely on the head_uuid filters.
+ @filters.map do |k|
+ if k[0] == 'head_uuid'
+ if k[1] == '=' && current_user.can?(manage: k[2])
+ @objects = Link.unscoped
+ elsif k[1] == 'in'
+ k[2].select! do |head_uuid|
+ current_user.can?(manage: head_uuid)
+ end
+ @objects = Link.unscoped
+ end
+ end
+ end
end
end
commit b15403da6cbb2a7a91a4f6bece0639177e892309
Author: Tom Clegg <tom at curii.com>
Date: Fri Mar 25 02:29:29 2022 -0400
18865: Test that can_manage reveals permission links to descendants.
Test fails because #18865.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index 194c0fa36..7502f70e3 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -451,36 +451,122 @@ class PermissionsTest < ActionDispatch::IntegrationTest
# Should be able to read links directly too
get "/arvados/v1/links/#{can_read_uuid}",
- params: {},
headers: auth(:active)
assert_response :success
+ ### Create a collection inside a subproject inside the shared
+ ### project, and share the collection with a 3rd user
+ ### ("spectator").
+ post '/arvados/v1/groups',
+ params: {
+ group: {
+ owner_uuid: groups(:public).uuid,
+ name: 'permission test subproject',
+ group_class: 'project',
+ },
+ },
+ headers: auth(:admin)
+ assert_response :success
+ subproject_uuid = json_response['uuid']
+
+ post '/arvados/v1/collections',
+ params: {
+ collection: {
+ owner_uuid: subproject_uuid,
+ name: 'permission test collection in subproject',
+ },
+ },
+ headers: auth(:admin)
+ assert_response :success
+ collection_uuid = json_response['uuid']
+
+ post '/arvados/v1/links',
+ params: {
+ link: {
+ tail_uuid: users(:spectator).uuid,
+ link_class: 'permission',
+ name: 'can_read',
+ head_uuid: collection_uuid,
+ }
+ },
+ headers: auth(:admin)
+ assert_response :success
+ can_read_collection_uuid = json_response['uuid']
+
+ # The "active-can_manage-project" permission should cause the
+ # "spectator-can_read-collection" link to be visible to the
+ # "active" user.
+ get "/arvados/v1/permissions/#{collection_uuid}",
+ headers: auth(:active)
+ assert_response :success
+ perm_uuids = json_response['items'].map { |item| item['uuid'] }
+ assert_includes perm_uuids, can_read_collection_uuid, "can_read_uuid not found"
+
+ get "/arvados/v1/links/#{can_read_collection_uuid}",
+ headers: auth(:active)
+ assert_response :success
+
+ [
+ ['head_uuid', '=', collection_uuid],
+ ['head_uuid', 'in', [collection_uuid]],
+ ['head_uuid', 'in', [users(:admin).uuid, collection_uuid]],
+ ].each do |filter|
+ get "/arvados/v1/links",
+ params: {
+ filters: ([['link_class', '=', 'permission'], filter]).to_json,
+ },
+ headers: auth(:active)
+ assert_response :success
+ assert_not_empty json_response['items'], "could not find can_read link using index with filter #{filter}"
+ assert_equal can_read_collection_uuid, json_response['items'][0]['uuid']
+ end
+
### Now delete the can_manage link
delete "/arvados/v1/links/#{can_manage_uuid}",
- params: nil,
headers: auth(:active)
assert_response :success
# Should not be able read these permission links again
get "/arvados/v1/permissions/#{groups(:public).uuid}",
- params: nil,
+ headers: auth(:active)
+ assert_response 404
+
+ get "/arvados/v1/permissions/#{collection_uuid}",
headers: auth(:active)
assert_response 404
get "/arvados/v1/links",
- params: {
- :filters => [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
- },
+ params: {
+ filters: [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
+ },
headers: auth(:active)
assert_response :success
assert_equal [], json_response['items']
+ [
+ ['head_uuid', '=', collection_uuid],
+ ['head_uuid', 'in', [users(:admin).uuid, collection_uuid]],
+ ['head_uuid', 'in', []],
+ ].each do |filter|
+ get "/arvados/v1/links",
+ params: {
+ :filters => [["link_class", "=", "permission"], filter].to_json
+ },
+ headers: auth(:active)
+ assert_response :success
+ assert_equal [], json_response['items']
+ end
+
# Should not be able to read links directly either
get "/arvados/v1/links/#{can_read_uuid}",
- params: {},
+ params: {},
headers: auth(:active)
assert_response 404
+ get "/arvados/v1/links/#{can_read_collection_uuid}",
+ headers: auth(:active)
+ assert_response 404
+
### Create a collection, and share it with a direct permission
### link (as opposed to sharing its parent project)
post "/arvados/v1/collections",
commit fca6909a08d0140b99be11b872d0b519f2ae7f59
Author: Tom Clegg <tom at curii.com>
Date: Wed Mar 23 16:05:02 2022 -0400
18865: Test visibility of user->collection permission links.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index 9eae518c1..194c0fa36 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -480,6 +480,73 @@ class PermissionsTest < ActionDispatch::IntegrationTest
params: {},
headers: auth(:active)
assert_response 404
+
+ ### Create a collection, and share it with a direct permission
+ ### link (as opposed to sharing its parent project)
+ post "/arvados/v1/collections",
+ params: {
+ collection: {
+ name: 'permission test',
+ }
+ },
+ headers: auth(:admin)
+ assert_response :success
+ collection_uuid = json_response['uuid']
+ post "/arvados/v1/links",
+ params: {
+ link: {
+ tail_uuid: users(:spectator).uuid,
+ link_class: 'permission',
+ name: 'can_read',
+ head_uuid: collection_uuid,
+ properties: {}
+ }
+ },
+ headers: auth(:admin)
+ assert_response :success
+ can_read_collection_uuid = json_response['uuid']
+
+ # Should not be able read the permission link via permissions API,
+ # because permission is only can_read, not can_manage
+ get "/arvados/v1/permissions/#{collection_uuid}",
+ headers: auth(:active)
+ assert_response 404
+
+ # Should not be able to read the permission link directly, for
+ # same reason
+ get "/arvados/v1/links/#{can_read_collection_uuid}",
+ headers: auth(:active)
+ assert_response 404
+
+ ### Now add a can_manage link
+ post "/arvados/v1/links",
+ params: {
+ link: {
+ tail_uuid: users(:active).uuid,
+ link_class: 'permission',
+ name: 'can_manage',
+ head_uuid: collection_uuid,
+ properties: {}
+ }
+ },
+ headers: auth(:admin)
+ assert_response :success
+ can_manage_collection_uuid = json_response['uuid']
+
+ # Should be able read both permission links via permissions API
+ get "/arvados/v1/permissions/#{collection_uuid}",
+ headers: auth(:active)
+ assert_response :success
+ perm_uuids = json_response['items'].map { |item| item['uuid'] }
+ assert_includes perm_uuids, can_read_collection_uuid, "can_read_uuid not found"
+ assert_includes perm_uuids, can_manage_collection_uuid, "can_manage_uuid not found"
+
+ # Should be able to read both permission links directly
+ [can_read_collection_uuid, can_manage_collection_uuid].each do |uuid|
+ get "/arvados/v1/links/#{uuid}",
+ headers: auth(:active)
+ assert_response :success
+ end
end
test "get_permissions returns 404 for nonexistent uuid" do
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list