diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index aed77d0358a7b90cbd8335afe12e80046d0bc1e8..aa7570cd896836419652a2db635a770f28545830 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/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 diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 0d2f4f6eb384d133812610abfb5f04829342a864..1d963bdf7d58df28917edef167ac5141a5e52458 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/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 diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 272164cd0ccc8a6cd32222789cb63afeff1222cf..9c323d7705a93b1482a5629dab0cff3e42635d4a 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/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 diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 42a7e5a2c30d6fe3e0bfa9ef8a16738cf59f3ca2..2343c7d20ec6840872d75863863a3bb574fa004d 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/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 diff --git a/app/finders/access_requests_finder.rb b/app/finders/access_requests_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..b6ee49df99b97d66296603a4b7c64efb4bfa8844 --- /dev/null +++ b/app/finders/access_requests_finder.rb @@ -0,0 +1,27 @@ +class AccessRequestsFinder + attr_accessor :source + + # Arguments: + # source - a Group or Project + def initialize(source) + @source = source + end + + def execute(*args) + execute!(*args) + rescue Gitlab::Access::AccessDeniedError + [] + end + + def execute!(current_user) + raise Gitlab::Access::AccessDeniedError unless can_see_access_requests?(current_user) + + source.requesters + end + + private + + def can_see_access_requests?(current_user) + source && Ability.allowed?(current_user, :"admin_#{source.class.to_s.underscore}", source) + end +end diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 9d1d9058996ccf6eebbb06c48b7fe9ef11df8b8f..7b9de7c9598bf39c2b82b797e3652874d4c5b2f5 100644 --- a/lib/api/access_requests.rb +++ b/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 diff --git a/spec/finders/access_requests_finder_spec.rb b/spec/finders/access_requests_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6cc902994178996edee81a63e7be44403d757fef --- /dev/null +++ b/spec/finders/access_requests_finder_spec.rb @@ -0,0 +1,89 @@ +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