Add support for generic package file uploads

Add new API endpoints to support uploading generic packages files
(accelerated through Workhorse).

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/235492.

app/services/packages/create_package_service.rb

@@ -10,6 +10,7 @@ module Packages
         .with_package_type(package_type)
         .safe_find_or_create_by!(name: name, version: version) do |pkg|
           pkg.creator = package_creator
+          yield pkg if block_given?
         end
     end
 

app/services/packages/generic/create_package_file_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Generic
+    class CreatePackageFileService < BaseService
+      def execute
+        ::Packages::Package.transaction do
+          create_package_file(find_or_create_package)
+        end
+      end
+
+      private
+
+      def find_or_create_package
+        package_params = {
+          name: params[:package_name],
+          version: params[:package_version],
+          build: params[:build]
+        }
+
+        ::Packages::Generic::FindOrCreatePackageService
+          .new(project, current_user, package_params)
+          .execute
+      end
+
+      def create_package_file(package)
+        file_params = {
+          file: params[:file],
+          size: params[:file].size,
+          file_sha256: params[:file].sha256,
+          file_name: params[:file_name]
+        }
+
+        ::Packages::CreatePackageFileService.new(package, file_params).execute
+      end
+    end
+  end
+end

app/services/packages/generic/find_or_create_package_service.rb

+# frozen_string_literal: true
+
+module Packages
+  module Generic
+    class FindOrCreatePackageService < ::Packages::CreatePackageService
+      def execute
+        find_or_create_package!(::Packages::Package.package_types['generic']) do |package|
+          if params[:build].present?
+            package.build_info = Packages::BuildInfo.new(pipeline: params[:build].pipeline)
+          end
+        end
+      end
+    end
+  end
+end

lib/api/generic_packages.rb

@@ -2,6 +2,11 @@
 
 module API
   class GenericPackages < Grape::API::Instance
+    GENERIC_PACKAGES_REQUIREMENTS = {
+      package_name: API::NO_SLASH_URL_PART_REGEX,
+      file_name: API::NO_SLASH_URL_PART_REGEX
+    }.freeze
+
     before do
       require_packages_enabled!
       authenticate!
@@ -17,17 +22,71 @@ module API
       route_setting :authentication, job_token_allowed: true
 
       namespace ':id/packages/generic' do
-        get 'ping' do
-          :pong
+        namespace ':package_name/*package_version/:file_name', requirements: GENERIC_PACKAGES_REQUIREMENTS do
+          desc 'Workhorse authorize generic package file' do
+            detail 'This feature was introduced in GitLab 13.5'
+          end
+
+          route_setting :authentication, job_token_allowed: true
+
+          params do
+            requires :package_name, type: String, desc: 'Package name'
+            requires :package_version, type: String, desc: 'Package version', regexp: Gitlab::Regex.generic_package_version_regex
+            requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.generic_package_file_name_regex, file_path: true
+          end
+
+          put 'authorize' do
+            authorize_workhorse!(subject: project, maximum_size: project.actual_limits.generic_packages_max_file_size)
+          end
+
+          desc 'Upload package file' do
+            detail 'This feature was introduced in GitLab 13.5'
+          end
+
+          params do
+            requires :package_name, type: String, desc: 'Package name'
+            requires :package_version, type: String, desc: 'Package version', regexp: Gitlab::Regex.generic_package_version_regex
+            requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.generic_package_file_name_regex, file_path: true
+            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
+
+          put do
+            authorize_upload!(project)
+            bad_request!('File is too large') if max_file_size_exceeded?
+
+            track_event('push_package')
+
+            create_package_file_params = declared_params.merge(build: current_authenticated_job)
+            ::Packages::Generic::CreatePackageFileService
+              .new(project, current_user, create_package_file_params)
+              .execute
+
+            created!
+          rescue ObjectStorage::RemoteStoreError => e
+            Gitlab::ErrorTracking.track_exception(e, extra: { file_name: params[:file_name], project_id: project.id })
+
+            forbidden!
+          end
         end
       end
     end
 
     helpers do
       include ::API::Helpers::PackagesHelpers
+      include ::API::Helpers::Packages::BasicAuthHelpers
 
       def require_generic_packages_available!
-        not_found! unless Feature.enabled?(:generic_packages, user_project)
+        not_found! unless Feature.enabled?(:generic_packages, project)
+      end
+
+      def project
+        authorized_user_project
+      end
+
+      def max_file_size_exceeded?
+        project.actual_limits.exceeded?(:generic_packages_max_file_size, params[:file].size)
       end
     end
   end

lib/gitlab/regex.rb

@@ -103,6 +103,10 @@ module Gitlab
       def generic_package_version_regex
         /\A\d+\.\d+\.\d+\z/
       end
+
+      def generic_package_file_name_regex
+        maven_file_name_regex
+      end
     end
 
     extend self

spec/lib/gitlab/regex_spec.rb

@@ -434,4 +434,18 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('%2e%2e%2f1.2.3') }
     it { is_expected.not_to match('') }
   end
+
+  describe '.generic_package_file_name_regex' do
+    subject { described_class.generic_package_file_name_regex }
+
+    it { is_expected.to match('123') }
+    it { is_expected.to match('foo') }
+    it { is_expected.to match('foo.bar.baz-2.0-20190901.47283-1.jar') }
+    it { is_expected.not_to match('../../foo') }
+    it { is_expected.not_to match('..\..\foo') }
+    it { is_expected.not_to match('%2f%2e%2e%2f%2essh%2fauthorized_keys') }
+    it { is_expected.not_to match('$foo/bar') }
+    it { is_expected.not_to match('my file name') }
+    it { is_expected.not_to match('!!()()') }
+  end
 end

spec/requests/api/generic_packages_spec.rb

@@ -4,79 +4,268 @@ require 'spec_helper'
 
 RSpec.describe API::GenericPackages do
   let_it_be(:personal_access_token) { create(:personal_access_token) }
-  let_it_be(:project) { create(:project) }
+  let_it_be(:project, reload: true) { create(:project) }
+  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(:user) { personal_access_token.user }
+  let(:ci_build) { create(:ci_build, :running, user: user) }
 
-  describe 'GET /api/v4/projects/:id/packages/generic/ping' do
-    let(:user) { personal_access_token.user }
-    let(:auth_token) { personal_access_token.token }
+  def auth_header
+    return {} if user_role == :anonymous
 
-    before do
-      project.add_developer(user)
+    case authenticate_with
+    when :personal_access_token
+      personal_access_token_header
+    when :job_token
+      job_token_header
+    when :invalid_personal_access_token
+      personal_access_token_header('wrong token')
+    when :invalid_job_token
+      job_token_header('wrong token')
     end
+  end
 
-    context 'packages feature is disabled' do
-      it 'responds with 404 Not Found' do
-        stub_packages_setting(enabled: false)
+  def personal_access_token_header(value = nil)
+    { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => value || personal_access_token.token }
+  end
 
-        ping(personal_access_token: auth_token)
+  def job_token_header(value = nil)
+    { Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => value || ci_build.token }
+  end
 
-        expect(response).to have_gitlab_http_status(:not_found)
+  describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize' do
+    context 'with valid project' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do
+        'PUBLIC'  | :developer | true  | :personal_access_token         | :success
+        'PUBLIC'  | :guest     | true  | :personal_access_token         | :forbidden
+        'PUBLIC'  | :developer | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :guest     | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :developer | false | :personal_access_token         | :forbidden
+        'PUBLIC'  | :guest     | false | :personal_access_token         | :forbidden
+        'PUBLIC'  | :developer | false | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :guest     | false | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :anonymous | false | :none                          | :unauthorized
+        'PRIVATE' | :developer | true  | :personal_access_token         | :success
+        'PRIVATE' | :guest     | true  | :personal_access_token         | :forbidden
+        'PRIVATE' | :developer | true  | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :guest     | true  | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :developer | false | :personal_access_token         | :not_found
+        'PRIVATE' | :guest     | false | :personal_access_token         | :not_found
+        'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :guest     | false | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :anonymous | false | :none                          | :unauthorized
+        'PUBLIC'  | :developer | true  | :job_token                     | :success
+        'PUBLIC'  | :developer | true  | :invalid_job_token             | :unauthorized
+        'PUBLIC'  | :developer | false | :job_token                     | :forbidden
+        'PUBLIC'  | :developer | false | :invalid_job_token             | :unauthorized
+        'PRIVATE' | :developer | true  | :job_token                     | :success
+        'PRIVATE' | :developer | true  | :invalid_job_token             | :unauthorized
+        'PRIVATE' | :developer | false | :job_token                     | :not_found
+        'PRIVATE' | :developer | false | :invalid_job_token             | :unauthorized
+      end
+
+      with_them do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false))
+          project.send("add_#{user_role}", user) if member? && user_role != :anonymous
+        end
+
+        it "responds with #{params[:expected_status]}" do
+          headers = workhorse_header.merge(auth_header)
+          url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize"
+
+          put api(url), headers: headers
+
+          expect(response).to have_gitlab_http_status(expected_status)
+        end
       end
     end
 
+    it 'rejects a malicious request' do
+      project.add_developer(user)
+      headers = workhorse_header.merge(personal_access_token_header)
+      url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/%2e%2e%2f.ssh%2fauthorized_keys/authorize"
+
+      put api(url), headers: headers
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
     context 'generic_packages feature flag is disabled' do
       it 'responds with 404 Not Found' do
         stub_feature_flags(generic_packages: false)
+        project.add_developer(user)
+        headers = workhorse_header.merge(personal_access_token_header)
+        url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize"
 
-        ping(personal_access_token: auth_token)
+        put api(url), headers: headers
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+  end
 
-    context 'generic_packages feature flag is enabled' do
-      before do
-        stub_feature_flags(generic_packages: true)
+  describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz' do
+    include WorkhorseHelpers
+
+    let(:file_upload) { fixture_file_upload('spec/fixtures/packages/generic/myfile.tar.gz') }
+    let(:params) { { file: file_upload } }
+
+    context 'authentication' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do
+        'PUBLIC'  | :guest     | true  | :personal_access_token         | :forbidden
+        'PUBLIC'  | :developer | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :guest     | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :developer | false | :personal_access_token         | :forbidden
+        'PUBLIC'  | :guest     | false | :personal_access_token         | :forbidden
+        'PUBLIC'  | :developer | false | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :guest     | false | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :anonymous | false | :none                          | :unauthorized
+        'PRIVATE' | :guest     | true  | :personal_access_token         | :forbidden
+        'PRIVATE' | :developer | true  | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :guest     | true  | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :developer | false | :personal_access_token         | :not_found
+        'PRIVATE' | :guest     | false | :personal_access_token         | :not_found
+        'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :guest     | false | :invalid_personal_access_token | :unauthorized
+        'PRIVATE' | :anonymous | false | :none                          | :unauthorized
+        'PUBLIC'  | :developer | true  | :invalid_job_token             | :unauthorized
+        'PUBLIC'  | :developer | false | :job_token                     | :forbidden
+        'PUBLIC'  | :developer | false | :invalid_job_token             | :unauthorized
+        'PRIVATE' | :developer | true  | :invalid_job_token             | :unauthorized
+        'PRIVATE' | :developer | false | :job_token                     | :not_found
+        'PRIVATE' | :developer | false | :invalid_job_token             | :unauthorized
       end
 
-      context 'authenticating using personal access token' do
-        it 'responds with 200 OK when valid personal access token is provided' do
-          ping(personal_access_token: auth_token)
+      with_them do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false))
+          project.send("add_#{user_role}", user) if member? && user_role != :anonymous
+        end
+
+        it "responds with #{params[:expected_status]}" do
+          headers = workhorse_header.merge(auth_header)
 
-          expect(response).to have_gitlab_http_status(:ok)
+          upload_file(params, headers)
+
+          expect(response).to have_gitlab_http_status(expected_status)
         end
+      end
+    end
 
-        it 'responds with 401 Unauthorized when invalid personal access token provided' do
-          ping(personal_access_token: 'invalid-token')
+    context 'when user can upload packages and has valid credentials' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'creates package and package file when valid personal access token is used' do
+        headers = workhorse_header.merge(personal_access_token_header)
+
+        expect { upload_file(params, headers) }
+          .to change { project.packages.generic.count }.by(1)
+          .and change { Packages::PackageFile.count }.by(1)
 
-          expect(response).to have_gitlab_http_status(:unauthorized)
+        aggregate_failures do
+          expect(response).to have_gitlab_http_status(:created)
+
+          package = project.packages.generic.last
+          expect(package.name).to eq('mypackage')
+          expect(package.version).to eq('0.0.1')
+          expect(package.build_info).to be_nil
+
+          package_file = package.package_files.last
+          expect(package_file.file_name).to eq('myfile.tar.gz')
         end
       end
 
-      context 'authenticating using job token' do
-        it 'responds with 200 OK when valid job token is provided' do
-          job_token = create(:ci_build, :running, user: user).token
+      it 'creates package, package file, and package build info when valid job token is used' do
+        headers = workhorse_header.merge(job_token_header)
+
+        expect { upload_file(params, headers) }
+          .to change { project.packages.generic.count }.by(1)
+          .and change { Packages::PackageFile.count }.by(1)
+
+        aggregate_failures do
+          expect(response).to have_gitlab_http_status(:created)
 
-          ping(job_token: job_token)
+          package = project.packages.generic.last
+          expect(package.name).to eq('mypackage')
+          expect(package.version).to eq('0.0.1')
+          expect(package.build_info.pipeline).to eq(ci_build.pipeline)
 
-          expect(response).to have_gitlab_http_status(:ok)
+          package_file = package.package_files.last
+          expect(package_file.file_name).to eq('myfile.tar.gz')
         end
+      end
+
+      context 'event tracking' do
+        subject { upload_file(params, workhorse_header.merge(personal_access_token_header)) }
+
+        it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package'
+      end
+
+      it 'rejects request without a file from workhorse' do
+        headers = workhorse_header.merge(personal_access_token_header)
+        upload_file({}, headers)
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
 
-        it 'responds with 401 Unauthorized when invalid job token provided' do
-          ping(job_token: 'invalid-token')
+      it 'rejects request without an auth token' do
+        upload_file(params, workhorse_header)
 
-          expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+
+      it 'rejects request without workhorse rewritten fields' do
+        headers = workhorse_header.merge(personal_access_token_header)
+        upload_file(params, headers, send_rewritten_field: false)
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+
+      it 'rejects request if file size is too large' do
+        allow_next_instance_of(UploadedFile) do |uploaded_file|
+          allow(uploaded_file).to receive(:size).and_return(project.actual_limits.generic_packages_max_file_size + 1)
         end
+
+        headers = workhorse_header.merge(personal_access_token_header)
+        upload_file(params, headers)
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+
+      it 'rejects request without workhorse header' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).once
+
+        upload_file(params, personal_access_token_header)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      it 'rejects a malicious request' do
+        headers = workhorse_header.merge(personal_access_token_header)
+        upload_file(params, headers, file_name: '%2e%2e%2f.ssh%2fauthorized_keys')
+
+        expect(response).to have_gitlab_http_status(:bad_request)
       end
     end
 
-    def ping(personal_access_token: nil, job_token: nil)
-      headers = {
-        Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.presence,
-        Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => job_token.presence
-      }.compact
+    def upload_file(params, request_headers, send_rewritten_field: true, file_name: 'myfile.tar.gz')
+      url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/#{file_name}"
 
-      get api('/projects/%d/packages/generic/ping' % project.id), headers: headers
+      workhorse_finalize(
+        api(url),
+        method: :put,
+        file_key: :file,
+        params: params,
+        headers: request_headers,
+        send_rewritten_field: send_rewritten_field
+      )
     end
   end
 end

spec/services/packages/generic/create_package_file_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Generic::CreatePackageFileService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+
+  describe '#execute' do
+    let(:sha256) { '440e5e148a25331bbd7991575f7d54933c0ebf6cc735a18ee5066ac1381bb590' }
+    let(:temp_file) { Tempfile.new("test") }
+    let(:file) { UploadedFile.new(temp_file.path, sha256: sha256) }
+    let(:package) { create(:generic_package, project: project) }
+    let(:params) do
+      {
+        package_name: 'mypackage',
+        package_version: '0.0.1',
+        file: file,
+        file_name: 'myfile.tar.gz.1'
+      }
+    end
+
+    before do
+      FileUtils.touch(temp_file)
+    end
+
+    after do
+      FileUtils.rm_f(temp_file)
+    end
+
+    it 'creates package file' do
+      package_service = double
+      package_params = {
+        name: params[:package_name],
+        version: params[:package_version],
+        build: params[:build]
+      }
+      expect(::Packages::Generic::FindOrCreatePackageService).to receive(:new).with(project, user, package_params).and_return(package_service)
+      expect(package_service).to receive(:execute).and_return(package)
+
+      service = described_class.new(project, user, params)
+
+      expect { service.execute }.to change { package.package_files.count }.by(1)
+
+      package_file = package.package_files.last
+      aggregate_failures do
+        expect(package_file.package).to eq(package)
+        expect(package_file.file_name).to eq('myfile.tar.gz.1')
+        expect(package_file.size).to eq(file.size)
+        expect(package_file.file_sha256).to eq(sha256)
+      end
+    end
+  end
+end

spec/services/packages/generic/find_or_create_package_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Generic::FindOrCreatePackageService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:ci_build) { create(:ci_build, :running, user: user) }
+
+  let(:params) do
+    {
+      name: 'mypackage',
+      version: '0.0.1'
+    }
+  end
+
+  describe '#execute' do
+    context 'when packages does not exist yet' do
+      it 'creates package' do
+        service = described_class.new(project, user, params)
+
+        expect { service.execute }.to change { project.packages.generic.count }.by(1)
+
+        package = project.packages.generic.last
+
+        aggregate_failures do
+          expect(package.creator).to eq(user)
+          expect(package.name).to eq('mypackage')
+          expect(package.version).to eq('0.0.1')
+          expect(package.build_info).to be_nil
+        end
+      end
+
+      it 'creates package and package build info when build is provided' do
+        service = described_class.new(project, user, params.merge(build: ci_build))
+
+        expect { service.execute }.to change { project.packages.generic.count }.by(1)
+
+        package = project.packages.generic.last
+
+        aggregate_failures do
+          expect(package.creator).to eq(user)
+          expect(package.name).to eq('mypackage')
+          expect(package.version).to eq('0.0.1')
+          expect(package.build_info.pipeline).to eq(ci_build.pipeline)
+        end
+      end
+    end
+
+    context 'when packages already exists' do
+      let!(:package) { project.packages.generic.create!(params) }
+
+      context 'when package was created manually' do
+        it 'finds the package and does not create package build info even if build is provided' do
+          service = described_class.new(project, user, params.merge(build: ci_build))
+
+          expect do
+            found_package = service.execute
+
+            expect(found_package).to eq(package)
+          end.not_to change { project.packages.generic.count }
+
+          expect(package.reload.build_info).to be_nil
+        end
+      end
+
+      context 'when package was created by pipeline' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+
+        before do
+          package.create_build_info!(pipeline: pipeline)
+        end
+
+        it 'finds the package and does not change package build info even if build is provided' do
+          service = described_class.new(project, user, params.merge(build: ci_build))
+
+          expect do
+            found_package = service.execute
+
+            expect(found_package).to eq(package)
+          end.not_to change { project.packages.generic.count }
+
+          expect(package.reload.build_info.pipeline).to eq(pipeline)
+        end
+      end
+    end
+  end
+end