diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-02-03 23:21:29 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-02-03 23:21:29 +0000 |
commit | c540186f36d48c24bd2f69905a82502719a41249 (patch) | |
tree | 19da68471f7c551c65c60df2e7258eea9cca3b42 /spec | |
parent | abbf44bd6cfb29413b3cf5768b691e5b222b89ea (diff) | |
parent | c25c22ccd8cdf0ec66b7e91ad7f8edd4d3a4f86c (diff) | |
download | gitlab-ce-c540186f36d48c24bd2f69905a82502719a41249.tar.gz |
Merge remote-tracking branch 'dev/14-7-stable' into 14-7-stable
Diffstat (limited to 'spec')
40 files changed, 825 insertions, 208 deletions
diff --git a/spec/factories/packages/packages.rb b/spec/factories/packages/packages.rb index 153518f4cd3..90c68074a3b 100644 --- a/spec/factories/packages/packages.rb +++ b/spec/factories/packages/packages.rb @@ -20,6 +20,10 @@ FactoryBot.define do status { :error } end + trait :pending_destruction do + status { :pending_destruction } + end + factory :maven_package do maven_metadatum diff --git a/spec/features/groups/members/manage_members_spec.rb b/spec/features/groups/members/manage_members_spec.rb index 0ce50107e54..e5dad5ee4be 100644 --- a/spec/features/groups/members/manage_members_spec.rb +++ b/spec/features/groups/members/manage_members_spec.rb @@ -103,7 +103,7 @@ RSpec.describe 'Groups > Members > Manage members' do find('[data-testid="members-token-select-input"]').set('undisclosed_email@gitlab.com') wait_for_requests - expect(page).to have_content("Jane 'invisible' Doe") + expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email') end context 'when Invite Members modal is disabled' do @@ -129,7 +129,7 @@ RSpec.describe 'Groups > Members > Manage members' do select_input.send_keys('undisclosed_email@gitlab.com') wait_for_requests - expect(page).to have_content("Jane 'invisible' Doe") + expect(page).to have_content('Invite "undisclosed_email@gitlab.com" by email') end end diff --git a/spec/features/issues/notes_on_issues_spec.rb b/spec/features/issues/notes_on_issues_spec.rb index be85d73d777..4e98062e8b2 100644 --- a/spec/features/issues/notes_on_issues_spec.rb +++ b/spec/features/issues/notes_on_issues_spec.rb @@ -91,4 +91,62 @@ RSpec.describe 'Create notes on issues', :js do expect(page).to have_selector '.gfm-project_member.current-user', text: user.username end + + shared_examples "when reference belongs to a private project" do + let(:project) { create(:project, :private, :repository) } + let(:issue) { create(:issue, project: project) } + + before do + sign_in(user) + end + + context 'when the user does not have permission to see the reference' do + before do + project.add_guest(user) + end + + it 'does not show the user the reference' do + visit project_issue_path(project, issue) + + expect(page).not_to have_content('closed via') + end + end + + context 'when the user has permission to see the reference' do + before do + project.add_developer(user) + end + + it 'shows the user the reference' do + visit project_issue_path(project, issue) + + page.within('div#notes li.note .system-note-message') do + expect(page).to have_content('closed via') + expect(page.find('a')).to have_content(reference_content) + end + end + end + end + + context 'when the issue is closed via a merge request' do + it_behaves_like "when reference belongs to a private project" do + let(:reference) { create(:merge_request, source_project: project) } + let(:reference_content) { reference.to_reference } + + before do + create(:resource_state_event, issue: issue, state: :closed, created_at: '2020-02-05', source_merge_request: reference) + end + end + end + + context 'when the issue is closed via a commit' do + it_behaves_like "when reference belongs to a private project" do + let(:reference) { create(:commit, project: project) } + let(:reference_content) { reference.short_sha } + + before do + create(:resource_state_event, issue: issue, state: :closed, created_at: '2020-02-05', source_commit: reference.id) + end + end + end end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 4278efc5a8f..389a51a10e0 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -38,6 +38,17 @@ RSpec.describe 'Protected Branches', :js do sign_in(user) end + it 'allows to create a protected branch with name containing HTML tags' do + visit project_protected_branches_path(project) + set_defaults + set_protected_branch_name('foo<b>bar<\b>') + click_on "Protect" + + within(".protected-branches-list") { expect(page).to have_content('foo<b>bar<\b>') } + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.name).to eq('foo<b>bar<\b>') + end + describe 'Delete protected branch' do before do create(:protected_branch, project: project, name: 'fix') diff --git a/spec/finders/users_finder_spec.rb b/spec/finders/users_finder_spec.rb index b0f8b803141..fab48cf3178 100644 --- a/spec/finders/users_finder_spec.rb +++ b/spec/finders/users_finder_spec.rb @@ -39,6 +39,12 @@ RSpec.describe UsersFinder do expect(users).to contain_exactly(blocked_user) end + it 'does not filter by private emails search' do + users = described_class.new(user, search: normal_user.email).execute + + expect(users).to be_empty + end + it 'filters by blocked users' do users = described_class.new(user, blocked: true).execute @@ -135,6 +141,12 @@ RSpec.describe UsersFinder do expect(users).to contain_exactly(normal_user) end + + it 'filters by private emails search' do + users = described_class.new(admin, search: normal_user.email).execute + + expect(users).to contain_exactly(normal_user) + end end end end diff --git a/spec/frontend/create_item_dropdown_spec.js b/spec/frontend/create_item_dropdown_spec.js index 56c09cd731e..143ccb9b930 100644 --- a/spec/frontend/create_item_dropdown_spec.js +++ b/spec/frontend/create_item_dropdown_spec.js @@ -17,6 +17,11 @@ const DROPDOWN_ITEM_DATA = [ id: 'three', text: 'three', }, + { + title: '<b>four</b>title', + id: '<b>four</b>id', + text: '<b>four</b>text', + }, ]; describe('CreateItemDropdown', () => { @@ -63,6 +68,10 @@ describe('CreateItemDropdown', () => { const $itemEls = $wrapperEl.find('.js-dropdown-content a'); expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length); + + DROPDOWN_ITEM_DATA.forEach((dataItem, i) => { + expect($($itemEls[i]).text()).toEqual(dataItem.text); + }); }); }); @@ -177,7 +186,7 @@ describe('CreateItemDropdown', () => { const $itemEls = $wrapperEl.find('.js-dropdown-content a'); expect($itemEls.length).toEqual(1 + DROPDOWN_ITEM_DATA.length); - expect($($itemEls[3]).text()).toEqual('new-item-text'); + expect($($itemEls[DROPDOWN_ITEM_DATA.length]).text()).toEqual('new-item-text'); expect($wrapperEl.find('.dropdown-toggle-text').text()).toEqual('new-item-title'); }); }); diff --git a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js index 803ac4a219d..70c7f56b62f 100644 --- a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js +++ b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js @@ -16,13 +16,20 @@ export default [ 'text/html table', { input: [ - '<table>\n', + '<style type="text/css">\n', + '\n', + 'body {\n', + ' background: red;\n', + '}\n', + '\n', + '</style>\n', + '<table data-myattr="XSS">\n', '<tr>\n', '<th>Header 1</th>\n', '<th>Header 2</th>\n', '</tr>\n', '<tr>\n', - '<td>row 1, cell 1</td>\n', + '<td style="background: red;">row 1, cell 1</td>\n', '<td>row 1, cell 2</td>\n', '</tr>\n', '<tr>\n', diff --git a/spec/graphql/types/packages/package_status_enum_spec.rb b/spec/graphql/types/packages/package_status_enum_spec.rb index 71d05da35ea..2c79c92498b 100644 --- a/spec/graphql/types/packages/package_status_enum_spec.rb +++ b/spec/graphql/types/packages/package_status_enum_spec.rb @@ -4,6 +4,6 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['PackageStatus'] do it 'exposes all package statuses' do - expect(described_class.values.keys).to contain_exactly(*%w[DEFAULT HIDDEN PROCESSING ERROR]) + expect(described_class.values.keys).to contain_exactly(*%w[DEFAULT HIDDEN PROCESSING ERROR PENDING_DESTRUCTION]) end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index cd216232569..961e12288d4 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -595,4 +595,45 @@ RSpec.describe GitlabSchema.types['Project'] do expect(cluster_agent.agent_tokens.size).to be(count) end end + + describe 'service_desk_address' do + let(:user) { create(:user) } + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + id + serviceDeskAddress + } + } + ) + end + + subject { GitlabSchema.execute(query, context: { current_user: user }).as_json } + + before do + allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?) { true } + allow(::Gitlab::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' } + end + + context 'when a user can admin issues' do + let(:project) { create(:project, :public, :service_desk_enabled) } + + before do + project.add_reporter(user) + end + + it 'is present' do + expect(subject.dig('data', 'project', 'serviceDeskAddress')).to be_present + end + end + + context 'when a user can not admin issues' do + let(:project) { create(:project, :public, :service_desk_disabled) } + + it 'is empty' do + expect(subject.dig('data', 'project', 'serviceDeskAddress')).to be_blank + end + end + end end diff --git a/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb index e736943914b..2d326bd77a6 100644 --- a/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb +++ b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb @@ -17,4 +17,14 @@ RSpec.describe Banzai::Filter::BlockquoteFenceFilter do it 'allows trailing whitespace on blockquote fence lines' do expect(filter(">>> \ntest\n>>> ")).to eq("\n> test\n") end + + context 'when incomplete blockquote fences with multiple blocks are present' do + it 'does not raise timeout error' do + test_string = ">>>#{"\n```\nfoo\n```" * 20}" + + expect do + Timeout.timeout(2.seconds) { filter(test_string) } + end.not_to raise_error + end + end end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 0713475d59b..5b77290ce2e 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -279,6 +279,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do end context 'when allow_local_network is' do + let(:shared_address_space_ips) { ['100.64.0.0', '100.64.127.127', '100.64.255.255'] } + let(:local_ips) do [ '192.168.1.2', @@ -292,7 +294,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do '[::ffff:ac10:20]', '[feef::1]', '[fee2::]', - '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]' + '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]', + *shared_address_space_ips ] end @@ -385,18 +388,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do '127.0.0.1', '127.0.0.2', '192.168.1.1', - '192.168.1.2', - '0:0:0:0:0:ffff:192.168.1.2', - '::ffff:c0a8:102', - '10.0.0.2', - '0:0:0:0:0:ffff:10.0.0.2', - '::ffff:a00:2', - '172.16.0.2', - '0:0:0:0:0:ffff:172.16.0.2', - '::ffff:ac10:20', - 'feef::1', - 'fee2::', - 'fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa', + *local_ips, '0:0:0:0:0:ffff:169.254.169.254', '::ffff:a9fe:a9fe', '::ffff:169.254.168.100', diff --git a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb new file mode 100644 index 00000000000..802e950c0c2 --- /dev/null +++ b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::EnableSslVerification do + let(:described_class) do + Class.new(Integration) do + prepend Integrations::EnableSslVerification + + def fields + [ + { name: 'main_url' }, + { name: 'other_url' }, + { name: 'username' } + ] + end + end + end + + let(:integration) { described_class.new } + + include_context Integrations::EnableSslVerification +end diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 60ff6685c3d..b5684d153f2 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -23,6 +23,8 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do ) end + include_context Integrations::EnableSslVerification + describe 'Validations' do context 'when active' do before do diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 062e23d628e..dd64dcfc52c 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers + subject(:integration) { described_class.new } + describe 'validations' do context 'active' do before do @@ -59,6 +61,52 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end end + include_context Integrations::EnableSslVerification do + describe '#enable_ssl_verification' do + before do + allow(integration).to receive(:new_record?).and_return(false) + end + + it 'returns true for a known hostname' do + integration.drone_url = 'https://cloud.drone.io' + + expect(integration.enable_ssl_verification).to be(true) + end + + it 'returns true for new records' do + allow(integration).to receive(:new_record?).and_return(true) + integration.drone_url = 'http://example.com' + + expect(integration.enable_ssl_verification).to be(true) + end + + it 'returns false for an unknown hostname' do + integration.drone_url = 'https://example.com' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns false for a HTTP URL' do + integration.drone_url = 'http://cloud.drone.io' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns false for an invalid URL' do + integration.drone_url = 'https://example.com:foo' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns the persisted value if present' do + integration.drone_url = 'https://cloud.drone.io' + integration.enable_ssl_verification = false + + expect(integration.enable_ssl_verification).to be(false) + end + end + end + it_behaves_like Integrations::HasWebHook do include_context :drone_ci_integration diff --git a/spec/models/integrations/irker_spec.rb b/spec/models/integrations/irker_spec.rb index 8b207e8b43e..8aea2c26dc5 100644 --- a/spec/models/integrations/irker_spec.rb +++ b/spec/models/integrations/irker_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' require 'socket' +require 'timeout' require 'json' RSpec.describe Integrations::Irker do @@ -37,6 +38,7 @@ RSpec.describe Integrations::Irker do before do @irker_server = TCPServer.new 'localhost', 0 + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true) allow(irker).to receive_messages( active: true, project: project, @@ -58,13 +60,17 @@ RSpec.describe Integrations::Irker do irker.execute(sample_data) conn = @irker_server.accept - conn.each_line do |line| - msg = Gitlab::Json.parse(line.chomp("\n")) - expect(msg.keys).to match_array(%w(to privmsg)) - expect(msg['to']).to match_array(["irc://chat.freenode.net/#commits", - "irc://test.net/#test"]) + + Timeout.timeout(5) do + conn.each_line do |line| + msg = Gitlab::Json.parse(line.chomp("\n")) + expect(msg.keys).to match_array(%w(to privmsg)) + expect(msg['to']).to match_array(["irc://chat.freenode.net/#commits", + "irc://test.net/#test"]) + end end - conn.close + ensure + conn.close if conn end end end diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 9286d026290..3d6393f2793 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -24,6 +24,10 @@ RSpec.describe Integrations::Jenkins do let(:jenkins_authorization) { "Basic " + ::Base64.strict_encode64(jenkins_username + ':' + jenkins_password) } + include_context Integrations::EnableSslVerification do + let(:integration) { described_class.new(jenkins_params) } + end + it_behaves_like Integrations::HasWebHook do let(:integration) { described_class.new(jenkins_params) } let(:hook_url) { "http://#{ERB::Util.url_encode jenkins_username}:#{ERB::Util.url_encode jenkins_password}@jenkins.example.com/project/my_project" } diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index e80fa6e3b70..6ce84c28044 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -130,6 +130,23 @@ RSpec.describe Integrations::Jira do end end + describe '.valid_jira_cloud_url?' do + using RSpec::Parameterized::TableSyntax + + where(:url, :result) do + 'https://abc.atlassian.net' | true + 'abc.atlassian.net' | false # This is how it behaves currently, but we may need to consider adding scheme if missing + 'https://somethingelse.com' | false + nil | false + end + + with_them do + specify do + expect(described_class.valid_jira_cloud_url?(url)).to eq(result) + end + end + end + describe '#create' do let(:params) do { diff --git a/spec/models/integrations/mock_ci_spec.rb b/spec/models/integrations/mock_ci_spec.rb new file mode 100644 index 00000000000..d29c63b3a97 --- /dev/null +++ b/spec/models/integrations/mock_ci_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::MockCi do + let_it_be(:project) { build(:project) } + + subject(:integration) { described_class.new(project: project, mock_service_url: generate(:url)) } + + include_context Integrations::EnableSslVerification + + describe '#commit_status' do + let(:sha) { generate(:sha) } + + def stub_request(*args) + WebMock.stub_request(:get, integration.commit_status_path(sha)).to_return(*args) + end + + def commit_status + integration.commit_status(sha, 'master') + end + + it 'returns allowed states' do + described_class::ALLOWED_STATES.each do |state| + stub_request(status: 200, body: { status: state }.to_json) + + expect(commit_status).to eq(state) + end + end + + it 'returns :pending for 404 responses' do + stub_request(status: 404) + + expect(commit_status).to eq(:pending) + end + + it 'returns :error for responses other than 200 or 404' do + stub_request(status: 500) + + expect(commit_status).to eq(:error) + end + + it 'returns :error for unknown states' do + stub_request(status: 200, body: { status: 'unknown' }.to_json) + + expect(commit_status).to eq(:error) + end + + it 'returns :error for invalid JSON' do + stub_request(status: 200, body: '') + + expect(commit_status).to eq(:error) + end + + it 'returns :error for non-hash JSON responses' do + stub_request(status: 200, body: 23.to_json) + + expect(commit_status).to eq(:error) + end + + it 'returns :error for JSON responses without a status' do + stub_request(status: 200, body: { foo: :bar }.to_json) + + expect(commit_status).to eq(:error) + end + + it 'returns :error when connection is refused' do + stub_request(status: 500).to_raise(Errno::ECONNREFUSED) + + expect(commit_status).to eq(:error) + end + end +end diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index 0713141ea08..e1f4e577503 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -6,8 +6,8 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers include StubRequests - let(:teamcity_url) { 'http://gitlab.com/teamcity' } - let(:teamcity_full_url) { 'http://gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' } + let(:teamcity_url) { 'https://gitlab.teamcity.com' } + let(:teamcity_full_url) { 'https://gitlab.teamcity.com/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' } let(:project) { create(:project) } subject(:integration) do @@ -22,6 +22,52 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do ) end + include_context Integrations::EnableSslVerification do + describe '#enable_ssl_verification' do + before do + allow(integration).to receive(:new_record?).and_return(false) + end + + it 'returns true for a known hostname' do + integration.teamcity_url = 'https://example.teamcity.com' + + expect(integration.enable_ssl_verification).to be(true) + end + + it 'returns true for new records' do + allow(integration).to receive(:new_record?).and_return(true) + integration.teamcity_url = 'http://example.com' + + expect(integration.enable_ssl_verification).to be(true) + end + + it 'returns false for an unknown hostname' do + integration.teamcity_url = 'https://sub.example.teamcity.com' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns false for a HTTP URL' do + integration.teamcity_url = 'http://example.teamcity.com' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns false for an invalid URL' do + integration.teamcity_url = 'https://example.com:foo' + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'returns the persisted value if present' do + integration.teamcity_url = 'https://example.teamcity.com' + integration.enable_ssl_verification = false + + expect(integration.enable_ssl_verification).to be(false) + end + end + end + describe 'Validations' do context 'when integration is active' do before do @@ -140,22 +186,22 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do it 'returns a specific URL when status is 500' do stub_request(status: 500) - is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo') + is_expected.to eq("#{teamcity_url}/viewLog.html?buildTypeId=foo") end it 'returns a build URL when teamcity_url has no trailing slash' do stub_request(body: %q({"build":{"id":"666"}})) - is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo') + is_expected.to eq("#{teamcity_url}/viewLog.html?buildId=666&buildTypeId=foo") end context 'teamcity_url has trailing slash' do - let(:teamcity_url) { 'http://gitlab.com/teamcity/' } + let(:teamcity_url) { 'https://gitlab.teamcity.com/' } it 'returns a build URL' do stub_request(body: %q({"build":{"id":"666"}})) - is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo') + is_expected.to eq('https://gitlab.teamcity.com/viewLog.html?buildId=666&buildTypeId=foo') end end @@ -299,7 +345,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do end def stub_post_to_build_queue(branch:) - teamcity_full_url = 'http://gitlab.com/teamcity/httpAuth/app/rest/buildQueue' + teamcity_full_url = "#{teamcity_url}/httpAuth/app/rest/buildQueue" body ||= %Q(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>) auth = %w(mic password) diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 122340f7bec..0cb92f81da2 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1201,6 +1201,30 @@ RSpec.describe Packages::Package, type: :model do end end + describe '#mark_package_files_for_destruction' do + let_it_be(:package) { create(:npm_package, :pending_destruction) } + + subject { package.mark_package_files_for_destruction } + + it 'enqueues a sync worker job' do + expect(::Packages::MarkPackageFilesForDestructionWorker) + .to receive(:perform_async).with(package.id) + + subject + end + + context 'for a package non pending destruction' do + let_it_be(:package) { create(:npm_package) } + + it 'does not enqueues a sync worker job' do + expect(::Packages::MarkPackageFilesForDestructionWorker) + .not_to receive(:perform_async).with(package.id) + + subject + end + end + end + describe '#create_build_infos!' do let_it_be(:package) { create(:package) } let_it_be(:pipeline) { create(:ci_pipeline) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ac2474ac393..c2535fd3698 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2582,6 +2582,12 @@ RSpec.describe User do describe '.search' do let_it_be(:user) { create(:user, name: 'user', username: 'usern', email: 'email@example.com') } + let_it_be(:public_email) do + create(:email, :confirmed, user: user, email: 'publicemail@example.com').tap do |email| + user.update!(public_email: email.email) + end + end + let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') } let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') } let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') } @@ -2609,30 +2615,31 @@ RSpec.describe User do end describe 'email matching' do - it 'returns users with a matching Email' do - expect(described_class.search(user.email)).to eq([user]) + it 'returns users with a matching public email' do + expect(described_class.search(user.public_email)).to match_array([user]) end - it 'does not return users with a partially matching Email' do - expect(described_class.search(user.email[1...-1])).to be_empty + it 'does not return users with a partially matching public email' do + expect(described_class.search(user.public_email[1...-1])).to be_empty end - it 'returns users with a matching Email regardless of the casing' do - expect(described_class.search(user2.email.upcase)).to eq([user2]) + it 'returns users with a matching public email regardless of the casing' do + expect(described_class.search(user.public_email.upcase)).to match_array([user]) end - end - describe 'secondary email matching' do - it 'returns users with a matching secondary email' do - expect(described_class.search(email.email)).to include(email.user) + it 'does not return users with a matching private email' do + expect(described_class.search(user.email)).to be_empty + expect(described_class.search(email.email)).to be_empty end - it 'does not return users with a matching part of secondary email' do - expect(described_class.search(email.email[1...-1])).to be_empty - end + context 'with private emails search' do + it 'returns users with matching private email' do + expect(described_class.search(user.email, with_private_emails: true)).to match_array([user]) + end - it 'returns users with a matching secondary email regardless of the casing' do - expect(described_class.search(email.email.upcase)).to include(email.user) + it 'returns users with matching private secondary email' do + expect(described_class.search(email.email, with_private_emails: true)).to match_array([user]) + end end end @@ -2733,6 +2740,80 @@ RSpec.describe User do end end + describe '.search_with_public_emails' do + let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) } + let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) } + let_it_be(:public_email) do + create(:email, :confirmed, user: another_user, email: 'alias@example.com').tap do |email| + another_user.update!(public_email: email.email) + end + end + + let_it_be(:secondary_email) do + create(:email, :confirmed, user: another_user, email: 'secondary@example.com') + end + + it 'returns users with a matching name' do + expect(described_class.search_with_public_emails(user.name)).to match_array([user]) + end + + it 'returns users with a partially matching name' do + expect(described_class.search_with_public_emails(user.name[0..2])).to match_array([user]) + end + + it 'returns users with a matching name regardless of the casing' do + expect(described_class.search_with_public_emails(user.name.upcase)).to match_array([user]) + end + + it 'returns users with a matching public email' do + expect(described_class.search_with_public_emails(another_user.public_email)).to match_array([another_user]) + end + + it 'does not return users with a partially matching email' do + expect(described_class.search_with_public_emails(another_user.public_email[1...-1])).to be_empty + end + + it 'returns users with a matching email regardless of the casing' do + expect(described_class.search_with_public_emails(another_user.public_email.upcase)).to match_array([another_user]) + end + + it 'returns users with a matching username' do + expect(described_class.search_with_public_emails(user.username)).to match_array([user]) + end + + it 'returns users with a partially matching username' do + expect(described_class.search_with_public_emails(user.username[0..2])).to match_array([user]) + end + + it 'returns users with a matching username regardless of the casing' do + expect(described_class.search_with_public_emails(user.username.upcase)).to match_array([user]) + end + + it 'does not return users with a matching whole private email' do + expect(described_class.search_with_public_emails(user.email)).not_to include(user) + end + + it 'does not return users with a matching whole private email' do + expect(described_class.search_with_public_emails(secondary_email.email)).to be_empty + end + + it 'does not return users with a matching part of secondary email' do + expect(described_class.search_with_public_emails(secondary_email.email[1...-1])).to be_empty + end + + it 'does not return users with a matching part of private email' do + expect(described_class.search_with_public_emails(user.email[1...-1])).to be_empty + end + + it 'returns no matches for an empty string' do + expect(described_class.search_with_public_emails('')).to be_empty + end + + it 'returns no matches for nil' do + expect(described_class.search_with_public_emails(nil)).to be_empty + end + end + describe '.search_with_secondary_emails' do let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) } let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) } diff --git a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb index 7be629f8f4b..cd25aba9e00 100644 --- a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb +++ b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb @@ -24,19 +24,16 @@ RSpec.describe 'Destroying a package file' do let(:mutation_response) { graphql_mutation_response(:destroyPackageFile) } shared_examples 'destroying the package file' do - it 'destroy the package file' do - expect { mutation_request }.to change { ::Packages::PackageFile.count }.by(-1) + it 'marks the package file as pending destruction' do + expect { mutation_request }.to change { ::Packages::PackageFile.pending_destruction.count }.by(1) end it_behaves_like 'returning response status', :success end shared_examples 'denying the mutation request' do - it 'does not destroy the package file' do - expect(::Packages::PackageFile) - .not_to receive(:destroy) - - expect { mutation_request }.not_to change { ::Packages::PackageFile.count } + it 'does not mark the package file as pending destruction' do + expect { mutation_request }.not_to change { ::Packages::PackageFile.pending_destruction.count } expect(mutation_response).to be_nil end @@ -71,7 +68,7 @@ RSpec.describe 'Destroying a package file' do it_behaves_like 'denying the mutation request' end - context 'when an error occures' do + context 'when an error occurs' do let(:error_messages) { ['some error'] } before do @@ -80,7 +77,7 @@ RSpec.describe 'Destroying a package file' do it 'returns the errors in the response' do allow_next_found_instance_of(::Packages::PackageFile) do |package_file| - allow(package_file).to receive(:destroy).and_return(false) + allow(package_file).to receive(:update).with(status: :pending_destruction).and_return(false) allow(package_file).to receive_message_chain(:errors, :full_messages).and_return(error_messages) end diff --git a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb index e5ced419ecf..2340a6a36d8 100644 --- a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb @@ -24,22 +24,27 @@ RSpec.describe 'Destroying a package' do let(:mutation_response) { graphql_mutation_response(:destroyPackage) } shared_examples 'destroying the package' do - it 'destroy the package' do - expect(::Packages::DestroyPackageService) + it 'marks the package as pending destruction' do + expect(::Packages::MarkPackageForDestructionService) .to receive(:new).with(container: package, current_user: user).and_call_original + expect_next_found_instance_of(::Packages::Package) do |package| + expect(package).to receive(:mark_package_files_for_destruction) + end - expect { mutation_request }.to change { ::Packages::Package.count }.by(-1) + expect { mutation_request } + .to change { ::Packages::Package.pending_destruction.count }.by(1) end it_behaves_like 'returning response status', :success end shared_examples 'denying the mutation request' do - it 'does not destroy the package' do - expect(::Packages::DestroyPackageService) + it 'does not mark the package as pending destruction' do + expect(::Packages::MarkPackageForDestructionService) .not_to receive(:new).with(container: package, current_user: user) - expect { mutation_request }.not_to change { ::Packages::Package.count } + expect { mutation_request } + .to not_change { ::Packages::Package.pending_destruction.count } expect(mutation_response).to be_nil end @@ -81,12 +86,12 @@ RSpec.describe 'Destroying a package' do it 'returns the errors in the response' do allow_next_found_instance_of(::Packages::Package) do |package| - allow(package).to receive(:destroy!).and_raise(StandardError) + allow(package).to receive(:pending_destruction!).and_raise(StandardError) end mutation_request - expect(mutation_response['errors']).to eq(['Failed to remove the package']) + expect(mutation_response['errors']).to match_array(['Failed to mark the package as pending destruction']) end end end diff --git a/spec/requests/api/graphql/project/cluster_agents_spec.rb b/spec/requests/api/graphql/project/cluster_agents_spec.rb index 585126f3849..c9900fea277 100644 --- a/spec/requests/api/graphql/project/cluster_agents_spec.rb +++ b/spec/requests/api/graphql/project/cluster_agents_spec.rb @@ -126,7 +126,7 @@ RSpec.describe 'Project.cluster_agents' do }) end - it 'preloads associations to prevent N+1 queries' do + it 'preloads associations to prevent N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350868' do user = create(:user) token = create(:cluster_agent_token, agent: agents.second) create(:agent_activity_event, agent: agents.second, agent_token: token, user: user) diff --git a/spec/requests/api/package_files_spec.rb b/spec/requests/api/package_files_spec.rb index 7a6b1599154..a7e6a97fd0e 100644 --- a/spec/requests/api/package_files_spec.rb +++ b/spec/requests/api/package_files_spec.rb @@ -114,14 +114,14 @@ RSpec.describe API::PackageFiles do let(:user) { nil } it 'returns 403 for non authenticated user', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end end it 'returns 403 for a user without access to the project', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end @@ -131,7 +131,7 @@ RSpec.describe API::PackageFiles do let_it_be_with_refind(:project) { create(:project, :private) } it 'returns 404 for a user without access to the project', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -139,7 +139,7 @@ RSpec.describe API::PackageFiles do it 'returns 403 for a user without enough permissions', :aggregate_failures do project.add_developer(user) - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end @@ -147,7 +147,7 @@ RSpec.describe API::PackageFiles do it 'returns 204', :aggregate_failures do project.add_maintainer(user) - expect { api_request }.to change { package.package_files.count }.by(-1) + expect { api_request }.to change { package.package_files.pending_destruction.count }.by(1) expect(response).to have_gitlab_http_status(:no_content) end @@ -156,7 +156,7 @@ RSpec.describe API::PackageFiles do let(:user) { nil } it 'returns 404 for non authenticated user', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -168,7 +168,7 @@ RSpec.describe API::PackageFiles do it 'returns 404 when the package file does not exist', :aggregate_failures do project.add_maintainer(user) - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -182,7 +182,7 @@ RSpec.describe API::PackageFiles do end it 'can not be accessed', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.count } + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -193,7 +193,7 @@ RSpec.describe API::PackageFiles do end it 'can be accessed', :aggregate_failures do - expect { api_request }.to change { package.package_files.count }.by(-1) + expect { api_request }.not_to change { package.package_files.pending_destruction.count } expect(response).to have_gitlab_http_status(:no_content) end diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 9b7538547f6..5f4b8899a33 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -293,13 +293,13 @@ RSpec.describe API::ProjectPackages do context 'without the need for a license' do context 'project is public' do it 'returns 403 for non authenticated user' do - delete api(package_url) + expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end it 'returns 403 for a user without access to the project' do - delete api(package_url, user) + expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end @@ -313,13 +313,13 @@ RSpec.describe API::ProjectPackages do end it 'returns 404 for non authenticated user' do - delete api(package_url) + expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end it 'returns 404 for a user without access to the project' do - delete api(package_url, user) + expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -327,7 +327,7 @@ RSpec.describe API::ProjectPackages do it 'returns 404 when the package does not exist' do project.add_maintainer(user) - delete api(no_package_url, user) + expect { delete api(no_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -335,7 +335,7 @@ RSpec.describe API::ProjectPackages do it 'returns 404 for the package from a different project' do project.add_maintainer(user) - delete api(wrong_package_url, user) + expect { delete api(wrong_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:not_found) end @@ -343,7 +343,7 @@ RSpec.describe API::ProjectPackages do it 'returns 403 for a user without enough permissions' do project.add_developer(user) - delete api(package_url, user) + expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count } expect(response).to have_gitlab_http_status(:forbidden) end @@ -351,7 +351,7 @@ RSpec.describe API::ProjectPackages do it 'returns 204' do project.add_maintainer(user) - delete api(package_url, user) + expect { delete api(package_url, user) }.to change { ::Packages::Package.pending_destruction.count }.by(1) expect(response).to have_gitlab_http_status(:no_content) end diff --git a/spec/requests/jira_connect/users_controller_spec.rb b/spec/requests/jira_connect/users_controller_spec.rb new file mode 100644 index 00000000000..c648d28c1bc --- /dev/null +++ b/spec/requests/jira_connect/users_controller_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::UsersController do + describe 'GET /-/jira_connect/users' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + context 'with a valid host' do + let(:return_to) { 'https://testcompany.atlassian.net/plugins/servlet/ac/gitlab-jira-connect-staging.gitlab.com/gitlab-configuration' } + + it 'includes a return url' do + get '/-/jira_connect/users', params: { return_to: return_to } + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to include('Return to GitLab') + end + end + + context 'with an invalid host' do + let(:return_to) { 'https://evil.com' } + + it 'does not include a return url' do + get '/-/jira_connect/users', params: { return_to: return_to } + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).not_to include('Return to GitLab') + end + end + end +end diff --git a/spec/services/packages/mark_package_files_for_destruction_service_spec.rb b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb new file mode 100644 index 00000000000..a836de1f7f6 --- /dev/null +++ b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::MarkPackageFilesForDestructionService, :aggregate_failures do + let(:service) { described_class.new(package_files) } + + describe '#execute', :aggregate_failures do + subject { service.execute } + + shared_examples 'executing successfully' do + it 'marks package files for destruction' do + expect { subject } + .to change { ::Packages::PackageFile.pending_destruction.count }.by(package_files.size) + end + + it 'executes successfully' do + expect(subject).to be_success + expect(subject.message).to eq('Package files are now pending destruction') + end + end + + context 'with no package files' do + let_it_be(:package_files) { ::Packages::PackageFile.none } + + it_behaves_like 'executing successfully' + end + + context 'with a single package file' do + let_it_be(:package_file) { create(:package_file) } + let_it_be(:package_files) { ::Packages::PackageFile.id_in(package_file.id) } + + it_behaves_like 'executing successfully' + end + + context 'with many package files' do + let_it_be(:package_files) { ::Packages::PackageFile.id_in(create_list(:package_file, 3).map(&:id)) } + + it_behaves_like 'executing successfully' + end + + context 'with an error during the update' do + let_it_be(:package_files) { ::Packages::PackageFile.none } + + before do + expect(package_files).to receive(:each_batch).and_raise('error!') + end + + it 'raises the error' do + expect { subject } + .to raise_error('error!') + .and not_change { ::Packages::PackageFile.pending_destruction.count } + end + end + end +end diff --git a/spec/services/packages/destroy_package_service_spec.rb b/spec/services/packages/mark_package_for_destruction_service_spec.rb index 92db8da968c..125ec53ad61 100644 --- a/spec/services/packages/destroy_package_service_spec.rb +++ b/spec/services/packages/mark_package_for_destruction_service_spec.rb @@ -2,10 +2,9 @@ require 'spec_helper' -RSpec.describe Packages::DestroyPackageService do +RSpec.describe Packages::MarkPackageForDestructionService do let_it_be(:user) { create(:user) } - - let!(:package) { create(:npm_package) } + let_it_be_with_reload(:package) { create(:npm_package) } describe '#execute' do subject(:service) { described_class.new(container: package, current_user: user) } @@ -15,10 +14,11 @@ RSpec.describe Packages::DestroyPackageService do package.project.add_maintainer(user) end - context 'when the destroy is successfull' do - it 'destroy the package' do + context 'when it is successful' do + it 'marks the package and package files as pending destruction' do expect(package).to receive(:sync_maven_metadata).and_call_original - expect { service.execute }.to change { Packages::Package.count }.by(-1) + expect(package).to receive(:mark_package_files_for_destruction).and_call_original + expect { service.execute }.to change { package.status }.from('default').to('pending_destruction') end it 'returns a success ServiceResponse' do @@ -26,13 +26,13 @@ RSpec.describe Packages::DestroyPackageService do expect(response).to be_a(ServiceResponse) expect(response).to be_success - expect(response.message).to eq("Package was successfully deleted") + expect(response.message).to eq("Package was successfully marked as pending destruction") end end - context 'when the destroy is not successful' do + context 'when it is not successful' do before do - allow(package).to receive(:destroy!).and_raise(StandardError, "test") + allow(package).to receive(:pending_destruction!).and_raise(StandardError, "test") end it 'returns an error ServiceResponse' do @@ -41,7 +41,7 @@ RSpec.describe Packages::DestroyPackageService do expect(package).not_to receive(:sync_maven_metadata) expect(response).to be_a(ServiceResponse) expect(response).to be_error - expect(response.message).to eq("Failed to remove the package") + expect(response.message).to eq("Failed to mark the package as pending destruction") expect(response.status).to eq(:error) end end diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index 0bea3edf203..3ac42d41377 100644 --- a/spec/services/protected_branches/create_service_spec.rb +++ b/spec/services/protected_branches/create_service_spec.rb @@ -24,38 +24,14 @@ RSpec.describe ProtectedBranches::CreateService do expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end - context 'when name has escaped HTML' do - let(:name) { 'feature->test' } + context 'when protecting a branch with a name that contains HTML tags' do + let(:name) { 'foo<b>bar<\b>' } - it 'creates the new protected branch matching the unescaped version' do - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(project.protected_branches.last.name).to eq('feature->test') - end - - context 'and name contains HTML tags' do - let(:name) { '<b>master</b>' } - - it 'creates the new protected branch with sanitized name' do - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(project.protected_branches.last.name).to eq('master') - end - - context 'and contains unsafe HTML' do - let(:name) { '<script>alert('foo');</script>' } + subject(:service) { described_class.new(project, user, params) } - it 'does not create the new protected branch' do - expect { service.execute }.not_to change(ProtectedBranch, :count) - end - end - end - - context 'when name contains unescaped HTML tags' do - let(:name) { '<b>master</b>' } - - it 'creates the new protected branch with sanitized name' do - expect { service.execute }.to change(ProtectedBranch, :count).by(1) - expect(project.protected_branches.last.name).to eq('master') - end + it 'creates a new protected branch' do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(project.protected_branches.last.name).to eq(name) end end diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 3d9b77dcfc0..4405af35c37 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -18,35 +18,14 @@ RSpec.describe ProtectedBranches::UpdateService do expect(result.reload.name).to eq(params[:name]) end - context 'when name has escaped HTML' do - let(:new_name) { 'feature->test' } + context 'when updating name of a protected branch to one that contains HTML tags' do + let(:new_name) { 'foo<b>bar<\b>' } + let(:result) { service.execute(protected_branch) } - it 'updates protected branch name with unescaped HTML' do - expect(result.reload.name).to eq('feature->test') - end - - context 'and name contains HTML tags' do - let(:new_name) { '<b>master</b>' } - - it 'updates protected branch name with sanitized name' do - expect(result.reload.name).to eq('master') - end - - context 'and contains unsafe HTML' do - let(:new_name) { '<script>alert('foo');</script>' } - - it 'does not update the protected branch' do - expect(result.reload.name).to eq(protected_branch.name) - end - end - end - end - - context 'when name contains unescaped HTML tags' do - let(:new_name) { '<b>master</b>' } + subject(:service) { described_class.new(project, user, params) } - it 'updates protected branch name with sanitized name' do - expect(result.reload.name).to eq('master') + it 'updates a protected branch' do + expect(result.reload.name).to eq(new_name) end end diff --git a/spec/services/protected_tags/create_service_spec.rb b/spec/services/protected_tags/create_service_spec.rb index 31059d17f10..a0b99b595e3 100644 --- a/spec/services/protected_tags/create_service_spec.rb +++ b/spec/services/protected_tags/create_service_spec.rb @@ -22,38 +22,14 @@ RSpec.describe ProtectedTags::CreateService do expect(project.protected_tags.last.create_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end - context 'when name has escaped HTML' do - let(:name) { 'tag->test' } + context 'protecting a tag with a name that contains HTML tags' do + let(:name) { 'foo<b>bar<\b>' } - it 'creates the new protected tag matching the unescaped version' do - expect { service.execute }.to change(ProtectedTag, :count).by(1) - expect(project.protected_tags.last.name).to eq('tag->test') - end - - context 'and name contains HTML tags' do - let(:name) { '<b>tag</b>' } - - it 'creates the new protected tag with sanitized name' do - expect { service.execute }.to change(ProtectedTag, :count).by(1) - expect(project.protected_tags.last.name).to eq('tag') - end - - context 'and contains unsafe HTML' do - let(:name) { '<script>alert('foo');</script>' } + subject(:service) { described_class.new(project, user, params) } - it 'does not create the new protected tag' do - expect { service.execute }.not_to change(ProtectedTag, :count) - end - end - end - - context 'when name contains unescaped HTML tags' do - let(:name) { '<b>tag</b>' } - - it 'creates the new protected tag with sanitized name' do - expect { service.execute }.to change(ProtectedTag, :count).by(1) - expect(project.protected_tags.last.name).to eq('tag') - end + it 'creates a new protected tag' do + expect { service.execute }.to change(ProtectedTag, :count).by(1) + expect(project.protected_tags.last.name).to eq(name) end end end diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb index 8d301dcd825..4b6e726bb6e 100644 --- a/spec/services/protected_tags/update_service_spec.rb +++ b/spec/services/protected_tags/update_service_spec.rb @@ -18,35 +18,14 @@ RSpec.describe ProtectedTags::UpdateService do expect(result.reload.name).to eq(params[:name]) end - context 'when name has escaped HTML' do - let(:new_name) { 'tag->test' } + context 'when updating protected tag with a name that contains HTML tags' do + let(:new_name) { 'foo<b>bar<\b>' } + let(:result) { service.execute(protected_tag) } - it 'updates protected tag name with unescaped HTML' do - expect(result.reload.name).to eq('tag->test') - end - - context 'and name contains HTML tags' do - let(:new_name) { '<b>tag</b>' } - - it 'updates protected tag name with sanitized name' do - expect(result.reload.name).to eq('tag') - end - - context 'and contains unsafe HTML' do - let(:new_name) { '<script>alert('foo');</script>' } - - it 'does not update the protected tag' do - expect(result.reload.name).to eq(protected_tag.name) - end - end - end - end - - context 'when name contains unescaped HTML tags' do - let(:new_name) { '<b>tag</b>' } + subject(:service) { described_class.new(project, user, params) } - it 'updates protected tag name with sanitized name' do - expect(result.reload.name).to eq('tag') + it 'updates a protected tag' do + expect(result.reload.name).to eq(new_name) end end diff --git a/spec/support/helpers/dns_helpers.rb b/spec/support/helpers/dns_helpers.rb index ba32ccbb6f1..b941e7c4808 100644 --- a/spec/support/helpers/dns_helpers.rb +++ b/spec/support/helpers/dns_helpers.rb @@ -23,7 +23,15 @@ module DnsHelpers end def permit_local_dns! - local_addresses = /\A(127|10)\.0\.0\.\d{1,3}|(192\.168|172\.16)\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost\z/i + local_addresses = %r{ + \A + ::1? | # IPV6 + (127|10)\.0\.0\.\d{1,3} | # 127.0.0.x or 10.0.0.x local network + (192\.168|172\.16)\.\d{1,3}\.\d{1,3} | # 192.168.x.x or 172.16.x.x local network + 0\.0\.0\.0 | # loopback + localhost + \z + }xi allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM).and_call_original allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM, anything, anything, any_args).and_call_original end diff --git a/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb b/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb new file mode 100644 index 00000000000..c698e06c2a2 --- /dev/null +++ b/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +RSpec.shared_context Integrations::EnableSslVerification do + # This is added to the global setup, to make sure all calls to + # `Gitlab::HTTP` in the main model spec are passing the `verify:` option. + before do + allow(Gitlab::HTTP).to receive(:perform_request) + .with(anything, anything, include(verify: true)) + .and_call_original + end + + describe 'accessors' do + it { is_expected.to respond_to(:enable_ssl_verification) } + it { is_expected.to respond_to(:enable_ssl_verification?) } + end + + describe '#initialize_properties' do + it 'enables the setting by default' do + expect(integration.enable_ssl_verification).to be(true) + end + + it 'does not enable the setting if the record is already persisted' do + allow(integration).to receive(:new_record?).and_return(false) + + integration.enable_ssl_verification = false + integration.send(:initialize_properties) + + expect(integration.enable_ssl_verification).to be(false) + end + + it 'does not enable the setting if a custom value was set' do + integration = described_class.new(enable_ssl_verification: false) + + expect(integration.enable_ssl_verification).to be(false) + end + end + + describe '#fields' do + it 'inserts the checkbox field after the first URL field, or at the end' do + names = integration.fields.pluck(:name) + url_index = names.index { |name| name.ends_with?('_url') } + insert_index = url_index ? url_index + 1 : names.size - 1 + + expect(names.index('enable_ssl_verification')).to eq insert_index + end + end +end diff --git a/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb index 1fa340a0cf4..ae72cb6ec5d 100644 --- a/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb +++ b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb @@ -37,6 +37,16 @@ RSpec.shared_examples Integrations::HasWebHook do it 'returns a boolean' do expect(integration.hook_ssl_verification).to be_in([true, false]) end + + it 'delegates to #enable_ssl_verification if the concern is included' do + next unless integration.is_a?(Integrations::EnableSslVerification) + + [true, false].each do |value| + integration.enable_ssl_verification = value + + expect(integration.hook_ssl_verification).to be(value) + end + end end describe '#update_web_hook!' do diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index bb4e2981070..4f9c207f976 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -364,6 +364,7 @@ RSpec.describe 'Every Sidekiq worker' do 'Packages::CleanupPackageFileWorker' => 0, 'Packages::Composer::CacheUpdateWorker' => false, 'Packages::Go::SyncPackagesWorker' => 3, + 'Packages::MarkPackageFilesForDestructionWorker' => 3, 'Packages::Maven::Metadata::SyncWorker' => 3, 'Packages::Nuget::ExtractionWorker' => 3, 'Packages::Rubygems::ExtractionWorker' => 3, diff --git a/spec/workers/irker_worker_spec.rb b/spec/workers/irker_worker_spec.rb index aa1f1d2fe1d..c3d40ad2783 100644 --- a/spec/workers/irker_worker_spec.rb +++ b/spec/workers/irker_worker_spec.rb @@ -21,7 +21,7 @@ RSpec.describe IrkerWorker, '#perform' do channels, false, push_data, - server_settings + HashWithIndifferentAccess.new(server_settings) ] end @@ -35,6 +35,14 @@ RSpec.describe IrkerWorker, '#perform' do allow(tcp_socket).to receive(:close).and_return(true) end + context 'local requests are not allowed' do + before do + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false) + end + + it { expect(worker.perform(*arguments)).to be_falsey } + end + context 'connection fails' do before do allow(TCPSocket).to receive(:new).and_raise(Errno::ECONNREFUSED.new('test')) @@ -44,6 +52,11 @@ RSpec.describe IrkerWorker, '#perform' do end context 'connection successful' do + before do + allow(Gitlab::CurrentSettings) + .to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true) + end + it { expect(subject.perform(*arguments)).to be_truthy } context 'new branch' do diff --git a/spec/workers/packages/cleanup_package_file_worker_spec.rb b/spec/workers/packages/cleanup_package_file_worker_spec.rb index b423c4d3f06..33f89826312 100644 --- a/spec/workers/packages/cleanup_package_file_worker_spec.rb +++ b/spec/workers/packages/cleanup_package_file_worker_spec.rb @@ -23,6 +23,7 @@ RSpec.describe Packages::CleanupPackageFileWorker do expect(worker).to receive(:log_extra_metadata_on_done).twice expect { subject }.to change { Packages::PackageFile.count }.by(-1) + .and not_change { Packages::Package.count } end end @@ -38,6 +39,17 @@ RSpec.describe Packages::CleanupPackageFileWorker do expect(package_file.reload).to be_error end end + + context 'removing the last package file' do + let_it_be(:package_file) { create(:package_file, :pending_destruction, package: package) } + + it 'deletes the package file and the package' do + expect(worker).to receive(:log_extra_metadata_on_done).twice + + expect { subject }.to change { Packages::PackageFile.count }.by(-1) + .and change { Packages::Package.count }.by(-1) + end + end end describe '#max_running_jobs' do diff --git a/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb b/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb new file mode 100644 index 00000000000..15d9e4c347b --- /dev/null +++ b/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::MarkPackageFilesForDestructionWorker, :aggregate_failures do + describe '#perform' do + let_it_be(:package) { create(:package) } + let_it_be(:package_files) { create_list(:package_file, 3, package: package) } + + let(:worker) { described_class.new } + let(:job_args) { [package.id] } + + subject { worker.perform(*job_args) } + + context 'with a valid package id' do + it_behaves_like 'an idempotent worker' + + it 'marks all package files as pending_destruction' do + package_files_count = package.package_files.count + + expect(package.package_files.pending_destruction.count).to eq(0) + expect(package.package_files.default.count).to eq(package_files_count) + + subject + + expect(package.package_files.default.count).to eq(0) + expect(package.package_files.pending_destruction.count).to eq(package_files_count) + end + end + + context 'with an invalid package id' do + let(:job_args) { [non_existing_record_id] } + + it_behaves_like 'an idempotent worker' + + it 'marks no packag files' do + expect(::Packages::MarkPackageFilesForDestructionService).not_to receive(:new) + + expect { subject }.not_to change { ::Packages::PackageFile.pending_destruction.count } + end + end + + context 'with a nil package id' do + let(:job_args) { [nil] } + + it_behaves_like 'an idempotent worker' + + it 'marks no packag files' do + expect(::Packages::MarkPackageFilesForDestructionService).not_to receive(:new) + + expect { subject }.not_to change { ::Packages::PackageFile.pending_destruction.count } + end + end + end +end |