Update LibraryItemController test with 403 tests

server/controllers/LibraryItemController.js

@@ -36,6 +36,24 @@ 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() {}
 
@@ -547,7 +565,13 @@ 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}"`)
@@ -581,6 +605,7 @@ class LibraryItemController {
     }
 
     await Database.resetLibraryIssuesFilterData(libraryId)
+
     res.sendStatus(200)
   }
 
@@ -593,6 +618,11 @@ 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`)
@@ -615,6 +645,11 @@ 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 = []
@@ -695,6 +730,10 @@ 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,7 +123,9 @@ describe('LibraryItemController', () => {
       const fakeReq = {
         query: {},
         user: {
-          canDelete: true
+          username: 'test',
+          canDelete: true,
+          checkCanAccessLibraryItem: () => true
         },
         body: {
           libraryItemIds: [libraryItem1Id]
@@ -199,4 +201,102 @@ 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
+    })
+  })
 })