From 511765c9f86fb07d5d856decd9dbf0ec2092f4fe Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 7 Aug 2019 15:35:29 +0200 Subject: [PATCH] Remove comment federation by video owner --- server/controllers/api/videos/comment.ts | 8 ++++ .../lib/activitypub/process/process-delete.ts | 6 +-- server/lib/activitypub/send/send-delete.ts | 5 ++- server/models/video/video-comment.ts | 43 +++---------------- server/tests/api/videos/multiple-servers.ts | 41 ++++++++++++------ 5 files changed, 49 insertions(+), 54 deletions(-) diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts index 39d521f5f..bc6d81a7c 100644 --- a/server/controllers/api/videos/comment.ts +++ b/server/controllers/api/videos/comment.ts @@ -27,6 +27,10 @@ import { auditLoggerFactory, CommentAuditView, getAuditIdFromRes } from '../../. import { AccountModel } from '../../../models/account/account' import { Notifier } from '../../../lib/notifier' import { Hooks } from '../../../lib/plugins/hooks' +import { ActorModel } from '../../../models/activitypub/actor' +import { VideoChannelModel } from '../../../models/video/video-channel' +import { VideoModel } from '../../../models/video/video' +import { sendDeleteVideoComment } from '../../../lib/activitypub/send' const auditLogger = auditLoggerFactory('comments') const videoCommentRouter = express.Router() @@ -179,6 +183,10 @@ async function removeVideoComment (req: express.Request, res: express.Response) await sequelizeTypescript.transaction(async t => { await videoCommentInstance.destroy({ transaction: t }) + + if (videoCommentInstance.isOwned() || videoCommentInstance.Video.isOwned()) { + await sendDeleteVideoComment(videoCommentInstance, t) + } }) auditLogger.delete(getAuditIdFromRes(res), new CommentAuditView(videoCommentInstance.toFormattedJSON())) diff --git a/server/lib/activitypub/process/process-delete.ts b/server/lib/activitypub/process/process-delete.ts index 845a7b249..9fcfd9e3a 100644 --- a/server/lib/activitypub/process/process-delete.ts +++ b/server/lib/activitypub/process/process-delete.ts @@ -34,7 +34,7 @@ async function processDeleteActivity (options: APProcessorOptions { - if (videoComment.Account.id !== byActor.Account.id) { - throw new Error('Account ' + byActor.url + ' does not own video comment ' + videoComment.url) + if (byActor.Account.id !== videoComment.Account.id && byActor.Account.id !== videoComment.Video.VideoChannel.accountId) { + throw new Error(`Account ${byActor.url} does not own video comment ${videoComment.url} or video ${videoComment.Video.url}`) } await videoComment.destroy({ transaction: t }) diff --git a/server/lib/activitypub/send/send-delete.ts b/server/lib/activitypub/send/send-delete.ts index 7a1d6f0ba..6c7fb8449 100644 --- a/server/lib/activitypub/send/send-delete.ts +++ b/server/lib/activitypub/send/send-delete.ts @@ -48,7 +48,10 @@ async function sendDeleteVideoComment (videoComment: VideoCommentModel, t: Trans const isVideoOrigin = videoComment.Video.isOwned() const url = getDeleteActivityPubUrl(videoComment.url) - const byActor = videoComment.Account.Actor + const byActor = videoComment.isOwned() + ? videoComment.Account.Actor + : videoComment.Video.VideoChannel.Account.Actor + const threadParentComments = await VideoCommentModel.listThreadParentComments(videoComment, t) const actorsInvolvedInComment = await getActorsInvolvedInVideo(videoComment.Video, t) diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index 6eda32f05..58b75510d 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -105,6 +105,10 @@ enum ScopeNames { model: VideoChannelModel.unscoped(), required: true, include: [ + { + model: ActorModel, + required: true + }, { model: AccountModel, required: true, @@ -208,41 +212,6 @@ export class VideoCommentModel extends Model { }) Account: AccountModel - @BeforeDestroy - static async sendDeleteIfOwned (instance: VideoCommentModel, options) { - if (!instance.Account || !instance.Account.Actor) { - instance.Account = await instance.$get('Account', { - include: [ ActorModel ], - transaction: options.transaction - }) as AccountModel - } - - if (!instance.Video) { - instance.Video = await instance.$get('Video', { - include: [ - { - model: VideoChannelModel, - include: [ - { - model: AccountModel, - include: [ - { - model: ActorModel - } - ] - } - ] - } - ], - transaction: options.transaction - }) as VideoModel - } - - if (instance.isOwned()) { - await sendDeleteVideoComment(instance, options.transaction) - } - } - static loadById (id: number, t?: Transaction) { const query: FindOptions = { where: { @@ -269,7 +238,7 @@ export class VideoCommentModel extends Model { .findOne(query) } - static loadByUrlAndPopulateAccount (url: string, t?: Transaction) { + static loadByUrlAndPopulateAccountAndVideo (url: string, t?: Transaction) { const query: FindOptions = { where: { url @@ -278,7 +247,7 @@ export class VideoCommentModel extends Model { if (t !== undefined) query.transaction = t - return VideoCommentModel.scope([ ScopeNames.WITH_ACCOUNT ]).findOne(query) + return VideoCommentModel.scope([ ScopeNames.WITH_ACCOUNT, ScopeNames.WITH_VIDEO ]).findOne(query) } static loadByUrlAndPopulateReplyAndVideoUrlAndAccount (url: string, t?: Transaction) { diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts index e811ccd8e..651765776 100644 --- a/server/tests/api/videos/multiple-servers.ts +++ b/server/tests/api/videos/multiple-servers.ts @@ -500,20 +500,18 @@ describe('Test multiple servers', function () { it('Should view multiple videos on owned servers', async function () { this.timeout(30000) - const tasks: Promise[] = [] - await viewVideo(servers[2].url, localVideosServer3[0]) - await viewVideo(servers[2].url, localVideosServer3[0]) await viewVideo(servers[2].url, localVideosServer3[0]) - await viewVideo(servers[2].url, localVideosServer3[1]) - - await Promise.all(tasks) - await waitJobs(servers) + await wait(1000) await viewVideo(servers[2].url, localVideosServer3[0]) + await viewVideo(servers[2].url, localVideosServer3[1]) - await waitJobs(servers) + await wait(1000) - await viewVideo(servers[2].url, localVideosServer3[0]) + await Promise.all([ + viewVideo(servers[2].url, localVideosServer3[0]), + viewVideo(servers[2].url, localVideosServer3[0]) + ]) await waitJobs(servers) @@ -894,14 +892,14 @@ describe('Test multiple servers', function () { it('Should delete the thread comments', async function () { this.timeout(10000) - const res1 = await getVideoCommentThreads(servers[0].url, videoUUID, 0, 5) - const threadId = res1.body.data.find(c => c.text === 'my super first comment').id - await deleteVideoComment(servers[0].url, servers[0].accessToken, videoUUID, threadId) + const res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5) + const threadId = res.body.data.find(c => c.text === 'my super first comment').id + await deleteVideoComment(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID, threadId) await waitJobs(servers) }) - it('Should have the thread comments deleted on other servers too', async function () { + it('Should have the threads deleted on other servers too', async function () { for (const server of servers) { const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) @@ -922,6 +920,23 @@ describe('Test multiple servers', function () { } }) + it('Should delete a remote thread by the origin server', async function () { + const res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5) + const threadId = res.body.data.find(c => c.text === 'my super second comment').id + await deleteVideoComment(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID, threadId) + + await waitJobs(servers) + }) + + it('Should have the threads deleted on other servers too', async function () { + for (const server of servers) { + const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5) + + expect(res.body.total).to.equal(0) + expect(res.body.data).to.have.lengthOf(0) + } + }) + it('Should disable comments and download', async function () { this.timeout(20000) -- 2.41.0