Add rejected state to follows

Prevent reprocessing already rejected follows
This commit is contained in:
Chocobozzz 2022-07-26 14:46:15 +02:00
parent 0f58b11f5c
commit 927fa4b11f
No known key found for this signature in database
GPG Key ID: 583A612D890159BE
12 changed files with 266 additions and 54 deletions

View File

@ -1,5 +1,6 @@
import express from 'express'
import { getServerActor } from '@server/models/application/application'
import { ServerFollowCreate } from '@shared/models'
import { HttpStatusCode } from '../../../../shared/models/http/http-error-codes'
import { UserRight } from '../../../../shared/models/users'
import { logger } from '../../../helpers/logger'
@ -20,16 +21,16 @@ import {
setDefaultSort
} from '../../../middlewares'
import {
acceptOrRejectFollowerValidator,
instanceFollowersSortValidator,
instanceFollowingSortValidator,
acceptFollowerValidator,
followValidator,
getFollowerValidator,
instanceFollowersSortValidator,
instanceFollowingSortValidator,
listFollowsValidator,
rejectFollowerValidator,
removeFollowingValidator
} from '../../../middlewares/validators'
import { ActorFollowModel } from '../../../models/actor/actor-follow'
import { ServerFollowCreate } from '@shared/models'
const serverFollowsRouter = express.Router()
serverFollowsRouter.get('/following',
@ -69,22 +70,22 @@ serverFollowsRouter.delete('/followers/:nameWithHost',
authenticate,
ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW),
asyncMiddleware(getFollowerValidator),
asyncMiddleware(removeOrRejectFollower)
asyncMiddleware(removeFollower)
)
serverFollowsRouter.post('/followers/:nameWithHost/reject',
authenticate,
ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW),
asyncMiddleware(getFollowerValidator),
acceptOrRejectFollowerValidator,
asyncMiddleware(removeOrRejectFollower)
rejectFollowerValidator,
asyncMiddleware(rejectFollower)
)
serverFollowsRouter.post('/followers/:nameWithHost/accept',
authenticate,
ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW),
asyncMiddleware(getFollowerValidator),
acceptOrRejectFollowerValidator,
acceptFollowerValidator,
asyncMiddleware(acceptFollower)
)
@ -176,10 +177,23 @@ async function removeFollowing (req: express.Request, res: express.Response) {
return res.status(HttpStatusCode.NO_CONTENT_204).end()
}
async function removeOrRejectFollower (req: express.Request, res: express.Response) {
async function rejectFollower (req: express.Request, res: express.Response) {
const follow = res.locals.follow
await sendReject(follow.url, follow.ActorFollower, follow.ActorFollowing)
follow.state = 'rejected'
await follow.save()
sendReject(follow.url, follow.ActorFollower, follow.ActorFollowing)
return res.status(HttpStatusCode.NO_CONTENT_204).end()
}
async function removeFollower (req: express.Request, res: express.Response) {
const follow = res.locals.follow
if (follow.state === 'accepted' || follow.state === 'pending') {
sendReject(follow.url, follow.ActorFollower, follow.ActorFollowing)
}
await follow.destroy()

View File

@ -4,7 +4,7 @@ import { FollowState } from '@shared/models'
function isFollowStateValid (value: FollowState) {
if (!exists(value)) return false
return value === 'pending' || value === 'accepted'
return value === 'pending' || value === 'accepted' || value === 'rejected'
}
function isRemoteHandleValid (value: string) {

View File

@ -129,7 +129,8 @@ const ACTOR_FOLLOW_SCORE = {
const FOLLOW_STATES: { [ id: string ]: FollowState } = {
PENDING: 'pending',
ACCEPTED: 'accepted'
ACCEPTED: 'accepted',
REJECTED: 'rejected'
}
const REMOTE_SCHEME = {

View File

@ -58,6 +58,11 @@ async function processFollow (byActor: MActorSignature, activityId: string, targ
transaction: t
})
// Already rejected
if (actorFollow.state === 'rejected') {
return { actorFollow: undefined as MActorFollowActors }
}
// 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)) {

View File

@ -25,7 +25,8 @@ async function processReject (follower: MActor, targetActor: MActor) {
if (!actorFollow) throw new Error(`'Unknown actor follow ${follower.id} -> ${targetActor.id}.`)
await actorFollow.destroy({ transaction: t })
actorFollow.state = 'rejected'
await actorFollow.save({ transaction: t })
return undefined
})

View File

@ -81,7 +81,11 @@ const removeFollowingValidator = [
const serverActor = await getServerActor()
const { name, host } = getRemoteNameAndHost(req.params.hostOrHandle)
const follow = await ActorFollowModel.loadByActorAndTargetNameAndHostForAPI(serverActor.id, name, host)
const follow = await ActorFollowModel.loadByActorAndTargetNameAndHostForAPI({
actorId: serverActor.id,
targetName: name,
targetHost: host
})
if (!follow) {
return res.fail({
@ -126,13 +130,26 @@ const getFollowerValidator = [
}
]
const acceptOrRejectFollowerValidator = [
const acceptFollowerValidator = [
(req: express.Request, res: express.Response, next: express.NextFunction) => {
logger.debug('Checking accept/reject follower parameters', { parameters: req.params })
logger.debug('Checking accept follower parameters', { parameters: req.params })
const follow = res.locals.follow
if (follow.state !== 'pending') {
return res.fail({ message: 'Follow is not in pending state.' })
if (follow.state !== 'pending' && follow.state !== 'rejected') {
return res.fail({ message: 'Follow is not in pending/rejected state.' })
}
return next()
}
]
const rejectFollowerValidator = [
(req: express.Request, res: express.Response, next: express.NextFunction) => {
logger.debug('Checking reject follower parameters', { parameters: req.params })
const follow = res.locals.follow
if (follow.state !== 'pending' && follow.state !== 'accepted') {
return res.fail({ message: 'Follow is not in pending/accepted state.' })
}
return next()
@ -145,6 +162,7 @@ export {
followValidator,
removeFollowingValidator,
getFollowerValidator,
acceptOrRejectFollowerValidator,
acceptFollowerValidator,
rejectFollowerValidator,
listFollowsValidator
}

View File

@ -58,7 +58,12 @@ const userSubscriptionGetValidator = [
if (host === WEBSERVER.HOST) host = null
const user = res.locals.oauth.token.User
const subscription = await ActorFollowModel.loadByActorAndTargetNameAndHostForAPI(user.Account.Actor.id, name, host)
const subscription = await ActorFollowModel.loadByActorAndTargetNameAndHostForAPI({
actorId: user.Account.Actor.id,
targetName: name,
targetHost: host,
state: 'accepted'
})
if (!subscription || !subscription.ActorFollowing.VideoChannel) {
return res.fail({

View File

@ -1,5 +1,5 @@
import { difference, values } from 'lodash'
import { Includeable, IncludeOptions, Op, QueryTypes, Transaction } from 'sequelize'
import { Attributes, FindOptions, Includeable, IncludeOptions, Op, QueryTypes, Transaction, WhereAttributeHash } from 'sequelize'
import {
AfterCreate,
AfterDestroy,
@ -209,7 +209,9 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
}
static isFollowedBy (actorId: number, followerActorId: number) {
const query = 'SELECT 1 FROM "actorFollow" WHERE "actorId" = $followerActorId AND "targetActorId" = $actorId LIMIT 1'
const query = `SELECT 1 FROM "actorFollow" ` +
`WHERE "actorId" = $followerActorId AND "targetActorId" = $actorId AND "state" = 'accepted' ` +
`LIMIT 1`
return doesExist(query, { actorId, followerActorId })
}
@ -238,12 +240,15 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
return ActorFollowModel.findOne(query)
}
static loadByActorAndTargetNameAndHostForAPI (
actorId: number,
targetName: string,
targetHost: string,
t?: Transaction
): Promise<MActorFollowActorsDefaultSubscription> {
static loadByActorAndTargetNameAndHostForAPI (options: {
actorId: number
targetName: string
targetHost: string
state?: FollowState
transaction?: Transaction
}): Promise<MActorFollowActorsDefaultSubscription> {
const { actorId, targetHost, targetName, state, transaction } = options
const actorFollowingPartInclude: IncludeOptions = {
model: ActorModel,
required: true,
@ -271,10 +276,11 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
})
}
const query = {
where: {
actorId
},
const where: WhereAttributeHash<Attributes<ActorFollowModel>> = { actorId}
if (state) where.state = state
const query: FindOptions<Attributes<ActorFollowModel>> = {
where,
include: [
actorFollowingPartInclude,
{
@ -283,7 +289,7 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
as: 'ActorFollower'
}
],
transaction: t
transaction
}
return ActorFollowModel.findOne(query)
@ -325,6 +331,7 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
[Op.or]: whereTab
},
{
state: 'accepted',
actorId
}
]
@ -372,6 +379,7 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
}) {
const { actorId, start, count, sort } = options
const where = {
state: 'accepted',
actorId
}
@ -512,13 +520,15 @@ export class ActorFollowModel extends Model<Partial<AttributesOnly<ActorFollowMo
const totalInstanceFollowing = await ActorFollowModel.count({
where: {
actorId: serverActor.id
actorId: serverActor.id,
state: 'accepted'
}
})
const totalInstanceFollowers = await ActorFollowModel.count({
where: {
targetActorId: serverActor.id
targetActorId: serverActor.id,
state: 'accepted'
}
})

View File

@ -462,7 +462,7 @@ export class ActorModel extends Model<Partial<AttributesOnly<ActorModel>>> {
}
return ActorModel.update({
[columnToUpdate]: literal(`(SELECT COUNT(*) FROM "actorFollow" WHERE "${columnOfCount}" = ${sanitizedOfId})`)
[columnToUpdate]: literal(`(SELECT COUNT(*) FROM "actorFollow" WHERE "${columnOfCount}" = ${sanitizedOfId} AND "state" = 'accepted')`)
}, { where, transaction })
}

View File

@ -2,6 +2,8 @@
import 'mocha'
import * as chai from 'chai'
import { expectStartWith } from '@server/tests/shared'
import { ActorFollow, FollowState } from '@shared/models'
import {
cleanupTests,
createMultipleServers,
@ -25,8 +27,51 @@ async function checkServer1And2HasFollowers (servers: PeerTubeServer[], state =
const follow = body.data[0]
expect(follow.state).to.equal(state)
expect(follow.follower.url).to.equal('http://localhost:' + servers[0].port + '/accounts/peertube')
expect(follow.following.url).to.equal('http://localhost:' + servers[1].port + '/accounts/peertube')
expect(follow.follower.url).to.equal(servers[0].url + '/accounts/peertube')
expect(follow.following.url).to.equal(servers[1].url + '/accounts/peertube')
}
}
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
const followerUrl = follower.server.url + '/accounts/peertube'
const followingUrl = following.server.url + '/accounts/peertube'
const finder = (d: ActorFollow) => d.follower.url === followerUrl && d.following.url === followingUrl
{
const { data } = await follower.server.follows.getFollowings()
const follow = data.find(finder)
if (!follower.state) {
expect(follow).to.not.exist
} else {
expect(follow.state).to.equal(follower.state)
expect(follow.follower.url).to.equal(followerUrl)
expect(follow.following.url).to.equal(followingUrl)
}
}
{
const { data } = await following.server.follows.getFollowers()
const follow = data.find(finder)
if (!following.state) {
expect(follow).to.not.exist
} else {
expect(follow.state).to.equal(following.state)
expect(follow.follower.url).to.equal(followerUrl)
expect(follow.following.url).to.equal(followingUrl)
}
}
}
@ -37,7 +82,7 @@ async function checkNoFollowers (servers: PeerTubeServer[]) {
]
for (const fn of fns) {
const body = await fn({ start: 0, count: 5, sort: 'createdAt' })
const body = await fn({ start: 0, count: 5, sort: 'createdAt', state: 'accepted' })
expect(body.total).to.equal(0)
}
}
@ -124,7 +169,7 @@ describe('Test follows moderation', function () {
it('Should manually approve followers', async function () {
this.timeout(20000)
await commands[1].removeFollower({ follower: servers[0] })
await commands[0].unfollow({ target: servers[1] })
await waitJobs(servers)
const subConfig = {
@ -148,7 +193,7 @@ describe('Test follows moderation', function () {
it('Should accept a follower', async function () {
this.timeout(10000)
await commands[1].acceptFollower({ follower: 'peertube@localhost:' + servers[0].port })
await commands[1].acceptFollower({ follower: 'peertube@' + servers[0].host })
await waitJobs(servers)
await checkServer1And2HasFollowers(servers)
@ -161,29 +206,142 @@ describe('Test follows moderation', function () {
await waitJobs(servers)
{
const body = await commands[0].getFollowings({ start: 0, count: 5, sort: 'createdAt' })
const body = await commands[0].getFollowings()
expect(body.total).to.equal(2)
}
{
const body = await commands[1].getFollowers({ start: 0, count: 5, sort: 'createdAt' })
const body = await commands[1].getFollowers()
expect(body.total).to.equal(1)
}
{
const body = await commands[2].getFollowers({ start: 0, count: 5, sort: 'createdAt' })
const body = await commands[2].getFollowers()
expect(body.total).to.equal(1)
}
await commands[2].rejectFollower({ follower: 'peertube@localhost:' + servers[0].port })
await commands[2].rejectFollower({ follower: 'peertube@' + servers[0].host })
await waitJobs(servers)
await checkServer1And2HasFollowers(servers)
{ // server 1
{
const { data } = await commands[0].getFollowings({ state: 'accepted' })
expect(data).to.have.lengthOf(1)
}
{
const body = await commands[2].getFollowers({ start: 0, count: 5, sort: 'createdAt' })
expect(body.total).to.equal(0)
{
const { data } = await commands[0].getFollowings({ state: 'rejected' })
expect(data).to.have.lengthOf(1)
expectStartWith(data[0].following.url, servers[2].url)
}
}
{ // server 3
{
const { data } = await commands[2].getFollowers({ state: 'accepted' })
expect(data).to.have.lengthOf(0)
}
{
const { data } = await commands[2].getFollowers({ state: 'rejected' })
expect(data).to.have.lengthOf(1)
expectStartWith(data[0].follower.url, servers[0].url)
}
}
})
it('Should not change the follow on refollow with and without auto accept', async function () {
const run = async () => {
await commands[0].follow({ hosts: [ servers[2].url ] })
await waitJobs(servers)
await checkFollows({
follower: {
server: servers[0],
state: 'rejected'
},
following: {
server: servers[2],
state: 'rejected'
}
})
}
await servers[2].config.updateExistingSubConfig({ newConfig: { followers: { instance: { manualApproval: false } } } })
await run()
await servers[2].config.updateExistingSubConfig({ newConfig: { followers: { instance: { manualApproval: true } } } })
await run()
})
it('Should not change the rejected status on unfollow', async function () {
await commands[0].unfollow({ target: servers[2] })
await waitJobs(servers)
await checkFollows({
follower: {
server: servers[0]
},
following: {
server: servers[2],
state: 'rejected'
}
})
})
it('Should delete the follower and add again the follower', async function () {
await commands[2].removeFollower({ follower: servers[0] })
await waitJobs(servers)
await commands[0].follow({ hosts: [ servers[2].url ] })
await waitJobs(servers)
await checkFollows({
follower: {
server: servers[0],
state: 'pending'
},
following: {
server: servers[2],
state: 'pending'
}
})
})
it('Should be able to reject a previously accepted follower', async function () {
await commands[1].rejectFollower({ follower: 'peertube@' + servers[0].host })
await waitJobs(servers)
await checkFollows({
follower: {
server: servers[0],
state: 'rejected'
},
following: {
server: servers[1],
state: 'rejected'
}
})
})
it('Should be able to re accept a previously rejected follower', async function () {
await commands[1].acceptFollower({ follower: 'peertube@' + servers[0].host })
await waitJobs(servers)
await checkFollows({
follower: {
server: servers[0],
state: 'accepted'
},
following: {
server: servers[1],
state: 'accepted'
}
})
})
it('Should ignore follow requests of muted servers', async function () {
})
after(async function () {

View File

@ -1,6 +1,6 @@
import { Actor } from './actor.model'
export type FollowState = 'pending' | 'accepted'
export type FollowState = 'pending' | 'accepted' | 'rejected'
export interface ActorFollow {
id: number

View File

@ -6,13 +6,13 @@ import { PeerTubeServer } from './server'
export class FollowsCommand extends AbstractCommand {
getFollowers (options: OverrideCommandOptions & {
start: number
count: number
sort: string
start?: number
count?: number
sort?: string
search?: string
actorType?: ActivityPubActorType
state?: FollowState
}) {
} = {}) {
const path = '/api/v1/server/followers'
const query = pick(options, [ 'start', 'count', 'sort', 'search', 'state', 'actorType' ])