Merge branch 'security-use-multipart-uploaded-file-in-maven-packages' into 'master'

Use the uploaded file set by multipart.rb in Maven packages

Closes #183

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

ee/changelogs/unreleased/security-use-multipart-uploaded-file-in-maven-packages.yml

+---
+title: Maven packages upload endpoint is now properly using the uploaded file set
+  by middleware
+merge_request:
+author:
+type: security

ee/lib/api/maven_packages.rb

@@ -216,13 +216,7 @@ module API
       params do
         requires :path, type: String, desc: 'Package path'
         requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.maven_file_name_regex
-        optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse))
-        optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse))
-        optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse))
-        optional 'file.size', type: Integer, desc: %q(real size of file (generated by Workhorse))
-        optional 'file.md5', type: String, desc: %q(md5 checksum of the file (generated by Workhorse))
-        optional 'file.sha1', type: String, desc: %q(sha1 checksum of the file (generated by Workhorse))
-        optional 'file.sha256', type: String, desc: %q(sha256 checksum of the file (generated by Workhorse))
+        requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
       end
       route_setting :authentication, job_token_allowed: true, deploy_token_allowed: true
       put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
@@ -230,9 +224,6 @@ module API
 
         file_name, format = extract_format(params[:file_name])
 
-        uploaded_file = UploadedFile.from_params(params, :file, ::Packages::PackageFileUploader.workhorse_local_upload_path)
-        bad_request!('Missing package file!') unless uploaded_file
-
         package = ::Packages::Maven::FindOrCreatePackageService
           .new(user_project, current_user, params.merge(build: current_authenticated_job)).execute
 
@@ -244,14 +235,14 @@ module API
           package_file = ::Packages::PackageFileFinder
             .new(package, file_name).execute!
 
-          verify_package_file(package_file, uploaded_file)
+          verify_package_file(package_file, params[:file])
         when 'md5'
           nil
         else
           track_event('push_package') if jar_file?(format)
 
           file_params = {
-            file:      uploaded_file,
+            file:      params[:file],
             size:      params['file.size'],
             file_name: file_name,
             file_type: params['file.type'],

ee/spec/requests/api/maven_packages_spec.rb

@@ -2,6 +2,8 @@
 require 'spec_helper'
 
 RSpec.describe API::MavenPackages do
+  include WorkhorseHelpers
+
   let_it_be(:group) { create(:group) }
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project, :public, namespace: group) }
@@ -484,6 +486,9 @@ RSpec.describe API::MavenPackages do
   end
 
   describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name' do
+    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(:send_rewritten_field) { true }
     let(:file_upload) { fixture_file_upload('spec/fixtures/packages/maven/my-app-1.0-20180724.124855-1.jar') }
 
     before do
@@ -511,13 +516,18 @@ RSpec.describe API::MavenPackages do
       expect(response).to have_gitlab_http_status(:forbidden)
     end
 
-    context 'when params from workhorse are correct' do
-      let(:params) do
-        {
-          'file.path' => file_upload.path,
-          'file.name' => file_upload.original_filename
-        }
+    context 'without workhorse rewritten field' do
+      let(:send_rewritten_field) { false }
+
+      it 'rejects the request' do
+        upload_file_with_token
+
+        expect(response).to have_gitlab_http_status(:bad_request)
       end
+    end
+
+    context 'when params from workhorse are correct' do
+      let(:params) { { file: file_upload } }
 
       it 'rejects a malicious request' do
         put api("/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/%2e%2e%2f.ssh%2fauthorized_keys"), params: params, headers: headers_with_token
@@ -526,6 +536,8 @@ RSpec.describe API::MavenPackages do
       end
 
       context 'without workhorse header' do
+        let(:workhorse_header) { {} }
+
         subject { upload_file_with_token(params) }
 
         it_behaves_like 'package workhorse uploads'
@@ -572,7 +584,15 @@ RSpec.describe API::MavenPackages do
     end
 
     def upload_file(params = {}, request_headers = headers)
-      put api("/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/my-app-1.0-20180724.124855-1.jar"), params: params, headers: request_headers
+      url = "/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/my-app-1.0-20180724.124855-1.jar"
+      workhorse_finalize(
+        api(url),
+        method: :put,
+        file_key: :file,
+        params: params,
+        headers: request_headers,
+        send_rewritten_field: send_rewritten_field
+      )
     end
 
     def upload_file_with_token(params = {}, request_headers = headers_with_token)