From 1cd3facc3de899ac864e979cd6d6a704b712cce3 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 10 Oct 2018 11:46:50 +0200 Subject: [PATCH] Add ability to list all local videos Including private/unlisted for moderators/admins --- server/controllers/api/accounts.ts | 4 +- server/controllers/api/search.ts | 1 + server/controllers/api/video-channel.ts | 4 +- server/controllers/feeds.ts | 10 +- server/helpers/custom-validators/videos.ts | 9 +- server/helpers/express-utils.ts | 3 +- server/middlewares/validators/search.ts | 38 +---- .../middlewares/validators/videos/videos.ts | 54 +++++++- server/models/video/video.ts | 26 +++- server/tests/api/check-params/index.ts | 1 + .../tests/api/check-params/videos-filter.ts | 127 +++++++++++++++++ server/tests/api/videos/index.ts | 1 + server/tests/api/videos/videos-filter.ts | 130 ++++++++++++++++++ .../search/videos-search-query.model.ts | 3 + shared/models/users/user-right.enum.ts | 1 + shared/models/users/user-role.ts | 3 +- shared/models/videos/video-query.type.ts | 2 +- 17 files changed, 363 insertions(+), 54 deletions(-) create mode 100644 server/tests/api/check-params/videos-filter.ts create mode 100644 server/tests/api/videos/videos-filter.ts diff --git a/server/controllers/api/accounts.ts b/server/controllers/api/accounts.ts index b7691ccba..8e3f60010 100644 --- a/server/controllers/api/accounts.ts +++ b/server/controllers/api/accounts.ts @@ -86,9 +86,11 @@ async function listAccountVideos (req: express.Request, res: express.Response, n languageOneOf: req.query.languageOneOf, tagsOneOf: req.query.tagsOneOf, tagsAllOf: req.query.tagsAllOf, + filter: req.query.filter, nsfw: buildNSFWFilter(res, req.query.nsfw), withFiles: false, - accountId: account.id + accountId: account.id, + userId: res.locals.oauth ? res.locals.oauth.token.User.id : undefined }) return res.json(getFormattedObjects(resultList.data, resultList.total)) diff --git a/server/controllers/api/search.ts b/server/controllers/api/search.ts index 4be2b5ef7..a8a6cfb08 100644 --- a/server/controllers/api/search.ts +++ b/server/controllers/api/search.ts @@ -118,6 +118,7 @@ async function searchVideosDB (query: VideosSearchQuery, res: express.Response) const options = Object.assign(query, { includeLocalVideos: true, nsfw: buildNSFWFilter(res, query.nsfw), + filter: query.filter, userId: res.locals.oauth ? res.locals.oauth.token.User.id : undefined }) const resultList = await VideoModel.searchAndPopulateAccountAndServer(options) diff --git a/server/controllers/api/video-channel.ts b/server/controllers/api/video-channel.ts index 1fa842d9c..c84d1be58 100644 --- a/server/controllers/api/video-channel.ts +++ b/server/controllers/api/video-channel.ts @@ -215,9 +215,11 @@ async function listVideoChannelVideos (req: express.Request, res: express.Respon languageOneOf: req.query.languageOneOf, tagsOneOf: req.query.tagsOneOf, tagsAllOf: req.query.tagsAllOf, + filter: req.query.filter, nsfw: buildNSFWFilter(res, req.query.nsfw), withFiles: false, - videoChannelId: videoChannelInstance.id + videoChannelId: videoChannelInstance.id, + userId: res.locals.oauth ? res.locals.oauth.token.User.id : undefined }) return res.json(getFormattedObjects(resultList.data, resultList.total)) diff --git a/server/controllers/feeds.ts b/server/controllers/feeds.ts index b30ad8e8d..ccb9b6029 100644 --- a/server/controllers/feeds.ts +++ b/server/controllers/feeds.ts @@ -1,7 +1,14 @@ import * as express from 'express' import { CONFIG, FEEDS, ROUTE_CACHE_LIFETIME } from '../initializers/constants' import { THUMBNAILS_SIZE } from '../initializers' -import { asyncMiddleware, setDefaultSort, videoCommentsFeedsValidator, videoFeedsValidator, videosSortValidator } from '../middlewares' +import { + asyncMiddleware, + commonVideosFiltersValidator, + setDefaultSort, + videoCommentsFeedsValidator, + videoFeedsValidator, + videosSortValidator +} from '../middlewares' import { VideoModel } from '../models/video/video' import * as Feed from 'pfeed' import { AccountModel } from '../models/account/account' @@ -22,6 +29,7 @@ feedsRouter.get('/feeds/videos.:format', videosSortValidator, setDefaultSort, asyncMiddleware(cacheRoute(ROUTE_CACHE_LIFETIME.FEEDS)), + commonVideosFiltersValidator, asyncMiddleware(videoFeedsValidator), asyncMiddleware(generateVideoFeed) ) diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts index 714f7ac95..a13b09ac8 100644 --- a/server/helpers/custom-validators/videos.ts +++ b/server/helpers/custom-validators/videos.ts @@ -3,7 +3,7 @@ import 'express-validator' import { values } from 'lodash' import 'multer' import * as validator from 'validator' -import { UserRight, VideoPrivacy, VideoRateType } from '../../../shared' +import { UserRight, VideoFilter, VideoPrivacy, VideoRateType } from '../../../shared' import { CONSTRAINTS_FIELDS, VIDEO_CATEGORIES, @@ -22,6 +22,10 @@ import { fetchVideo, VideoFetchType } from '../video' const VIDEOS_CONSTRAINTS_FIELDS = CONSTRAINTS_FIELDS.VIDEOS +function isVideoFilterValid (filter: VideoFilter) { + return filter === 'local' || filter === 'all-local' +} + function isVideoCategoryValid (value: any) { return value === null || VIDEO_CATEGORIES[ value ] !== undefined } @@ -225,5 +229,6 @@ export { isVideoExist, isVideoImage, isVideoChannelOfAccountExist, - isVideoSupportValid + isVideoSupportValid, + isVideoFilterValid } diff --git a/server/helpers/express-utils.ts b/server/helpers/express-utils.ts index 8a9cee8c5..162fe2244 100644 --- a/server/helpers/express-utils.ts +++ b/server/helpers/express-utils.ts @@ -2,7 +2,6 @@ import * as express from 'express' import * as multer from 'multer' import { CONFIG, REMOTE_SCHEME } from '../initializers' import { logger } from './logger' -import { User } from '../../shared/models/users' import { deleteFileAsync, generateRandomString } from './utils' import { extname } from 'path' import { isArray } from './custom-validators/misc' @@ -101,7 +100,7 @@ function createReqFiles ( } function isUserAbleToSearchRemoteURI (res: express.Response) { - const user: User = res.locals.oauth ? res.locals.oauth.token.User : undefined + const user: UserModel = res.locals.oauth ? res.locals.oauth.token.User : undefined return CONFIG.SEARCH.REMOTE_URI.ANONYMOUS === true || (CONFIG.SEARCH.REMOTE_URI.USERS === true && user !== undefined) diff --git a/server/middlewares/validators/search.ts b/server/middlewares/validators/search.ts index 8baf643a5..6a95d6095 100644 --- a/server/middlewares/validators/search.ts +++ b/server/middlewares/validators/search.ts @@ -2,8 +2,7 @@ import * as express from 'express' import { areValidationErrors } from './utils' import { logger } from '../../helpers/logger' import { query } from 'express-validator/check' -import { isNumberArray, isStringArray, isNSFWQueryValid } from '../../helpers/custom-validators/search' -import { isBooleanValid, isDateValid, toArray } from '../../helpers/custom-validators/misc' +import { isDateValid } from '../../helpers/custom-validators/misc' const videosSearchValidator = [ query('search').optional().not().isEmpty().withMessage('Should have a valid search'), @@ -35,44 +34,9 @@ const videoChannelsSearchValidator = [ } ] -const commonVideosFiltersValidator = [ - query('categoryOneOf') - .optional() - .customSanitizer(toArray) - .custom(isNumberArray).withMessage('Should have a valid one of category array'), - query('licenceOneOf') - .optional() - .customSanitizer(toArray) - .custom(isNumberArray).withMessage('Should have a valid one of licence array'), - query('languageOneOf') - .optional() - .customSanitizer(toArray) - .custom(isStringArray).withMessage('Should have a valid one of language array'), - query('tagsOneOf') - .optional() - .customSanitizer(toArray) - .custom(isStringArray).withMessage('Should have a valid one of tags array'), - query('tagsAllOf') - .optional() - .customSanitizer(toArray) - .custom(isStringArray).withMessage('Should have a valid all of tags array'), - query('nsfw') - .optional() - .custom(isNSFWQueryValid).withMessage('Should have a valid NSFW attribute'), - - (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking commons video filters query', { parameters: req.query }) - - if (areValidationErrors(req, res)) return - - return next() - } -] - // --------------------------------------------------------------------------- export { - commonVideosFiltersValidator, videoChannelsSearchValidator, videosSearchValidator } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index 1d0a64bb1..9dc52a134 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -1,6 +1,6 @@ import * as express from 'express' import 'express-validator' -import { body, param, ValidationChain } from 'express-validator/check' +import { body, param, query, ValidationChain } from 'express-validator/check' import { UserRight, VideoChangeOwnershipStatus, VideoPrivacy } from '../../../../shared' import { isBooleanValid, @@ -8,6 +8,7 @@ import { isIdOrUUIDValid, isIdValid, isUUIDValid, + toArray, toIntOrNull, toValueOrNull } from '../../../helpers/custom-validators/misc' @@ -19,6 +20,7 @@ import { isVideoDescriptionValid, isVideoExist, isVideoFile, + isVideoFilterValid, isVideoImage, isVideoLanguageValid, isVideoLicenceValid, @@ -42,6 +44,7 @@ import { VideoChangeOwnershipAccept } from '../../../../shared/models/videos/vid import { VideoChangeOwnershipModel } from '../../../models/video/video-change-ownership' import { AccountModel } from '../../../models/account/account' import { VideoFetchType } from '../../../helpers/video' +import { isNSFWQueryValid, isNumberArray, isStringArray } from '../../../helpers/custom-validators/search' const videosAddValidator = getCommonVideoAttributes().concat([ body('videofile') @@ -359,6 +362,51 @@ function getCommonVideoAttributes () { ] as (ValidationChain | express.Handler)[] } +const commonVideosFiltersValidator = [ + query('categoryOneOf') + .optional() + .customSanitizer(toArray) + .custom(isNumberArray).withMessage('Should have a valid one of category array'), + query('licenceOneOf') + .optional() + .customSanitizer(toArray) + .custom(isNumberArray).withMessage('Should have a valid one of licence array'), + query('languageOneOf') + .optional() + .customSanitizer(toArray) + .custom(isStringArray).withMessage('Should have a valid one of language array'), + query('tagsOneOf') + .optional() + .customSanitizer(toArray) + .custom(isStringArray).withMessage('Should have a valid one of tags array'), + query('tagsAllOf') + .optional() + .customSanitizer(toArray) + .custom(isStringArray).withMessage('Should have a valid all of tags array'), + query('nsfw') + .optional() + .custom(isNSFWQueryValid).withMessage('Should have a valid NSFW attribute'), + query('filter') + .optional() + .custom(isVideoFilterValid).withMessage('Should have a valid filter attribute'), + + (req: express.Request, res: express.Response, next: express.NextFunction) => { + logger.debug('Checking commons video filters query', { parameters: req.query }) + + if (areValidationErrors(req, res)) return + + const user: UserModel = res.locals.oauth ? res.locals.oauth.token.User : undefined + if (req.query.filter === 'all-local' && (!user || user.hasRight(UserRight.SEE_ALL_VIDEOS) === false)) { + res.status(401) + .json({ error: 'You are not allowed to see all local videos.' }) + + return + } + + return next() + } +] + // --------------------------------------------------------------------------- export { @@ -375,7 +423,9 @@ export { videosTerminateChangeOwnershipValidator, videosAcceptChangeOwnershipValidator, - getCommonVideoAttributes + getCommonVideoAttributes, + + commonVideosFiltersValidator } // --------------------------------------------------------------------------- diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 070ac7623..4f3f75613 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -235,7 +235,14 @@ type AvailableForListIDsOptions = { ) } ] - }, + } + }, + include: [] + } + + // Only list public/published videos + if (!options.filter || options.filter !== 'all-local') { + const privacyWhere = { // Always list public videos privacy: VideoPrivacy.PUBLIC, // Always list published videos, or videos that are being transcoded but on which we don't want to wait for transcoding @@ -250,8 +257,9 @@ type AvailableForListIDsOptions = { } } ] - }, - include: [] + } + + Object.assign(query.where, privacyWhere) } if (options.filter || options.accountId || options.videoChannelId) { @@ -969,6 +977,10 @@ export class VideoModel extends Model { trendingDays?: number, userId?: number }, countVideos = true) { + if (options.filter && options.filter === 'all-local' && !options.userId) { + throw new Error('Try to filter all-local but no userId is provided') + } + const query: IFindOptions = { offset: options.start, limit: options.count, @@ -1021,7 +1033,8 @@ export class VideoModel extends Model { tagsAllOf?: string[] durationMin?: number // seconds durationMax?: number // seconds - userId?: number + userId?: number, + filter?: VideoFilter }) { const whereAnd = [] @@ -1098,7 +1111,8 @@ export class VideoModel extends Model { languageOneOf: options.languageOneOf, tagsOneOf: options.tagsOneOf, tagsAllOf: options.tagsAllOf, - userId: options.userId + userId: options.userId, + filter: options.filter } return VideoModel.getAvailableForApi(query, queryOptions) @@ -1262,7 +1276,7 @@ export class VideoModel extends Model { } private static buildActorWhereWithFilter (filter?: VideoFilter) { - if (filter && filter === 'local') { + if (filter && (filter === 'local' || filter === 'all-local')) { return { serverId: null } diff --git a/server/tests/api/check-params/index.ts b/server/tests/api/check-params/index.ts index 71a217649..bfc550ae5 100644 --- a/server/tests/api/check-params/index.ts +++ b/server/tests/api/check-params/index.ts @@ -15,4 +15,5 @@ import './video-channels' import './video-comments' import './video-imports' import './videos' +import './videos-filter' import './videos-history' diff --git a/server/tests/api/check-params/videos-filter.ts b/server/tests/api/check-params/videos-filter.ts new file mode 100644 index 000000000..784cd8ba1 --- /dev/null +++ b/server/tests/api/check-params/videos-filter.ts @@ -0,0 +1,127 @@ +/* tslint:disable:no-unused-expression */ + +import * as chai from 'chai' +import 'mocha' +import { + createUser, + flushTests, + killallServers, + makeGetRequest, + runServer, + ServerInfo, + setAccessTokensToServers, + userLogin +} from '../../utils' +import { UserRole } from '../../../../shared/models/users' + +const expect = chai.expect + +async function testEndpoints (server: ServerInfo, token: string, filter: string, statusCodeExpected: number) { + const paths = [ + '/api/v1/video-channels/root_channel/videos', + '/api/v1/accounts/root/videos', + '/api/v1/videos', + '/api/v1/search/videos' + ] + + for (const path of paths) { + await makeGetRequest({ + url: server.url, + path, + token, + query: { + filter + }, + statusCodeExpected + }) + } +} + +describe('Test videos filters', function () { + let server: ServerInfo + let userAccessToken: string + let moderatorAccessToken: string + + // --------------------------------------------------------------- + + before(async function () { + this.timeout(30000) + + await flushTests() + + server = await runServer(1) + + await setAccessTokensToServers([ server ]) + + const user = { username: 'user1', password: 'my super password' } + await createUser(server.url, server.accessToken, user.username, user.password) + userAccessToken = await userLogin(server, user) + + const moderator = { username: 'moderator', password: 'my super password' } + await createUser( + server.url, + server.accessToken, + moderator.username, + moderator.password, + undefined, + undefined, + UserRole.MODERATOR + ) + moderatorAccessToken = await userLogin(server, moderator) + }) + + describe('When setting a video filter', function () { + + it('Should fail with a bad filter', async function () { + await testEndpoints(server, server.accessToken, 'bad-filter', 400) + }) + + it('Should succeed with a good filter', async function () { + await testEndpoints(server, server.accessToken,'local', 200) + }) + + it('Should fail to list all-local with a simple user', async function () { + await testEndpoints(server, userAccessToken, 'all-local', 401) + }) + + it('Should succeed to list all-local with a moderator', async function () { + await testEndpoints(server, moderatorAccessToken, 'all-local', 200) + }) + + it('Should succeed to list all-local with an admin', async function () { + await testEndpoints(server, server.accessToken, 'all-local', 200) + }) + + // Because we cannot authenticate the user on the RSS endpoint + it('Should fail on the feeds endpoint with the all-local filter', async function () { + await makeGetRequest({ + url: server.url, + path: '/feeds/videos.json', + statusCodeExpected: 401, + query: { + filter: 'all-local' + } + }) + }) + + it('Should succed on the feeds endpoint with the local filter', async function () { + await makeGetRequest({ + url: server.url, + path: '/feeds/videos.json', + statusCodeExpected: 200, + query: { + filter: 'local' + } + }) + }) + }) + + after(async function () { + killallServers([ server ]) + + // Keep the logs if the test failed + if (this['ok']) { + await flushTests() + } + }) +}) diff --git a/server/tests/api/videos/index.ts b/server/tests/api/videos/index.ts index 09bb62a8d..9bdb78491 100644 --- a/server/tests/api/videos/index.ts +++ b/server/tests/api/videos/index.ts @@ -14,5 +14,6 @@ import './video-nsfw' import './video-privacy' import './video-schedule-update' import './video-transcoder' +import './videos-filter' import './videos-history' import './videos-overview' diff --git a/server/tests/api/videos/videos-filter.ts b/server/tests/api/videos/videos-filter.ts new file mode 100644 index 000000000..a7588129f --- /dev/null +++ b/server/tests/api/videos/videos-filter.ts @@ -0,0 +1,130 @@ +/* tslint:disable:no-unused-expression */ + +import * as chai from 'chai' +import 'mocha' +import { + createUser, + doubleFollow, + flushAndRunMultipleServers, + flushTests, + killallServers, + makeGetRequest, + ServerInfo, + setAccessTokensToServers, + uploadVideo, + userLogin +} from '../../utils' +import { Video, VideoPrivacy } from '../../../../shared/models/videos' +import { UserRole } from '../../../../shared/models/users' + +const expect = chai.expect + +async function getVideosNames (server: ServerInfo, token: string, filter: string, statusCodeExpected = 200) { + const paths = [ + '/api/v1/video-channels/root_channel/videos', + '/api/v1/accounts/root/videos', + '/api/v1/videos', + '/api/v1/search/videos' + ] + + const videosResults: Video[][] = [] + + for (const path of paths) { + const res = await makeGetRequest({ + url: server.url, + path, + token, + query: { + sort: 'createdAt', + filter + }, + statusCodeExpected + }) + + videosResults.push(res.body.data.map(v => v.name)) + } + + return videosResults +} + +describe('Test videos filter validator', function () { + let servers: ServerInfo[] + + // --------------------------------------------------------------- + + before(async function () { + this.timeout(120000) + + await flushTests() + + servers = await flushAndRunMultipleServers(2) + + await setAccessTokensToServers(servers) + + for (const server of servers) { + const moderator = { username: 'moderator', password: 'my super password' } + await createUser( + server.url, + server.accessToken, + moderator.username, + moderator.password, + undefined, + undefined, + UserRole.MODERATOR + ) + server['moderatorAccessToken'] = await userLogin(server, moderator) + + await uploadVideo(server.url, server.accessToken, { name: 'public ' + server.serverNumber }) + + { + const attributes = { name: 'unlisted ' + server.serverNumber, privacy: VideoPrivacy.UNLISTED } + await uploadVideo(server.url, server.accessToken, attributes) + } + + { + const attributes = { name: 'private ' + server.serverNumber, privacy: VideoPrivacy.PRIVATE } + await uploadVideo(server.url, server.accessToken, attributes) + } + } + + await doubleFollow(servers[0], servers[1]) + }) + + describe('Check videos filter', function () { + + it('Should display local videos', async function () { + for (const server of servers) { + const namesResults = await getVideosNames(server, server.accessToken, 'local') + for (const names of namesResults) { + expect(names).to.have.lengthOf(1) + expect(names[ 0 ]).to.equal('public ' + server.serverNumber) + } + } + }) + + it('Should display all local videos by the admin or the moderator', async function () { + for (const server of servers) { + for (const token of [ server.accessToken, server['moderatorAccessToken'] ]) { + + const namesResults = await getVideosNames(server, token, 'all-local') + for (const names of namesResults) { + expect(names).to.have.lengthOf(3) + + expect(names[ 0 ]).to.equal('public ' + server.serverNumber) + expect(names[ 1 ]).to.equal('unlisted ' + server.serverNumber) + expect(names[ 2 ]).to.equal('private ' + server.serverNumber) + } + } + } + }) + }) + + after(async function () { + killallServers(servers) + + // Keep the logs if the test failed + if (this['ok']) { + await flushTests() + } + }) +}) diff --git a/shared/models/search/videos-search-query.model.ts b/shared/models/search/videos-search-query.model.ts index 29aa5c100..0db220758 100644 --- a/shared/models/search/videos-search-query.model.ts +++ b/shared/models/search/videos-search-query.model.ts @@ -1,4 +1,5 @@ import { NSFWQuery } from './nsfw-query.model' +import { VideoFilter } from '../videos' export interface VideosSearchQuery { search?: string @@ -23,4 +24,6 @@ export interface VideosSearchQuery { durationMin?: number // seconds durationMax?: number // seconds + + filter?: VideoFilter } diff --git a/shared/models/users/user-right.enum.ts b/shared/models/users/user-right.enum.ts index c4ccd632f..ed2c536ce 100644 --- a/shared/models/users/user-right.enum.ts +++ b/shared/models/users/user-right.enum.ts @@ -14,5 +14,6 @@ export enum UserRight { REMOVE_ANY_VIDEO_CHANNEL, REMOVE_ANY_VIDEO_COMMENT, UPDATE_ANY_VIDEO, + SEE_ALL_VIDEOS, CHANGE_VIDEO_OWNERSHIP } diff --git a/shared/models/users/user-role.ts b/shared/models/users/user-role.ts index 552aad999..d7020c0f2 100644 --- a/shared/models/users/user-role.ts +++ b/shared/models/users/user-role.ts @@ -26,7 +26,8 @@ const userRoleRights: { [ id: number ]: UserRight[] } = { UserRight.REMOVE_ANY_VIDEO, UserRight.REMOVE_ANY_VIDEO_CHANNEL, UserRight.REMOVE_ANY_VIDEO_COMMENT, - UserRight.UPDATE_ANY_VIDEO + UserRight.UPDATE_ANY_VIDEO, + UserRight.SEE_ALL_VIDEOS ], [UserRole.USER]: [] diff --git a/shared/models/videos/video-query.type.ts b/shared/models/videos/video-query.type.ts index ff0f527f3..f76a91aad 100644 --- a/shared/models/videos/video-query.type.ts +++ b/shared/models/videos/video-query.type.ts @@ -1 +1 @@ -export type VideoFilter = 'local' +export type VideoFilter = 'local' | 'all-local'