Commit 430e6f6a authored by Mario de la Ossa's avatar Mario de la Ossa Committed by Thong Kuah

Do not set fallback mirror user

- Notify owners and maintainers when mirror disabled. They should have
some notification if the mirror was disabled due to the mirror user
being deleted

- Do not send mirror disabled email if user is deleted. It is possible
to schedule a mail to a user while the user is being deleted in another
worker. So we do a check that the user exists first.
parent f5df1117
......@@ -11,6 +11,17 @@ module EE
subject: subject('Repository mirroring paused'))
end
def mirror_was_disabled_email(project_id, user_id, deleted_user_name)
@project = ::Project.find(project_id)
user = ::User.find_by_id(user_id)
@deleted_user_name = deleted_user_name
return unless user
mail(to: user.notification_email_for(@project.group),
subject: subject('Repository mirroring disabled'))
end
def project_mirror_user_changed_email(new_mirror_user_id, deleted_user_name, project_id)
@project = ::Project.find(project_id)
@deleted_user_name = deleted_user_name
......
......@@ -24,6 +24,10 @@ module EE
::Notify.mirror_was_hard_failed_email(project.id, user.id).message
end
def mirror_was_disabled_email
::Notify.mirror_was_disabled_email(project.id, user.id, 'deleted_user_name').message
end
def project_mirror_user_changed_email
::Notify.project_mirror_user_changed_email(user.id, 'deleted_user_name', project.id).message
end
......
......@@ -38,6 +38,14 @@ module EE
end
end
def mirror_was_disabled(project, deleted_user_name)
return if project.emails_disabled?
owners_and_maintainers_without_invites(project).each do |recipient|
mailer.mirror_was_disabled_email(project.id, recipient.user.id, deleted_user_name).deliver_later
end
end
def new_epic(epic)
new_resource_email(epic, :new_epic_email)
end
......
......@@ -20,10 +20,14 @@ module EE
user_mirrors = ::Project.where(mirror_user: user) # rubocop: disable CodeReuse/ActiveRecord
user_mirrors.find_each do |mirror|
new_mirror_user = first_mirror_owner(user, mirror)
mirror.update(mirror_user: new_mirror_user)
::NotificationService.new.project_mirror_user_changed(new_mirror_user, user.name, mirror)
mirror.update(mirror: false, mirror_user: nil)
::Gitlab::ErrorTracking.track_exception(
RuntimeError.new('Disabled mirroring'),
user_id: user.id,
project_id: mirror.id
)
::NotificationService.new.mirror_was_disabled(mirror, user.name)
end
end
......
%p
Repository mirroring on #{@project.full_path} was disabled because the mirror user #{sanitize_name(@deleted_user_name)} was deleted.
%p
To re-enable mirroring, update your #{link_to("repository mirroring settings", project_settings_repository_url(@project))}.
Repository mirroring on <%= @project.full_path %> was disabled because the mirror user <%= sanitize_name(@deleted_user_name) %> was deleted.
To re-enable mirroring, update your repository settings at <%= project_settings_repository_url(@project) %>.
---
title: Do not set fallback mirror user
merge_request:
author:
type: security
......@@ -372,6 +372,33 @@ RSpec.describe Notify do
end
end
describe 'mirror was disabled' do
let(:project) { create(:project) }
subject { described_class.mirror_was_disabled_email(project.id, user.id, 'deleted_user_name') }
it_behaves_like 'an email sent from GitLab'
it_behaves_like 'it should not have Gmail Actions links'
it_behaves_like "a user cannot unsubscribe through footer link"
it 'has the correct subject and body' do
is_expected.to have_subject("#{project.name} | Repository mirroring disabled")
is_expected.to have_body_text(project.full_path)
is_expected.to have_body_text(project_settings_repository_url(project))
is_expected.to have_body_text('deleted_user_name')
end
context 'user was deleted' do
before do
user.destroy!
end
it 'does not send email' do
expect(subject.message).to be_a_kind_of ActionMailer::Base::NullMail
end
end
end
describe 'mirror user changed' do
let(:mirror_user) { create(:user) }
let(:project) { create(:project, :mirror, mirror_user_id: mirror_user.id) }
......
......@@ -307,6 +307,125 @@ RSpec.describe EE::NotificationService, :mailer do
end
end
describe 'mirror was disabled' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let(:deleted_username) { 'deleted_user_name' }
context 'when the project has invited members' do
let!(:project_member) { create(:project_member, :invited, project: project) }
it 'sends email' do
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.owner.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
it_behaves_like 'project emails are disabled' do
let(:notification_target) { project }
let(:notification_trigger) { subject.mirror_was_disabled(project, deleted_username) }
around do |example|
perform_enqueued_jobs { example.run }
end
end
end
context 'when user is owner' do
it 'sends email' do
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.owner.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
it_behaves_like 'project emails are disabled' do
let(:notification_target) { project }
let(:notification_trigger) { subject.mirror_was_disabled(project, deleted_username) }
around do |example|
perform_enqueued_jobs { example.run }
end
end
context 'when owner is blocked' do
it 'does not send email' do
project.owner.block!
expect(Notify).not_to receive(:mirror_was_disabled_email)
subject.mirror_was_disabled(project, deleted_username)
end
context 'when project belongs to group' do
it 'does not send email to the blocked owner' do
blocked_user = create(:user, :blocked)
group = create(:group, :public)
group.add_owner(blocked_user)
group.add_owner(user)
project = create(:project, namespace: group)
expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, blocked_user.id, deleted_username).and_call_original
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
end
end
end
context 'when user is maintainer' do
it 'sends email' do
project.add_maintainer(user)
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.owner.id, deleted_username).and_call_original
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
end
context 'when user is not owner nor maintainer' do
it 'does not send email' do
project.add_developer(user)
expect(Notify).not_to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, project.creator.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
context 'when user is group owner' do
it 'sends email' do
group = create(:group, :public) do |group|
group.add_owner(user)
end
project = create(:project, namespace: group)
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
end
context 'when user is group maintainer' do
it 'sends email' do
group = create(:group, :public) do |group|
group.add_maintainer(user)
end
project = create(:project, namespace: group)
expect(Notify).to receive(:mirror_was_disabled_email).with(project.id, user.id, deleted_username).and_call_original
subject.mirror_was_disabled(project, deleted_username)
end
end
end
end
context 'mirror user changed' do
let(:mirror_user) { create(:user) }
let(:project) { create(:project, :mirror, mirror_user_id: mirror_user.id) }
......
......@@ -21,15 +21,17 @@ RSpec.describe Users::DestroyService do
context 'when project is a mirror' do
let(:project) { create(:project, :mirror, mirror_user_id: user.id) }
it 'assigns mirror_user to a project owner' do
new_mirror_user = project.team.owners.first
it 'disables mirror and does not assign a new mirror_user' do
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
expect_any_instance_of(EE::NotificationService)
.to receive(:project_mirror_user_changed)
.with(new_mirror_user, user.name, project)
allow_next_instance_of(::NotificationService) do |notification|
expect(notification).to receive(:mirror_was_disabled)
.with(project, user.name)
.and_call_original
end
expect { operation }.to change { project.reload.mirror_user }
.from(user).to(new_mirror_user)
expect { operation }.to change { project.reload.mirror_user }.from(user).to(nil)
.and change { project.reload.mirror }.from(true).to(false)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment