server/controllers/LibraryItemController.js

@@ -36,24 +36,6 @@ const ShareManager = require('../managers/ShareManager')
  * @typedef {RequestWithUser & RequestEntityObject & RequestLibraryFileObject} LibraryItemControllerRequestWithFile
  */
 
-/**
- * Enforce per-item access for batch item routes
- *
- * @param {RequestWithUser} req
- * @param {Response} res
- * @param {import('../models/LibraryItem')[]} libraryItems
- * @returns {boolean} true if the user may access every item; false if 403 was sent
- */
-function ensureUserCanAccessLibraryItemsForBatch(req, res, libraryItems) {
-  for (const libraryItem of libraryItems) {
-    if (!req.user.checkCanAccessLibraryItem(libraryItem)) {
-      res.sendStatus(403)
-      return false
-    }
-  }
-  return true
-}
-
 class LibraryItemController {
   constructor() {}
 
@@ -565,13 +547,7 @@ class LibraryItemController {
       return res.sendStatus(404)
     }
 
-    // Ensure user has permission to delete these library items
-    if (!ensureUserCanAccessLibraryItemsForBatch(req, res, itemsToDelete)) {
-      return
-    }
-
     const libraryId = itemsToDelete[0].libraryId
-
     for (const libraryItem of itemsToDelete) {
       const libraryItemPath = libraryItem.path
       Logger.info(`[LibraryItemController] (${hardDelete ? 'Hard' : 'Soft'}) deleting Library Item "${libraryItem.media.title}" with id "${libraryItem.id}"`)
@@ -605,7 +581,6 @@ class LibraryItemController {
     }
 
     await Database.resetLibraryIssuesFilterData(libraryId)
-
     res.sendStatus(200)
   }
 
@@ -618,11 +593,6 @@ class LibraryItemController {
    * @param {Response} res
    */
   async batchUpdate(req, res) {
-    if (!req.user.canUpdate) {
-      Logger.warn(`[LibraryItemController] User "${req.user.username}" attempted to batch update without permission`)
-      return res.sendStatus(403)
-    }
-
     const updatePayloads = req.body
     if (!Array.isArray(updatePayloads) || !updatePayloads.length) {
       Logger.error(`[LibraryItemController] Batch update failed. Invalid payload`)
@@ -645,11 +615,6 @@ class LibraryItemController {
       return res.sendStatus(404)
     }
 
-    // Ensure user has permission to update these library items
-    if (!ensureUserCanAccessLibraryItemsForBatch(req, res, libraryItems)) {
-      return
-    }
-
     let itemsUpdated = 0
 
     const seriesIdsRemoved = []
@@ -730,10 +695,6 @@ class LibraryItemController {
     const libraryItems = await Database.libraryItemModel.findAllExpandedWhere({
       id: libraryItemIds
     })
-    // Ensure user has permission to access these library items
-    if (!ensureUserCanAccessLibraryItemsForBatch(req, res, libraryItems)) {
-      return
-    }
     res.json({
       libraryItems: libraryItems.map((li) => li.toOldJSONExpanded())
     })

test/server/controllers/LibraryItemController.test.js

@@ -123,9 +123,7 @@ describe('LibraryItemController', () => {
       const fakeReq = {
         query: {},
         user: {
-          username: 'test',
-          canDelete: true,
-          checkCanAccessLibraryItem: () => true
+          canDelete: true
         },
         body: {
           libraryItemIds: [libraryItem1Id]
@@ -201,102 +199,4 @@ describe('LibraryItemController', () => {
       expect(series2Exists).to.be.true
     })
   })
-
-  describe('batch item access control', () => {
-    let lib1Id
-    let itemLib1Id
-    let itemLib2Id
-
-    beforeEach(async () => {
-      const lib1 = await Database.libraryModel.create({ name: 'Lib 1', mediaType: 'book' })
-      const folder1 = await Database.libraryFolderModel.create({ path: '/l1', libraryId: lib1.id })
-      const book1 = await Database.bookModel.create({ title: 'B1', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] })
-      const li1 = await Database.libraryItemModel.create({
-        libraryFiles: [],
-        mediaId: book1.id,
-        mediaType: 'book',
-        libraryId: lib1.id,
-        libraryFolderId: folder1.id
-      })
-      lib1Id = lib1.id
-      itemLib1Id = li1.id
-
-      const lib2 = await Database.libraryModel.create({ name: 'Lib 2', mediaType: 'book' })
-      const folder2 = await Database.libraryFolderModel.create({ path: '/l2', libraryId: lib2.id })
-      const book2 = await Database.bookModel.create({ title: 'B2', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] })
-      const li2 = await Database.libraryItemModel.create({
-        libraryFiles: [],
-        mediaId: book2.id,
-        mediaType: 'book',
-        libraryId: lib2.id,
-        libraryFolderId: folder2.id
-      })
-      itemLib2Id = li2.id
-    })
-
-    const userLimitedToLib1 = () => ({
-      username: 'limited',
-      canDelete: true,
-      canUpdate: true,
-      checkCanAccessLibraryItem(li) {
-        return li.libraryId === lib1Id
-      }
-    })
-
-    it('batchGet returns 403 for a library item the user cannot access', async () => {
-      const fakeRes = { sendStatus: sinon.spy(), json: sinon.spy() }
-      const fakeReq = {
-        body: { libraryItemIds: [itemLib2Id] },
-        user: userLimitedToLib1()
-      }
-      await LibraryItemController.batchGet.bind(apiRouter)(fakeReq, fakeRes)
-      expect(fakeRes.sendStatus.calledWith(403)).to.be.true
-    })
-
-    it('batchGet returns items when the user can access them', async () => {
-      const fakeRes = { sendStatus: sinon.spy(), json: sinon.spy() }
-      const fakeReq = {
-        body: { libraryItemIds: [itemLib1Id] },
-        user: userLimitedToLib1()
-      }
-      await LibraryItemController.batchGet.bind(apiRouter)(fakeReq, fakeRes)
-      expect(fakeRes.json.calledOnce).to.be.true
-      const payload = fakeRes.json.firstCall.args[0]
-      expect(payload.libraryItems).to.have.length(1)
-      expect(payload.libraryItems[0].id).to.equal(itemLib1Id)
-    })
-
-    it('batchUpdate returns 403 for a library item the user cannot access', async () => {
-      const fakeRes = { sendStatus: sinon.spy(), json: sinon.spy() }
-      const fakeReq = {
-        user: userLimitedToLib1(),
-        body: [{ id: itemLib2Id, mediaPayload: {} }]
-      }
-      await LibraryItemController.batchUpdate.bind(apiRouter)(fakeReq, fakeRes)
-      expect(fakeRes.sendStatus.calledWith(403)).to.be.true
-    })
-
-    it('batchUpdate returns 403 when the user lacks canUpdate', async () => {
-      const u = userLimitedToLib1()
-      u.canUpdate = false
-      const fakeRes = { sendStatus: sinon.spy(), json: sinon.spy() }
-      const fakeReq = {
-        user: u,
-        body: [{ id: itemLib1Id, mediaPayload: {} }]
-      }
-      await LibraryItemController.batchUpdate.bind(apiRouter)(fakeReq, fakeRes)
-      expect(fakeRes.sendStatus.calledWith(403)).to.be.true
-    })
-
-    it('batchDelete returns 403 for a library item the user cannot access', async () => {
-      const fakeRes = { sendStatus: sinon.spy() }
-      const fakeReq = {
-        query: {},
-        user: userLimitedToLib1(),
-        body: { libraryItemIds: [itemLib2Id] }
-      }
-      await LibraryItemController.batchDelete.bind(apiRouter)(fakeReq, fakeRes)
-      expect(fakeRes.sendStatus.calledWith(403)).to.be.true
-    })
-  })
 })