Merge branch '8794-download-package-files-from-ui' into 'master'

Resolve "Download package files from UI"

Closes #8794

See merge request gitlab-org/gitlab-ee!8888

config/routes/project.rb

@@ -314,10 +314,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       scope '-' do
         get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive'
 
-        ## EE-specific
-        resources :packages, only: [:index, :show, :destroy], module: :packages
-        ## EE-specific
-
         resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do
           collection do
             post :cancel_all

ee/app/controllers/concerns/packages_access.rb

+# frozen_string_literal: true
+
+module PackagesAccess
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :verify_packages_enabled!
+    before_action :authorize_read_package!
+  end
+
+  private
+
+  def verify_packages_enabled!
+    render_404 unless Gitlab.config.packages.enabled &&
+        project.feature_available?(:packages)
+  end
+end

ee/app/controllers/projects/packages/package_files_controller.rb

+# frozen_string_literal: true
+
+module Projects
+  module Packages
+    class PackageFilesController < ApplicationController
+      include PackagesAccess
+      include SendFileUpload
+
+      def download
+        package_file = project.package_files.find(params[:id])
+
+        send_upload(package_file.file, attachment: package_file.file_name)
+      end
+    end
+  end
+end

ee/app/controllers/projects/packages/packages_controller.rb

@@ -3,8 +3,8 @@
 module Projects
   module Packages
     class PackagesController < ApplicationController
-      before_action :verify_packages_enabled!
-      before_action :authorize_read_package!
+      include PackagesAccess
+
       before_action :authorize_destroy_package!, only: [:destroy]
 
       def index
@@ -23,12 +23,6 @@ module Projects
 
         redirect_to project_packages_path(@project), status: 302, notice: _('Package was removed')
       end
-
-      private
-
-      def verify_packages_enabled!
-        render_404 unless Gitlab.config.packages.enabled
-      end
     end
   end
 end

ee/app/models/ee/project.rb

@@ -54,6 +54,7 @@ module EE
       has_many :software_license_policies, inverse_of: :project, class_name: 'SoftwareLicensePolicy'
       accepts_nested_attributes_for :software_license_policies, allow_destroy: true
       has_many :packages, class_name: 'Packages::Package'
+      has_many :package_files, through: :packages, class_name: 'Packages::PackageFile'
 
       has_many :sourced_pipelines, class_name: 'Ci::Sources::Pipeline', foreign_key: :source_project_id
 

ee/app/views/projects/packages/packages/show.html.haml

@@ -67,7 +67,7 @@
       %tr
         %td
           = icon('file-o fw')
-          = package_file.file.identifier
+          = link_to package_file.file.identifier, download_project_package_file_path(@project, package_file)
         %td
           = number_to_human_size(package_file.size, precision: 2)
         %td

ee/changelogs/unreleased/8794-download-package-files-from-ui.yml

+---
+title: Allow downloading package files from UI
+merge_request: 8888
+author:
+type: changed

ee/config/routes/project.rb

@@ -31,6 +31,15 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           post :check_config
         end
       end
+
+      scope '-' do
+        resources :packages, only: [:index, :show, :destroy], module: :packages
+        resources :package_files, only: [], module: :packages do
+          member do
+            get :download
+          end
+        end
+      end
     end
   end
 end

ee/spec/features/projects/package_files_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'PackageFiles' do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let!(:package) { create(:maven_package, project: project) }
+  let!(:package_file) { package.package_files.first }
+
+  before do
+    sign_in(user)
+    stub_licensed_features(packages: true)
+  end
+
+  context 'user with master role' do
+    before do
+      project.add_master(user)
+    end
+
+    it 'allows file download from package page' do
+      visit project_package_path(project, package)
+
+      click_link package_file.file_name
+
+      expect(status_code).to eq(200)
+      expect(page.response_headers['Content-Type']).to eq 'application/xml'
+      expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary'
+    end
+
+    it 'allows direct download by url' do
+      visit download_project_package_file_path(project, package_file)
+
+      expect(status_code).to eq(200)
+    end
+
+    it 'does not allow download of package belonging to different project' do
+      another_package = create(:maven_package)
+      another_file = another_package.package_files.first
+
+      visit download_project_package_file_path(project, another_file)
+
+      expect(status_code).to eq(404)
+    end
+
+    it 'gives 404 when packages feature is not available' do
+      stub_licensed_features(packages: false)
+
+      visit download_project_package_file_path(project, package_file)
+
+      expect(status_code).to eq(404)
+    end
+  end
+
+  it 'does not allow direct download when no access to the project' do
+    visit download_project_package_file_path(project, package_file)
+
+    expect(status_code).to eq(404)
+  end
+
+  it 'gives 404 when no package file exist' do
+    visit download_project_package_file_path(project, '9999')
+
+    expect(status_code).to eq(404)
+  end
+end

ee/spec/features/projects/packages_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Packages' do
@@ -7,6 +9,31 @@ describe 'Packages' do
   before do
     sign_in(user)
     project.add_master(user)
+    stub_licensed_features(packages: true)
+  end
+
+  context 'packages feature is not available because of license' do
+    before do
+      stub_licensed_features(packages: false)
+    end
+
+    it 'gives 404' do
+      visit_project_packages
+
+      expect(status_code).to eq(404)
+    end
+  end
+
+  context 'packages feature is disabled by config' do
+    before do
+      allow(Gitlab.config.packages).to receive(:enabled).and_return(false)
+    end
+
+    it 'gives 404' do
+      visit_project_packages
+
+      expect(status_code).to eq(404)
+    end
   end
 
   context 'when there are no packages' do
@@ -21,8 +48,6 @@ describe 'Packages' do
     let!(:package) { create(:maven_package, project: project) }
 
     before do
-      package
-
       visit_project_packages
     end
 

ee/spec/lib/gitlab/import_export/all_models.yml

@@ -68,6 +68,7 @@ project:
 - software_license_policies
 - project_registry
 - packages
+- package_files
 - tracing_setting
 - webide_pipelines
 - reviews

ee/spec/models/project_spec.rb

@@ -25,6 +25,8 @@ describe Project do
     it { is_expected.to have_many(:audit_events).dependent(false) }
     it { is_expected.to have_many(:protected_environments) }
     it { is_expected.to have_many(:approver_groups).dependent(:destroy) }
+    it { is_expected.to have_many(:packages).class_name('Packages::Package') }
+    it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') }
   end
 
   describe 'validations' do