diff options
| -rw-r--r-- | app/controllers/admin/groups_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/admin/projects_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/groups/group_members_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/projects/project_members_controller.rb | 2 | ||||
| -rw-r--r-- | app/finders/access_requests_finder.rb | 29 | ||||
| -rw-r--r-- | lib/api/access_requests.rb | 4 | ||||
| -rw-r--r-- | spec/finders/access_requests_finder_spec.rb | 89 |
7 files changed, 124 insertions, 6 deletions
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index aed77d0358a..aa7570cd896 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 0d2f4f6eb38..1d963bdf7d5 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 272164cd0cc..9c323d7705a 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 42a7e5a2c30..2343c7d20ec 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 00000000000..78fb7627741 --- /dev/null +++ b/app/finders/access_requests_finder.rb @@ -0,0 +1,29 @@ +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 diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 9d1d9058996..7b9de7c9598 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 00000000000..6cc90299417 --- /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 |