From b5c0e95544cec5a33cee3df41c1607d2a0cd5403 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 23 Feb 2018 16:39:51 +0100 Subject: [PATCH] Avoids easy cheating on vidoe views --- server/controllers/api/videos/index.ts | 10 +++++++++ server/initializers/constants.ts | 6 +++++- server/lib/redis.ts | 24 ++++++++++++++++++++- server/tests/api/videos/multiple-servers.ts | 17 ++++++++++----- server/tests/api/videos/single-server.ts | 20 ++++++++++++----- 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((res, rej) => { this.client.get(this.prefix + key, (err, value) => { @@ -68,10 +76,24 @@ class Redis { }) } + private exists (key: string) { + return new Promise((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[] = [] - 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