2873: more code review changes

* get_permissions sets @offset and @limit explicitly to ensure that
  render_list does the right thing.

* Tests updated to use permission links on Groups (permissions are not
  yet working for other objects)

* Added tests for "uuid exists but is unreadable" and "uuid is readable
  but not manageable"

Refs #2873.

diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index 43bab061faba6625a89c226eed284ebeca90e796..722afd13a977fe6b603c71bff11a20e1ff74a11c 100644 (file)
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -23,6 +23,8 @@ class Arvados::V1::LinksController < ApplicationController
       # find all links and return them
       @objects = Link.where(link_class: "permission",
                             head_uuid: params[:uuid])
+      @offset = 0
+      @limit = @objects.count
       render_list
     else
       render :json => { errors: ['Forbidden'] }.to_json, status: 403

diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index ae338b74dc2274f4b98e6c376fcec5a5b3cb6dfc..095c2dcc2e6d420fbd73b2e640779d393ddba5d4 100644 (file)
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -285,12 +285,11 @@ class PermissionsTest < ActionDispatch::IntegrationTest
 
   test "get_permissions returns list" do
     # First confirm that user :active cannot get permissions on group :public
-    get "/arvados/v1/permissions/#{groups(:public).uuid}", {
-      :format => :json,
-    }, auth(:active)
+    get "/arvados/v1/permissions/#{groups(:public).uuid}", nil, auth(:active)
     assert_response 404
 
-    # add some permissions
+    # add some permissions, including can_manage
+    # permission for user :active
     post "/arvados/v1/links", {
       :format => :json,
       :link => {
@@ -332,9 +331,9 @@ class PermissionsTest < ActionDispatch::IntegrationTest
 
     # Now user :active should be able to retrieve permissions
     # on group :public.
-    get "/arvados/v1/permissions/#{groups(:public).uuid}", {
-      :format => :json,
-    }, auth(:active)
+    get("/arvados/v1/permissions/#{groups(:public).uuid}",
+        { :format => :json },
+        auth(:active))
     assert_response :success
 
     perm_uuids = json_response['items'].map { |item| item['uuid'] }
@@ -344,17 +343,33 @@ class PermissionsTest < ActionDispatch::IntegrationTest
   end
 
   test "get_permissions returns 404 for nonexistent uuid" do
-    nonexistent = Collection.generate_uuid
+    nonexistent = Group.generate_uuid
     # make sure it really doesn't exist
-    get "/arvados/v1/collections/#{nonexistent}", { :format => :json }, auth(:admin)
+    get "/arvados/v1/groups/#{nonexistent}", nil, auth(:admin)
+    assert_response 404
+
+    get "/arvados/v1/permissions/#{nonexistent}", nil, auth(:active)
     assert_response 404
+  end
 
-    get "/arvados/v1/permissions/#{nonexistent}", { :format => :json }, auth(:active)
+  test "get_permissions returns 404 for unreadable uuid" do
+    get "/arvados/v1/permissions/#{groups(:public).uuid}", nil, auth(:active)
     assert_response 404
   end
 
-  test "get_permissions returns 403 if user lacks manage permission" do
-    get "/arvados/v1/permissions/#{collections(:foo_file).uuid}", { :format => :json }, auth(:active)
+  test "get_permissions returns 403 if user can read but not manage" do
+    post "/arvados/v1/links", {
+      :link => {
+        tail_uuid: users(:active).uuid,
+        link_class: 'permission',
+        name: 'can_read',
+        head_uuid: groups(:public).uuid,
+        properties: {}
+      }
+    }, auth(:admin)
+    assert_response :success
+
+    get "/arvados/v1/permissions/#{groups(:public).uuid}", nil, auth(:active)
     assert_response 403
   end
 end