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>

1  2 
services/api/app/controllers/application_controller.rb
services/api/app/controllers/arvados/v1/groups_controller.rb
services/api/test/functional/arvados/v1/groups_controller_test.rb

index 71176056fa3f24c5340796d2ef2fe20069acd666,81e4b961e441ab3a7b032ef217c8c65b32eb725e..1cb49f46749054859980471772aa8f4b3f7f51e1
@@@ -78,6 -78,7 +78,7 @@@ class ApplicationController < ActionCon
      @distinct = nil
      @response_resource_name = nil
      @attrs = nil
+     @extra_included = nil
    end
  
    def default_url_options
        req_id = "req-" + Random::DEFAULT.rand(2**128).to_s(36)[0..19]
      end
      response.headers['X-Request-Id'] = Thread.current[:request_id] = req_id
 -    yield
 +    Rails.logger.tagged(req_id) do
 +      yield
 +    end
      Thread.current[:request_id] = nil
    end
  
        :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,e8ce4aae65b9c41dbae1385229ee77a5651acdd7..a963d1fc4d875d2d69129ebb6ec6606e7f65d666
@@@ -7,6 -7,9 +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({
      })
    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
      all_objects = []
      @items_available = 0
  
 +    # Reload the orders param, this time without prefixing unqualified
 +    # columns ("name" => "groups.name"). Here, unqualified orders
 +    # apply to each table being searched, not "groups".
 +    load_limit_offset_order_params(fill_table_names: false)
 +
      # Trick apply_where_limit_order_params into applying suitable
      # per-table values. *_all are the real ones we'll apply to the
      # aggregate set.
        # table_name for the current klass, apply that order.
        # Otherwise, order by recency.
        request_order =
 -        request_orders.andand.find { |r| r =~ /^#{klass.table_name}\./i } ||
 +        request_orders.andand.find { |r| r =~ /^#{klass.table_name}\./i || r !~ /\./ } ||
          klass.default_orders.join(", ")
  
        @select = nil
index d2b2ad7de824e1d0805fafdda493a7a9eab99fe4,5269701900abbf05433511464237e160bab65aff..4506f76c6dd56655fb15c03677f0fddc26efb18e
@@@ -139,59 -139,45 +139,59 @@@ class Arvados::V1::GroupsControllerTes
      assert_includes ids, collections(:baz_file_in_asubproject).uuid
    end
  
 -  [['asc', :<=],
 -   ['desc', :>=]].each do |order, operator|
 -    test "user with project read permission can sort project collections #{order}" do
 +  [
 +    ['collections.name', 'asc', :<=, "name"],
 +    ['collections.name', 'desc', :>=, "name"],
 +    ['name', 'asc', :<=, "name"],
 +    ['name', 'desc', :>=, "name"],
 +    ['collections.created_at', 'asc', :<=, "created_at"],
 +    ['collections.created_at', 'desc', :>=, "created_at"],
 +    ['created_at', 'asc', :<=, "created_at"],
 +    ['created_at', 'desc', :>=, "created_at"],
 +  ].each do |column, order, operator, field|
 +    test "user with project read permission can sort projects on #{column} #{order}" do
        authorize_with :project_viewer
        get :contents, {
          id: groups(:asubproject).uuid,
          format: :json,
          filters: [['uuid', 'is_a', "arvados#collection"]],
 -        order: "collections.name #{order}"
 +        order: "#{column} #{order}"
        }
 -      sorted_names = json_response['items'].collect { |item| item["name"] }
 -      # Here we avoid assuming too much about the database
 -      # collation. Both "alice"<"Bob" and "alice">"Bob" can be
 -      # correct. Hopefully it _is_ safe to assume that if "a" comes
 -      # before "b" in the ascii alphabet, "aX">"bY" is never true for
 -      # any strings X and Y.
 -      reliably_sortable_names = sorted_names.select do |name|
 -        name[0] >= 'a' and name[0] <= 'z'
 -      end.uniq do |name|
 -        name[0]
 -      end
 -      # Preserve order of sorted_names. But do not use &=. If
 -      # sorted_names has out-of-order duplicates, we want to preserve
 -      # them here, so we can detect them and fail the test below.
 -      sorted_names.select! do |name|
 -        reliably_sortable_names.include? name
 -      end
 -      actually_checked_anything = false
 -      previous = nil
 -      sorted_names.each do |entry|
 -        if previous
 -          assert_operator(previous, operator, entry,
 -                          "Entries sorted incorrectly.")
 -          actually_checked_anything = true
 +      sorted_values = json_response['items'].collect { |item| item[field] }
 +      if field == "name"
 +        # Here we avoid assuming too much about the database
 +        # collation. Both "alice"<"Bob" and "alice">"Bob" can be
 +        # correct. Hopefully it _is_ safe to assume that if "a" comes
 +        # before "b" in the ascii alphabet, "aX">"bY" is never true for
 +        # any strings X and Y.
 +        reliably_sortable_names = sorted_values.select do |name|
 +          name[0] >= 'a' && name[0] <= 'z'
 +        end.uniq do |name|
 +          name[0]
          end
 -        previous = entry
 +        # Preserve order of sorted_values. But do not use &=. If
 +        # sorted_values has out-of-order duplicates, we want to preserve
 +        # them here, so we can detect them and fail the test below.
 +        sorted_values.select! do |name|
 +          reliably_sortable_names.include? name
 +        end
 +      end
 +      assert_sorted(operator, sorted_values)
 +    end
 +  end
 +
 +  def assert_sorted(operator, sorted_items)
 +    actually_checked_anything = false
 +    previous = nil
 +    sorted_items.each do |entry|
 +      if !previous.nil?
 +        assert_operator(previous, operator, entry,
 +                        "Entries sorted incorrectly.")
 +        actually_checked_anything = true
        end
 -      assert actually_checked_anything, "Didn't even find two names to compare."
 +      previous = entry
      end
 +    assert actually_checked_anything, "Didn't even find two items to compare."
    end
  
    test 'list objects across multiple projects' do
        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