From 073deef8862f462de5f159a57877ef415ebe4c69 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 27 Jul 2022 11:05:32 +0200 Subject: [PATCH] Handle rejected follows in client Also add quick filters so it's easier to find pending follows --- .../followers-list.component.html | 19 ++- .../followers-list.component.ts | 7 +- .../following-list.component.html | 13 +- .../following-list.component.ts | 5 + .../instance-follow.service.ts | 41 ++++++- .../lib/activitypub/process/process-follow.ts | 108 ++++++++++++----- server/tests/api/server/follows-moderation.ts | 113 +++++++++--------- 7 files changed, 198 insertions(+), 108 deletions(-) diff --git a/client/src/app/+admin/follows/followers-list/followers-list.component.html b/client/src/app/+admin/follows/followers-list/followers-list.component.html index 3081098c4..4f11f261d 100644 --- a/client/src/app/+admin/follows/followers-list/followers-list.component.html +++ b/client/src/app/+admin/follows/followers-list/followers-list.component.html @@ -13,7 +13,7 @@
- +
@@ -31,12 +31,10 @@ - - - - + + - + @@ -45,11 +43,10 @@ - - Accepted - - - Pending + + Accepted + Pending + Rejected {{ follow.score }} diff --git a/client/src/app/+admin/follows/followers-list/followers-list.component.ts b/client/src/app/+admin/follows/followers-list/followers-list.component.ts index 329e3bcc7..d09e74fef 100644 --- a/client/src/app/+admin/follows/followers-list/followers-list.component.ts +++ b/client/src/app/+admin/follows/followers-list/followers-list.component.ts @@ -1,6 +1,7 @@ import { SortMeta } from 'primeng/api' import { Component, OnInit } from '@angular/core' import { ConfirmService, Notifier, RestPagination, RestTable } from '@app/core' +import { AdvancedInputFilter } from '@app/shared/shared-forms' import { InstanceFollowService } from '@app/shared/shared-instance' import { ActorFollow } from '@shared/models' @@ -15,12 +16,16 @@ export class FollowersListComponent extends RestTable implements OnInit { sort: SortMeta = { field: 'createdAt', order: -1 } pagination: RestPagination = { count: this.rowsPerPage, start: 0 } + searchFilters: AdvancedInputFilter[] + constructor ( private confirmService: ConfirmService, private notifier: Notifier, private followService: InstanceFollowService ) { super() + + this.searchFilters = this.followService.buildFollowsListFilters() } ngOnInit () { @@ -70,7 +75,7 @@ export class FollowersListComponent extends RestTable implements OnInit { } async deleteFollower (follow: ActorFollow) { - const message = $localize`Do you really want to delete this follower?` + const message = $localize`Do you really want to delete this follower? It will be able to send again another follow request.` const res = await this.confirmService.confirm(message, $localize`Delete`) if (res === false) return diff --git a/client/src/app/+admin/follows/following-list/following-list.component.html b/client/src/app/+admin/follows/following-list/following-list.component.html index 302dc9528..856c4a31f 100644 --- a/client/src/app/+admin/follows/following-list/following-list.component.html +++ b/client/src/app/+admin/follows/following-list/following-list.component.html @@ -20,7 +20,7 @@
- +
@@ -47,11 +47,10 @@ - - Accepted - - - Pending + + Accepted + Pending + Rejected {{ follow.createdAt | date: 'short' }} @@ -66,7 +65,7 @@ - +
No host found matching current filters. Your instance is not following anyone. diff --git a/client/src/app/+admin/follows/following-list/following-list.component.ts b/client/src/app/+admin/follows/following-list/following-list.component.ts index 2c0f6db0c..7a854be81 100644 --- a/client/src/app/+admin/follows/following-list/following-list.component.ts +++ b/client/src/app/+admin/follows/following-list/following-list.component.ts @@ -1,6 +1,7 @@ import { SortMeta } from 'primeng/api' import { Component, OnInit, ViewChild } from '@angular/core' import { ConfirmService, Notifier, RestPagination, RestTable } from '@app/core' +import { AdvancedInputFilter } from '@app/shared/shared-forms' import { InstanceFollowService } from '@app/shared/shared-instance' import { ActorFollow } from '@shared/models' import { FollowModalComponent } from './follow-modal.component' @@ -17,12 +18,16 @@ export class FollowingListComponent extends RestTable implements OnInit { sort: SortMeta = { field: 'createdAt', order: -1 } pagination: RestPagination = { count: this.rowsPerPage, start: 0 } + searchFilters: AdvancedInputFilter[] + constructor ( private notifier: Notifier, private confirmService: ConfirmService, private followService: InstanceFollowService ) { super() + + this.searchFilters = this.followService.buildFollowsListFilters() } ngOnInit () { diff --git a/client/src/app/shared/shared-instance/instance-follow.service.ts b/client/src/app/shared/shared-instance/instance-follow.service.ts index a83f7c4ad..06484d938 100644 --- a/client/src/app/shared/shared-instance/instance-follow.service.ts +++ b/client/src/app/shared/shared-instance/instance-follow.service.ts @@ -6,6 +6,7 @@ import { Injectable } from '@angular/core' import { RestExtractor, RestPagination, RestService } from '@app/core' import { ActivityPubActorType, ActorFollow, FollowState, ResultList, ServerFollowCreate } from '@shared/models' import { environment } from '../../../environments/environment' +import { AdvancedInputFilter } from '../shared-forms' @Injectable() export class InstanceFollowService { @@ -30,7 +31,10 @@ export class InstanceFollowService { let params = new HttpParams() params = this.restService.addRestGetParams(params, pagination, sort) - if (search) params = params.append('search', search) + if (search) { + params = this.restService.addObjectParams(params, this.parseFollowsListFilters(search)) + } + if (state) params = params.append('state', state) if (actorType) params = params.append('actorType', actorType) @@ -53,7 +57,10 @@ export class InstanceFollowService { let params = new HttpParams() params = this.restService.addRestGetParams(params, pagination, sort) - if (search) params = params.append('search', search) + if (search) { + params = this.restService.addObjectParams(params, this.parseFollowsListFilters(search)) + } + if (state) params = params.append('state', state) if (actorType) params = params.append('actorType', actorType) @@ -101,4 +108,34 @@ export class InstanceFollowService { return this.authHttp.delete(`${InstanceFollowService.BASE_APPLICATION_URL}/followers/${handle}`) .pipe(catchError(res => this.restExtractor.handleError(res))) } + + buildFollowsListFilters (): AdvancedInputFilter[] { + return [ + { + title: $localize`Advanced filters`, + children: [ + { + value: 'state:accepted', + label: $localize`Accepted follows` + }, + { + value: 'state:rejected', + label: $localize`Rejected follows` + }, + { + value: 'state:pending', + label: $localize`Pending follows` + } + ] + } + ] + } + + private parseFollowsListFilters (search: string) { + return this.restService.parseQueryStringFilter(search, { + state: { + prefix: 'state:' + } + }) + } } diff --git a/server/lib/activitypub/process/process-follow.ts b/server/lib/activitypub/process/process-follow.ts index a1958f464..e633cd3ae 100644 --- a/server/lib/activitypub/process/process-follow.ts +++ b/server/lib/activitypub/process/process-follow.ts @@ -1,3 +1,6 @@ +import { Transaction } from 'sequelize/types' +import { isBlockedByServerOrAccount } from '@server/lib/blocklist' +import { AccountModel } from '@server/models/account/account' import { getServerActor } from '@server/models/application/application' import { ActivityFollow } from '../../../../shared/models/activitypub' import { retryTransactionWrapper } from '../../../helpers/database-utils' @@ -8,7 +11,7 @@ import { getAPId } from '../../../lib/activitypub/activity' import { ActorModel } from '../../../models/actor/actor' import { ActorFollowModel } from '../../../models/actor/actor-follow' import { APProcessorOptions } from '../../../types/activitypub-processor.model' -import { MActorFollowActors, MActorSignature } from '../../../types/models' +import { MActorFollow, MActorFull, MActorId, MActorSignature } from '../../../types/models' import { Notifier } from '../../notifier' import { autoFollowBackIfNeeded } from '../follow' import { sendAccept, sendReject } from '../send' @@ -31,22 +34,14 @@ export { // --------------------------------------------------------------------------- async function processFollow (byActor: MActorSignature, activityId: string, targetActorURL: string) { - const { actorFollow, created, isFollowingInstance, targetActor } = await sequelizeTypescript.transaction(async t => { + const { actorFollow, created, targetActor } = await sequelizeTypescript.transaction(async t => { const targetActor = await ActorModel.loadByUrlAndPopulateAccountAndChannel(targetActorURL, t) if (!targetActor) throw new Error('Unknown actor') if (targetActor.isOwned() === false) throw new Error('This is not a local actor.') - const serverActor = await getServerActor() - const isFollowingInstance = targetActor.id === serverActor.id - - if (isFollowingInstance && CONFIG.FOLLOWERS.INSTANCE.ENABLED === false) { - logger.info('Rejecting %s because instance followers are disabled.', targetActor.url) - - sendReject(activityId, byActor, targetActor) - - return { actorFollow: undefined as MActorFollowActors } - } + if (rejectIfInstanceFollowDisabled(byActor, activityId, targetActor)) return { actorFollow: undefined } + if (await rejectIfMuted(byActor, activityId, targetActor)) return { actorFollow: undefined } const [ actorFollow, created ] = await ActorFollowModel.findOrCreateCustom({ byActor, @@ -58,24 +53,11 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ transaction: t }) - // Already rejected - if (actorFollow.state === 'rejected') { - return { actorFollow: undefined as MActorFollowActors } - } + if (rejectIfAlreadyRejected(actorFollow, byActor, activityId, targetActor)) return { actorFollow: undefined } - // Set the follow as accepted if the remote actor follows a channel or account - // Or if the instance automatically accepts followers - if (actorFollow.state !== 'accepted' && (isFollowingInstance === false || CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL === false)) { - actorFollow.state = 'accepted' + await acceptIfNeeded(actorFollow, targetActor, t) - await actorFollow.save({ transaction: t }) - } - - // Before PeerTube V3 we did not save the follow ID. Try to fix these old follows - if (!actorFollow.url) { - actorFollow.url = activityId - await actorFollow.save({ transaction: t }) - } + await fixFollowURLIfNeeded(actorFollow, activityId, t) actorFollow.ActorFollower = byActor actorFollow.ActorFollowing = targetActor @@ -87,7 +69,7 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ await autoFollowBackIfNeeded(actorFollow, t) } - return { actorFollow, created, isFollowingInstance, targetActor } + return { actorFollow, created, targetActor } }) // Rejected @@ -97,7 +79,7 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ const follower = await ActorModel.loadFull(byActor.id) const actorFollowFull = Object.assign(actorFollow, { ActorFollowing: targetActor, ActorFollower: follower }) - if (isFollowingInstance) { + if (isFollowingInstance(targetActor)) { Notifier.Instance.notifyOfNewInstanceFollow(actorFollowFull) } else { Notifier.Instance.notifyOfNewUserFollow(actorFollowFull) @@ -106,3 +88,69 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ logger.info('Actor %s is followed by actor %s.', targetActorURL, byActor.url) } + +function rejectIfInstanceFollowDisabled (byActor: MActorSignature, activityId: string, targetActor: MActorFull) { + if (isFollowingInstance(targetActor) && CONFIG.FOLLOWERS.INSTANCE.ENABLED === false) { + logger.info('Rejecting %s because instance followers are disabled.', targetActor.url) + + sendReject(activityId, byActor, targetActor) + + return true + } + + return false +} + +async function rejectIfMuted (byActor: MActorSignature, activityId: string, targetActor: MActorFull) { + const followerAccount = await AccountModel.load(byActor.Account.id) + const followingAccountId = targetActor.Account + + if (followerAccount && await isBlockedByServerOrAccount(followerAccount, followingAccountId)) { + logger.info('Rejecting %s because follower is muted.', byActor.url) + + sendReject(activityId, byActor, targetActor) + + return true + } + + return false +} + +function rejectIfAlreadyRejected (actorFollow: MActorFollow, byActor: MActorSignature, activityId: string, targetActor: MActorFull) { + // Already rejected + if (actorFollow.state === 'rejected') { + logger.info('Rejecting %s because follow is already rejected.', byActor.url) + + sendReject(activityId, byActor, targetActor) + + return true + } + + return false +} + +async function acceptIfNeeded (actorFollow: MActorFollow, targetActor: MActorFull, transaction: Transaction) { + // Set the follow as accepted if the remote actor follows a channel or account + // Or if the instance automatically accepts followers + if (actorFollow.state === 'accepted') return + if (!isFollowingInstance(targetActor)) return + if (CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL === true) return + + actorFollow.state = 'accepted' + + await actorFollow.save({ transaction }) +} + +async function fixFollowURLIfNeeded (actorFollow: MActorFollow, activityId: string, transaction: Transaction) { + // Before PeerTube V3 we did not save the follow ID. Try to fix these old follows + if (!actorFollow.url) { + actorFollow.url = activityId + await actorFollow.save({ transaction }) + } +} + +async function isFollowingInstance (targetActor: MActorId) { + const serverActor = await getServerActor() + + return targetActor.id === serverActor.id +} diff --git a/server/tests/api/server/follows-moderation.ts b/server/tests/api/server/follows-moderation.ts index a0e94c10e..a34eb9bf0 100644 --- a/server/tests/api/server/follows-moderation.ts +++ b/server/tests/api/server/follows-moderation.ts @@ -33,42 +33,39 @@ async function checkServer1And2HasFollowers (servers: PeerTubeServer[], state = } async function checkFollows (options: { - follower: { - server: PeerTubeServer - state?: FollowState // if not provided, it means it does not exist - } - following: { - server: PeerTubeServer - state?: FollowState // if not provided, it means it does not exist - } -}) { - const { follower, following } = options + follower: PeerTubeServer + followerState: FollowState | 'deleted' - const followerUrl = follower.server.url + '/accounts/peertube' - const followingUrl = following.server.url + '/accounts/peertube' + following: PeerTubeServer + followingState: FollowState | 'deleted' +}) { + const { follower, followerState, followingState, following } = options + + const followerUrl = follower.url + '/accounts/peertube' + const followingUrl = following.url + '/accounts/peertube' const finder = (d: ActorFollow) => d.follower.url === followerUrl && d.following.url === followingUrl { - const { data } = await follower.server.follows.getFollowings() + const { data } = await follower.follows.getFollowings() const follow = data.find(finder) - if (!follower.state) { + if (followerState === 'deleted') { expect(follow).to.not.exist } else { - expect(follow.state).to.equal(follower.state) + expect(follow.state).to.equal(followerState) expect(follow.follower.url).to.equal(followerUrl) expect(follow.following.url).to.equal(followingUrl) } } { - const { data } = await following.server.follows.getFollowers() + const { data } = await following.follows.getFollowers() const follow = data.find(finder) - if (!following.state) { + if (followingState === 'deleted') { expect(follow).to.not.exist } else { - expect(follow.state).to.equal(following.state) + expect(follow.state).to.equal(followingState) expect(follow.follower.url).to.equal(followerUrl) expect(follow.following.url).to.equal(followingUrl) } @@ -256,14 +253,10 @@ describe('Test follows moderation', function () { await waitJobs(servers) await checkFollows({ - follower: { - server: servers[0], - state: 'rejected' - }, - following: { - server: servers[2], - state: 'rejected' - } + follower: servers[0], + followerState: 'rejected', + following: servers[2], + followingState: 'rejected' }) } @@ -279,13 +272,10 @@ describe('Test follows moderation', function () { await waitJobs(servers) await checkFollows({ - follower: { - server: servers[0] - }, - following: { - server: servers[2], - state: 'rejected' - } + follower: servers[0], + followerState: 'deleted', + following: servers[2], + followingState: 'rejected' }) }) @@ -297,14 +287,10 @@ describe('Test follows moderation', function () { await waitJobs(servers) await checkFollows({ - follower: { - server: servers[0], - state: 'pending' - }, - following: { - server: servers[2], - state: 'pending' - } + follower: servers[0], + followerState: 'pending', + following: servers[2], + followingState: 'pending' }) }) @@ -313,14 +299,10 @@ describe('Test follows moderation', function () { await waitJobs(servers) await checkFollows({ - follower: { - server: servers[0], - state: 'rejected' - }, - following: { - server: servers[1], - state: 'rejected' - } + follower: servers[0], + followerState: 'rejected', + following: servers[1], + followingState: 'rejected' }) }) @@ -329,19 +311,36 @@ describe('Test follows moderation', function () { await waitJobs(servers) await checkFollows({ - follower: { - server: servers[0], - state: 'accepted' - }, - following: { - server: servers[1], - state: 'accepted' - } + follower: servers[0], + followerState: 'accepted', + following: servers[1], + followingState: 'accepted' }) }) it('Should ignore follow requests of muted servers', async function () { + await servers[1].blocklist.addToServerBlocklist({ server: servers[0].host }) + await commands[0].unfollow({ target: servers[1] }) + + await waitJobs(servers) + + await checkFollows({ + follower: servers[0], + followerState: 'deleted', + following: servers[1], + followingState: 'deleted' + }) + + await commands[0].follow({ hosts: [ servers[1].host ] }) + await waitJobs(servers) + + await checkFollows({ + follower: servers[0], + followerState: 'rejected', + following: servers[1], + followingState: 'deleted' + }) }) after(async function () {