diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-29 19:21:38 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-29 19:21:38 +0000 |
commit | 11e9b7b58837da351f08c18e6f0f4faba4d7d301 (patch) | |
tree | d9b28159a53c3814c8a2e6b33a5f01557b757439 /spec | |
parent | 2b0b97e746e327c6168505df7740e667b690a27f (diff) | |
download | gitlab-ce-11e9b7b58837da351f08c18e6f0f4faba4d7d301.tar.gz |
Add latest changes from gitlab-org/security/gitlab@13-1-stable-ee
Diffstat (limited to 'spec')
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 <img onerror=alert(1) src=x>', project, issue, link_content: true) + + expect(data_attributes[:original]).to eq('xss &lt;img onerror=alert(1) src=x&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='\">bad things'}) + expect(result).to include(%{data-original='\"&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 |