Merge pull request #4648 from NotTheEvilOne/pr/support-upload-avatar-validation

Add support to validate avatar uploads by type, size and external program

.devcontainer/Dockerfile

@@ -33,6 +33,9 @@ ENV \
     ATTACHMENTS_UPLOAD_EXTERNAL_PROGRAM="" \
     ATTACHMENTS_UPLOAD_MIME_TYPES="" \
     ATTACHMENTS_UPLOAD_MAX_SIZE=0 \
+    AVATARS_UPLOAD_EXTERNAL_PROGRAM="" \
+    AVATARS_UPLOAD_MIME_TYPES="" \
+    AVATARS_UPLOAD_MAX_SIZE=0 \
     MAX_IMAGE_PIXEL="" \
     IMAGE_COMPRESS_RATIO="" \
     NOTIFICATION_TRAY_AFTER_READ_DAYS_BEFORE_REMOVE="" \

Dockerfile

@@ -36,6 +36,9 @@ ENV BUILD_DEPS="apt-utils libarchive-tools gnupg gosu wget curl bzip2 g++ build-
     ATTACHMENTS_UPLOAD_EXTERNAL_PROGRAM="" \
     ATTACHMENTS_UPLOAD_MIME_TYPES="" \
     ATTACHMENTS_UPLOAD_MAX_SIZE=0 \
+    AVATARS_UPLOAD_EXTERNAL_PROGRAM="" \
+    AVATARS_UPLOAD_MIME_TYPES="" \
+    AVATARS_UPLOAD_MAX_SIZE=0 \
     RICHER_CARD_COMMENT_EDITOR=false \
     CARD_OPENED_WEBHOOK_ENABLED=false \
     MAX_IMAGE_PIXEL="" \

client/components/users/userAvatar.js

@@ -3,7 +3,6 @@ import Avatars from '/models/avatars';
 import Users from '/models/users';
 import Org from '/models/org';
 import Team from '/models/team';
-import { formatFleURL } from 'meteor/ostrio:files/lib';
 
 Template.userAvatar.helpers({
   userData() {
@@ -218,13 +217,6 @@ BlazeComponent.extendComponent({
               },
               false,
             );
-            uploader.on('uploaded', (error, fileRef) => {
-              if (!error) {
-                self.setAvatar(
-                  `${formatFleURL(fileRef)}?auth=false&brokenIsFine=true`,
-                );
-              }
-            });
             uploader.on('error', (error, fileData) => {
               self.setError(error.reason);
             });
@@ -238,8 +230,9 @@ BlazeComponent.extendComponent({
         'click .js-select-initials'() {
           this.setAvatar('');
         },
-        'click .js-delete-avatar'() {
+        'click .js-delete-avatar'(event) {
           Avatars.remove(this.currentData()._id);
+          event.stopPropagation();
         },
       },
     ];

docker-compose.yml

@@ -271,6 +271,11 @@ services:
       #-ATTACHMENTS_UPLOAD_MIME_TYPES=image/*,text/*
       #-ATTACHMENTS_UPLOAD_MAX_SIZE=5000000
       #---------------------------------------------------------------
+      # ==== Allow configuration to validate uploaded avatars ====
+      #-AVATARS_UPLOAD_EXTERNAL_PROGRAM=/usr/local/bin/avscan {file}
+      #-AVATARS_UPLOAD_MIME_TYPES=image/*
+      #-AVATARS_UPLOAD_MAX_SIZE=500000
+      #---------------------------------------------------------------
       # ==== Allow to shrink attached/pasted image ====
       # https://github.com/wekan/wekan/pull/2544
       #-MAX_IMAGE_PIXEL=1024

models/attachments.js

@@ -1,15 +1,12 @@
 import { Meteor } from 'meteor/meteor';
 import { FilesCollection } from 'meteor/ostrio:files';
-import { exec } from 'node:child_process';
-import { promisify } from 'node:util';
+import { isFileValid } from './fileValidation';
 import { createBucket } from './lib/grid/createBucket';
 import fs from 'fs';
-import FileType from 'file-type';
 import path from 'path';
 import { AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs} from '/models/lib/attachmentStoreStrategy';
 import FileStoreStrategyFactory, {moveToStorage, rename, STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS} from '/models/lib/fileStoreStrategy';
 
-let asyncExec;
 let attachmentUploadExternalProgram;
 let attachmentUploadMimeTypes = [];
 let attachmentUploadSize = 0;
@@ -17,7 +14,6 @@ let attachmentBucket;
 let storagePath;
 
 if (Meteor.isServer) {
-  asyncExec = promisify(exec);
   attachmentBucket = createBucket('attachments');
 
   if (process.env.ATTACHMENTS_UPLOAD_MIME_TYPES) {
@@ -155,34 +151,7 @@ if (Meteor.isServer) {
       check(fileObjId, String);
 
       const fileObj = Attachments.findOne({_id: fileObjId});
-      let isValid = true;
-
-      if (attachmentUploadMimeTypes.length) {
-        const mimeTypeResult = Promise.await(FileType.fromFile(fileObj.path));
-
-        const mimeType = (mimeTypeResult ? mimeTypeResult.mime : fileObj.type);
-        const baseMimeType = mimeType.split('/', 1)[0];
-
-        isValid = attachmentUploadMimeTypes.includes(mimeType) || attachmentUploadMimeTypes.includes(baseMimeType + '/*') || attachmentUploadMimeTypes.includes('*');
-
-        if (!isValid) {
-          console.log("Validation of uploaded file failed: file " + fileObj.path + " - mimetype " + mimeType);
-        }
-      }
-
-      if (attachmentUploadSize && fileObj.size > attachmentUploadSize) {
-        console.log("Validation of uploaded file failed: file " + fileObj.path + " - size " + fileObj.size);
-        isValid = false;
-      }
-
-      if (isValid && attachmentUploadExternalProgram) {
-        Promise.await(asyncExec(attachmentUploadExternalProgram.replace("{file}", '"' + fileObj.path + '"')));
-        isValid = fs.existsSync(fileObj.path);
-
-        if (!isValid) {
-          console.log("Validation of uploaded file failed: file " + fileObj.path + " has been deleted externally");
-        }
-      }
+      const isValid = Promise.await(isFileValid(fileObj, attachmentUploadMimeTypes, attachmentUploadSize, attachmentUploadExternalProgram));
 
       if (!isValid) {
         Attachments.remove(fileObjId);
@@ -192,12 +161,11 @@ if (Meteor.isServer) {
       check(fileObjId, String);
       check(storageDestination, String);
 
-      Meteor.defer(() => Meteor.call('validateAttachment', fileObjId));
+      Meteor.call('validateAttachment', fileObjId);
 
       const fileObj = Attachments.findOne({_id: fileObjId});
 
       if (fileObj) {
-        console.debug("Validation of uploaded file completed: file " + fileObj.path + " - storage destination " + storageDestination);
         Meteor.defer(() => Meteor.call('moveAttachmentToStorage', fileObjId, storageDestination));
       }
     },

models/avatars.js

@@ -1,13 +1,40 @@
 import { Meteor } from 'meteor/meteor';
 import { FilesCollection } from 'meteor/ostrio:files';
+import { formatFleURL } from 'meteor/ostrio:files/lib';
+import { isFileValid } from './fileValidation';
 import { createBucket } from './lib/grid/createBucket';
 import fs from 'fs';
 import path from 'path';
-import FileStoreStrategyFactory, { FileStoreStrategyFilesystem, FileStoreStrategyGridFs} from '/models/lib/fileStoreStrategy';
+import FileStoreStrategyFactory, { FileStoreStrategyFilesystem, FileStoreStrategyGridFs, STORAGE_NAME_FILESYSTEM } from '/models/lib/fileStoreStrategy';
 
+let avatarsUploadExternalProgram;
+let avatarsUploadMimeTypes = [];
+let avatarsUploadSize = 72000;
 let avatarsBucket;
 let storagePath;
+
 if (Meteor.isServer) {
+  if (process.env.AVATARS_UPLOAD_MIME_TYPES) {
+    avatarsUploadMimeTypes = process.env.AVATARS_UPLOAD_MIME_TYPES.split(',');
+    avatarsUploadMimeTypes = avatarsUploadMimeTypes.map(value => value.trim());
+  }
+
+  if (process.env.AVATARS_UPLOAD_MAX_SIZE) {
+    avatarsUploadSize = parseInt(process.env.AVATARS_UPLOAD_MAX_SIZE);
+
+    if (isNaN(avatarsUploadSize)) {
+      avatarsUploadSize = 0
+    }
+  }
+
+  if (process.env.AVATARS_UPLOAD_EXTERNAL_PROGRAM) {
+    avatarsUploadExternalProgram = process.env.AVATARS_UPLOAD_EXTERNAL_PROGRAM;
+
+    if (!avatarsUploadExternalProgram.includes("{file}")) {
+      avatarsUploadExternalProgram = undefined;
+    }
+  }
+
   avatarsBucket = createBucket('avatars');
   storagePath = path.join(process.env.WRITABLE_PATH, 'avatars');
 }
@@ -23,7 +50,7 @@ Avatars = new FilesCollection({
     return ret;
   },
   onBeforeUpload(file) {
-    if (file.size <= 72000 && file.type.startsWith('image/')) {
+    if (file.size <= avatarsUploadSize && file.type.startsWith('image/')) {
       return true;
     }
     return 'avatar-too-big';
@@ -31,14 +58,32 @@ Avatars = new FilesCollection({
   onAfterUpload(fileObj) {
     // current storage is the filesystem, update object and database
     Object.keys(fileObj.versions).forEach(versionName => {
-      fileObj.versions[versionName].storage = "fs";
+      fileObj.versions[versionName].storage = STORAGE_NAME_FILESYSTEM;
     });
-    Avatars.update({ _id: fileObj._id }, { $set: { "versions" : fileObj.versions } });
+
+    Avatars.update({ _id: fileObj._id }, { $set: { "versions": fileObj.versions } });
+
+    const isValid = Promise.await(isFileValid(fileObj, avatarsUploadMimeTypes, avatarsUploadSize, avatarsUploadExternalProgram));
+
+    if (isValid) {
+      Users.findOne(fileObj.userId).setAvatarUrl(`${formatFleURL(fileObj)}?auth=false&brokenIsFine=true`);
+    } else {
+      Avatars.remove(fileObj._id);
+    }
   },
   interceptDownload(http, fileObj, versionName) {
     const ret = fileStoreStrategyFactory.getFileStrategy(fileObj, versionName).interceptDownload(http, this.cacheControl);
     return ret;
   },
+  onBeforeRemove(files) {
+    files.forEach(fileObj => {
+      if (fileObj.userId) {
+        Users.findOne(fileObj.userId).setAvatarUrl('');
+      }
+    });
+
+    return true;
+  },
   onAfterRemove(files) {
     files.forEach(fileObj => {
       Object.keys(fileObj.versions).forEach(versionName => {

models/fileValidation.js

@@ -0,0 +1,48 @@
+import { Meteor } from 'meteor/meteor';
+import { exec } from 'node:child_process';
+import { promisify } from 'node:util';
+import fs from 'fs';
+import FileType from 'file-type';
+
+let asyncExec;
+
+if (Meteor.isServer) {
+  asyncExec = promisify(exec);
+}
+
+export async function isFileValid(fileObj, mimeTypesAllowed, sizeAllowed, externalCommandLine) {
+  let isValid = true;
+
+  if (mimeTypesAllowed.length) {
+    const mimeTypeResult = await FileType.fromFile(fileObj.path);
+
+    const mimeType = (mimeTypeResult ? mimeTypeResult.mime : fileObj.type);
+    const baseMimeType = mimeType.split('/', 1)[0];
+
+    isValid = mimeTypesAllowed.includes(mimeType) || mimeTypesAllowed.includes(baseMimeType + '/*') || mimeTypesAllowed.includes('*');
+
+    if (!isValid) {
+      console.log("Validation of uploaded file failed: file " + fileObj.path + " - mimetype " + mimeType);
+    }
+  }
+
+  if (isValid && sizeAllowed && fileObj.size > sizeAllowed) {
+    console.log("Validation of uploaded file failed: file " + fileObj.path + " - size " + fileObj.size);
+    isValid = false;
+  }
+
+  if (isValid && externalCommandLine) {
+    await asyncExec(externalCommandLine.replace("{file}", '"' + fileObj.path + '"'));
+    isValid = fs.existsSync(fileObj.path);
+
+    if (!isValid) {
+      console.log("Validation of uploaded file failed: file " + fileObj.path + " has been deleted externally");
+    }
+  }
+
+  if (isValid) {
+    console.debug("Validation of uploaded file successful: file " + fileObj.path);
+  }
+
+  return isValid;
+}

releases/virtualbox/start-wekan.sh

@@ -53,6 +53,11 @@
       #export ATTACHMENTS_UPLOAD_MIME_TYPES="image/*,text/*"
       #export ATTACHMENTS_UPLOAD_MAX_SIZE=5000000
       #---------------------------------------------------------------
+      # ==== Allow configuration to validate uploaded avatars ====
+      #export AVATARS_UPLOAD_EXTERNAL_PROGRAM="/usr/local/bin/avscan {file}"
+      #export AVATARS_UPLOAD_MIME_TYPES="image/*"
+      #export AVATARS_UPLOAD_MAX_SIZE=500000
+      #---------------------------------------------------------------
       # ==== RICH TEXT EDITOR IN CARD COMMENTS ====
       # https://github.com/wekan/wekan/pull/2560
       export RICHER_CARD_COMMENT_EDITOR=false

start-wekan.bat

@@ -55,9 +55,15 @@ REM # ==== ACCOUNT OPTIONS ====
 REM SET ACCOUNTS_COMMON_LOGIN_EXPIRATION_IN_DAYS=90
 
 REM # ==== Allow configuration to validate uploaded attachments ====
+REM SET ATTACHMENTS_UPLOAD_EXTERNAL_PROGRAM="avscan {file}"
 REM SET ATTACHMENTS_UPLOAD_MIME_TYPES="image/*,text/*"
 REM SET ATTACHMENTS_UPLOAD_MAX_SIZE=5000000
 
+REM # ==== Allow configuration to validate uploaded avatars ====
+REM SET AVATARS_UPLOAD_EXTERNAL_PROGRAM="avscan {file}"
+REM SET AVATARS_UPLOAD_MIME_TYPES="image/*"
+REM SET AVATARS_UPLOAD_MAX_SIZE=500000
+
 REM # ==== NOTIFICATION TRAY AFTER READ DAYS BEFORE REMOVE =====
 REM # Number of days after a notification is read before we remove it.
 REM # Default: 2

start-wekan.sh

@@ -58,6 +58,11 @@
       #export ATTACHMENTS_UPLOAD_MIME_TYPES="image/*,text/*"
       #export ATTACHMENTS_UPLOAD_MAX_SIZE=5000000
       #---------------------------------------------------------------
+      # ==== Allow configuration to validate uploaded avatars ====
+      #export AVATARS_UPLOAD_EXTERNAL_PROGRAM="/usr/local/bin/avscan {file}"
+      #export AVATARS_UPLOAD_MIME_TYPES="image/*"
+      #export AVATARS_UPLOAD_MAX_SIZE=500000
+      #---------------------------------------------------------------
       # ==== RICH TEXT EDITOR IN CARD COMMENTS ====
       # https://github.com/wekan/wekan/pull/2560
       export RICHER_CARD_COMMENT_EDITOR=false

torodb-postgresql/docker-compose.yml

@@ -281,6 +281,11 @@ services:
       #-ATTACHMENTS_UPLOAD_MIME_TYPES=image/*,text/*
       #-ATTACHMENTS_UPLOAD_MAX_SIZE=5000000
       #---------------------------------------------------------------
+      # ==== Allow configuration to validate uploaded avatars ====
+      #-AVATARS_UPLOAD_EXTERNAL_PROGRAM=/usr/local/bin/avscan {file}
+      #-AVATARS_UPLOAD_MIME_TYPES=image/*
+      #-AVATARS_UPLOAD_MAX_SIZE=500000
+      #---------------------------------------------------------------
       # ==== Allow to shrink attached/pasted image ====
       # https://github.com/wekan/wekan/pull/2544
       #-MAX_IMAGE_PIXEL=1024