From 80b39abaa2723cf9bc0065fec702f7353721f340 Mon Sep 17 00:00:00 2001 From: advplyr Date: Tue, 21 Apr 2026 17:13:06 -0500 Subject: [PATCH 1/2] Update library item batch api endpoints check users per-item access & return 403 --- server/controllers/LibraryItemController.js | 39 +++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/server/controllers/LibraryItemController.js b/server/controllers/LibraryItemController.js index 1a6b8ac11..e524ebcfd 100644 --- a/server/controllers/LibraryItemController.js +++ b/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()) }) From 5b2a788cfc9afd8f38a2a21a2f5fe65a5a8a1006 Mon Sep 17 00:00:00 2001 From: advplyr Date: Tue, 21 Apr 2026 17:13:52 -0500 Subject: [PATCH 2/2] Update LibraryItemController test with 403 tests --- .../controllers/LibraryItemController.test.js | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/test/server/controllers/LibraryItemController.test.js b/test/server/controllers/LibraryItemController.test.js index 5a0422392..1d57219d3 100644 --- a/test/server/controllers/LibraryItemController.test.js +++ b/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 + }) + }) })