diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 14:33:21 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 14:33:21 +0300 |
commit | 7021455bd1ed7b125c55eb1b33c5a01f2bc55ee0 (patch) | |
tree | 5bdc2229f5198d516781f8d24eace62fc7e589e9 /spec/services/members | |
parent | 185b095e93520f96e9cfc31d9c3e69b498cdab7c (diff) |
Add latest changes from gitlab-org/gitlab@15-6-stable-eev15.6.0-rc42
Diffstat (limited to 'spec/services/members')
-rw-r--r-- | spec/services/members/destroy_service_spec.rb | 25 | ||||
-rw-r--r-- | spec/services/members/invite_service_spec.rb | 6 | ||||
-rw-r--r-- | spec/services/members/update_service_spec.rb | 356 |
3 files changed, 291 insertions, 96 deletions
diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 8559c02be57..d0f009f1321 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -77,7 +77,7 @@ RSpec.describe Members::DestroyService do end end - shared_examples 'a service destroying an access requester' do + shared_examples 'a service destroying an access request of another user' do it_behaves_like 'a service destroying a member' it 'calls Member#after_decline_request' do @@ -85,12 +85,16 @@ RSpec.describe Members::DestroyService do described_class.new(current_user).execute(member, **opts) end + end + + shared_examples 'a service destroying an access request of self' do + it_behaves_like 'a service destroying a member' context 'when current user is the member' do it 'does not call Member#after_decline_request' do expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member) - described_class.new(member_user).execute(member, **opts) + described_class.new(current_user).execute(member, **opts) end end end @@ -277,11 +281,24 @@ RSpec.describe Members::DestroyService do group.add_owner(current_user) end - it_behaves_like 'a service destroying an access requester' do + it_behaves_like 'a service destroying an access request of another user' do + let(:member) { group_project.requesters.find_by(user_id: member_user.id) } + end + + it_behaves_like 'a service destroying an access request of another user' do + let(:member) { group.requesters.find_by(user_id: member_user.id) } + end + end + + context 'on withdrawing their own access request' do + let(:opts) { { skip_subresources: true } } + let(:current_user) { member_user } + + it_behaves_like 'a service destroying an access request of self' do let(:member) { group_project.requesters.find_by(user_id: member_user.id) } end - it_behaves_like 'a service destroying an access requester' do + it_behaves_like 'a service destroying an access request of self' do let(:member) { group.requesters.find_by(user_id: member_user.id) } end end diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb index 6dbe161ee02..23d4d671afc 100644 --- a/spec/services/members/invite_service_spec.rb +++ b/spec/services/members/invite_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ let(:params) { {} } let(:base_params) { { access_level: Gitlab::Access::GUEST, source: project, invite_source: '_invite_source_' } } - subject(:result) { described_class.new(user, base_params.merge(params) ).execute } + subject(:result) { described_class.new(user, base_params.merge(params)).execute } context 'when there is a valid member invited' do let(:params) { { email: 'email@example.org' } } @@ -393,7 +393,7 @@ 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}", access_level: ProjectMember::MAINTAINER } } + let(:params) { { email: existing_member.user.email.to_s, access_level: ProjectMember::MAINTAINER } } it 'allows re-invite of an already invited email and updates the access_level' do expect { result }.not_to change(ProjectMember, :count) @@ -403,7 +403,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ context 'when email belongs to an existing user as a confirmed secondary email' do let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: existing_member.user) } - let(:params) { { email: "#{secondary_email.email}" } } + let(:params) { { email: secondary_email.email.to_s } } it 'allows re-invite to an already invited email' do expect_to_create_members(count: 0) diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index f919d6d1516..eb8fae03c39 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -3,18 +3,34 @@ require 'spec_helper' RSpec.describe Members::UpdateService do - let(:project) { create(:project, :public) } - let(:group) { create(:group, :public) } - let(:current_user) { create(:user) } - 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: access_level } + let_it_be(:project) { create(:project, :public) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:current_user) { create(:user) } + let_it_be(:member_user1) { create(:user) } + let_it_be(:member_user2) { create(:user) } + let_it_be(:member_users) { [member_user1, member_user2] } + let_it_be(:permission) { :update } + let_it_be(:access_level) { Gitlab::Access::MAINTAINER } + let(:members) { source.members_and_requesters.where(user_id: member_users).to_a } + let(:update_service) { described_class.new(current_user, params) } + let(:params) { { access_level: access_level } } + let(:updated_members) do + result = subject + Array.wrap(result[:members] || result[:member]) end - subject { described_class.new(current_user, params).execute(member, permission: permission) } + before do + member_users.first.tap do |member_user| + expires_at = 10.days.from_now + project.add_member(member_user, Gitlab::Access::DEVELOPER, expires_at: expires_at) + group.add_member(member_user, Gitlab::Access::DEVELOPER, expires_at: expires_at) + end + + member_users[1..].each do |member_user| + project.add_developer(member_user) + group.add_developer(member_user) + end + end shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do it 'raises Gitlab::Access::AccessDeniedError' do @@ -23,164 +39,326 @@ RSpec.describe Members::UpdateService do end end - shared_examples 'a service updating a member' do - it 'updates the member' do - expect(TodosDestroyer::EntityLeaveWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name) + shared_examples 'current user cannot update the given members' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let_it_be(:source) { project } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let_it_be(:source) { group } + end + end + + shared_examples 'returns error status when params are invalid' do + let_it_be(:params) { { expires_at: 2.days.ago } } - updated_member = subject.fetch(:member) + specify do + expect(subject[:status]).to eq(:error) + end + end - expect(updated_member).to be_valid - expect(updated_member.access_level).to eq(access_level) + shared_examples 'a service updating members' do + it 'updates the members' do + new_access_levels = updated_members.map(&:access_level) + + expect(updated_members).not_to be_empty + expect(updated_members).to all(be_valid) + expect(new_access_levels).to all(be access_level) end it 'returns success status' do - result = subject.fetch(:status) + expect(subject.fetch(:status)).to eq(:success) + end - expect(result).to eq(:success) + it 'invokes after_execute with correct args' do + members.each do |member| + expect(update_service).to receive(:after_execute).with( + action: permission, + old_access_level: member.human_access, + old_expiry: member.expires_at, + member: member + ) + end + + subject end - context 'when member is downgraded to guest' do - shared_examples 'schedules to delete confidential todos' do - it do - expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once + it 'authorization update callback is triggered' do + expect(members).to all(receive(:refresh_member_authorized_projects).once) - updated_member = subject.fetch(:member) + subject + end + + it 'does not enqueues todos for deletion' do + members.each do |member| + expect(TodosDestroyer::EntityLeaveWorker) + .not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name) + end - expect(updated_member).to be_valid - expect(updated_member.access_level).to eq(Gitlab::Access::GUEST) + subject + end + + context 'when members are downgraded to guest' do + shared_examples 'schedules to delete confidential todos' do + it do + members.each do |member| + expect(TodosDestroyer::EntityLeaveWorker) + .to receive(:perform_in) + .with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once + end + + new_access_levels = updated_members.map(&:access_level) + expect(updated_members).to all(be_valid) + expect(new_access_levels).to all(be Gitlab::Access::GUEST) end end context 'with Gitlab::Access::GUEST level as a string' do - let(:params) { { access_level: Gitlab::Access::GUEST.to_s } } + let_it_be(:params) { { access_level: Gitlab::Access::GUEST.to_s } } it_behaves_like 'schedules to delete confidential todos' end context 'with Gitlab::Access::GUEST level as an integer' do - let(:params) { { access_level: Gitlab::Access::GUEST } } + let_it_be(:params) { { access_level: Gitlab::Access::GUEST } } it_behaves_like 'schedules to delete confidential todos' end end context 'when access_level is invalid' do - let(:params) { { access_level: 'invalid' } } + let_it_be(:params) { { access_level: 'invalid' } } it 'raises an error' do - expect { described_class.new(current_user, params) }.to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"') + expect { described_class.new(current_user, params) } + .to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"') end end - context 'when member is not valid' do - let(:params) { { expires_at: 2.days.ago } } + context 'when members update results in no change' do + let(:params) { { access_level: members.first.access_level } } - it 'returns error status' do - result = subject + it 'does not invoke update! and post_update' do + expect(update_service).not_to receive(:save!) + expect(update_service).not_to receive(:post_update) - expect(result[:status]).to eq(:error) + expect(subject[:status]).to eq(:success) end - end - end - - before do - project.add_developer(member_user) - group.add_developer(member_user) - end - context 'when current user cannot update the given member' do - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do - let(:source) { project } - end + it 'authorization update callback is not triggered' do + members.each { |member| expect(member).not_to receive(:refresh_member_authorized_projects) } - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do - let(:source) { group } + subject + end end end - context 'when current user can update the given member' do + shared_examples 'updating a project' do + let_it_be(:group_project) { create(:project, group: create(:group)) } + let_it_be(:source) { group_project } + before do - project.add_maintainer(current_user) - group.add_owner(current_user) + member_users.each { |member_user| group_project.add_developer(member_user) } end - it_behaves_like 'a service updating a member' do - let(:source) { project } - end + context 'as a project maintainer' do + before do + group_project.add_maintainer(current_user) + end - it_behaves_like 'a service updating a member' do - let(:source) { group } - end - end + it_behaves_like 'a service updating members' - context 'in a project' do - let_it_be(:group_project) { create(:project, group: create(:group)) } + context 'when member update results in an error' do + it_behaves_like 'a service returning an error' + end - let(:source) { group_project } + context 'and updating members to OWNER' do + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let_it_be(:access_level) { Gitlab::Access::OWNER } + end + end - context 'a project maintainer' do - before do - group_project.add_maintainer(current_user) + context 'and updating themselves to OWNER' do + let(:members) { source.members_and_requesters.find_by!(user_id: current_user.id) } + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let_it_be(:access_level) { Gitlab::Access::OWNER } + end end - context 'cannot update a member to OWNER' do + context 'and downgrading members from OWNER' do before do - group_project.add_developer(member_user) + member_users.each { |member_user| group_project.add_owner(member_user) } end it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do - let(:access_level) { Gitlab::Access::OWNER } + let_it_be(:access_level) { Gitlab::Access::MAINTAINER } end end + end - context 'cannot update themselves to OWNER' do - let(:member) { source.members_and_requesters.find_by!(user_id: current_user.id) } + context 'when current_user is considered an owner in the project via inheritance' do + before do + group_project.group.add_owner(current_user) + end + context 'and can update members to OWNER' do before do - group_project.add_developer(member_user) + member_users.each { |member_user| group_project.add_developer(member_user) } end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do - let(:access_level) { Gitlab::Access::OWNER } + it_behaves_like 'a service updating members' do + let_it_be(:access_level) { Gitlab::Access::OWNER } end end - context 'cannot downgrade a member from OWNER' do + context 'and can downgrade members from OWNER' do before do - group_project.add_owner(member_user) + member_users.each { |member_user| group_project.add_owner(member_user) } end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do - let(:access_level) { Gitlab::Access::MAINTAINER } + it_behaves_like 'a service updating members' do + let_it_be(:access_level) { Gitlab::Access::MAINTAINER } end end end + end + + shared_examples 'updating a group' do + let_it_be(:source) { group } + + before do + group.add_owner(current_user) + end + + it_behaves_like 'a service updating members' + + context 'when member update results in an error' do + it_behaves_like 'a service returning an error' + end + + context 'when group members expiration date is updated' do + let_it_be(:params) { { expires_at: 20.days.from_now } } + let(:notification_service) { instance_double(NotificationService) } - 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) + allow(NotificationService).to receive(:new).and_return(notification_service) end - context 'can update a member to OWNER' do - before do - group_project.add_developer(member_user) + it 'emails the users that their group membership expiry has changed' do + members.each do |member| + expect(notification_service).to receive(:updated_group_member_expiration).with(member) end - it_behaves_like 'a service updating a member' do - let(:access_level) { Gitlab::Access::OWNER } - end + subject end + end + end - context 'can downgrade a member from OWNER' do - before do - group_project.add_owner(member_user) + context 'when :bulk_update_membership_roles feature flag is disabled' do + let(:member) { source.members_and_requesters.find_by!(user_id: member_user1.id) } + let(:members) { [member] } + + subject { update_service.execute(member, permission: permission) } + + shared_examples 'a service returning an error' do + before do + allow(member).to receive(:save) do + member.errors.add(:user_id) + member.errors.add(:access_level) end + .and_return(false) + end + + it_behaves_like 'returns error status when params are invalid' + + it 'returns the error' do + response = subject + + expect(response[:status]).to eq(:error) + expect(response[:message]).to eq('User is invalid and Access level is invalid') + end + end + + before do + stub_feature_flags(bulk_update_membership_roles: false) + end + + it_behaves_like 'current user cannot update the given members' + it_behaves_like 'updating a project' + it_behaves_like 'updating a group' + end + + subject { update_service.execute(members, permission: permission) } + + shared_examples 'a service returning an error' do + it_behaves_like 'returns error status when params are invalid' - it_behaves_like 'a service updating a member' do - let(:access_level) { Gitlab::Access::MAINTAINER } + context 'when a member update results in invalid record' do + let(:member2) { members.second } + + before do + allow(member2).to receive(:save!) do + member2.errors.add(:user_id) + member2.errors.add(:access_level) + end.and_raise(ActiveRecord::RecordInvalid) + end + + it 'returns the error' do + response = subject + + expect(response[:status]).to eq(:error) + expect(response[:message]).to eq('User is invalid and Access level is invalid') + end + + it 'rollbacks back the entire update' do + old_access_levels = members.pluck(:access_level) + + subject + + expect(members.each(&:reset).pluck(:access_level)).to eq(old_access_levels) + end + end + end + + it_behaves_like 'current user cannot update the given members' + it_behaves_like 'updating a project' + it_behaves_like 'updating a group' + + context 'with a single member' do + let(:member) { create(:group_member, group: group) } + let(:members) { member } + + before do + group.add_owner(current_user) + end + + it 'returns the correct response' do + expect(subject[:member]).to eq(member) + end + end + + context 'when current user is an admin', :enable_admin_mode do + let_it_be(:current_user) { create(:admin) } + let_it_be(:source) { group } + + context 'when all owners are being downgraded' do + before do + member_users.each { |member_user| group.add_owner(member_user) } + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + end + + context 'when all blocked owners are being downgraded' do + before do + member_users.each do |member_user| + group.add_owner(member_user) + member_user.block end end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' end end end |