From 0a16f589e9f19a9e0f9c27ee7246b2480eba8cc5 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 6 Sep 2022 00:00:59 +0200 Subject: [PATCH] Use X-Sendfile to transmit native files and handle file uploads --- .../code_ocean/files_controller.rb | 9 +++++++++ app/controllers/submissions_controller.rb | 6 +++++- app/models/code_ocean/file.rb | 9 +++++++-- app/policies/code_ocean/file_policy.rb | 12 +++++++++++ config/environments/production.rb | 2 +- config/environments/staging.rb | 2 +- config/routes.rb | 1 + .../code_ocean/files_controller_spec.rb | 20 +++++++++++++++++++ .../submissions_controller_spec.rb | 6 ++---- 9 files changed, 58 insertions(+), 9 deletions(-) diff --git a/app/controllers/code_ocean/files_controller.rb b/app/controllers/code_ocean/files_controller.rb index 79331f6a..4d87684e 100644 --- a/app/controllers/code_ocean/files_controller.rb +++ b/app/controllers/code_ocean/files_controller.rb @@ -10,6 +10,15 @@ module CodeOcean end private :authorize! + def show_protected_upload + @file = CodeOcean::File.find(params[:id]) + authorize! + raise Pundit::NotAuthorizedError if @embed_options[:disable_download] || @file.name_with_extension != params[:filename] + + real_location = Pathname(@file.native_file.current_path).realpath + send_file(real_location, type: @file.native_file.content_type, filename: @file.name_with_extension, disposition: 'attachment') + end + def create @file = CodeOcean::File.new(file_params) if @file.file_template_id diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 2fa21912..fe6d67e8 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -56,7 +56,11 @@ class SubmissionsController < ApplicationController def download_file raise Pundit::NotAuthorizedError if @embed_options[:disable_download] - send_data(@file.read, filename: @file.name_with_extension) + if @file.native_file? + redirect_to protected_upload_path(id: @file.id, filename: @file.name_with_extension) + else + send_data(@file.content, filename: @file.name_with_extension) + end end def index diff --git a/app/models/code_ocean/file.rb b/app/models/code_ocean/file.rb index bdbc3657..5083f8c4 100644 --- a/app/models/code_ocean/file.rb +++ b/app/models/code_ocean/file.rb @@ -58,8 +58,7 @@ module CodeOcean def read if native_file? - valid = Pathname(native_file.current_path).realpath.fnmatch? ::File.join(native_file.root, '**') - return nil unless valid + return nil unless native_file_location_valid? native_file.read else @@ -67,6 +66,12 @@ module CodeOcean end end + def native_file_location_valid? + real_location = Pathname(native_file.current_path).realpath + upload_location = Pathname(::File.join(native_file.root, 'uploads')).realpath + real_location.fnmatch? ::File.join(upload_location.to_s, '**') + end + def ancestor_id file_id || id end diff --git a/app/policies/code_ocean/file_policy.rb b/app/policies/code_ocean/file_policy.rb index c9faf30d..d5be20cb 100644 --- a/app/policies/code_ocean/file_policy.rb +++ b/app/policies/code_ocean/file_policy.rb @@ -7,6 +7,8 @@ module CodeOcean end def show? + return false if @record.native_file? && !@record.native_file_location_valid? + if @record.context.is_a?(Exercise) admin? || author? || !@record.hidden else @@ -14,6 +16,16 @@ module CodeOcean end end + def show_protected_upload? + return false if @record.native_file? && !@record.native_file_location_valid? + + if @record.context.is_a?(Exercise) + admin? || author? || (!@record.context.unpublished && !@record.hidden) + else + admin? || author? + end + end + def create? if @record.context.is_a?(Exercise) admin? || author? diff --git a/config/environments/production.rb b/config/environments/production.rb index a06058c7..17fc2800 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -38,7 +38,7 @@ Rails.application.configure do # Specifies the header that your server uses for sending files. # config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache - # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX + config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX # Store uploaded files on the local file system (see config/storage.yml for options). config.active_storage.service = :local diff --git a/config/environments/staging.rb b/config/environments/staging.rb index a5b908ee..b2ee0889 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -53,7 +53,7 @@ Rails.application.configure do # Specifies the header that your server uses for sending files. # config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache - # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX + config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX # Store uploaded files on the local file system (see config/storage.yml for options). config.active_storage.service = :local diff --git a/config/routes.rb b/config/routes.rb index 69c3795b..dc276e13 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -128,6 +128,7 @@ Rails.application.routes.draw do namespace :code_ocean do resources :files, only: %i[create destroy] end + get '/uploads/files/:id/:filename', to: 'code_ocean/files#show_protected_upload', as: :protected_upload, constraints: {filename: FILENAME_REGEXP} resources :file_types diff --git a/spec/controllers/code_ocean/files_controller_spec.rb b/spec/controllers/code_ocean/files_controller_spec.rb index 2c555b08..476b72bc 100644 --- a/spec/controllers/code_ocean/files_controller_spec.rb +++ b/spec/controllers/code_ocean/files_controller_spec.rb @@ -7,6 +7,26 @@ describe CodeOcean::FilesController do before { allow(controller).to receive(:current_user).and_return(user) } + describe 'GET #show_protected_upload' do + context 'with a valid filename' do + let(:submission) { create(:submission, exercise: create(:audio_video)) } + + before { get :show_protected_upload, params: {filename: file.name_with_extension, id: file.id} } + + context 'with a binary file' do + let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.mp4' } } + + expect_assigns(file: :file) + expect_content_type('video/mp4') + expect_http_status(:ok) + + it 'sets the correct filename' do + expect(response.headers['Content-Disposition']).to include("attachment; filename=\"#{file.name_with_extension}\"") + end + end + end + end + describe 'POST #create' do let(:submission) { create(:submission, user: user) } diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 29244a11..a55e1cf9 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -74,11 +74,9 @@ describe SubmissionsController do expect_assigns(file: :file) expect_assigns(submission: :submission) - expect_content_type('video/mp4') - expect_http_status(:ok) - it 'sets the correct filename' do - expect(response.headers['Content-Disposition']).to include("attachment; filename=\"#{file.name_with_extension}\"") + it 'sets the correct redirect' do + expect(response.location).to eq protected_upload_url(id: file, filename: file.name_with_extension) end end