From 696d83fd1377486dd03cc1bd02a21d9b6ddd9fcd Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 22 May 2020 17:06:26 +0200 Subject: [PATCH] Block comments from muted accounts/servers Add better control for users of comments displayed on their videos: * Do not forward comments from muted remote accounts/servers (muted by the current server or by the video owner) * Do not list threads and hide replies (with their children) of accounts/servers muted by the video owner * Hide from RSS comments of muted accounts/servers by video owners Use case: * Try to limit spam propagation in the federation * Add ability for users to automatically hide comments on their videos from undesirable accounts/servers (the comment section belongs to videomakers, so they choose what's posted there) --- server/controllers/activitypub/client.ts | 2 +- server/controllers/api/videos/comment.ts | 2 + .../lib/activitypub/process/process-create.ts | 23 ++- server/lib/blocklist.ts | 25 ++- server/lib/notifier.ts | 55 +++--- server/models/account/account.ts | 26 ++- server/models/utils.ts | 5 +- server/models/video/video-abuse.ts | 2 +- server/models/video/video-comment.ts | 76 +++++--- server/tests/api/users/blocklist.ts | 170 +++++++++++++++--- server/tests/api/videos/multiple-servers.ts | 9 +- server/tests/feeds/feeds.ts | 41 ++++- shared/extra-utils/videos/video-comments.ts | 7 + shared/extra-utils/videos/videos.ts | 9 +- 14 files changed, 350 insertions(+), 102 deletions(-) diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index f94abf808..e48641836 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -285,7 +285,7 @@ async function videoCommentsController (req: express.Request, res: express.Respo const video = res.locals.onlyImmutableVideo const handler = async (start: number, count: number) => { - const result = await VideoCommentModel.listAndCountByVideoId(video.id, start, count) + const result = await VideoCommentModel.listAndCountByVideoForAP(video, start, count) return { total: result.count, data: result.rows.map(r => r.url) diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index 2dcb85ecf..45ff969d9 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -78,6 +78,7 @@ async function listVideoThreads (req: express.Request, res: express.Response) { if (video.commentsEnabled === true) { const apiOptions = await Hooks.wrapObject({ videoId: video.id, + isVideoOwned: video.isOwned(), start: req.query.start, count: req.query.count, sort: req.query.sort, @@ -108,6 +109,7 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo if (video.commentsEnabled === true) { const apiOptions = await Hooks.wrapObject({ videoId: video.id, + isVideoOwned: video.isOwned(), threadId: res.locals.videoCommentThread.id, user }, 'filter:api.video-thread-comments.list.params') diff --git a/server/lib/activitypub/process/process-create.ts b/server/lib/activitypub/process/process-create.ts index 566bf6992..f8f9b80c6 100644 --- a/server/lib/activitypub/process/process-create.ts +++ b/server/lib/activitypub/process/process-create.ts @@ -1,18 +1,19 @@ +import { isRedundancyAccepted } from '@server/lib/redundancy' import { ActivityCreate, CacheFileObject, VideoTorrentObject } from '../../../../shared' +import { PlaylistObject } from '../../../../shared/models/activitypub/objects/playlist-object' import { VideoCommentObject } from '../../../../shared/models/activitypub/objects/video-comment-object' import { retryTransactionWrapper } from '../../../helpers/database-utils' import { logger } from '../../../helpers/logger' import { sequelizeTypescript } from '../../../initializers/database' -import { resolveThread } from '../video-comments' -import { getOrCreateVideoAndAccountAndChannel } from '../videos' -import { forwardVideoRelatedActivity } from '../send/utils' -import { createOrUpdateCacheFile } from '../cache-file' -import { Notifier } from '../../notifier' -import { PlaylistObject } from '../../../../shared/models/activitypub/objects/playlist-object' -import { createOrUpdateVideoPlaylist } from '../playlist' import { APProcessorOptions } from '../../../typings/activitypub-processor.model' import { MActorSignature, MCommentOwnerVideo, MVideoAccountLightBlacklistAllFiles } from '../../../typings/models' -import { isRedundancyAccepted } from '@server/lib/redundancy' +import { Notifier } from '../../notifier' +import { createOrUpdateCacheFile } from '../cache-file' +import { createOrUpdateVideoPlaylist } from '../playlist' +import { forwardVideoRelatedActivity } from '../send/utils' +import { resolveThread } from '../video-comments' +import { getOrCreateVideoAndAccountAndChannel } from '../videos' +import { isBlockedByServerOrAccount } from '@server/lib/blocklist' async function processCreateActivity (options: APProcessorOptions) { const { activity, byActor } = options @@ -101,6 +102,12 @@ async function processCreateVideoComment (activity: ActivityCreate, byActor: MAc return } + // Try to not forward unwanted commments on our videos + if (video.isOwned() && await isBlockedByServerOrAccount(comment.Account, video.VideoChannel.Account)) { + logger.info('Skip comment forward from blocked account or server %s.', comment.Account.Actor.url) + return + } + if (video.isOwned() && created === true) { // Don't resend the activity to the sender const exceptions = [ byActor ] diff --git a/server/lib/blocklist.ts b/server/lib/blocklist.ts index 842eecb5b..d282d091b 100644 --- a/server/lib/blocklist.ts +++ b/server/lib/blocklist.ts @@ -1,5 +1,6 @@ import { sequelizeTypescript } from '@server/initializers/database' -import { MAccountBlocklist, MServerBlocklist } from '@server/typings/models' +import { getServerActor } from '@server/models/application/application' +import { MAccountBlocklist, MAccountId, MAccountServer, MServerBlocklist } from '@server/typings/models' import { AccountBlocklistModel } from '../models/account/account-blocklist' import { ServerBlocklistModel } from '../models/server/server-blocklist' @@ -33,9 +34,29 @@ function removeServerFromBlocklist (serverBlock: MServerBlocklist) { }) } +async function isBlockedByServerOrAccount (targetAccount: MAccountServer, userAccount?: MAccountId) { + const serverAccountId = (await getServerActor()).Account.id + const sourceAccounts = [ serverAccountId ] + + if (userAccount) sourceAccounts.push(userAccount.id) + + const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, targetAccount.id) + if (accountMutedHash[serverAccountId] || (userAccount && accountMutedHash[userAccount.id])) { + return true + } + + const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, targetAccount.Actor.serverId) + if (instanceMutedHash[serverAccountId] || (userAccount && instanceMutedHash[userAccount.id])) { + return true + } + + return false +} + export { addAccountInBlocklist, addServerInBlocklist, removeAccountFromBlocklist, - removeServerFromBlocklist + removeServerFromBlocklist, + isBlockedByServerOrAccount } diff --git a/server/lib/notifier.ts b/server/lib/notifier.ts index 017739523..89f91e031 100644 --- a/server/lib/notifier.ts +++ b/server/lib/notifier.ts @@ -1,12 +1,22 @@ +import { getServerActor } from '@server/models/application/application' +import { ServerBlocklistModel } from '@server/models/server/server-blocklist' +import { + MUser, + MUserAccount, + MUserDefault, + MUserNotifSettingAccount, + MUserWithNotificationSetting, + UserNotificationModelForApi +} from '@server/typings/models/user' +import { MVideoImportVideo } from '@server/typings/models/video/video-import' import { UserNotificationSettingValue, UserNotificationType, UserRight } from '../../shared/models/users' +import { VideoAbuse, VideoPrivacy, VideoState } from '../../shared/models/videos' import { logger } from '../helpers/logger' -import { Emailer } from './emailer' -import { UserNotificationModel } from '../models/account/user-notification' -import { UserModel } from '../models/account/user' -import { PeerTubeSocket } from './peertube-socket' import { CONFIG } from '../initializers/config' -import { VideoPrivacy, VideoState, VideoAbuse } from '../../shared/models/videos' import { AccountBlocklistModel } from '../models/account/account-blocklist' +import { UserModel } from '../models/account/user' +import { UserNotificationModel } from '../models/account/user-notification' +import { MAccountServer, MActorFollowFull } from '../typings/models' import { MCommentOwnerVideo, MVideoAbuseVideo, @@ -15,18 +25,9 @@ import { MVideoBlacklistVideo, MVideoFullLight } from '../typings/models/video' -import { - MUser, - MUserAccount, - MUserDefault, - MUserNotifSettingAccount, - MUserWithNotificationSetting, - UserNotificationModelForApi -} from '@server/typings/models/user' -import { MAccountDefault, MActorFollowFull } from '../typings/models' -import { MVideoImportVideo } from '@server/typings/models/video/video-import' -import { ServerBlocklistModel } from '@server/models/server/server-blocklist' -import { getServerActor } from '@server/models/application/application' +import { isBlockedByServerOrAccount } from './blocklist' +import { Emailer } from './emailer' +import { PeerTubeSocket } from './peertube-socket' class Notifier { @@ -169,7 +170,7 @@ class Notifier { // Not our user or user comments its own video if (!user || comment.Account.userId === user.id) return - if (await this.isBlockedByServerOrAccount(user, comment.Account)) return + if (await this.isBlockedByServerOrUser(comment.Account, user)) return logger.info('Notifying user %s of new comment %s.', user.username, comment.url) @@ -270,7 +271,7 @@ class Notifier { const followerAccount = actorFollow.ActorFollower.Account const followerAccountWithActor = Object.assign(followerAccount, { Actor: actorFollow.ActorFollower }) - if (await this.isBlockedByServerOrAccount(user, followerAccountWithActor)) return + if (await this.isBlockedByServerOrUser(followerAccountWithActor, user)) return logger.info('Notifying user %s of new follower: %s.', user.username, followerAccount.getDisplayName()) @@ -299,6 +300,9 @@ class Notifier { private async notifyAdminsOfNewInstanceFollow (actorFollow: MActorFollowFull) { const admins = await UserModel.listWithRight(UserRight.MANAGE_SERVER_FOLLOW) + const follower = Object.assign(actorFollow.ActorFollower.Account, { Actor: actorFollow.ActorFollower }) + if (await this.isBlockedByServerOrUser(follower)) return + logger.info('Notifying %d administrators of new instance follower: %s.', admins.length, actorFollow.ActorFollower.url) function settingGetter (user: MUserWithNotificationSetting) { @@ -590,17 +594,8 @@ class Notifier { return value & UserNotificationSettingValue.WEB } - private async isBlockedByServerOrAccount (user: MUserAccount, targetAccount: MAccountDefault) { - const serverAccountId = (await getServerActor()).Account.id - const sourceAccounts = [ serverAccountId, user.Account.id ] - - const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, targetAccount.id) - if (accountMutedHash[serverAccountId] || accountMutedHash[user.Account.id]) return true - - const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, targetAccount.Actor.serverId) - if (instanceMutedHash[serverAccountId] || instanceMutedHash[user.Account.id]) return true - - return false + private isBlockedByServerOrUser (targetAccount: MAccountServer, user?: MUserAccount) { + return isBlockedByServerOrAccount(targetAccount, user?.Account) } static get Instance () { diff --git a/server/models/account/account.ts b/server/models/account/account.ts index a0081f259..ad649837a 100644 --- a/server/models/account/account.ts +++ b/server/models/account/account.ts @@ -32,9 +32,10 @@ import { FindOptions, IncludeOptions, Op, Transaction, WhereOptions } from 'sequ import { AccountBlocklistModel } from './account-blocklist' import { ServerBlocklistModel } from '../server/server-blocklist' import { ActorFollowModel } from '../activitypub/actor-follow' -import { MAccountActor, MAccountAP, MAccountDefault, MAccountFormattable, MAccountSummaryFormattable } from '../../typings/models' +import { MAccountActor, MAccountAP, MAccountDefault, MAccountFormattable, MAccountSummaryFormattable, MAccount } from '../../typings/models' import * as Bluebird from 'bluebird' import { ModelCache } from '@server/models/model-cache' +import { VideoModel } from '../video/video' export enum ScopeNames { SUMMARY = 'SUMMARY' @@ -343,6 +344,29 @@ export class AccountModel extends Model { }) } + static loadAccountIdFromVideo (videoId: number): Bluebird { + const query = { + include: [ + { + attributes: [ 'id', 'accountId' ], + model: VideoChannelModel.unscoped(), + required: true, + include: [ + { + attributes: [ 'id', 'channelId' ], + model: VideoModel.unscoped(), + where: { + id: videoId + } + } + ] + } + ] + } + + return AccountModel.findOne(query) + } + static listLocalsForSitemap (sort: string): Bluebird { const query = { attributes: [ ], diff --git a/server/models/utils.ts b/server/models/utils.ts index b2573cd35..88c9b4adb 100644 --- a/server/models/utils.ts +++ b/server/models/utils.ts @@ -136,10 +136,7 @@ function createSimilarityAttribute (col: string, value: string) { ) } -function buildBlockedAccountSQL (serverAccountId: number, userAccountId?: number) { - const blockerIds = [ serverAccountId ] - if (userAccountId) blockerIds.push(userAccountId) - +function buildBlockedAccountSQL (blockerIds: number[]) { const blockerIdsString = blockerIds.join(', ') return 'SELECT "targetAccountId" AS "id" FROM "accountBlocklist" WHERE "accountId" IN (' + blockerIdsString + ')' + diff --git a/server/models/video/video-abuse.ts b/server/models/video/video-abuse.ts index 0844f702d..e0cf50b59 100644 --- a/server/models/video/video-abuse.ts +++ b/server/models/video/video-abuse.ts @@ -57,7 +57,7 @@ export enum ScopeNames { }) => { const where = { reporterAccountId: { - [Op.notIn]: literal('(' + buildBlockedAccountSQL(options.serverAccountId, options.userAccountId) + ')') + [Op.notIn]: literal('(' + buildBlockedAccountSQL([ options.serverAccountId, options.userAccountId ]) + ')') } } diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index dfeb1c4e7..ba09522cc 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -21,7 +21,8 @@ import { MCommentOwnerReplyVideoLight, MCommentOwnerVideo, MCommentOwnerVideoFeed, - MCommentOwnerVideoReply + MCommentOwnerVideoReply, + MVideoImmutable } from '../../typings/models/video' import { AccountModel } from '../account/account' import { ActorModel, unusedActorAttributesForAPI } from '../activitypub/actor' @@ -38,14 +39,14 @@ enum ScopeNames { } @Scopes(() => ({ - [ScopeNames.ATTRIBUTES_FOR_API]: (serverAccountId: number, userAccountId?: number) => { + [ScopeNames.ATTRIBUTES_FOR_API]: (blockerAccountIds: number[]) => { return { attributes: { include: [ [ Sequelize.literal( '(' + - 'WITH "blocklist" AS (' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')' + + 'WITH "blocklist" AS (' + buildBlockedAccountSQL(blockerAccountIds) + ')' + 'SELECT COUNT("replies"."id") - (' + 'SELECT COUNT("replies"."id") ' + 'FROM "videoComment" AS "replies" ' + @@ -276,16 +277,15 @@ export class VideoCommentModel extends Model { static async listThreadsForApi (parameters: { videoId: number + isVideoOwned: boolean start: number count: number sort: string user?: MUserAccountId }) { - const { videoId, start, count, sort, user } = parameters + const { videoId, isVideoOwned, start, count, sort, user } = parameters - const serverActor = await getServerActor() - const serverAccountId = serverActor.Account.id - const userAccountId = user ? user.Account.id : undefined + const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned }) const query = { offset: start, @@ -304,7 +304,7 @@ export class VideoCommentModel extends Model { { accountId: { [Op.notIn]: Sequelize.literal( - '(' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')' + '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' ) } }, @@ -320,7 +320,7 @@ export class VideoCommentModel extends Model { const scopes: (string | ScopeOptions)[] = [ ScopeNames.WITH_ACCOUNT_FOR_API, { - method: [ ScopeNames.ATTRIBUTES_FOR_API, serverAccountId, userAccountId ] + method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ] } ] @@ -334,14 +334,13 @@ export class VideoCommentModel extends Model { static async listThreadCommentsForApi (parameters: { videoId: number + isVideoOwned: boolean threadId: number user?: MUserAccountId }) { - const { videoId, threadId, user } = parameters + const { videoId, threadId, user, isVideoOwned } = parameters - const serverActor = await getServerActor() - const serverAccountId = serverActor.Account.id - const userAccountId = user ? user.Account.id : undefined + const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned }) const query = { order: [ [ 'createdAt', 'ASC' ], [ 'updatedAt', 'ASC' ] ] as Order, @@ -353,7 +352,7 @@ export class VideoCommentModel extends Model { ], accountId: { [Op.notIn]: Sequelize.literal( - '(' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')' + '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' ) } } @@ -362,7 +361,7 @@ export class VideoCommentModel extends Model { const scopes: any[] = [ ScopeNames.WITH_ACCOUNT_FOR_API, { - method: [ ScopeNames.ATTRIBUTES_FOR_API, serverAccountId, userAccountId ] + method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ] } ] @@ -399,13 +398,23 @@ export class VideoCommentModel extends Model { .findAll(query) } - static listAndCountByVideoId (videoId: number, start: number, count: number, t?: Transaction, order: 'ASC' | 'DESC' = 'ASC') { + static async listAndCountByVideoForAP (video: MVideoImmutable, start: number, count: number, t?: Transaction) { + const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ + videoId: video.id, + isVideoOwned: video.isOwned() + }) + const query = { - order: [ [ 'createdAt', order ] ] as Order, + order: [ [ 'createdAt', 'ASC' ] ] as Order, offset: start, limit: count, where: { - videoId + videoId: video.id, + accountId: { + [Op.notIn]: Sequelize.literal( + '(' + buildBlockedAccountSQL(blockerAccountIds) + ')' + ) + } }, transaction: t } @@ -424,7 +433,7 @@ export class VideoCommentModel extends Model { deletedAt: null, accountId: { [Op.notIn]: Sequelize.literal( - '(' + buildBlockedAccountSQL(serverActor.Account.id) + ')' + '(' + buildBlockedAccountSQL([ serverActor.Account.id, '"Video->VideoChannel"."accountId"' ]) + ')' ) } }, @@ -435,7 +444,14 @@ export class VideoCommentModel extends Model { required: true, where: { privacy: VideoPrivacy.PUBLIC - } + }, + include: [ + { + attributes: [ 'accountId' ], + model: VideoChannelModel.unscoped(), + required: true + } + ] } ] } @@ -650,4 +666,24 @@ export class VideoCommentModel extends Model { tag } } + + private static async buildBlockerAccountIds (options: { + videoId: number + isVideoOwned: boolean + user?: MUserAccountId + }) { + const { videoId, user, isVideoOwned } = options + + const serverActor = await getServerActor() + const blockerAccountIds = [ serverActor.Account.id ] + + if (user) blockerAccountIds.push(user.Account.id) + + if (isVideoOwned) { + const videoOwnerAccount = await AccountModel.loadAccountIdFromVideo(videoId) + blockerAccountIds.push(videoOwnerAccount.id) + } + + return blockerAccountIds + } } diff --git a/server/tests/api/users/blocklist.ts b/server/tests/api/users/blocklist.ts index e37dbb5a4..8c9107a50 100644 --- a/server/tests/api/users/blocklist.ts +++ b/server/tests/api/users/blocklist.ts @@ -2,7 +2,7 @@ import * as chai from 'chai' import 'mocha' -import { AccountBlock, ServerBlock, Video } from '../../../../shared/index' +import { AccountBlock, ServerBlock, Video, UserNotification, UserNotificationType } from '../../../../shared/index' import { cleanupTests, createUser, @@ -11,7 +11,9 @@ import { flushAndRunMultipleServers, ServerInfo, uploadVideo, - userLogin + userLogin, + follow, + unfollow } from '../../../../shared/extra-utils/index' import { setAccessTokensToServers } from '../../../../shared/extra-utils/users/login' import { getVideosList, getVideosListWithToken } from '../../../../shared/extra-utils/videos/videos' @@ -19,7 +21,8 @@ import { addVideoCommentReply, addVideoCommentThread, getVideoCommentThreads, - getVideoThreadComments + getVideoThreadComments, + findCommentId } from '../../../../shared/extra-utils/videos/video-comments' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' import { VideoComment, VideoCommentThreadTree } from '../../../../shared/models/videos/video-comment.model' @@ -45,13 +48,13 @@ async function checkAllVideos (url: string, token: string) { { const res = await getVideosListWithToken(url, token) - expect(res.body.data).to.have.lengthOf(4) + expect(res.body.data).to.have.lengthOf(5) } { const res = await getVideosList(url) - expect(res.body.data).to.have.lengthOf(4) + expect(res.body.data).to.have.lengthOf(5) } } @@ -76,13 +79,15 @@ async function checkCommentNotification ( check: 'presence' | 'absence' ) { const resComment = await addVideoCommentThread(comment.server.url, comment.token, comment.videoUUID, comment.text) - const threadId = resComment.body.comment.id + const created = resComment.body.comment as VideoComment + const threadId = created.id + const createdAt = created.createdAt await waitJobs([ mainServer, comment.server ]) const res = await getUserNotifications(mainServer.url, mainServer.accessToken, 0, 30) - const commentNotifications = res.body.data - .filter(n => n.comment && n.comment.id === threadId) + const commentNotifications = (res.body.data as UserNotification[]) + .filter(n => n.comment && n.comment.video.uuid === comment.videoUUID && n.createdAt >= createdAt) if (check === 'presence') expect(commentNotifications).to.have.lengthOf(1) else expect(commentNotifications).to.have.lengthOf(0) @@ -96,6 +101,7 @@ describe('Test blocklist', function () { let servers: ServerInfo[] let videoUUID1: string let videoUUID2: string + let videoUUID3: string let userToken1: string let userModeratorToken: string let userToken2: string @@ -103,7 +109,7 @@ describe('Test blocklist', function () { before(async function () { this.timeout(60000) - servers = await flushAndRunMultipleServers(2) + servers = await flushAndRunMultipleServers(3) await setAccessTokensToServers(servers) { @@ -139,7 +145,13 @@ describe('Test blocklist', function () { videoUUID2 = res.body.video.uuid } + { + const res = await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'video 2 server 1' }) + videoUUID3 = res.body.video.uuid + } + await doubleFollow(servers[0], servers[1]) + await doubleFollow(servers[0], servers[2]) { const resComment = await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID1, 'comment root 1') @@ -174,7 +186,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(3) + expect(videos).to.have.lengthOf(4) const v = videos.find(v => v.name === 'video user 2') expect(v).to.be.undefined @@ -188,7 +200,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(2) + expect(videos).to.have.lengthOf(3) const v = videos.find(v => v.name === 'video user 1') expect(v).to.be.undefined @@ -235,10 +247,6 @@ describe('Test blocklist', function () { return checkAllVideos(servers[0].url, userToken1) }) - it('Should list all the comments with another user', async function () { - return checkAllComments(servers[0].url, userToken1, videoUUID1) - }) - it('Should list blocked accounts', async function () { { const res = await getAccountBlocklistByAccount(servers[0].url, servers[0].accessToken, 0, 1, 'createdAt') @@ -269,6 +277,61 @@ describe('Test blocklist', function () { } }) + it('Should not allow a remote blocked user to comment my videos', async function () { + this.timeout(60000) + + { + await addVideoCommentThread(servers[1].url, userToken2, videoUUID3, 'comment user 2') + await waitJobs(servers) + + await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID3, 'uploader') + await waitJobs(servers) + + const commentId = await findCommentId(servers[1].url, videoUUID3, 'uploader') + const message = 'reply by user 2' + const resReply = await addVideoCommentReply(servers[1].url, userToken2, videoUUID3, commentId, message) + await addVideoCommentReply(servers[1].url, servers[1].accessToken, videoUUID3, resReply.body.comment.id, 'another reply') + + await waitJobs(servers) + } + + // Server 2 has all the comments + { + const resThreads = await getVideoCommentThreads(servers[1].url, videoUUID3, 0, 25, '-createdAt') + const threads: VideoComment[] = resThreads.body.data + + expect(threads).to.have.lengthOf(2) + expect(threads[0].text).to.equal('uploader') + expect(threads[1].text).to.equal('comment user 2') + + const resReplies = await getVideoThreadComments(servers[1].url, videoUUID3, threads[0].id) + + const tree: VideoCommentThreadTree = resReplies.body + expect(tree.children).to.have.lengthOf(1) + expect(tree.children[0].comment.text).to.equal('reply by user 2') + expect(tree.children[0].children).to.have.lengthOf(1) + expect(tree.children[0].children[0].comment.text).to.equal('another reply') + } + + // Server 1 and 3 should only have uploader comments + for (const server of [ servers[0], servers[2] ]) { + const resThreads = await getVideoCommentThreads(server.url, videoUUID3, 0, 25, '-createdAt') + const threads: VideoComment[] = resThreads.body.data + + expect(threads).to.have.lengthOf(1) + expect(threads[0].text).to.equal('uploader') + + const resReplies = await getVideoThreadComments(server.url, videoUUID3, threads[0].id) + + const tree: VideoCommentThreadTree = resReplies.body + if (server.serverNumber === 1) { + expect(tree.children).to.have.lengthOf(0) + } else { + expect(tree.children).to.have.lengthOf(1) + } + } + }) + it('Should unblock the remote account', async function () { await removeAccountFromAccountBlocklist(servers[0].url, servers[0].accessToken, 'user2@localhost:' + servers[1].port) }) @@ -277,12 +340,37 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(3) + expect(videos).to.have.lengthOf(4) const v = videos.find(v => v.name === 'video user 2') expect(v).not.to.be.undefined }) + it('Should display its comments on my video', async function () { + for (const server of servers) { + const resThreads = await getVideoCommentThreads(server.url, videoUUID3, 0, 25, '-createdAt') + const threads: VideoComment[] = resThreads.body.data + + // Server 3 should not have 2 comment threads, because server 1 did not forward the server 2 comment + if (server.serverNumber === 3) { + expect(threads).to.have.lengthOf(1) + continue + } + + expect(threads).to.have.lengthOf(2) + expect(threads[0].text).to.equal('uploader') + expect(threads[1].text).to.equal('comment user 2') + + const resReplies = await getVideoThreadComments(server.url, videoUUID3, threads[0].id) + + const tree: VideoCommentThreadTree = resReplies.body + expect(tree.children).to.have.lengthOf(1) + expect(tree.children[0].comment.text).to.equal('reply by user 2') + expect(tree.children[0].children).to.have.lengthOf(1) + expect(tree.children[0].children[0].comment.text).to.equal('another reply') + } + }) + it('Should unblock the local account', async function () { await removeAccountFromAccountBlocklist(servers[0].url, servers[0].accessToken, 'user1') }) @@ -328,7 +416,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(2) + expect(videos).to.have.lengthOf(3) const v1 = videos.find(v => v.name === 'video user 2') const v2 = videos.find(v => v.name === 'video server 2') @@ -442,7 +530,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, token) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(3) + expect(videos).to.have.lengthOf(4) const v = videos.find(v => v.name === 'video user 2') expect(v).to.be.undefined @@ -458,7 +546,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, token) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(2) + expect(videos).to.have.lengthOf(3) const v = videos.find(v => v.name === 'video user 1') expect(v).to.be.undefined @@ -545,7 +633,7 @@ describe('Test blocklist', function () { const res = await getVideosListWithToken(servers[0].url, token) const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(3) + expect(videos).to.have.lengthOf(4) const v = videos.find(v => v.name === 'video user 2') expect(v).not.to.be.undefined @@ -606,7 +694,7 @@ describe('Test blocklist', function () { for (const res of [ res1, res2 ]) { const videos: Video[] = res.body.data - expect(videos).to.have.lengthOf(2) + expect(videos).to.have.lengthOf(3) const v1 = videos.find(v => v.name === 'video user 2') const v2 = videos.find(v => v.name === 'video server 2') @@ -631,7 +719,7 @@ describe('Test blocklist', function () { }) it('Should not have notification from blocked instances by instance', async function () { - this.timeout(20000) + this.timeout(50000) { const comment = { server: servers[1], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' } @@ -647,6 +735,24 @@ describe('Test blocklist', function () { } await checkCommentNotification(servers[0], comment, 'absence') } + + { + const now = new Date() + await unfollow(servers[1].url, servers[1].accessToken, servers[0]) + await waitJobs(servers) + await follow(servers[1].url, [ servers[0].host ], servers[1].accessToken) + + await waitJobs(servers) + + const res = await getUserNotifications(servers[0].url, servers[0].accessToken, 0, 30) + const commentNotifications = (res.body.data as UserNotification[]) + .filter(n => { + return n.type === UserNotificationType.NEW_INSTANCE_FOLLOWER && + n.createdAt >= now.toISOString() + }) + + expect(commentNotifications).to.have.lengthOf(0) + } }) it('Should list blocked servers', async function () { @@ -678,7 +784,7 @@ describe('Test blocklist', function () { }) it('Should have notification from unblocked instances', async function () { - this.timeout(20000) + this.timeout(50000) { const comment = { server: servers[1], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' } @@ -694,6 +800,24 @@ describe('Test blocklist', function () { } await checkCommentNotification(servers[0], comment, 'presence') } + + { + const now = new Date() + await unfollow(servers[1].url, servers[1].accessToken, servers[0]) + await waitJobs(servers) + await follow(servers[1].url, [ servers[0].host ], servers[1].accessToken) + + await waitJobs(servers) + + const res = await getUserNotifications(servers[0].url, servers[0].accessToken, 0, 30) + const commentNotifications = (res.body.data as UserNotification[]) + .filter(n => { + return n.type === UserNotificationType.NEW_INSTANCE_FOLLOWER && + n.createdAt >= now.toISOString() + }) + + expect(commentNotifications).to.have.lengthOf(1) + } }) }) }) diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts index e3029f1ae..d7b04373f 100644 --- a/server/tests/api/videos/multiple-servers.ts +++ b/server/tests/api/videos/multiple-servers.ts @@ -37,7 +37,8 @@ import { addVideoCommentThread, deleteVideoComment, getVideoCommentThreads, - getVideoThreadComments + getVideoThreadComments, + findCommentId } from '../../../../shared/extra-utils/videos/video-comments' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' @@ -773,8 +774,7 @@ describe('Test multiple servers', function () { await waitJobs(servers) { - const res = await getVideoCommentThreads(servers[1].url, videoUUID, 0, 5) - const threadId = res.body.data.find(c => c.text === 'my super first comment').id + const threadId = await findCommentId(servers[1].url, videoUUID, 'my super first comment') const text = 'my super answer to thread 1' await addVideoCommentReply(servers[1].url, servers[1].accessToken, videoUUID, threadId, text) @@ -783,8 +783,7 @@ describe('Test multiple servers', function () { await waitJobs(servers) { - const res1 = await getVideoCommentThreads(servers[2].url, videoUUID, 0, 5) - const threadId = res1.body.data.find(c => c.text === 'my super first comment').id + const threadId = await findCommentId(servers[2].url, videoUUID, 'my super first comment') const res2 = await getVideoThreadComments(servers[2].url, videoUUID, threadId) const childCommentId = res2.body.children[0].comment.id diff --git a/server/tests/feeds/feeds.ts b/server/tests/feeds/feeds.ts index 7fac921a3..ba961cdba 100644 --- a/server/tests/feeds/feeds.ts +++ b/server/tests/feeds/feeds.ts @@ -1,7 +1,14 @@ /* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */ -import * as chai from 'chai' import 'mocha' +import * as chai from 'chai' +import * as libxmljs from 'libxmljs' +import { + addAccountToAccountBlocklist, + addAccountToServerBlocklist, + removeAccountFromServerBlocklist +} from '@shared/extra-utils/users/blocklist' +import { VideoPrivacy } from '@shared/models' import { cleanupTests, createUser, @@ -13,14 +20,12 @@ import { ServerInfo, setAccessTokensToServers, uploadVideo, + uploadVideoAndGetId, userLogin } from '../../../shared/extra-utils' -import * as libxmljs from 'libxmljs' -import { addVideoCommentThread } from '../../../shared/extra-utils/videos/video-comments' import { waitJobs } from '../../../shared/extra-utils/server/jobs' +import { addVideoCommentThread } from '../../../shared/extra-utils/videos/video-comments' import { User } from '../../../shared/models/users' -import { VideoPrivacy } from '@shared/models' -import { addAccountToServerBlocklist } from '@shared/extra-utils/users/blocklist' chai.use(require('chai-xml')) chai.use(require('chai-json-schema')) @@ -219,7 +224,11 @@ describe('Test syndication feeds', () => { }) it('Should not list comments from muted accounts or instances', async function () { - await addAccountToServerBlocklist(servers[1].url, servers[1].accessToken, 'root@localhost:' + servers[0].port) + this.timeout(30000) + + const remoteHandle = 'root@localhost:' + servers[0].port + + await addAccountToServerBlocklist(servers[1].url, servers[1].accessToken, remoteHandle) { const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 2 }) @@ -227,6 +236,26 @@ describe('Test syndication feeds', () => { expect(jsonObj.items.length).to.be.equal(0) } + await removeAccountFromServerBlocklist(servers[1].url, servers[1].accessToken, remoteHandle) + + { + const videoUUID = (await uploadVideoAndGetId({ server: servers[1], videoName: 'server 2' })).uuid + await waitJobs(servers) + await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID, 'super comment') + await waitJobs(servers) + + const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 3 }) + const jsonObj = JSON.parse(json.text) + expect(jsonObj.items.length).to.be.equal(3) + } + + await addAccountToAccountBlocklist(servers[1].url, servers[1].accessToken, remoteHandle) + + { + const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 4 }) + const jsonObj = JSON.parse(json.text) + expect(jsonObj.items.length).to.be.equal(2) + } }) }) diff --git a/shared/extra-utils/videos/video-comments.ts b/shared/extra-utils/videos/video-comments.ts index 81c48412d..831e5e7d4 100644 --- a/shared/extra-utils/videos/video-comments.ts +++ b/shared/extra-utils/videos/video-comments.ts @@ -61,6 +61,12 @@ function addVideoCommentReply ( .expect(expectedStatus) } +async function findCommentId (url: string, videoId: number | string, text: string) { + const res = await getVideoCommentThreads(url, videoId, 0, 25, '-createdAt') + + return res.body.data.find(c => c.text === text).id as number +} + function deleteVideoComment ( url: string, token: string, @@ -85,5 +91,6 @@ export { getVideoThreadComments, addVideoCommentThread, addVideoCommentReply, + findCommentId, deleteVideoComment } diff --git a/shared/extra-utils/videos/videos.ts b/shared/extra-utils/videos/videos.ts index 0d36a38a2..99e591cb2 100644 --- a/shared/extra-utils/videos/videos.ts +++ b/shared/extra-utils/videos/videos.ts @@ -95,6 +95,12 @@ function getVideo (url: string, id: number | string, expectedStatus = 200) { .expect(expectedStatus) } +async function getVideoIdFromUUID (url: string, uuid: string) { + const res = await getVideo(url, uuid) + + return res.body.id +} + function getVideoFileMetadataUrl (url: string) { return request(url) .get('/') @@ -669,5 +675,6 @@ export { checkVideoFilesWereRemoved, getPlaylistVideos, uploadVideoAndGetId, - getLocalIdByUUID + getLocalIdByUUID, + getVideoIdFromUUID } -- 2.25.1