From 83f2087d9950975591ce5fe9c7f7c9e5db6749d9 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Fri, 18 Jul 2014 16:13:11 -0400 Subject: [PATCH] 2044: API server lets group managers see group permission links. This implementation hooks into find_object_by_uuid because that was the simplest thing that could work and unblock larger development on this story. Longer-term, we would like to solve this problem more generally. See the comments added to the links controller test for more explanation about that. --- .../arvados/v1/links_controller.rb | 14 +++- services/api/test/fixtures/links.yml | 17 +++++ .../arvados/v1/links_controller_test.rb | 67 +++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb index f76af60bb9..798217dc0c 100644 --- a/services/api/app/controllers/arvados/v1/links_controller.rb +++ b/services/api/app/controllers/arvados/v1/links_controller.rb @@ -34,16 +34,26 @@ class Arvados::V1::LinksController < ApplicationController protected - # Override find_object_by_uuid: the get_permissions method may be - # called on a uuid belonging to any class. def find_object_by_uuid 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 else super + if @object.nil? + # Normally group permission links are not readable_by users. + # Make an exception for users with permission to manage the group. + # FIXME: Solve this more generally - see the controller tests. + link = Link.find_by_uuid(params[:uuid]) + if (not link.nil?) and + (link.link_class == "permission") and + (@read_users.any? { |u| u.can?(manage: link.head_uuid) }) + @object = link + end + end end end diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml index 313db7f206..1227618257 100644 --- a/services/api/test/fixtures/links.yml +++ b/services/api/test/fixtures/links.yml @@ -318,6 +318,23 @@ test_timestamps: name: test properties: {} +admin_can_write_aproject: + # Yes, this permission is effectively redundant. + # We use it to test that other project admins can see + # all the project's sharing. + uuid: zzzzz-o0j2j-adminmgsproject + owner_uuid: zzzzz-tpzed-000000000000000 + created_at: 2014-01-24 20:42:26 -0800 + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + modified_at: 2014-01-24 20:42:26 -0800 + updated_at: 2014-01-24 20:42:26 -0800 + tail_uuid: zzzzz-tpzed-d9tiejq69daie8f + link_class: permission + name: can_write + head_uuid: zzzzz-j7d0g-v955i6s2oi1cbso + properties: {} + project_viewer_can_read_project: uuid: zzzzz-o0j2j-projviewerreadp owner_uuid: zzzzz-tpzed-000000000000000 diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb index d5b42665c3..b131947bc9 100644 --- a/services/api/test/functional/arvados/v1/links_controller_test.rb +++ b/services/api/test/functional/arvados/v1/links_controller_test.rb @@ -283,4 +283,71 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase } assert_response 422 end + + test "project owner can show a project permission" do + uuid = links(:project_viewer_can_read_project).uuid + authorize_with :active + get :show, id: uuid + assert_response :success + assert_equal(uuid, assigns(:object).andand.uuid) + end + + test "admin can show a project permission" do + uuid = links(:project_viewer_can_read_project).uuid + authorize_with :admin + get :show, id: uuid + assert_response :success + assert_equal(uuid, assigns(:object).andand.uuid) + end + + test "project viewer can't show others' project permissions" do + authorize_with :project_viewer + get :show, id: links(:admin_can_write_aproject).uuid + assert_response 404 + end + + test "requesting a nonexistent link returns 404" do + authorize_with :active + get :show, id: 'zzzzz-zzzzz-zzzzzzzzzzzzzzz' + assert_response 404 + end + + test "project owner can index project permissions" do + skip "Test tickles known bug" + # readable_by only lets users see permission links that relate to them + # directly. It does not allow users to see permission links for groups + # they manage. + # We'd like to fix this general issue, but we haven't settled on a general + # way to do it that doesn't involve making readable_by ridiculously hairy. + # This test demonstrates the desired behavior once we're ready to tackle + # it. In the meantime, clients should use /permissions to get this + # information. + authorize_with :active + get :index, filters: [['link_class', '=', 'permission'], + ['head_uuid', '=', groups(:aproject).uuid]] + assert_response :success + assert_not_nil assigns(:objects) + assert_includes(assigns(:objects).map(&:uuid), + links(:project_viewer_can_read_project).uuid) + end + + test "admin can index project permissions" do + authorize_with :admin + get :index, filters: [['link_class', '=', 'permission'], + ['head_uuid', '=', groups(:aproject).uuid]] + assert_response :success + assert_not_nil assigns(:objects) + assert_includes(assigns(:objects).map(&:uuid), + links(:project_viewer_can_read_project).uuid) + end + + test "project viewer can't index others' project permissions" do + authorize_with :project_viewer + get :index, filters: [['link_class', '=', 'permission'], + ['head_uuid', '=', groups(:aproject).uuid], + ['tail_uuid', '!=', users(:project_viewer).uuid]] + assert_response :success + assert_not_nil assigns(:objects) + assert_empty assigns(:objects) + end end -- 2.30.2