Refactor video rights checker

diff --git a/server/middlewares/validators/feeds.ts b/server/middlewares/validators/feeds.ts
index f8ebaf6ed87d65cb360fe6465c56b625d3acd0ca..04b4e00c9c6a6b6f7b43346c433d0327a471843a 100644 (file)
--- 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 8807435f6c9b6d3cb274564fd9a1b170ad4a88f7..39aab6df7207aae0bf24c1c37dcef2132076b717 100644 (file)
--- 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 441c6b4be2554c8117eca74a7814efb81105a603..dfb8fefc5c83a122832d09ec62f5ab4144a197c8 100644 (file)
--- 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 698afdbd1170271889fb069ad6e64eba4c69d22e..b22a4e3b79e5a2eb75377d5644d2478537e421b1 100644 (file)
--- 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 1a973603464268429ad9dd9814033a1898035c88..8b8eeedb6df9e822c4a8a6010c4978f291f43d7e 100644 (file)
--- 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 c75c3640bef2ca2244b1dc6f0f51b833b1c5194a..c6d31f8f053ff2aa845c1d5519e8bf3fc90517a1 100644 (file)
--- 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 a25551ecde79aa858626c2662e866a9f0fe0c66a..dc260e512e2eec609473f506440cd3fc17579420 100644 (file)
--- 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<Partial<AttributesOnly<UserModel>>> {
                     .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 41064d2ffdc5aeb94a0731e09d36a8f818b725df..5ff51d1ff46f8055ae054669bf88d448fb2df948 100644 (file)
--- 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()