From 14893eb71cb2d4ca47e07589c81958863603aba4 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 8 Apr 2019 15:18:04 +0200 Subject: [PATCH] Add ability to manually approves instance followers in REST API --- config/default.yaml | 2 + config/production.yaml.example | 2 + server/controllers/api/config.ts | 3 +- server/controllers/api/server/follows.ts | 38 +++++++-- server/initializers/checker-before-init.ts | 2 +- server/initializers/constants.ts | 3 +- .../lib/activitypub/process/process-follow.ts | 8 +- server/middlewares/validators/config.ts | 3 + server/middlewares/validators/follows.ts | 20 ++++- server/tests/api/check-params/config.ts | 3 +- server/tests/api/check-params/follows.ts | 80 ++++++++++++++++++ server/tests/api/server/config.ts | 5 +- server/tests/api/server/follows-moderation.ts | 83 +++++++++++++++++-- shared/models/server/custom-config.model.ts | 3 +- shared/utils/server/config.ts | 3 +- shared/utils/server/follows.ts | 33 ++++++-- 16 files changed, 261 insertions(+), 30 deletions(-) diff --git a/config/default.yaml b/config/default.yaml index 51f3ad833..0d6e34d86 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -205,3 +205,5 @@ followers: instance: # Allow or not other instances to follow yours enabled: true + # Whether or not an administrator must manually validate a new follower + manual_approval: false diff --git a/config/production.yaml.example b/config/production.yaml.example index a2811abd6..5029cc25b 100644 --- a/config/production.yaml.example +++ b/config/production.yaml.example @@ -222,3 +222,5 @@ followers: instance: # Allow or not other instances to follow yours enabled: true + # Whether or not an administrator must manually validate a new follower + manual_approval: false diff --git a/server/controllers/api/config.ts b/server/controllers/api/config.ts index f9bb0b947..5c4f455ee 100644 --- a/server/controllers/api/config.ts +++ b/server/controllers/api/config.ts @@ -282,7 +282,8 @@ function customConfig (): CustomConfig { }, followers: { instance: { - enabled: CONFIG.FOLLOWERS.INSTANCE.ENABLED + enabled: CONFIG.FOLLOWERS.INSTANCE.ENABLED, + manualApproval: CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL } } } diff --git a/server/controllers/api/server/follows.ts b/server/controllers/api/server/follows.ts index 87cf091cb..207a09a4c 100644 --- a/server/controllers/api/server/follows.ts +++ b/server/controllers/api/server/follows.ts @@ -3,7 +3,7 @@ import { UserRight } from '../../../../shared/models/users' import { logger } from '../../../helpers/logger' import { getFormattedObjects, getServerActor } from '../../../helpers/utils' import { sequelizeTypescript, SERVER_ACTOR_NAME } from '../../../initializers' -import { sendReject, sendUndoFollow } from '../../../lib/activitypub/send' +import { sendAccept, sendReject, sendUndoFollow } from '../../../lib/activitypub/send' import { asyncMiddleware, authenticate, @@ -14,10 +14,11 @@ import { setDefaultSort } from '../../../middlewares' import { + acceptOrRejectFollowerValidator, followersSortValidator, followingSortValidator, followValidator, - removeFollowerValidator, + getFollowerValidator, removeFollowingValidator } from '../../../middlewares/validators' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' @@ -59,8 +60,24 @@ serverFollowsRouter.get('/followers', serverFollowsRouter.delete('/followers/:nameWithHost', authenticate, ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW), - asyncMiddleware(removeFollowerValidator), - asyncMiddleware(removeFollower) + asyncMiddleware(getFollowerValidator), + asyncMiddleware(removeOrRejectFollower) +) + +serverFollowsRouter.post('/followers/:nameWithHost/reject', + authenticate, + ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW), + asyncMiddleware(getFollowerValidator), + acceptOrRejectFollowerValidator, + asyncMiddleware(removeOrRejectFollower) +) + +serverFollowsRouter.post('/followers/:nameWithHost/accept', + authenticate, + ensureUserHasRight(UserRight.MANAGE_SERVER_FOLLOW), + asyncMiddleware(getFollowerValidator), + acceptOrRejectFollowerValidator, + asyncMiddleware(acceptFollower) ) // --------------------------------------------------------------------------- @@ -136,7 +153,7 @@ async function removeFollowing (req: express.Request, res: express.Response) { return res.status(204).end() } -async function removeFollower (req: express.Request, res: express.Response) { +async function removeOrRejectFollower (req: express.Request, res: express.Response) { const follow = res.locals.follow await sendReject(follow.ActorFollower, follow.ActorFollowing) @@ -145,3 +162,14 @@ async function removeFollower (req: express.Request, res: express.Response) { return res.status(204).end() } + +async function acceptFollower (req: express.Request, res: express.Response) { + const follow = res.locals.follow + + await sendAccept(follow) + + follow.state = 'accepted' + await follow.save() + + return res.status(204).end() +} diff --git a/server/initializers/checker-before-init.ts b/server/initializers/checker-before-init.ts index a9896907d..3095913a3 100644 --- a/server/initializers/checker-before-init.ts +++ b/server/initializers/checker-before-init.ts @@ -25,7 +25,7 @@ function checkMissedConfig () { 'instance.name', 'instance.short_description', 'instance.description', 'instance.terms', 'instance.default_client_route', 'instance.is_nsfw', 'instance.default_nsfw_policy', 'instance.robots', 'instance.securitytxt', 'services.twitter.username', 'services.twitter.whitelisted', - 'followers.instance.enabled' + 'followers.instance.enabled', 'followers.instance.manual_approval' ] const requiredAlternatives = [ [ // set diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 43c5ec54c..7d9ffc668 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -327,7 +327,8 @@ const CONFIG = { }, FOLLOWERS: { INSTANCE: { - get ENABLED () { return config.get('followers.instance.enabled') } + get ENABLED () { return config.get('followers.instance.enabled') }, + get MANUAL_APPROVAL () { return config.get('followers.instance.manual_approval') } } } } diff --git a/server/lib/activitypub/process/process-follow.ts b/server/lib/activitypub/process/process-follow.ts index cecf09b47..140bbe9f1 100644 --- a/server/lib/activitypub/process/process-follow.ts +++ b/server/lib/activitypub/process/process-follow.ts @@ -32,6 +32,8 @@ async function processFollow (actor: ActorModel, targetActorURL: string) { const serverActor = await getServerActor() if (targetActor.id === serverActor.id && CONFIG.FOLLOWERS.INSTANCE.ENABLED === false) { + logger.info('Rejecting %s because instance followers are disabled.', targetActor.url) + return sendReject(actor, targetActor) } @@ -43,7 +45,7 @@ async function processFollow (actor: ActorModel, targetActorURL: string) { defaults: { actorId: actor.id, targetActorId: targetActor.id, - state: 'accepted' + state: CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL ? 'pending' : 'accepted' }, transaction: t }) @@ -51,7 +53,7 @@ async function processFollow (actor: ActorModel, targetActorURL: string) { actorFollow.ActorFollower = actor actorFollow.ActorFollowing = targetActor - if (actorFollow.state !== 'accepted') { + if (actorFollow.state !== 'accepted' && CONFIG.FOLLOWERS.INSTANCE.MANUAL_APPROVAL === false) { actorFollow.state = 'accepted' await actorFollow.save({ transaction: t }) } @@ -60,7 +62,7 @@ async function processFollow (actor: ActorModel, targetActorURL: string) { actorFollow.ActorFollowing = targetActor // Target sends to actor he accepted the follow request - await sendAccept(actorFollow) + if (actorFollow.state === 'accepted') await sendAccept(actorFollow) return { actorFollow, created } }) diff --git a/server/middlewares/validators/config.ts b/server/middlewares/validators/config.ts index 270ce66f6..d015fa6fe 100644 --- a/server/middlewares/validators/config.ts +++ b/server/middlewares/validators/config.ts @@ -44,6 +44,9 @@ const customConfigUpdateValidator = [ body('import.videos.http.enabled').isBoolean().withMessage('Should have a valid import video http enabled boolean'), body('import.videos.torrent.enabled').isBoolean().withMessage('Should have a valid import video torrent enabled boolean'), + body('followers.instance.enabled').isBoolean().withMessage('Should have a valid followers of instance boolean'), + body('followers.instance.manualApproval').isBoolean().withMessage('Should have a valid manual approval boolean'), + async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking customConfigUpdateValidator parameters', { parameters: req.body }) diff --git a/server/middlewares/validators/follows.ts b/server/middlewares/validators/follows.ts index 38df39fda..b360cf95e 100644 --- a/server/middlewares/validators/follows.ts +++ b/server/middlewares/validators/follows.ts @@ -57,11 +57,11 @@ const removeFollowingValidator = [ } ] -const removeFollowerValidator = [ +const getFollowerValidator = [ param('nameWithHost').custom(isValidActorHandle).withMessage('Should have a valid nameWithHost'), async (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking remove follower parameters', { parameters: req.params }) + logger.debug('Checking get follower parameters', { parameters: req.params }) if (areValidationErrors(req, res)) return @@ -90,10 +90,24 @@ const removeFollowerValidator = [ } ] +const acceptOrRejectFollowerValidator = [ + (req: express.Request, res: express.Response, next: express.NextFunction) => { + logger.debug('Checking accept/reject follower parameters', { parameters: req.params }) + + const follow = res.locals.follow + if (follow.state !== 'pending') { + return res.status(400).json({ error: 'Follow is not in pending state.' }).end() + } + + return next() + } +] + // --------------------------------------------------------------------------- export { followValidator, removeFollowingValidator, - removeFollowerValidator + getFollowerValidator, + acceptOrRejectFollowerValidator } diff --git a/server/tests/api/check-params/config.ts b/server/tests/api/check-params/config.ts index d117f26e6..01ab84584 100644 --- a/server/tests/api/check-params/config.ts +++ b/server/tests/api/check-params/config.ts @@ -90,7 +90,8 @@ describe('Test config API validators', function () { }, followers: { instance: { - enabled: false + enabled: false, + manualApproval: true } } } diff --git a/server/tests/api/check-params/follows.ts b/server/tests/api/check-params/follows.ts index 67fa43778..ed1d2db59 100644 --- a/server/tests/api/check-params/follows.ts +++ b/server/tests/api/check-params/follows.ts @@ -184,6 +184,86 @@ describe('Test server follows API validators', function () { }) }) + describe('When accepting a follower', function () { + const path = '/api/v1/server/followers' + + it('Should fail with an invalid token', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9002/accept', + token: 'fake_token', + statusCodeExpected: 401 + }) + }) + + it('Should fail if the user is not an administrator', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9002/accept', + token: userAccessToken, + statusCodeExpected: 403 + }) + }) + + it('Should fail with an invalid follower', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto/accept', + token: server.accessToken, + statusCodeExpected: 400 + }) + }) + + it('Should fail with an unknown follower', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9003/accept', + token: server.accessToken, + statusCodeExpected: 404 + }) + }) + }) + + describe('When rejecting a follower', function () { + const path = '/api/v1/server/followers' + + it('Should fail with an invalid token', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9002/reject', + token: 'fake_token', + statusCodeExpected: 401 + }) + }) + + it('Should fail if the user is not an administrator', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9002/reject', + token: userAccessToken, + statusCodeExpected: 403 + }) + }) + + it('Should fail with an invalid follower', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto/reject', + token: server.accessToken, + statusCodeExpected: 400 + }) + }) + + it('Should fail with an unknown follower', async function () { + await makePostBodyRequest({ + url: server.url, + path: path + '/toto@localhost:9003/reject', + token: server.accessToken, + statusCodeExpected: 404 + }) + }) + }) + describe('When removing following', function () { const path = '/api/v1/server/following' diff --git a/server/tests/api/server/config.ts b/server/tests/api/server/config.ts index cb2700f29..5373d02f2 100644 --- a/server/tests/api/server/config.ts +++ b/server/tests/api/server/config.ts @@ -65,6 +65,7 @@ function checkInitialConfig (data: CustomConfig) { expect(data.autoBlacklist.videos.ofUsers.enabled).to.be.false expect(data.followers.instance.enabled).to.be.true + expect(data.followers.instance.manualApproval).to.be.false } function checkUpdatedConfig (data: CustomConfig) { @@ -109,6 +110,7 @@ function checkUpdatedConfig (data: CustomConfig) { expect(data.autoBlacklist.videos.ofUsers.enabled).to.be.true expect(data.followers.instance.enabled).to.be.false + expect(data.followers.instance.manualApproval).to.be.true } describe('Test config', function () { @@ -241,7 +243,8 @@ describe('Test config', function () { }, followers: { instance: { - enabled: false + enabled: false, + manualApproval: true } } } diff --git a/server/tests/api/server/follows-moderation.ts b/server/tests/api/server/follows-moderation.ts index a360706f2..0bb3aa866 100644 --- a/server/tests/api/server/follows-moderation.ts +++ b/server/tests/api/server/follows-moderation.ts @@ -3,6 +3,7 @@ import * as chai from 'chai' import 'mocha' import { + acceptFollower, flushAndRunMultipleServers, killallServers, ServerInfo, @@ -13,19 +14,21 @@ import { follow, getFollowersListPaginationAndSort, getFollowingListPaginationAndSort, - removeFollower + removeFollower, + rejectFollower } from '../../../../shared/utils/server/follows' import { waitJobs } from '../../../../shared/utils/server/jobs' import { ActorFollow } from '../../../../shared/models/actors' const expect = chai.expect -async function checkHasFollowers (servers: ServerInfo[]) { +async function checkServer1And2HasFollowers (servers: ServerInfo[], state = 'accepted') { { const res = await getFollowingListPaginationAndSort(servers[0].url, 0, 5, 'createdAt') expect(res.body.total).to.equal(1) const follow = res.body.data[0] as ActorFollow + expect(follow.state).to.equal(state) expect(follow.follower.url).to.equal('http://localhost:9001/accounts/peertube') expect(follow.following.url).to.equal('http://localhost:9002/accounts/peertube') } @@ -35,6 +38,7 @@ async function checkHasFollowers (servers: ServerInfo[]) { expect(res.body.total).to.equal(1) const follow = res.body.data[0] as ActorFollow + expect(follow.state).to.equal(state) expect(follow.follower.url).to.equal('http://localhost:9001/accounts/peertube') expect(follow.following.url).to.equal('http://localhost:9002/accounts/peertube') } @@ -58,7 +62,7 @@ describe('Test follows moderation', function () { before(async function () { this.timeout(30000) - servers = await flushAndRunMultipleServers(2) + servers = await flushAndRunMultipleServers(3) // Get the access tokens await setAccessTokensToServers(servers) @@ -73,7 +77,7 @@ describe('Test follows moderation', function () { }) it('Should have correct follows', async function () { - await checkHasFollowers(servers) + await checkServer1And2HasFollowers(servers) }) it('Should remove follower on server 2', async function () { @@ -90,7 +94,8 @@ describe('Test follows moderation', function () { const subConfig = { followers: { instance: { - enabled: false + enabled: false, + manualApproval: false } } } @@ -107,7 +112,8 @@ describe('Test follows moderation', function () { const subConfig = { followers: { instance: { - enabled: true + enabled: true, + manualApproval: false } } } @@ -117,7 +123,70 @@ describe('Test follows moderation', function () { await follow(servers[0].url, [ servers[1].url ], servers[0].accessToken) await waitJobs(servers) - await checkHasFollowers(servers) + await checkServer1And2HasFollowers(servers) + }) + + it('Should manually approve followers', async function () { + this.timeout(20000) + + await removeFollower(servers[1].url, servers[1].accessToken, servers[0]) + await waitJobs(servers) + + const subConfig = { + followers: { + instance: { + enabled: true, + manualApproval: true + } + } + } + + await updateCustomSubConfig(servers[1].url, servers[1].accessToken, subConfig) + await updateCustomSubConfig(servers[2].url, servers[2].accessToken, subConfig) + + await follow(servers[0].url, [ servers[1].url ], servers[0].accessToken) + await waitJobs(servers) + + await checkServer1And2HasFollowers(servers, 'pending') + }) + + it('Should accept a follower', async function () { + await acceptFollower(servers[1].url, servers[1].accessToken, 'peertube@localhost:9001') + await waitJobs(servers) + + await checkServer1And2HasFollowers(servers) + }) + + it('Should reject another follower', async function () { + this.timeout(20000) + + await follow(servers[0].url, [ servers[2].url ], servers[0].accessToken) + await waitJobs(servers) + + { + const res = await getFollowingListPaginationAndSort(servers[0].url, 0, 5, 'createdAt') + expect(res.body.total).to.equal(2) + } + + { + const res = await getFollowersListPaginationAndSort(servers[1].url, 0, 5, 'createdAt') + expect(res.body.total).to.equal(1) + } + + { + const res = await getFollowersListPaginationAndSort(servers[2].url, 0, 5, 'createdAt') + expect(res.body.total).to.equal(1) + } + + await rejectFollower(servers[2].url, servers[2].accessToken, 'peertube@localhost:9001') + await waitJobs(servers) + + await checkServer1And2HasFollowers(servers) + + { + const res = await getFollowersListPaginationAndSort(servers[ 2 ].url, 0, 5, 'createdAt') + expect(res.body.total).to.equal(0) + } }) after(async function () { diff --git a/shared/models/server/custom-config.model.ts b/shared/models/server/custom-config.model.ts index 642ffea39..ca52eff4b 100644 --- a/shared/models/server/custom-config.model.ts +++ b/shared/models/server/custom-config.model.ts @@ -88,7 +88,8 @@ export interface CustomConfig { followers: { instance: { - enabled: boolean + enabled: boolean, + manualApproval: boolean } } diff --git a/shared/utils/server/config.ts b/shared/utils/server/config.ts index 21c689714..deb77e9c0 100644 --- a/shared/utils/server/config.ts +++ b/shared/utils/server/config.ts @@ -122,7 +122,8 @@ function updateCustomSubConfig (url: string, token: string, newConfig: any) { }, followers: { instance: { - enabled: true + enabled: true, + manualApproval: false } } } diff --git a/shared/utils/server/follows.ts b/shared/utils/server/follows.ts index 949fd8109..1505804de 100644 --- a/shared/utils/server/follows.ts +++ b/shared/utils/server/follows.ts @@ -1,6 +1,7 @@ import * as request from 'supertest' import { ServerInfo } from './servers' import { waitJobs } from './jobs' +import { makeGetRequest, makePostBodyRequest } from '..' function getFollowersListPaginationAndSort (url: string, start: number, count: number, sort: string, search?: string) { const path = '/api/v1/server/followers' @@ -16,6 +17,28 @@ function getFollowersListPaginationAndSort (url: string, start: number, count: n .expect('Content-Type', /json/) } +function acceptFollower (url: string, token: string, follower: string, statusCodeExpected = 204) { + const path = '/api/v1/server/followers/' + follower + '/accept' + + return makePostBodyRequest({ + url, + token, + path, + statusCodeExpected + }) +} + +function rejectFollower (url: string, token: string, follower: string, statusCodeExpected = 204) { + const path = '/api/v1/server/followers/' + follower + '/reject' + + return makePostBodyRequest({ + url, + token, + path, + statusCodeExpected + }) +} + function getFollowingListPaginationAndSort (url: string, start: number, count: number, sort: string, search?: string) { const path = '/api/v1/server/following' @@ -30,18 +53,16 @@ function getFollowingListPaginationAndSort (url: string, start: number, count: n .expect('Content-Type', /json/) } -async function follow (follower: string, following: string[], accessToken: string, expectedStatus = 204) { +function follow (follower: string, following: string[], accessToken: string, expectedStatus = 204) { const path = '/api/v1/server/following' const followingHosts = following.map(f => f.replace(/^http:\/\//, '')) - const res = await request(follower) + return request(follower) .post(path) .set('Accept', 'application/json') .set('Authorization', 'Bearer ' + accessToken) .send({ 'hosts': followingHosts }) .expect(expectedStatus) - - return res } async function unfollow (url: string, accessToken: string, target: ServerInfo, expectedStatus = 204) { @@ -84,5 +105,7 @@ export { unfollow, removeFollower, follow, - doubleFollow + doubleFollow, + acceptFollower, + rejectFollower } -- 2.25.1