Skip to content
Snippets Groups Projects
Commit 45f94ea7 authored by Stan Hu's avatar Stan Hu
Browse files

Prevent project team from being truncated too early during project destruction

There are two issues with truncating the project team early:

1. `Projects::UnlinkForkService` may not close merge requests properly since
   permissions may be revoked early.

2. If an error is encountered during flushing of caches, then the user will
   lose all privileges, possibly causing an issue on deletion on retry.
parent 12cd4c83
No related branches found
No related tags found
1 merge request!9361Prevent project team from being truncated too early during project destruction
Pipeline #
Loading
@@ -17,8 +17,6 @@ module Projects
Loading
@@ -17,8 +17,6 @@ module Projects
def execute def execute
return false unless can?(current_user, :remove_project, project) return false unless can?(current_user, :remove_project, project)
   
project.team.truncate
repo_path = project.path_with_namespace repo_path = project.path_with_namespace
wiki_path = repo_path + '.wiki' wiki_path = repo_path + '.wiki'
   
Loading
@@ -30,6 +28,7 @@ module Projects
Loading
@@ -30,6 +28,7 @@ module Projects
Projects::UnlinkForkService.new(project, current_user).execute Projects::UnlinkForkService.new(project, current_user).execute
   
Project.transaction do Project.transaction do
project.team.truncate
project.destroy! project.destroy!
   
unless remove_registry_tags unless remove_registry_tags
Loading
Loading
Loading
@@ -50,6 +50,25 @@ describe Projects::DestroyService, services: true do
Loading
@@ -50,6 +50,25 @@ describe Projects::DestroyService, services: true do
it { expect(Dir.exist?(remove_path)).to be_truthy } it { expect(Dir.exist?(remove_path)).to be_truthy }
end end
   
context 'when flushing caches fail' do
before do
new_user = create(:user)
project.team.add_user(new_user, Gitlab::Access::DEVELOPER)
allow_any_instance_of(Projects::DestroyService).to receive(:flush_caches).and_raise(Redis::CannotConnectError)
end
it 'keeps project team intact upon an error' do
Sidekiq::Testing.inline! do
begin
destroy_project(project, user, {})
rescue Redis::CannotConnectError
end
end
expect(project.team.members.count).to eq 1
end
end
context 'with async_execute' do context 'with async_execute' do
let(:async) { true } let(:async) { true }
   
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment