Avoids easy cheating on vidoe views

5 files changed, 65 insertions, 12 deletions

diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts
index c9334676e..c3d3acd26 100644
--- a/server/controllers/api/videos/index.ts
+++ b/server/controllers/api/videos/index.ts
@@ -22,6 +22,7 @@ import {
 import { fetchRemoteVideoDescription, getVideoActivityPubUrl, shareVideoByServerAndChannel } from '../../../lib/activitypub'
 import { sendCreateVideo, sendCreateViewToOrigin, sendCreateViewToVideoFollowers, sendUpdateVideo } from '../../../lib/activitypub/send'
 import { JobQueue } from '../../../lib/job-queue'
+import { Redis } from '../../../lib/redis'
 import {
   asyncMiddleware,
   authenticate,
@@ -352,7 +353,16 @@ function getVideo (req: express.Request, res: express.Response) {
 async function viewVideo (req: express.Request, res: express.Response) {
   const videoInstance = res.locals.video
 
+  const ip = req.ip
+  const exists = await Redis.Instance.isViewExists(ip, videoInstance.uuid)
+  if (exists) {
+    logger.debug('View for ip %s and video %s already exists.', ip, videoInstance.uuid)
+    return res.status(204).end()
+  }
+
   await videoInstance.increment('views')
+  await Redis.Instance.setView(ip, videoInstance.uuid)
+
   const serverAccount = await getServerActor()
 
   if (videoInstance.isOwned()) {
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 328a3e70a..2dc73770d 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -231,6 +231,8 @@ const CONSTRAINTS_FIELDS = {
   }
 }
 
+let VIDEO_VIEW_LIFETIME = 60000 * 60 // 1 hour
+
 const VIDEO_RATE_TYPES: { [ id: string ]: VideoRateType } = {
   LIKE: 'like',
   DISLIKE: 'dislike'
@@ -400,6 +402,7 @@ if (isTestInstance() === true) {
   ACTIVITY_PUB.ACTOR_REFRESH_INTERVAL = 10 * 1000 // 10 seconds
   CONSTRAINTS_FIELDS.ACTORS.AVATAR.FILE_SIZE.max = 100 * 1024 // 100KB
   SCHEDULER_INTERVAL = 10000
+  VIDEO_VIEW_LIFETIME = 1000 // 1 second
 }
 
 updateWebserverConfig()
@@ -442,7 +445,8 @@ export {
   USER_PASSWORD_RESET_LIFETIME,
   IMAGE_MIMETYPE_EXT,
   SCHEDULER_INTERVAL,
-  JOB_COMPLETED_LIFETIME
+  JOB_COMPLETED_LIFETIME,
+  VIDEO_VIEW_LIFETIME
 }
 
 // ---------------------------------------------------------------------------
diff --git a/server/lib/redis.ts b/server/lib/redis.ts
index 4240cc162..b284cab8f 100644
--- a/server/lib/redis.ts
+++ b/server/lib/redis.ts
@@ -1,7 +1,7 @@
 import { createClient, RedisClient } from 'redis'
 import { logger } from '../helpers/logger'
 import { generateRandomString } from '../helpers/utils'
-import { CONFIG, USER_PASSWORD_RESET_LIFETIME } from '../initializers'
+import { CONFIG, USER_PASSWORD_RESET_LIFETIME, VIDEO_VIEW_LIFETIME } from '../initializers'
 
 class Redis {
 
@@ -46,6 +46,14 @@ class Redis {
     return this.getValue(this.generateResetPasswordKey(userId))
   }
 
+  setView (ip: string, videoUUID: string) {
+    return this.setValue(this.buildViewKey(ip, videoUUID), '1', VIDEO_VIEW_LIFETIME)
+  }
+
+  async isViewExists (ip: string, videoUUID: string) {
+    return this.exists(this.buildViewKey(ip, videoUUID))
+  }
+
   private getValue (key: string) {
     return new Promise<string>((res, rej) => {
       this.client.get(this.prefix + key, (err, value) => {
@@ -68,10 +76,24 @@ class Redis {
     })
   }
 
+  private exists (key: string) {
+    return new Promise<boolean>((res, rej) => {
+      this.client.exists(this.prefix + key, (err, existsNumber) => {
+        if (err) return rej(err)
+
+        return res(existsNumber === 1)
+      })
+    })
+  }
+
   private generateResetPasswordKey (userId: number) {
     return 'reset-password-' + userId
   }
 
+  private buildViewKey (ip: string, videoUUID: string) {
+    return videoUUID + '-' + ip
+  }
+
   static get Instance () {
     return this.instance || (this.instance = new this())
   }
diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts
index c82ac1348..27c4c30b8 100644
--- a/server/tests/api/videos/multiple-servers.ts
+++ b/server/tests/api/videos/multiple-servers.ts
@@ -421,15 +421,22 @@ describe('Test multiple servers', function () {
     })
 
     it('Should view multiple videos on owned servers', async function () {
-      this.timeout(10000)
+      this.timeout(15000)
 
       const tasks: Promise<any>[] = []
-      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(viewVideo(servers[2].url, localVideosServer3[0]))
-      tasks.push(viewVideo(servers[2].url, localVideosServer3[1]))
+      await viewVideo(servers[2].url, localVideosServer3[0])
+      await viewVideo(servers[2].url, localVideosServer3[0])
+      await viewVideo(servers[2].url, localVideosServer3[0])
+      await viewVideo(servers[2].url, localVideosServer3[1])
 
       await Promise.all(tasks)
+      await wait(1500)
+
+      await viewVideo(servers[2].url, localVideosServer3[0])
+
+      await wait(1500)
+
+      await viewVideo(servers[2].url, localVideosServer3[0])
 
       await wait(5000)
 
diff --git a/server/tests/api/videos/single-server.ts b/server/tests/api/videos/single-server.ts
index 83b6a0e9a..cf2721838 100644
--- a/server/tests/api/videos/single-server.ts
+++ b/server/tests/api/videos/single-server.ts
@@ -8,7 +8,7 @@ import {
   checkVideoFilesWereRemoved, completeVideoCheck, flushTests, getVideo, getVideoCategories, getVideoLanguages, getVideoLicences,
   getVideoPrivacies, getVideosList, getVideosListPagination, getVideosListSort, killallServers, rateVideo, removeVideo, runServer,
   searchVideo, searchVideoWithPagination, searchVideoWithSort, ServerInfo, setAccessTokensToServers, testImage, updateVideo, uploadVideo,
-  viewVideo
+  viewVideo, wait
 } from '../../utils'
 
 const expect = chai.expect
@@ -149,8 +149,7 @@ describe('Test a single server', function () {
   })
 
   it('Should get and seed the uploaded video', async function () {
-    // Yes, this could be long
-    this.timeout(60000)
+    this.timeout(5000)
 
     const res = await getVideosList(server.url)
 
@@ -163,8 +162,7 @@ describe('Test a single server', function () {
   })
 
   it('Should get the video by UUID', async function () {
-    // Yes, this could be long
-    this.timeout(60000)
+    this.timeout(5000)
 
     const res = await getVideo(server.url, videoUUID)
 
@@ -173,10 +171,22 @@ describe('Test a single server', function () {
   })
 
   it('Should have the views updated', async function () {
+    this.timeout(10000)
+
     await viewVideo(server.url, videoId)
     await viewVideo(server.url, videoId)
     await viewVideo(server.url, videoId)
 
+    await wait(1500)
+
+    await viewVideo(server.url, videoId)
+    await viewVideo(server.url, videoId)
+
+    await wait(1500)
+
+    await viewVideo(server.url, videoId)
+    await viewVideo(server.url, videoId)
+
     const res = await getVideo(server.url, videoId)
 
     const video = res.body