From ff9d43f62a4f4737c5bfe955883b48c5440f323a Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 22 Jun 2022 09:44:08 +0200 Subject: [PATCH] Refactor video rights checker --- server/middlewares/validators/feeds.ts | 6 +- .../middlewares/validators/shared/videos.ts | 84 +++++++++++++++---- .../validators/videos/video-captions.ts | 4 +- .../validators/videos/video-comments.ts | 10 +-- .../validators/videos/video-rates.ts | 4 +- .../middlewares/validators/videos/videos.ts | 28 +------ server/models/user/user.ts | 21 +---- server/tests/api/check-params/videos.ts | 5 +- 8 files changed, 89 insertions(+), 73 deletions(-) diff --git a/server/middlewares/validators/feeds.ts b/server/middlewares/validators/feeds.ts index f8ebaf6ed..04b4e00c9 100644 --- a/server/middlewares/validators/feeds.ts +++ b/server/middlewares/validators/feeds.ts @@ -6,6 +6,7 @@ import { exists, isIdOrUUIDValid, isIdValid, toCompleteUUID } from '../../helper import { logger } from '../../helpers/logger' import { areValidationErrors, + checkCanSeeVideo, doesAccountIdExist, doesAccountNameWithHostExist, doesUserFeedTokenCorrespond, @@ -112,7 +113,10 @@ const videoCommentsFeedsValidator = [ return res.fail({ message: 'videoId cannot be mixed with a channel filter' }) } - if (req.query.videoId && !await doesVideoExist(req.query.videoId, res)) return + if (req.query.videoId) { + if (!await doesVideoExist(req.query.videoId, res)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.query.videoId, video: res.locals.videoAll })) return + } return next() } diff --git a/server/middlewares/validators/shared/videos.ts b/server/middlewares/validators/shared/videos.ts index 8807435f6..39aab6df7 100644 --- a/server/middlewares/validators/shared/videos.ts +++ b/server/middlewares/validators/shared/videos.ts @@ -1,4 +1,5 @@ import { Request, Response } from 'express' +import { isUUIDValid } from '@server/helpers/custom-validators/misc' import { loadVideo, VideoLoadType } from '@server/lib/model-loaders' import { isAbleToUploadVideo } from '@server/lib/user' import { authenticatePromiseIfNeeded } from '@server/middlewares/auth' @@ -18,18 +19,19 @@ import { MVideoThumbnail, MVideoWithRights } from '@server/types/models' -import { HttpStatusCode, ServerErrorCode, UserRight } from '@shared/models' +import { HttpStatusCode, ServerErrorCode, UserRight, VideoPrivacy } from '@shared/models' async function doesVideoExist (id: number | string, res: Response, fetchType: VideoLoadType = 'all') { const userId = res.locals.oauth ? res.locals.oauth.token.User.id : undefined const video = await loadVideo(id, fetchType, userId) - if (video === null) { + if (!video) { res.fail({ status: HttpStatusCode.NOT_FOUND_404, message: 'Video not found' }) + return false } @@ -58,6 +60,8 @@ async function doesVideoExist (id: number | string, res: Response, fetchType: Vi return true } +// --------------------------------------------------------------------------- + async function doesVideoFileOfVideoExist (id: number, videoIdOrUUID: number | string, res: Response) { if (!await VideoFileModel.doesVideoExistForVideoFile(id, videoIdOrUUID)) { res.fail({ @@ -70,6 +74,8 @@ async function doesVideoFileOfVideoExist (id: number, videoIdOrUUID: number | st return true } +// --------------------------------------------------------------------------- + async function doesVideoChannelOfAccountExist (channelId: number, user: MUserAccountId, res: Response) { const videoChannel = await VideoChannelModel.loadAndPopulateAccount(channelId) @@ -95,32 +101,77 @@ async function doesVideoChannelOfAccountExist (channelId: number, user: MUserAcc return true } -async function checkCanSeeVideoIfPrivate (req: Request, res: Response, video: MVideo, authenticateInQuery = false) { - if (!video.requiresAuth()) return true +// --------------------------------------------------------------------------- - const videoWithRights = await VideoModel.loadAndPopulateAccountAndServerAndTags(video.id) +async function checkCanSeeVideo (options: { + req: Request + res: Response + paramId: string + video: MVideo + authenticateInQuery?: boolean // default false +}) { + const { req, res, video, paramId, authenticateInQuery = false } = options + + if (video.requiresAuth()) { + return checkCanSeeAuthVideo(req, res, video, authenticateInQuery) + } - return checkCanSeePrivateVideo(req, res, videoWithRights, authenticateInQuery) -} + if (video.privacy === VideoPrivacy.UNLISTED) { + if (isUUIDValid(paramId)) return true -async function checkCanSeePrivateVideo (req: Request, res: Response, video: MVideoWithRights, authenticateInQuery = false) { - await authenticatePromiseIfNeeded(req, res, authenticateInQuery) + return checkCanSeeAuthVideo(req, res, video, authenticateInQuery) + } - const user = res.locals.oauth ? res.locals.oauth.token.User : null + if (video.privacy === VideoPrivacy.PUBLIC) return true + + throw new Error('Fatal error when checking video right ' + video.url) +} - // Only the owner or a user that have blocklist rights can see the video - if (!user || !user.canGetVideo(video)) { +async function checkCanSeeAuthVideo (req: Request, res: Response, video: MVideoId | MVideoWithRights, authenticateInQuery = false) { + const fail = () => { res.fail({ status: HttpStatusCode.FORBIDDEN_403, - message: 'Cannot fetch information of private/internal/blocklisted video' + message: 'Cannot fetch information of private/internal/blocked video' }) return false } - return true + await authenticatePromiseIfNeeded(req, res, authenticateInQuery) + + const user = res.locals.oauth?.token.User + if (!user) return fail() + + const videoWithRights = (video as MVideoWithRights).VideoChannel?.Account?.userId + ? video as MVideoWithRights + : await VideoModel.loadAndPopulateAccountAndServerAndTags(video.id) + + const privacy = videoWithRights.privacy + + if (privacy === VideoPrivacy.INTERNAL) { + // We know we have a user + return true + } + + const isOwnedByUser = videoWithRights.VideoChannel.Account.userId === user.id + if (privacy === VideoPrivacy.PRIVATE || privacy === VideoPrivacy.UNLISTED) { + if (isOwnedByUser && user.hasRight(UserRight.SEE_ALL_VIDEOS)) return true + + return fail() + } + + if (videoWithRights.isBlacklisted()) { + if (isOwnedByUser || user.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST)) return true + + return fail() + } + + // Should not happen + return fail() } +// --------------------------------------------------------------------------- + function checkUserCanManageVideo (user: MUser, video: MVideoAccountLight, right: UserRight, res: Response, onlyOwned = true) { // Retrieve the user who did the request if (onlyOwned && video.isOwned() === false) { @@ -146,6 +197,8 @@ function checkUserCanManageVideo (user: MUser, video: MVideoAccountLight, right: return true } +// --------------------------------------------------------------------------- + async function checkUserQuota (user: MUserId, videoFileSize: number, res: Response) { if (await isAbleToUploadVideo(user.id, videoFileSize) === false) { res.fail({ @@ -167,7 +220,6 @@ export { doesVideoFileOfVideoExist, checkUserCanManageVideo, - checkCanSeeVideoIfPrivate, - checkCanSeePrivateVideo, + checkCanSeeVideo, checkUserQuota } diff --git a/server/middlewares/validators/videos/video-captions.ts b/server/middlewares/validators/videos/video-captions.ts index 441c6b4be..dfb8fefc5 100644 --- a/server/middlewares/validators/videos/video-captions.ts +++ b/server/middlewares/validators/videos/video-captions.ts @@ -7,7 +7,7 @@ import { logger } from '../../../helpers/logger' import { CONSTRAINTS_FIELDS, MIMETYPES } from '../../../initializers/constants' import { areValidationErrors, - checkCanSeeVideoIfPrivate, + checkCanSeeVideo, checkUserCanManageVideo, doesVideoCaptionExist, doesVideoExist, @@ -74,7 +74,7 @@ const listVideoCaptionsValidator = [ if (!await doesVideoExist(req.params.videoId, res, 'only-video')) return const video = res.locals.onlyVideo - if (!await checkCanSeeVideoIfPrivate(req, res, video)) return + if (!await checkCanSeeVideo({ req, res, video, paramId: req.params.videoId })) return return next() } diff --git a/server/middlewares/validators/videos/video-comments.ts b/server/middlewares/validators/videos/video-comments.ts index 698afdbd1..b22a4e3b7 100644 --- a/server/middlewares/validators/videos/video-comments.ts +++ b/server/middlewares/validators/videos/video-comments.ts @@ -10,7 +10,7 @@ import { Hooks } from '../../../lib/plugins/hooks' import { MCommentOwnerVideoReply, MVideo, MVideoFullLight } from '../../../types/models/video' import { areValidationErrors, - checkCanSeeVideoIfPrivate, + checkCanSeeVideo, doesVideoCommentExist, doesVideoCommentThreadExist, doesVideoExist, @@ -54,7 +54,7 @@ const listVideoCommentThreadsValidator = [ if (areValidationErrors(req, res)) return if (!await doesVideoExist(req.params.videoId, res, 'only-video')) return - if (!await checkCanSeeVideoIfPrivate(req, res, res.locals.onlyVideo)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.params.videoId, video: res.locals.onlyVideo })) return return next() } @@ -73,7 +73,7 @@ const listVideoThreadCommentsValidator = [ if (!await doesVideoExist(req.params.videoId, res, 'only-video')) return if (!await doesVideoCommentThreadExist(req.params.threadId, res.locals.onlyVideo, res)) return - if (!await checkCanSeeVideoIfPrivate(req, res, res.locals.onlyVideo)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.params.videoId, video: res.locals.onlyVideo })) return return next() } @@ -91,7 +91,7 @@ const addVideoCommentThreadValidator = [ if (areValidationErrors(req, res)) return if (!await doesVideoExist(req.params.videoId, res)) return - if (!await checkCanSeeVideoIfPrivate(req, res, res.locals.videoAll)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.params.videoId, video: res.locals.videoAll })) return if (!isVideoCommentsEnabled(res.locals.videoAll, res)) return if (!await isVideoCommentAccepted(req, res, res.locals.videoAll, false)) return @@ -113,7 +113,7 @@ const addVideoCommentReplyValidator = [ if (areValidationErrors(req, res)) return if (!await doesVideoExist(req.params.videoId, res)) return - if (!await checkCanSeeVideoIfPrivate(req, res, res.locals.videoAll)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.params.videoId, video: res.locals.videoAll })) return if (!isVideoCommentsEnabled(res.locals.videoAll, res)) return if (!await doesVideoCommentExist(req.params.commentId, res.locals.videoAll, res)) return diff --git a/server/middlewares/validators/videos/video-rates.ts b/server/middlewares/validators/videos/video-rates.ts index 1a9736034..8b8eeedb6 100644 --- a/server/middlewares/validators/videos/video-rates.ts +++ b/server/middlewares/validators/videos/video-rates.ts @@ -8,7 +8,7 @@ import { isRatingValid } from '../../../helpers/custom-validators/video-rates' import { isVideoRatingTypeValid } from '../../../helpers/custom-validators/videos' import { logger } from '../../../helpers/logger' import { AccountVideoRateModel } from '../../../models/account/account-video-rate' -import { areValidationErrors, checkCanSeeVideoIfPrivate, doesVideoExist, isValidVideoIdParam } from '../shared' +import { areValidationErrors, checkCanSeeVideo, doesVideoExist, isValidVideoIdParam } from '../shared' const videoUpdateRateValidator = [ isValidVideoIdParam('id'), @@ -21,7 +21,7 @@ const videoUpdateRateValidator = [ if (areValidationErrors(req, res)) return if (!await doesVideoExist(req.params.id, res)) return - if (!await checkCanSeeVideoIfPrivate(req, res, res.locals.videoAll)) return + if (!await checkCanSeeVideo({ req, res, paramId: req.params.id, video: res.locals.videoAll })) return return next() } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index c75c3640b..c6d31f8f0 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -7,14 +7,13 @@ import { getServerActor } from '@server/models/application/application' import { ExpressPromiseHandler } from '@server/types/express-handler' import { MUserAccountId, MVideoFullLight } from '@server/types/models' import { getAllPrivacies } from '@shared/core-utils' -import { HttpStatusCode, ServerErrorCode, UserRight, VideoInclude, VideoPrivacy } from '@shared/models' +import { HttpStatusCode, ServerErrorCode, UserRight, VideoInclude } from '@shared/models' import { exists, isBooleanValid, isDateValid, isFileValid, isIdValid, - isUUIDValid, toArray, toBooleanOrNull, toIntOrNull, @@ -50,7 +49,7 @@ import { Hooks } from '../../../lib/plugins/hooks' import { VideoModel } from '../../../models/video/video' import { areValidationErrors, - checkCanSeePrivateVideo, + checkCanSeeVideo, checkUserCanManageVideo, checkUserQuota, doesVideoChannelOfAccountExist, @@ -297,28 +296,9 @@ const videosCustomGetValidator = ( const video = getVideoWithAttributes(res) as MVideoFullLight - // Video private or blacklisted - if (video.requiresAuth()) { - if (await checkCanSeePrivateVideo(req, res, video, authenticateInQuery)) { - return next() - } + if (!await checkCanSeeVideo({ req, res, video, paramId: req.params.id, authenticateInQuery })) return - return - } - - // Video is public, anyone can access it - if (video.privacy === VideoPrivacy.PUBLIC) return next() - - // Video is unlisted, check we used the uuid to fetch it - if (video.privacy === VideoPrivacy.UNLISTED) { - if (isUUIDValid(req.params.id)) return next() - - // Don't leak this unlisted video - return res.fail({ - status: HttpStatusCode.NOT_FOUND_404, - message: 'Video not found' - }) - } + return next() } ] } diff --git a/server/models/user/user.ts b/server/models/user/user.ts index a25551ecd..dc260e512 100644 --- a/server/models/user/user.ts +++ b/server/models/user/user.ts @@ -29,12 +29,11 @@ import { MUserDefault, MUserFormattable, MUserNotifSettingChannelDefault, - MUserWithNotificationSetting, - MVideoWithRights + MUserWithNotificationSetting } from '@server/types/models' import { AttributesOnly } from '@shared/typescript-utils' import { hasUserRight, USER_ROLE_LABELS } from '../../../shared/core-utils/users' -import { AbuseState, MyUser, UserRight, VideoPlaylistType, VideoPrivacy } from '../../../shared/models' +import { AbuseState, MyUser, UserRight, VideoPlaylistType } from '../../../shared/models' import { User, UserRole } from '../../../shared/models/users' import { UserAdminFlag } from '../../../shared/models/users/user-flag.model' import { NSFWPolicyType } from '../../../shared/models/videos/nsfw-policy.type' @@ -851,22 +850,6 @@ export class UserModel extends Model>> { .then(u => u.map(u => u.username)) } - canGetVideo (video: MVideoWithRights) { - const videoUserId = video.VideoChannel.Account.userId - - if (video.isBlacklisted()) { - return videoUserId === this.id || this.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST) - } - - if (video.privacy === VideoPrivacy.PRIVATE) { - return video.VideoChannel && videoUserId === this.id || this.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST) - } - - if (video.privacy === VideoPrivacy.INTERNAL) return true - - return false - } - hasRight (right: UserRight) { return hasUserRight(this.role, right) } diff --git a/server/tests/api/check-params/videos.ts b/server/tests/api/check-params/videos.ts index 41064d2ff..5ff51d1ff 100644 --- a/server/tests/api/check-params/videos.ts +++ b/server/tests/api/check-params/videos.ts @@ -39,10 +39,7 @@ describe('Test videos API validator', function () { await setAccessTokensToServers([ server ]) - const username = 'user1' - const password = 'my super password' - await server.users.create({ username: username, password: password }) - userAccessToken = await server.login.getAccessToken({ username, password }) + userAccessToken = await server.users.generateUserAndToken('user1') { const body = await server.users.getMyInfo() -- 2.41.0