Merge branch '235490-generic-packages/download' into 'master'

Implement generic packages download

See merge request gitlab-org/gitlab!43503

app/finders/packages/generic/package_finder.rb

+# frozen_string_literal: true
+
+module Packages
+  module Generic
+    class PackageFinder
+      def initialize(project)
+        @project = project
+      end
+
+      def execute!(package_name, package_version)
+        project
+          .packages
+          .generic
+          .by_name_and_version!(package_name, package_version)
+      end
+
+      private
+
+      attr_reader :project
+    end
+  end
+end

app/models/packages/package.rb

@@ -26,7 +26,7 @@ class Packages::Package < ApplicationRecord
   validates :project, presence: true
   validates :name, presence: true
 
-  validates :name, format: { with: Gitlab::Regex.package_name_regex }, unless: :conan?
+  validates :name, format: { with: Gitlab::Regex.package_name_regex }, unless: -> { conan? || generic? }
 
   validates :name,
     uniqueness: { scope: %i[project_id version package_type] }, unless: :conan?
@@ -35,8 +35,9 @@ class Packages::Package < ApplicationRecord
   validate :valid_npm_package_name, if: :npm?
   validate :valid_composer_global_name, if: :composer?
   validate :package_already_taken, if: :npm?
-  validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { npm? || nuget? }
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
+  validates :name, format: { with: Gitlab::Regex.generic_package_name_regex }, if: :generic?
+  validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { npm? || nuget? }
   validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.maven_version_regex }, if: -> { version? && maven? }
   validates :version, format: { with: Gitlab::Regex.pypi_version_regex }, if: :pypi?
@@ -120,6 +121,10 @@ class Packages::Package < ApplicationRecord
       .where(packages_package_files: { file_name: file_name, file_sha256: sha256 }).last!
   end
 
+  def self.by_name_and_version!(name, version)
+    find_by!(name: name, version: version)
+  end
+
   def self.pluck_names
     pluck(:name)
   end

lib/api/generic_packages.rb

@@ -30,7 +30,7 @@ module API
           route_setting :authentication, job_token_allowed: true
 
           params do
-            requires :package_name, type: String, desc: 'Package name'
+            requires :package_name, type: String, desc: 'Package name', regexp: Gitlab::Regex.generic_package_name_regex, file_path: true
             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
@@ -44,7 +44,7 @@ module API
           end
 
           params do
-            requires :package_name, type: String, desc: 'Package name'
+            requires :package_name, type: String, desc: 'Package name', regexp: Gitlab::Regex.generic_package_name_regex, file_path: true
             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)'
@@ -69,6 +69,29 @@ module API
 
             forbidden!
           end
+
+          desc 'Download package file' do
+            detail 'This feature was introduced in GitLab 13.5'
+          end
+
+          params do
+            requires :package_name, type: String, desc: 'Package name', regexp: Gitlab::Regex.generic_package_name_regex, file_path: true
+            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
+
+          route_setting :authentication, job_token_allowed: true
+
+          get do
+            authorize_read_package!(project)
+
+            package = ::Packages::Generic::PackageFinder.new(project).execute!(params[:package_name], params[:package_version])
+            package_file = ::Packages::PackageFileFinder.new(package, params[:file_name]).execute!
+
+            track_event('pull_package')
+
+            present_carrierwave_file!(package_file.file)
+          end
         end
       end
     end

lib/gitlab/regex.rb

@@ -142,9 +142,13 @@ module Gitlab
         /\A\d+\.\d+\.\d+\z/
       end
 
-      def generic_package_file_name_regex
+      def generic_package_name_regex
         maven_file_name_regex
       end
+
+      def generic_package_file_name_regex
+        generic_package_name_regex
+      end
     end
 
     extend self

spec/factories/packages/package_file.rb

@@ -140,6 +140,14 @@ FactoryBot.define do
       size { 1149.bytes }
     end
 
+    trait(:generic) do
+      package
+      file_fixture { 'spec/fixtures/packages/generic/myfile.tar.gz' }
+      file_name { "#{package.name}.tar.gz" }
+      file_sha256 { '440e5e148a25331bbd7991575f7d54933c0ebf6cc735a18ee5066ac1381bb590' }
+      size { 1149.bytes }
+    end
+
     trait(:object_storage) do
       file_store { Packages::PackageFileUploader::Store::REMOTE }
     end

spec/finders/packages/generic/package_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Packages::Generic::PackageFinder do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:package) { create(:generic_package, project: project) }
+
+  describe '#execute!' do
+    subject(:finder) { described_class.new(project) }
+
+    it 'finds package by name and version' do
+      found_package = finder.execute!(package.name, package.version)
+
+      expect(found_package).to eq(package)
+    end
+
+    it 'ignores packages with same name but different version' do
+      create(:generic_package, project: project, name: package.name, version: '3.1.4')
+
+      found_package = finder.execute!(package.name, package.version)
+
+      expect(found_package).to eq(package)
+    end
+
+    it 'raises ActiveRecord::RecordNotFound if package is not found' do
+      expect { finder.execute!(package.name, '3.1.4') }
+        .to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end

spec/lib/gitlab/regex_spec.rb

@@ -599,6 +599,20 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('') }
   end
 
+  describe '.generic_package_name_regex' do
+    subject { described_class.generic_package_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') }
+    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
+
   describe '.generic_package_file_name_regex' do
     subject { described_class.generic_package_file_name_regex }
 

spec/models/packages/package_spec.rb

@@ -108,6 +108,20 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('.foobar').for(:name) }
         it { is_expected.not_to allow_value('%foo%bar').for(:name) }
       end
+
+      context 'generic package' do
+        subject { build_stubbed(:generic_package) }
+
+        it { is_expected.to allow_value('123').for(:name) }
+        it { is_expected.to allow_value('foo').for(:name) }
+        it { is_expected.to allow_value('foo.bar.baz-2.0-20190901.47283-1').for(:name) }
+        it { is_expected.not_to allow_value('../../foo').for(:name) }
+        it { is_expected.not_to allow_value('..\..\foo').for(:name) }
+        it { is_expected.not_to allow_value('%2f%2e%2e%2f%2essh%2fauthorized_keys').for(:name) }
+        it { is_expected.not_to allow_value('$foo/bar').for(:name) }
+        it { is_expected.not_to allow_value('my file name').for(:name) }
+        it { is_expected.not_to allow_value('!!().for(:name)().for(:name)').for(:name) }
+      end
     end
 
     describe '#version' do

spec/requests/api/generic_packages_spec.rb

@@ -33,7 +33,19 @@ RSpec.describe API::GenericPackages do
     { Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => value || ci_build.token }
   end
 
-  describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize' do
+  shared_examples 'secure endpoint' do
+    before do
+      project.add_developer(user)
+    end
+
+    it 'rejects malicious request' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+  end
+
+  describe 'PUT /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name/authorize' do
     context 'with valid project' do
       using RSpec::Parameterized::TableSyntax
 
@@ -73,41 +85,49 @@ RSpec.describe API::GenericPackages do
         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
+          authorize_upload_file(workhorse_header.merge(auth_header))
 
           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"
+    context 'application security' do
+      using RSpec::Parameterized::TableSyntax
 
-      put api(url), headers: headers
+      where(:param_name, :param_value) do
+        :package_name | 'my-package/../'
+        :package_name | 'my-package%2f%2e%2e%2f'
+        :file_name    | '../.ssh%2fauthorized_keys'
+        :file_name    | '%2e%2e%2f.ssh%2fauthorized_keys'
+      end
 
-      expect(response).to have_gitlab_http_status(:bad_request)
+      with_them do
+        subject { authorize_upload_file(workhorse_header.merge(personal_access_token_header), param_name => param_value) }
+
+        it_behaves_like 'secure endpoint'
+      end
     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"
 
-        put api(url), headers: headers
+        authorize_upload_file(workhorse_header.merge(personal_access_token_header))
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    def authorize_upload_file(request_headers, package_name: 'mypackage', file_name: 'myfile.tar.gz')
+      url = "/projects/#{project.id}/packages/generic/#{package_name}/0.0.1/#{file_name}/authorize"
+
+      put api(url), headers: request_headers
+    end
   end
 
-  describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz' do
+  describe 'PUT /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name' do
     include WorkhorseHelpers
 
     let(:file_upload) { fixture_file_upload('spec/fixtures/packages/generic/myfile.tar.gz') }
@@ -246,17 +266,27 @@ RSpec.describe API::GenericPackages do
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
+    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')
+    context 'application security' do
+      using RSpec::Parameterized::TableSyntax
 
-        expect(response).to have_gitlab_http_status(:bad_request)
+      where(:param_name, :param_value) do
+        :package_name | 'my-package/../'
+        :package_name | 'my-package%2f%2e%2e%2f'
+        :file_name    | '../.ssh%2fauthorized_keys'
+        :file_name    | '%2e%2e%2f.ssh%2fauthorized_keys'
+      end
+
+      with_them do
+        subject { upload_file(params, workhorse_header.merge(personal_access_token_header), param_name => param_value) }
+
+        it_behaves_like 'secure endpoint'
       end
     end
 
-    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}"
+    def upload_file(params, request_headers, send_rewritten_field: true, package_name: 'mypackage', file_name: 'myfile.tar.gz')
+      url = "/projects/#{project.id}/packages/generic/#{package_name}/0.0.1/#{file_name}"
 
       workhorse_finalize(
         api(url),
@@ -268,4 +298,138 @@ RSpec.describe API::GenericPackages do
       )
     end
   end
+
+  describe 'GET /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name' do
+    using RSpec::Parameterized::TableSyntax
+
+    let_it_be(:package) { create(:generic_package, project: project) }
+    let_it_be(:package_file) { create(:package_file, :generic, package: package) }
+
+    context 'authentication' do
+      where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do
+        'PUBLIC'  | :developer | true  | :personal_access_token         | :success
+        'PUBLIC'  | :guest     | true  | :personal_access_token         | :success
+        'PUBLIC'  | :developer | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :guest     | true  | :invalid_personal_access_token | :unauthorized
+        'PUBLIC'  | :developer | false | :personal_access_token         | :success
+        'PUBLIC'  | :guest     | false | :personal_access_token         | :success
+        '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                     | :success
+        '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
+          download_file(auth_header)
+
+          expect(response).to have_gitlab_http_status(expected_status)
+        end
+      end
+    end
+
+    context 'event tracking' do
+      before do
+        project.add_developer(user)
+      end
+
+      subject { download_file(personal_access_token_header) }
+
+      it_behaves_like 'a gitlab tracking event', described_class.name, 'pull_package'
+    end
+
+    it 'rejects a malicious file name request' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, file_name: '../.ssh%2fauthorized_keys')
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
+    it 'rejects a malicious file name request' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, file_name: '%2e%2e%2f.ssh%2fauthorized_keys')
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
+    it 'rejects a malicious package name request' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, package_name: 'my-package/../')
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
+    it 'rejects a malicious package name request' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, package_name: 'my-package%2f%2e%2e%2f')
+
+      expect(response).to have_gitlab_http_status(:bad_request)
+    end
+
+    context 'application security' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:param_name, :param_value) do
+        :package_name | 'my-package/../'
+        :package_name | 'my-package%2f%2e%2e%2f'
+        :file_name    | '../.ssh%2fauthorized_keys'
+        :file_name    | '%2e%2e%2f.ssh%2fauthorized_keys'
+      end
+
+      with_them do
+        subject { download_file(personal_access_token_header, param_name => param_value) }
+
+        it_behaves_like 'secure endpoint'
+      end
+    end
+
+    it 'responds with 404 Not Found for non existing package' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, package_name: 'no-such-package')
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    it 'responds with 404 Not Found for non existing package file' do
+      project.add_developer(user)
+
+      download_file(personal_access_token_header, file_name: 'no-such-file')
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+
+    def download_file(request_headers, package_name: nil, file_name: nil)
+      package_name ||= package.name
+      file_name ||= package_file.file_name
+      url = "/projects/#{project.id}/packages/generic/#{package_name}/#{package.version}/#{file_name}"
+
+      get api(url), headers: request_headers
+    end
+  end
 end