From 54e8fb0d89422240bd801fca3b9e77554550b1e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 12 Jun 2017 22:41:29 +0200 Subject: [PATCH] FEATURE: new 'allow_staff_to_upload_any_file_in_pm' site setting --- .../components/composer-editor.js.es6 | 5 ++-- .../discourse/lib/utilities.js.es6 | 6 ++++ app/controllers/uploads_controller.rb | 12 +++++--- app/models/upload.rb | 3 +- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 3 ++ lib/email/receiver.rb | 2 +- lib/upload_creator.rb | 13 ++++----- lib/validators/upload_validator.rb | 7 ++++- spec/controllers/uploads_controller_spec.rb | 29 ++++++++++++++----- test/javascripts/lib/utilities-test.js.es6 | 17 +++++++++-- 11 files changed, 71 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index b0213647fe3..b978272f703 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -239,8 +239,9 @@ export default Ember.Component.extend({ }); $element.on('fileuploadsubmit', (e, data) => { - const isUploading = validateUploadedFiles(data.files); - data.formData = { type: "composer" }; + const isPrivateMessage = this.get("composer.privateMessage"); + const isUploading = validateUploadedFiles(data.files, { isPrivateMessage }); + data.formData = { type: "composer", for_private_message: isPrivateMessage }; this.setProperties({ uploadProgress: 0, isUploading }); return isUploading; }); diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index d80b84cabc5..ba409d12da6 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -185,6 +185,12 @@ export function validateUploadedFile(file, opts) { if (!name) { return false; } // check that the uploaded file is authorized + if (Discourse.SiteSettings.allow_staff_to_upload_any_file_in_pm) { + if (opts["isPrivateMessage"] && Discourse.User.current("staff")) { + return true + } + } + if (opts["imagesOnly"]) { if (!isAnImage(name) && !isAuthorizedImage(name)) { bootbox.alert(I18n.t('post.errors.upload_not_authorized', { authorized_extensions: authorizedImagesExtensions() })); diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 4ea747cedf9..7313165d353 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -14,14 +14,15 @@ class UploadsController < ApplicationController url = params[:url] file = params[:file] || params[:files]&.first + for_private_message = params[:for_private_message] if params[:synchronous] && (current_user.staff? || is_api?) - data = create_upload(file, url, type) + data = create_upload(file, url, type, for_private_message) render json: data.as_json else Scheduler::Defer.later("Create Upload") do begin - data = create_upload(file, url, type) + data = create_upload(file, url, type, for_private_message) ensure MessageBus.publish("/uploads/#{type}", (data || {}).as_json, client_ids: [params[:client_id]]) end @@ -58,7 +59,7 @@ class UploadsController < ApplicationController raise Discourse::NotFound end - def create_upload(file, url, type) + def create_upload(file, url, type, for_private_message = false) if file.nil? if url.present? && is_api? maximum_upload_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes @@ -77,7 +78,10 @@ class UploadsController < ApplicationController return { errors: [I18n.t("upload.file_missing")] } if tempfile.nil? - upload = UploadCreator.new(tempfile, filename, type: type, content_type: content_type).create_for(current_user.id) + opts = { type: type, content_type: content_type } + opts[:for_private_message] = true if for_private_message + + upload = UploadCreator.new(tempfile, filename, opts).create_for(current_user.id) if upload.errors.empty? && current_user.admin? retain_hours = params[:retain_hours].to_i diff --git a/app/models/upload.rb b/app/models/upload.rb index d6e1916c720..d40ac15836e 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -13,8 +13,9 @@ class Upload < ActiveRecord::Base has_many :optimized_images, dependent: :destroy - attr_accessor :is_attachment_for_group_message + attr_accessor :for_group_message attr_accessor :for_theme + attr_accessor :for_private_message validates_presence_of :filesize validates_presence_of :original_filename diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3cafb4cd4c5..60d0d19e118 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1191,6 +1191,8 @@ en: png_to_jpg_quality: "Quality of the converted JPG file (1 is lowest quality, 99 is best quality, 100 to disable)." + allow_staff_to_upload_any_file_in_pm: "Allow staff members to upload any files in PM." + enable_flash_video_onebox: "Enable embedding of swf and flv (Adobe Flash) links in oneboxes. WARNING: may introduce security risks." default_invitee_trust_level: "Default trust level (0-4) for invited users." diff --git a/config/site_settings.yml b/config/site_settings.yml index 62a6b009bd8..869b47ba8e9 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -798,6 +798,9 @@ files: default: 95 min: 1 max: 100 + allow_staff_to_upload_any_file_in_pm: + default: true + client: true trust: default_trust_level: diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index ba5fdb9a4cb..ae9caadbd77 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -577,7 +577,7 @@ module Email # read attachment File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded } # create the upload for the user - opts = { is_attachment_for_group_message: options[:is_group_message] } + opts = { for_group_message: options[:is_group_message] } upload = UploadCreator.new(tmp, attachment.filename, opts).create_for(options[:user].id) if upload && upload.errors.empty? # try to inline images diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index a00b13f08fc..247fcc4e860 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -16,8 +16,9 @@ class UploadCreator # - type (string) # - content_type (string) # - origin (string) - # - is_attachment_for_group_message (boolean) + # - for_group_message (boolean) # - for_theme (boolean) + # - for_private_message (boolean) def initialize(file, filename, opts = {}) @upload = Upload.new @file = file @@ -78,13 +79,9 @@ class UploadCreator @upload.width, @upload.height = ImageSizer.resize(*@image_info.size) end - if @opts[:is_attachment_for_group_message] - @upload.is_attachment_for_group_message = true - end - - if @opts[:for_theme] - @upload.for_theme = true - end + @upload.for_private_message = true if @opts[:for_private_message] + @upload.for_group_message = true if @opts[:for_group_message] + @upload.for_theme = true if @opts[:for_theme] return @upload unless @upload.save diff --git a/lib/validators/upload_validator.rb b/lib/validators/upload_validator.rb index cf73111f438..6c2013565db 100644 --- a/lib/validators/upload_validator.rb +++ b/lib/validators/upload_validator.rb @@ -5,8 +5,13 @@ module Validators; end class Validators::UploadValidator < ActiveModel::Validator def validate(upload) + # staff can upload any file in PM + if upload.for_private_message && SiteSetting.allow_staff_to_upload_any_file_in_pm + return true if upload.user&.staff? + end + # check the attachment blacklist - if upload.is_attachment_for_group_message && SiteSetting.allow_all_attachments_for_group_messages + if upload.for_group_message && SiteSetting.allow_all_attachments_for_group_messages return upload.original_filename =~ SiteSetting.attachment_filename_blacklist_regex end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 46f987d16e5..b44436df7b2 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -38,7 +38,7 @@ describe UploadsController do end it 'parameterize the type' do - subject.expects(:create_upload).with(logo, nil, "super_long_type_with_charssuper_long_type_with_char") + subject.expects(:create_upload).with(logo, nil, "super_long_type_with_charssuper_long_type_with_char", nil) xhr :post, :create, file: logo, type: "super \# long \//\\ type with \\. $%^&*( chars" * 5 end @@ -52,11 +52,11 @@ describe UploadsController do expect(response.status).to eq 200 expect(message.channel).to eq("/uploads/avatar") - expect(message.data).to be + expect(message.data["id"]).to be end it 'is successful with an attachment' do - SiteSetting.stubs(:authorized_extensions).returns("*") + SiteSetting.authorized_extensions = "*" Jobs.expects(:enqueue).never @@ -66,7 +66,7 @@ describe UploadsController do expect(response.status).to eq 200 expect(message.channel).to eq("/uploads/composer") - expect(message.data).to be + expect(message.data["id"]).to be end it 'is successful with synchronous api' do @@ -110,7 +110,7 @@ describe UploadsController do end it 'properly returns errors' do - SiteSetting.stubs(:max_attachment_size_kb).returns(1) + SiteSetting.max_attachment_size_kb = 1 Jobs.expects(:enqueue).never @@ -123,17 +123,30 @@ describe UploadsController do end it 'ensures allow_uploaded_avatars is enabled when uploading an avatar' do - SiteSetting.stubs(:allow_uploaded_avatars).returns(false) + SiteSetting.allow_uploaded_avatars = false xhr :post, :create, file: logo, type: "avatar" expect(response).to_not be_success end it 'ensures sso_overrides_avatar is not enabled when uploading an avatar' do - SiteSetting.stubs(:sso_overrides_avatar).returns(true) + SiteSetting.sso_overrides_avatar = true xhr :post, :create, file: logo, type: "avatar" expect(response).to_not be_success end + it 'allows staff to upload any file in PM' do + SiteSetting.authorized_extensions = "jpg" + SiteSetting.allow_staff_to_upload_any_file_in_pm = true + @user.update_columns(moderator: true) + + message = MessageBus.track_publish do + xhr :post, :create, file: text_file, type: "composer", for_private_message: true + end.first + + expect(response).to be_success + expect(message.data["id"]).to be + end + it 'returns an error when it could not determine the dimensions of an image' do Jobs.expects(:enqueue).with(:create_avatar_thumbnails, anything).never @@ -194,7 +207,7 @@ describe UploadsController do context "prevent anons from downloading files" do - before { SiteSetting.stubs(:prevent_anons_from_downloading_files).returns(true) } + before { SiteSetting.prevent_anons_from_downloading_files = true } it "returns 404 when an anonymous user tries to download a file" do Upload.expects(:find_by).never diff --git a/test/javascripts/lib/utilities-test.js.es6 b/test/javascripts/lib/utilities-test.js.es6 index 403cb2c9719..796c1afb396 100644 --- a/test/javascripts/lib/utilities-test.js.es6 +++ b/test/javascripts/lib/utilities-test.js.es6 @@ -65,11 +65,22 @@ test("new user cannot upload attachments", function() { }); test("ensures an authorized upload", function() { - const html = { name: "unauthorized.html" }; + sandbox.stub(bootbox, "alert"); + not(validUpload([{ name: "unauthorized.html" }])); + ok(bootbox.alert.calledWith(I18n.t('post.errors.upload_not_authorized', { authorized_extensions: authorizedExtensions() }))); +}); + +test("staff can upload anything in PM", function() { + const files = [{ name: "some.docx" }]; + Discourse.SiteSettings.authorized_extensions = "jpeg"; + Discourse.SiteSettings.allow_staff_to_upload_any_file_in_pm = true; + + Discourse.User.resetCurrent(Discourse.User.create({ moderator: true })); + sandbox.stub(bootbox, "alert"); - not(validUpload([html])); - ok(bootbox.alert.calledWith(I18n.t('post.errors.upload_not_authorized', { authorized_extensions: authorizedExtensions() }))); + not(validUpload(files)); + ok(validUpload(files, { isPrivateMessage: true })); }); var imageSize = 10 * 1024;