Merge branch '13345-conan-file-download-endpoint' into 'master'

Conan file download endpoint

See merge request gitlab-org/gitlab!17798

ee/app/finders/packages/package_file_finder.rb

 # frozen_string_literal: true
 class Packages::PackageFileFinder
-  attr_reader :package, :file_name
+  attr_reader :package, :file_name, :params
 
-  def initialize(package, file_name)
+  def initialize(package, file_name, params = {})
     @package = package
     @file_name = file_name
+    @params = params
   end
 
   def execute
@@ -17,9 +18,22 @@ class Packages::PackageFileFinder
 
   private
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def package_files
-    package.package_files.where(file_name: file_name)
+    files = package.package_files
+
+    files = by_file_name(files)
+    files = by_conan_file_type(files)
+
+    files
+  end
+
+  def by_file_name(files)
+    files.where(file_name: file_name) # rubocop: disable CodeReuse/ActiveRecord
+  end
+
+  def by_conan_file_type(files)
+    return files unless params[:conan_file_type]
+
+    files.with_conan_file_type(params[:conan_file_type])
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end

ee/app/models/packages/package_file.rb

@@ -3,6 +3,7 @@ class Packages::PackageFile < ApplicationRecord
   include UpdateProjectStatistics
 
   delegate :project, :project_id, to: :package
+  delegate :conan_file_type, to: :conan_file_metadatum
 
   update_project_statistics project_statistics_name: :packages_size
 
@@ -20,6 +21,11 @@ class Packages::PackageFile < ApplicationRecord
   scope :with_files_stored_locally, -> { where(file_store: ::Packages::PackageFileUploader::Store::LOCAL) }
   scope :with_conan_file_metadata, -> { includes(:conan_file_metadatum) }
 
+  scope :with_conan_file_type, ->(file_type) do
+    joins(:conan_file_metadatum)
+      .where(packages_conan_file_metadata: { conan_file_type: ::Packages::ConanFileMetadatum.conan_file_types[file_type] })
+  end
+
   mount_uploader :file, Packages::PackageFileUploader
 
   after_save :update_file_store, if: :saved_change_to_file?

ee/app/presenters/conan_package_presenter.rb

@@ -66,8 +66,12 @@ class ConanPackagePresenter
 
   def map_package_files
     package_files.to_a.map do |package_file|
-      [package_file.file_name, yield(package_file)]
-    end.to_h.compact
+      key = package_file.file_name
+      value = yield(package_file)
+      next unless key && value
+
+      [key, value]
+    end.compact.to_h
   end
 
   def package_files

ee/lib/api/conan_packages.rb

@@ -20,7 +20,7 @@ module API
     }.freeze
 
     FILE_NAME_REQUIREMENTS = {
-      file_name: Gitlab::Regex.conan_file_name_regex
+      file_name: API::NO_SLASH_URL_PART_REGEX
     }.freeze
 
     PACKAGE_COMPONENT_REGEX = Gitlab::Regex.conan_recipe_component_regex
@@ -197,7 +197,7 @@ module API
         requires :package_channel, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package channel'
         requires :recipe_revision, type: String, desc: 'Conan Recipe Revision'
       end
-      namespace 'files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision' do
+      namespace 'files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision', requirements: PACKAGE_REQUIREMENTS do
         before do
           authenticate_non_get!
         end
@@ -205,11 +205,13 @@ module API
         params do
           requires :file_name, type: String, desc: 'Package file name'
         end
-        desc 'Download recipe files' do
-          detail 'This feature was introduced in GitLab 12.5'
-        end
-        get 'export/:file_name' do
-          not_found!
+        namespace 'export/:file_name', requirements: FILE_NAME_REQUIREMENTS do
+          desc 'Download recipe files' do
+            detail 'This feature was introduced in GitLab 12.5'
+          end
+          get do
+            download_package_file(:recipe_file)
+          end
         end
 
         params do
@@ -217,11 +219,13 @@ module API
           requires :package_revision, type: String, desc: 'Conan Package Revision'
           requires :file_name, type: String, desc: 'Package file name'
         end
-        desc 'Download package files' do
-          detail 'This feature was introduced in GitLab 12.5'
-        end
-        get 'package/:conan_package_reference/:package_revision/:file_name' do
-          not_found!
+        namespace 'package/:conan_package_reference/:package_revision/:file_name', requirements: FILE_NAME_REQUIREMENTS do
+          desc 'Download package files' do
+            detail 'This feature was introduced in GitLab 12.5'
+          end
+          get do
+            download_package_file(:package_file)
+          end
         end
       end
     end
@@ -316,6 +320,15 @@ module API
         end
       end
 
+      def download_package_file(file_type)
+        authorize!(:read_package, project)
+
+        package_file = ::Packages::PackageFileFinder
+          .new(package, "#{params[:file_name]}", conan_file_type: file_type).execute!
+
+        present_carrierwave_file!(package_file.file)
+      end
+
       def find_personal_access_token
         personal_access_token = find_personal_access_token_from_conan_jwt ||
           find_personal_access_token_from_conan_http_basic_auth

ee/spec/factories/packages.rb

@@ -62,8 +62,8 @@ FactoryBot.define do
           create :conan_file_metadatum, :recipe_file, package_file: package_file
         end
 
-        file { fixture_file_upload('ee/spec/fixtures/conan/recipe_conanfile.py') }
-        file_name { 'recipe_conanfile.py' }
+        file { fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanfile.py') }
+        file_name { 'conanfile.py' }
         file_sha1 { 'be93151dc23ac34a82752444556fe79b32c7a1ad' }
         file_md5 { '12345abcde' }
         file_type { 'py' }
@@ -75,8 +75,8 @@ FactoryBot.define do
           create :conan_file_metadatum, :recipe_file, package_file: package_file
         end
 
-        file { fixture_file_upload('ee/spec/fixtures/conan/recipe_conanmanifest.txt') }
-        file_name { 'recipe_conanmanifest.txt' }
+        file { fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanmanifest.txt') }
+        file_name { 'conanmanifest.txt' }
         file_sha1 { 'be93151dc23ac34a82752444556fe79b32c7a1ad' }
         file_md5 { '12345abcde' }
         file_type { 'txt' }
@@ -88,8 +88,8 @@ FactoryBot.define do
           create :conan_file_metadatum, :package_file, package_file: package_file
         end
 
-        file { fixture_file_upload('ee/spec/fixtures/conan/package_conanmanifest.txt') }
-        file_name { 'package_conanmanifest.txt' }
+        file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conanmanifest.txt') }
+        file_name { 'conanmanifest.txt' }
         file_sha1 { 'be93151dc23ac34a82752444556fe79b32c7a1ad' }
         file_md5 { '12345abcde' }
         file_type { 'txt' }
@@ -101,8 +101,8 @@ FactoryBot.define do
           create :conan_file_metadatum, :package_file, package_file: package_file
         end
 
-        file { fixture_file_upload('ee/spec/fixtures/conan/package_conaninfo.txt') }
-        file_name { 'package_conaninfo.txt' }
+        file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conaninfo.txt') }
+        file_name { 'conaninfo.txt' }
         file_sha1 { 'be93151dc23ac34a82752444556fe79b32c7a1ad' }
         file_md5 { '12345abcde' }
         file_type { 'txt' }
@@ -114,7 +114,7 @@ FactoryBot.define do
           create :conan_file_metadatum, :package_file, package_file: package_file
         end
 
-        file { fixture_file_upload('ee/spec/fixtures/conan/conan_package.tgz') }
+        file { fixture_file_upload('ee/spec/fixtures/conan/package_files/conan_package.tgz') }
         file_name { 'conan_package.tgz' }
         file_sha1 { 'be93151dc23ac34a82752444556fe79b32c7a1ad' }
         file_md5 { '12345abcde' }

ee/spec/finders/packages/package_file_finder_spec.rb

@@ -17,5 +17,17 @@ describe Packages::PackageFileFinder do
 
       expect { finder.execute! }.to raise_error(ActiveRecord::RecordNotFound)
     end
+
+    context 'with conan_file_type' do
+      let(:package) { create(:conan_package) }
+
+      it 'returns a package of the correct file_type' do
+        # conan packages contain a conanmanifest.txt file for both conan_file_types
+        result = described_class.new(package, 'conanmanifest.txt', conan_file_type: :recipe_file).execute!
+
+        expect(result.conan_file_type).to eq('recipe_file')
+        expect(result.conan_file_type).not_to eq('package_file')
+      end
+    end
   end
 end

ee/spec/models/packages/conan_file_metadatum_spec.rb

@@ -10,8 +10,8 @@ RSpec.describe Packages::ConanFileMetadatum, type: :model do
   describe 'validations' do
     let(:package_file) do
       create(:package_file,
-             file: fixture_file_upload('ee/spec/fixtures/conan/recipe_conanfile.py'),
-             file_name: 'recipe_conanfile.py')
+             file: fixture_file_upload('ee/spec/fixtures/conan/recipe_files/conanfile.py'),
+             file_name: 'conanfile.py')
     end
 
     it { is_expected.to validate_presence_of(:package_file) }

ee/spec/presenters/conan_package_presenter_spec.rb

@@ -21,8 +21,8 @@ describe ConanPackagePresenter do
 
       let(:expected_result) do
         {
-          "recipe_conanfile.py" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/recipe_conanfile.py",
-          "recipe_conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/recipe_conanmanifest.txt"
+          "conanfile.py" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanfile.py",
+          "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt"
         }
       end
 
@@ -45,8 +45,8 @@ describe ConanPackagePresenter do
 
       let(:expected_result) do
         {
-          "recipe_conanfile.py" => '12345abcde',
-          "recipe_conanmanifest.txt" => '12345abcde'
+          "conanfile.py" => '12345abcde',
+          "conanmanifest.txt" => '12345abcde'
         }
       end
 
@@ -69,8 +69,8 @@ describe ConanPackagePresenter do
 
       let(:expected_result) do
         {
-          "package_conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/package_conaninfo.txt",
-          "package_conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/package_conanmanifest.txt",
+          "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conaninfo.txt",
+          "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conanmanifest.txt",
           "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conan_package.tgz"
         }
       end
@@ -94,8 +94,8 @@ describe ConanPackagePresenter do
 
       let(:expected_result) do
         {
-          "package_conaninfo.txt" => '12345abcde',
-          "package_conanmanifest.txt" => '12345abcde',
+          "conaninfo.txt" => '12345abcde',
+          "conanmanifest.txt" => '12345abcde',
           "conan_package.tgz" => '12345abcde'
         }
       end

ee/spec/requests/api/conan_packages_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 describe API::ConanPackages do
-  let_it_be(:package) { create(:conan_package) }
+  let(:package) { create(:conan_package) }
   let_it_be(:personal_access_token) { create(:personal_access_token) }
   let_it_be(:user) { personal_access_token.user }
   let(:project) { package.project }
@@ -442,68 +442,108 @@ describe API::ConanPackages do
   context 'file endpoints' do
     let(:jwt) { build_jwt(personal_access_token) }
     let(:headers) { build_auth_headers(jwt.encoded) }
-    let(:package_file_tgz) { package.package_files.find_by(file_type: 'tgz') }
-    let(:metadata) { package_file_tgz.conan_file_metadatum }
+    let(:recipe_path) { package.conan_recipe_path }
 
-    describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/
-:recipe_revision/export/:file_name' do
-      let(:recipe_path) { package.conan_recipe_path }
+    shared_examples 'denies download with no token' do
+      context 'with no private token' do
+        let(:headers) { {} }
 
-      subject do
-        get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/export/#{file_name}"),
-            headers: headers
+        it 'returns 400' do
+          subject
+
+          expect(response).to have_gitlab_http_status(401)
+        end
       end
+    end
 
-      context 'invalid file' do
-        let(:file_name) { 'badfile.txt' }
+    shared_examples 'a public project with packages' do
+      it 'returns the file' do
+        subject
 
-        it 'returns 404 not found' do
-          subject
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.content_type.to_s).to eq('application/octet-stream')
+      end
+    end
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+    shared_examples 'an internal project with packages' do
+      before do
+        project.team.truncate
+        project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
       end
 
-      context 'valid file' do
-        let(:file_name) { package_file_tgz.file_name }
+      it_behaves_like 'denies download with no token'
 
-        it 'returns forbidden' do
-          subject
+      it 'returns the file' do
+        subject
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.content_type.to_s).to eq('application/octet-stream')
       end
     end
 
-    describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/
-:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do
-      let(:recipe_path) { package.conan_recipe_path }
+    shared_examples 'a private project with packages' do
+      before do
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      end
 
-      subject do
-        get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/package/" \
-              "#{metadata.conan_package_reference}/#{metadata.package_revision}/#{file_name}"),
-            headers: headers
+      it_behaves_like 'denies download with no token'
+
+      it 'returns the file' do
+        subject
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.content_type.to_s).to eq('application/octet-stream')
       end
 
-      context 'invalid file' do
-        let(:file_name) { 'badfile.txt' }
+      it 'denies download when not enough permissions' do
+        project.add_guest(user)
 
-        it 'returns 404 not found' do
-          subject
+        subject
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        expect(response).to have_gitlab_http_status(403)
       end
+    end
 
-      context 'valid file' do
-        let(:file_name) { package_file_tgz.file_name }
+    shared_examples 'a project is not found' do
+      let(:recipe_path) { 'not/package/for/project' }
 
-        it 'returns forbidden' do
-          subject
+      it 'returns forbidden' do
+        subject
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/
+:recipe_revision/export/:file_name' do
+      let(:recipe_file) { package.package_files.find_by(file_name: 'conanfile.py') }
+      let(:metadata) { recipe_file.conan_file_metadatum }
+
+      subject do
+        get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/export/#{recipe_file.file_name}"),
+            headers: headers
       end
+
+      it_behaves_like 'a public project with packages'
+      it_behaves_like 'an internal project with packages'
+      it_behaves_like 'a private project with packages'
+      it_behaves_like 'a project is not found'
+    end
+
+    describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/
+:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do
+      let(:package_file) { package.package_files.find_by(file_name: 'conaninfo.txt') }
+      let(:metadata) { package_file.conan_file_metadatum }
+
+      subject do
+        get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/package/#{metadata.conan_package_reference}/#{metadata.package_revision}/#{package_file.file_name}"),
+            headers: headers
+      end
+
+      it_behaves_like 'a public project with packages'
+      it_behaves_like 'an internal project with packages'
+      it_behaves_like 'a private project with packages'
+      it_behaves_like 'a project is not found'
     end
   end