Merge branch '13146-shared-rails' refs #13146
authorPeter Amstutz <pamstutz@veritasgenetics.com>
Fri, 17 Aug 2018 17:13:47 +0000 (13:13 -0400)
committerPeter Amstutz <pamstutz@veritasgenetics.com>
Fri, 17 Aug 2018 17:13:47 +0000 (13:13 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

doc/api/methods/groups.html.textile.liquid
services/api/app/controllers/application_controller.rb
services/api/app/controllers/arvados/v1/groups_controller.rb
services/api/config/routes.rb
services/api/test/fixtures/api_client_authorizations.yml
services/api/test/fixtures/groups.yml
services/api/test/functional/arvados/v1/groups_controller_test.rb
services/fuse/arvados_fuse/fusedir.py

index e87bc51ad4a590b4102fd4f1047c9b878de466a2..c50366d4de6c4a14b5e1203283301a2938d54623 100644 (file)
@@ -125,3 +125,24 @@ table(table table-bordered table-condensed).
 |_. Argument |_. Type |_. Description |_. Location |_. Example |
 {background:#ccffcc}.|uuid|string|The UUID of the Group to untrash.|path||
 |ensure_unique_name|boolean (default false)|Rename project uniquely if untrashing it would fail with a unique name conflict.|query||
+
+h3. shared
+
+This endpoint returns the toplevel set of groups to which access is granted through a chain of one or more permission links rather than through direct ownership by the current user account.  This is useful for clients which wish to browse the list of projects the user has permission to read which are not part of the "home" project tree.
+
+When called with "include=owner_uuid" this also returns (in the "included" field) the objects that own those projects (users or non-project groups).
+
+Specifically, the logic is:
+
+<pre>
+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 OR
+     the owner_uuid is a group but group_class is not a project)
+</pre>
+
+In addition to the "include" parameter this endpoint also supports the same parameters as the "list method.":{{site.baseurl}}/api/methods.html#index
+
+table(table table-bordered table-condensed).
+|_. Argument |_. Type |_. Description |_. Location |_. Example |
+|include|string|If provided with the value "owner_uuid", this will return owner objects in the "included" field of the response.|query|?include=owner_uuid|
index 71176056fa3f24c5340796d2ef2fe20069acd666..1cb49f46749054859980471772aa8f4b3f7f51e1 100644 (file)
@@ -78,6 +78,7 @@ class ApplicationController < ActionController::Base
     @distinct = nil
     @response_resource_name = nil
     @attrs = nil
+    @extra_included = nil
   end
 
   def default_url_options
@@ -494,6 +495,9 @@ class ApplicationController < ActionController::Base
       :limit => @limit,
       :items => @objects.as_api_response(nil, {select: @select})
     }
+    if @extra_included
+      list[:included] = @extra_included.as_api_response(nil, {select: @select})
+    end
     case params[:count]
     when nil, '', 'exact'
       if @objects.respond_to? :except
index 33140be8efe2eb05542e5fd11443450772ce3556..a963d1fc4d875d2d69129ebb6ec6606e7f65d666 100644 (file)
@@ -7,6 +7,9 @@ require "trashable"
 class Arvados::V1::GroupsController < ApplicationController
   include TrashableController
 
+  skip_before_filter :find_object_by_uuid, only: :shared
+  skip_before_filter :render_404_if_no_object, only: :shared
+
   def self._index_requires_parameters
     (super rescue {}).
       merge({
@@ -63,6 +66,59 @@ class Arvados::V1::GroupsController < ApplicationController
     })
   end
 
+  def shared
+    # The purpose of this endpoint is to return the toplevel set of
+    # groups which are *not* reachable through a direct ownership
+    # chain of projects starting from the current user account.  In
+    # other words, groups which to which access was granted via a
+    # permission link or chain of links.
+    #
+    # 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
+    # are not part of the "home" project.
+
+    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 = 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"
+      @extra_included = []
+      [Group, User].each do |klass|
+        @extra_included += klass.readable_by(*@read_users).where(uuid: owners).to_a
+      end
+    end
+
+    index
+  end
+
+  def self._shared_requires_parameters
+    rp = self._index_requires_parameters
+    rp[:include] = { type: 'string', required: false }
+    rp
+  end
+
   protected
 
   def load_searchable_objects
index b0c09840d790db1d634139bd796691d82f2b7c8c..b54c3c5bf170cc431140a0925b9846e7f172b397 100644 (file)
@@ -30,6 +30,7 @@ Server::Application.routes.draw do
       resources :groups do
         get 'contents', on: :collection
         get 'contents', on: :member
+        get 'shared', on: :collection
         post 'trash', on: :member
         post 'untrash', on: :member
       end
index 92bd7cf872cfeca1c53d38c5ea05d7836e929f4f..2073d8b1bacccfaa0422643a34ddfe5ed0144461 100644 (file)
@@ -275,6 +275,13 @@ user_foo_in_sharing_group:
   api_token: 2p1pou8p4ls208mcbedeewlotghppenobcyrmyhq8pyf51xd8u
   expires_at: 2038-01-01 00:00:00
 
+user_bar_in_sharing_group:
+  uuid: zzzzz-gj3su-62hryf5fht531mz
+  api_client: untrusted
+  user: user_bar_in_sharing_group
+  api_token: 5vy55akwq85vghh80wc2cuxl4p8psay73lkpqf5c2cxvp6rmm6
+  expires_at: 2038-01-01 00:00:00
+
 user1_with_load:
   uuid: zzzzz-gj3su-357z32aux8dg2s1
   api_client: untrusted
index 68cc76949afc5b21e0f7586fb777944788a9f6cd..92a1ced52841942b60f3898a58b5818d53b3b14f 100644 (file)
@@ -152,6 +152,14 @@ group_for_sharing_tests:
   description: Users who can share objects with each other
   group_class: role
 
+project_owned_by_foo:
+  uuid:  zzzzz-j7d0g-lsjm0ibr0ydwpzx
+  owner_uuid: zzzzz-tpzed-81hsbo6mk8nl05c
+  created_at: 2014-02-03T17:22:54Z
+  modified_at: 2014-02-03T17:22:54Z
+  name: project_owned_by_foo
+  group_class: project
+
 empty_project:
   uuid: zzzzz-j7d0g-9otoxmrksam74q6
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
index d2b2ad7de824e1d0805fafdda493a7a9eab99fe4..4506f76c6dd56655fb15c03677f0fddc26efb18e 100644 (file)
@@ -719,4 +719,61 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
       assert_not_nil Group.readable_by(users(auth)).where(uuid: groups(:trashed_subproject).uuid).first
     end
   end
+
+  test 'get shared 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)
+    end
+
+    get :shared, {:filters => [["group_class", "=", "project"]], :include => "owner_uuid"}
+
+    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"], users(:user_foo_in_sharing_group).uuid
+  end
+
+  test 'get shared, 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 :shared, {:filters => [["group_class", "=", "project"]], :include => "owner_uuid"}
+
+    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 'get shared, 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 :shared, {:filters => [["group_class", "=", "project"]], :include => "owner_uuid"}
+
+    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
+
 end
index 769771e7beb52cafd5fa11033921973de501e54c..2d58012fa805b506215d3ce5f1ee4e6699efb3b1 100644 (file)
@@ -925,7 +925,7 @@ class ProjectDirectory(Directory):
         with llfuse.lock_released:
             if not self._current_user:
                 self._current_user = self.api.users().current().execute(num_retries=self.num_retries)
-            return self._current_user["uuid"] in self.project_object["writable_by"]
+            return self._current_user["uuid"] in self.project_object.get("writable_by", [])
 
     def persisted(self):
         return True
@@ -1049,35 +1049,55 @@ class SharedDirectory(Directory):
                 if not self.stale():
                     return
 
-                all_projects = arvados.util.list_all(
-                    self.api.groups().list, self.num_retries,
-                    filters=[['group_class','=','project']],
-                    select=["uuid", "owner_uuid"])
-                objects = {}
-                for ob in all_projects:
-                    objects[ob['uuid']] = ob
-
+                contents = {}
                 roots = []
                 root_owners = set()
-                current_uuid = self.current_user['uuid']
-                for ob in all_projects:
-                    if ob['owner_uuid'] != current_uuid and ob['owner_uuid'] not in objects:
-                        roots.append(ob['uuid'])
-                        root_owners.add(ob['owner_uuid'])
-
-                lusers = arvados.util.list_all(
-                    self.api.users().list, self.num_retries,
-                    filters=[['uuid','in', list(root_owners)]])
-                lgroups = arvados.util.list_all(
-                    self.api.groups().list, self.num_retries,
-                    filters=[['uuid','in', list(root_owners)+roots]])
-
-                for l in lusers:
-                    objects[l["uuid"]] = l
-                for l in lgroups:
-                    objects[l["uuid"]] = l
+                objects = {}
+
+                methods = self.api._rootDesc.get('resources')["groups"]['methods']
+                if 'httpMethod' in methods.get('shared', {}):
+                    page = []
+                    while True:
+                        resp = self.api.groups().shared(filters=[['group_class', '=', 'project']]+page,
+                                                        order="uuid",
+                                                        limit=10000,
+                                                        count="none",
+                                                        include="owner_uuid").execute()
+                        if not resp["items"]:
+                            break
+                        page = [["uuid", ">", resp["items"][len(resp["items"])-1]["uuid"]]]
+                        for r in resp["items"]:
+                            objects[r["uuid"]] = r
+                            roots.append(r["uuid"])
+                        for r in resp["included"]:
+                            objects[r["uuid"]] = r
+                            root_owners.add(r["uuid"])
+                else:
+                    all_projects = arvados.util.list_all(
+                        self.api.groups().list, self.num_retries,
+                        filters=[['group_class','=','project']],
+                        select=["uuid", "owner_uuid"])
+                    for ob in all_projects:
+                        objects[ob['uuid']] = ob
+
+                    current_uuid = self.current_user['uuid']
+                    for ob in all_projects:
+                        if ob['owner_uuid'] != current_uuid and ob['owner_uuid'] not in objects:
+                            roots.append(ob['uuid'])
+                            root_owners.add(ob['owner_uuid'])
+
+                    lusers = arvados.util.list_all(
+                        self.api.users().list, self.num_retries,
+                        filters=[['uuid','in', list(root_owners)]])
+                    lgroups = arvados.util.list_all(
+                        self.api.groups().list, self.num_retries,
+                        filters=[['uuid','in', list(root_owners)+roots]])
+
+                    for l in lusers:
+                        objects[l["uuid"]] = l
+                    for l in lgroups:
+                        objects[l["uuid"]] = l
 
-                contents = {}
                 for r in root_owners:
                     if r in objects:
                         obr = objects[r]