From feb34f6b6b991046aab6a10df747b48fa4da07a7 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 6 May 2020 17:39:07 +0200 Subject: Use video abuse filters on client side --- server/controllers/api/videos/abuse.ts | 11 +- server/helpers/custom-validators/video-abuses.ts | 10 ++ .../middlewares/validators/videos/video-abuses.ts | 45 +++++++- server/models/utils.ts | 60 +--------- server/models/video/video-abuse.ts | 127 +++++++++++---------- server/tests/api/check-params/video-abuses.ts | 16 +++ server/tests/api/users/users.ts | 2 +- server/tests/api/videos/video-abuse.ts | 70 +++++++++--- 8 files changed, 205 insertions(+), 136 deletions(-) (limited to 'server') diff --git a/server/controllers/api/videos/abuse.ts b/server/controllers/api/videos/abuse.ts index bc7df48c8..3fe7f7e51 100644 --- a/server/controllers/api/videos/abuse.ts +++ b/server/controllers/api/videos/abuse.ts @@ -14,7 +14,8 @@ import { videoAbuseGetValidator, videoAbuseReportValidator, videoAbusesSortValidator, - videoAbuseUpdateValidator + videoAbuseUpdateValidator, + videoAbuseListValidator } from '../../../middlewares' import { AccountModel } from '../../../models/account/account' import { VideoAbuseModel } from '../../../models/video/video-abuse' @@ -34,6 +35,7 @@ abuseVideoRouter.get('/abuse', videoAbusesSortValidator, setDefaultSort, setDefaultPagination, + videoAbuseListValidator, asyncMiddleware(listVideoAbuses) ) abuseVideoRouter.put('/:videoId/abuse/:id', @@ -70,7 +72,14 @@ async function listVideoAbuses (req: express.Request, res: express.Response) { start: req.query.start, count: req.query.count, sort: req.query.sort, + id: req.query.id, search: req.query.search, + state: req.query.state, + videoIs: req.query.videoIs, + searchReporter: req.query.searchReporter, + searchReportee: req.query.searchReportee, + searchVideo: req.query.searchVideo, + searchVideoChannel: req.query.searchVideoChannel, serverAccountId: serverActor.Account.id, user }) diff --git a/server/helpers/custom-validators/video-abuses.ts b/server/helpers/custom-validators/video-abuses.ts index 5c7bc6fd9..05e11b1c6 100644 --- a/server/helpers/custom-validators/video-abuses.ts +++ b/server/helpers/custom-validators/video-abuses.ts @@ -1,6 +1,8 @@ import validator from 'validator' + import { CONSTRAINTS_FIELDS, VIDEO_ABUSE_STATES } from '../../initializers/constants' import { exists } from './misc' +import { VideoAbuseVideoIs } from '@shared/models/videos/abuse/video-abuse-video-is.type' const VIDEO_ABUSES_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEO_ABUSES @@ -16,10 +18,18 @@ function isVideoAbuseStateValid (value: string) { return exists(value) && VIDEO_ABUSE_STATES[value] !== undefined } +function isAbuseVideoIsValid (value: VideoAbuseVideoIs) { + return exists(value) && ( + value === 'deleted' || + value === 'blacklisted' + ) +} + // --------------------------------------------------------------------------- export { isVideoAbuseStateValid, isVideoAbuseReasonValid, + isAbuseVideoIsValid, isVideoAbuseModerationCommentValid } diff --git a/server/middlewares/validators/videos/video-abuses.ts b/server/middlewares/validators/videos/video-abuses.ts index 7c316fe13..901997bcb 100644 --- a/server/middlewares/validators/videos/video-abuses.ts +++ b/server/middlewares/validators/videos/video-abuses.ts @@ -1,14 +1,15 @@ import * as express from 'express' -import { body, param } from 'express-validator' -import { isIdOrUUIDValid, isIdValid } from '../../../helpers/custom-validators/misc' -import { logger } from '../../../helpers/logger' -import { areValidationErrors } from '../utils' +import { body, param, query } from 'express-validator' +import { exists, isIdOrUUIDValid, isIdValid } from '../../../helpers/custom-validators/misc' import { + isAbuseVideoIsValid, isVideoAbuseModerationCommentValid, isVideoAbuseReasonValid, isVideoAbuseStateValid } from '../../../helpers/custom-validators/video-abuses' +import { logger } from '../../../helpers/logger' import { doesVideoAbuseExist, doesVideoExist } from '../../../helpers/middlewares' +import { areValidationErrors } from '../utils' const videoAbuseReportValidator = [ param('videoId').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid videoId'), @@ -58,9 +59,45 @@ const videoAbuseUpdateValidator = [ } ] +const videoAbuseListValidator = [ + query('id') + .optional() + .custom(isIdValid).withMessage('Should have a valid id'), + query('search') + .optional() + .custom(exists).withMessage('Should have a valid search'), + query('state') + .optional() + .custom(isVideoAbuseStateValid).withMessage('Should have a valid video abuse state'), + query('videoIs') + .optional() + .custom(isAbuseVideoIsValid).withMessage('Should have a valid "video is" attribute'), + query('searchReporter') + .optional() + .custom(exists).withMessage('Should have a valid reporter search'), + query('searchReportee') + .optional() + .custom(exists).withMessage('Should have a valid reportee search'), + query('searchVideo') + .optional() + .custom(exists).withMessage('Should have a valid video search'), + query('searchVideoChannel') + .optional() + .custom(exists).withMessage('Should have a valid video channel search'), + + (req: express.Request, res: express.Response, next: express.NextFunction) => { + logger.debug('Checking videoAbuseListValidator parameters', { parameters: req.body }) + + if (areValidationErrors(req, res)) return + + return next() + } +] + // --------------------------------------------------------------------------- export { + videoAbuseListValidator, videoAbuseReportValidator, videoAbuseGetValidator, videoAbuseUpdateValidator diff --git a/server/models/utils.ts b/server/models/utils.ts index fe4596d31..b2573cd35 100644 --- a/server/models/utils.ts +++ b/server/models/utils.ts @@ -207,60 +207,13 @@ function buildDirectionAndField (value: string) { return { direction, field } } -function searchAttribute (sourceField, targetField) { - if (sourceField) { - return { - [targetField]: { - [Op.iLike]: `%${sourceField}%` - } - } - } else { - return {} - } -} - -interface QueryStringFilterPrefixes { - [key: string]: string | { prefix: string, handler: Function, multiple?: boolean } -} - -function parseQueryStringFilter (q: string, prefixes: QueryStringFilterPrefixes): { - search: string - [key: string]: string | number | string[] | number[] -} { - const tokens = q // tokenize only if we have a querystring - ? [].concat.apply([], q.split('"').map((v, i) => i % 2 ? v : v.split(' '))).filter(Boolean) // split by space unless using double quotes - : [] - - const objectMap = (obj, fn) => Object.fromEntries( - Object.entries(obj).map( - ([ k, v ], i) => [ k, fn(v, k, i) ] - ) - ) +function searchAttribute (sourceField?: string, targetField?: string) { + if (!sourceField) return {} return { - // search is the querystring minus defined filters - search: tokens.filter(e => !Object.values(prefixes).some(p => { - if (typeof p === 'string') { - return e.startsWith(p) - } else { - return e.startsWith(p.prefix) - } - })).join(' '), - // filters defined in prefixes are added under their own name - ...objectMap(prefixes, p => { - if (typeof p === 'string') { - return tokens.filter(e => e.startsWith(p)).map(e => e.slice(p.length)) // we keep the matched item, and remove its prefix - } else { - const _tokens = tokens.filter(e => e.startsWith(p.prefix)).map(e => e.slice(p.prefix.length)).map(p.handler) - // multiple is false by default, meaning we usually just keep the first occurence of a given prefix - if (!p.multiple && _tokens.length > 0) { - return _tokens[0] - } else if (!p.multiple) { - return '' - } - return _tokens - } - }) + [targetField]: { + [Op.iLike]: `%${sourceField}%` + } } } @@ -286,8 +239,7 @@ export { getFollowsSort, buildDirectionAndField, createSafeIn, - searchAttribute, - parseQueryStringFilter + searchAttribute } // --------------------------------------------------------------------------- diff --git a/server/models/video/video-abuse.ts b/server/models/video/video-abuse.ts index 6cd2c0418..0844f702d 100644 --- a/server/models/video/video-abuse.ts +++ b/server/models/video/video-abuse.ts @@ -1,6 +1,21 @@ +import * as Bluebird from 'bluebird' +import { literal, Op } from 'sequelize' import { - AllowNull, BelongsTo, Column, CreatedAt, DataType, Default, ForeignKey, Is, Model, Table, UpdatedAt, Scopes + AllowNull, + BelongsTo, + Column, + CreatedAt, + DataType, + Default, + ForeignKey, + Is, + Model, + Scopes, + Table, + UpdatedAt } from 'sequelize-typescript' +import { VideoAbuseVideoIs } from '@shared/models/videos/abuse/video-abuse-video-is.type' +import { VideoAbuseState, VideoDetails } from '../../../shared' import { VideoAbuseObject } from '../../../shared/models/activitypub/objects' import { VideoAbuse } from '../../../shared/models/videos' import { @@ -8,15 +23,12 @@ import { isVideoAbuseReasonValid, isVideoAbuseStateValid } from '../../helpers/custom-validators/video-abuses' -import { AccountModel } from '../account/account' -import { buildBlockedAccountSQL, getSort, throwIfNotValid, searchAttribute, parseQueryStringFilter } from '../utils' -import { VideoModel } from './video' -import { VideoAbuseState, VideoDetails } from '../../../shared' import { CONSTRAINTS_FIELDS, VIDEO_ABUSE_STATES } from '../../initializers/constants' import { MUserAccountId, MVideoAbuse, MVideoAbuseFormattable, MVideoAbuseVideo } from '../../typings/models' -import * as Bluebird from 'bluebird' -import { literal, Op } from 'sequelize' +import { AccountModel } from '../account/account' +import { buildBlockedAccountSQL, getSort, searchAttribute, throwIfNotValid } from '../utils' import { ThumbnailModel } from './thumbnail' +import { VideoModel } from './video' import { VideoBlacklistModel } from './video-blacklist' import { ScopeNames as VideoChannelScopeNames, SummaryOptions, VideoChannelModel } from './video-channel' @@ -35,21 +47,22 @@ export enum ScopeNames { // filters id?: number + state?: VideoAbuseState - is?: 'deleted' | 'blacklisted' + videoIs?: VideoAbuseVideoIs // accountIds serverAccountId: number userAccountId: number }) => { - let where = { + const where = { reporterAccountId: { [Op.notIn]: literal('(' + buildBlockedAccountSQL(options.serverAccountId, options.userAccountId) + ')') } } if (options.search) { - where = Object.assign(where, { + Object.assign(where, { [Op.or]: [ { [Op.and]: [ @@ -80,26 +93,18 @@ export enum ScopeNames { }) } - if (options.id) { - where = Object.assign(where, { - id: options.id - }) - } + if (options.id) Object.assign(where, { id: options.id }) + if (options.state) Object.assign(where, { state: options.state }) - if (options.state) { - where = Object.assign(where, { - state: options.state + if (options.videoIs === 'deleted') { + Object.assign(where, { + deletedVideo: { + [Op.not]: null + } }) } - let onlyBlacklisted = false - if (options.is === 'deleted') { - where = Object.assign(where, { - deletedVideo: { [Op.not]: null } - }) - } else if (options.is === 'blacklisted') { - onlyBlacklisted = true - } + const onlyBlacklisted = options.videoIs === 'blacklisted' return { attributes: { @@ -189,7 +194,7 @@ export enum ScopeNames { }, { model: VideoModel, - required: onlyBlacklisted, + required: !!(onlyBlacklisted || options.searchVideo || options.searchReportee || options.searchVideoChannel), where: searchAttribute(options.searchVideo, 'name'), include: [ { @@ -301,11 +306,36 @@ export class VideoAbuseModel extends Model { start: number count: number sort: string - search?: string + serverAccountId: number user?: MUserAccountId + + id?: number + state?: VideoAbuseState + videoIs?: VideoAbuseVideoIs + + search?: string + searchReporter?: string + searchReportee?: string + searchVideo?: string + searchVideoChannel?: string }) { - const { start, count, sort, search, user, serverAccountId } = parameters + const { + start, + count, + sort, + search, + user, + serverAccountId, + state, + videoIs, + searchReportee, + searchVideo, + searchVideoChannel, + searchReporter, + id + } = parameters + const userAccountId = user ? user.Account.id : undefined const query = { @@ -317,37 +347,14 @@ export class VideoAbuseModel extends Model { } const filters = { - ...parseQueryStringFilter(search, { - id: { - prefix: '#', - handler: v => v - }, - state: { - prefix: 'state:', - handler: v => { - if (v === 'accepted') return VideoAbuseState.ACCEPTED - if (v === 'pending') return VideoAbuseState.PENDING - if (v === 'rejected') return VideoAbuseState.REJECTED - return undefined - } - }, - is: { - prefix: 'is:', - handler: v => { - if (v === 'deleted') return v - if (v === 'blacklisted') return v - return undefined - } - }, - searchReporter: { - prefix: 'reporter:', - handler: v => v - }, - searchReportee: { - prefix: 'reportee:', - handler: v => v - } - }), + id, + search, + state, + videoIs, + searchReportee, + searchVideo, + searchVideoChannel, + searchReporter, serverAccountId, userAccountId } diff --git a/server/tests/api/check-params/video-abuses.ts b/server/tests/api/check-params/video-abuses.ts index bea2177f3..e643cb95e 100644 --- a/server/tests/api/check-params/video-abuses.ts +++ b/server/tests/api/check-params/video-abuses.ts @@ -76,6 +76,22 @@ describe('Test video abuses API validators', function () { statusCodeExpected: 403 }) }) + + it('Should fail with a bad id filter', async function () { + await makeGetRequest({ url: server.url, path, token: server.accessToken, query: { id: 'toto' } }) + }) + + it('Should fail with a bad state filter', async function () { + await makeGetRequest({ url: server.url, path, token: server.accessToken, query: { state: 'toto' } }) + }) + + it('Should fail with a bad videoIs filter', async function () { + await makeGetRequest({ url: server.url, path, token: server.accessToken, query: { videoIs: 'toto' } }) + }) + + it('Should succeed with the correct params', async function () { + await makeGetRequest({ url: server.url, path, token: server.accessToken, query: { id: 13 }, statusCodeExpected: 200 }) + }) }) describe('When reporting a video abuse', function () { diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index 60fbd2a20..f3b732632 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -901,7 +901,7 @@ describe('Test users', function () { const reason = 'my super bad reason' await reportVideoAbuse(server.url, user17AccessToken, videoId, reason) - const res1 = await getVideoAbusesList(server.url, server.accessToken) + const res1 = await getVideoAbusesList({ url: server.url, token: server.accessToken }) const abuseId = res1.body.data[0].id const res2 = await getUserInformation(server.url, server.accessToken, user17Id, true) diff --git a/server/tests/api/videos/video-abuse.ts b/server/tests/api/videos/video-abuse.ts index 26bc3783b..a96be97f6 100644 --- a/server/tests/api/videos/video-abuse.ts +++ b/server/tests/api/videos/video-abuse.ts @@ -71,7 +71,7 @@ describe('Test video abuses', function () { }) it('Should not have video abuses', async function () { - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(0) expect(res.body.data).to.be.an('array') @@ -89,7 +89,7 @@ describe('Test video abuses', function () { }) it('Should have 1 video abuses on server 1 and 0 on server 2', async function () { - const res1 = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res1 = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res1.body.total).to.equal(1) expect(res1.body.data).to.be.an('array') @@ -106,7 +106,7 @@ describe('Test video abuses', function () { expect(abuse.countReportsForReporter).to.equal(1) expect(abuse.countReportsForReportee).to.equal(1) - const res2 = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res2 = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res2.body.total).to.equal(0) expect(res2.body.data).to.be.an('array') expect(res2.body.data.length).to.equal(0) @@ -123,7 +123,7 @@ describe('Test video abuses', function () { }) it('Should have 2 video abuses on server 1 and 1 on server 2', async function () { - const res1 = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res1 = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res1.body.total).to.equal(2) expect(res1.body.data).to.be.an('array') expect(res1.body.data.length).to.equal(2) @@ -148,7 +148,7 @@ describe('Test video abuses', function () { expect(abuse2.state.label).to.equal('Pending') expect(abuse2.moderationComment).to.be.null - const res2 = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res2 = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res2.body.total).to.equal(1) expect(res2.body.data).to.be.an('array') expect(res2.body.data.length).to.equal(1) @@ -166,7 +166,7 @@ describe('Test video abuses', function () { const body = { state: VideoAbuseState.REJECTED } await updateVideoAbuse(servers[1].url, servers[1].accessToken, abuseServer2.video.uuid, abuseServer2.id, body) - const res = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res.body.data[0].state.id).to.equal(VideoAbuseState.REJECTED) }) @@ -174,7 +174,7 @@ describe('Test video abuses', function () { const body = { state: VideoAbuseState.ACCEPTED, moderationComment: 'It is valid' } await updateVideoAbuse(servers[1].url, servers[1].accessToken, abuseServer2.video.uuid, abuseServer2.id, body) - const res = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res.body.data[0].state.id).to.equal(VideoAbuseState.ACCEPTED) expect(res.body.data[0].moderationComment).to.equal('It is valid') }) @@ -186,7 +186,7 @@ describe('Test video abuses', function () { await reportVideoAbuse(servers[1].url, servers[1].accessToken, servers[0].video.uuid, 'will mute this') await waitJobs(servers) - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(3) } @@ -195,7 +195,7 @@ describe('Test video abuses', function () { { await addAccountToServerBlocklist(servers[0].url, servers[0].accessToken, accountToBlock) - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(2) const abuse = res.body.data.find(a => a.reason === 'will mute this') @@ -205,7 +205,7 @@ describe('Test video abuses', function () { { await removeAccountFromServerBlocklist(servers[0].url, servers[0].accessToken, accountToBlock) - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(3) } }) @@ -216,7 +216,7 @@ describe('Test video abuses', function () { { await addServerToServerBlocklist(servers[0].url, servers[0].accessToken, servers[1].host) - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(2) const abuse = res.body.data.find(a => a.reason === 'will mute this') @@ -226,7 +226,7 @@ describe('Test video abuses', function () { { await removeServerFromServerBlocklist(servers[0].url, servers[0].accessToken, serverToBlock) - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(3) } }) @@ -238,7 +238,7 @@ describe('Test video abuses', function () { await waitJobs(servers) - const res = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res.body.total).to.equal(2, "wrong number of videos returned") expect(res.body.data.length).to.equal(2, "wrong number of videos returned") expect(res.body.data[0].id).to.equal(abuseServer2.id, "wrong origin server id for first video") @@ -274,7 +274,7 @@ describe('Test video abuses', function () { const reason4 = 'my super bad reason 4' await reportVideoAbuse(servers[0].url, userAccessToken, servers[0].video.id, reason4) - const res2 = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res2 = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) { for (const abuse of res2.body.data as VideoAbuse[]) { @@ -299,18 +299,56 @@ describe('Test video abuses', function () { await waitJobs(servers) { - const res = await getVideoAbusesList(servers[1].url, servers[1].accessToken) + const res = await getVideoAbusesList({ url: servers[1].url, token: servers[1].accessToken }) expect(res.body.total).to.equal(1) expect(res.body.data.length).to.equal(1) expect(res.body.data[0].id).to.not.equal(abuseServer2.id) } { - const res = await getVideoAbusesList(servers[0].url, servers[0].accessToken) + const res = await getVideoAbusesList({ url: servers[0].url, token: servers[0].accessToken }) expect(res.body.total).to.equal(5) } }) + it('Should list and filter video abuses', async function () { + async function list (query: Omit[0], 'url' | 'token'>) { + const options = { + url: servers[0].url, + token: servers[0].accessToken + } + + Object.assign(options, query) + + const res = await getVideoAbusesList(options) + + return res.body.data as VideoAbuse[] + } + + expect(await list({ id: 56 })).to.have.lengthOf(0) + expect(await list({ id: 1 })).to.have.lengthOf(1) + + expect(await list({ search: 'my super name for server 1' })).to.have.lengthOf(3) + expect(await list({ search: 'aaaaaaaaaaaaaaaaaaaaaaaaaa' })).to.have.lengthOf(0) + + expect(await list({ searchVideo: 'my second super name for server 1' })).to.have.lengthOf(1) + + expect(await list({ searchVideoChannel: 'root' })).to.have.lengthOf(3) + expect(await list({ searchVideoChannel: 'aaaa' })).to.have.lengthOf(0) + + expect(await list({ searchReporter: 'user2' })).to.have.lengthOf(1) + expect(await list({ searchReporter: 'root' })).to.have.lengthOf(4) + + expect(await list({ searchReportee: 'root' })).to.have.lengthOf(3) + expect(await list({ searchReportee: 'aaaa' })).to.have.lengthOf(0) + + expect(await list({ videoIs: 'deleted' })).to.have.lengthOf(1) + expect(await list({ videoIs: 'blacklisted' })).to.have.lengthOf(0) + + expect(await list({ state: VideoAbuseState.ACCEPTED })).to.have.lengthOf(0) + expect(await list({ state: VideoAbuseState.PENDING })).to.have.lengthOf(5) + }) + after(async function () { await cleanupTests(servers) }) -- cgit v1.2.3