diff options
Diffstat (limited to 'spec/services/members')
11 files changed, 193 insertions, 103 deletions
diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb index f7fbac612ee..d26bab7bb0a 100644 --- a/spec/services/members/approve_access_request_service_spec.rb +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -9,36 +9,34 @@ RSpec.describe Members::ApproveAccessRequestService do let(:access_requester_user) { create(:user) } let(:access_requester) { source.requesters.find_by!(user_id: access_requester_user.id) } let(:opts) { {} } - - shared_examples 'a service raising ActiveRecord::RecordNotFound' do - it 'raises ActiveRecord::RecordNotFound' do - expect { described_class.new(current_user).execute(access_requester, **opts) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + let(:params) { {} } + let(:custom_access_level) { Gitlab::Access::MAINTAINER } shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do it 'raises Gitlab::Access::AccessDeniedError' do - expect { described_class.new(current_user).execute(access_requester, **opts) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect { described_class.new(current_user, params).execute(access_requester, **opts) }.to raise_error(Gitlab::Access::AccessDeniedError) end end shared_examples 'a service approving an access request' do it 'succeeds' do - expect { described_class.new(current_user).execute(access_requester, **opts) }.to change { source.requesters.count }.by(-1) + expect { described_class.new(current_user, params).execute(access_requester, **opts) }.to change { source.requesters.count }.by(-1) end it 'returns a <Source>Member' do - member = described_class.new(current_user).execute(access_requester, **opts) + member = described_class.new(current_user, params).execute(access_requester, **opts) expect(member).to be_a "#{source.class}Member".constantize expect(member.requested_at).to be_nil end context 'with a custom access level' do + let(:params) { { access_level: custom_access_level } } + it 'returns a ProjectMember with the custom access level' do - member = described_class.new(current_user, access_level: Gitlab::Access::MAINTAINER).execute(access_requester, **opts) + member = described_class.new(current_user, params).execute(access_requester, **opts) - expect(member.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(member.access_level).to eq(custom_access_level) end end end @@ -111,5 +109,38 @@ RSpec.describe Members::ApproveAccessRequestService do let(:source) { group } end end + + context 'in a project' do + let_it_be(:group_project) { create(:project, :public, group: create(:group, :public)) } + + let(:source) { group_project } + let(:custom_access_level) { Gitlab::Access::OWNER } + let(:params) { { access_level: custom_access_level } } + + before do + group_project.request_access(access_requester_user) + end + + context 'maintainers' do + before do + group_project.add_maintainer(current_user) + end + + context 'cannot approve the access request of a requester to give them OWNER permissions' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + end + end + + context 'owners' do + before do + # so that `current_user` is considered an `OWNER` in the project via inheritance. + group_project.group.add_owner(current_user) + end + + context 'can approve the access request of a requester to give them OWNER permissions' do + it_behaves_like 'a service approving an access request' + end + end + end end end diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 730175af0bb..e79e13af769 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -33,6 +33,18 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ it 'raises a Gitlab::Access::AccessDeniedError' do expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError) end + + context 'when a project maintainer attempts to add owners' do + let(:access_level) { Gitlab::Access::OWNER } + + before do + source.add_maintainer(current_user) + end + + it 'raises a Gitlab::Access::AccessDeniedError' do + expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end end context 'when passing an invalid source' do diff --git a/spec/services/members/creator_service_spec.rb b/spec/services/members/creator_service_spec.rb index ff5bf705b6c..8b1df2ab86d 100644 --- a/spec/services/members/creator_service_spec.rb +++ b/spec/services/members/creator_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Members::CreatorService do describe '#execute' do it 'raises error for new member on authorization check implementation' do expect do - described_class.new(source, user, :maintainer, current_user: current_user).execute + described_class.add_user(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end @@ -19,7 +19,7 @@ RSpec.describe Members::CreatorService do source.add_developer(user) expect do - described_class.new(source, user, :maintainer, current_user: current_user).execute + described_class.add_user(source, user, :maintainer, current_user: current_user) end.to raise_error(NotImplementedError) end end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 1a1283b1078..9f0daba3327 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -105,26 +105,46 @@ RSpec.describe Members::DestroyService do context 'with a project member' do let(:member) { group_project.members.find_by(user_id: member_user.id) } - before do - group_project.add_developer(member_user) + context 'when current user does not have any membership management permissions' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + + context 'when skipping authorisation' do + it_behaves_like 'a service destroying a member with access' do + let(:opts) { { skip_authorization: true, unassign_issuables: true } } + end + end end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + context 'when a project maintainer tries to destroy a project owner' do + before do + group_project.add_owner(member_user) + end - it_behaves_like 'a service destroying a member with access' do - let(:opts) { { skip_authorization: true, unassign_issuables: true } } + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + + context 'when skipping authorisation' do + it_behaves_like 'a service destroying a member with access' do + let(:opts) { { skip_authorization: true, unassign_issuables: true } } + end + end end end + end - context 'with a group member' do - let(:member) { group.members.find_by(user_id: member_user.id) } + context 'with a group member' do + let(:member) { group.members.find_by(user_id: member_user.id) } - before do - group.add_developer(member_user) - end + before do + group.add_developer(member_user) + end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + context 'when skipping authorisation' do it_behaves_like 'a service destroying a member with access' do let(:opts) { { skip_authorization: true, unassign_issuables: true } } end diff --git a/spec/services/members/groups/bulk_creator_service_spec.rb b/spec/services/members/groups/bulk_creator_service_spec.rb deleted file mode 100644 index 0623ae00080..00000000000 --- a/spec/services/members/groups/bulk_creator_service_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Members::Groups::BulkCreatorService do - it_behaves_like 'bulk member creation' do - let_it_be(:source, reload: true) { create(:group, :public) } - let_it_be(:member_type) { GroupMember } - end -end diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb index c3ba7c0374d..b80b7998eac 100644 --- a/spec/services/members/groups/creator_service_spec.rb +++ b/spec/services/members/groups/creator_service_spec.rb @@ -3,16 +3,24 @@ require 'spec_helper' RSpec.describe Members::Groups::CreatorService do + let_it_be(:source, reload: true) { create(:group, :public) } + let_it_be(:user) { create(:user) } + describe '.access_levels' do it 'returns Gitlab::Access.options_with_owner' do expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) end end - describe '#execute' do - let_it_be(:source, reload: true) { create(:group, :public) } - let_it_be(:user) { create(:user) } + it_behaves_like 'owner management' + + describe '.add_users' do + it_behaves_like 'bulk member creation' do + let_it_be(:member_type) { GroupMember } + end + end + describe '.add_user' do it_behaves_like 'member creation' do let_it_be(:member_type) { GroupMember } end @@ -22,7 +30,7 @@ RSpec.describe Members::Groups::CreatorService do expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).once 1.upto(3) do - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) end end end diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb index 8213e8baae0..a948041479b 100644 --- a/spec/services/members/invite_service_spec.rb +++ b/spec/services/members/invite_service_spec.rb @@ -367,20 +367,21 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ context 'when email is already a member with a user on the project' do let!(:existing_member) { create(:project_member, :guest, project: project) } - let(:params) { { email: "#{existing_member.user.email}" } } + let(:params) { { email: "#{existing_member.user.email}", access_level: ProjectMember::MAINTAINER } } - it 'returns an error for the already invited email' do - expect_not_to_create_members - expect(result[:message][existing_member.user.email]).to eq("User already exists in source") + it 'allows re-invite of an already invited email and updates the access_level' do + expect { result }.not_to change(ProjectMember, :count) + expect(result[:status]).to eq(:success) + expect(existing_member.reset.access_level).to eq ProjectMember::MAINTAINER end context 'when email belongs to an existing user as a secondary email' do let(:secondary_email) { create(:email, email: 'secondary@example.com', user: existing_member.user) } let(:params) { { email: "#{secondary_email.email}" } } - it 'returns an error for the already invited email' do - expect_not_to_create_members - expect(result[:message][secondary_email.email]).to eq("User already exists in source") + it 'allows re-invite to an already invited email' do + expect_to_create_members(count: 0) + expect(result[:status]).to eq(:success) end end end diff --git a/spec/services/members/mailgun/process_webhook_service_spec.rb b/spec/services/members/mailgun/process_webhook_service_spec.rb deleted file mode 100644 index d6a21183395..00000000000 --- a/spec/services/members/mailgun/process_webhook_service_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Members::Mailgun::ProcessWebhookService do - describe '#execute', :aggregate_failures do - let_it_be(:member) { create(:project_member, :invited) } - - let(:raw_invite_token) { member.raw_invite_token } - let(:payload) { { 'user-variables' => { ::Members::Mailgun::INVITE_EMAIL_TOKEN_KEY => raw_invite_token } } } - - subject(:service) { described_class.new(payload).execute } - - it 'marks the member invite email success as false' do - expect(Gitlab::AppLogger).to receive(:info).with(/^UPDATED MEMBER INVITE_EMAIL_SUCCESS/).and_call_original - - expect { service }.to change { member.reload.invite_email_success }.from(true).to(false) - end - - context 'when member can not be found' do - let(:raw_invite_token) { '_foobar_' } - - it 'does not change member status' do - expect(Gitlab::AppLogger).not_to receive(:info).with(/^UPDATED MEMBER INVITE_EMAIL_SUCCESS/) - - expect { service }.not_to change { member.reload.invite_email_success } - end - end - - context 'when invite token is not found in payload' do - let(:payload) { {} } - - it 'does not change member status and logs an error' do - expect(Gitlab::AppLogger).not_to receive(:info).with(/^UPDATED MEMBER INVITE_EMAIL_SUCCESS/) - expect(Gitlab::ErrorTracking).to receive(:track_exception).with( - an_instance_of(described_class::ProcessWebhookServiceError)) - - expect { service }.not_to change { member.reload.invite_email_success } - end - end - end -end diff --git a/spec/services/members/projects/bulk_creator_service_spec.rb b/spec/services/members/projects/bulk_creator_service_spec.rb deleted file mode 100644 index 7acb7d79fe7..00000000000 --- a/spec/services/members/projects/bulk_creator_service_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Members::Projects::BulkCreatorService do - it_behaves_like 'bulk member creation' do - let_it_be(:source, reload: true) { create(:project, :public) } - let_it_be(:member_type) { ProjectMember } - end -end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb index 7605238c3c5..38955122ab0 100644 --- a/spec/services/members/projects/creator_service_spec.rb +++ b/spec/services/members/projects/creator_service_spec.rb @@ -3,16 +3,24 @@ require 'spec_helper' RSpec.describe Members::Projects::CreatorService do + let_it_be(:source, reload: true) { create(:project, :public) } + let_it_be(:user) { create(:user) } + describe '.access_levels' do it 'returns Gitlab::Access.sym_options_with_owner' do expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) end end - describe '#execute' do - let_it_be(:source, reload: true) { create(:project, :public) } - let_it_be(:user) { create(:user) } + it_behaves_like 'owner management' + + describe '.add_users' do + it_behaves_like 'bulk member creation' do + let_it_be(:member_type) { ProjectMember } + end + end + describe '.add_user' do it_behaves_like 'member creation' do let_it_be(:member_type) { ProjectMember } end @@ -22,7 +30,7 @@ RSpec.describe Members::Projects::CreatorService do expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to receive(:bulk_perform_in).once 1.upto(3) do - described_class.new(source, user, :maintainer).execute + described_class.add_user(source, user, :maintainer) end end end diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index a1b1397d444..f919d6d1516 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -9,8 +9,9 @@ RSpec.describe Members::UpdateService do let(:member_user) { create(:user) } let(:permission) { :update } let(:member) { source.members_and_requesters.find_by!(user_id: member_user.id) } + let(:access_level) { Gitlab::Access::MAINTAINER } let(:params) do - { access_level: Gitlab::Access::MAINTAINER } + { access_level: access_level } end subject { described_class.new(current_user, params).execute(member, permission: permission) } @@ -29,7 +30,7 @@ RSpec.describe Members::UpdateService do updated_member = subject.fetch(:member) expect(updated_member).to be_valid - expect(updated_member.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(updated_member.access_level).to eq(access_level) end it 'returns success status' do @@ -111,4 +112,75 @@ RSpec.describe Members::UpdateService do let(:source) { group } end end + + context 'in a project' do + let_it_be(:group_project) { create(:project, group: create(:group)) } + + let(:source) { group_project } + + context 'a project maintainer' do + before do + group_project.add_maintainer(current_user) + end + + context 'cannot update a member to OWNER' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'cannot update themselves to OWNER' do + let(:member) { source.members_and_requesters.find_by!(user_id: current_user.id) } + + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'cannot downgrade a member from OWNER' do + before do + group_project.add_owner(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::MAINTAINER } + end + end + end + + context 'owners' do + before do + # so that `current_user` is considered an `OWNER` in the project via inheritance. + group_project.group.add_owner(current_user) + end + + context 'can update a member to OWNER' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service updating a member' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'can downgrade a member from OWNER' do + before do + group_project.add_owner(member_user) + end + + it_behaves_like 'a service updating a member' do + let(:access_level) { Gitlab::Access::MAINTAINER } + end + end + end + end end |