From 6e4e1050d9dba2b7b2523fdd1768823ab85feef4 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 20 Aug 2020 18:42:06 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-3-stable-ee --- .../api/entities/nuget/dependency_group_spec.rb | 1 + spec/lib/api/entities/nuget/dependency_spec.rb | 1 + spec/lib/api/entities/nuget/metadatum_spec.rb | 2 + spec/lib/api/entities/nuget/search_result_spec.rb | 1 + spec/lib/api/entities/snippet_spec.rb | 8 +- .../lib/api/helpers/merge_requests_helpers_spec.rb | 63 ++++++++++++++++ .../packages_manager_clients_helpers_spec.rb | 34 --------- spec/lib/api/helpers_spec.rb | 86 ++++++++++++++++++++++ spec/lib/api/support/git_access_actor_spec.rb | 48 ++++++++++-- .../api/validations/validators/file_path_spec.rb | 73 +++++++++++++----- 10 files changed, 254 insertions(+), 63 deletions(-) create mode 100644 spec/lib/api/helpers/merge_requests_helpers_spec.rb (limited to 'spec/lib/api') diff --git a/spec/lib/api/entities/nuget/dependency_group_spec.rb b/spec/lib/api/entities/nuget/dependency_group_spec.rb index 5a649be846b..5e6de45adf2 100644 --- a/spec/lib/api/entities/nuget/dependency_group_spec.rb +++ b/spec/lib/api/entities/nuget/dependency_group_spec.rb @@ -34,6 +34,7 @@ RSpec.describe API::Entities::Nuget::DependencyGroup do ] } end + let(:entity) { described_class.new(dependency_group) } subject { entity.as_json } diff --git a/spec/lib/api/entities/nuget/dependency_spec.rb b/spec/lib/api/entities/nuget/dependency_spec.rb index 13897cc91f0..fb87b21bd1e 100644 --- a/spec/lib/api/entities/nuget/dependency_spec.rb +++ b/spec/lib/api/entities/nuget/dependency_spec.rb @@ -20,6 +20,7 @@ RSpec.describe API::Entities::Nuget::Dependency do 'range': '2.0.0' } end + let(:entity) { described_class.new(dependency) } subject { entity.as_json } diff --git a/spec/lib/api/entities/nuget/metadatum_spec.rb b/spec/lib/api/entities/nuget/metadatum_spec.rb index fe94ea3a69a..210ff0abdd3 100644 --- a/spec/lib/api/entities/nuget/metadatum_spec.rb +++ b/spec/lib/api/entities/nuget/metadatum_spec.rb @@ -10,6 +10,7 @@ RSpec.describe API::Entities::Nuget::Metadatum do icon_url: 'http://sandbox.com/icon' } end + let(:expected) do { 'projectUrl': 'http://sandbox.com/project', @@ -17,6 +18,7 @@ RSpec.describe API::Entities::Nuget::Metadatum do 'iconUrl': 'http://sandbox.com/icon' } end + let(:entity) { described_class.new(metadatum) } subject { entity.as_json } diff --git a/spec/lib/api/entities/nuget/search_result_spec.rb b/spec/lib/api/entities/nuget/search_result_spec.rb index 2a760c70224..a24cd44be9e 100644 --- a/spec/lib/api/entities/nuget/search_result_spec.rb +++ b/spec/lib/api/entities/nuget/search_result_spec.rb @@ -27,6 +27,7 @@ RSpec.describe API::Entities::Nuget::SearchResult do } } end + let(:expected) do { '@type': 'Package', diff --git a/spec/lib/api/entities/snippet_spec.rb b/spec/lib/api/entities/snippet_spec.rb index bcb8c364392..068851f7f6c 100644 --- a/spec/lib/api/entities/snippet_spec.rb +++ b/spec/lib/api/entities/snippet_spec.rb @@ -123,11 +123,11 @@ RSpec.describe ::API::Entities::Snippet do it_behaves_like 'common attributes' it 'returns snippet web_url attribute' do - expect(subject[:web_url]).to match("/snippets/#{snippet.id}") + expect(subject[:web_url]).to match("/-/snippets/#{snippet.id}") end it 'returns snippet raw_url attribute' do - expect(subject[:raw_url]).to match("/snippets/#{snippet.id}/raw") + expect(subject[:raw_url]).to match("/-/snippets/#{snippet.id}/raw") end end @@ -137,11 +137,11 @@ RSpec.describe ::API::Entities::Snippet do it_behaves_like 'common attributes' it 'returns snippet web_url attribute' do - expect(subject[:web_url]).to match("#{snippet.project.full_path}/snippets/#{snippet.id}") + expect(subject[:web_url]).to match("#{snippet.project.full_path}/-/snippets/#{snippet.id}") end it 'returns snippet raw_url attribute' do - expect(subject[:raw_url]).to match("#{snippet.project.full_path}/snippets/#{snippet.id}/raw") + expect(subject[:raw_url]).to match("#{snippet.project.full_path}/-/snippets/#{snippet.id}/raw") end end end diff --git a/spec/lib/api/helpers/merge_requests_helpers_spec.rb b/spec/lib/api/helpers/merge_requests_helpers_spec.rb new file mode 100644 index 00000000000..1d68b7985f1 --- /dev/null +++ b/spec/lib/api/helpers/merge_requests_helpers_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::MergeRequestsHelpers do + describe '#handle_merge_request_errors!' do + let(:helper) do + Class.new do + include API::Helpers::MergeRequestsHelpers + end.new + end + + let(:merge_request) { double } + + context 'when merge request is valid' do + it 'returns nil' do + allow(merge_request).to receive(:valid?).and_return(true) + + expect(merge_request).not_to receive(:errors) + + helper.handle_merge_request_errors!(merge_request) + end + end + + context 'when merge request is invalid' do + before do + allow(merge_request).to receive(:valid?).and_return(false) + allow(helper).to receive_messages([ + :unprocessable_entity!, :conflict!, :render_validation_error! + ]) + end + + API::Helpers::MergeRequestsHelpers::UNPROCESSABLE_ERROR_KEYS.each do |error_key| + it "responds to a #{error_key} error with unprocessable_entity" do + error = double + allow(merge_request).to receive(:errors).and_return({ error_key => error }) + + expect(helper).to receive(:unprocessable_entity!).with(error) + + helper.handle_merge_request_errors!(merge_request) + end + end + + it 'responds to a validate_branches error with conflict' do + error = double + allow(merge_request).to receive(:errors).and_return({ validate_branches: error }) + + expect(helper).to receive(:conflict!).with(error) + + helper.handle_merge_request_errors!(merge_request) + end + + it 'responds with bad request' do + error = double + allow(merge_request).to receive(:errors).and_return({ other_error: error }) + + expect(helper).to receive(:render_validation_error!).with(merge_request) + + helper.handle_merge_request_errors!(merge_request) + end + end + end +end diff --git a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb index 80be5f7d10a..832f4abe545 100644 --- a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb +++ b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb @@ -8,40 +8,6 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do let_it_be(:helper) { Class.new.include(described_class).new } let(:password) { personal_access_token.token } - describe '#find_personal_access_token_from_http_basic_auth' do - let(:headers) { { Authorization: basic_http_auth(username, password) } } - - subject { helper.find_personal_access_token_from_http_basic_auth } - - before do - allow(helper).to receive(:headers).and_return(headers&.with_indifferent_access) - end - - context 'with a valid Authorization header' do - it { is_expected.to eq personal_access_token } - end - - context 'with an invalid Authorization header' do - where(:headers) do - [ - [{ Authorization: 'Invalid' }], - [{}], - [nil] - ] - end - - with_them do - it { is_expected.to be nil } - end - end - - context 'with an unknown Authorization header' do - let(:password) { 'Unknown' } - - it { is_expected.to be nil } - end - end - describe '#find_job_from_http_basic_auth' do let_it_be(:user) { personal_access_token.user } diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index 8cba1e0794a..d0fe9163c6e 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -230,4 +230,90 @@ RSpec.describe API::Helpers do end end end + + describe "#destroy_conditionally!" do + let!(:project) { create(:project) } + + context 'when unmodified check passes' do + before do + allow(subject).to receive(:check_unmodified_since!).with(project.updated_at).and_return(true) + end + + it 'destroys given project' do + allow(subject).to receive(:status).with(204) + allow(subject).to receive(:body).with(false) + expect(project).to receive(:destroy).and_call_original + + expect { subject.destroy_conditionally!(project) }.to change(Project, :count).by(-1) + end + end + + context 'when unmodified check fails' do + before do + allow(subject).to receive(:check_unmodified_since!).with(project.updated_at).and_throw(:error) + end + + # #destroy_conditionally! uses Grape errors which Ruby-throws a symbol, shifting execution to somewhere else. + # Since this spec isn't in the Grape context, we need to simulate this ourselves. + # Grape throws here: https://github.com/ruby-grape/grape/blob/470f80cd48933cdf11d4c1ee02cb43e0f51a7300/lib/grape/dsl/inside_route.rb#L168-L171 + # And catches here: https://github.com/ruby-grape/grape/blob/cf57d250c3d77a9a488d9f56918d62fd4ac745ff/lib/grape/middleware/error.rb#L38-L40 + it 'does not destroy given project' do + expect(project).not_to receive(:destroy) + + expect { subject.destroy_conditionally!(project) }.to throw_symbol(:error).and change { Project.count }.by(0) + end + end + end + + describe "#check_unmodified_since!" do + let(:unmodified_since_header) { Time.now.change(usec: 0) } + + before do + allow(subject).to receive(:headers).and_return('If-Unmodified-Since' => unmodified_since_header.to_s) + end + + context 'when last modified is later than header value' do + it 'renders error' do + expect(subject).to receive(:render_api_error!) + + subject.check_unmodified_since!(unmodified_since_header + 1.hour) + end + end + + context 'when last modified is earlier than header value' do + it 'does not render error' do + expect(subject).not_to receive(:render_api_error!) + + subject.check_unmodified_since!(unmodified_since_header - 1.hour) + end + end + + context 'when last modified is equal to header value' do + it 'does not render error' do + expect(subject).not_to receive(:render_api_error!) + + subject.check_unmodified_since!(unmodified_since_header) + end + end + + context 'when there is no header value present' do + let(:unmodified_since_header) { nil } + + it 'does not render error' do + expect(subject).not_to receive(:render_api_error!) + + subject.check_unmodified_since!(Time.now) + end + end + + context 'when header value is not a valid time value' do + let(:unmodified_since_header) { "abcd" } + + it 'does not render error' do + expect(subject).not_to receive(:render_api_error!) + + subject.check_unmodified_since!(Time.now) + end + end + end end diff --git a/spec/lib/api/support/git_access_actor_spec.rb b/spec/lib/api/support/git_access_actor_spec.rb index 70753856419..143cc6e56ee 100644 --- a/spec/lib/api/support/git_access_actor_spec.rb +++ b/spec/lib/api/support/git_access_actor_spec.rb @@ -36,7 +36,7 @@ RSpec.describe API::Support::GitAccessActor do describe 'attributes' do describe '#user' do context 'when initialized with a User' do - let(:user) { create(:user) } + let(:user) { build(:user) } it 'returns the User' do expect(subject.user).to eq(user) @@ -44,7 +44,7 @@ RSpec.describe API::Support::GitAccessActor do end context 'when initialized with a Key' do - let(:user_for_key) { create(:user) } + let(:user_for_key) { build(:user) } let(:key) { create(:key, user: user_for_key) } it 'returns the User associated to the Key' do @@ -85,7 +85,7 @@ RSpec.describe API::Support::GitAccessActor do describe '#username' do context 'when initialized with a User' do - let(:user) { create(:user) } + let(:user) { build(:user) } it 'returns the username' do expect(subject.username).to eq(user.username) @@ -104,7 +104,7 @@ RSpec.describe API::Support::GitAccessActor do end context 'that has a User associated' do - let(:user_for_key) { create(:user) } + let(:user_for_key) { build(:user) } it 'returns the username of the User associated to the Key' do expect(subject.username).to eq(user_for_key.username) @@ -113,9 +113,47 @@ RSpec.describe API::Support::GitAccessActor do end end + describe '#key_details' do + context 'when initialized with a User' do + let(:user) { build(:user) } + + it 'returns an empty Hash' do + expect(subject.key_details).to eq({}) + end + end + + context 'when initialized with a Key' do + let(:key) { create(:key, user: user_for_key) } + + context 'that has no User associated' do + let(:user_for_key) { nil } + + it 'returns a Hash' do + expect(subject.key_details).to eq({ gl_key_type: 'key', gl_key_id: key.id }) + end + end + + context 'that has a User associated' do + let(:user_for_key) { build(:user) } + + it 'returns a Hash' do + expect(subject.key_details).to eq({ gl_key_type: 'key', gl_key_id: key.id }) + end + end + end + + context 'when initialized with a DeployKey' do + let(:key) { create(:deploy_key) } + + it 'returns a Hash' do + expect(subject.key_details).to eq({ gl_key_type: 'deploy_key', gl_key_id: key.id }) + end + end + end + describe '#update_last_used_at!' do context 'when initialized with a User' do - let(:user) { create(:user) } + let(:user) { build(:user) } it 'does nothing' do expect(user).not_to receive(:update_last_used_at) diff --git a/spec/lib/api/validations/validators/file_path_spec.rb b/spec/lib/api/validations/validators/file_path_spec.rb index 2c79260b8d5..cbeada6faa1 100644 --- a/spec/lib/api/validations/validators/file_path_spec.rb +++ b/spec/lib/api/validations/validators/file_path_spec.rb @@ -6,31 +6,64 @@ RSpec.describe API::Validations::Validators::FilePath do include ApiValidatorsHelpers subject do - described_class.new(['test'], {}, false, scope.new) + described_class.new(['test'], params, false, scope.new) end - context 'valid file path' do - it 'does not raise a validation error' do - expect_no_validation_error('test' => './foo') - expect_no_validation_error('test' => './bar.rb') - expect_no_validation_error('test' => 'foo%2Fbar%2Fnew%2Ffile.rb') - expect_no_validation_error('test' => 'foo%2Fbar%2Fnew') - expect_no_validation_error('test' => 'foo%252Fbar%252Fnew%252Ffile.rb') + context 'when allowlist is not set' do + shared_examples 'file validation' do + context 'valid file path' do + it 'does not raise a validation error' do + expect_no_validation_error('test' => './foo') + expect_no_validation_error('test' => './bar.rb') + expect_no_validation_error('test' => 'foo%2Fbar%2Fnew%2Ffile.rb') + expect_no_validation_error('test' => 'foo%2Fbar%2Fnew') + expect_no_validation_error('test' => 'foo/bar') + end + end + + context 'invalid file path' do + it 'raise a validation error' do + expect_validation_error('test' => '../foo') + expect_validation_error('test' => '../') + expect_validation_error('test' => 'foo/../../bar') + expect_validation_error('test' => 'foo/../') + expect_validation_error('test' => 'foo/..') + expect_validation_error('test' => '../') + expect_validation_error('test' => '..\\') + expect_validation_error('test' => '..\/') + expect_validation_error('test' => '%2e%2e%2f') + expect_validation_error('test' => '/etc/passwd') + expect_validation_error('test' => 'test%0a/etc/passwd') + expect_validation_error('test' => '%2Ffoo%2Fbar%2Fnew%2Ffile.rb') + expect_validation_error('test' => '%252Ffoo%252Fbar%252Fnew%252Ffile.rb') + expect_validation_error('test' => 'foo%252Fbar%252Fnew%252Ffile.rb') + expect_validation_error('test' => 'foo%25252Fbar%25252Fnew%25252Ffile.rb') + end + end + end + + it_behaves_like 'file validation' do + let(:params) { {} } + end + + it_behaves_like 'file validation' do + let(:params) { true } end end - context 'invalid file path' do - it 'raise a validation error' do - expect_validation_error('test' => '../foo') - expect_validation_error('test' => '../') - expect_validation_error('test' => 'foo/../../bar') - expect_validation_error('test' => 'foo/../') - expect_validation_error('test' => 'foo/..') - expect_validation_error('test' => '../') - expect_validation_error('test' => '..\\') - expect_validation_error('test' => '..\/') - expect_validation_error('test' => '%2e%2e%2f') - expect_validation_error('test' => '/etc/passwd') + context 'when allowlist is set' do + let(:params) { { allowlist: ['/home/bar'] } } + + context 'when file path is included in the allowlist' do + it 'does not raise a validation error' do + expect_no_validation_error('test' => '/home/bar') + end + end + + context 'when file path is not included in the allowlist' do + it 'raises a validation error' do + expect_validation_error('test' => '/foo/xyz') + end end end end -- cgit v1.2.3