From 198b205c10dba362b9ae1ef6895b29d7e0dd685f Mon Sep 17 00:00:00 2001 From: Green-Star Date: Wed, 26 Apr 2017 21:22:10 +0200 Subject: [PATCH] Add ability for an administrator to remove any video (#61) * Add ability for an admin to remove every video on the pod. * Server: add BlacklistedVideos relation. * Server: Insert in BlacklistedVideos relation upon deletion of a video. * Server: Modify BlacklistedVideos schema to add Pod id information. * Server: Moving insertion of a blacklisted video from the `afterDestroy` hook into the process of deletion of a video. To avoid inserting a video when it is removed on its origin pod. When a video is removed on its origin pod, the `afterDestroy` hook is fire, but no request is made on the delete('/:videoId') interface. Hence, we insert into `BlacklistedVideos` only on request on delete('/:videoId') (if requirements for insertion are met). * Server: Add removeVideoFromBlacklist hook on deletion of a video. We are going to proceed in another way :). We will add a new route : /:videoId/blacklist to blacklist a video. We do not blacklist a video upon its deletion now (to distinguish a video blacklist from a regular video delete) When we blacklist a video, the video remains in the DB, so we don't have any concern about its update. It just doesn't appear in the video list. When we remove a video, we then have to remove it from the blacklist too. We could also remove a video from the blacklist to 'unremove' it and make it appear again in the video list (will be another feature). * Server: Add handler for new route post(/:videoId/blacklist) * Client: Add isBlacklistable method * Client: Update isRemovableBy method. * Client: Move 'Delete video' feature from the video-list to the video-watch module. * Server: Exclude blacklisted videos from the video list * Server: Use findAll() in BlacklistedVideos.list() method * Server: Fix addVideoToBlacklist function. * Client: Add blacklist feature. * Server: Use JavaScript Standard Style. * Server: In checkUserCanDeleteVideo, move the callback call inside the db callback function * Server: Modify BlacklistVideo relation * Server: Modifiy Videos methods. * Server: Add checkVideoIsBlacklistable method * Server: Rewrite addVideoToBlacklist method * Server: Fix checkVideoIsBlacklistable method * Server: Add return to addVideoToBlacklist method --- client/src/app/videos/shared/video.model.ts | 8 +- client/src/app/videos/shared/video.service.ts | 6 ++ .../videos/video-list/video-list.component.ts | 5 -- .../video-list/video-miniature.component.html | 4 - .../video-list/video-miniature.component.scss | 14 --- .../video-list/video-miniature.component.ts | 20 ----- .../video-watch/video-watch.component.html | 12 +++ .../video-watch/video-watch.component.ts | 47 ++++++++++ server/controllers/api/videos.js | 25 ++++++ server/middlewares/validators/videos.js | 63 ++++++++++--- server/models/user.js | 8 +- server/models/video-blacklist.js | 89 +++++++++++++++++++ server/models/video.js | 48 +++++++--- 13 files changed, 283 insertions(+), 66 deletions(-) create mode 100644 server/models/video-blacklist.js diff --git a/client/src/app/videos/shared/video.model.ts b/client/src/app/videos/shared/video.model.ts index 404e3bf45..1cfb312b6 100644 --- a/client/src/app/videos/shared/video.model.ts +++ b/client/src/app/videos/shared/video.model.ts @@ -85,8 +85,12 @@ export class Video { this.by = Video.createByString(hash.author, hash.podHost); } - isRemovableBy(user: User) { - return this.isLocal === true && user && this.author === user.username; + isRemovableBy(user) { + return user && this.isLocal === true && (this.author === user.username || user.isAdmin() === true); + } + + isBlackistableBy(user) { + return user && user.isAdmin() === true && this.isLocal === false; } isVideoNSFWForUser(user: User) { diff --git a/client/src/app/videos/shared/video.service.ts b/client/src/app/videos/shared/video.service.ts index ee67bc1ae..a0965e20c 100644 --- a/client/src/app/videos/shared/video.service.ts +++ b/client/src/app/videos/shared/video.service.ts @@ -150,6 +150,12 @@ export class VideoService { .catch((res) => this.restExtractor.handleError(res)); } + blacklistVideo(id: string) { + return this.authHttp.post(VideoService.BASE_VIDEO_URL + id + '/blacklist', {}) + .map(this.restExtractor.extractDataBool) + .catch((res) => this.restExtractor.handleError(res)); + } + private setVideoRate(id: string, rateType: RateType) { const url = VideoService.BASE_VIDEO_URL + id + '/rate'; const body = { diff --git a/client/src/app/videos/video-list/video-list.component.ts b/client/src/app/videos/video-list/video-list.component.ts index ede1b51a9..b9f19b4f1 100644 --- a/client/src/app/videos/video-list/video-list.component.ts +++ b/client/src/app/videos/video-list/video-list.component.ts @@ -108,11 +108,6 @@ export class VideoListComponent implements OnInit, OnDestroy { this.navigateToNewParams(); } - onRemoved(video: Video) { - this.notificationsService.success('Success', `Video ${video.name} deleted.`); - this.getVideos(); - } - onSort(sort: SortField) { this.sort = sort; diff --git a/client/src/app/videos/video-list/video-miniature.component.html b/client/src/app/videos/video-list/video-miniature.component.html index 94b892698..b8b448631 100644 --- a/client/src/app/videos/video-list/video-miniature.component.html +++ b/client/src/app/videos/video-list/video-miniature.component.html @@ -10,10 +10,6 @@ {{ video.duration }} -
diff --git a/client/src/app/videos/video-list/video-miniature.component.scss b/client/src/app/videos/video-list/video-miniature.component.scss index b8e90e8c5..f88ced819 100644 --- a/client/src/app/videos/video-list/video-miniature.component.scss +++ b/client/src/app/videos/video-list/video-miniature.component.scss @@ -42,20 +42,6 @@ } } - .video-miniature-remove { - display: inline-block; - position: absolute; - left: 16px; - background-color: rgba(0, 0, 0, 0.8); - color: rgba(255, 255, 255, 0.8); - padding: 2px; - cursor: pointer; - - &:hover { - color: rgba(255, 255, 255, 0.9); - } - } - .video-miniature-informations { width: 200px; diff --git a/client/src/app/videos/video-list/video-miniature.component.ts b/client/src/app/videos/video-list/video-miniature.component.ts index 888026dde..13deec381 100644 --- a/client/src/app/videos/video-list/video-miniature.component.ts +++ b/client/src/app/videos/video-list/video-miniature.component.ts @@ -13,8 +13,6 @@ import { User } from '../../shared'; }) export class VideoMiniatureComponent { - @Output() removed = new EventEmitter(); - @Input() currentSort: SortField; @Input() user: User; @Input() video: Video; @@ -28,10 +26,6 @@ export class VideoMiniatureComponent { private videoService: VideoService ) {} - displayRemoveIcon() { - return this.hovering && this.video.isRemovableBy(this.user); - } - getVideoName() { if (this.isVideoNSFWForThisUser()) return 'NSFW'; @@ -47,20 +41,6 @@ export class VideoMiniatureComponent { this.hovering = true; } - removeVideo(id: string) { - this.confirmService.confirm('Do you really want to delete this video?', 'Delete').subscribe( - res => { - if (res === false) return; - - this.videoService.removeVideo(id).subscribe( - status => this.removed.emit(true), - - error => this.notificationsService.error('Error', error.text) - ); - } - ); - } - isVideoNSFWForThisUser() { return this.video.isVideoNSFWForUser(this.user); } diff --git a/client/src/app/videos/video-watch/video-watch.component.html b/client/src/app/videos/video-watch/video-watch.component.html index 19e9bd9ed..ed26b513e 100644 --- a/client/src/app/videos/video-watch/video-watch.component.html +++ b/client/src/app/videos/video-watch/video-watch.component.html @@ -96,6 +96,18 @@ Report + +
  • + + Delete + +
    • + +
  • + + Blacklist + +
    • diff --git a/client/src/app/videos/video-watch/video-watch.component.ts b/client/src/app/videos/video-watch/video-watch.component.ts index e04626a67..f582df45c 100644 --- a/client/src/app/videos/video-watch/video-watch.component.ts +++ b/client/src/app/videos/video-watch/video-watch.component.ts @@ -169,6 +169,45 @@ export class VideoWatchComponent implements OnInit, OnDestroy { ); } + removeVideo(event: Event) { + event.preventDefault(); + this.confirmService.confirm('Do you really want to delete this video?', 'Delete').subscribe( + res => { + if (res === false) return; + + this.videoService.removeVideo(this.video.id) + .subscribe( + status => { + this.notificationsService.success('Success', `Video ${this.video.name} deleted.`) + // Go back to the video-list. + this.router.navigate(['/videos/list']) + }, + + error => this.notificationsService.error('Error', error.text) + ); + } + ); + } + + blacklistVideo(event: Event) { + event.preventDefault() + this.confirmService.confirm('Do you really want to blacklist this video ?', 'Blacklist').subscribe( + res => { + if (res === false) return; + + this.videoService.blacklistVideo(this.video.id) + .subscribe( + status => { + this.notificationsService.success('Success', `Video ${this.video.name} had been blacklisted.`) + this.router.navigate(['/videos/list']) + }, + + error => this.notificationsService.error('Error', error.text) + ) + } + ) + } + showReportModal(event: Event) { event.preventDefault(); this.videoReportModal.show(); @@ -192,6 +231,14 @@ export class VideoWatchComponent implements OnInit, OnDestroy { this.authService.getUser().username === this.video.author; } + isVideoRemovable() { + return this.video.isRemovableBy(this.authService.getUser()); + } + + isVideoBlacklistable() { + return this.video.isBlackistableBy(this.authService.getUser()); + } + private checkUserRating() { // Unlogged users do not have ratings if (this.isUserLoggedIn() === false) return; diff --git a/server/controllers/api/videos.js b/server/controllers/api/videos.js index 5e9ff482f..1f7d30eef 100644 --- a/server/controllers/api/videos.js +++ b/server/controllers/api/videos.js @@ -93,11 +93,13 @@ router.get('/:id', validatorsVideos.videosGet, getVideo ) + router.delete('/:id', oAuth.authenticate, validatorsVideos.videosRemove, removeVideo ) + router.get('/search/:value', validatorsVideos.videosSearch, validatorsPagination.pagination, @@ -108,6 +110,13 @@ router.get('/search/:value', searchVideos ) +router.post('/:id/blacklist', + oAuth.authenticate, + admin.ensureIsAdmin, + validatorsVideos.videosBlacklist, + addVideoToBlacklist +) + // --------------------------------------------------------------------------- module.exports = router @@ -622,3 +631,19 @@ function reportVideoAbuse (req, res, finalCallback) { return finalCallback(null) }) } + +function addVideoToBlacklist (req, res, next) { + const videoInstance = res.locals.video + + db.BlacklistedVideo.create({ + videoId: videoInstance.id + }) + .asCallback(function (err) { + if (err) { + logger.error('Errors when blacklisting video ', { error: err }) + return next(err) + } + + return res.type('json').status(204).end() + }) +} diff --git a/server/middlewares/validators/videos.js b/server/middlewares/validators/videos.js index c07825e50..86a7e39ae 100644 --- a/server/middlewares/validators/videos.js +++ b/server/middlewares/validators/videos.js @@ -15,7 +15,9 @@ const validatorsVideos = { videoAbuseReport, - videoRate + videoRate, + + videosBlacklist } function videosAdd (req, res, next) { @@ -95,15 +97,10 @@ function videosRemove (req, res, next) { checkVideoExists(req.params.id, res, function () { // We need to make additional checks - if (res.locals.video.isOwned() === false) { - return res.status(403).send('Cannot remove video of another pod') - } - - if (res.locals.video.Author.userId !== res.locals.oauth.token.User.id) { - return res.status(403).send('Cannot remove video of another user') - } - - next() + // Check if the user who did the request is able to delete the video + checkUserCanDeleteVideo(res.locals.oauth.token.User.id, res, function () { + next() + }) }) }) } @@ -159,3 +156,49 @@ function checkVideoExists (id, res, callback) { callback() }) } + +function checkUserCanDeleteVideo (userId, res, callback) { + // Retrieve the user who did the request + db.User.loadById(userId, function (err, user) { + if (err) { + logger.error('Error in video request validator.', { error: err }) + return res.sendStatus(500) + } + + // Check if the user can delete the video + // The user can delete it if s/he an admin + // Or if s/he is the video's author + if (user.isAdmin() === false) { + if (res.locals.video.isOwned() === false) { + return res.status(403).send('Cannot remove video of another pod') + } + + if (res.locals.video.Author.userId !== res.locals.oauth.token.User.id) { + return res.status(403).send('Cannot remove video of another user') + } + } + + // If we reach this comment, we can delete the video + callback() + }) +} + +function checkVideoIsBlacklistable (req, res, callback) { + if (res.locals.video.isOwned() === true) { + return res.status(403).send('Cannot blacklist a local video') + } + + callback() +} + +function videosBlacklist (req, res, next) { + req.checkParams('id', 'Should have a valid id').notEmpty().isUUID(4) + + logger.debug('Checking videosBlacklist parameters', { parameters: req.params }) + + checkErrors(req, res, function () { + checkVideoExists(req.params.id, res, function() { + checkVideoIsBlacklistable(req, res, next) + }) + }) +} diff --git a/server/models/user.js b/server/models/user.js index e64bab8ab..8f9c2bf65 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -79,7 +79,8 @@ module.exports = function (sequelize, DataTypes) { }, instanceMethods: { isPasswordMatch, - toFormatedJSON + toFormatedJSON, + isAdmin }, hooks: { beforeCreate: beforeCreateOrUpdate, @@ -117,6 +118,11 @@ function toFormatedJSON () { createdAt: this.createdAt } } + +function isAdmin () { + return this.role === constants.USER_ROLES.ADMIN +} + // ------------------------------ STATICS ------------------------------ function associate (models) { diff --git a/server/models/video-blacklist.js b/server/models/video-blacklist.js new file mode 100644 index 000000000..02ea15760 --- /dev/null +++ b/server/models/video-blacklist.js @@ -0,0 +1,89 @@ +'use strict' + +const modelUtils = require('./utils') + +// --------------------------------------------------------------------------- + +module.exports = function (sequelize, DataTypes) { + const BlacklistedVideo = sequelize.define('BlacklistedVideo', + {}, + { + indexes: [ + { + fields: [ 'videoId' ], + unique: true + } + ], + classMethods: { + associate, + + countTotal, + list, + listForApi, + loadById, + loadByVideoId + }, + instanceMethods: { + toFormatedJSON + }, + hooks: {} + } + ) + + return BlacklistedVideo +} + +// ------------------------------ METHODS ------------------------------ + +function toFormatedJSON () { + return { + id: this.id, + videoId: this.videoId, + createdAt: this.createdAt + } +} + +// ------------------------------ STATICS ------------------------------ + +function associate (models) { + this.belongsTo(models.Video, { + foreignKey: 'videoId', + onDelete: 'cascade' + }) +} + +function countTotal (callback) { + return this.count().asCallback(callback) +} + +function list (callback) { + return this.findAll().asCallback(callback) +} + +function listForApi (start, count, sort, callback) { + const query = { + offset: start, + limit: count, + order: [ modelUtils.getSort(sort) ] + } + + return this.findAndCountAll(query).asCallback(function (err, result) { + if (err) return callback(err) + + return callback(null, result.rows, result.count) + }) +} + +function loadById (id, callback) { + return this.findById(id).asCallback(callback) +} + +function loadByVideoId (id, callback) { + const query = { + where: { + videoId: id + } + } + + return this.find(query).asCallback(callback) +} diff --git a/server/models/video.js b/server/models/video.js index 39eb28ed9..1addfa682 100644 --- a/server/models/video.js +++ b/server/models/video.js @@ -16,6 +16,7 @@ const logger = require('../helpers/logger') const friends = require('../lib/friends') const modelUtils = require('./utils') const customVideosValidators = require('../helpers/custom-validators').videos +const db = require('../initializers/database') // --------------------------------------------------------------------------- @@ -201,7 +202,8 @@ module.exports = function (sequelize, DataTypes) { isOwned, toFormatedJSON, toAddRemoteJSON, - toUpdateRemoteJSON + toUpdateRemoteJSON, + removeFromBlacklist }, hooks: { beforeValidate, @@ -528,6 +530,7 @@ function list (callback) { } function listForApi (start, count, sort, callback) { + // Exclude Blakclisted videos from the list const query = { offset: start, limit: count, @@ -540,7 +543,12 @@ function listForApi (start, count, sort, callback) { }, this.sequelize.models.Tag - ] + ], + where: { + id: { $notIn: this.sequelize.literal( + '(SELECT "BlacklistedVideos"."videoId" FROM "BlacklistedVideos")' + )} + } } return this.findAndCountAll(query).asCallback(function (err, result) { @@ -648,7 +656,11 @@ function searchAndPopulateAuthorAndPodAndTags (value, field, start, count, sort, } const query = { - where: {}, + where: { + id: { $notIn: this.sequelize.literal( + '(SELECT "BlacklistedVideos"."videoId" FROM "BlacklistedVideos")' + )} + }, offset: start, limit: count, distinct: true, // For the count, a video can have many tags @@ -661,13 +673,9 @@ function searchAndPopulateAuthorAndPodAndTags (value, field, start, count, sort, query.where.infoHash = infoHash } else if (field === 'tags') { const escapedValue = this.sequelize.escape('%' + value + '%') - query.where = { - id: { - $in: this.sequelize.literal( - '(SELECT "VideoTags"."videoId" FROM "Tags" INNER JOIN "VideoTags" ON "Tags"."id" = "VideoTags"."tagId" WHERE name LIKE ' + escapedValue + ')' - ) - } - } + query.where.id.$in = this.sequelize.literal( + '(SELECT "VideoTags"."videoId" FROM "Tags" INNER JOIN "VideoTags" ON "Tags"."id" = "VideoTags"."tagId" WHERE name LIKE ' + escapedValue + ')' + ) } else if (field === 'host') { // FIXME: Include our pod? (not stored in the database) podInclude.where = { @@ -755,3 +763,23 @@ function generateImage (video, videoPath, folder, imageName, size, callback) { }) .thumbnail(options) } + +function removeFromBlacklist (video, callback) { + // Find the blacklisted video + db.BlacklistedVideo.loadByVideoId(video.id, function (err, video) { + // If an error occured, stop here + if (err) { + logger.error('Error when fetching video from blacklist.', { error: err }) + + return callback(err) + } + + // If we found the video, remove it from the blacklist + if (video) { + video.destroy().asCallback(callback) + } else { + // If haven't found it, simply ignore it and do nothing + return callback() + } + }) +} -- 2.25.1