fix: Secure S3 attachments, bump SDK, refactoring

Closes #673

docker-compose-dev.yml

@@ -25,9 +25,15 @@ services:
       # - KNEX_REJECT_UNAUTHORIZED_SSL_CERTIFICATE=false
 
       # - SHOW_DETAILED_AUTH_ERRORS=false # Set to true to show more detailed authentication error messages. It should not be enabled without a rate limiter for security reasons.
-
       # - ALLOW_ALL_TO_CREATE_PROJECTS=true
 
+      # - S3_ENDPOINT=
+      # - S3_REGION=
+      # - S3_ACCESS_KEY_ID=
+      # - S3_SECRET_ACCESS_KEY=
+      # - S3_BUCKET=
+      # - S3_FORCE_PATH_STYLE=true
+
       # - OIDC_ISSUER=
       # - OIDC_CLIENT_ID=
       # - OIDC_CLIENT_SECRET=

docker-compose.yml

@@ -32,9 +32,15 @@ services:
       # - DEFAULT_ADMIN_USERNAME=demo
 
       # - SHOW_DETAILED_AUTH_ERRORS=false # Set to true to show more detailed authentication error messages. It should not be enabled without a rate limiter for security reasons.
-
       # - ALLOW_ALL_TO_CREATE_PROJECTS=true
 
+      # - S3_ENDPOINT=
+      # - S3_REGION=
+      # - S3_ACCESS_KEY_ID=
+      # - S3_SECRET_ACCESS_KEY=
+      # - S3_BUCKET=
+      # - S3_FORCE_PATH_STYLE=true
+
       # - OIDC_ISSUER=
       # - OIDC_CLIENT_ID=
       # - OIDC_CLIENT_SECRET=
@@ -80,14 +86,6 @@ services:
       # - TELEGRAM_BOT_TOKEN=
       # - TELEGRAM_CHAT_ID=
       # - TELEGRAM_THREAD_ID=
-
-      # Attachments S3
-      # - S3_ENABLE=true
-      # - S3_REGION=
-      # - S3_ENDPOINT=
-      # - S3_BUCKET=
-      # - S3_ACCESS_KEY=
-      # - S3_SECRET_KEY=
     depends_on:
       postgres:
         condition: service_healthy
@@ -101,7 +99,7 @@ services:
       - POSTGRES_DB=planka
       - POSTGRES_HOST_AUTH_METHOD=trust
     healthcheck:
-      test: ['CMD-SHELL', 'pg_isready -U postgres -d planka']
+      test: ["CMD-SHELL", "pg_isready -U postgres -d planka"]
       interval: 10s
       timeout: 5s
       retries: 5

server/.env.sample

@@ -25,9 +25,15 @@ SECRET_KEY=notsecretkey
 # DEFAULT_ADMIN_USERNAME=demo
 
 # SHOW_DETAILED_AUTH_ERRORS=false # Set to true to show more detailed authentication error messages. It should not be enabled without a rate limiter for security reasons.
-
 # ALLOW_ALL_TO_CREATE_PROJECTS=true
 
+# S3_ENDPOINT=
+# S3_REGION=
+# S3_ACCESS_KEY_ID=
+# S3_SECRET_ACCESS_KEY=
+# S3_BUCKET=
+# S3_FORCE_PATH_STYLE=true
+
 # OIDC_ISSUER=
 # OIDC_CLIENT_ID=
 # OIDC_CLIENT_SECRET=
@@ -75,10 +81,3 @@ SECRET_KEY=notsecretkey
 ## Do not edit this
 
 TZ=UTC
-
-# S3_ENABLE=true
-# S3_REGION=
-# S3_ENDPOINT=
-# S3_BUCKET=
-# S3_ACCESS_KEY=
-# S3_SECRET_KEY=

server/api/controllers/attachments/download-thumbnail.js

@@ -1,6 +1,3 @@
-const fs = require('fs');
-const path = require('path');
-
 const Errors = {
   ATTACHMENT_NOT_FOUND: {
     attachmentNotFound: 'Attachment not found',
@@ -46,20 +43,20 @@ module.exports = {
       throw Errors.ATTACHMENT_NOT_FOUND;
     }
 
-    const filePath = path.join(
-      sails.config.custom.attachmentsPath,
-      attachment.dirname,
-      'thumbnails',
-      `cover-256.${attachment.image.thumbnailsExtension}`,
-    );
+    const fileManager = sails.hooks['file-manager'].getInstance();
 
-    if (!fs.existsSync(filePath)) {
+    let readStream;
+    try {
+      readStream = await fileManager.read(
+        `${sails.config.custom.attachmentsPathSegment}/${attachment.dirname}/thumbnails/cover-256.${attachment.image.thumbnailsExtension}`,
+      );
+    } catch (error) {
       throw Errors.ATTACHMENT_NOT_FOUND;
     }
 
     this.res.type('image/jpeg');
     this.res.set('Cache-Control', 'private, max-age=900'); // TODO: move to config
 
-    return exits.success(fs.createReadStream(filePath));
+    return exits.success(readStream);
   },
 };

server/api/controllers/attachments/download.js

@@ -1,4 +1,3 @@
-const fs = require('fs');
 const path = require('path');
 
 const Errors = {
@@ -42,13 +41,14 @@ module.exports = {
       }
     }
 
-    const filePath = path.join(
-      sails.config.custom.attachmentsPath,
-      attachment.dirname,
-      attachment.filename,
-    );
+    const fileManager = sails.hooks['file-manager'].getInstance();
 
-    if (!fs.existsSync(filePath)) {
+    let readStream;
+    try {
+      readStream = await fileManager.read(
+        `${sails.config.custom.attachmentsPathSegment}/${attachment.dirname}/${attachment.filename}`,
+      );
+    } catch (error) {
       throw Errors.ATTACHMENT_NOT_FOUND;
     }
 
@@ -58,6 +58,6 @@ module.exports = {
     }
     this.res.set('Cache-Control', 'private, max-age=900'); // TODO: move to config
 
-    return exits.success(fs.createReadStream(filePath));
+    return exits.success(readStream);
   },
 };

server/api/helpers/attachments/delete-one.js

@@ -1,6 +1,3 @@
-const path = require('path');
-const rimraf = require('rimraf');
-
 module.exports = {
   inputs: {
     record: {
@@ -50,26 +47,12 @@ module.exports = {
     const attachment = await Attachment.archiveOne(inputs.record.id);
 
     if (attachment) {
+      const fileManager = sails.hooks['file-manager'].getInstance();
+
       try {
-        const type = attachment.type || 'local';
-        if (type === 's3') {
-          const client = await sails.helpers.utils.getSimpleStorageServiceClient();
-          if (client) {
-            if (attachment.url) {
-              const parsedUrl = new URL(attachment.url);
-              await client.delete({ Key: parsedUrl.pathname.replace(/^\/+/, '') });
-            }
-            if (attachment.thumb) {
-              const parsedUrl = new URL(attachment.thumb);
-              await client.delete({ Key: parsedUrl.pathname.replace(/^\/+/, '') });
-            }
-          }
-        }
-      } catch (error) {
-        console.warn(error.stack); // eslint-disable-line no-console
-      }
-      try {
-        rimraf.sync(path.join(sails.config.custom.attachmentsPath, attachment.dirname));
+        await fileManager.deleteFolder(
+          `${sails.config.custom.attachmentsPathSegment}/${attachment.dirname}`,
+        );
       } catch (error) {
         console.warn(error.stack); // eslint-disable-line no-console
       }

server/api/helpers/attachments/process-uploaded-file.js

@@ -1,7 +1,4 @@
-const fs = require('fs');
-const path = require('path');
-const rimraf = require('rimraf');
-const moveFile = require('move-file');
+const { rimraf } = require('rimraf');
 const { v4: uuid } = require('uuid');
 const sharp = require('sharp');
 
@@ -16,86 +13,19 @@ module.exports = {
   },
 
   async fn(inputs) {
-    const dirname = uuid();
+    const fileManager = sails.hooks['file-manager'].getInstance();
 
+    const dirname = uuid();
+    const folderPathSegment = `${sails.config.custom.attachmentsPathSegment}/${dirname}`;
     const filename = filenamify(inputs.file.filename);
 
-    const rootPath = path.join(sails.config.custom.attachmentsPath, dirname);
-    const filePath = path.join(rootPath, filename);
-
-    if (sails.config.custom.s3Config) {
-      const client = await sails.helpers.utils.getSimpleStorageServiceClient();
-      const s3Image = await client.upload({
-        Body: fs.createReadStream(inputs.file.fd),
-        Key: `attachments/${dirname}/${filename}`,
-        ContentType: inputs.file.type,
-      });
-
-      let image = sharp(inputs.file.fd, {
-        animated: true,
-      });
-
-      let metadata;
-      try {
-        metadata = await image.metadata();
-      } catch (error) {} // eslint-disable-line no-empty
-
-      const fileData = {
-        type: 's3',
-        dirname,
-        filename,
-        thumb: null,
-        image: null,
-        url: s3Image.Location,
-        name: inputs.file.filename,
-      };
-
-      if (metadata && !['svg', 'pdf'].includes(metadata.format)) {
-        let { width, pageHeight: height = metadata.height } = metadata;
-        if (metadata.orientation && metadata.orientation > 4) {
-          [image, width, height] = [image.rotate(), height, width];
-        }
+    const filePath = await fileManager.move(
+      inputs.file.fd,
+      `${folderPathSegment}/${filename}`,
+      inputs.file.type,
+    );
 
-        const isPortrait = height > width;
-        const thumbnailsExtension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
-
-        try {
-          const resizeBuffer = await image
-            .resize(
-              256,
-              isPortrait ? 320 : undefined,
-              width < 256 || (isPortrait && height < 320)
-                ? {
-                    kernel: sharp.kernel.nearest,
-                  }
-                : undefined,
-            )
-            .toBuffer();
-          const s3Thumb = await client.upload({
-            Key: `attachments/${dirname}/thumbnails/cover-256.${thumbnailsExtension}`,
-            Body: resizeBuffer,
-            ContentType: inputs.file.type,
-          });
-          fileData.thumb = s3Thumb.Location;
-          fileData.image = { width, height };
-        } catch (error1) {
-          console.warn(error2.stack); // eslint-disable-line no-console
-        }
-      }
-
-      try {
-        rimraf.sync(inputs.file.fd);
-      } catch (error) {
-        console.warn(error.stack); // eslint-disable-line no-console
-      }
-
-      return fileData;
-    }
-
-    fs.mkdirSync(rootPath);
-    await moveFile(inputs.file.fd, filePath);
-
-    let image = sharp(filePath, {
+    let image = sharp(filePath || inputs.file.fd, {
       animated: true,
     });
 
@@ -105,17 +35,13 @@ module.exports = {
     } catch (error) {} // eslint-disable-line no-empty
 
     const fileData = {
-      type: 'local',
       dirname,
       filename,
       image: null,
       name: inputs.file.filename,
     };
 
     if (metadata && !['svg', 'pdf'].includes(metadata.format)) {
-      const thumbnailsPath = path.join(rootPath, 'thumbnails');
-      fs.mkdirSync(thumbnailsPath);
-
       let { width, pageHeight: height = metadata.height } = metadata;
       if (metadata.orientation && metadata.orientation > 4) {
         [image, width, height] = [image.rotate(), height, width];
@@ -125,7 +51,7 @@ module.exports = {
       const thumbnailsExtension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
 
       try {
-        await image
+        const resizeBuffer = await image
           .resize(
             256,
             isPortrait ? 320 : undefined,
@@ -135,19 +61,29 @@ module.exports = {
                 }
               : undefined,
           )
-          .toFile(path.join(thumbnailsPath, `cover-256.${thumbnailsExtension}`));
+          .toBuffer();
+
+        await fileManager.save(
+          `${folderPathSegment}/thumbnails/cover-256.${thumbnailsExtension}`,
+          resizeBuffer,
+          inputs.file.type,
+        );
 
         fileData.image = {
           width,
           height,
           thumbnailsExtension,
         };
-      } catch (error1) {
-        try {
-          rimraf.sync(thumbnailsPath);
-        } catch (error2) {
-          console.warn(error2.stack); // eslint-disable-line no-console
-        }
+      } catch (error) {
+        console.warn(error.stack); // eslint-disable-line no-console
+      }
+    }
+
+    if (!filePath) {
+      try {
+        await rimraf(inputs.file.fd);
+      } catch (error) {
+        console.warn(error.stack); // eslint-disable-line no-console
       }
     }
 

server/api/helpers/boards/process-uploaded-trello-import-file.js

@@ -1,5 +1,5 @@
-const fs = require('fs').promises;
-const rimraf = require('rimraf');
+const fs = require('fs');
+const { rimraf } = require('rimraf');
 
 module.exports = {
   inputs: {
@@ -14,7 +14,7 @@ module.exports = {
   },
 
   async fn(inputs) {
-    const content = await fs.readFile(inputs.file.fd);
+    const content = await fs.promises.readFile(inputs.file.fd);
     const trelloBoard = JSON.parse(content);
 
     if (
@@ -28,7 +28,7 @@ module.exports = {
     }
 
     try {
-      rimraf.sync(inputs.file.fd);
+      await rimraf(inputs.file.fd);
     } catch (error) {
       console.warn(error.stack); // eslint-disable-line no-console
     }