build(backend): upgrade outdated S3 client (#8463)

I think we can merge this already. The S3 client dependency is outdated
and since the code is behind a feature flag it doesn't get executed. Why
not merge it then?

backend/package.json

@@ -28,10 +28,12 @@
     "prod:db:data:categories": "node build/src/db/categories.js"
   },
   "dependencies": {
+    "@aws-sdk/client-s3": "^3.796.0",
+    "@aws-sdk/lib-storage": "^3.797.0",
     "@sentry/node": "^5.15.4",
+    "@types/mime-types": "^2.1.4",
     "apollo-server": "~2.14.2",
     "apollo-server-express": "^2.14.2",
-    "aws-sdk": "^2.1692.0",
     "bcryptjs": "~3.0.2",
     "body-parser": "^1.20.3",
     "cheerio": "~1.0.0",

backend/src/graphql/resolvers/images/images.spec.ts

@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/require-await */
-/* eslint-disable @typescript-eslint/await-thenable */
+
 /* eslint-disable @typescript-eslint/no-unsafe-return */
 /* eslint-disable @typescript-eslint/restrict-template-expressions */
 /* eslint-disable @typescript-eslint/no-unsafe-call */
@@ -13,6 +13,9 @@ import { getNeode, getDriver } from '@db/neo4j'
 
 import { deleteImage, mergeImage } from './images'
 
+import type { ImageInput } from './images'
+import type { FileUpload } from 'graphql-upload'
+
 const driver = getDriver()
 const neode = getNeode()
 const uuid = '[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}'
@@ -55,7 +58,7 @@ describe('deleteImage', () => {
       user = await user.toJson()
     })
 
-    it('soft deletes `Image` node', async () => {
+    it('deletes `Image` node', async () => {
       await expect(neode.all('Image')).resolves.toHaveLength(1)
       await deleteImage(user, 'AVATAR_IMAGE', { deleteCallback })
       await expect(neode.all('Image')).resolves.toHaveLength(0)
@@ -71,7 +74,7 @@ describe('deleteImage', () => {
     describe('given a transaction parameter', () => {
       it('executes cypher statements within the transaction', async () => {
         const session = driver.session()
-        let someString
+        let someString: string
         try {
           someString = await session.writeTransaction(async (transaction) => {
             await deleteImage(user, 'AVATAR_IMAGE', {
@@ -86,7 +89,7 @@ describe('deleteImage', () => {
           await session.close()
         }
         await expect(neode.all('Image')).resolves.toHaveLength(0)
-        await expect(someString).toEqual('Hello')
+        expect(someString).toEqual('Hello')
       })
 
       it('rolls back the transaction in case of errors', async () => {
@@ -114,7 +117,7 @@ describe('deleteImage', () => {
 })
 
 describe('mergeImage', () => {
-  let imageInput
+  let imageInput: ImageInput
   let post
   beforeEach(() => {
     imageInput = {
@@ -124,18 +127,19 @@ describe('mergeImage', () => {
 
   describe('given image.upload', () => {
     beforeEach(() => {
+      const createReadStream: FileUpload['createReadStream'] = (() => ({
+        pipe: () => ({
+          on: (_, callback) => callback(),
+        }),
+      })) as unknown as FileUpload['createReadStream']
       imageInput = {
         ...imageInput,
-        upload: {
+        upload: Promise.resolve({
           filename: 'image.jpg',
           mimetype: 'image/jpeg',
           encoding: '7bit',
-          createReadStream: () => ({
+          createReadStream,
-            pipe: () => ({
+        }),
-              on: (_, callback) => callback(),
-            }),
-          }),
-        },
       }
     })
 
@@ -173,7 +177,12 @@ describe('mergeImage', () => {
       })
 
       it('creates a url safe name', async () => {
-        imageInput.upload.filename = '/path/to/awkward?/ file-location/?foo- bar-avatar.jpg'
+        if (!imageInput.upload) {
+          throw new Error('Test imageInput was not setup correctly.')
+        }
+        const upload = await imageInput.upload
+        upload.filename = '/path/to/awkward?/ file-location/?foo- bar-avatar.jpg'
+        imageInput.upload = Promise.resolve(upload)
         await expect(
           mergeImage(post, 'HERO_IMAGE', imageInput, { uploadCallback, deleteCallback }),
         ).resolves.toMatchObject({
@@ -181,21 +190,6 @@ describe('mergeImage', () => {
         })
       })
 
-      // eslint-disable-next-line jest/no-disabled-tests
-      it.skip('automatically creates different image sizes', async () => {
-        await expect(
-          mergeImage(post, 'HERO_IMAGE', imageInput, { uploadCallback, deleteCallback }),
-        ).resolves.toEqual({
-          url: expect.any(String),
-          alt: expect.any(String),
-          urlW34: expect.stringMatching(new RegExp(`^/uploads/W34/${uuid}-image.jpg`)),
-          urlW160: expect.stringMatching(new RegExp(`^/uploads/W160/${uuid}-image.jpg`)),
-          urlW320: expect.stringMatching(new RegExp(`^/uploads/W320/${uuid}-image.jpg`)),
-          urlW640: expect.stringMatching(new RegExp(`^/uploads/W640/${uuid}-image.jpg`)),
-          urlW1024: expect.stringMatching(new RegExp(`^/uploads/W1024/${uuid}-image.jpg`)),
-        })
-      })
-
       it('connects resource with image via given image type', async () => {
         await mergeImage(post, 'HERO_IMAGE', imageInput, { uploadCallback, deleteCallback })
         const result = await neode.cypher(

backend/src/graphql/resolvers/images/images.ts

@@ -7,22 +7,57 @@
 /* eslint-disable @typescript-eslint/no-unsafe-argument */
 /* eslint-disable promise/avoid-new */
 /* eslint-disable security/detect-non-literal-fs-filename */
+
 import { existsSync, unlinkSync, createWriteStream } from 'node:fs'
 import path from 'node:path'
 
+import { S3Client, DeleteObjectCommand, ObjectCannedACL } from '@aws-sdk/client-s3'
+import { Upload } from '@aws-sdk/lib-storage'
 import { UserInputError } from 'apollo-server'
-import { S3 } from 'aws-sdk'
 import slug from 'slug'
 import { v4 as uuid } from 'uuid'
 
 import CONFIG from '@config/index'
 import { getDriver } from '@db/neo4j'
 
-// const widths = [34, 160, 320, 640, 1024]
+import type { FileUpload } from 'graphql-upload'
-const { AWS_ENDPOINT: endpoint, AWS_REGION: region, AWS_BUCKET: Bucket, S3_CONFIGURED } = CONFIG
+import type { Transaction } from 'neo4j-driver'
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
+type FileDeleteCallback = (url: string) => Promise<void>
-export async function deleteImage(resource, relationshipType, opts: any = {}) {
+type FileUploadCallback = (
+  upload: Pick<FileUpload, 'createReadStream' | 'mimetype'> & { uniqueFilename: string },
+) => Promise<string>
+export interface ImageInput {
+  upload?: Promise<FileUpload>
+  alt?: string
+  sensitive?: boolean
+  aspectRatio?: number
+  type?: string
+}
+
+// const widths = [34, 160, 320, 640, 1024]
+const { AWS_BUCKET: Bucket, S3_CONFIGURED } = CONFIG
+
+const createS3Client = () => {
+  const { AWS_ENDPOINT, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY } = CONFIG
+  if (!(AWS_ENDPOINT && AWS_ACCESS_KEY_ID && AWS_SECRET_ACCESS_KEY)) {
+    throw new Error('Missing AWS credentials.')
+  }
+  return new S3Client({
+    credentials: {
+      accessKeyId: AWS_ACCESS_KEY_ID,
+      secretAccessKey: AWS_SECRET_ACCESS_KEY,
+    },
+    endpoint: AWS_ENDPOINT,
+    forcePathStyle: true,
+  })
+}
+
+interface DeleteImageOpts {
+  transaction?: Transaction
+  deleteCallback?: FileDeleteCallback
+}
+export async function deleteImage(resource, relationshipType, opts: DeleteImageOpts = {}) {
   sanitizeRelationshipType(relationshipType)
   const { transaction, deleteCallback } = opts
   if (!transaction) return wrapTransaction(deleteImage, [resource, relationshipType], opts)
@@ -44,8 +79,18 @@ export async function deleteImage(resource, relationshipType, opts: any = {}) {
   return image
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
+interface MergeImageOpts {
-export async function mergeImage(resource, relationshipType, imageInput, opts: any = {}) {
+  transaction?: Transaction
+  uploadCallback?: FileUploadCallback
+  deleteCallback?: FileDeleteCallback
+}
+
+export async function mergeImage(
+  resource,
+  relationshipType,
+  imageInput: ImageInput | null | undefined,
+  opts: MergeImageOpts = {},
+) {
   if (typeof imageInput === 'undefined') return
   if (imageInput === null) return deleteImage(resource, relationshipType, opts)
   sanitizeRelationshipType(relationshipType)
@@ -95,20 +140,25 @@ const wrapTransaction = async (wrappedCallback, args, opts) => {
   }
 }
 
-const deleteImageFile = (image, deleteCallback) => {
+const deleteImageFile = (image, deleteCallback: FileDeleteCallback | undefined) => {
   if (!deleteCallback) {
     deleteCallback = S3_CONFIGURED ? s3Delete : localFileDelete
   }
   const { url } = image
+  // eslint-disable-next-line @typescript-eslint/no-floating-promises
   deleteCallback(url)
   return url
 }
 
-const uploadImageFile = async (upload, uploadCallback) => {
+const uploadImageFile = async (
+  upload: Promise<FileUpload> | undefined,
+  uploadCallback: FileUploadCallback | undefined,
+) => {
   if (!upload) return undefined
   if (!uploadCallback) {
     uploadCallback = S3_CONFIGURED ? s3Upload : localFileUpload
   }
+  // eslint-disable-next-line @typescript-eslint/unbound-method
   const { createReadStream, filename, mimetype } = await upload
   const { name, ext } = path.parse(filename)
   const uniqueFilename = `${uuid()}-${slug(name)}${ext}`
@@ -123,7 +173,7 @@ const sanitizeRelationshipType = (relationshipType) => {
   }
 }
 
-const localFileUpload = ({ createReadStream, uniqueFilename }) => {
+const localFileUpload: FileUploadCallback = ({ createReadStream, uniqueFilename }) => {
   const destination = `/uploads/${uniqueFilename}`
   return new Promise((resolve, reject) =>
     createReadStream().pipe(
@@ -134,41 +184,42 @@ const localFileUpload = ({ createReadStream, uniqueFilename }) => {
   )
 }
 
-const s3Upload = async ({ createReadStream, uniqueFilename, mimetype }) => {
+const s3Upload: FileUploadCallback = async ({ createReadStream, uniqueFilename, mimetype }) => {
-  const s3 = new S3({ region, endpoint })
   const s3Location = `original/${uniqueFilename}`
-
-  if (!Bucket) {
-    throw new Error('AWS_BUCKET is undefined')
-  }
   const params = {
     Bucket,
     Key: s3Location,
-    ACL: 'public-read',
+    ACL: ObjectCannedACL.public_read,
     ContentType: mimetype,
     Body: createReadStream(),
   }
-  const data = await s3.upload(params).promise()
+  const s3 = createS3Client()
+  const command = new Upload({ client: s3, params })
+  const data = await command.done()
   const { Location } = data
+  if (!Location) {
+    throw new Error('File upload did not return `Location`')
+  }
   return Location
 }
 
-const localFileDelete = async (url) => {
+const localFileDelete: FileDeleteCallback = async (url) => {
   const location = `public${url}`
   // eslint-disable-next-line n/no-sync
   if (existsSync(location)) unlinkSync(location)
 }
 
-const s3Delete = async (url) => {
+const s3Delete: FileDeleteCallback = async (url) => {
-  const s3 = new S3({ region, endpoint })
   let { pathname } = new URL(url, 'http://example.org') // dummy domain to avoid invalid URL error
   pathname = pathname.substring(1) // remove first character '/'
-  if (!Bucket) {
+  const prefix = `${Bucket}/`
-    throw new Error('AWS_BUCKET is undefined')
+  if (pathname.startsWith(prefix)) {
+    pathname = pathname.slice(prefix.length)
   }
   const params = {
     Bucket,
     Key: pathname,
   }
-  await s3.deleteObject(params).promise()
+  const s3 = createS3Client()
+  await s3.send(new DeleteObjectCommand(params))
 }