Move directory key validation into FileHandler

services/filestore/app/js/FileHandler.js

@@ -5,7 +5,7 @@ const LocalFileWriter = require('./LocalFileWriter')
 const FileConverter = require('./FileConverter')
 const KeyBuilder = require('./KeyBuilder')
 const ImageOptimiser = require('./ImageOptimiser')
-const { ConversionError } = require('./Errors')
+const { ConversionError, WriteError } = require('./Errors')
 
 module.exports = {
   insertFile: callbackify(insertFile),
@@ -24,12 +24,24 @@ module.exports = {
 
 async function insertFile(bucket, key, stream) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
+  if (!convertedKey.match(/^[0-9a-f]{24}\/[0-9a-f]{24}/i)) {
+    throw new WriteError({
+      message: 'key does not match validation regex',
+      info: { bucket, key, convertedKey }
+    })
+  }
   await PersistorManager.promises.deleteDirectory(bucket, convertedKey)
   await PersistorManager.promises.sendStream(bucket, key, stream)
 }
 
 async function deleteFile(bucket, key) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
+  if (!convertedKey.match(/^[0-9a-f]{24}\/[0-9a-f]{24}/i)) {
+    throw new WriteError({
+      message: 'key does not match validation regex',
+      info: { bucket, key, convertedKey }
+    })
+  }
   await Promise.all([
     PersistorManager.promises.deleteFile(bucket, key),
     PersistorManager.promises.deleteDirectory(bucket, convertedKey)

services/filestore/app/js/GcsPersistor.js

@@ -204,13 +204,6 @@ async function deleteFile(bucketName, key) {
 }
 
 async function deleteDirectory(bucketName, key) {
-  if (!key.match(settings.filestore.gcs.directoryKeyRegex)) {
-    throw new NotFoundError({
-      message: 'deleteDirectoryKey is invalid or missing',
-      info: { bucketName, key }
-    })
-  }
-
   try {
     const [files] = await storage
       .bucket(bucketName)

services/filestore/config/settings.defaults.coffee

@@ -40,8 +40,6 @@ settings =
 					apiEndpoint: process.env['GCS_API_ENDPOINT']
 					apiScheme: process.env['GCS_API_SCHEME']
 					projectId: process.env['GCS_PROJECT_ID']
-			# only keys that match this regex can be deleted
-			directoryKeyRegex: new RegExp(process.env['GCS_DIRECTORY_KEY_REGEX'] || "^[0-9a-fA-F]{24}/[0-9a-fA-F]{24}")
 			unlockBeforeDelete: process.env['GCS_UNLOCK_BEFORE_DELETE'] == "true"  # unlock an event-based hold before deleting. default false
 			deletedBucketSuffix: process.env['GCS_DELETED_BUCKET_SUFFIX']          # if present, copy file to another bucket on delete. default null
 

services/filestore/test/unit/js/FileHandlerTests.js

@@ -3,6 +3,7 @@ const chai = require('chai')
 const { expect } = chai
 const modulePath = '../../../app/js/FileHandler.js'
 const SandboxedModule = require('sandboxed-module')
+const { ObjectId } = require('mongodb')
 
 chai.use(require('sinon-chai'))
 chai.use(require('chai-as-promised'))
@@ -24,8 +25,8 @@ describe('FileHandler', function() {
   }
 
   const bucket = 'my_bucket'
-  const key = 'key/here'
-  const convertedFolderKey = 'convertedFolder'
+  const key = `${ObjectId()}/${ObjectId()}`
+  const convertedFolderKey = `${ObjectId()}/${ObjectId()}`
   const sourceStream = 'sourceStream'
   const convertedKey = 'convertedKey'
   const readStream = {
@@ -112,6 +113,14 @@ describe('FileHandler', function() {
         done()
       })
     })
+
+    it('should throw an error when the key is in the wrong format', function(done) {
+      KeyBuilder.getConvertedFolderKey.returns('wombat')
+      FileHandler.insertFile(bucket, key, stream, err => {
+        expect(err).to.exist
+        done()
+      })
+    })
   })
 
   describe('deleteFile', function() {
@@ -135,6 +144,14 @@ describe('FileHandler', function() {
         done()
       })
     })
+
+    it('should throw an error when the key is in the wrong format', function(done) {
+      KeyBuilder.getConvertedFolderKey.returns('wombat')
+      FileHandler.deleteFile(bucket, key, err => {
+        expect(err).to.exist
+        done()
+      })
+    })
   })
 
   describe('getFile', function() {

services/filestore/test/unit/js/GcsPersistorTests.js

@@ -556,22 +556,6 @@ describe('GcsPersistorTests', function() {
         expect(error.cause).to.equal(genericError)
       })
     })
-
-    describe('when the directory name is in the wrong format', function() {
-      let error
-
-      beforeEach(async function() {
-        try {
-          await GcsPersistor.promises.deleteDirectory(bucket, 'carbonara')
-        } catch (err) {
-          error = err
-        }
-      })
-
-      it('should throw a NotFoundError', function() {
-        expect(error).to.be.an.instanceOf(Errors.NotFoundError)
-      })
-    })
   })
 
   describe('directorySize', function() {