Merge pull request #5388 from mikiher/fix/bulk-download-permissions

Enforce per-item access checks on bulk library download

server/auth/TokenManager.js

@@ -67,6 +67,18 @@ class TokenManager {
     })
   }
 
+  /**
+   * Whether a decoded JWT payload may authenticate API/socket requests (not refresh-only credentials).
+   *
+   * @param {Object} decoded
+   * @returns {boolean}
+   */
+  static isBearerAccessTokenPayload(decoded) {
+    if (!decoded?.userId) return false
+    if (decoded.type === 'refresh') return false
+    return true
+  }
+
   /**
    * Function to validate a jwt token for a given user
    * Used to authenticate socket connections
@@ -77,7 +89,9 @@ class TokenManager {
    */
   static validateAccessToken(token) {
     try {
-      return jwt.verify(token, TokenManager.TokenSecret)
+      const decoded = jwt.verify(token, TokenManager.TokenSecret)
+      if (!TokenManager.isBearerAccessTokenPayload(decoded)) return null
+      return decoded
     } catch (err) {
       return null
     }
@@ -283,7 +297,11 @@ class TokenManager {
 
       done(null, user)
     } else {
-      // JWT based authentication
+      // JWT based authentication — refresh tokens are only valid at POST /auth/refresh
+      if (!TokenManager.isBearerAccessTokenPayload(jwt_payload)) {
+        done(null, null)
+        return
+      }
 
       // Check if the jwt is expired
       if (jwt_payload.exp && jwt_payload.exp < Date.now() / 1000) {

server/controllers/LibraryController.js

@@ -1432,14 +1432,17 @@ class LibraryController {
 
     const itemIds = req.query.ids.split(',')
 
-    const libraryItems = await Database.libraryItemModel.findAll({
-      attributes: ['id', 'libraryId', 'path', 'isFile'],
-      where: {
-        id: itemIds,
-        libraryId: req.library.id
-      }
+    const libraryItems = await Database.libraryItemModel.findAllExpandedWhere({
+      id: itemIds,
+      libraryId: req.library.id
     })
 
+    for (const libraryItem of libraryItems) {
+      if (!req.user.checkCanAccessLibraryItem(libraryItem)) {
+        return res.sendStatus(403)
+      }
+    }
+
     if (libraryItems.length < itemIds.length) {
       Logger.warn(`[LibraryController] User "${req.user.username}" requested ${itemIds.length} items but only ${libraryItems.length} are in library "${req.library.id}"`)
     }

test/server/auth/TokenManager.test.js

@@ -0,0 +1,68 @@
+const { expect } = require('chai')
+const sinon = require('sinon')
+
+const Database = require('../../../server/Database')
+const jwt = require('../../../server/libs/jsonwebtoken')
+
+// Database → Auth → TokenManager circular require can leave TokenManager with a partial Database reference; reload before each test
+function loadTokenManager() {
+  delete require.cache[require.resolve('../../../server/auth/TokenManager')]
+  return require('../../../server/auth/TokenManager')
+}
+
+describe('TokenManager', () => {
+  const secret = 'test-jwt-secret'
+  const userId = 'user-uuid-1'
+  let TokenManager
+  let tokenManager
+
+  beforeEach(() => {
+    TokenManager = loadTokenManager()
+    TokenManager.TokenSecret = secret
+    tokenManager = new TokenManager()
+  })
+
+  afterEach(() => {
+    sinon.restore()
+  })
+
+  describe('validateAccessToken', () => {
+    it('rejects refresh tokens', () => {
+      const refreshToken = jwt.sign({ userId, type: 'refresh' }, secret, { expiresIn: 3600 })
+      expect(TokenManager.validateAccessToken(refreshToken)).to.equal(null)
+    })
+
+    it('accepts access tokens', () => {
+      const accessToken = jwt.sign({ userId, type: 'access' }, secret, { expiresIn: 3600 })
+      const decoded = TokenManager.validateAccessToken(accessToken)
+      expect(decoded.userId).to.equal(userId)
+      expect(decoded.type).to.equal('access')
+    })
+  })
+
+  describe('jwtAuthCheck', () => {
+    const user = { id: userId, username: 'testuser', isActive: true }
+
+    it('rejects refresh tokens for API auth', async () => {
+      const refreshToken = tokenManager.generateRefreshToken(user)
+      const decoded = jwt.verify(refreshToken, secret)
+      const done = sinon.spy()
+
+      await tokenManager.jwtAuthCheck(decoded, done)
+
+      expect(done.calledWith(null, null)).to.be.true
+    })
+
+    it('allows access tokens for active users', async () => {
+      sinon.stub(Database, 'userModel').get(() => ({
+        getUserByIdOrOldId: sinon.stub().resolves(user)
+      }))
+      const decoded = jwt.verify(tokenManager.generateTempAccessToken(user), secret)
+      const done = sinon.spy()
+
+      await tokenManager.jwtAuthCheck(decoded, done)
+
+      expect(done.calledWith(null, user)).to.be.true
+    })
+  })
+})

test/server/controllers/LibraryController.test.js

@@ -0,0 +1,178 @@
+const { expect } = require('chai')
+const { Sequelize } = require('sequelize')
+const sinon = require('sinon')
+
+const Database = require('../../../server/Database')
+const LibraryController = require('../../../server/controllers/LibraryController')
+const zipHelpers = require('../../../server/utils/zipHelpers')
+const Logger = require('../../../server/Logger')
+
+describe('LibraryController.downloadMultiple', () => {
+  let library
+  let libraryFolder
+  let allowedItemId
+  let explicitItemId
+  let taggedItemId
+  let restrictedUser
+  let libraryRecord
+
+  beforeEach(async () => {
+    global.ServerSettings = {}
+    Database.sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false })
+    Database.sequelize.uppercaseFirst = (str) => (str ? `${str[0].toUpperCase()}${str.substr(1)}` : '')
+    await Database.buildModels()
+
+    library = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' })
+    libraryFolder = await Database.libraryFolderModel.create({ path: '/test-lib', libraryId: library.id })
+    libraryRecord = await Database.libraryModel.findByIdWithFolders(library.id)
+
+    const allowedBook = await Database.bookModel.create({
+      title: 'Allowed Book',
+      explicit: false,
+      audioFiles: [],
+      tags: ['allowed-tag'],
+      narrators: [],
+      genres: [],
+      chapters: []
+    })
+    const allowedItem = await Database.libraryItemModel.create({
+      path: '/test-lib/allowed',
+      isFile: false,
+      libraryFiles: [],
+      mediaId: allowedBook.id,
+      mediaType: 'book',
+      libraryId: library.id,
+      libraryFolderId: libraryFolder.id
+    })
+    allowedItemId = allowedItem.id
+
+    const explicitBook = await Database.bookModel.create({
+      title: 'Explicit Book',
+      explicit: true,
+      audioFiles: [],
+      tags: [],
+      narrators: [],
+      genres: [],
+      chapters: []
+    })
+    const explicitItem = await Database.libraryItemModel.create({
+      path: '/test-lib/explicit',
+      isFile: false,
+      libraryFiles: [],
+      mediaId: explicitBook.id,
+      mediaType: 'book',
+      libraryId: library.id,
+      libraryFolderId: libraryFolder.id
+    })
+    explicitItemId = explicitItem.id
+
+    const taggedBook = await Database.bookModel.create({
+      title: 'Tagged Book',
+      explicit: false,
+      audioFiles: [],
+      tags: ['restricted-tag'],
+      narrators: [],
+      genres: [],
+      chapters: []
+    })
+    const taggedItem = await Database.libraryItemModel.create({
+      path: '/test-lib/tagged',
+      isFile: false,
+      libraryFiles: [],
+      mediaId: taggedBook.id,
+      mediaType: 'book',
+      libraryId: library.id,
+      libraryFolderId: libraryFolder.id
+    })
+    taggedItemId = taggedItem.id
+
+    const permissions = Database.userModel.getDefaultPermissionsForUserType('user')
+    permissions.download = true
+    permissions.accessExplicitContent = false
+    permissions.accessAllLibraries = false
+    permissions.accessAllTags = false
+    permissions.librariesAccessible = [library.id]
+    permissions.itemTagsSelected = ['allowed-tag']
+    permissions.selectedTagsNotAccessible = false
+
+    restrictedUser = await Database.userModel.create({
+      username: 'restricted',
+      pash: 'hash',
+      token: 'token',
+      type: 'user',
+      isActive: true,
+      permissions,
+      bookmarks: [],
+      extraData: {}
+    })
+
+    sinon.stub(Logger, 'info')
+    sinon.stub(Logger, 'warn')
+    sinon.stub(Logger, 'error')
+    sinon.stub(zipHelpers, 'zipDirectoriesPipe').resolves()
+  })
+
+  afterEach(async () => {
+    sinon.restore()
+    await Database.sequelize.sync({ force: true })
+  })
+
+  function makeReq(ids) {
+    return {
+      query: { ids: ids.join(',') },
+      user: restrictedUser,
+      library: libraryRecord
+    }
+  }
+
+  function makeRes() {
+    return {
+      sendStatus: sinon.spy(),
+      status: sinon.stub().returnsThis(),
+      send: sinon.spy()
+    }
+  }
+
+  it('returns 403 for bulk download of an explicit item', async () => {
+    const req = makeReq([explicitItemId])
+    const res = makeRes()
+
+    await LibraryController.downloadMultiple(req, res)
+
+    expect(res.sendStatus.calledWith(403)).to.be.true
+    expect(zipHelpers.zipDirectoriesPipe.called).to.be.false
+  })
+
+  it('returns 403 for bulk download of a tag-restricted item', async () => {
+    const req = makeReq([taggedItemId])
+    const res = makeRes()
+
+    await LibraryController.downloadMultiple(req, res)
+
+    expect(res.sendStatus.calledWith(403)).to.be.true
+    expect(zipHelpers.zipDirectoriesPipe.called).to.be.false
+  })
+
+  it('returns 403 when bulk download includes both allowed and forbidden items', async () => {
+    const req = makeReq([allowedItemId, explicitItemId])
+    const res = makeRes()
+
+    await LibraryController.downloadMultiple(req, res)
+
+    expect(res.sendStatus.calledWith(403)).to.be.true
+    expect(zipHelpers.zipDirectoriesPipe.called).to.be.false
+  })
+
+  it('starts zip download for allowed items only', async () => {
+    const req = makeReq([allowedItemId])
+    const res = makeRes()
+
+    await LibraryController.downloadMultiple(req, res)
+
+    expect(res.sendStatus.called).to.be.false
+    expect(zipHelpers.zipDirectoriesPipe.calledOnce).to.be.true
+    const pathObjects = zipHelpers.zipDirectoriesPipe.firstCall.args[0]
+    expect(pathObjects).to.have.length(1)
+    expect(pathObjects[0].path).to.equal('/test-lib/allowed')
+  })
+})