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.

diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index f76af60bb93503a3908d48afae6609ff593e9414..798217dc0cfef7aaea385a67ede5fae198ce6e41 100644 (file)
--- 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 313db7f206dda60281e52c49fc7f04c4c92c3ec1..1227618257f4297a42ca3cd7ce04b6608f3f13b7 100644 (file)
--- 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 d5b42665c38b8599c5fa0e3bc088d851469f4f33..b131947bc939f603168839635cb81255a1ddaad7 100644 (file)
--- 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