Commit 9c921da0 authored by Imre Farkas's avatar Imre Farkas

Merge branch '33257-prevent-accidental-deletions-via-soft-delete-for-groups-services' into 'master'

Resolve "Prevent accidental deletions via soft delete for groups" - services  (MR: 3/n)

See merge request gitlab-org/gitlab!19358
parents dd7dec17 67ea490e
---
title: Add services for 'soft-delete for groups' feature
merge_request: 19358
author:
type: added
...@@ -243,6 +243,12 @@ module EE ...@@ -243,6 +243,12 @@ module EE
feature_available?(:epics) feature_available?(:epics)
end end
def marked_for_deletion?
return false unless feature_available?(:adjourned_deletion_for_projects_and_groups)
marked_for_deletion_on.present?
end
private private
def custom_project_templates_group_allowed def custom_project_templates_group_allowed
......
...@@ -3,4 +3,6 @@ ...@@ -3,4 +3,6 @@
class GroupDeletionSchedule < ApplicationRecord class GroupDeletionSchedule < ApplicationRecord
belongs_to :group belongs_to :group
belongs_to :deleting_user, foreign_key: 'user_id', class_name: 'User' belongs_to :deleting_user, foreign_key: 'user_id', class_name: 'User'
validates :marked_for_deletion_on, presence: true
end end
...@@ -41,6 +41,7 @@ class License < ApplicationRecord ...@@ -41,6 +41,7 @@ class License < ApplicationRecord
].freeze ].freeze
EEP_FEATURES = EES_FEATURES + %i[ EEP_FEATURES = EES_FEATURES + %i[
adjourned_deletion_for_projects_and_groups
admin_audit_log admin_audit_log
auditor_user auditor_user
batch_comments batch_comments
......
# frozen_string_literal: true
module Groups
class MarkForDeletionService < Groups::BaseService
def execute
return error(_('You are not authorized to perform this action')) unless can?(current_user, :admin_group, group)
return error(_('Group has been already marked for deletion')) if group.marked_for_deletion?
result = create_deletion_schedule
log_audit_event if result[:status] == :success
result
end
private
def create_deletion_schedule
deletion_schedule = group.build_deletion_schedule(deletion_schedule_params)
if deletion_schedule.save
success
else
errors = deletion_schedule.errors.full_messages.to_sentence
error(errors)
end
end
def deletion_schedule_params
{ marked_for_deletion_on: Time.now.utc, deleting_user: current_user }
end
def log_audit_event
EE::AuditEvents::CustomAuditEventService.new(
current_user,
group,
nil,
'Group marked for deletion'
).for_group.security_event
end
end
end
# frozen_string_literal: true
module Groups
class RestoreService < Groups::BaseService
def execute
return error(_('You are not authorized to perform this action')) unless can?(current_user, :admin_group, group)
return error(_('Group has not been marked for deletion')) unless group.marked_for_deletion?
result = remove_deletion_schedule
group.reset
log_audit_event if result[:status] == :success
result
end
private
def remove_deletion_schedule
deletion_schedule = group.deletion_schedule
if deletion_schedule.destroy
success
else
error(_('Could not restore the group'))
end
end
def log_audit_event
EE::AuditEvents::CustomAuditEventService.new(
current_user,
group,
nil,
'Group restored'
).for_group.security_event
end
end
end
...@@ -7,4 +7,8 @@ describe GroupDeletionSchedule do ...@@ -7,4 +7,8 @@ describe GroupDeletionSchedule do
it { is_expected.to belong_to :group } it { is_expected.to belong_to :group }
it { is_expected.to belong_to(:deleting_user).class_name('User').with_foreign_key('user_id') } it { is_expected.to belong_to(:deleting_user).class_name('User').with_foreign_key('user_id') }
end end
describe 'Validations' do
it { is_expected.to validate_presence_of(:marked_for_deletion_on) }
end
end end
...@@ -541,4 +541,52 @@ describe Group do ...@@ -541,4 +541,52 @@ describe Group do
end end
end end
end end
describe '#marked_for_deletion?' do
subject { group.marked_for_deletion? }
shared_examples_for 'returns false' do
it { is_expected.to be_falsey }
end
shared_examples_for 'returns true' do
it { is_expected.to be_truthy }
end
context 'adjourned deletion feature is available' do
before do
stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
end
context 'when the group is marked for adjourned deletion' do
before do
create(:group_deletion_schedule, group: group, marked_for_deletion_on: 1.day.ago)
end
it_behaves_like 'returns true'
end
context 'when the group is not marked for adjourned deletion' do
it_behaves_like 'returns false'
end
end
context 'adjourned deletion feature is not available' do
before do
stub_licensed_features(adjourned_deletion_for_projects_and_groups: false)
end
context 'when the group is marked for adjourned deletion' do
before do
create(:group_deletion_schedule, group: group, marked_for_deletion_on: 1.day.ago)
end
it_behaves_like 'returns false'
end
context 'when the group is not marked for adjourned deletion' do
it_behaves_like 'returns false'
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe Groups::MarkForDeletionService do
let(:user) { create(:user) }
let(:group) { create(:group) }
subject { described_class.new(group, user, {}).execute }
before do
stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
end
context 'marking the group for deletion' do
context 'with user that can admin the group' do
before do
group.add_owner(user)
end
context 'for a group that has not been marked for deletion' do
it 'marks the group for deletion' do
subject
expect(group.marked_for_deletion_on).to eq(Date.today)
expect(group.deleting_user).to eq(user)
end
it 'returns success' do
expect(subject).to eq({ status: :success })
end
context 'marking for deletion fails' do
before do
expect_next_instance_of(GroupDeletionSchedule) do |group_deletion_schedule|
allow(group_deletion_schedule).to receive_message_chain(:errors, :full_messages)
.and_return(['error message'])
allow(group_deletion_schedule).to receive(:save).and_return(false)
end
end
it 'returns error' do
expect(subject).to eq({ status: :error, message: 'error message' })
end
end
end
context 'for a group that has been marked for deletion' do
let(:deletion_date) { 3.days.ago }
let(:group) do
create(:group_with_deletion_schedule,
marked_for_deletion_on: deletion_date,
deleting_user: user)
end
it 'does not change the attributes associated with adjourned deletion' do
subject
expect(group.marked_for_deletion_on).to eq(deletion_date.to_date)
expect(group.deleting_user).to eq(user)
end
it 'returns error' do
expect(subject).to eq({ status: :error, message: 'Group has been already marked for deletion' })
end
end
context 'audit events' do
it 'logs audit event' do
expect { subject }.to change { AuditEvent.count }.by(1)
end
end
end
context 'with a user that cannot admin the group' do
it 'does not mark the group for deletion' do
subject
expect(group.marked_for_deletion?).to be_falsey
end
it 'returns error' do
expect(subject).to eq({ status: :error, message: 'You are not authorized to perform this action' })
end
context 'audit events' do
it 'does not log audit event' do
expect { subject }.not_to change { AuditEvent.count }
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Groups::RestoreService do
let(:user) { create(:user) }
let(:group) do
create(:group_with_deletion_schedule,
marked_for_deletion_on: 1.day.ago,
deleting_user: user)
end
subject { described_class.new(group, user, {}).execute }
before do
stub_licensed_features(adjourned_deletion_for_projects_and_groups: true)
end
context 'restoring the group' do
context 'with a user that can admin the group' do
before do
group.add_owner(user)
end
context 'for a group that has been marked for deletion' do
it 'removes the mark for deletion' do
subject
expect(group.marked_for_deletion_on).to be_nil
expect(group.deleting_user).to be_nil
end
it 'returns success' do
result = subject
expect(result).to eq({ status: :success })
end
context 'restoring fails' do
it 'returns error' do
allow(group.deletion_schedule).to receive(:destroy).and_return(false)
result = subject
expect(result).to eq({ status: :error, message: 'Could not restore the group' })
end
end
end
context 'for a group that has not been marked for deletion' do
let(:group) { create(:group) }
it 'does not change the attributes associated with adjourned deletion' do
subject
expect(group.marked_for_deletion_on).to be_nil
expect(group.deleting_user).to be_nil
end
it 'returns error' do
result = subject
expect(result).to eq({ status: :error, message: 'Group has not been marked for deletion' })
end
end
context 'audit events' do
it 'logs audit event' do
expect { subject }.to change { AuditEvent.count }.by(1)
end
end
end
context 'with a user that cannot admin the group' do
it 'does not restore the group' do
subject
expect(group.marked_for_deletion?).to be_truthy
end
it 'returns error' do
result = subject
expect(result).to eq({ status: :error, message: 'You are not authorized to perform this action' })
end
context 'audit events' do
it 'does not log audit event' do
expect { subject }.not_to change { AuditEvent.count }
end
end
end
end
end
...@@ -4866,6 +4866,9 @@ msgstr "" ...@@ -4866,6 +4866,9 @@ msgstr ""
msgid "Could not remove the trigger." msgid "Could not remove the trigger."
msgstr "" msgstr ""
msgid "Could not restore the group"
msgstr ""
msgid "Could not revoke impersonation token %{token_name}." msgid "Could not revoke impersonation token %{token_name}."
msgstr "" msgstr ""
...@@ -8681,6 +8684,12 @@ msgstr "" ...@@ -8681,6 +8684,12 @@ msgstr ""
msgid "Group details" msgid "Group details"
msgstr "" msgstr ""
msgid "Group has been already marked for deletion"
msgstr ""
msgid "Group has not been marked for deletion"
msgstr ""
msgid "Group info:" msgid "Group info:"
msgstr "" msgstr ""
...@@ -19985,6 +19994,9 @@ msgstr "" ...@@ -19985,6 +19994,9 @@ msgstr ""
msgid "You are not allowed to unlink your primary login account" msgid "You are not allowed to unlink your primary login account"
msgstr "" msgstr ""
msgid "You are not authorized to perform this action"
msgstr ""
msgid "You are now impersonating %{username}" msgid "You are now impersonating %{username}"
msgstr "" msgstr ""
......
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