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

6 files changed, 90 insertions, 124 deletions

diff --git a/spec/services/packages/mark_package_files_for_destruction_service_spec.rb b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb
new file mode 100644
index 00000000000..a836de1f7f6
--- /dev/null
+++ b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::MarkPackageFilesForDestructionService, :aggregate_failures do
+  let(:service) { described_class.new(package_files) }
+
+  describe '#execute', :aggregate_failures do
+    subject { service.execute }
+
+    shared_examples 'executing successfully' do
+      it 'marks package files for destruction' do
+        expect { subject }
+          .to change { ::Packages::PackageFile.pending_destruction.count }.by(package_files.size)
+      end
+
+      it 'executes successfully' do
+        expect(subject).to be_success
+        expect(subject.message).to eq('Package files are now pending destruction')
+      end
+    end
+
+    context 'with no package files' do
+      let_it_be(:package_files) { ::Packages::PackageFile.none }
+
+      it_behaves_like 'executing successfully'
+    end
+
+    context 'with a single package file' do
+      let_it_be(:package_file) { create(:package_file) }
+      let_it_be(:package_files) { ::Packages::PackageFile.id_in(package_file.id) }
+
+      it_behaves_like 'executing successfully'
+    end
+
+    context 'with many package files' do
+      let_it_be(:package_files) { ::Packages::PackageFile.id_in(create_list(:package_file, 3).map(&:id)) }
+
+      it_behaves_like 'executing successfully'
+    end
+
+    context 'with an error during the update' do
+      let_it_be(:package_files) { ::Packages::PackageFile.none }
+
+      before do
+        expect(package_files).to receive(:each_batch).and_raise('error!')
+      end
+
+      it 'raises the error' do
+        expect { subject }
+          .to raise_error('error!')
+          .and not_change { ::Packages::PackageFile.pending_destruction.count }
+      end
+    end
+  end
+end
diff --git a/spec/services/packages/destroy_package_service_spec.rb b/spec/services/packages/mark_package_for_destruction_service_spec.rb
index 92db8da968c..125ec53ad61 100644
--- a/spec/services/packages/destroy_package_service_spec.rb
+++ b/spec/services/packages/mark_package_for_destruction_service_spec.rb
@@ -2,10 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe Packages::DestroyPackageService do
+RSpec.describe Packages::MarkPackageForDestructionService do
   let_it_be(:user) { create(:user) }
-
-  let!(:package) { create(:npm_package) }
+  let_it_be_with_reload(:package) { create(:npm_package) }
 
   describe '#execute' do
     subject(:service) { described_class.new(container: package, current_user: user) }
@@ -15,10 +14,11 @@ RSpec.describe Packages::DestroyPackageService do
         package.project.add_maintainer(user)
       end
 
-      context 'when the destroy is successfull' do
-        it 'destroy the package' do
+      context 'when it is successful' do
+        it 'marks the package and package files as pending destruction' do
           expect(package).to receive(:sync_maven_metadata).and_call_original
-          expect { service.execute }.to change { Packages::Package.count }.by(-1)
+          expect(package).to receive(:mark_package_files_for_destruction).and_call_original
+          expect { service.execute }.to change { package.status }.from('default').to('pending_destruction')
         end
 
         it 'returns a success ServiceResponse' do
@@ -26,13 +26,13 @@ RSpec.describe Packages::DestroyPackageService do
 
           expect(response).to be_a(ServiceResponse)
           expect(response).to be_success
-          expect(response.message).to eq("Package was successfully deleted")
+          expect(response.message).to eq("Package was successfully marked as pending destruction")
         end
       end
 
-      context 'when the destroy is not successful' do
+      context 'when it is not successful' do
         before do
-          allow(package).to receive(:destroy!).and_raise(StandardError, "test")
+          allow(package).to receive(:pending_destruction!).and_raise(StandardError, "test")
         end
 
         it 'returns an error ServiceResponse' do
@@ -41,7 +41,7 @@ RSpec.describe Packages::DestroyPackageService do
           expect(package).not_to receive(:sync_maven_metadata)
           expect(response).to be_a(ServiceResponse)
           expect(response).to be_error
-          expect(response.message).to eq("Failed to remove the package")
+          expect(response.message).to eq("Failed to mark the package as pending destruction")
           expect(response.status).to eq(:error)
         end
       end
diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb
index 0bea3edf203..3ac42d41377 100644
--- a/spec/services/protected_branches/create_service_spec.rb
+++ b/spec/services/protected_branches/create_service_spec.rb
@@ -24,38 +24,14 @@ RSpec.describe ProtectedBranches::CreateService do
       expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
     end
 
-    context 'when name has escaped HTML' do
-      let(:name) { 'feature->test' }
+    context 'when protecting a branch with a name that contains HTML tags' do
+      let(:name) { 'foo<b>bar<\b>' }
 
-      it 'creates the new protected branch matching the unescaped version' do
-        expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-        expect(project.protected_branches.last.name).to eq('feature->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:name) { '&lt;b&gt;master&lt;/b&gt;' }
-
-        it 'creates the new protected branch with sanitized name' do
-          expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-          expect(project.protected_branches.last.name).to eq('master')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+      subject(:service) { described_class.new(project, user, params) }
 
-          it 'does not create the new protected branch' do
-            expect { service.execute }.not_to change(ProtectedBranch, :count)
-          end
-        end
-      end
-
-      context 'when name contains unescaped HTML tags' do
-        let(:name) { '<b>master</b>' }
-
-        it 'creates the new protected branch with sanitized name' do
-          expect { service.execute }.to change(ProtectedBranch, :count).by(1)
-          expect(project.protected_branches.last.name).to eq('master')
-        end
+      it 'creates a new protected branch' do
+        expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+        expect(project.protected_branches.last.name).to eq(name)
       end
     end
 
diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb
index 3d9b77dcfc0..4405af35c37 100644
--- a/spec/services/protected_branches/update_service_spec.rb
+++ b/spec/services/protected_branches/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedBranches::UpdateService do
       expect(result.reload.name).to eq(params[:name])
     end
 
-    context 'when name has escaped HTML' do
-      let(:new_name) { 'feature-&gt;test' }
+    context 'when updating name of a protected branch to one that contains HTML tags' do
+      let(:new_name) { 'foo<b>bar<\b>' }
+      let(:result) { service.execute(protected_branch) }
 
-      it 'updates protected branch name with unescaped HTML' do
-        expect(result.reload.name).to eq('feature->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:new_name) { '&lt;b&gt;master&lt;/b&gt;' }
-
-        it 'updates protected branch name with sanitized name' do
-          expect(result.reload.name).to eq('master')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
-          it 'does not update the protected branch' do
-            expect(result.reload.name).to eq(protected_branch.name)
-          end
-        end
-      end
-    end
-
-    context 'when name contains unescaped HTML tags' do
-      let(:new_name) { '<b>master</b>' }
+      subject(:service) { described_class.new(project, user, params) }
 
-      it 'updates protected branch name with sanitized name' do
-        expect(result.reload.name).to eq('master')
+      it 'updates a protected branch' do
+        expect(result.reload.name).to eq(new_name)
       end
     end
 
diff --git a/spec/services/protected_tags/create_service_spec.rb b/spec/services/protected_tags/create_service_spec.rb
index 31059d17f10..a0b99b595e3 100644
--- a/spec/services/protected_tags/create_service_spec.rb
+++ b/spec/services/protected_tags/create_service_spec.rb
@@ -22,38 +22,14 @@ RSpec.describe ProtectedTags::CreateService do
       expect(project.protected_tags.last.create_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
     end
 
-    context 'when name has escaped HTML' do
-      let(:name) { 'tag-&gt;test' }
+    context 'protecting a tag with a name that contains HTML tags' do
+      let(:name) { 'foo<b>bar<\b>' }
 
-      it 'creates the new protected tag matching the unescaped version' do
-        expect { service.execute }.to change(ProtectedTag, :count).by(1)
-        expect(project.protected_tags.last.name).to eq('tag->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
-        it 'creates the new protected tag with sanitized name' do
-          expect { service.execute }.to change(ProtectedTag, :count).by(1)
-          expect(project.protected_tags.last.name).to eq('tag')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+      subject(:service) { described_class.new(project, user, params) }
 
-          it 'does not create the new protected tag' do
-            expect { service.execute }.not_to change(ProtectedTag, :count)
-          end
-        end
-      end
-
-      context 'when name contains unescaped HTML tags' do
-        let(:name) { '<b>tag</b>' }
-
-        it 'creates the new protected tag with sanitized name' do
-          expect { service.execute }.to change(ProtectedTag, :count).by(1)
-          expect(project.protected_tags.last.name).to eq('tag')
-        end
+      it 'creates a new protected tag' do
+        expect { service.execute }.to change(ProtectedTag, :count).by(1)
+        expect(project.protected_tags.last.name).to eq(name)
       end
     end
   end
diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb
index 8d301dcd825..4b6e726bb6e 100644
--- a/spec/services/protected_tags/update_service_spec.rb
+++ b/spec/services/protected_tags/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedTags::UpdateService do
       expect(result.reload.name).to eq(params[:name])
     end
 
-    context 'when name has escaped HTML' do
-      let(:new_name) { 'tag-&gt;test' }
+    context 'when updating protected tag with a name that contains HTML tags' do
+      let(:new_name) { 'foo<b>bar<\b>' }
+      let(:result) { service.execute(protected_tag) }
 
-      it 'updates protected tag name with unescaped HTML' do
-        expect(result.reload.name).to eq('tag->test')
-      end
-
-      context 'and name contains HTML tags' do
-        let(:new_name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
-        it 'updates protected tag name with sanitized name' do
-          expect(result.reload.name).to eq('tag')
-        end
-
-        context 'and contains unsafe HTML' do
-          let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
-          it 'does not update the protected tag' do
-            expect(result.reload.name).to eq(protected_tag.name)
-          end
-        end
-      end
-    end
-
-    context 'when name contains unescaped HTML tags' do
-      let(:new_name) { '<b>tag</b>' }
+      subject(:service) { described_class.new(project, user, params) }
 
-      it 'updates protected tag name with sanitized name' do
-        expect(result.reload.name).to eq('tag')
+      it 'updates a protected tag' do
+        expect(result.reload.name).to eq(new_name)
       end
     end