Remove resumable cache after upload success

5 files changed, 124 insertions, 18 deletions

diff --git a/server/controllers/api/videos/upload.ts b/server/controllers/api/videos/upload.ts
index 02aadd426..3e9979330 100644
--- a/server/controllers/api/videos/upload.ts
+++ b/server/controllers/api/videos/upload.ts
@@ -19,7 +19,7 @@ import { VideoPathManager } from '@server/lib/video-path-manager'
 import { buildNextVideoState } from '@server/lib/video-state'
 import { openapiOperationDoc } from '@server/middlewares/doc'
 import { MVideo, MVideoFile, MVideoFullLight } from '@server/types/models'
-import { uploadx } from '@uploadx/core'
+import { Uploadx } from '@uploadx/core'
 import { VideoCreate, VideoState } from '../../../../shared'
 import { HttpStatusCode } from '../../../../shared/models'
 import { auditLoggerFactory, getAuditIdFromRes, VideoAuditView } from '../../../helpers/audit-logger'
@@ -41,6 +41,7 @@ import {
   authenticate,
   videosAddLegacyValidator,
   videosAddResumableInitValidator,
+  videosResumableUploadIdValidator,
   videosAddResumableValidator
 } from '../../../middlewares'
 import { ScheduleVideoUpdateModel } from '../../../models/video/schedule-video-update'
@@ -50,7 +51,9 @@ import { VideoFileModel } from '../../../models/video/video-file'
 const lTags = loggerTagsFactory('api', 'video')
 const auditLogger = auditLoggerFactory('videos')
 const uploadRouter = express.Router()
-const uploadxMiddleware = uploadx.upload({ directory: getResumableUploadPath() })
+
+const uploadx = new Uploadx({ directory: getResumableUploadPath() })
+uploadx.getUserId = (_, res: express.Response) => res.locals.oauth?.token.user.id
 
 const reqVideoFileAdd = createReqFiles(
   [ 'videofile', 'thumbnailfile', 'previewfile' ],
@@ -84,18 +87,21 @@ uploadRouter.post('/upload-resumable',
   authenticate,
   reqVideoFileAddResumable,
   asyncMiddleware(videosAddResumableInitValidator),
-  uploadxMiddleware
+  uploadx.upload
 )
 
 uploadRouter.delete('/upload-resumable',
   authenticate,
-  uploadxMiddleware
+  videosResumableUploadIdValidator,
+  asyncMiddleware(deleteUploadResumableCache),
+  uploadx.upload
 )
 
 uploadRouter.put('/upload-resumable',
   openapiOperationDoc({ operationId: 'uploadResumable' }),
   authenticate,
-  uploadxMiddleware, // uploadx doesn't next() before the file upload completes
+  videosResumableUploadIdValidator,
+  uploadx.upload, // uploadx doesn't next() before the file upload completes
   asyncMiddleware(videosAddResumableValidator),
   asyncMiddleware(addVideoResumable)
 )
@@ -108,7 +114,7 @@ export {
 
 // ---------------------------------------------------------------------------
 
-export async function addVideoLegacy (req: express.Request, res: express.Response) {
+async function addVideoLegacy (req: express.Request, res: express.Response) {
   // Uploading the video could be long
   // Set timeout to 10 minutes, as Express's default is 2 minutes
   req.setTimeout(1000 * 60 * 10, () => {
@@ -128,7 +134,7 @@ export async function addVideoLegacy (req: express.Request, res: express.Respons
   return res.json(response)
 }
 
-export async function addVideoResumable (req: express.Request, res: express.Response) {
+async function addVideoResumable (req: express.Request, res: express.Response) {
   const videoPhysicalFile = res.locals.videoFileResumable
   const videoInfo = videoPhysicalFile.metadata
   const files = { previewfile: videoInfo.previewfile }
@@ -291,3 +297,9 @@ function createTorrentFederate (video: MVideoFullLight, videoFile: MVideoFile) {
     })
     .catch(err => logger.error('Cannot federate or notify video creation %s', video.url, { err, ...lTags(video.uuid) }))
 }
+
+async function deleteUploadResumableCache (req: express.Request, res: express.Response, next: express.NextFunction) {
+  await Redis.Instance.deleteUploadSession(req.query.upload_id)
+
+  return next()
+}
diff --git a/server/lib/redis.ts b/server/lib/redis.ts
index 76b7868e8..8aec4b793 100644
--- a/server/lib/redis.ts
+++ b/server/lib/redis.ts
@@ -271,6 +271,10 @@ class Redis {
       : ''
   }
 
+  deleteUploadSession (uploadId: string) {
+    return this.deleteKey('resumable-upload-' + uploadId)
+  }
+
   /* ************ Keys generation ************ */
 
   generateCachedRouteKey (req: express.Request) {
diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 5f1234379..53643635c 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -103,6 +103,22 @@ const videosAddLegacyValidator = getCommonVideoEditAttributes().concat([
   }
 ])
 
+const videosResumableUploadIdValidator = [
+  (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    const user = res.locals.oauth.token.User
+    const uploadId = req.query.upload_id
+
+    if (uploadId.startsWith(user.id + '-') !== true) {
+      return res.fail({
+        status: HttpStatusCode.FORBIDDEN_403,
+        message: 'You cannot send chunks in another user upload'
+      })
+    }
+
+    return next()
+  }
+]
+
 /**
  * Gets called after the last PUT request
  */
@@ -110,7 +126,7 @@ const videosAddResumableValidator = [
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     const user = res.locals.oauth.token.User
     const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body
-    const file = { ...body, duration: undefined, path: getResumableUploadPath(body.id), filename: body.metadata.filename }
+    const file = { ...body, duration: undefined, path: getResumableUploadPath(body.name), filename: body.metadata.filename }
     const cleanup = () => deleteFileAndCatch(file.path)
 
     const uploadId = req.query.upload_id
@@ -552,6 +568,7 @@ export {
   videosAddLegacyValidator,
   videosAddResumableValidator,
   videosAddResumableInitValidator,
+  videosResumableUploadIdValidator,
 
   videosUpdateValidator,
   videosGetValidator,
diff --git a/server/tests/api/videos/resumable-upload.ts b/server/tests/api/videos/resumable-upload.ts
index 6b5e0c09d..1ba7cdbcc 100644
--- a/server/tests/api/videos/resumable-upload.ts
+++ b/server/tests/api/videos/resumable-upload.ts
@@ -22,6 +22,8 @@ describe('Test resumable upload', function () {
   const defaultFixture = 'video_short.mp4'
   let server: PeerTubeServer
   let rootId: number
+  let userAccessToken: string
+  let userChannelId: number
 
   async function buildSize (fixture: string, size?: number) {
     if (size !== undefined) return size
@@ -30,24 +32,33 @@ describe('Test resumable upload', function () {
     return (await stat(baseFixture)).size
   }
 
-  async function prepareUpload (sizeArg?: number) {
-    const size = await buildSize(defaultFixture, sizeArg)
+  async function prepareUpload (options: {
+    channelId?: number
+    token?: string
+    size?: number
+    originalName?: string
+    lastModified?: number
+  } = {}) {
+    const { token, originalName, lastModified } = options
+
+    const size = await buildSize(defaultFixture, options.size)
 
     const attributes = {
       name: 'video',
-      channelId: server.store.channel.id,
+      channelId: options.channelId ?? server.store.channel.id,
       privacy: VideoPrivacy.PUBLIC,
       fixture: defaultFixture
     }
 
     const mimetype = 'video/mp4'
 
-    const res = await server.videos.prepareResumableUpload({ attributes, size, mimetype })
+    const res = await server.videos.prepareResumableUpload({ token, attributes, size, mimetype, originalName, lastModified })
 
     return res.header['location'].split('?')[1]
   }
 
   async function sendChunks (options: {
+    token?: string
     pathUploadId: string
     size?: number
     expectedStatus?: HttpStatusCode
@@ -55,12 +66,13 @@ describe('Test resumable upload', function () {
     contentRange?: string
     contentRangeBuilder?: (start: number, chunk: any) => string
   }) {
-    const { pathUploadId, expectedStatus, contentLength, contentRangeBuilder } = options
+    const { token, pathUploadId, expectedStatus, contentLength, contentRangeBuilder } = options
 
     const size = await buildSize(defaultFixture, options.size)
     const absoluteFilePath = buildAbsoluteFixturePath(defaultFixture)
 
     return server.videos.sendResumableChunks({
+      token,
       pathUploadId,
       videoFilePath: absoluteFilePath,
       size,
@@ -105,6 +117,12 @@ describe('Test resumable upload', function () {
     const body = await server.users.getMyInfo()
     rootId = body.id
 
+    {
+      userAccessToken = await server.users.generateUserAndToken('user1')
+      const { videoChannels } = await server.users.getMyInfo({ token: userAccessToken })
+      userChannelId = videoChannels[0].id
+    }
+
     await server.users.update({ userId: rootId, videoQuota: 10_000_000 })
   })
 
@@ -147,14 +165,14 @@ describe('Test resumable upload', function () {
     })
 
     it('Should not accept more chunks than expected', async function () {
-      const uploadId = await prepareUpload(100)
+      const uploadId = await prepareUpload({ size: 100 })
 
       await sendChunks({ pathUploadId: uploadId, expectedStatus: HttpStatusCode.CONFLICT_409 })
       await checkFileSize(uploadId, 0)
     })
 
     it('Should not accept more chunks than expected with an invalid content length/content range', async function () {
-      const uploadId = await prepareUpload(1500)
+      const uploadId = await prepareUpload({ size: 1500 })
 
       // Content length check seems to have changed in v16
       if (process.version.startsWith('v16')) {
@@ -167,7 +185,7 @@ describe('Test resumable upload', function () {
     })
 
     it('Should not accept more chunks than expected with an invalid content length', async function () {
-      const uploadId = await prepareUpload(500)
+      const uploadId = await prepareUpload({ size: 500 })
 
       const size = 1000
 
@@ -195,6 +213,51 @@ describe('Test resumable upload', function () {
 
       await checkFileSize(uploadId, null)
     })
+
+    it('Should not have the same upload id with 2 different users', async function () {
+      const originalName = 'toto.mp4'
+      const lastModified = new Date().getTime()
+
+      const uploadId1 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
+      const uploadId2 = await prepareUpload({ originalName, lastModified, channelId: userChannelId, token: userAccessToken })
+
+      expect(uploadId1).to.not.equal(uploadId2)
+    })
+
+    it('Should have the same upload id with the same user', async function () {
+      const originalName = 'toto.mp4'
+      const lastModified = new Date().getTime()
+
+      const uploadId1 = await prepareUpload({ originalName, lastModified })
+      const uploadId2 = await prepareUpload({ originalName, lastModified })
+
+      expect(uploadId1).to.equal(uploadId2)
+    })
+
+    it('Should not cache a request with 2 different users', async function () {
+      const originalName = 'toto.mp4'
+      const lastModified = new Date().getTime()
+
+      const uploadId = await prepareUpload({ originalName, lastModified, token: server.accessToken })
+
+      await sendChunks({ pathUploadId: uploadId, token: server.accessToken })
+      await sendChunks({ pathUploadId: uploadId, token: userAccessToken, expectedStatus: HttpStatusCode.FORBIDDEN_403 })
+    })
+
+    it('Should not cache a request after a delete', async function () {
+      const originalName = 'toto.mp4'
+      const lastModified = new Date().getTime()
+      const uploadId1 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
+
+      await sendChunks({ pathUploadId: uploadId1 })
+      await server.videos.endResumableUpload({ pathUploadId: uploadId1 })
+
+      const uploadId2 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
+      expect(uploadId1).to.equal(uploadId2)
+
+      const result2 = await sendChunks({ pathUploadId: uploadId1 })
+      expect(result2.headers['x-resumable-upload-cached']).to.not.exist
+    })
   })
 
   after(async function () {
diff --git a/shared/extra-utils/videos/videos-command.ts b/shared/extra-utils/videos/videos-command.ts
index 68241f062..167fae22d 100644
--- a/shared/extra-utils/videos/videos-command.ts
+++ b/shared/extra-utils/videos/videos-command.ts
@@ -469,8 +469,11 @@ export class VideosCommand extends AbstractCommand {
     attributes: VideoEdit
     size: number
     mimetype: string
+
+    originalName?: string
+    lastModified?: number
   }) {
-    const { attributes, size, mimetype } = options
+    const { attributes, originalName, lastModified, size, mimetype } = options
 
     const path = '/api/v1/videos/upload-resumable'
 
@@ -482,7 +485,14 @@ export class VideosCommand extends AbstractCommand {
         'X-Upload-Content-Type': mimetype,
         'X-Upload-Content-Length': size.toString()
       },
-      fields: { filename: attributes.fixture, ...this.buildUploadFields(options.attributes) },
+      fields: {
+        filename: attributes.fixture,
+        originalName,
+        lastModified,
+
+        ...this.buildUploadFields(options.attributes)
+      },
+
       // Fixture will be sent later
       attaches: this.buildUploadAttaches(omit(options.attributes, 'fixture')),
       implicitToken: true,