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

10 files changed, 254 insertions, 63 deletions

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