Commit 41d52bbf authored by Jan Provaznik's avatar Jan Provaznik

Add direct upload support for personal snippets

parent 36a729f0
......@@ -127,4 +127,8 @@ module UploadsActions
def model
strong_memoize(:model) { find_model }
end
def workhorse_authorize_request?
action_name == 'authorize'
end
end
......@@ -2,6 +2,7 @@
class UploadsController < ApplicationController
include UploadsActions
include WorkhorseRequest
UnknownUploadModelError = Class.new(StandardError)
......@@ -21,7 +22,8 @@ class UploadsController < ApplicationController
before_action :upload_mount_satisfied?
before_action :find_model
before_action :authorize_access!, only: [:show]
before_action :authorize_create_access!, only: [:create]
before_action :authorize_create_access!, only: [:create, :authorize]
before_action :verify_workhorse_api!, only: [:authorize]
def uploader_class
PersonalFileUploader
......@@ -72,7 +74,7 @@ class UploadsController < ApplicationController
end
def render_unauthorized
if current_user
if current_user || workhorse_authorize_request?
render_404
else
authenticate_user!
......
......@@ -6,6 +6,10 @@ class PersonalFileUploader < FileUploader
options.storage_path
end
def self.workhorse_local_upload_path
File.join(options.storage_path, 'uploads', TMP_UPLOAD_PATH)
end
def self.base_dir(model, _store = nil)
# base_dir is the path seen by the user when rendering Markdown, so
# it should be the same for both local and object storage. It is
......
---
title: Remove EXIF from users/personal snippet uploads.
merge_request:
author:
type: security
......@@ -30,6 +30,10 @@ scope path: :uploads do
to: 'uploads#create',
constraints: { model: /personal_snippet|user/, id: /\d+/ },
as: 'upload'
post ':model/authorize',
to: 'uploads#authorize',
constraints: { model: /personal_snippet|user/ }
end
# Redirect old note attachments path to new uploads path.
......
......@@ -37,6 +37,8 @@ Parameter | Type | Description
`stop_id` | integer | Only uploads with equal or smaller ID will be processed
`dry_run` | boolean | Do not remove EXIF data, only check if EXIF data are present or not, default: true
`sleep_time` | float | Pause for number of seconds after processing each image, default: 0.3 seconds
`uploader` | string | Run sanitization only for uploads of the given uploader (`FileUploader`, `PersonalFileUploader`, `NamespaceFileUploader`)
`since` | date | Run sanitization only for uploads newer than given date (e.g. `2019-05-01`)
If you have too many uploads, you can speed up sanitization by setting
`sleep_time` to a lower value or by running multiple rake tasks in parallel,
......
......@@ -53,15 +53,18 @@ module Gitlab
end
# rubocop: disable CodeReuse/ActiveRecord
def batch_clean(start_id: nil, stop_id: nil, dry_run: true, sleep_time: nil)
def batch_clean(start_id: nil, stop_id: nil, dry_run: true, sleep_time: nil, uploader: nil, since: nil)
relation = Upload.where('lower(path) like ? or lower(path) like ? or lower(path) like ?',
'%.jpg', '%.jpeg', '%.tiff')
relation = relation.where(uploader: uploader) if uploader
relation = relation.where('created_at > ?', since) if since
logger.info "running in dry run mode, no images will be rewritten" if dry_run
find_params = {
start: start_id.present? ? start_id.to_i : nil,
finish: stop_id.present? ? stop_id.to_i : Upload.last&.id
finish: stop_id.present? ? stop_id.to_i : Upload.last&.id,
batch_size: 1000
}
relation.find_each(find_params) do |upload|
......
......@@ -2,7 +2,7 @@ namespace :gitlab do
namespace :uploads do
namespace :sanitize do
desc 'GitLab | Uploads | Remove EXIF from images.'
task :remove_exif, [:start_id, :stop_id, :dry_run, :sleep_time] => :environment do |task, args|
task :remove_exif, [:start_id, :stop_id, :dry_run, :sleep_time, :uploader, :since] => :environment do |task, args|
args.with_defaults(dry_run: 'true')
args.with_defaults(sleep_time: 0.3)
......@@ -11,7 +11,9 @@ namespace :gitlab do
sanitizer = Gitlab::Sanitizers::Exif.new(logger: logger)
sanitizer.batch_clean(start_id: args.start_id, stop_id: args.stop_id,
dry_run: args.dry_run != 'false',
sleep_time: args.sleep_time.to_f)
sleep_time: args.sleep_time.to_f,
uploader: args.uploader,
since: args.since)
end
end
end
......
......@@ -21,8 +21,20 @@ shared_examples 'content publicly cached' do
end
describe UploadsController do
include WorkhorseHelpers
let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
describe 'POST #authorize' do
it_behaves_like 'handle uploads authorize' do
let(:uploader_class) { PersonalFileUploader }
let(:model) { create(:personal_snippet, :public) }
let(:params) do
{ model: 'personal_snippet', id: model.id }
end
end
end
describe 'POST create' do
let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') }
let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') }
......@@ -636,4 +648,10 @@ describe UploadsController do
end
end
end
def post_authorize(verified: true)
request.headers.merge!(workhorse_internal_api_request_header) if verified
post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json
end
end
......@@ -5,7 +5,9 @@ describe Gitlab::Sanitizers::Exif do
describe '#batch_clean' do
context 'with image uploads' do
let!(:uploads) { create_list(:upload, 3, :with_file, :issuable_upload) }
set(:upload1) { create(:upload, :with_file, :issuable_upload) }
set(:upload2) { create(:upload, :with_file, :personal_snippet_upload) }
set(:upload3) { create(:upload, :with_file, created_at: 3.days.ago) }
it 'processes all uploads if range ID is not set' do
expect(sanitizer).to receive(:clean).exactly(3).times
......@@ -16,7 +18,19 @@ describe Gitlab::Sanitizers::Exif do
it 'processes only uploads in the selected range' do
expect(sanitizer).to receive(:clean).once
sanitizer.batch_clean(start_id: uploads[1].id, stop_id: uploads[1].id)
sanitizer.batch_clean(start_id: upload1.id, stop_id: upload1.id)
end
it 'processes only uploads for the selected uploader' do
expect(sanitizer).to receive(:clean).once
sanitizer.batch_clean(uploader: 'PersonalFileUploader')
end
it 'processes only uploads created since specified date' do
expect(sanitizer).to receive(:clean).exactly(2).times
sanitizer.batch_clean(since: 2.days.ago)
end
it 'pauses if sleep_time is set' do
......
......@@ -7,6 +7,8 @@ shared_examples 'handle uploads' do
let(:secret) { FileUploader.generate_secret }
let(:uploader_class) { FileUploader }
it_behaves_like 'handle uploads authorize'
describe "POST #create" do
context 'when a user is not authorized to upload a file' do
it 'returns 404 status' do
......@@ -271,7 +273,9 @@ shared_examples 'handle uploads' do
end
end
end
end
shared_examples 'handle uploads authorize' do
describe "POST #authorize" do
context 'when a user is not authorized to upload a file' do
it 'returns 404 status' do
......@@ -284,7 +288,12 @@ shared_examples 'handle uploads' do
context 'when a user can upload a file' do
before do
sign_in(user)
model.add_developer(user)
if model.is_a?(PersonalSnippet)
model.update!(author: user)
else
model.add_developer(user)
end
end
context 'and the request bypassed workhorse' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment