Add Workhorse authorization

- Add authorize route
- Add controller specs

app/controllers/concerns/workhorse_import_export_upload.rb → app/controllers/concerns/workhorse_authorization.rb

 # frozen_string_literal: true
 
-module WorkhorseImportExportUpload
+module WorkhorseAuthorization
   extend ActiveSupport::Concern
   include WorkhorseRequest
 
@@ -12,10 +12,9 @@ module WorkhorseImportExportUpload
   def authorize
     set_workhorse_internal_api_content_type
 
-    authorized = ImportExportUploader.workhorse_authorize(
+    authorized = uploader_class.workhorse_authorize(
       has_length: false,
-      maximum_size: Gitlab::CurrentSettings.max_import_size.megabytes
-    )
+      maximum_size: Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i)
 
     render json: authorized
   rescue SocketError
@@ -27,7 +26,14 @@ module WorkhorseImportExportUpload
   def file_is_valid?(file)
     return false unless file.is_a?(::UploadedFile)
 
+    file_extension_whitelist.include?(File.extname(file.original_filename).downcase.delete('.'))
+  end
+
+  def uploader_class
+    raise NotImplementedError
+  end
+
+  def file_extension_whitelist
     ImportExportUploader::EXTENSION_WHITELIST
-      .include?(File.extname(file.original_filename).delete('.'))
   end
 end

app/controllers/import/gitlab_groups_controller.rb

 # frozen_string_literal: true
 
 class Import::GitlabGroupsController < ApplicationController
-  include WorkhorseImportExportUpload
+  include WorkhorseAuthorization
 
   before_action :ensure_group_import_enabled
   before_action :import_rate_limit, only: %i[create]
@@ -64,4 +64,8 @@ class Import::GitlabGroupsController < ApplicationController
       redirect_to new_group_path
     end
   end
+
+  def uploader_class
+    ImportExportUploader
+  end
 end

app/controllers/import/gitlab_projects_controller.rb

 # frozen_string_literal: true
 
 class Import::GitlabProjectsController < Import::BaseController
-  include WorkhorseImportExportUpload
+  include WorkhorseAuthorization
 
   before_action :whitelist_query_limiting, only: [:create]
   before_action :verify_gitlab_project_import_enabled
@@ -45,4 +45,8 @@ class Import::GitlabProjectsController < Import::BaseController
   def whitelist_query_limiting
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42437')
   end
+
+  def uploader_class
+    ImportExportUploader
+  end
 end

ee/app/controllers/projects/requirements_management/requirements_controller.rb

 # frozen_string_literal: true
 
 class Projects::RequirementsManagement::RequirementsController < Projects::ApplicationController
+  include WorkhorseAuthorization
+
+  EXTENSION_WHITELIST = %w[csv].map(&:downcase).freeze
+
   before_action :authorize_read_requirement!
-  before_action :authorize_import_access!, only: [:import_csv]
+  before_action :authorize_import_access!, only: [:import_csv, :authorize]
   before_action do
     push_frontend_feature_flag(:import_requirements_csv, project)
   end
@@ -16,10 +20,12 @@ class Projects::RequirementsManagement::RequirementsController < Projects::Appli
   end
 
   def import_csv
+    verify_valid_file!
+
     if uploader = UploadService.new(project, params[:file]).execute
       RequirementsManagement::ImportRequirementsCsvWorker.perform_async(current_user.id, project.id, uploader.upload.id) # rubocop:disable CodeReuse/Worker
 
-      flash[:notice] = _("Your requirements are being imported. Once finished, you'll get a confirmation email.")
+      flash[:notice] = _("Your requirements are being imported. Once finished, you'll receive a confirmation email.")
     else
       flash[:alert] = _("File upload error.")
     end
@@ -28,12 +34,31 @@ class Projects::RequirementsManagement::RequirementsController < Projects::Appli
   end
 
   def authorize_import_access!
-    ensure_import_enabled
+    render_404 unless Feature.enabled?(:import_requirements_csv, project, default_enabled: false)
 
     return if can?(current_user, :import_requirements, project)
+
+    if current_user || action_name == 'authorize'
+      render_404
+    else
+      authenticate_user!
+    end
   end
 
-  def ensure_import_enabled
-    render_404 unless Feature.enabled?(:import_requirements_csv, project, default_enabled: false)
+  def verify_valid_file!
+    return if file_is_valid?(params[:file])
+
+    supported_file_extensions = ".#{EXTENSION_WHITELIST.join(', .')}"
+    flash[:alert] = _("The uploaded file was invalid. Supported file extensions are %{extensions}.") % { extensions: supported_file_extensions }
+
+    redirect_to project_requirements_management_requirements_path(project)
+  end
+
+  def uploader_class
+    FileUploader
+  end
+
+  def file_extension_whitelist
+    EXTENSION_WHITELIST
   end
 end

ee/config/routes/project.rb

@@ -15,6 +15,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           resources :requirements, only: [:index] do
             collection do
               post :import_csv
+              post :authorize
             end
           end
         end

ee/spec/controllers/projects/requirements_management/requirements_controller_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 RSpec.describe Projects::RequirementsManagement::RequirementsController do
   let_it_be(:user) { create(:user) }
 
+  subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
+
   shared_examples 'response with 404 status' do
     it 'returns 404' do
       subject
@@ -14,8 +16,6 @@ RSpec.describe Projects::RequirementsManagement::RequirementsController do
   end
 
   describe 'GET #index' do
-    subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
-
     context 'private project' do
       let(:project) { create(:project) }
 
@@ -132,89 +132,4 @@ RSpec.describe Projects::RequirementsManagement::RequirementsController do
       end
     end
   end
-
-  describe 'POST #import_csv' do
-    let_it_be(:project) { create(:project, :public) }
-    let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
-
-    subject { import_csv }
-
-    context 'unauthorized' do
-      context 'when user is not signed in' do
-        before do
-          sign_out(:user)
-        end
-
-        it_behaves_like 'response with 404 status'
-      end
-
-      context 'with project members with guest role' do
-        before do
-          sign_in(user)
-          project.add_guest(user)
-        end
-
-        it_behaves_like 'response with 404 status'
-      end
-    end
-
-    context 'authorized' do
-      before do
-        sign_in(user)
-        project.add_reporter(user)
-      end
-
-      context 'when requirements feature is available' do
-        before do
-          stub_licensed_features(requirements: true)
-        end
-
-        context 'when import_requirements_csv feature flag is enabled' do
-          before do
-            stub_feature_flags(import_requirements_csv: true)
-          end
-
-          it "returns 302 for project members with reporter role" do
-            subject
-
-            expect(flash[:notice]).to eq(_("Your requirements are being imported. Once finished, you'll get a confirmation email."))
-            expect(response).to redirect_to(project_requirements_management_requirements_path(project))
-          end
-
-          it "shows error when upload fails" do
-            expect_next_instance_of(UploadService) do |upload_service|
-              expect(upload_service).to receive(:execute).and_return(nil)
-            end
-
-            subject
-
-            expect(flash[:alert]).to include(_('File upload error.'))
-            expect(response).to redirect_to(project_requirements_management_requirements_path(project))
-          end
-        end
-
-        context 'when import_requirements_csv feature flag is disabled' do
-          before do
-            stub_feature_flags(import_requirements_csv: false)
-          end
-
-          it_behaves_like 'response with 404 status'
-        end
-      end
-
-      context 'when requirements feature is available' do
-        before do
-          stub_licensed_features(requirements: false)
-        end
-
-        it_behaves_like 'response with 404 status'
-      end
-    end
-
-    def import_csv
-      post :import_csv, params: { namespace_id: project.namespace.to_param,
-                                  project_id: project.to_param,
-                                  file: file }
-    end
-  end
 end

ee/spec/requests/projects/requirements_management/requirements_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::RequirementsManagement::RequirementsController do
+  include WorkhorseHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :public) }
+  let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
+  let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
+
+  shared_examples 'response with 404 status' do
+    it 'returns 404' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  describe 'POST #import_csv' do
+    let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
+    let(:params) { { namespace_id: project.namespace.id, path: 'test' } }
+
+    subject { upload_file(file, workhorse_headers, params) }
+
+    before do
+      stub_feature_flags(import_requirements_csv: true)
+    end
+
+    context 'unauthorized' do
+      context 'when user is not signed in' do
+        it_behaves_like 'response with 404 status'
+      end
+
+      context 'with project member with a guest role' do
+        before do
+          login_as(user)
+          project.add_guest(user)
+        end
+
+        it_behaves_like 'response with 404 status'
+      end
+    end
+
+    context 'authorized' do
+      before do
+        login_as(user)
+        project.add_reporter(user)
+      end
+
+      context 'when requirements feature is available and member is a reporter' do
+        before do
+          stub_licensed_features(requirements: true)
+        end
+
+        shared_examples 'response with 302 status' do
+          it 'returns 302 status and redirects to the correct path' do
+            subject
+
+            expect(flash[:notice]).to eq(_("Your requirements are being imported. Once finished, you'll receive a confirmation email."))
+            expect(response).to redirect_to(project_requirements_management_requirements_path(project))
+            expect(response).to have_gitlab_http_status(:found)
+          end
+        end
+
+        it_behaves_like 'response with 302 status'
+
+        context 'when file extension is in upper case' do
+          let(:file) { fixture_file_upload('spec/fixtures/csv_uppercase.CSV') }
+
+          it_behaves_like 'response with 302 status'
+        end
+
+        it 'shows error when upload fails' do
+          expect_next_instance_of(UploadService) do |upload_service|
+            expect(upload_service).to receive(:execute).and_return(nil)
+          end
+
+          subject
+
+          expect(flash[:alert]).to include(_('File upload error.'))
+          expect(response).to redirect_to(project_requirements_management_requirements_path(project))
+        end
+      end
+
+      context 'when requirements feature is not available' do
+        before do
+          stub_licensed_features(requirements: false)
+        end
+
+        it_behaves_like 'response with 404 status'
+      end
+    end
+
+    context 'when requirements import FF is disabled' do
+      before do
+        stub_feature_flags(import_requirements_csv: false)
+      end
+
+      it_behaves_like 'response with 404 status'
+    end
+
+    def upload_file(file, headers = {}, params = {})
+      workhorse_finalize(
+        import_csv_project_requirements_management_requirements_path(project),
+        method: :post,
+        file_key: :file,
+        params: params.merge(file: file),
+        headers: headers,
+        send_rewritten_field: true
+      )
+    end
+  end
+
+  describe 'POST #authorize' do
+    subject do
+      post authorize_project_requirements_management_requirements_path(project),
+        headers: workhorse_headers
+    end
+
+    context 'with an authorized user' do
+      before do
+        project.add_reporter(user)
+      end
+
+      context 'when feature is available' do
+        before do
+          stub_licensed_features(requirements: true)
+          stub_feature_flags(import_requirements_csv: true)
+        end
+
+        it_behaves_like 'handle uploads authorize request' do
+          let(:uploader_class) { FileUploader }
+        end
+      end
+
+      context 'when feature is disabled' do
+        before do
+          stub_licensed_features(requirements: true)
+          stub_feature_flags(import_requirements_csv: true)
+        end
+
+        it_behaves_like 'response with 404 status'
+      end
+    end
+
+    context 'with an authorized user' do
+      it_behaves_like 'response with 404 status'
+    end
+  end
+end

locale/gitlab.pot

@@ -27254,6 +27254,9 @@ msgstr ""
 msgid "The update action will time out after %{number_of_minutes} minutes. For big repositories, use a clone/push combination."
 msgstr ""
 
+msgid "The uploaded file was invalid. Supported file extensions are %{extensions}."
+msgstr ""
+
 msgid "The usage ping is disabled, and cannot be configured through this form."
 msgstr ""
 
@@ -31653,6 +31656,9 @@ msgstr ""
 msgid "Your request for access has been queued for review."
 msgstr ""
 
+msgid "Your requirements are being imported. Once finished, you'll receive a confirmation email."
+msgstr ""
+
 msgid "Your response has been recorded."
 msgstr ""
 

spec/fixtures/csv_uppercase.CSV

+TITLE,DESCRIPTION
+Issue in 中文,Test description
+"Hello","World"
+"Title with quote""",Description

spec/requests/import/gitlab_groups_controller_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Import::GitlabGroupsController do
   include WorkhorseHelpers
 
+  let_it_be(:user) { create(:user) }
   let(:import_path) { "#{Dir.tmpdir}/gitlab_groups_controller_spec" }
   let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
   let(:workhorse_headers) do
@@ -28,8 +29,6 @@ RSpec.describe Import::GitlabGroupsController do
   describe 'POST create' do
     subject(:import_request) { upload_archive(file_upload, workhorse_headers, request_params) }
 
-    let_it_be(:user) { create(:user) }
-
     let(:file) { File.join('spec', %w[fixtures group_export.tar.gz]) }
     let(:file_upload) { fixture_file_upload(file) }
 
@@ -194,67 +193,10 @@ RSpec.describe Import::GitlabGroupsController do
   end
 
   describe 'POST authorize' do
-    let_it_be(:user) { create(:user) }
-
-    before do
-      login_as(user)
-    end
-
-    context 'when using a workhorse header' do
-      subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers }
-
-      it 'authorizes the request' do
-        authorize_request
-
-        expect(response).to have_gitlab_http_status :ok
-        expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-        expect(json_response['TempPath']).to eq ImportExportUploader.workhorse_local_upload_path
-      end
-    end
-
-    context 'when the request bypasses gitlab-workhorse' do
-      subject(:authorize_request) { post authorize_import_gitlab_group_path }
-
-      it 'rejects the request' do
-        expect { authorize_request }.to raise_error(JWT::DecodeError)
-      end
-    end
-
-    context 'when direct upload is enabled' do
-      subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers }
+    it_behaves_like 'handle uploads authorize request' do
+      let(:uploader_class) { ImportExportUploader }
 
-      before do
-        stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: true)
-      end
-
-      it 'accepts the request and stores the files' do
-        authorize_request
-
-        expect(response).to have_gitlab_http_status :ok
-        expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-        expect(json_response).not_to have_key 'TempPath'
-
-        expect(json_response['RemoteObject'].keys)
-          .to include('ID', 'GetURL', 'StoreURL', 'DeleteURL', 'MultipartUpload')
-      end
-    end
-
-    context 'when direct upload is disabled' do
-      subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers }
-
-      before do
-        stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: false)
-      end
-
-      it 'handles the local file' do
-        authorize_request
-
-        expect(response).to have_gitlab_http_status :ok
-        expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-
-        expect(json_response['TempPath']).to eq ImportExportUploader.workhorse_local_upload_path
-        expect(json_response['RemoteObject']).to be_nil
-      end
+      subject { post authorize_import_gitlab_group_path, headers: workhorse_headers }
     end
   end
 end

spec/requests/import/gitlab_projects_controller_spec.rb

@@ -84,56 +84,10 @@ RSpec.describe Import::GitlabProjectsController do
   end
 
   describe 'POST authorize' do
-    subject { post authorize_import_gitlab_project_path, headers: workhorse_headers }
+    it_behaves_like 'handle uploads authorize request' do
+      let(:uploader_class) { ImportExportUploader }
 
-    it 'authorizes importing project with workhorse header' do
-      subject
-
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-      expect(json_response['TempPath']).to eq(ImportExportUploader.workhorse_local_upload_path)
-    end
-
-    it 'rejects requests that bypassed gitlab-workhorse' do
-      workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
-
-      expect { subject }.to raise_error(JWT::DecodeError)
-    end
-
-    context 'when using remote storage' do
-      context 'when direct upload is enabled' do
-        before do
-          stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: true)
-        end
-
-        it 'responds with status 200, location of file remote store and object details' do
-          subject
-
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-          expect(json_response).not_to have_key('TempPath')
-          expect(json_response['RemoteObject']).to have_key('ID')
-          expect(json_response['RemoteObject']).to have_key('GetURL')
-          expect(json_response['RemoteObject']).to have_key('StoreURL')
-          expect(json_response['RemoteObject']).to have_key('DeleteURL')
-          expect(json_response['RemoteObject']).to have_key('MultipartUpload')
-        end
-      end
-
-      context 'when direct upload is disabled' do
-        before do
-          stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: false)
-        end
-
-        it 'handles as a local file' do
-          subject
-
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-          expect(json_response['TempPath']).to eq(ImportExportUploader.workhorse_local_upload_path)
-          expect(json_response['RemoteObject']).to be_nil
-        end
-      end
+      subject { post authorize_import_gitlab_project_path, headers: workhorse_headers }
     end
   end
 end

spec/support/shared_examples/requests/uploads_auhorize_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'handle uploads authorize request' do
+  before do
+    login_as(user)
+  end
+
+  describe 'POST authorize' do
+    it 'authorizes workhorse header' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+      expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path)
+    end
+
+    it 'rejects requests that bypassed gitlab-workhorse' do
+      workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
+
+      expect { subject }.to raise_error(JWT::DecodeError)
+    end
+
+    context 'when using remote storage' do
+      context 'when direct upload is enabled' do
+        before do
+          stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true)
+        end
+
+        it 'responds with status 200, location of file remote store and object details' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          expect(json_response).not_to have_key('TempPath')
+          expect(json_response['RemoteObject']).to have_key('ID')
+          expect(json_response['RemoteObject']).to have_key('GetURL')
+          expect(json_response['RemoteObject']).to have_key('StoreURL')
+          expect(json_response['RemoteObject']).to have_key('DeleteURL')
+          expect(json_response['RemoteObject']).to have_key('MultipartUpload')
+        end
+      end
+
+      context 'when direct upload is disabled' do
+        before do
+          stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: false)
+        end
+
+        it 'handles as a local file' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path)
+          expect(json_response['RemoteObject']).to be_nil
+        end
+      end
+    end
+  end
+end