New AccessRequestsFinder

Signed-off-by: Rémy Coutable <remy@rymai.me>

app/controllers/admin/groups_controller.rb

@@ -10,7 +10,7 @@ class Admin::GroupsController < Admin::ApplicationController
 
   def show
     @members = @group.members.order("access_level DESC").page(params[:members_page])
-    @requesters = @group.requesters
+    @requesters = AccessRequestsFinder.new(@group).execute(current_user)
     @projects = @group.projects.page(params[:projects_page])
   end
 

app/controllers/admin/projects_controller.rb

@@ -22,7 +22,7 @@ class Admin::ProjectsController < Admin::ApplicationController
     end
 
     @project_members = @project.members.page(params[:project_members_page])
-    @requesters = @project.requesters
+    @requesters = AccessRequestsFinder.new(@project).execute(current_user)
   end
 
   def transfer

app/controllers/groups/group_members_controller.rb

@@ -15,7 +15,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
     end
 
     @members = @members.order('access_level DESC').page(params[:page]).per(50)
-    @requesters = @group.requesters if can?(current_user, :admin_group, @group)
+    @requesters = AccessRequestsFinder.new(@group).execute(current_user)
 
     @group_member = @group.group_members.new
   end

app/controllers/projects/project_members_controller.rb

@@ -29,7 +29,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
       @group_members = @group_members.order('access_level DESC')
     end
 
-    @requesters = @project.requesters if can?(current_user, :admin_project, @project)
+    @requesters = AccessRequestsFinder.new(@project).execute(current_user)
 
     @project_member = @project.project_members.new
     @project_group_links = @project.project_group_links

app/finders/access_requests_finder.rb

+class AccessRequestsFinder
+  attr_accessor :source
+
+  # Arguments:
+  #   source - a Group or Project
+  def initialize(source)
+    @source = source
+  end
+
+  def execute(current_user, raise_error: false)
+    if cannot_see_access_requests?(current_user)
+      raise Gitlab::Access::AccessDeniedError if raise_error
+
+      return []
+    end
+
+    source.requesters
+  end
+
+  def execute!(current_user)
+    execute(current_user, raise_error: true)
+  end
+
+  private
+
+  def cannot_see_access_requests?(current_user)
+    !source || !current_user || !current_user.can?(:"admin_#{source.class.to_s.underscore}", source)
+  end
+end

lib/api/access_requests.rb

@@ -16,9 +16,9 @@ module API
         #  GET /projects/:id/access_requests
         get ":id/access_requests" do
           source = find_source(source_type, params[:id])
-          authorize_admin_source!(source_type, source)
 
-          access_requesters = paginate(source.requesters.includes(:user))
+          access_requesters = AccessRequestsFinder.new(source).execute!(current_user)
+          access_requesters = paginate(access_requesters.includes(:user))
 
           present access_requesters.map(&:user), with: Entities::AccessRequester, source: source
         end

spec/finders/access_requests_finder_spec.rb

+require 'spec_helper'
+
+describe AccessRequestsFinder, services: true do
+  let(:user) { create(:user) }
+  let(:access_requester) { create(:user) }
+  let(:project) { create(:project) }
+  let(:group) { create(:group) }
+
+  before do
+    project.request_access(access_requester)
+    group.request_access(access_requester)
+  end
+
+  shared_examples 'a finder returning access requesters' do |method_name|
+    it 'returns access requesters' do
+      access_requesters = described_class.new(source).public_send(method_name, user)
+
+      expect(access_requesters.size).to eq(1)
+      expect(access_requesters.first).to be_a "#{source.class.to_s}Member".constantize
+      expect(access_requesters.first.user).to eq(access_requester)
+    end
+  end
+
+  shared_examples 'a finder returning no results' do |method_name|
+    it 'raises Gitlab::Access::AccessDeniedError' do
+      expect(described_class.new(source).public_send(method_name, user)).to be_empty
+    end
+  end
+
+  shared_examples 'a finder raising Gitlab::Access::AccessDeniedError' do |method_name|
+    it 'raises Gitlab::Access::AccessDeniedError' do
+      expect { described_class.new(source).public_send(method_name, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+    end
+  end
+
+  describe '#execute' do
+    context 'when current user cannot see project access requests' do
+      it_behaves_like 'a finder returning no results', :execute do
+        let(:source) { project }
+      end
+
+      it_behaves_like 'a finder returning no results', :execute do
+        let(:source) { group }
+      end
+    end
+
+    context 'when current user can see access requests' do
+      before do
+        project.team << [user, :master]
+        group.add_owner(user)
+      end
+
+      it_behaves_like 'a finder returning access requesters', :execute do
+        let(:source) { project }
+      end
+
+      it_behaves_like 'a finder returning access requesters', :execute do
+        let(:source) { group }
+      end
+    end
+  end
+
+  describe '#execute!' do
+    context 'when current user cannot see access requests' do
+      it_behaves_like 'a finder raising Gitlab::Access::AccessDeniedError', :execute! do
+        let(:source) { project }
+      end
+
+      it_behaves_like 'a finder raising Gitlab::Access::AccessDeniedError', :execute! do
+        let(:source) { group }
+      end
+    end
+
+    context 'when current user can see access requests' do
+      before do
+        project.team << [user, :master]
+        group.add_owner(user)
+      end
+
+      it_behaves_like 'a finder returning access requesters', :execute! do
+        let(:source) { project }
+      end
+
+      it_behaves_like 'a finder returning access requesters', :execute! do
+        let(:source) { group }
+      end
+    end
+  end
+end