[ARVADOS] updated: 2.1.0-2153-g9ed617606
Git user
git at public.arvados.org
Fri Mar 25 18:19:52 UTC 2022
Summary of changes:
.../app/controllers/arvados/v1/links_controller.rb | 44 +++--
services/api/test/integration/permissions_test.rb | 190 ++++++++++++---------
2 files changed, 141 insertions(+), 93 deletions(-)
via 9ed6176060dfe4a7cd477c201d6a1323f9489664 (commit)
via 8cb3621c299dced53af49d51a4de2a9a10d82bb8 (commit)
from 66ed4f9aff314ab3f65d6481a0e28a645d597430 (commit)
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 9ed6176060dfe4a7cd477c201d6a1323f9489664
Author: Tom Clegg <tom at curii.com>
Date: Fri Mar 25 14:19:15 2022 -0400
18865: Test perm links to workflows and container_requests, too.
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 a5e04165e..65f5adc1d 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -454,9 +454,9 @@ class PermissionsTest < ActionDispatch::IntegrationTest
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").
+ ### Create some objects of different types (other than projects)
+ ### inside a subproject inside the shared project, and share those
+ ### individual objects with a 3rd user ("spectator").
post '/arvados/v1/groups',
params: {
group: {
@@ -469,72 +469,91 @@ class PermissionsTest < ActionDispatch::IntegrationTest
assert_response :success
subproject_uuid = json_response['uuid']
- post '/arvados/v1/collections',
- params: {
- collection: {
- owner_uuid: subproject_uuid,
- name: 'permission test collection in subproject',
+ test_types = ['collection', 'workflow', 'container_request']
+ test_type_create_attrs = {
+ 'container_request' => {
+ command: ["echo", "foo"],
+ container_image: links(:docker_image_collection_tag).name,
+ cwd: "/tmp",
+ environment: {},
+ mounts: {"/out" => {kind: "tmp", capacity: 1000000}},
+ output_path: "/out",
+ runtime_constraints: {"vcpus" => 1, "ram" => 2},
+ },
+ }
+
+ test_object = {}
+ test_object_perm_link = {}
+ test_types.each do |test_type|
+ post "/arvados/v1/#{test_type}s",
+ params: {
+ test_type.to_sym => {
+ owner_uuid: subproject_uuid,
+ name: "permission test #{test_type} in subproject",
+ }.merge(test_type_create_attrs[test_type] || {}).to_json,
},
- },
- 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']
+ headers: auth(:admin)
+ assert_response :success
+ test_object[test_type] = json_response
+
+ post '/arvados/v1/links',
+ params: {
+ link: {
+ tail_uuid: users(:spectator).uuid,
+ link_class: 'permission',
+ name: 'can_read',
+ head_uuid: test_object[test_type]['uuid'],
+ }
+ },
+ headers: auth(:admin)
+ assert_response :success
+ test_object_perm_link[test_type] = json_response
+ end
# 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,
- },
+ # "spectator-can_read-object" links to be visible to the "active"
+ # user.
+ test_types.each do |test_type|
+ get "/arvados/v1/permissions/#{test_object[test_type]['uuid']}",
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
+ perm_uuids = json_response['items'].map { |item| item['uuid'] }
+ assert_includes perm_uuids, test_object_perm_link[test_type]['uuid'], "can_read_uuid not found"
- # The "spectator-can_read-collection" link should be visible to
- # the subject user ("spectator") in a filter query, even without
- # can_manage permission on the target object.
- [
- ['tail_uuid', '=', users(:spectator).uuid],
- ].each do |filter|
- get "/arvados/v1/links",
- params: {
- filters: ([['link_class', '=', 'permission'], filter]).to_json,
- },
- headers: auth(:spectator)
+ get "/arvados/v1/links/#{test_object_perm_link[test_type]['uuid']}",
+ headers: auth(:active)
assert_response :success
- perm_uuids = json_response['items'].map { |item| item['uuid'] }
- assert_includes perm_uuids, can_read_collection_uuid, "could not find can_read link using index with filter #{filter}"
+
+ [
+ ['head_uuid', '=', test_object[test_type]['uuid']],
+ ['head_uuid', 'in', [test_object[test_type]['uuid']]],
+ ['head_uuid', 'in', [users(:admin).uuid, test_object[test_type]['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 test_object_perm_link[test_type]['uuid'], json_response['items'][0]['uuid']
+ end
+
+ # The "spectator-can_read-object" link should be visible to the
+ # subject user ("spectator") in a filter query, even without
+ # can_manage permission on the target object.
+ [
+ ['tail_uuid', '=', users(:spectator).uuid],
+ ].each do |filter|
+ get "/arvados/v1/links",
+ params: {
+ filters: ([['link_class', '=', 'permission'], filter]).to_json,
+ },
+ headers: auth(:spectator)
+ assert_response :success
+ perm_uuids = json_response['items'].map { |item| item['uuid'] }
+ assert_includes perm_uuids, test_object_perm_link[test_type]['uuid'], "could not find can_read link using index with filter #{filter}"
+ end
end
### Now delete the can_manage link
@@ -543,45 +562,48 @@ class PermissionsTest < ActionDispatch::IntegrationTest
assert_response :success
# Should not be able read these permission links again
- get "/arvados/v1/permissions/#{groups(:public).uuid}",
- headers: auth(:active)
- assert_response 404
-
- get "/arvados/v1/permissions/#{collection_uuid}",
- headers: auth(:active)
- assert_response 404
+ test_types.each do |test_type|
+ get "/arvados/v1/permissions/#{groups(:public).uuid}",
+ headers: auth(:active)
+ assert_response 404
- get "/arvados/v1/links",
- params: {
- filters: [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].to_json
- },
- headers: auth(:active)
- assert_response :success
- assert_equal [], json_response['items']
+ get "/arvados/v1/permissions/#{test_object[test_type]['uuid']}",
+ headers: auth(:active)
+ assert_response 404
- [
- ['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
+ filters: [["link_class", "=", "permission"], ["head_uuid", "=", groups(:public).uuid]].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: {},
- headers: auth(:active)
- assert_response 404
+ [
+ ['head_uuid', '=', test_object[test_type]['uuid']],
+ ['head_uuid', 'in', [users(:admin).uuid, test_object[test_type]['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
- get "/arvados/v1/links/#{can_read_collection_uuid}",
- headers: auth(:active)
- assert_response 404
+ # Should not be able to read links directly either
+ get "/arvados/v1/links/#{can_read_uuid}",
+ headers: auth(:active)
+ assert_response 404
+
+ test_types.each do |test_type|
+ get "/arvados/v1/links/#{test_object_perm_link[test_type]['uuid']}",
+ headers: auth(:active)
+ assert_response 404
+ end
+ end
### Create a collection, and share it with a direct permission
### link (as opposed to sharing its parent project)
commit 8cb3621c299dced53af49d51a4de2a9a10d82bb8
Author: Tom Clegg <tom at curii.com>
Date: Fri Mar 25 12:03:23 2022 -0400
18865: Support "permission links with me as tail_uuid" query.
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 e20b8a41e..14e010640 100644
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -49,15 +49,18 @@ class Arvados::V1::LinksController < ApplicationController
.where(uuid: params[:uuid])
.first
elsif !current_user
- @object = nil
+ super
else
# 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.
+ # inefficient, and doesn't match all permission 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 &&
+ if @object.link_class != 'permission'
+ super
+ elsif @object &&
current_user.uuid != @object.tail_uuid &&
!current_user.can?(manage: @object.head_uuid)
@object = nil
@@ -102,19 +105,26 @@ class Arvados::V1::LinksController < ApplicationController
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])
+ # If the provided filters are enough to limit the results to
+ # permission links with specific head_uuids or
+ # tail_uuid=current_user, bypass the normal readable_by query
+ # (which doesn't match all can_manage-able items, see #18865) --
+ # just ensure the current user actually has can_manage permission
+ # for the provided head_uuids, removing any that don't. At that
+ # point the caller's filters are an effective permission filter.
+ if @filters.include?(['link_class', '=', 'permission'])
+ @filters.map do |k|
+ if k[0] == 'tail_uuid' && k[1] == '=' && k[2] == current_user.uuid
@objects = Link.unscoped
- elsif k[1] == 'in'
- k[2].select! do |head_uuid|
- current_user.can?(manage: head_uuid)
+ elsif 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
- @objects = Link.unscoped
end
end
end
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index 7502f70e3..a5e04165e 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -521,6 +521,22 @@ class PermissionsTest < ActionDispatch::IntegrationTest
assert_equal can_read_collection_uuid, json_response['items'][0]['uuid']
end
+ # The "spectator-can_read-collection" link should be visible to
+ # the subject user ("spectator") in a filter query, even without
+ # can_manage permission on the target object.
+ [
+ ['tail_uuid', '=', users(:spectator).uuid],
+ ].each do |filter|
+ get "/arvados/v1/links",
+ params: {
+ filters: ([['link_class', '=', 'permission'], filter]).to_json,
+ },
+ headers: auth(:spectator)
+ assert_response :success
+ perm_uuids = json_response['items'].map { |item| item['uuid'] }
+ assert_includes perm_uuids, can_read_collection_uuid, "could not find can_read link using index with filter #{filter}"
+ end
+
### Now delete the can_manage link
delete "/arvados/v1/links/#{can_manage_uuid}",
headers: auth(:active)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list