refactor(media): store filenames, use pre-signed s3/azure URLs, UUIDs

Signed-off-by: Erik Michelson <github@erik.michelson.eu>

backend/test/private-api/me.e2e-spec.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -70,16 +70,44 @@ describe('Me', () => {
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
     const imageIds = [];
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note1,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note1,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note2,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note2,
+        )
+      ).uuid,
     );
 
     const response = await agent
@@ -87,10 +115,10 @@ describe('Me', () => {
       .expect('Content-Type', /json/)
       .expect(200);
     expect(response.body).toHaveLength(4);
-    expect(imageIds).toContain(response.body[0].id);
-    expect(imageIds).toContain(response.body[1].id);
-    expect(imageIds).toContain(response.body[2].id);
-    expect(imageIds).toContain(response.body[3].id);
+    expect(imageIds).toContain(response.body[0].uuid);
+    expect(imageIds).toContain(response.body[1].uuid);
+    expect(imageIds).toContain(response.body[2].uuid);
+    expect(imageIds).toContain(response.body[3].uuid);
     const mediaUploads = await testSetup.mediaService.listUploadsByUser(user);
     for (const upload of mediaUploads) {
       await testSetup.mediaService.deleteFile(upload);
@@ -114,6 +142,7 @@ describe('Me', () => {
   it('DELETE /me', async () => {
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
     const upload = await testSetup.mediaService.saveFile(
+      'test.png',
       testImage,
       user,
       note1,
@@ -122,7 +151,7 @@ describe('Me', () => {
     expect(dbUser).toBeInstanceOf(User);
     const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser);
     expect(mediaUploads).toHaveLength(1);
-    expect(mediaUploads[0].id).toEqual(upload.id);
+    expect(mediaUploads[0].uuid).toEqual(upload.uuid);
     await agent.delete('/api/private/me').expect(204);
     await expect(
       testSetup.userService.getUserByUsername('hardcoded'),

backend/test/private-api/media.e2e-spec.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -71,17 +71,17 @@ describe('Media', () => {
           .set('HedgeDoc-Note', 'test_upload_media')
           .expect('Content-Type', /json/)
           .expect(201);
-        const fileName: string = uploadResponse.body.id;
+        const uuid: string = uploadResponse.body.uuid;
         const testImage = await fs.readFile(
           'test/private-api/fixtures/test.png',
         );
-        const path = '/api/private/media/' + fileName;
+        const path = '/api/private/media/' + uuid;
         const apiResponse = await agent.get(path);
-        expect(apiResponse.statusCode).toEqual(302);
-        const downloadResponse = await agent.get(apiResponse.header.location);
+        expect(apiResponse.statusCode).toEqual(200);
+        const downloadResponse = await agent.get(`/uploads/${uuid}.png`);
         expect(downloadResponse.body).toEqual(testImage);
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, uuid + '.png'));
       });
       it('without user', async () => {
         const agent = request.agent(testSetup.app.getHttpServer());
@@ -91,17 +91,17 @@ describe('Media', () => {
           .set('HedgeDoc-Note', 'test_upload_media')
           .expect('Content-Type', /json/)
           .expect(201);
-        const fileName: string = uploadResponse.body.id;
+        const uuid: string = uploadResponse.body.uuid;
         const testImage = await fs.readFile(
           'test/private-api/fixtures/test.png',
         );
-        const path = '/api/private/media/' + fileName;
+        const path = '/api/private/media/' + uuid;
         const apiResponse = await agent.get(path);
-        expect(apiResponse.statusCode).toEqual(302);
-        const downloadResponse = await agent.get(apiResponse.header.location);
+        expect(apiResponse.statusCode).toEqual(200);
+        const downloadResponse = await agent.get(`/uploads/${uuid}.png`);
         expect(downloadResponse.body).toEqual(testImage);
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, uuid + '.png'));
       });
     });
     describe('fails:', () => {
@@ -158,11 +158,12 @@ describe('Media', () => {
       );
       const testImage = await fs.readFile('test/private-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         user,
         testNote,
       );
-      const filename = upload.id;
+      const uuid = upload.uuid;
 
       // login with a different user;
       const agent2 = request.agent(testSetup.app.getHttpServer());
@@ -172,15 +173,15 @@ describe('Media', () => {
         .expect(201);
 
       // try to delete upload with second user
-      await agent2.delete('/api/private/media/' + filename).expect(403);
+      await agent2.delete('/api/private/media/' + uuid).expect(403);
 
-      await agent.get('/uploads/' + filename).expect(200);
+      await agent.get(`/uploads/${uuid}.png`).expect(200);
 
       // delete upload for real
-      await agent.delete('/api/private/media/' + filename).expect(204);
+      await agent.delete('/api/private/media/' + uuid).expect(204);
 
       // Test if file is really deleted
-      await agent.get('/uploads/' + filename).expect(404);
+      await agent.get(`/uploads/${uuid}.png`).expect(404);
     });
     it('deleting user is owner of note', async () => {
       // upload a file with the default test user
@@ -191,11 +192,12 @@ describe('Media', () => {
       );
       const testImage = await fs.readFile('test/private-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         null,
         testNote,
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
+      const uuid = upload.uuid;
 
       // login with a different user;
       const agent2 = request.agent(testSetup.app.getHttpServer());
@@ -207,18 +209,18 @@ describe('Media', () => {
       const agentGuest = request.agent(testSetup.app.getHttpServer());
 
       // try to delete upload with second user
-      await agent.delete('/api/private/media/' + filename).expect(403);
+      await agent.delete('/api/private/media/' + uuid).expect(403);
 
-      await agent.get('/uploads/' + filename).expect(200);
+      await agent.get(`/uploads/${uuid}.png`).expect(200);
 
-      await agentGuest.delete('/api/private/media/' + filename).expect(401);
+      await agentGuest.delete('/api/private/media/' + uuid).expect(401);
 
-      await agent.get('/uploads/' + filename).expect(200);
+      await agent.get(`/uploads/${uuid}.png`).expect(200);
       // delete upload for real
-      await agent2.delete('/api/private/media/' + filename).expect(204);
+      await agent2.delete('/api/private/media/' + uuid).expect(204);
 
       // Test if file is really deleted
-      await agent.get('/uploads/' + filename).expect(404);
+      await agent.get(`/uploads/${uuid}.png`).expect(404);
     });
   });
 });

backend/test/private-api/notes.e2e-spec.ts

@@ -165,7 +165,12 @@ describe('Notes', () => {
           user1,
           noteId,
         );
-        await testSetup.mediaService.saveFile(testImage, user1, note);
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user1,
+          note,
+        );
         await agent
           .delete(`/api/private/notes/${noteId}`)
           .set('Content-Type', 'application/json')
@@ -191,6 +196,7 @@ describe('Notes', () => {
           noteId,
         );
         const upload = await testSetup.mediaService.saveFile(
+          'test.png',
           testImage,
           user1,
           note,
@@ -210,10 +216,8 @@ describe('Notes', () => {
         expect(
           await testSetup.mediaService.listUploadsByUser(user1),
         ).toHaveLength(1);
-        // Remove /upload/ from path as we just need the filename.
-        const fileName = upload.fileUrl.replace('/uploads/', '');
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, upload.uuid + '.png'));
         await fs.rmdir(uploadPath);
       });
     });
@@ -406,11 +410,13 @@ describe('Notes', () => {
 
       const testImage = await fs.readFile('test/private-api/fixtures/test.png');
       const upload0 = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         user1,
         note1,
       );
       const upload1 = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         user1,
         note2,
@@ -421,11 +427,11 @@ describe('Notes', () => {
         .expect('Content-Type', /json/)
         .expect(200);
       expect(responseAfter.body).toHaveLength(1);
-      expect(responseAfter.body[0].id).toEqual(upload0.id);
-      expect(responseAfter.body[0].id).not.toEqual(upload1.id);
+      expect(responseAfter.body[0].uuid).toEqual(upload0.uuid);
+      expect(responseAfter.body[0].uuid).not.toEqual(upload1.uuid);
       for (const upload of [upload0, upload1]) {
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, upload.id));
+        await fs.unlink(join(uploadPath, upload.uuid + '.png'));
       }
       await fs.rm(uploadPath, { recursive: true });
     });

backend/test/public-api/me.e2e-spec.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -201,16 +201,44 @@ describe('Me', () => {
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
     const imageIds = [];
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note1,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note1,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note2,
+        )
+      ).uuid,
     );
     imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
+      (
+        await testSetup.mediaService.saveFile(
+          'test.png',
+          testImage,
+          user,
+          note2,
+        )
+      ).uuid,
     );
 
     const response = await request(httpServer)
@@ -218,13 +246,13 @@ describe('Me', () => {
       .expect('Content-Type', /json/)
       .expect(200);
     expect(response.body).toHaveLength(4);
-    expect(imageIds).toContain(response.body[0].id);
-    expect(imageIds).toContain(response.body[1].id);
-    expect(imageIds).toContain(response.body[2].id);
-    expect(imageIds).toContain(response.body[3].id);
+    expect(imageIds).toContain(response.body[0].uuid);
+    expect(imageIds).toContain(response.body[1].uuid);
+    expect(imageIds).toContain(response.body[2].uuid);
+    expect(imageIds).toContain(response.body[3].uuid);
     for (const imageId of imageIds) {
       // delete the file afterwards
-      await fs.unlink(join(uploadPath, imageId));
+      await fs.unlink(join(uploadPath, imageId + '.png'));
     }
     await fs.rm(uploadPath, { recursive: true });
   });

backend/test/public-api/media.e2e-spec.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -49,17 +49,17 @@ describe('Media', () => {
         .set('HedgeDoc-Note', 'testAlias1')
         .expect('Content-Type', /json/)
         .expect(201);
-      const fileName = uploadResponse.body.id;
-      const path: string = '/api/v2/media/' + fileName;
+      const uuid = uploadResponse.body.uuid;
+      const path: string = '/api/v2/media/' + uuid;
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const apiResponse = await agent
         .get(path)
         .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`);
-      expect(apiResponse.statusCode).toEqual(302);
-      const downloadResponse = await agent.get(apiResponse.header.location);
+      expect(apiResponse.statusCode).toEqual(200);
+      const downloadResponse = await agent.get(`/uploads/${uuid}.png`);
       expect(downloadResponse.body).toEqual(testImage);
       // delete the file afterwards
-      await fs.unlink(join(uploadPath, fileName));
+      await fs.unlink(join(uploadPath, uuid + '.png'));
     });
     describe('fails:', () => {
       beforeEach(async () => {
@@ -114,26 +114,26 @@ describe('Media', () => {
     it('successfully deletes an uploaded file', async () => {
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         testSetup.ownedNotes[0],
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
       await request(testSetup.app.getHttpServer())
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`)
         .expect(204);
     });
     it('returns an error if the user does not own the file', async () => {
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         testSetup.ownedNotes[0],
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
       await request(testSetup.app.getHttpServer())
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(403);
     });
@@ -146,34 +146,34 @@ describe('Media', () => {
       );
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         testNote,
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
 
       const agent2 = request.agent(testSetup.app.getHttpServer());
 
       // try to delete upload with second user
       await agent2
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(403);
 
       await agent2
-        .get('/uploads/' + filename)
+        .get(`/uploads/${upload.uuid}.png`)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(200);
 
       // delete upload for real
       await agent2
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`)
         .expect(204);
 
       // Test if file is really deleted
       await agent2
-        .get('/uploads/' + filename)
+        .get(`/uploads/${upload.uuid}.png`)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(404);
     });
@@ -186,33 +186,33 @@ describe('Media', () => {
       );
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const upload = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         testNote,
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
 
       const agent2 = request.agent(testSetup.app.getHttpServer());
       // try to delete upload with second user
       await agent2
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(403);
 
       await agent2
-        .get('/uploads/' + filename)
+        .get(`/uploads/${upload.uuid}.png`)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(200);
 
       // delete upload for real
       await agent2
-        .delete('/api/v2/media/' + filename)
+        .delete('/api/v2/media/' + upload.uuid)
         .set('Authorization', `Bearer ${testSetup.authTokens[2].secret}`)
         .expect(204);
 
       // Test if file is really deleted
       await agent2
-        .get('/uploads/' + filename)
+        .get(`/uploads/${upload.uuid}.png`)
         .set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
         .expect(404);
     });

backend/test/public-api/notes.e2e-spec.ts

@@ -158,6 +158,7 @@ describe('Notes', () => {
           noteId,
         );
         await testSetup.mediaService.saveFile(
+          'test.png',
           testImage,
           testSetup.users[0],
           note,
@@ -187,6 +188,7 @@ describe('Notes', () => {
           noteId,
         );
         const upload = await testSetup.mediaService.saveFile(
+          'test.png',
           testImage,
           testSetup.users[0],
           note,
@@ -207,10 +209,8 @@ describe('Notes', () => {
         expect(
           await testSetup.mediaService.listUploadsByUser(testSetup.users[0]),
         ).toHaveLength(1);
-        // Remove /upload/ from path as we just need the filename.
-        const fileName = upload.fileUrl.replace('/uploads/', '');
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, upload.uuid + '.png'));
       });
     });
     it('works with an existing alias with permissions', async () => {
@@ -326,7 +326,6 @@ describe('Notes', () => {
       expect(metadata.body.editedBy).toEqual([]);
       expect(metadata.body.permissions.owner).toEqual('testuser1');
       expect(metadata.body.permissions.sharedToUsers).toEqual([]);
-      expect(metadata.body.permissions.sharedToUsers).toEqual([]);
       expect(metadata.body.tags).toEqual([]);
       expect(typeof metadata.body.updatedAt).toEqual('string');
       expect(typeof metadata.body.updateUsername).toEqual('string');
@@ -489,11 +488,13 @@ describe('Notes', () => {
 
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
       const upload0 = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         note1,
       );
       const upload1 = await testSetup.mediaService.saveFile(
+        'test.png',
         testImage,
         testSetup.users[0],
         note2,
@@ -505,11 +506,11 @@ describe('Notes', () => {
         .expect('Content-Type', /json/)
         .expect(200);
       expect(responseAfter.body).toHaveLength(1);
-      expect(responseAfter.body[0].id).toEqual(upload0.id);
-      expect(responseAfter.body[0].id).not.toEqual(upload1.id);
+      expect(responseAfter.body[0].uuid).toEqual(upload0.uuid);
+      expect(responseAfter.body[0].uuid).not.toEqual(upload1.uuid);
       for (const upload of [upload0, upload1]) {
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, upload.id));
+        await fs.unlink(join(uploadPath, upload.uuid + '.png'));
       }
       await fs.rm(uploadPath, { recursive: true });
     });