Add latest changes from gitlab-org/security/gitlab@14-7-stable-ee

5 files changed, 37 insertions, 35 deletions

diff --git a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
index 7be629f8f4b..cd25aba9e00 100644
--- a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
+++ b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
@@ -24,19 +24,16 @@ RSpec.describe 'Destroying a package file' do
   let(:mutation_response) { graphql_mutation_response(:destroyPackageFile) }
 
   shared_examples 'destroying the package file' do
-    it 'destroy the package file' do
-      expect { mutation_request }.to change { ::Packages::PackageFile.count }.by(-1)
+    it 'marks the package file as pending destruction' do
+      expect { mutation_request }.to change { ::Packages::PackageFile.pending_destruction.count }.by(1)
     end
 
     it_behaves_like 'returning response status', :success
   end
 
   shared_examples 'denying the mutation request' do
-    it 'does not destroy the package file' do
-      expect(::Packages::PackageFile)
-          .not_to receive(:destroy)
-
-      expect { mutation_request }.not_to change { ::Packages::PackageFile.count }
+    it 'does not mark the package file as pending destruction' do
+      expect { mutation_request }.not_to change { ::Packages::PackageFile.pending_destruction.count }
 
       expect(mutation_response).to be_nil
     end
@@ -71,7 +68,7 @@ RSpec.describe 'Destroying a package file' do
       it_behaves_like 'denying the mutation request'
     end
 
-    context 'when an error occures' do
+    context 'when an error occurs' do
       let(:error_messages) { ['some error'] }
 
       before do
@@ -80,7 +77,7 @@ RSpec.describe 'Destroying a package file' do
 
       it 'returns the errors in the response' do
         allow_next_found_instance_of(::Packages::PackageFile) do |package_file|
-          allow(package_file).to receive(:destroy).and_return(false)
+          allow(package_file).to receive(:update).with(status: :pending_destruction).and_return(false)
           allow(package_file).to receive_message_chain(:errors, :full_messages).and_return(error_messages)
         end
 
diff --git a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
index e5ced419ecf..2340a6a36d8 100644
--- a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
+++ b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
@@ -24,22 +24,27 @@ RSpec.describe 'Destroying a package' do
   let(:mutation_response) { graphql_mutation_response(:destroyPackage) }
 
   shared_examples 'destroying the package' do
-    it 'destroy the package' do
-      expect(::Packages::DestroyPackageService)
+    it 'marks the package as pending destruction' do
+      expect(::Packages::MarkPackageForDestructionService)
           .to receive(:new).with(container: package, current_user: user).and_call_original
+      expect_next_found_instance_of(::Packages::Package) do |package|
+        expect(package).to receive(:mark_package_files_for_destruction)
+      end
 
-      expect { mutation_request }.to change { ::Packages::Package.count }.by(-1)
+      expect { mutation_request }
+        .to change { ::Packages::Package.pending_destruction.count }.by(1)
     end
 
     it_behaves_like 'returning response status', :success
   end
 
   shared_examples 'denying the mutation request' do
-    it 'does not destroy the package' do
-      expect(::Packages::DestroyPackageService)
+    it 'does not mark the package as pending destruction' do
+      expect(::Packages::MarkPackageForDestructionService)
           .not_to receive(:new).with(container: package, current_user: user)
 
-      expect { mutation_request }.not_to change { ::Packages::Package.count }
+      expect { mutation_request }
+        .to not_change { ::Packages::Package.pending_destruction.count }
 
       expect(mutation_response).to be_nil
     end
@@ -81,12 +86,12 @@ RSpec.describe 'Destroying a package' do
 
       it 'returns the errors in the response' do
         allow_next_found_instance_of(::Packages::Package) do |package|
-          allow(package).to receive(:destroy!).and_raise(StandardError)
+          allow(package).to receive(:pending_destruction!).and_raise(StandardError)
         end
 
         mutation_request
 
-        expect(mutation_response['errors']).to eq(['Failed to remove the package'])
+        expect(mutation_response['errors']).to match_array(['Failed to mark the package as pending destruction'])
       end
     end
   end
diff --git a/spec/requests/api/graphql/project/cluster_agents_spec.rb b/spec/requests/api/graphql/project/cluster_agents_spec.rb
index 585126f3849..c9900fea277 100644
--- a/spec/requests/api/graphql/project/cluster_agents_spec.rb
+++ b/spec/requests/api/graphql/project/cluster_agents_spec.rb
@@ -126,7 +126,7 @@ RSpec.describe 'Project.cluster_agents' do
       })
     end
 
-    it 'preloads associations to prevent N+1 queries' do
+    it 'preloads associations to prevent N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350868' do
       user = create(:user)
       token = create(:cluster_agent_token, agent: agents.second)
       create(:agent_activity_event, agent: agents.second, agent_token: token, user: user)
diff --git a/spec/requests/api/package_files_spec.rb b/spec/requests/api/package_files_spec.rb
index 7a6b1599154..a7e6a97fd0e 100644
--- a/spec/requests/api/package_files_spec.rb
+++ b/spec/requests/api/package_files_spec.rb
@@ -114,14 +114,14 @@ RSpec.describe API::PackageFiles do
         let(:user) { nil }
 
         it 'returns 403 for non authenticated user', :aggregate_failures do
-          expect { api_request }.not_to change { package.package_files.count }
+          expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
       end
 
       it 'returns 403 for a user without access to the project', :aggregate_failures do
-        expect { api_request }.not_to change { package.package_files.count }
+        expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -131,7 +131,7 @@ RSpec.describe API::PackageFiles do
       let_it_be_with_refind(:project) { create(:project, :private) }
 
       it 'returns 404 for a user without access to the project', :aggregate_failures do
-        expect { api_request }.not_to change { package.package_files.count }
+        expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -139,7 +139,7 @@ RSpec.describe API::PackageFiles do
       it 'returns 403 for a user without enough permissions', :aggregate_failures do
         project.add_developer(user)
 
-        expect { api_request }.not_to change { package.package_files.count }
+        expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -147,7 +147,7 @@ RSpec.describe API::PackageFiles do
       it 'returns 204', :aggregate_failures do
         project.add_maintainer(user)
 
-        expect { api_request }.to change { package.package_files.count }.by(-1)
+        expect { api_request }.to change { package.package_files.pending_destruction.count }.by(1)
 
         expect(response).to have_gitlab_http_status(:no_content)
       end
@@ -156,7 +156,7 @@ RSpec.describe API::PackageFiles do
         let(:user) { nil }
 
         it 'returns 404 for non authenticated user', :aggregate_failures do
-          expect { api_request }.not_to change { package.package_files.count }
+          expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -168,7 +168,7 @@ RSpec.describe API::PackageFiles do
         it 'returns 404 when the package file does not exist', :aggregate_failures do
           project.add_maintainer(user)
 
-          expect { api_request }.not_to change { package.package_files.count }
+          expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -182,7 +182,7 @@ RSpec.describe API::PackageFiles do
         end
 
         it 'can not be accessed', :aggregate_failures do
-          expect { api_request }.not_to change { package.package_files.count }
+          expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -193,7 +193,7 @@ RSpec.describe API::PackageFiles do
           end
 
           it 'can be accessed', :aggregate_failures do
-            expect { api_request }.to change { package.package_files.count }.by(-1)
+            expect { api_request }.not_to change { package.package_files.pending_destruction.count }
 
             expect(response).to have_gitlab_http_status(:no_content)
           end
diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb
index 9b7538547f6..5f4b8899a33 100644
--- a/spec/requests/api/project_packages_spec.rb
+++ b/spec/requests/api/project_packages_spec.rb
@@ -293,13 +293,13 @@ RSpec.describe API::ProjectPackages do
     context 'without the need for a license' do
       context 'project is public' do
         it 'returns 403 for non authenticated user' do
-          delete api(package_url)
+          expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
 
         it 'returns 403 for a user without access to the project' do
-          delete api(package_url, user)
+          expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
@@ -313,13 +313,13 @@ RSpec.describe API::ProjectPackages do
         end
 
         it 'returns 404 for non authenticated user' do
-          delete api(package_url)
+          expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
 
         it 'returns 404 for a user without access to the project' do
-          delete api(package_url, user)
+          expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -327,7 +327,7 @@ RSpec.describe API::ProjectPackages do
         it 'returns 404 when the package does not exist' do
           project.add_maintainer(user)
 
-          delete api(no_package_url, user)
+          expect { delete api(no_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -335,7 +335,7 @@ RSpec.describe API::ProjectPackages do
         it 'returns 404 for the package from a different project' do
           project.add_maintainer(user)
 
-          delete api(wrong_package_url, user)
+          expect { delete api(wrong_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
@@ -343,7 +343,7 @@ RSpec.describe API::ProjectPackages do
         it 'returns 403 for a user without enough permissions' do
           project.add_developer(user)
 
-          delete api(package_url, user)
+          expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
@@ -351,7 +351,7 @@ RSpec.describe API::ProjectPackages do
         it 'returns 204' do
           project.add_maintainer(user)
 
-          delete api(package_url, user)
+          expect { delete api(package_url, user) }.to change { ::Packages::Package.pending_destruction.count }.by(1)
 
           expect(response).to have_gitlab_http_status(:no_content)
         end