diff options
Diffstat (limited to 'spec')
28 files changed, 757 insertions, 159 deletions
diff --git a/spec/frontend/ide/components/preview/clientside_spec.js b/spec/frontend/ide/components/preview/clientside_spec.js index 426fbd5c04c..8e8cf4ab929 100644 --- a/spec/frontend/ide/components/preview/clientside_spec.js +++ b/spec/frontend/ide/components/preview/clientside_spec.js @@ -2,15 +2,15 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import { dispatch } from 'codesandbox-api'; -import smooshpack from 'smooshpack'; +import { SandpackClient } from '@codesandbox/sandpack-client'; import Vuex from 'vuex'; import waitForPromises from 'helpers/wait_for_promises'; import Clientside from '~/ide/components/preview/clientside.vue'; import { PING_USAGE_PREVIEW_KEY, PING_USAGE_PREVIEW_SUCCESS_KEY } from '~/ide/constants'; import eventHub from '~/ide/eventhub'; -jest.mock('smooshpack', () => ({ - Manager: jest.fn(), +jest.mock('@codesandbox/sandpack-client', () => ({ + SandpackClient: jest.fn(), })); Vue.use(Vuex); @@ -78,8 +78,8 @@ describe('IDE clientside preview', () => { // eslint-disable-next-line no-restricted-syntax wrapper.setData({ sandpackReady: true, - manager: { - listener: jest.fn(), + client: { + cleanup: jest.fn(), updatePreview: jest.fn(), }, }); @@ -90,9 +90,9 @@ describe('IDE clientside preview', () => { }); describe('without main entry', () => { - it('creates sandpack manager', () => { + it('creates sandpack client', () => { createComponent(); - expect(smooshpack.Manager).not.toHaveBeenCalled(); + expect(SandpackClient).not.toHaveBeenCalled(); }); }); describe('with main entry', () => { @@ -102,8 +102,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', expectedSandpackOptions(), expectedSandpackSettings(), @@ -141,8 +141,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager with bundlerURL', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { + it('creates sandpack client with bundlerURL', () => { + expect(SandpackClient).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { ...expectedSandpackSettings(), bundlerURL: TEST_BUNDLER_URL, }); @@ -156,8 +156,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', { files: {}, @@ -332,7 +332,7 @@ describe('IDE clientside preview', () => { }); describe('update', () => { - it('initializes manager if manager is empty', () => { + it('initializes client if client is empty', () => { createComponent({ getters: { packageJson: dummyPackageJson } }); // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details // eslint-disable-next-line no-restricted-syntax @@ -340,7 +340,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); return waitForPromises().then(() => { - expect(smooshpack.Manager).toHaveBeenCalled(); + expect(SandpackClient).toHaveBeenCalled(); }); }); @@ -349,7 +349,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); @@ -361,7 +361,7 @@ describe('IDE clientside preview', () => { }); it('calls updatePreview', () => { - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); }); @@ -405,7 +405,7 @@ describe('IDE clientside preview', () => { beforeEach(() => { createInitializedComponent(); - spy = wrapper.vm.manager.updatePreview; + spy = wrapper.vm.client.updatePreview; wrapper.destroy(); }); diff --git a/spec/frontend/ide/components/preview/navigator_spec.js b/spec/frontend/ide/components/preview/navigator_spec.js index a199f4704f7..bccfb75835f 100644 --- a/spec/frontend/ide/components/preview/navigator_spec.js +++ b/spec/frontend/ide/components/preview/navigator_spec.js @@ -11,7 +11,7 @@ jest.mock('codesandbox-api', () => ({ describe('IDE clientside preview navigator', () => { let wrapper; - let manager; + let client; let listenHandler; const findBackButton = () => wrapper.findAll('button').at(0); @@ -20,9 +20,9 @@ describe('IDE clientside preview navigator', () => { beforeEach(() => { listen.mockClear(); - manager = { bundlerURL: TEST_HOST, iframe: { src: '' } }; + client = { bundlerURL: TEST_HOST, iframe: { src: '' } }; - wrapper = shallowMount(ClientsideNavigator, { propsData: { manager } }); + wrapper = shallowMount(ClientsideNavigator, { propsData: { client } }); [[listenHandler]] = listen.mock.calls; }); @@ -31,7 +31,7 @@ describe('IDE clientside preview navigator', () => { }); it('renders readonly URL bar', async () => { - listenHandler({ type: 'urlchange', url: manager.bundlerURL }); + listenHandler({ type: 'urlchange', url: client.bundlerURL }); await nextTick(); expect(wrapper.find('input[readonly]').element.value).toBe('/'); }); @@ -89,13 +89,13 @@ describe('IDE clientside preview navigator', () => { expect(findBackButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -133,13 +133,13 @@ describe('IDE clientside preview navigator', () => { expect(findForwardButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -152,10 +152,10 @@ describe('IDE clientside preview navigator', () => { }); it('calls refresh with current path', () => { - manager.iframe.src = 'something-other'; + client.iframe.src = 'something-other'; findRefreshButton().trigger('click'); - expect(manager.iframe.src).toBe(url); + expect(client.iframe.src).toBe(url); }); }); }); diff --git a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js index 70c7f56b62f..296d01ddd99 100644 --- a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js +++ b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js @@ -38,7 +38,7 @@ export default [ '</tr>\n', '</table>', ].join(''), - output: '<table>', + output: '<table data-myattr="XSS">', }, ], // Note: style is sanitized out @@ -98,7 +98,7 @@ export default [ '</svg>', ].join(), output: - '<svg xmlns="http://www.w3.org/2000/svg" width="388.84pt" version="1.0" id="svg2" height="115.02pt">', + '<svg height="115.02pt" id="svg2" version="1.0" width="388.84pt" xmlns="http://www.w3.org/2000/svg">', }, ], ]; diff --git a/spec/frontend/notebook/cells/output/index_spec.js b/spec/frontend/notebook/cells/output/index_spec.js index 4d1d03e5e34..97a7e22be60 100644 --- a/spec/frontend/notebook/cells/output/index_spec.js +++ b/spec/frontend/notebook/cells/output/index_spec.js @@ -49,15 +49,17 @@ describe('Output component', () => { const htmlType = json.cells[4]; createComponent(htmlType.outputs[0]); - expect(wrapper.findAll('p')).toHaveLength(1); - expect(wrapper.text()).toContain('test'); + const iframe = wrapper.find('iframe'); + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<p>test</p>'); }); it('renders multiple raw HTML outputs', () => { const htmlType = json.cells[4]; createComponent([htmlType.outputs[0], htmlType.outputs[0]]); - expect(wrapper.findAll('p')).toHaveLength(2); + expect(wrapper.findAll('iframe')).toHaveLength(2); }); }); @@ -84,7 +86,11 @@ describe('Output component', () => { }); it('renders as an svg', () => { - expect(wrapper.find('svg').exists()).toBe(true); + const iframe = wrapper.find('iframe'); + + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<svg></svg>'); }); }); diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index b5b572e9719..79687905fc2 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -312,7 +312,7 @@ RSpec.describe CommitsHelper do let(:current_path) { "test" } before do - expect(commit).to receive(:status_for).with(ref).and_return(commit_status) + expect(commit).to receive(:detailed_status_for).with(ref).and_return(commit_status) assign(:path, current_path) end diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 5efa88a2a7d..90366d7772c 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -112,6 +112,14 @@ RSpec.describe LabelsHelper do end end + describe 'render_label_text' do + it 'html escapes the bg_color correctly' do + xss_payload = '"><img src=x onerror=prompt(1)>' + label_text = render_label_text('xss', bg_color: xss_payload) + expect(label_text).to include(html_escape(xss_payload)) + end + end + describe 'text_color_for_bg' do it 'uses light text on dark backgrounds' do expect(text_color_for_bg('#222E2E')).to be_color('#FFFFFF') diff --git a/spec/initializers/rack_VULNDB-255039_patch_spec.rb b/spec/initializers/rack_VULNDB-255039_patch_spec.rb new file mode 100644 index 00000000000..754ff2f10e0 --- /dev/null +++ b/spec/initializers/rack_VULNDB-255039_patch_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Rack VULNDB-255039' do + context 'when handling query params in GET requests' do + it 'does not treat semicolons as query delimiters' do + env = ::Rack::MockRequest.env_for('http://gitlab.com?a=b;c=1') + + query_hash = ::Rack::Request.new(env).GET + + # Prior to this patch, this was splitting around the semicolon, which + # would return {"a"=>"b", "c"=>"1"} + expect(query_hash).to eq({ "a" => "b;c=1" }) + end + end +end diff --git a/spec/initializers/sawyer_patch_spec.rb b/spec/initializers/sawyer_patch_spec.rb new file mode 100644 index 00000000000..dc922654d7d --- /dev/null +++ b/spec/initializers/sawyer_patch_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true +require 'fast_spec_helper' +require 'sawyer' + +require_relative '../../config/initializers/sawyer_patch' + +RSpec.describe 'sawyer_patch' do + it 'raises error when acessing a method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + to_s: 'Overriding method', + user: { to_s: 'Overriding method', name: 'User name' } + } + ) + + error_message = 'Sawyer method "to_s" overlaps Ruby method. Convert to a hash to access the attribute.' + expect { sawyer_resource.to_s }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s? }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s = 'new value' }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.user.to_s }.to raise_error(Sawyer::Error, error_message) + expect(sawyer_resource.user.name).to eq('User name') + end + + it 'raises error when acessing a boolean method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + nil?: 'value' + } + ) + + expect { sawyer_resource.nil? }.to raise_error(Sawyer::Error) + end + + it 'raises error when acessing a method that expects an argument' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + 'user': 'value', + 'user=': 'value', + '==': 'value', + '!=': 'value', + '+': 'value' + } + ) + + expect(sawyer_resource.user).to eq('value') + expect { sawyer_resource.user = 'New user' }.to raise_error(ArgumentError) + expect { sawyer_resource == true }.to raise_error(ArgumentError) + expect { sawyer_resource != true }.to raise_error(ArgumentError) + expect { sawyer_resource + 1 }.to raise_error(ArgumentError) + end + + it 'does not raise error if is not an overlapping method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + count_total: 1, + user: { name: 'User name' } + } + ) + + expect(sawyer_resource.count_total).to eq(1) + expect(sawyer_resource.count_total?).to eq(true) + expect(sawyer_resource.count_total + 1).to eq(2) + expect(sawyer_resource.user.name).to eq('User name') + end +end diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index f7cb6b92b48..44738e5e771 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -18,10 +18,20 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context 'detects' do let(:email) { FFaker::Internet.email } - it 'trailers in the form of *-by and replace users with links' do - doc = filter(commit_message_html) + context 'trailers in the form of *-by' do + where(:commit_trailer) do + ["#{FFaker::Lorem.word}-by:", "#{FFaker::Lorem.word}-BY:", "#{FFaker::Lorem.word}-By:"] + end - expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + with_them do + let(:trailer) { commit_trailer } + + it 'replaces users with links' do + doc = filter(commit_message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + end + end end it 'trailers prefixed with whitespaces' do @@ -121,7 +131,14 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context "ignores" do it 'commit messages without trailers' do - exp = message = commit_html(FFaker::Lorem.sentence) + exp = message = commit_html(Array.new(5) { FFaker::Lorem.sentence }.join("\n")) + doc = filter(message) + + expect(doc.to_html).to match Regexp.escape(exp) + end + + it 'trailers without emails' do + exp = message = commit_html(Array.new(5) { 'Merged-By:' }.join("\n")) doc = filter(message) expect(doc.to_html).to match Regexp.escape(exp) diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb index 6326d894b08..78d68697ac7 100644 --- a/spec/lib/banzai/filter/image_link_filter_spec.rb +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -92,5 +92,50 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do expect(doc.at_css('a')['class']).to match(%r{with-attachment-icon}) end + + context 'when link attributes contain malicious code' do + let(:malicious_code) do + # rubocop:disable Layout/LineLength + %q(<a class='fixed-top fixed-bottom' data-create-path=/malicious-url><style> .tab-content>.tab-pane{display: block !important}</style>) + # rubocop:enable Layout/LineLength + end + + context 'when image alt contains malicious code' do + it 'ignores image alt and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, alt: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + expect(doc.at_css('a')['href']).to eq(path) + end + end + + context 'when image src contains malicious code' do + it 'ignores image src and does not use it as the link text' do + doc = filter(image(malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*></a>$}) + end + + it 'keeps image src unchanged, malicious code does not execute as part of url' do + doc = filter(image(malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + + context 'when image data-src contains malicious code' do + it 'ignores data-src and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + end + + it 'uses image data-src, malicious code does not execute as part of url' do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + end end end diff --git a/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb new file mode 100644 index 00000000000..e0a07d1ea77 --- /dev/null +++ b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::PathologicalMarkdownFilter do + include FilterSpecHelper + + let_it_be(:short_text) { '![a' * 5 } + let_it_be(:long_text) { ([short_text] * 10).join(' ') } + let_it_be(:with_images_text) { "![One ![one](one.jpg) #{'and\n' * 200} ![two ![two](two.jpg)" } + + it 'detects a significat number of unclosed image links' do + msg = <<~TEXT + _Unable to render markdown - too many unclosed markdown image links detected._ + TEXT + + expect(filter(long_text)).to eq(msg.strip) + end + + it 'does nothing when there are only a few unclosed image links' do + expect(filter(short_text)).to eq(short_text) + end + + it 'does nothing when there are only a few unclosed image links and images' do + expect(filter(with_images_text)).to eq(with_images_text) + end +end diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index 376edfb99fc..c07f99dc9fc 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -167,4 +167,16 @@ RSpec.describe Banzai::Pipeline::FullPipeline do expect(output).to include('<em>@test_</em>') end end + + describe 'unclosed image links' do + it 'detects a significat number of unclosed image links' do + markdown = '![a ' * 30 + msg = <<~TEXT + Unable to render markdown - too many unclosed markdown image links detected. + TEXT + output = described_class.to_html(markdown, project: nil) + + expect(output).to include(msg.strip) + end + end end diff --git a/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb b/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb index 09d2919c6c4..76f2dd4b659 100644 --- a/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb @@ -10,9 +10,9 @@ RSpec.describe Banzai::Pipeline::IncidentManagement::TimelineEventPipeline do expect(described_class.filters).to eq( [ *Banzai::Pipeline::PlainMarkdownPipeline.filters, + Banzai::Filter::SanitizationFilter, *Banzai::Pipeline::GfmPipeline.reference_filters, Banzai::Filter::EmojiFilter, - Banzai::Filter::SanitizationFilter, Banzai::Filter::ExternalLinkFilter, Banzai::Filter::ImageLinkFilter ] @@ -62,7 +62,32 @@ RSpec.describe Banzai::Pipeline::IncidentManagement::TimelineEventPipeline do context 'when markdown contains emojis' do let(:markdown) { ':+1:π' } - it { is_expected.to eq('<p>ππ</p>') } + it 'renders emojis wrapped in <gl-emoji> tag' do + # rubocop:disable Layout/LineLength + is_expected.to eq( + %q(<p><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji></p>) + ) + # rubocop:enable Layout/LineLength + end + end + + context 'when markdown contains labels' do + let(:label) { create(:label, project: project, title: 'backend') } + let(:markdown) { %Q(~"#{label.name}" ~unknown) } + + it 'replaces existing label to a link' do + # rubocop:disable Layout/LineLength + is_expected.to match( + %r(<p>.+<a href=\"[\w/]+-/issues\?label_name=#{label.name}\".+style=\"background-color: #\d{6}\".*>#{label.name}</span></a></span> ~unknown</p>) + ) + # rubocop:enable Layout/LineLength + end + end + + context 'when markdown contains table' do + let(:markdown) { '<table><tr><th>table head</th><tr><tr><td>table content</td></tr></table>'} + + it { is_expected.to eq('table headtable content') } end context 'when markdown contains a reference to an issue' do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index d5d1bef7bff..d5f190d572d 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -156,10 +156,11 @@ RSpec.describe Gitlab::GitalyClient::CommitService do let(:recursive) { false } let(:pagination_params) { nil } - it 'sends a get_tree_entries message' do + it 'sends a get_tree_entries message with default limit' do + expected_pagination_params = Gitaly::PaginationParameter.new(limit: Gitlab::GitalyClient::CommitService::TREE_ENTRIES_DEFAULT_LIMIT) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([]) is_expected.to eq([[], nil]) @@ -189,9 +190,10 @@ RSpec.describe Gitlab::GitalyClient::CommitService do pagination_cursor: pagination_cursor ) + expected_pagination_params = Gitaly::PaginationParameter.new(limit: 3) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([response]) is_expected.to eq([[], pagination_cursor]) diff --git a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb index f405b2ad86e..207ac1c0eaa 100644 --- a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb +++ b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb @@ -72,4 +72,18 @@ RSpec.describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do it { is_expected.to be(true) } end end + + describe 'count' do + subject { cache.count(cache_prefix) } + + it { is_expected.to be(0) } + + context 'item added' do + before do + cache.write(cache_prefix, 'test_item') + end + + it { is_expected.to be(1) } + end + end end diff --git a/spec/lib/gitlab/zentao/client_spec.rb b/spec/lib/gitlab/zentao/client_spec.rb index 135f13e6265..b17ad867f0d 100644 --- a/spec/lib/gitlab/zentao/client_spec.rb +++ b/spec/lib/gitlab/zentao/client_spec.rb @@ -2,17 +2,21 @@ require 'spec_helper' -RSpec.describe Gitlab::Zentao::Client do - subject(:integration) { described_class.new(zentao_integration) } +RSpec.describe Gitlab::Zentao::Client, :clean_gitlab_redis_cache do + subject(:client) { described_class.new(zentao_integration) } let(:zentao_integration) { create(:zentao_integration) } def mock_get_products_url - integration.send(:url, "products/#{zentao_integration.zentao_product_xid}") + client.send(:url, "products/#{zentao_integration.zentao_product_xid}") + end + + def mock_fetch_issues_url + client.send(:url, "products/#{zentao_integration.zentao_product_xid}/issues") end def mock_fetch_issue_url(issue_id) - integration.send(:url, "issues/#{issue_id}") + client.send(:url, "issues/#{issue_id}") end let(:mock_headers) do @@ -29,13 +33,13 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { nil } it 'raises ConfigError' do - expect { integration }.to raise_error(described_class::ConfigError) + expect { client }.to raise_error(described_class::ConfigError) end end context 'integration is provided' do it 'is initialized successfully' do - expect { integration }.not_to raise_error + expect { client }.not_to raise_error end end end @@ -50,7 +54,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'fetches the product' do - expect(integration.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response + expect(client.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response end end @@ -62,8 +66,8 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) - end.to raise_error(Gitlab::Zentao::Client::Error, 'request error') + client.fetch_product(zentao_integration.zentao_product_xid) + end.to raise_error(Gitlab::Zentao::Client::RequestError) end end @@ -75,7 +79,7 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) + client.fetch_product(zentao_integration.zentao_product_xid) end.to raise_error(Gitlab::Zentao::Client::Error, 'invalid response format') end end @@ -89,7 +93,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with success' do - expect(integration.ping[:success]).to eq true + expect(client.ping[:success]).to eq true end end @@ -100,7 +104,69 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with unsuccess' do - expect(integration.ping[:success]).to eq false + expect(client.ping[:success]).to eq false + end + end + end + + describe '#fetch_issues' do + let(:mock_response) { { 'issues' => [{ 'id' => 'story-1' }, { 'id' => 'bug-11' }] } } + + before do + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: mock_response.to_json) + end + + it 'returns the response' do + expect(client.fetch_issues).to eq(mock_response) + end + + describe 'marking the issues as seen in the product' do + let(:cache) { ::Gitlab::SetCache.new } + let(:cache_key) do + [ + :zentao_product_issues, + OpenSSL::Digest::SHA256.hexdigest(zentao_integration.client_url), + zentao_integration.zentao_product_xid + ].join(':') + end + + it 'adds issue ids to the cache' do + expect { client.fetch_issues }.to change { cache.read(cache_key) } + .from(be_empty) + .to match_array(%w[bug-11 story-1]) + end + + it 'does not add issue ids to the cache if max set size has been reached' do + cache.write(cache_key, %w[foo bar]) + stub_const("#{described_class}::CACHE_MAX_SET_SIZE", 1) + + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[foo bar]) + end + + it 'does not duplicate issue ids in the cache' do + client.fetch_issues + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[bug-11 story-1]) + end + + it 'touches the cache ttl every time issues are fetched' do + fresh_ttl = 1.month.to_i + + freeze_time do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end + + travel_to(1.minute.from_now) do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end end end end @@ -109,9 +175,9 @@ RSpec.describe Gitlab::Zentao::Client do context 'with invalid id' do let(:invalid_ids) { ['story', 'story-', '-', '123', ''] } - it 'returns empty object' do + it 'raises Error' do invalid_ids.each do |id| - expect { integration.fetch_issue(id) } + expect { client.fetch_issue(id) } .to raise_error(Gitlab::Zentao::Client::Error, 'invalid issue id') end end @@ -120,12 +186,31 @@ RSpec.describe Gitlab::Zentao::Client do context 'with valid id' do let(:valid_ids) { %w[story-1 bug-23] } - it 'fetches current issue' do - valid_ids.each do |id| - WebMock.stub_request(:get, mock_fetch_issue_url(id)) - .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + context 'when issue has been seen on the index' do + before do + issues_body = { issues: valid_ids.map { { id: _1 } } }.to_json + + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: issues_body) + + client.fetch_issues + end + + it 'fetches the issue' do + valid_ids.each do |id| + WebMock.stub_request(:get, mock_fetch_issue_url(id)) + .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + + expect(client.fetch_issue(id).dig('issue', 'id')).to eq id + end + end + end - expect(integration.fetch_issue(id).dig('issue', 'id')).to eq id + context 'when issue has not been seen on the index' do + it 'raises RequestError' do + valid_ids.each do |id| + expect { client.fetch_issue(id) }.to raise_error(Gitlab::Zentao::Client::RequestError) + end end end end @@ -135,7 +220,7 @@ RSpec.describe Gitlab::Zentao::Client do context 'api url' do shared_examples 'joins api_url correctly' do it 'verify url' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/zentao/api.php/v1/products/1") end end @@ -157,7 +242,7 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { create(:zentao_integration, url: 'https://jihudemo.zentao.net') } it 'joins url correctly' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/api.php/v1/products/1") end end diff --git a/spec/models/incident_management/timeline_event_spec.rb b/spec/models/incident_management/timeline_event_spec.rb index 17150fc9266..9f4011fe6a7 100644 --- a/spec/models/incident_management/timeline_event_spec.rb +++ b/spec/models/incident_management/timeline_event_spec.rb @@ -47,11 +47,20 @@ RSpec.describe IncidentManagement::TimelineEvent do describe '#cache_markdown_field' do let(:note) { 'note **bold** _italic_ `code` ![image](/path/img.png) :+1:π' } + + let(:expected_image_html) do + '<a class="with-attachment-icon" href="/path/img.png" target="_blank" rel="noopener noreferrer">image</a>' + end + + # rubocop:disable Layout/LineLength + let(:expected_emoji_html) do + '<gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji>' + end + let(:expected_note_html) do - # rubocop:disable Layout/LineLength - '<p>note <strong>bold</strong> <em>italic</em> <code>code</code> <a class="with-attachment-icon" href="/path/img.png" target="_blank" rel="noopener noreferrer">image</a> ππ</p>' - # rubocop:enable Layout/LineLength + %Q(<p>note <strong>bold</strong> <em>italic</em> <code>code</code> #{expected_image_html} #{expected_emoji_html}</p>) end + # rubocop:enable Layout/LineLength before do allow(Banzai::Renderer).to receive(:cacheless_render_field).and_call_original diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 4ef977ba3d2..1a32453819d 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -81,4 +81,24 @@ RSpec.describe Integrations::Zentao do expect(zentao_integration.help).not_to be_empty end end + + describe '#client_url' do + subject(:integration) { build(:zentao_integration, api_url: api_url, url: 'url').client_url } + + context 'when api_url is set' do + let(:api_url) { 'api_url' } + + it 'returns the api_url' do + is_expected.to eq(api_url) + end + end + + context 'when api_url is not set' do + let(:api_url) { '' } + + it 'returns the url' do + is_expected.to eq('url') + end + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 89c440dc49c..90e87a7ed2e 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -670,14 +670,22 @@ RSpec.describe Issue do end describe '#to_branch_name exists ending with -index' do - before do + it 'returns #to_branch_name ending with max index + 1' do allow(repository).to receive(:branch_exists?).and_return(true) allow(repository).to receive(:branch_exists?).with("#{subject.to_branch_name}-3").and_return(false) - end - it 'returns #to_branch_name ending with max index + 1' do expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-3") end + + context 'when branch name still exists after 5 attempts' do + it 'returns #to_branch_name ending with random characters' do + allow(repository).to receive(:branch_exists?).with(subject.to_branch_name).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\d/).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\h{8}/).and_return(false) + + expect(subject.suggested_branch_name).to match(/#{subject.to_branch_name}-\h{8}/) + end + end end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index a54edc8510e..da94441d621 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -91,6 +91,45 @@ RSpec.describe Snippet do end end end + + context 'description validations' do + let_it_be(:invalid_description) { 'a' * (described_class::DESCRIPTION_LENGTH_MAX * 2) } + + context 'with existing snippets' do + let(:snippet) { create(:personal_snippet, description: 'This is a valid content at the time of creation') } + + it 'does not raise a validation error if the description is not changed' do + snippet.title = 'new title' + + expect(snippet).to be_valid + end + + it 'raises and error if the description is changed and the size is bigger than limit' do + expect(snippet).to be_valid + + snippet.description = invalid_description + + expect(snippet).not_to be_valid + end + end + + context 'with new snippets' do + it 'is valid when description is smaller than the limit' do + snippet = build(:personal_snippet, description: 'Valid Desc') + + expect(snippet).to be_valid + end + + it 'raises error when description is bigger than setting limit' do + snippet = build(:personal_snippet, description: invalid_description) + + aggregate_failures do + expect(snippet).not_to be_valid + expect(snippet.errors.messages_for(:description)).to include("is too long (2 MB). The maximum size is 1 MB.") + end + end + end + end end describe 'callbacks' do diff --git a/spec/presenters/commit_presenter_spec.rb b/spec/presenters/commit_presenter_spec.rb index b221c9ca8f7..df3ee69621b 100644 --- a/spec/presenters/commit_presenter_spec.rb +++ b/spec/presenters/commit_presenter_spec.rb @@ -12,29 +12,51 @@ RSpec.describe CommitPresenter do it { expect(presenter.web_path).to eq("/#{project.full_path}/-/commit/#{commit.sha}") } end - describe '#status_for' do - subject { presenter.status_for('ref') } + describe '#detailed_status_for' do + using RSpec::Parameterized::TableSyntax + + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha, ref: 'ref') } - context 'when user can read_commit_status' do + subject { presenter.detailed_status_for('ref')&.text } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'passed' + true | false | nil + false | true | nil + false | false | nil + end + + with_them do before do - allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(true) + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end - it 'returns commit status for ref' do - pipeline = double - status = double + it { is_expected.to eq expected_result } + end + end - expect(commit).to receive(:latest_pipeline).with('ref').and_return(pipeline) - expect(pipeline).to receive(:detailed_status).with(user).and_return(status) + describe '#status_for' do + using RSpec::Parameterized::TableSyntax - expect(subject).to eq(status) - end + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha) } + + subject { presenter.status_for } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'success' + true | false | nil + false | true | nil + false | false | nil end - context 'when user can not read_commit_status' do - it 'is nil' do - is_expected.to eq(nil) + with_them do + before do + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end + + it { is_expected.to eq expected_result } end end diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb index 31fef75f679..bcbb1f11d43 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -6,11 +6,16 @@ RSpec.describe 'getting incident timeline events' do include GraphqlHelpers let_it_be(:project) { create(:project) } + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: private_project) } let_it_be(:current_user) { create(:user) } let_it_be(:updated_by_user) { create(:user) } let_it_be(:incident) { create(:incident, project: project) } let_it_be(:another_incident) { create(:incident, project: project) } let_it_be(:promoted_from_note) { create(:note, project: project, noteable: incident) } + let_it_be(:issue_url) { project_issue_url(private_project, issue) } + let_it_be(:issue_ref) { "#{private_project.full_path}##{issue.iid}" } + let_it_be(:issue_link) { %Q(<a href="#{issue_url}">#{issue_url}</a>) } let_it_be(:timeline_event) do create( @@ -18,7 +23,8 @@ RSpec.describe 'getting incident timeline events' do incident: incident, project: project, updated_by_user: updated_by_user, - promoted_from_note: promoted_from_note + promoted_from_note: promoted_from_note, + note: "Referencing #{issue.to_reference(full: true)} - Full URL #{issue_url}" ) end @@ -89,7 +95,7 @@ RSpec.describe 'getting incident timeline events' do 'title' => incident.title }, 'note' => timeline_event.note, - 'noteHtml' => timeline_event.note_html, + 'noteHtml' => "<p>Referencing #{issue_ref} - Full URL #{issue_link}</p>", 'promotedFromNote' => { 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 4d2a69cd85b..7d8213c48f7 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -752,6 +752,96 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :commits, search: 'merge' it_behaves_like 'ping counters', scope: :commits + + describe 'pipeline visibility' do + shared_examples 'pipeline information visible' do + it 'contains status and last_pipeline' do + request + + expect(json_response[0]['status']).to eq 'success' + expect(json_response[0]['last_pipeline']).not_to be_nil + end + end + + shared_examples 'pipeline information not visible' do + it 'does not contain status and last_pipeline' do + request + + expect(json_response[0]['status']).to be_nil + expect(json_response[0]['last_pipeline']).to be_nil + end + end + + let(:request) { get api(endpoint, user), params: { scope: 'commits', search: repo_project.commit.sha } } + + before do + create(:ci_pipeline, :success, project: repo_project, sha: repo_project.commit.sha) + end + + context 'with non public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: false, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information not visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information not visible' + end + end + + context 'with public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: true, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information visible' + + context 'when CI/CD is set to only project members' do + before do + repo_project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'pipeline information not visible' + end + end + end + end end context 'for commits scope with project path as id' do diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 05b16119a0e..21d92adebce 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -643,17 +643,17 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -750,17 +750,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -771,10 +771,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'displays the generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1300,17 +1300,18 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1381,17 +1382,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -1402,10 +1403,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'returns a generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 70097234762..843d8f22d75 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -22,6 +22,37 @@ RSpec.describe JwtController do end end + shared_examples 'a token that expires today' do + let(:pat) { create(:personal_access_token, user: user, scopes: ['api'], expires_at: Date.today ) } + let(:headers) { { authorization: credentials('personal_access_token', pat.token) } } + + it 'fails authentication' do + expect(::Gitlab::AuthLogger).to receive(:warn).with( + hash_including(message: 'JWT authentication failed', + http_user: 'personal_access_token')).and_call_original + + get '/jwt/auth', params: parameters, headers: headers + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + shared_examples "with invalid credentials" do + it "returns a generic error message" do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response).to eq( + { + "errors" => [{ + "code" => "UNAUTHORIZED", + "message" => "HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/user/profile/account/two_factor_authentication#troubleshooting" + }] + } + ) + end + end + context 'authenticating against container registry' do context 'existing service' do subject! { get '/jwt/auth', params: parameters } @@ -40,10 +71,7 @@ RSpec.describe JwtController do context 'with blocked user' do let(:user) { create(:user, :blocked) } - it 'rejects the request as unauthorized' do - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('HTTP Basic: Access denied') - end + it_behaves_like 'with invalid credentials' end end @@ -142,10 +170,7 @@ RSpec.describe JwtController do let(:user) { create(:user, :two_factor) } context 'without personal token' do - it 'rejects the authorization attempt' do - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'with personal token' do @@ -169,14 +194,10 @@ RSpec.describe JwtController do context 'using invalid login' do let(:headers) { { authorization: credentials('invalid', 'password') } } + let(:subject) { get '/jwt/auth', params: parameters, headers: headers } context 'when internal auth is enabled' do - it 'rejects the authorization attempt' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'when internal auth is disabled' do @@ -184,12 +205,7 @@ RSpec.describe JwtController do stub_application_setting(password_authentication_enabled_for_git: false) end - it 'rejects the authorization attempt with personal access token message' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end end end diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index 1a248bb04e7..ba8311bf0be 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -170,6 +170,17 @@ RSpec.shared_examples 'PyPI package download' do |user_type, status, add_member end end +RSpec.shared_examples 'rejected package download' do |user_type, status, add_member = true| + context "for user type #{user_type}" do + before do + project.send("add_#{user_type}", user) if add_member && user_type != :anonymous + group.send("add_#{user_type}", user) if add_member && user_type != :anonymous + end + + it_behaves_like 'returning response status', status + end +end + RSpec.shared_examples 'process PyPI api request' do |user_type, status, add_member = true| context "for user type #{user_type}" do before do @@ -330,25 +341,25 @@ RSpec.shared_examples 'pypi file download endpoint' do using RSpec::Parameterized::TableSyntax context 'with valid project' do - where(:visibility_level, :user_role, :member, :user_token) do - :public | :developer | true | true - :public | :guest | true | true - :public | :developer | true | false - :public | :guest | true | false - :public | :developer | false | true - :public | :guest | false | true - :public | :developer | false | false - :public | :guest | false | false - :public | :anonymous | false | true - :private | :developer | true | true - :private | :guest | true | true - :private | :developer | true | false - :private | :guest | true | false - :private | :developer | false | true - :private | :guest | false | true - :private | :developer | false | false - :private | :guest | false | false - :private | :anonymous | false | true + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + :public | :developer | true | true | 'PyPI package download' | :success + :public | :guest | true | true | 'PyPI package download' | :success + :public | :developer | true | false | 'PyPI package download' | :success + :public | :guest | true | false | 'PyPI package download' | :success + :public | :developer | false | true | 'PyPI package download' | :success + :public | :guest | false | true | 'PyPI package download' | :success + :public | :developer | false | false | 'PyPI package download' | :success + :public | :guest | false | false | 'PyPI package download' | :success + :public | :anonymous | false | true | 'PyPI package download' | :success + :private | :developer | true | true | 'PyPI package download' | :success + :private | :guest | true | true | 'rejected package download' | :forbidden + :private | :developer | true | false | 'rejected package download' | :unauthorized + :private | :guest | true | false | 'rejected package download' | :unauthorized + :private | :developer | false | true | 'rejected package download' | :not_found + :private | :guest | false | true | 'rejected package download' | :not_found + :private | :developer | false | false | 'rejected package download' | :unauthorized + :private | :guest | false | false | 'rejected package download' | :unauthorized + :private | :anonymous | false | true | 'rejected package download' | :unauthorized end with_them do @@ -360,7 +371,7 @@ RSpec.shared_examples 'pypi file download endpoint' do group.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility_level.to_s)) end - it_behaves_like 'PyPI package download', params[:user_role], :success, params[:member] + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] end end diff --git a/spec/validators/bytesize_validator_spec.rb b/spec/validators/bytesize_validator_spec.rb new file mode 100644 index 00000000000..1914ccedd87 --- /dev/null +++ b/spec/validators/bytesize_validator_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BytesizeValidator do + let(:model) do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + + attr_accessor :content + alias_method :content_before_type_cast, :content + + validates :content, bytesize: { maximum: -> { 7 } } + end.new + end + + using RSpec::Parameterized::TableSyntax + + where(:content, :validity, :errors) do + 'short' | true | {} + 'very long' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'shortπ' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'shortβ' | false | { content: ['is too long (8 Bytes). The maximum size is 7 Bytes.'] } + end + + with_them do + before do + model.content = content + model.validate + end + + it { expect(model.valid?).to eq(validity) } + it { expect(model.errors.messages).to eq(errors) } + end +end diff --git a/spec/views/projects/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb index 2ca23d4cb2d..4e8a680b6de 100644 --- a/spec/views/projects/commits/_commit.html.haml_spec.rb +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -47,13 +47,12 @@ RSpec.describe 'projects/commits/_commit.html.haml' do context 'with ci status' do let(:ref) { 'master' } - let(:user) { create(:user) } + + let_it_be(:user) { create(:user) } before do allow(view).to receive(:current_user).and_return(user) - project.add_developer(user) - create( :ci_empty_pipeline, ref: 'master', @@ -80,18 +79,32 @@ RSpec.describe 'projects/commits/_commit.html.haml' do end context 'when pipelines are enabled' do - before do - allow(project).to receive(:builds_enabled?).and_return(true) + context 'when user has access' do + before do + project.add_developer(user) + end + + it 'displays a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } + + expect(rendered).to have_css('.ci-status-link') + end end - it 'does display a ci status icon when pipelines are enabled' do - render partial: template, formats: :html, locals: { - project: project, - ref: ref, - commit: commit - } + context 'when user does not have access' do + it 'does not display a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } - expect(rendered).to have_css('.ci-status-link') + expect(rendered).not_to have_css('.ci-status-link') + end end end end |