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

5 files changed, 136 insertions, 84 deletions

diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb
index 8e652d2f150..3a7d4d12513 100644
--- a/spec/helpers/integrations_helper_spec.rb
+++ b/spec/helpers/integrations_helper_spec.rb
@@ -98,4 +98,19 @@ RSpec.describe IntegrationsHelper do
       end
     end
   end
+
+  describe '#jira_issue_breadcrumb_link' do
+    let(:issue_reference) { nil }
+
+    subject { helper.jira_issue_breadcrumb_link(issue_reference) }
+
+    context 'when issue_reference contains HTML' do
+      let(:issue_reference) { "<script>alert('XSS')</script>" }
+
+      it 'escapes issue reference' do
+        is_expected.not_to include(issue_reference)
+        is_expected.to include(html_escape(issue_reference))
+      end
+    end
+  end
 end
diff --git a/spec/lib/banzai/filter/references/design_reference_filter_spec.rb b/spec/lib/banzai/filter/references/design_reference_filter_spec.rb
index cb1f3d520a4..d616aabea45 100644
--- a/spec/lib/banzai/filter/references/design_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/references/design_reference_filter_spec.rb
@@ -90,11 +90,8 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do
       [
         ['simple.png'],
         ['SIMPLE.PNG'],
-        ['has spaces.png'],
         ['has-hyphen.jpg'],
-        ['snake_case.svg'],
-        ['has "quotes".svg'],
-        ['has <special> characters [o].svg']
+        ['snake_case.svg']
       ]
     end
 
@@ -138,40 +135,25 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do
     end
   end
 
-  context 'a design with a quoted filename' do
-    let(:filename) { %q{A "very" good file.png} }
-    let(:design) { create(:design, :with_versions, issue: issue, filename: filename) }
-
-    it 'links to the design' do
-      expect(doc.css('a').first.attr('href'))
-        .to eq url_for_design(design)
-    end
-  end
-
   context 'internal reference' do
     it_behaves_like 'a reference containing an element node'
 
-    context 'the reference is valid' do
-      it_behaves_like 'a good link reference'
+    it_behaves_like 'a good link reference'
 
-      context 'the filename needs to be escaped' do
-        where(:filename) do
-          [
-            ['with some spaces.png'],
-            ['with <script>console.log("pwded")<%2Fscript>.png']
-          ]
-        end
+    context 'the filename contains invalid characters' do
+      where(:filename) do
+        [
+          ['with some spaces.png'],
+          ['with <script>console.log("pwded")<%2Fscript>.png'],
+          ['foo"bar.png'],
+          ['A "very" good file.png']
+        ]
+      end
 
-        with_them do
-          let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
-          let(:link) { doc.css('a').first }
+      with_them do
+        let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
 
-          it 'replaces the content with the reference, but keeps the link', :aggregate_failures do
-            expect(doc.text).to eq(CGI.unescapeHTML("Added #{design.to_reference}"))
-            expect(link.attr('title')).to eq(design.filename)
-            expect(link.attr('href')).to eq(design_url)
-          end
-        end
+        it_behaves_like 'a no-op filter'
       end
     end
 
diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb
index f2ce5e42eaf..b0601ea3f08 100644
--- a/spec/models/design_management/design_spec.rb
+++ b/spec/models/design_management/design_spec.rb
@@ -572,6 +572,12 @@ RSpec.describe DesignManagement::Design do
       expect(described_class.link_reference_pattern).not_to match(url_for_designs(issue))
     end
 
+    it 'intentionally ignores filenames with any special character' do
+      design = build(:design, issue: issue, filename: '"invalid')
+
+      expect(described_class.link_reference_pattern).not_to match(url_for_design(design))
+    end
+
     where(:ext) do
       (described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT).flat_map do |ext|
         [[ext], [ext.upcase]]
@@ -593,14 +599,6 @@ RSpec.describe DesignManagement::Design do
         )
       end
 
-      context 'the file needs to be encoded' do
-        let(:filename) { "my file.#{ext}" }
-
-        it 'extracts the encoded filename' do
-          expect(captures).to include('url_filename' => 'my%20file.' + ext)
-        end
-      end
-
       context 'the file is all upper case' do
         let(:filename) { "file.#{ext}".upcase }
 
diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb
index 677bd4c5e48..7049e64c2ce 100644
--- a/spec/models/integrations/datadog_spec.rb
+++ b/spec/models/integrations/datadog_spec.rb
@@ -127,18 +127,6 @@ RSpec.describe Integrations::Datadog do
     end
   end
 
-  describe '#api_keys_url' do
-    subject { instance.api_keys_url }
-
-    it { is_expected.to eq("https://app.#{dd_site}/account/settings#api") }
-
-    context 'with unset datadog_site' do
-      let(:dd_site) { '' }
-
-      it { is_expected.to eq("https://docs.datadoghq.com/account_management/api-app-keys/") }
-    end
-  end
-
   describe '#test' do
     subject(:result) { saved_instance.test(pipeline_data) }
 
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index c794acdf76d..5135be8fff5 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -152,56 +152,125 @@ RSpec.describe Groups::DestroyService do
     end
 
     context 'for shared groups within different hierarchies' do
-      let(:shared_with_group) { group }
-      let!(:shared_group) { create(:group, :private) }
-      let!(:shared_group_child) { create(:group, :private, parent: shared_group) }
-      let!(:shared_group_user) { create(:user) }
+      let(:group1) { create(:group, :private) }
+      let(:group2) { create(:group, :private) }
 
-      let!(:project) { create(:project, group: shared_group) }
-      let!(:project_child) { create(:project, group: shared_group_child) }
+      let(:group1_user) { create(:user) }
+      let(:group2_user) { create(:user) }
 
       before do
-        shared_group.add_user(shared_group_user, Gitlab::Access::OWNER)
-
-        create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
-        shared_with_group.refresh_members_authorized_projects
+        group1.add_user(group1_user, Gitlab::Access::OWNER)
+        group2.add_user(group2_user, Gitlab::Access::OWNER)
       end
 
-      context 'the shared group is deleted' do
-        it 'updates project authorization' do
-          expect(shared_group_user.can?(:read_project, project)).to eq(true)
-          expect(shared_group_user.can?(:read_project, project_child)).to eq(true)
+      context 'when a project is shared with a group' do
+        let!(:group1_project) { create(:project, :private, group: group1) }
+
+        before do
+          create(:project_group_link, project: group1_project, group: group2)
+        end
+
+        context 'and the shared group is deleted' do
+          it 'updates project authorizations so group2 users no longer have access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(true)
 
-          destroy_group(shared_group, shared_group_user, false)
+            destroy_group(group2, group2_user, false)
 
-          expect(shared_group_user.can?(:read_project, project)).to eq(false)
-          expect(shared_group_user.can?(:read_project, project_child)).to eq(false)
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(false)
+          end
+
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with(array_including(group2_user.id)).and_call_original
+
+            destroy_group(group2, group2_user, false)
+          end
         end
 
-        it 'does not make use of specific service to update project_authorizations records' do
-          expect(UserProjectAccessChangedService)
-            .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations).and_call_original
+        context 'and the group is shared with another group' do
+          let(:group3) { create(:group, :private) }
+          let(:group3_user) { create(:user) }
+
+          before do
+            group3.add_user(group3_user, Gitlab::Access::OWNER)
+
+            create(:group_group_link, shared_group: group2, shared_with_group: group3)
+            group3.refresh_members_authorized_projects
+          end
+
+          it 'updates project authorizations so group2 and group3 users no longer have access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group3_user.can?(:read_project, group1_project)).to eq(true)
+
+            destroy_group(group2, group2_user, false)
+
+            expect(group1_user.can?(:read_project, group1_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group1_project)).to eq(false)
+            expect(group3_user.can?(:read_project, group1_project)).to eq(false)
+          end
 
-          destroy_group(shared_group, shared_group_user, false)
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with(array_including(group2_user.id, group3_user.id)).and_call_original
+
+            destroy_group(group2, group2_user, false)
+          end
         end
       end
 
-      context 'the shared_with group is deleted' do
-        it 'updates project authorization' do
-          expect(user.can?(:read_project, project)).to eq(true)
-          expect(user.can?(:read_project, project_child)).to eq(true)
+      context 'when a group is shared with a group' do
+        let!(:group2_project) { create(:project, :private, group: group2) }
 
-          destroy_group(shared_with_group, user, false)
+        before do
+          create(:group_group_link, shared_group: group2, shared_with_group: group1)
+          group1.refresh_members_authorized_projects
+        end
+
+        context 'and the shared group is deleted' do
+          it 'updates project authorizations since the project has been deleted with the group', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+
+            destroy_group(group2, group2_user, false)
 
-          expect(user.can?(:read_project, project)).to eq(false)
-          expect(user.can?(:read_project, project_child)).to eq(false)
+            expect(group1_user.can?(:read_project, group2_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(false)
+          end
+
+          it 'does not call the service to update project authorizations' do
+            expect(UserProjectAccessChangedService).not_to receive(:new)
+
+            destroy_group(group2, group2_user, false)
+          end
         end
 
-        it 'makes use of a specific service to update project_authorizations records' do
-          expect(UserProjectAccessChangedService)
-            .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+        context 'the shared_with group is deleted' do
+          let!(:group2_subgroup) { create(:group, :private, parent: group2)}
+          let!(:group2_subgroup_project) { create(:project, :private, group: group2_subgroup) }
 
-          destroy_group(shared_with_group, user, false)
+          it 'updates project authorizations so users of both groups lose access', :aggregate_failures do
+            expect(group1_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+            expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+
+            destroy_group(group1, group1_user, false)
+
+            expect(group1_user.can?(:read_project, group2_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_project)).to eq(true)
+            expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(false)
+            expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true)
+          end
+
+          it 'calls the service to update project authorizations only with necessary user ids' do
+            expect(UserProjectAccessChangedService)
+              .to receive(:new).with([group1_user.id]).and_call_original
+
+            destroy_group(group1, group1_user, false)
+          end
         end
       end
     end