Merge branch 'sh-validate-path-project-import-10-3' into 'security-10-3'

Validate project path in Gitlab import - 10.3 port

See merge request gitlab/gitlabhq!2268

(cherry picked from commit 94c82376d66fc80d46dd2d5eeb5bade408ec6a7e)

2b94a7c2 Validate project path in Gitlab import

4 files changed, 71 insertions, 2 deletions

diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb
index 4ca6414b73b..a3d7f5cbed5 100644
--- a/app/services/projects/gitlab_projects_import_service.rb
+++ b/app/services/projects/gitlab_projects_import_service.rb
@@ -26,7 +26,7 @@ module Projects
     end
 
     def tmp_filename
-      "#{SecureRandom.hex}_#{params[:path]}"
+      SecureRandom.hex
     end
 
     def file
diff --git a/spec/controllers/import/gitlab_projects_controller_spec.rb b/spec/controllers/import/gitlab_projects_controller_spec.rb
new file mode 100644
index 00000000000..8759d3c0b97
--- /dev/null
+++ b/spec/controllers/import/gitlab_projects_controller_spec.rb
@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+describe Import::GitlabProjectsController do
+  set(:namespace) { create(:namespace) }
+  set(:user) { namespace.owner }
+  let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'POST create' do
+    context 'with an invalid path' do
+      it 'redirects with an error' do
+        post :create, namespace_id: namespace.id, path: '/test', file: file
+
+        expect(flash[:alert]).to start_with('Project could not be imported')
+        expect(response).to have_gitlab_http_status(302)
+      end
+
+      it 'redirects with an error when a relative path is used' do
+        post :create, namespace_id: namespace.id, path: '../test', file: file
+
+        expect(flash[:alert]).to start_with('Project could not be imported')
+        expect(response).to have_gitlab_http_status(302)
+      end
+    end
+
+    context 'with a valid path' do
+      it 'redirects to the new project path' do
+        post :create, namespace_id: namespace.id, path: 'test', file: file
+
+        expect(flash[:notice]).to include('is being imported')
+        expect(response).to have_gitlab_http_status(302)
+      end
+    end
+  end
+end
diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb
index af125e1b9d3..e8bb9c6a86c 100644
--- a/spec/features/projects/import_export/import_file_spec.rb
+++ b/spec/features/projects/import_export/import_file_spec.rb
@@ -32,7 +32,7 @@ feature 'Import/Export - project import integration test', :js do
 
         expect(page).to have_content('Import an exported GitLab project')
         expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=#{project_path}")
-        expect(Gitlab::ImportExport).to receive(:import_upload_path).with(filename: /\A\h{32}_test-project-path\h*\z/).and_call_original
+        expect(Gitlab::ImportExport).to receive(:import_upload_path).with(filename: /\A\h{32}\z/).and_call_original
 
         attach_file('file', file)
         click_on 'Import project'
diff --git a/spec/services/projects/gitlab_projects_import_service_spec.rb b/spec/services/projects/gitlab_projects_import_service_spec.rb
new file mode 100644
index 00000000000..bb0e274c93e
--- /dev/null
+++ b/spec/services/projects/gitlab_projects_import_service_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe Projects::GitlabProjectsImportService do
+  set(:namespace) { build(:namespace) }
+  let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+  subject { described_class.new(namespace.owner, { namespace_id: namespace.id, path: path, file: file }) }
+
+  describe '#execute' do
+    context 'with an invalid path' do
+      let(:path) { '/invalid-path/' }
+
+      it 'returns an invalid project' do
+        project = subject.execute
+
+        expect(project).not_to be_persisted
+        expect(project).not_to be_valid
+      end
+    end
+
+    context 'with a valid path' do
+      let(:path) { 'test-path' }
+
+      it 'creates a project' do
+        project = subject.execute
+
+        expect(project).to be_persisted
+        expect(project).to be_valid
+      end
+    end
+  end
+end