Add latest changes from gitlab-org/gitlab@master

12 files changed, 299 insertions, 111 deletions

diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 87a30ed1234..31257fd3a30 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -673,14 +673,6 @@ RSpec.describe GroupsController, factory_default: :keep, feature_category: :code
       expect(controller).to set_flash[:notice]
     end
 
-    it 'does not update the path on error' do
-      allow_any_instance_of(Group).to receive(:move_dir).and_raise(Gitlab::UpdatePathError)
-      post :update, params: { id: group.to_param, group: { path: 'new_path' } }
-
-      expect(assigns(:group).errors).not_to be_empty
-      expect(assigns(:group).path).not_to eq('new_path')
-    end
-
     it 'updates the project_creation_level successfully' do
       post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } }
 
diff --git a/spec/frontend/search/sidebar/components/archived_filter_spec.js b/spec/frontend/search/sidebar/components/archived_filter_spec.js
index 69bf2ebd72e..a4c1f3b758f 100644
--- a/spec/frontend/search/sidebar/components/archived_filter_spec.js
+++ b/spec/frontend/search/sidebar/components/archived_filter_spec.js
@@ -12,9 +12,14 @@ Vue.use(Vuex);
 describe('ArchivedFilter', () => {
   let wrapper;
 
+  const defaultActions = {
+    setQuery: jest.fn(),
+  };
+
   const createComponent = (state) => {
     const store = new Vuex.Store({
       state,
+      actions: defaultActions,
     });
 
     wrapper = shallowMount(ArchivedFilter, {
@@ -70,4 +75,20 @@ describe('ArchivedFilter', () => {
       expect(findCheckboxFilter().attributes('checked')).toBe(checkboxState);
     });
   });
+
+  describe('selectedFilter logic', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('correctly executes setQuery without mutating the input', () => {
+      const selectedFilter = [false];
+      findCheckboxFilter().vm.$emit('input', selectedFilter);
+      expect(defaultActions.setQuery).toHaveBeenCalledWith(expect.any(Object), {
+        key: 'include_archived',
+        value: 'false',
+      });
+      expect(selectedFilter).toEqual([false]);
+    });
+  });
 });
diff --git a/spec/frontend/super_sidebar/components/create_menu_spec.js b/spec/frontend/super_sidebar/components/create_menu_spec.js
index b967fb18a39..ffbc789d220 100644
--- a/spec/frontend/super_sidebar/components/create_menu_spec.js
+++ b/spec/frontend/super_sidebar/components/create_menu_spec.js
@@ -47,7 +47,7 @@ describe('CreateMenu component', () => {
       createWrapper();
 
       expect(findGlDisclosureDropdown().props('dropdownOffset')).toEqual({
-        crossAxis: -179,
+        crossAxis: -177,
         mainAxis: 4,
       });
     });
@@ -98,7 +98,7 @@ describe('CreateMenu component', () => {
     createWrapper({ provide: { isImpersonating: true } });
 
     expect(findGlDisclosureDropdown().props('dropdownOffset')).toEqual({
-      crossAxis: -147,
+      crossAxis: -143,
       mainAxis: 4,
     });
   });
diff --git a/spec/frontend/super_sidebar/components/user_menu_spec.js b/spec/frontend/super_sidebar/components/user_menu_spec.js
index bcc3383bcd4..d41a414f69e 100644
--- a/spec/frontend/super_sidebar/components/user_menu_spec.js
+++ b/spec/frontend/super_sidebar/components/user_menu_spec.js
@@ -56,7 +56,7 @@ describe('UserMenu component', () => {
     createWrapper(null, null, { isImpersonating: true });
 
     expect(findDropdown().props('dropdownOffset')).toEqual({
-      crossAxis: -179,
+      crossAxis: -177,
       mainAxis: 4,
     });
   });
diff --git a/spec/lib/api/entities/namespace_basic_spec.rb b/spec/lib/api/entities/namespace_basic_spec.rb
new file mode 100644
index 00000000000..9a0352991c8
--- /dev/null
+++ b/spec/lib/api/entities/namespace_basic_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::API::Entities::NamespaceBasic, feature_category: :groups_and_projects do
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:namespace) { create(:namespace) }
+
+  let(:options) { { current_user: current_user } }
+
+  let(:entity) do
+    described_class.new(namespace, options)
+  end
+
+  subject(:json) { entity.as_json }
+
+  shared_examples 'returns a response' do
+    it 'returns required fields' do
+      expect(json[:id]).to be_present
+      expect(json[:name]).to be_present
+      expect(json[:path]).to be_present
+      expect(json[:kind]).to be_present
+      expect(json[:full_path]).to be_present
+      expect(json[:web_url]).to be_present
+    end
+  end
+
+  include_examples 'returns a response'
+
+  context 'for a user namespace' do
+    let_it_be(:namespace) { create(:user_namespace) }
+
+    include_examples 'returns a response'
+
+    context 'when user namespece owner is missing' do
+      before do
+        namespace.update_column(:owner_id, non_existing_record_id)
+      end
+
+      include_examples 'returns a response'
+
+      it 'returns correct web_url' do
+        expect(json[:web_url]).to include(namespace.path)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/middleware/path_traversal_check_spec.rb b/spec/lib/gitlab/middleware/path_traversal_check_spec.rb
new file mode 100644
index 00000000000..4d5a62e2b32
--- /dev/null
+++ b/spec/lib/gitlab/middleware/path_traversal_check_spec.rb
@@ -0,0 +1,197 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::Middleware::PathTraversalCheck, feature_category: :shared do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:fake_response) { [200, { 'Content-Type' => 'text/plain' }, ['OK']] }
+  let(:fake_app) { ->(_) { fake_response } }
+  let(:middleware) { described_class.new(fake_app) }
+
+  describe '#call' do
+    let(:fullpath) { ::Rack::Request.new(env).fullpath }
+    let(:decoded_fullpath) { CGI.unescape(fullpath) }
+
+    let(:env) do
+      Rack::MockRequest.env_for(
+        path,
+        method: method,
+        params: query_params
+      )
+    end
+
+    subject { middleware.call(env) }
+
+    shared_examples 'no issue' do
+      it 'measures and logs the execution time' do
+        expect(::Gitlab::PathTraversal)
+          .to receive(:check_path_traversal!)
+                .with(decoded_fullpath, skip_decoding: true)
+                .and_call_original
+        expect(::Gitlab::AppLogger)
+          .to receive(:warn)
+                .with({ class_name: described_class.name, duration_s: instance_of(Float) })
+                .and_call_original
+
+        expect(subject).to eq(fake_response)
+      end
+
+      context 'with log_execution_time_path_traversal_middleware disabled' do
+        before do
+          stub_feature_flags(log_execution_time_path_traversal_middleware: false)
+        end
+
+        it 'does nothing' do
+          expect(::Gitlab::PathTraversal)
+            .to receive(:check_path_traversal!)
+                  .with(decoded_fullpath, skip_decoding: true)
+                  .and_call_original
+          expect(::Gitlab::AppLogger)
+            .not_to receive(:warn)
+
+          expect(subject).to eq(fake_response)
+        end
+      end
+    end
+
+    shared_examples 'path traversal' do
+      it 'logs the problem and measures the execution time' do
+        expect(::Gitlab::PathTraversal)
+          .to receive(:check_path_traversal!)
+                .with(decoded_fullpath, skip_decoding: true)
+                .and_call_original
+        expect(::Gitlab::AppLogger)
+          .to receive(:warn)
+                .with({ message: described_class::PATH_TRAVERSAL_MESSAGE, path: instance_of(String) })
+        expect(::Gitlab::AppLogger)
+          .to receive(:warn)
+                .with({
+                  class_name: described_class.name,
+                  duration_s: instance_of(Float),
+                  message: described_class::PATH_TRAVERSAL_MESSAGE,
+                  fullpath: fullpath
+                }).and_call_original
+
+        expect(subject).to eq(fake_response)
+      end
+
+      context 'with log_execution_time_path_traversal_middleware disabled' do
+        before do
+          stub_feature_flags(log_execution_time_path_traversal_middleware: false)
+        end
+
+        it 'logs the problem without the execution time' do
+          expect(::Gitlab::PathTraversal)
+            .to receive(:check_path_traversal!)
+                  .with(decoded_fullpath, skip_decoding: true)
+                  .and_call_original
+          expect(::Gitlab::AppLogger)
+            .to receive(:warn)
+                  .with({ message: described_class::PATH_TRAVERSAL_MESSAGE, path: instance_of(String) })
+          expect(::Gitlab::AppLogger)
+            .to receive(:warn)
+                  .with({
+                    class_name: described_class.name,
+                    message: described_class::PATH_TRAVERSAL_MESSAGE,
+                    fullpath: fullpath
+                  }).and_call_original
+
+          expect(subject).to eq(fake_response)
+        end
+      end
+    end
+
+    # we use Rack request.full_path, this will dump the accessed path and
+    # the query string. The query string is only for GETs requests.
+    # Hence different expectation (when params are set) for GETs and
+    # the other methods (see below)
+    context 'when using get' do
+      let(:method) { 'get' }
+
+      where(:path, :query_params, :shared_example_name) do
+        '/foo/bar'            | {}                          | 'no issue'
+        '/foo/../bar'         | {}                          | 'path traversal'
+        '/foo%2Fbar'          | {}                          | 'no issue'
+        '/foo%2F..%2Fbar'     | {}                          | 'path traversal'
+        '/foo%252F..%252Fbar' | {}                          | 'no issue'
+        '/foo/bar'            | { x: 'foo' }                | 'no issue'
+        '/foo/bar'            | { x: 'foo/../bar' }         | 'path traversal'
+        '/foo/bar'            | { x: 'foo%2Fbar' }          | 'no issue'
+        '/foo/bar'            | { x: 'foo%2F..%2Fbar' }     | 'no issue'
+        '/foo/bar'            | { x: 'foo%252F..%252Fbar' } | 'no issue'
+        '/foo%2F..%2Fbar'     | { x: 'foo%252F..%252Fbar' } | 'path traversal'
+      end
+
+      with_them do
+        it_behaves_like params[:shared_example_name]
+      end
+
+      context 'with a issues search path' do
+        let(:query_params) { {} }
+        let(:path) do
+          'project/-/issues/?sort=updated_desc&milestone_title=16.0&search=Release%20%252525&first_page_size=20'
+        end
+
+        it_behaves_like 'no issue'
+      end
+    end
+
+    %w[post put post delete patch].each do |http_method|
+      context "when using #{http_method}" do
+        let(:method) { http_method }
+
+        where(:path, :query_params, :shared_example_name) do
+          '/foo/bar'            | {}                          | 'no issue'
+          '/foo/../bar'         | {}                          | 'path traversal'
+          '/foo%2Fbar'          | {}                          | 'no issue'
+          '/foo%2F..%2Fbar'     | {}                          | 'path traversal'
+          '/foo%252F..%252Fbar' | {}                          | 'no issue'
+          '/foo/bar'            | { x: 'foo' }                | 'no issue'
+          '/foo/bar'            | { x: 'foo/../bar' }         | 'no issue'
+          '/foo/bar'            | { x: 'foo%2Fbar' }          | 'no issue'
+          '/foo/bar'            | { x: 'foo%2F..%2Fbar' }     | 'no issue'
+          '/foo/bar'            | { x: 'foo%252F..%252Fbar' } | 'no issue'
+          '/foo%2F..%2Fbar'     | { x: 'foo%252F..%252Fbar' } | 'path traversal'
+        end
+
+        with_them do
+          it_behaves_like params[:shared_example_name]
+        end
+      end
+    end
+
+    context 'with check_path_traversal_middleware disabled' do
+      before do
+        stub_feature_flags(check_path_traversal_middleware: false)
+      end
+
+      where(:path, :query_params) do
+        '/foo/bar'            | {}
+        '/foo/../bar'         | {}
+        '/foo%2Fbar'          | {}
+        '/foo%2F..%2Fbar'     | {}
+        '/foo%252F..%252Fbar' | {}
+        '/foo/bar'            | { x: 'foo' }
+        '/foo/bar'            | { x: 'foo/../bar' }
+        '/foo/bar'            | { x: 'foo%2Fbar' }
+        '/foo/bar'            | { x: 'foo%2F..%2Fbar' }
+        '/foo/bar'            | { x: 'foo%252F..%252Fbar' }
+      end
+
+      with_them do
+        %w[get post put post delete patch].each do |http_method|
+          context "when using #{http_method}" do
+            let(:method) { http_method }
+
+            it 'does not check for path traversals' do
+              expect(::Gitlab::PathTraversal).not_to receive(:check_path_traversal!)
+
+              subject
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/path_traversal_spec.rb b/spec/lib/gitlab/path_traversal_spec.rb
index bba6f8293c2..063919dd985 100644
--- a/spec/lib/gitlab/path_traversal_spec.rb
+++ b/spec/lib/gitlab/path_traversal_spec.rb
@@ -93,6 +93,13 @@ RSpec.describe Gitlab::PathTraversal, feature_category: :shared do
     it 'raises for other non-strings' do
       expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/)
     end
+
+    context 'when skip_decoding is used' do
+      it 'does not detect double encoded chars' do
+        expect(check_path_traversal!('foo%252F..%2Fbar', skip_decoding: true)).to eq('foo%252F..%2Fbar')
+        expect(check_path_traversal!('foo%252F%2E%2E%2Fbar', skip_decoding: true)).to eq('foo%252F%2E%2E%2Fbar')
+      end
+    end
   end
 
   describe '.check_allowed_absolute_path!' do
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index 049b8d4ed86..49d6c0f9f26 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -151,17 +151,6 @@ RSpec.describe Gitlab::Shell do
       end
     end
 
-    describe '#remove' do
-      it 'removes the namespace' do
-        Gitlab::GitalyClient::NamespaceService.allow do
-          subject.add_namespace(storage, "mepmep")
-          subject.rm_namespace(storage, "mepmep")
-
-          expect(Gitlab::GitalyClient::NamespaceService.new(storage).exists?("mepmep")).to be(false)
-        end
-      end
-    end
-
     describe '#mv_namespace' do
       it 'renames the namespace' do
         Gitlab::GitalyClient::NamespaceService.allow do
diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb
index 5a29966e4b9..fb1e43a426d 100644
--- a/spec/models/merge_request_reviewer_spec.rb
+++ b/spec/models/merge_request_reviewer_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe MergeRequestReviewer do
+RSpec.describe MergeRequestReviewer, feature_category: :code_review_workflow do
   let(:reviewer) { create(:user) }
   let(:merge_request) { create(:merge_request) }
 
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index a1ba5302d41..77a863e81e1 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -833,6 +833,14 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do
 
   describe '#human_name' do
     it { expect(namespace.human_name).to eq(namespace.owner_name) }
+
+    context 'when the owner is missing' do
+      before do
+        namespace.update_column(:owner_id, non_existing_record_id)
+      end
+
+      it { expect(namespace.human_name).to eq(namespace.path) }
+    end
   end
 
   describe '#any_project_has_container_registry_tags?' do
@@ -1195,70 +1203,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do
     end
   end
 
-  describe '#move_dir', :request_store do
-    context 'hashed storage' do
-      let_it_be(:namespace) { create(:namespace) }
-      let_it_be(:project) { create(:project_empty_repo, namespace: namespace) }
-
-      context 'when any project has container images' do
-        let(:container_repository) { create(:container_repository) }
-
-        before do
-          stub_container_registry_config(enabled: true)
-          stub_container_registry_tags(repository: :any, tags: ['tag'])
-
-          create(:project, namespace: namespace, container_repositories: [container_repository])
-
-          allow(namespace).to receive(:path_was).and_return(namespace.path)
-          allow(namespace).to receive(:path).and_return('new_path')
-          allow(namespace).to receive(:first_project_with_container_registry_tags).and_return(project)
-        end
-
-        it 'raises an error about not movable project' do
-          expect { namespace.move_dir }.to raise_error(
-            Gitlab::UpdatePathError, /Namespace .* cannot be moved/
-          )
-        end
-      end
-
-      it "repository directory remains unchanged if path changed" do
-        before_disk_path = project.disk_path
-        namespace.update!(path: namespace.full_path + '_new')
-
-        expect(before_disk_path).to eq(project.disk_path)
-        expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy
-      end
-    end
-
-    context 'for each project inside the namespace' do
-      let!(:parent) { create(:group, name: 'mygroup', path: 'mygroup') }
-      let!(:subgroup) { create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) }
-      let!(:project_in_parent_group) { create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1') }
-      let!(:hashed_project_in_subgroup) { create(:project, :repository, namespace: subgroup, name: 'foo2') }
-      let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') }
-
-      it 'updates project full path in .git/config' do
-        parent.update!(path: 'mygroup_new')
-
-        expect(project_in_parent_group.reload.repository.full_path).to eq "mygroup_new/#{project_in_parent_group.path}"
-        expect(hashed_project_in_subgroup.reload.repository.full_path).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
-        expect(legacy_project_in_subgroup.reload.repository.full_path).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"
-      end
-
-      it 'updates the project storage location' do
-        repository_project_in_parent_group = project_in_parent_group.project_repository
-        repository_hashed_project_in_subgroup = hashed_project_in_subgroup.project_repository
-        repository_legacy_project_in_subgroup = legacy_project_in_subgroup.project_repository
-
-        parent.update!(path: 'mygroup_moved')
-
-        expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}"
-        expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path
-        expect(repository_legacy_project_in_subgroup.reload.disk_path).to eq "mygroup_moved/mysubgroup/#{legacy_project_in_subgroup.path}"
-      end
-    end
-  end
-
   describe '.find_by_path_or_name' do
     before do
       @namespace = create(:namespace, name: 'WoW', path: 'woW')
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 12898060e22..896521b1632 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -823,6 +823,21 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and
         let(:admin_mode) { true }
         let(:projects) { Project.all }
       end
+
+      it 'returns a project with user namespace that has a missing owner' do
+        project.namespace.update_column(:owner_id, non_existing_record_id)
+        project.route.update_column(:name, nil)
+
+        get api(path, admin, admin_mode: true), params: { search: project.path }
+        expect(response).to have_gitlab_http_status(:ok)
+
+        project_response = json_response.find { |p| p['id'] == project.id }
+        expect(project_response).to be_present
+        expect(project_response['path']).to eq(project.path)
+
+        namespace_response = project_response['namespace']
+        expect(project_response['web_url']).to include(namespace_response['web_url'])
+      end
     end
 
     context 'with default created_at desc order' do
diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb
index 5e37f33e4f2..78deb3cf254 100644
--- a/spec/services/groups/update_service_spec.rb
+++ b/spec/services/groups/update_service_spec.rb
@@ -491,30 +491,6 @@ RSpec.describe Groups::UpdateService, feature_category: :groups_and_projects do
     it 'returns true' do
       expect(service.execute).to eq(true)
     end
-
-    context 'error moving group' do
-      before do
-        allow(internal_group).to receive(:move_dir).and_raise(Gitlab::UpdatePathError)
-      end
-
-      it 'does not raise an error' do
-        expect { service.execute }.not_to raise_error
-      end
-
-      it 'returns false' do
-        expect(service.execute).to eq(false)
-      end
-
-      it 'has the right error' do
-        service.execute
-
-        expect(internal_group.errors.full_messages.first).to eq('Gitlab::UpdatePathError')
-      end
-
-      it "hasn't changed the path" do
-        expect { service.execute }.not_to change { internal_group.reload.path }
-      end
-    end
   end
 
   context 'for a subgroup' do