diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb
index eedd32a729fb660a8825cd7d5103ca2589e0ca58..62bc6b809f405bff4ec3035bc5e776c40656416b 100644
--- a/app/models/concerns/access_requestable.rb
+++ b/app/models/concerns/access_requestable.rb
@@ -8,9 +8,6 @@ module AccessRequestable
extend ActiveSupport::Concern
def request_access(user)
- members.create(
- access_level: Gitlab::Access::DEVELOPER,
- user: user,
- requested_at: Time.now.utc)
+ Members::RequestAccessService.new(self, user).execute
end
end
diff --git a/app/services/members/request_access_service.rb b/app/services/members/request_access_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2614153d9006b337d9d8ed8f43b0ef6b861a5011
--- /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 unless can_request_access?(source)
+
+ source.members.create(
+ access_level: Gitlab::Access::DEVELOPER,
+ user: current_user,
+ requested_at: Time.now.utc)
+ end
+
+ private
+
+ def can_request_access?(source)
+ source && can?(current_user, :request_access, source)
+ end
+ end
+end
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index 7998209b7b00e7a759eade60dfa2e42ed37e7990..6703d88e3574ff1a9851bab0327b62344543b48d 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -11,7 +11,7 @@ describe MembersHelper do
describe '#remove_member_message' do
let(:requester) { build(:user) }
- let(:project) { create(:project) }
+ let(:project) { create(:empty_project, :public) }
let(:project_member) { build(:project_member, project: project) }
let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } }
let(:project_member_request) { project.request_access(requester) }
@@ -32,7 +32,7 @@ describe MembersHelper do
describe '#remove_member_title' do
let(:requester) { build(:user) }
- let(:project) { create(:project) }
+ let(:project) { create(:empty_project, :public) }
let(:project_member) { build(:project_member, project: project) }
let(:project_member_request) { project.request_access(requester) }
let(:group) { create(:group) }
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 0363bc7493912c93408496a9367b45e7e369a000..2e558018d7414037c2e65465164469ad020d6f84 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -402,7 +402,7 @@ describe Notify do
describe 'project access requested' do
context 'for a project in a user namespace' do
- let(:project) { create(:project).tap { |p| p.team << [p.owner, :master, p.owner] } }
+ let(:project) { create(:project, :public).tap { |p| p.team << [p.owner, :master, p.owner] } }
let(:user) { create(:user) }
let(:project_member) do
project.request_access(user)
@@ -429,7 +429,7 @@ describe Notify do
context 'for a project in a group' do
let(:group_owner) { create(:user) }
let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } }
- let(:project) { create(:project, namespace: group) }
+ let(:project) { create(:project, :public, namespace: group) }
let(:user) { create(:user) }
let(:project_member) do
project.request_access(user)
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 0b1634f654af88cd808b5b359530877918414b7d..c2f4601790d32e9139a14b5f1ad1c756b19fc628 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -57,7 +57,7 @@ describe Member, models: true do
describe 'Scopes & finders' do
before do
- project = create(:empty_project)
+ project = create(:empty_project, :public)
group = create(:group)
@owner_user = create(:user).tap { |u| group.add_owner(u) }
@owner = group.members.find_by(user_id: @owner_user.id)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 83f61f0af0a3b5c52db5314052b482af78140d50..98f5305a855f693c7952537fd33b896f46bb86f6 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -68,7 +68,7 @@ describe Project, models: true do
it { is_expected.to have_many(:forks).through(:forked_project_links) }
describe '#members & #requesters' do
- let(:project) { create(:project) }
+ let(:project) { create(:project, :public) }
let(:requester) { create(:user) }
let(:developer) { create(:user) }
before do
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index f979d66c88ca072cead62191c2c6463c25592ae6..e0f2dadf1896eff40b804c69666de1dad838b592 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -137,7 +137,7 @@ describe ProjectTeam, models: true do
describe '#find_member' do
context 'personal project' do
- let(:project) { create(:empty_project) }
+ let(:project) { create(:empty_project, :public) }
let(:requester) { create(:user) }
before do
@@ -200,7 +200,7 @@ describe ProjectTeam, models: true do
let(:requester) { create(:user) }
context 'personal project' do
- let(:project) { create(:empty_project) }
+ let(:project) { create(:empty_project, :public) }
context 'when project is not shared with group' do
before do
diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb
index d78494b76fac2a281852498c867b575a40a2454e..905a73113726ad66dddcb295832b7d8f71529556 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 0000000000000000000000000000000000000000..dff5b4917aef128d656307b9c55ce44dc5637a26
--- /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