Add latest changes from gitlab-org/security/gitlab@13-1-stable-ee

11 files changed, 158 insertions, 21 deletions

diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb
index aaaa61fec62..55031183e10 100644
--- a/spec/features/snippets/notes_on_personal_snippets_spec.rb
+++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb
@@ -5,15 +5,17 @@ require 'spec_helper'
 RSpec.describe 'Comments on personal snippets', :js do
   include NoteInteractionHelpers
 
-  let!(:user)    { create(:user) }
-  let!(:snippet) { create(:personal_snippet, :public) }
+  let_it_be(:snippet) { create(:personal_snippet, :public) }
+  let_it_be(:other_note) { create(:note_on_personal_snippet) }
+
+  let(:user_name) { 'Test User' }
+  let!(:user) { create(:user, name: user_name) }
   let!(:snippet_notes) do
     [
       create(:note_on_personal_snippet, noteable: snippet, author: user),
       create(:note_on_personal_snippet, noteable: snippet)
     ]
   end
-  let!(:other_note) { create(:note_on_personal_snippet) }
 
   before do
     stub_feature_flags(snippets_vue: false)
@@ -56,6 +58,26 @@ RSpec.describe 'Comments on personal snippets', :js do
         expect(page).to show_user_status(status)
       end
     end
+
+    it 'shows the author name' do
+      visit snippet_path(snippet)
+
+      within("#note_#{snippet_notes[0].id}") do
+        expect(page).to have_content(user_name)
+      end
+    end
+
+    context 'when the author name contains HTML' do
+      let(:user_name) { '<h1><a href="https://bad.link/malicious.exe" class="evil">Fake Content<img class="fake-icon" src="image.png"></a></h1>' }
+
+      it 'renders the name as plain text' do
+        visit snippet_path(snippet)
+
+        content = find("#note_#{snippet_notes[0].id} .note-header-author-name").text
+
+        expect(content).to eq user_name
+      end
+    end
   end
 
   context 'when submitting a note' do
diff --git a/spec/frontend/error_tracking/components/stacktrace_entry_spec.js b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js
index 2a4e826b4ab..de746b8ac84 100644
--- a/spec/frontend/error_tracking/components/stacktrace_entry_spec.js
+++ b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js
@@ -1,8 +1,10 @@
 import { shallowMount } from '@vue/test-utils';
+import { GlSprintf } from '@gitlab/ui';
 import StackTraceEntry from '~/error_tracking/components/stacktrace_entry.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import Icon from '~/vue_shared/components/icon.vue';
+import { trimText } from 'helpers/text_helper';
 
 describe('Stacktrace Entry', () => {
   let wrapper;
@@ -21,6 +23,9 @@ describe('Stacktrace Entry', () => {
         errorLine: 24,
         ...props,
       },
+      stubs: {
+        GlSprintf,
+      },
     });
   }
 
@@ -53,7 +58,7 @@ describe('Stacktrace Entry', () => {
       const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: 77 };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
       expect(wrapper.find(Icon).exists()).toBe(false);
-      expect(findFileHeaderContent()).toContain(
+      expect(trimText(findFileHeaderContent())).toContain(
         `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}:${extraInfo.errorColumn}`,
       );
     });
@@ -61,17 +66,17 @@ describe('Stacktrace Entry', () => {
     it('should render only lineNo:columnNO when there is no errorFn ', () => {
       const extraInfo = { errorLine: 34, errorFn: null, errorColumn: 77 };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
-      expect(findFileHeaderContent()).not.toContain(`in ${extraInfo.errorFn}`);
-      expect(findFileHeaderContent()).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`);
+      const fileHeaderContent = trimText(findFileHeaderContent());
+      expect(fileHeaderContent).not.toContain(`in ${extraInfo.errorFn}`);
+      expect(fileHeaderContent).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`);
     });
 
     it('should render only lineNo when there is no errorColumn ', () => {
       const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: null };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
-      expect(findFileHeaderContent()).toContain(
-        `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`,
-      );
-      expect(findFileHeaderContent()).not.toContain(`:${extraInfo.errorColumn}`);
+      const fileHeaderContent = trimText(findFileHeaderContent());
+      expect(fileHeaderContent).toContain(`in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`);
+      expect(fileHeaderContent).not.toContain(`:${extraInfo.errorColumn}`);
     });
   });
 });
diff --git a/spec/frontend/issuables_list/components/issuable_spec.js b/spec/frontend/issuables_list/components/issuable_spec.js
index 980def06078..834d18246a5 100644
--- a/spec/frontend/issuables_list/components/issuable_spec.js
+++ b/spec/frontend/issuables_list/components/issuable_spec.js
@@ -1,5 +1,5 @@
 import { shallowMount } from '@vue/test-utils';
-import { GlLink } from '@gitlab/ui';
+import { GlSprintf } from '@gitlab/ui';
 import { TEST_HOST } from 'helpers/test_constants';
 import { trimText } from 'helpers/text_helper';
 import initUserPopovers from '~/user_popovers';
@@ -44,6 +44,10 @@ describe('Issuable component', () => {
         baseUrl: TEST_BASE_URL,
         ...props,
       },
+      stubs: {
+        'gl-sprintf': GlSprintf,
+        'gl-link': '<a><slot></slot></a>',
+      },
     });
   };
 
@@ -66,12 +70,12 @@ describe('Issuable component', () => {
 
   const findConfidentialIcon = () => wrapper.find('.fa-eye-slash');
   const findTaskStatus = () => wrapper.find('.task-status');
-  const findOpenedAgoContainer = () => wrapper.find({ ref: 'openedAgoByContainer' });
+  const findOpenedAgoContainer = () => wrapper.find('[data-testid="openedByMessage"]');
   const findMilestone = () => wrapper.find('.js-milestone');
   const findMilestoneTooltip = () => findMilestone().attributes('title');
   const findDueDate = () => wrapper.find('.js-due-date');
   const findLabelContainer = () => wrapper.find('.js-labels');
-  const findLabelLinks = () => findLabelContainer().findAll(GlLink);
+  const findLabelLinks = () => findLabelContainer().findAll('a');
   const findWeight = () => wrapper.find('.js-weight');
   const findAssignees = () => wrapper.find(IssueAssignees);
   const findMergeRequestsCount = () => wrapper.find('.js-merge-requests');
@@ -86,7 +90,7 @@ describe('Issuable component', () => {
 
       factory();
 
-      expect(initUserPopovers).toHaveBeenCalledWith([findOpenedAgoContainer().find('a').element]);
+      expect(initUserPopovers).toHaveBeenCalledWith([wrapper.vm.$refs.openedAgoByContainer.$el]);
     });
   });
 
@@ -135,7 +139,7 @@ describe('Issuable component', () => {
     });
 
     it('renders fuzzy opened date and author', () => {
-      expect(trimText(findOpenedAgoContainer().text())).toEqual(
+      expect(trimText(findOpenedAgoContainer().text())).toContain(
         `opened 1 month ago by ${TEST_USER_NAME}`,
       );
     });
diff --git a/spec/initializers/cookies_serializer_spec.rb b/spec/initializers/cookies_serializer_spec.rb
new file mode 100644
index 00000000000..6167039d3c2
--- /dev/null
+++ b/spec/initializers/cookies_serializer_spec.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Cookies serializer initializer' do
+  def load_initializer
+    load Rails.root.join('config/initializers/cookies_serializer.rb')
+  end
+
+  subject { Rails.application.config.action_dispatch.cookies_serializer }
+
+  it 'uses JSON serializer by default' do
+    load_initializer
+
+    expect(subject).to eq(:json)
+  end
+
+  it 'uses the unsafe hybrid serializer when the environment variables is set' do
+    stub_env('USE_UNSAFE_HYBRID_COOKIES', 'true')
+
+    load_initializer
+
+    expect(subject).to eq(:hybrid)
+  end
+end
diff --git a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb
index 798112d0f53..6890a70518b 100644
--- a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb
@@ -20,6 +20,18 @@ describe Banzai::Filter::AbstractReferenceFilter do
     end
   end
 
+  describe '#data_attributes_for' do
+    let_it_be(:issue) { create(:issue, project: project) }
+
+    it 'is not an XSS vector' do
+      allow(described_class).to receive(:object_class).and_return(Issue)
+
+      data_attributes = filter.data_attributes_for('xss &lt;img onerror=alert(1) src=x&gt;', project, issue, link_content: true)
+
+      expect(data_attributes[:original]).to eq('xss &amp;lt;img onerror=alert(1) src=x&amp;gt;')
+    end
+  end
+
   describe '#parent_per_reference' do
     it 'returns a Hash containing projects grouped per parent paths' do
       expect(filter).to receive(:references_per_parent)
diff --git a/spec/lib/banzai/filter/upload_link_filter_spec.rb b/spec/lib/banzai/filter/upload_link_filter_spec.rb
index c366f774895..8844ad78306 100644
--- a/spec/lib/banzai/filter/upload_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/upload_link_filter_spec.rb
@@ -229,6 +229,7 @@ describe Banzai::Filter::UploadLinkFilter do
       'invalid UTF-8 byte sequences' | '%FF'
       'garbled path'                 | 'open(/var/tmp/):%20/location%0Afrom:%20/test'
       'whitespace'                   | "d18213acd3732630991986120e167e3d/Landscape_8.jpg\nand more"
+      'null byte'                    | "%00"
     end
 
     with_them do
diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb
index 4fa39da3eb4..b4047e369fb 100644
--- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb
@@ -24,7 +24,7 @@ describe Banzai::Pipeline::FullPipeline do
     it 'escapes the data-original attribute on a reference' do
       markdown = %Q{[">bad things](#{issue.to_reference})}
       result = described_class.to_html(markdown, project: project)
-      expect(result).to include(%{data-original='\"&gt;bad things'})
+      expect(result).to include(%{data-original='\"&amp;gt;bad things'})
     end
   end
 
diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb
index 499c334d491..2b86d59fbba 100644
--- a/spec/requests/api/deploy_tokens_spec.rb
+++ b/spec/requests/api/deploy_tokens_spec.rb
@@ -204,7 +204,7 @@ describe API::DeployTokens do
   end
 
   context 'deploy token creation' do
-    shared_examples 'creating a deploy token' do |entity, unauthenticated_response|
+    shared_examples 'creating a deploy token' do |entity, unauthenticated_response, authorized_role|
       let(:expires_time) { 1.year.from_now }
       let(:params) do
         {
@@ -231,9 +231,9 @@ describe API::DeployTokens do
         it { is_expected.to have_gitlab_http_status(:forbidden) }
       end
 
-      context 'when authenticated as maintainer' do
+      context "when authenticated as #{authorized_role}" do
         before do
-          send(entity).add_maintainer(user)
+          send(entity).send("add_#{authorized_role}", user)
         end
 
         it 'creates the deploy token' do
@@ -282,7 +282,7 @@ describe API::DeployTokens do
         response
       end
 
-      it_behaves_like 'creating a deploy token', :project, :not_found
+      it_behaves_like 'creating a deploy token', :project, :not_found, :maintainer
     end
 
     describe 'POST /groups/:id/deploy_tokens' do
@@ -291,7 +291,17 @@ describe API::DeployTokens do
         response
       end
 
-      it_behaves_like 'creating a deploy token', :group, :forbidden
+      it_behaves_like 'creating a deploy token', :group, :forbidden, :owner
+
+      context 'when authenticated as maintainer' do
+        before do
+          group.add_maintainer(user)
+        end
+
+        let(:params) { { name: 'test', scopes: ['read_repository'] } }
+
+        it { is_expected.to have_gitlab_http_status(:forbidden) }
+      end
     end
   end
 
@@ -320,6 +330,14 @@ describe API::DeployTokens do
         group.add_maintainer(user)
       end
 
+      it { is_expected.to have_gitlab_http_status(:forbidden) }
+    end
+
+    context 'when authenticated as owner' do
+      before do
+        group.add_owner(user)
+      end
+
       it 'calls the deploy token destroy service' do
         expect(::Groups::DeployTokens::DestroyService).to receive(:new)
           .with(group, user, token_id: group_deploy_token.id)
diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb
index ecdb02beeb0..f33436df40e 100644
--- a/spec/requests/api/import_github_spec.rb
+++ b/spec/requests/api/import_github_spec.rb
@@ -26,6 +26,18 @@ describe API::ImportGithub do
       end
     end
 
+    it 'rejects requests when Github Importer is disabled' do
+      stub_application_setting(import_sources: nil)
+
+      post api("/import/github", user), params: {
+        target_namespace: user.namespace_path,
+        personal_access_token: token,
+        repo_id: non_existing_record_id
+      }
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
     it 'returns 201 response when the project is imported successfully' do
       allow(Gitlab::LegacyGithubImport::ProjectCreator)
         .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider)
diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
index 53183ac89f8..fb6d6603beb 100644
--- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
@@ -4,6 +4,16 @@ RSpec.shared_examples 'an unauthorized API user' do
   it { is_expected.to eq(403) }
 end
 
+RSpec.shared_examples 'API user with insufficient permissions' do
+  context 'with non member that is the author' do
+    before do
+      issuable.update!(author: non_member) # an external author can't admin issuable
+    end
+
+    it_behaves_like 'an unauthorized API user'
+  end
+end
+
 RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
   let(:non_member) { create(:user) }
 
@@ -14,6 +24,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), params: { duration: '1w' }) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "sets the time estimate for #{issuable_name}" do
@@ -53,6 +64,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", non_member)) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "resets the time estimate for #{issuable_name}" do
@@ -70,6 +82,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       end
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "add spent time for #{issuable_name}" do
@@ -119,6 +132,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", non_member)) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "resets spent time for #{issuable_name}" do
diff --git a/spec/validators/html_safety_validator_spec.rb b/spec/validators/html_safety_validator_spec.rb
new file mode 100644
index 00000000000..4d9425235e3
--- /dev/null
+++ b/spec/validators/html_safety_validator_spec.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe HtmlSafetyValidator do
+  let(:validator) { described_class.new(attributes: [:name]) }
+  let(:group) { build(:group) }
+
+  def validate(value)
+    validator.validate_each(group, :name, value)
+  end
+
+  it 'adds an error when a script is included in the name' do
+    validate('My group <script>evil_script</script>')
+
+    expect(group.errors[:name]).to eq([HtmlSafetyValidator.error_message])
+  end
+
+  it 'does not add an error when an ampersand is included in the name' do
+    validate('Group with 1 & 2')
+
+    expect(group.errors).to be_empty
+  end
+end