diff options
| -rw-r--r-- | app/controllers/concerns/membership_actions.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/access_requestable.rb | 16 | ||||
| -rw-r--r-- | app/models/group.rb | 5 | ||||
| -rw-r--r-- | app/models/project.rb | 5 | ||||
| -rw-r--r-- | app/services/members/request_access_service.rb | 25 | ||||
| -rw-r--r-- | lib/api/access_requests.rb | 2 | ||||
| -rw-r--r-- | spec/models/concerns/access_requestable_spec.rb | 40 | ||||
| -rw-r--r-- | spec/models/group_spec.rb | 7 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/access_requests_spec.rb | 18 | ||||
| -rw-r--r-- | spec/services/members/request_access_service_spec.rb | 57 |
11 files changed, 123 insertions, 62 deletions
diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 52682ef9dc9..1d34441e8ea 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -3,7 +3,7 @@ module MembershipActions include MembersHelper def request_access - membershipable.request_access(current_user) + Members::RequestAccessService.new(membershipable, current_user).execute redirect_to polymorphic_path(membershipable), notice: 'Your request for access has been queued for review.' diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb deleted file mode 100644 index eedd32a729f..00000000000 --- a/app/models/concerns/access_requestable.rb +++ /dev/null @@ -1,16 +0,0 @@ -# == AccessRequestable concern -# -# Contains functionality related to objects that can receive request for access. -# -# Used by Project, and Group. -# -module AccessRequestable - extend ActiveSupport::Concern - - def request_access(user) - members.create( - access_level: Gitlab::Access::DEVELOPER, - user: user, - requested_at: Time.now.utc) - end -end diff --git a/app/models/group.rb b/app/models/group.rb index aefb94b2ada..1f610aef87a 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -3,7 +3,6 @@ require 'carrierwave/orm/activerecord' class Group < Namespace include Gitlab::ConfigHelper include Gitlab::VisibilityLevel - include AccessRequestable include Referable has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' @@ -102,6 +101,10 @@ class Group < Namespace self[:lfs_enabled] end + def request_access(user) + Members::RequestAccessService.new(self, user).execute + end + def add_users(user_ids, access_level, current_user: nil, expires_at: nil) user_ids.each do |user_id| Member.add_user( diff --git a/app/models/project.rb b/app/models/project.rb index 7265cb55594..cebf6199974 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -5,7 +5,6 @@ class Project < ActiveRecord::Base include Gitlab::ShellAdapter include Gitlab::VisibilityLevel include Gitlab::CurrentSettings - include AccessRequestable include Referable include Sortable include AfterCommitQueue @@ -1012,6 +1011,10 @@ class Project < ActiveRecord::Base update_all(updated_at: Time.now) end + def request_access(user) + Members::RequestAccessService.new(self, user).execute + end + def project_member(user) project_members.find_by(user_id: user) end diff --git a/app/services/members/request_access_service.rb b/app/services/members/request_access_service.rb new file mode 100644 index 00000000000..5c0d2d83445 --- /dev/null +++ b/app/services/members/request_access_service.rb @@ -0,0 +1,25 @@ +module Members + class RequestAccessService < BaseService + attr_accessor :source + + def initialize(source, current_user) + @source = source + @current_user = current_user + end + + def execute + raise Gitlab::Access::AccessDeniedError if cannot_request_access?(source) + + source.members.create( + access_level: Gitlab::Access::DEVELOPER, + user: current_user, + requested_at: Time.now.utc) + end + + private + + def cannot_request_access?(source) + !source || !can?(current_user, :request_access, source) + end + end +end diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 29a97ccbd75..895847add9e 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -33,7 +33,7 @@ module API # POST /projects/:id/access_requests post ":id/access_requests" do source = find_source(source_type, params[:id]) - access_requester = source.request_access(current_user) + access_requester = ::Members::RequestAccessService.new(source, current_user).execute if access_requester.persisted? present access_requester.user, with: Entities::AccessRequester, access_requester: access_requester diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb deleted file mode 100644 index 96eee0e8bdd..00000000000 --- a/spec/models/concerns/access_requestable_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'spec_helper' - -describe AccessRequestable do - describe 'Group' do - describe '#request_access' do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } - - it { expect(group.request_access(user)).to be_a(GroupMember) } - it { expect(group.request_access(user).user).to eq(user) } - end - - describe '#access_requested?' do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } - - before { group.request_access(user) } - - it { expect(group.requesters.exists?(user_id: user)).to be_truthy } - end - end - - describe 'Project' do - describe '#request_access' do - let(:project) { create(:empty_project, :public) } - let(:user) { create(:user) } - - it { expect(project.request_access(user)).to be_a(ProjectMember) } - end - - describe '#access_requested?' do - let(:project) { create(:empty_project, :public) } - let(:user) { create(:user) } - - before { project.request_access(user) } - - it { expect(project.requesters.exists?(user_id: user)).to be_truthy } - end - end -end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0b3ef9b98fd..c015147f0db 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -105,6 +105,13 @@ describe Group, models: true do it { expect(group.human_name).to eq(group.name) } end + describe '#request_access' do + let(:user) { create(:user) } + + it { expect(group.request_access(user)).to be_a(GroupMember) } + it { expect(group.request_access(user).user).to eq(user) } + end + describe '#add_user' do let(:user) { create(:user) } before { group.add_user(user, GroupMember::MASTER) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 83f61f0af0a..f88636e1c19 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -942,6 +942,14 @@ describe Project, models: true do end end + describe '#request_access' do + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + + it { expect(project.request_access(user)).to be_a(ProjectMember) } + it { expect(project.request_access(user).user).to eq(user) } + end + describe '.search' do let(:project) { create(:project, description: 'kitten mittens') } diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index d78494b76fa..905a7311372 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -64,12 +64,12 @@ describe API::AccessRequests, api: true do context 'when authenticated as a member' do %i[developer master].each do |type| context "as a #{type}" do - it 'returns 400' do + it 'returns 403' do expect do user = public_send(type) post api("/#{source_type.pluralize}/#{source.id}/access_requests", user) - expect(response).to have_http_status(400) + expect(response).to have_http_status(403) end.not_to change { source.requesters.count } end end @@ -87,6 +87,20 @@ describe API::AccessRequests, api: true do end context 'when authenticated as a stranger' do + context "when access request is disabled for the #{source_type}" do + before do + source.update(request_access_enabled: false) + end + + it 'returns 403' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) + + expect(response).to have_http_status(403) + end.not_to change { source.requesters.count } + end + end + it 'returns 201' do expect do post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb new file mode 100644 index 00000000000..dff5b4917ae --- /dev/null +++ b/spec/services/members/request_access_service_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Members::RequestAccessService, services: true do + let(:user) { create(:user) } + let(:project) { create(:project, :private) } + let(:group) { create(:group, :private) } + + shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do + it 'raises Gitlab::Access::AccessDeniedError' do + expect { described_class.new(source, user).execute }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end + + shared_examples 'a service creating a access request' do + it 'succeeds' do + expect { described_class.new(source, user).execute }.to change { source.requesters.count }.by(1) + end + + it 'returns a <Source>Member' do + member = described_class.new(source, user).execute + + expect(member).to be_a "#{source.class.to_s}Member".constantize + expect(member.requested_at).to be_present + end + end + + context 'when source is nil' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { nil } + end + end + + context 'when current user cannot request access to the project' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:source) { group } + end + end + + context 'when current user can request access to the project' do + before do + project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + group.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it_behaves_like 'a service creating a access request' do + let(:source) { project } + end + + it_behaves_like 'a service creating a access request' do + let(:source) { group } + end + end +end |