[ARVADOS] created: 1.2.0-111-g15f312dfe

Git user git at public.curoverse.com
Mon Oct 1 13:58:45 EDT 2018


        at  15f312dfec5cf19c2c71508b2ed7a9d65758eeb0 (commit)


commit 15f312dfec5cf19c2c71508b2ed7a9d65758eeb0
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Oct 1 13:41:44 2018 -0400

    14201: add exclude_home_project to 'contents' endpoint
    
    Adds tests and documentation.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/doc/api/methods/groups.html.textile.liquid b/doc/api/methods/groups.html.textile.liquid
index c50366d4d..c6f78b929 100644
--- a/doc/api/methods/groups.html.textile.liquid
+++ b/doc/api/methods/groups.html.textile.liquid
@@ -53,6 +53,7 @@ table(table table-bordered table-condensed).
 |order|array|Attributes to use as sort keys to determine the order resources are returned, each optionally followed by @asc@ or @desc@ to indicate ascending or descending order. Sort within a resource type by prefixing the attribute with the resource name and a period.|query|@["collections.modified_at desc"]@|
 |filters|array|Conditions for filtering items.|query|@[["uuid", "is_a", "arvados#job"]]@|
 |recursive|boolean (default false)|Include items owned by subprojects.|query|@true@|
+|exclude_home_project|boolean (default false)|Only return items which are visible to the user but not accessible within the user's home project.  Use this to get a list of items that are shared with the user.|query|@true@|
 
 Note: Because adding access tokens to manifests can be computationally expensive, the @manifest_text@ field is not included in listed collections.  If you need it, request a "list of collections":{{site.baseurl}}/api/methods/collections.html with the filter @["owner_uuid", "=", GROUP_UUID]@, and @"manifest_text"@ listed in the select parameter.
 
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index a963d1fc4..83314fb38 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -55,7 +55,7 @@ class Arvados::V1::GroupsController < ApplicationController
 
   def contents
     load_searchable_objects
-    send_json({
+    list = {
       :kind => "arvados#objectList",
       :etag => "",
       :self_link => "",
@@ -63,7 +63,30 @@ class Arvados::V1::GroupsController < ApplicationController
       :limit => @limit,
       :items_available => @items_available,
       :items => @objects.as_api_response(nil)
-    })
+    }
+    if @extra_included
+      list[:included] = @extra_included.as_api_response(nil, {select: @select})
+    end
+    send_json(list)
+  end
+
+  def exclude_home objectlist, klass
+    # select records that are readable by current user AND
+    #   the owner_uuid is a user (but not the current user) OR
+    #   the owner_uuid is not readable by the current user
+    #   the owner_uuid is a group but group_class is not a project
+
+    read_parent_check = if current_user.is_admin
+                          ""
+                        else
+                          "NOT EXISTS(SELECT 1 FROM #{PERMISSION_VIEW} WHERE "+
+                            "user_uuid=(:user_uuid) AND target_uuid=#{klass.table_name}.owner_uuid AND perm_level >= 1) OR "
+                        end
+
+    objectlist.where("#{klass.table_name}.owner_uuid IN (SELECT users.uuid FROM users WHERE users.uuid != (:user_uuid)) OR "+
+                     read_parent_check+
+                     "EXISTS(SELECT 1 FROM groups as gp where gp.uuid=#{klass.table_name}.owner_uuid and gp.group_class != 'project')",
+                     user_uuid: current_user.uuid)
   end
 
   def shared
@@ -76,10 +99,6 @@ class Arvados::V1::GroupsController < ApplicationController
     # This also returns (in the "included" field) the objects that own
     # those projects (users or non-project groups).
     #
-    # select groups that are readable by current user AND
-    #   the owner_uuid is a user (but not the current user) OR
-    #   the owner_uuid is not readable by the current user
-    #   the owner_uuid is a group but group_class is not a project
     #
     # The intended use of this endpoint is to support clients which
     # wish to browse those projects which are visible to the user but
@@ -88,22 +107,12 @@ class Arvados::V1::GroupsController < ApplicationController
     load_limit_offset_order_params
     load_filters_param
 
-    read_parent_check = if current_user.is_admin
-                          ""
-                        else
-                          "NOT EXISTS(SELECT 1 FROM #{PERMISSION_VIEW} WHERE "+
-                            "user_uuid=(:user_uuid) AND target_uuid=groups.owner_uuid AND perm_level >= 1) OR "
-                        end
+    @objects = exclude_home Group.readable_by(*@read_users), Group
 
-    @objects = Group.readable_by(*@read_users).where("groups.owner_uuid IN (SELECT users.uuid FROM users WHERE users.uuid != (:user_uuid)) OR "+
-                                                     read_parent_check+
-                                                     "EXISTS(SELECT 1 FROM groups as gp where gp.uuid=groups.owner_uuid and gp.group_class != 'project')",
-                                            user_uuid: current_user.uuid)
     apply_where_limit_order_params
 
-    owners = @objects.map(&:owner_uuid).to_a
-
     if params["include"] == "owner_uuid"
+      owners = @objects.map(&:owner_uuid).to_a
       @extra_included = []
       [Group, User].each do |klass|
         @extra_included += klass.readable_by(*@read_users).where(uuid: owners).to_a
@@ -178,8 +187,14 @@ class Arvados::V1::GroupsController < ApplicationController
       else
         filter_by_owner[:owner_uuid] = @object.uuid
       end
+
+      if params['exclude_home_project']
+        raise ArgumentError.new "Cannot use 'exclude_home_project' with a parent object"
+      end
     end
 
+    included_by_uuid = {}
+
     seen_last_class = false
     klasses.each do |klass|
       @offset = 0 if seen_last_class  # reset offset for the new next type being processed
@@ -227,6 +242,10 @@ class Arvados::V1::GroupsController < ApplicationController
       @objects = klass.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
                  order(request_order).where(where_conds)
 
+      if params['exclude_home_project']
+        @objects = exclude_home @objects, klass
+      end
+
       klass_limit = limit_all - all_objects.count
       @limit = klass_limit
       apply_where_limit_order_params klass
@@ -242,6 +261,19 @@ class Arvados::V1::GroupsController < ApplicationController
         # with limit=0 and just accumulate items_available.
         limit_all = all_objects.count
       end
+
+      if params["include"] == "owner_uuid"
+        owners = klass_object_list[:items].map {|i| i[:owner_uuid]}
+        [Group, User].each do |ownerklass|
+          ownerklass.readable_by(*@read_users).where(uuid: owners).each do |ow|
+            included_by_uuid[ow.uuid] = ow
+          end
+        end
+      end
+    end
+
+    if params["include"]
+      @extra_included = included_by_uuid.values
     end
 
     @objects = all_objects
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 05bcec2e2..55493046e 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -776,4 +776,77 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
     assert_equal json_response['included'][0]["uuid"], groups(:group_for_sharing_tests).uuid
   end
 
+  ### contents with exclude_home_project
+
+  test 'contents, exclude home owned by another user' do
+    authorize_with :user_bar_in_sharing_group
+
+    act_as_system_user do
+      Link.create!(
+        tail_uuid: users(:user_bar_in_sharing_group).uuid,
+        link_class: 'permission',
+        name: 'can_read',
+        head_uuid: groups(:project_owned_by_foo).uuid)
+      Link.create!(
+        tail_uuid: users(:user_bar_in_sharing_group).uuid,
+        link_class: 'permission',
+        name: 'can_read',
+        head_uuid: collections(:collection_owned_by_foo).uuid)
+    end
+
+    get :contents, {:include => "owner_uuid", :exclude_home_project => true}
+
+    assert_equal 2, json_response['items'].length
+    assert_equal json_response['items'][0]["uuid"], groups(:project_owned_by_foo).uuid
+    assert_equal json_response['items'][1]["uuid"], collections(:collection_owned_by_foo).uuid
+
+    assert_equal 1, json_response['included'].length
+    assert_equal json_response['included'][0]["uuid"], users(:user_foo_in_sharing_group).uuid
+  end
+
+  test 'contents, exclude home, owned by unreadable project' do
+    authorize_with :user_bar_in_sharing_group
+
+    act_as_system_user do
+      Group.find_by_uuid(groups(:project_owned_by_foo).uuid).update!(owner_uuid: groups(:aproject).uuid)
+      Link.create!(
+        tail_uuid: users(:user_bar_in_sharing_group).uuid,
+        link_class: 'permission',
+        name: 'can_read',
+        head_uuid: groups(:project_owned_by_foo).uuid)
+    end
+
+    get :contents, {:include => "owner_uuid", :exclude_home_project => true}
+
+    assert_equal 1, json_response['items'].length
+    assert_equal json_response['items'][0]["uuid"], groups(:project_owned_by_foo).uuid
+
+    assert_equal 0, json_response['included'].length
+  end
+
+  test 'contents, exclude home, owned by non-project' do
+    authorize_with :user_bar_in_sharing_group
+
+    act_as_system_user do
+      Group.find_by_uuid(groups(:project_owned_by_foo).uuid).update!(owner_uuid: groups(:group_for_sharing_tests).uuid)
+    end
+
+    get :contents, {:include => "owner_uuid", :exclude_home_project => true}
+
+    assert_equal 1, json_response['items'].length
+    assert_equal json_response['items'][0]["uuid"], groups(:project_owned_by_foo).uuid
+
+    assert_equal 1, json_response['included'].length
+    assert_equal json_response['included'][0]["uuid"], groups(:group_for_sharing_tests).uuid
+  end
+
+
+  test 'contents, exclude home, with parent specified' do
+    authorize_with :active
+
+    get :contents, {id: groups(:aproject).uuid, :include => "owner_uuid", :exclude_home_project => true}
+
+    assert_response 422
+  end
+
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list