Merge branch 'security-group-import-file-enuming' into 'master'

Fix file enuming using Group Import

Closes #125

See merge request gitlab-org/security/gitlab!479

(cherry picked from commit 7528f5cd)

cd52ee90 Fix file enuming using Group Import

changelogs/unreleased/security-group-import-file-enuming.yml

+---
+title: Fix file enuming using Group Import
+merge_request:
+author:
+type: security

lib/api/group_import.rb

@@ -2,6 +2,8 @@
 module API
   class GroupImport < Grape::API
+    helpers Helpers::FileUploadHelpers
+
     helpers do
       def parent_group
         find_group!(params[:parent_id]) if params[:parent_id].present?
@@ -49,29 +51,20 @@ def closest_allowed_visibility_level
       params do
         requires :path, type: String, desc: 'Group path'
         requires :name, type: String, desc: 'Group name'
+        requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The group export file to be imported'
         optional :parent_id, type: Integer, desc: "The ID of the parent group that the group will be imported into. Defaults to the current user's namespace."
-        optional 'file.path', type: String, desc: 'Path to locally stored body (generated by Workhorse)'
-        optional 'file.name', type: String, desc: 'Real filename as send in Content-Disposition (generated by Workhorse)'
-        optional 'file.type', type: String, desc: 'Real content type as send in Content-Type (generated by Workhorse)'
-        optional 'file.size', type: Integer, desc: 'Real size of file (generated by Workhorse)'
-        optional 'file.md5', type: String, desc: 'MD5 checksum of the file (generated by Workhorse)'
-        optional 'file.sha1', type: String, desc: 'SHA1 checksum of the file (generated by Workhorse)'
-        optional 'file.sha256', type: String, desc: 'SHA256 checksum of the file (generated by Workhorse)'
       end
       post 'import' do
         authorize_create_group!
         require_gitlab_workhorse!
-
-        uploaded_file = UploadedFile.from_params(params, :file, ImportExportUploader.workhorse_local_upload_path)
-
-        bad_request!('Unable to process group import file') unless uploaded_file
+        validate_file!
         group_params = {
           path: params[:path],
           name: params[:name],
           parent_id: params[:parent_id],
           visibility_level: closest_allowed_visibility_level,
-          import_export_upload: ImportExportUpload.new(import_file: uploaded_file)
+          import_export_upload: ImportExportUpload.new(import_file: params[:file])
         }
         group = ::Groups::CreateService.new(current_user, group_params).execute

spec/requests/api/group_import_spec.rb

@@ -11,7 +11,7 @@
   let(:file) { File.join('spec', 'fixtures', 'group_export.tar.gz') }
   let(:export_path) { "#{Dir.tmpdir}/group_export_spec" }
   let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
-  let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
+  let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
   before do
     allow_next_instance_of(Gitlab::ImportExport) do |import_export|
@@ -35,7 +35,7 @@
       }
     end
-    subject { post api('/groups/import', user), params: params, headers: workhorse_header }
+    subject { upload_archive(file_upload, workhorse_headers, params) }
     shared_examples 'when all params are correct' do
       context 'when user is authorized to create new group' do
@@ -151,7 +151,7 @@
             params[:file] = file_upload
             expect do
-              post api('/groups/import', user), params: params, headers: workhorse_header
+              upload_archive(file_upload, workhorse_headers, params)
             end.not_to change { Group.count }.from(1)
             expect(response).to have_gitlab_http_status(:bad_request)
@@ -171,7 +171,7 @@
       context 'without a file from workhorse' do
         it 'rejects the request' do
-          subject
+          upload_archive(nil, workhorse_headers, params)
           expect(response).to have_gitlab_http_status(:bad_request)
         end
@@ -179,7 +179,7 @@
       context 'without a workhorse header' do
         it 'rejects request without a workhorse header' do
-          post api('/groups/import', user), params: params
+          upload_archive(file_upload, {}, params)
           expect(response).to have_gitlab_http_status(:forbidden)
         end
@@ -189,9 +189,7 @@
         let(:params) do
           {
             path: 'test-import-group',
-            name: 'test-import-group',
-            'file.path' => file_upload.path,
-            'file.name' => file_upload.original_filename
+            name: 'test-import-group'
           }
         end
@@ -229,9 +227,7 @@
           {
             path: 'test-import-group',
             name: 'test-import-group',
-            file: fog_file,
-            'file.remote_id' => file_name,
-            'file.size' => fog_file.size
+            file: fog_file
           }
         end
@@ -245,10 +241,21 @@
         include_examples 'when some params are missing'
       end
     end
+
+    def upload_archive(file, headers = {}, params = {})
+      workhorse_finalize(
+        api('/groups/import', user),
+        method: :post,
+        file_key: :file,
+        params: params.merge(file: file),
+        headers: headers,
+        send_rewritten_field: true
+      )
+    end
   end
   describe 'POST /groups/import/authorize' do
-    subject { post api('/groups/import/authorize', user), headers: workhorse_header }
+    subject { post api('/groups/import/authorize', user), headers: workhorse_headers }
     it 'authorizes importing group with workhorse header' do
       subject
@@ -258,7 +265,7 @@
     end
     it 'rejects requests that bypassed gitlab-workhorse' do
-      workhorse_header.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
+      workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
       subject