From 8d4273463fb19d503b1aa0a32dc289f292ed614e Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 15:02:48 +0100 Subject: [PATCH] Check follow constraints when getting a video --- .../+video-watch/video-watch.component.ts | 2 +- config/default.yaml | 5 +- config/production.yaml.example | 5 +- server/controllers/api/videos/index.ts | 2 + server/middlewares/oauth.ts | 16 ++ .../middlewares/validators/videos/videos.ts | 52 ++++- server/models/video/video.ts | 17 ++ server/tests/api/server/follow-constraints.ts | 215 ++++++++++++++++++ server/tests/api/server/index.ts | 1 + 9 files changed, 301 insertions(+), 14 deletions(-) create mode 100644 server/tests/api/server/follow-constraints.ts diff --git a/client/src/app/videos/+video-watch/video-watch.component.ts b/client/src/app/videos/+video-watch/video-watch.component.ts index d0151ceb1..09ee96bdc 100644 --- a/client/src/app/videos/+video-watch/video-watch.component.ts +++ b/client/src/app/videos/+video-watch/video-watch.component.ts @@ -114,7 +114,7 @@ export class VideoWatchComponent implements OnInit, OnDestroy { ) .pipe( // If 401, the video is private or blacklisted so redirect to 404 - catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ 400, 401, 404 ])) + catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ 400, 401, 403, 404 ])) ) .subscribe(([ video, captionsResult ]) => { const startTime = this.route.snapshot.queryParams.start diff --git a/config/default.yaml b/config/default.yaml index 0d7d948c2..257ec7ed1 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -58,7 +58,10 @@ log: level: 'info' # debug/info/warning/error search: - remote_uri: # Add ability to fetch remote videos/actors by their URI, that may not be federated with your instance + # Add ability to fetch remote videos/actors by their URI, that may not be federated with your instance + # If enabled, the associated group will be able to "escape" from the instance follows + # That means they will be able to follow channels, watch videos, list videos of non followed instances + remote_uri: users: true anonymous: false diff --git a/config/production.yaml.example b/config/production.yaml.example index f9da8e0dd..ac15fc736 100644 --- a/config/production.yaml.example +++ b/config/production.yaml.example @@ -59,7 +59,10 @@ log: level: 'info' # debug/info/warning/error search: - remote_uri: # Add ability to search remote videos/actors by URI, that may not be federated with your instance + # Add ability to fetch remote videos/actors by their URI, that may not be federated with your instance + # If enabled, the associated group will be able to "escape" from the instance follows + # That means they will be able to follow channels, watch videos, list videos of non followed instances + remote_uri: users: true anonymous: false diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index e654bdd09..89fd0432f 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -31,6 +31,7 @@ import { asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, + checkVideoFollowConstraints, commonVideosFiltersValidator, optionalAuthenticate, paginationValidator, @@ -123,6 +124,7 @@ videosRouter.get('/:id/description', videosRouter.get('/:id', optionalAuthenticate, asyncMiddleware(videosGetValidator), + asyncMiddleware(checkVideoFollowConstraints), getVideo ) videosRouter.post('/:id/views', diff --git a/server/middlewares/oauth.ts b/server/middlewares/oauth.ts index 5233b66bd..8c1df2c3e 100644 --- a/server/middlewares/oauth.ts +++ b/server/middlewares/oauth.ts @@ -28,9 +28,24 @@ function authenticate (req: express.Request, res: express.Response, next: expres }) } +function authenticatePromiseIfNeeded (req: express.Request, res: express.Response) { + return new Promise(resolve => { + // Already authenticated? (or tried to) + if (res.locals.oauth && res.locals.oauth.token.User) return resolve() + + if (res.locals.authenticated === false) return res.sendStatus(401) + + authenticate(req, res, () => { + return resolve() + }) + }) +} + function optionalAuthenticate (req: express.Request, res: express.Response, next: express.NextFunction) { if (req.header('authorization')) return authenticate(req, res, next) + res.locals.authenticated = false + return next() } @@ -53,6 +68,7 @@ function token (req: express.Request, res: express.Response, next: express.NextF export { authenticate, + authenticatePromiseIfNeeded, optionalAuthenticate, token } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index bf21bca8c..051a19e16 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -31,8 +31,8 @@ import { } from '../../../helpers/custom-validators/videos' import { getDurationFromVideoFile } from '../../../helpers/ffmpeg-utils' import { logger } from '../../../helpers/logger' -import { CONSTRAINTS_FIELDS } from '../../../initializers' -import { authenticate } from '../../oauth' +import { CONFIG, CONSTRAINTS_FIELDS } from '../../../initializers' +import { authenticatePromiseIfNeeded } from '../../oauth' import { areValidationErrors } from '../utils' import { cleanUpReqFiles } from '../../../helpers/express-utils' import { VideoModel } from '../../../models/video/video' @@ -43,6 +43,7 @@ import { VideoChangeOwnershipModel } from '../../../models/video/video-change-ow import { AccountModel } from '../../../models/account/account' import { VideoFetchType } from '../../../helpers/video' import { isNSFWQueryValid, isNumberArray, isStringArray } from '../../../helpers/custom-validators/search' +import { getServerActor } from '../../../helpers/utils' const videosAddValidator = getCommonVideoAttributes().concat([ body('videofile') @@ -127,6 +128,31 @@ const videosUpdateValidator = getCommonVideoAttributes().concat([ } ]) +async function checkVideoFollowConstraints (req: express.Request, res: express.Response, next: express.NextFunction) { + const video: VideoModel = res.locals.video + + // Anybody can watch local videos + if (video.isOwned() === true) return next() + + // Logged user + if (res.locals.oauth) { + // Users can search or watch remote videos + if (CONFIG.SEARCH.REMOTE_URI.USERS === true) return next() + } + + // Anybody can search or watch remote videos + if (CONFIG.SEARCH.REMOTE_URI.ANONYMOUS === true) return next() + + // Check our instance follows an actor that shared this video + const serverActor = await getServerActor() + if (await VideoModel.checkVideoHasInstanceFollow(video.id, serverActor.id) === true) return next() + + return res.status(403) + .json({ + error: 'Cannot get this video regarding follow constraints.' + }) +} + const videosCustomGetValidator = (fetchType: VideoFetchType) => { return [ param('id').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid id'), @@ -141,17 +167,20 @@ const videosCustomGetValidator = (fetchType: VideoFetchType) => { // Video private or blacklisted if (video.privacy === VideoPrivacy.PRIVATE || video.VideoBlacklist) { - return authenticate(req, res, () => { - const user: UserModel = res.locals.oauth.token.User + await authenticatePromiseIfNeeded(req, res) + + const user: UserModel = res.locals.oauth ? res.locals.oauth.token.User : null - // Only the owner or a user that have blacklist rights can see the video - if (video.VideoChannel.Account.userId !== user.id && !user.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST)) { - return res.status(403) - .json({ error: 'Cannot get this private or blacklisted video.' }) - } + // Only the owner or a user that have blacklist rights can see the video + if ( + !user || + (video.VideoChannel.Account.userId !== user.id && !user.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST)) + ) { + return res.status(403) + .json({ error: 'Cannot get this private or blacklisted video.' }) + } - return next() - }) + return next() } // Video is public, anyone can access it @@ -376,6 +405,7 @@ export { videosAddValidator, videosUpdateValidator, videosGetValidator, + checkVideoFollowConstraints, videosCustomGetValidator, videosRemoveValidator, diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 6c183933b..1e68b380c 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1253,6 +1253,23 @@ export class VideoModel extends Model { }) } + static checkVideoHasInstanceFollow (videoId: number, followerActorId: number) { + // Instances only share videos + const query = 'SELECT 1 FROM "videoShare" ' + + 'INNER JOIN "actorFollow" ON "actorFollow"."targetActorId" = "videoShare"."actorId" ' + + 'WHERE "actorFollow"."actorId" = $followerActorId AND "videoShare"."videoId" = $videoId ' + + 'LIMIT 1' + + const options = { + type: Sequelize.QueryTypes.SELECT, + bind: { followerActorId, videoId }, + raw: true + } + + return VideoModel.sequelize.query(query, options) + .then(results => results.length === 1) + } + // threshold corresponds to how many video the field should have to be returned static async getRandomFieldSamples (field: 'category' | 'channelId', threshold: number, count: number) { const serverActor = await getServerActor() diff --git a/server/tests/api/server/follow-constraints.ts b/server/tests/api/server/follow-constraints.ts new file mode 100644 index 000000000..3135fc568 --- /dev/null +++ b/server/tests/api/server/follow-constraints.ts @@ -0,0 +1,215 @@ +/* tslint:disable:no-unused-expression */ + +import * as chai from 'chai' +import 'mocha' +import { doubleFollow, getAccountVideos, getVideo, getVideoChannelVideos, getVideoWithToken } from '../../utils' +import { flushAndRunMultipleServers, killallServers, ServerInfo, setAccessTokensToServers, uploadVideo } from '../../utils/index' +import { unfollow } from '../../utils/server/follows' +import { userLogin } from '../../utils/users/login' +import { createUser } from '../../utils/users/users' + +const expect = chai.expect + +describe('Test follow constraints', function () { + let servers: ServerInfo[] = [] + let video1UUID: string + let video2UUID: string + let userAccessToken: string + + before(async function () { + this.timeout(30000) + + servers = await flushAndRunMultipleServers(2) + + // Get the access tokens + await setAccessTokensToServers(servers) + + { + const res = await uploadVideo(servers[ 0 ].url, servers[ 0 ].accessToken, { name: 'video server 1' }) + video1UUID = res.body.video.uuid + } + { + const res = await uploadVideo(servers[ 1 ].url, servers[ 1 ].accessToken, { name: 'video server 2' }) + video2UUID = res.body.video.uuid + } + + const user = { + username: 'user1', + password: 'super_password' + } + await createUser(servers[0].url, servers[0].accessToken, user.username, user.password) + userAccessToken = await userLogin(servers[0], user) + + await doubleFollow(servers[0], servers[1]) + }) + + describe('With a followed instance', function () { + + describe('With an unlogged user', function () { + + it('Should get the local video', async function () { + await getVideo(servers[0].url, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideo(servers[0].url, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + + describe('With a logged user', function () { + it('Should get the local video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + }) + + describe('With a non followed instance', function () { + + before(async function () { + this.timeout(30000) + + await unfollow(servers[0].url, servers[0].accessToken, servers[1]) + }) + + describe('With an unlogged user', function () { + + it('Should get the local video', async function () { + await getVideo(servers[0].url, video1UUID, 200) + }) + + it('Should not get the remote video', async function () { + await getVideo(servers[0].url, video2UUID, 403) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should not list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(0) + expect(res.body.data).to.have.lengthOf(0) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should not list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(0) + expect(res.body.data).to.have.lengthOf(0) + }) + }) + + describe('With a logged user', function () { + it('Should get the local video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + }) + + after(async function () { + killallServers(servers) + }) +}) diff --git a/server/tests/api/server/index.ts b/server/tests/api/server/index.ts index 78ab7e18b..6afcab1f9 100644 --- a/server/tests/api/server/index.ts +++ b/server/tests/api/server/index.ts @@ -1,5 +1,6 @@ import './config' import './email' +import './follow-constraints' import './follows' import './handle-down' import './jobs' -- 2.25.1