diff options
-rw-r--r-- | app/models/concerns/access_requestable.rb | 5 | ||||
-rw-r--r-- | app/services/members/request_access_service.rb | 25 | ||||
-rw-r--r-- | spec/helpers/members_helper_spec.rb | 4 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/member_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_team_spec.rb | 4 | ||||
-rw-r--r-- | spec/requests/api/access_requests_spec.rb | 18 | ||||
-rw-r--r-- | spec/services/members/request_access_service_spec.rb | 57 |
9 files changed, 107 insertions, 14 deletions
diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb index eedd32a729f..62bc6b809f4 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 00000000000..2614153d900 --- /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 7998209b7b0..6703d88e357 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 0363bc74939..2e558018d74 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 0b1634f654a..c2f4601790d 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 83f61f0af0a..98f5305a855 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 f979d66c88c..e0f2dadf189 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 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 |