FEATURE: New site setting for additional allowed filetypes for staff (#5364)

* FEATURE: New site setting for additional allowed filetypes for staff

* Problematic variable name

* feedback

* small issues

* fix indentation

* failing tests

* Remove message bus and fix minor issues

* Missed this message bus

app/assets/javascripts/discourse/lib/utilities.js.es6

@@ -203,7 +203,7 @@ export function validateUploadedFile(file, opts) {
 
   // check that the uploaded file is authorized
   if (opts.allowStaffToUploadAnyFileInPm && opts.isPrivateMessage) {
-    if (Discourse.User.current("staff")) {
+    if (Discourse.User.currentProp('staff')) {
       return true;
     }
   }
@@ -239,16 +239,28 @@ export function validateUploadedFile(file, opts) {
 
 const IMAGES_EXTENSIONS_REGEX = /(png|jpe?g|gif|bmp|tiff?|svg|webp|ico)/i;
 
+function extensionsToArray(exts) {
+  return exts.toLowerCase()
+             .replace(/[\s\.]+/g, "")
+             .split("|")
+             .filter(ext => ext.indexOf("*") === -1);
+}
+
 function extensions() {
-  return Discourse.SiteSettings.authorized_extensions
-                               .toLowerCase()
-                               .replace(/[\s\.]+/g, "")
-                               .split("|")
-                               .filter(ext => ext.indexOf("*") === -1);
+  return extensionsToArray(Discourse.SiteSettings.authorized_extensions);
+}
+
+function staffExtensions() {
+  return extensionsToArray(Discourse.SiteSettings.authorized_extensions_for_staff);
 }
 
 function imagesExtensions() {
-  return extensions().filter(ext => IMAGES_EXTENSIONS_REGEX.test(ext));
+  let exts =  extensions().filter(ext => IMAGES_EXTENSIONS_REGEX.test(ext));
+  if (Discourse.User.currentProp('staff')) {
+    const staffExts = staffExtensions().filter(ext => IMAGES_EXTENSIONS_REGEX.test(ext));
+    exts = _.union(exts, staffExts);
+  }
+  return exts;
 }
 
 function extensionsRegex() {
@@ -259,7 +271,14 @@ function imagesExtensionsRegex() {
   return new RegExp("\\.(" + imagesExtensions().join("|") + ")$", "i");
 }
 
+function staffExtensionsRegex() {
+  return new RegExp("\\.(" + staffExtensions().join("|") + ")$", "i");
+}
+
 function isAuthorizedFile(fileName) {
+  if (Discourse.User.currentProp('staff') && staffExtensionsRegex().test(fileName)) {
+    return true;
+  }
   return extensionsRegex().test(fileName);
 }
 
@@ -268,7 +287,8 @@ function isAuthorizedImage(fileName){
 }
 
 export function authorizedExtensions() {
-  return authorizesAllExtensions() ? "*" : extensions().join(", ");
+  const exts = Discourse.User.currentProp('staff') ? [...extensions(), ...staffExtensions()] : extensions();
+  return exts.filter(ext => ext.length > 0).join(", ");
 }
 
 export function authorizedImagesExtensions() {
@@ -276,7 +296,9 @@ export function authorizedImagesExtensions() {
 }
 
 export function authorizesAllExtensions() {
-  return Discourse.SiteSettings.authorized_extensions.indexOf("*") >= 0;
+  return Discourse.SiteSettings.authorized_extensions.indexOf("*") >= 0 || (
+         Discourse.SiteSettings.authorized_extensions_for_staff.indexOf("*") >= 0 &&
+         Discourse.User.currentProp('staff'));
 }
 
 export function authorizesOneOrMoreExtensions() {
@@ -322,7 +344,7 @@ export function allowsImages() {
 }
 
 export function allowsAttachments() {
-  return authorizesAllExtensions() || extensions().length > imagesExtensions().length;
+  return authorizesAllExtensions() || authorizedExtensions().split(", ").length > imagesExtensions().length;
 }
 
 export function uploadLocation(url) {

config/locales/server.en.yml

@@ -1334,6 +1334,7 @@ en:
     max_image_size_kb: "The maximum image upload size in kB. This must be configured in nginx (client_max_body_size) / apache or proxy as well."
     max_attachment_size_kb: "The maximum attachment files upload size in kB. This must be configured in nginx (client_max_body_size) / apache or proxy as well."
     authorized_extensions: "A list of file extensions allowed for upload (use '*' to enable all file types)"
+    authorized_extensions_for_staff: "A list of file extensions allowed for upload for staff users in addition to the list defined in the `authorized_extensions` site setting. (use '*' to enable all file types)"
     theme_authorized_extensions: "A list of file extensions allowed for theme uploads (use '*' to enable all file types)"
     max_similar_results: "How many similar topics to show above the editor when composing a new topic. Comparison is based on title and body."
 

config/site_settings.yml

@@ -796,6 +796,11 @@ files:
     default: 'jpg|jpeg|png|gif'
     refresh: true
     type: list
+  authorized_extensions_for_staff:
+    client: true
+    default: ''
+    refresh: true
+    type: list
   crawl_images:
     default: true
   max_image_width:

lib/validators/upload_validator.rb

@@ -29,11 +29,11 @@ class Validators::UploadValidator < ActiveModel::Validator
   end
 
   def is_authorized?(upload, extension)
-    authorized_extensions(upload, extension, authorized_uploads(upload))
+    extension_authorized?(upload, extension, authorized_extensions(upload))
   end
 
   def authorized_image_extension(upload, extension)
-    authorized_extensions(upload, extension, authorized_images(upload))
+    extension_authorized?(upload, extension, authorized_images(upload))
   end
 
   def maximum_image_file_size(upload)
@@ -41,7 +41,7 @@ class Validators::UploadValidator < ActiveModel::Validator
   end
 
   def authorized_attachment_extension(upload, extension)
-    authorized_extensions(upload, extension, authorized_attachments(upload))
+    extension_authorized?(upload, extension, authorized_attachments(upload))
   end
 
   def maximum_attachment_file_size(upload)
@@ -50,38 +50,50 @@ class Validators::UploadValidator < ActiveModel::Validator
 
   private
 
-  def authorized_uploads(upload)
-    authorized_uploads = Set.new
+  def extensions_to_set(exts)
+    extensions = Set.new
 
-    extensions = upload.for_theme ? SiteSetting.theme_authorized_extensions : SiteSetting.authorized_extensions
-
-    extensions
+    exts
       .gsub(/[\s\.]+/, "")
       .downcase
       .split("|")
-      .each { |extension| authorized_uploads << extension unless extension.include?("*") }
+      .each { |extension| extensions << extension unless extension.include?("*") }
 
-    authorized_uploads
+    extensions
+  end
+
+  def authorized_extensions(upload)
+    extensions = upload.for_theme ? SiteSetting.theme_authorized_extensions : SiteSetting.authorized_extensions
+    extensions_to_set(extensions)
   end
 
   def authorized_images(upload)
-    authorized_uploads(upload) & FileHelper.images
+    authorized_extensions(upload) & FileHelper.images
   end
 
   def authorized_attachments(upload)
-    authorized_uploads(upload) - FileHelper.images
+    authorized_extensions(upload) - FileHelper.images
   end
 
   def authorizes_all_extensions?(upload)
+    if upload.user&.staff?
+      return true if SiteSetting.authorized_extensions_for_staff.include?("*")
+    end
     extensions = upload.for_theme ? SiteSetting.theme_authorized_extensions : SiteSetting.authorized_extensions
     extensions.include?("*")
   end
 
-  def authorized_extensions(upload, extension, extensions)
+  def extension_authorized?(upload, extension, extensions)
     return true if authorizes_all_extensions?(upload)
 
+    staff_extensions = Set.new
+    if upload.user&.staff?
+      staff_extensions = extensions_to_set(SiteSetting.authorized_extensions_for_staff)
+      return true if staff_extensions.include?(extension.downcase)
+    end
+
     unless authorized = extensions.include?(extension.downcase)
-      message = I18n.t("upload.unauthorized", authorized_extensions: extensions.to_a.join(", "))
+      message = I18n.t("upload.unauthorized", authorized_extensions: (extensions | staff_extensions).to_a.join(", "))
       upload.errors.add(:original_filename, message)
     end
 

spec/controllers/uploads_controller_spec.rb

@@ -148,6 +148,36 @@ describe UploadsController do
         expect(id).to be
       end
 
+      it 'respects `authorized_extensions_for_staff` setting when staff upload file' do
+        SiteSetting.authorized_extensions = ""
+        SiteSetting.authorized_extensions_for_staff = "*"
+        @user.update_columns(moderator: true)
+
+        post :create, params: {
+          file: text_file,
+          type: "composer",
+          format: :json
+        }
+
+        expect(response).to be_success
+        data = JSON.parse(response.body)
+        expect(data["id"]).to be
+      end
+
+      it 'ignores `authorized_extensions_for_staff` setting when non-staff upload file' do
+        SiteSetting.authorized_extensions = ""
+        SiteSetting.authorized_extensions_for_staff = "*"
+
+        post :create, params: {
+          file: text_file,
+          type: "composer",
+          format: :json
+        }
+
+        data = JSON.parse(response.body)
+        expect(data["errors"].first).to eq(I18n.t("upload.unauthorized", authorized_extensions: ''))
+      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
 

test/javascripts/helpers/site-settings.js

@@ -59,6 +59,7 @@ Discourse.SiteSettingsOriginal = {
   "autohighlight_all_code":false,
   "email_in":false,
   "authorized_extensions":".jpg|.jpeg|.png|.gif|.svg|.txt|.ico|.yml",
+  "authorized_extensions_for_staff": "",
   "max_image_width":690,
   "max_image_height":500,
   "allow_profile_backgrounds":true,