fix(auth): Add admin-level auth to LibraryController 'delete', 'update' and 'delete items with issues'

[Alexshch09]

Brief summary

This PR addresses a security vulnerability in the LibraryController by enforcing admin-level authorization for critical endpoints that were previously accessible to non-admin users. Specifically, it restricts access to library settings updates, library issue removal, and library deletion to only admin users.

Which issue is fixed?

No issue reported (security fix).

In-depth Description

This PR implements admin-level authorization checks in the LibraryController for the following methods:

• PATCH /api/libraries/:id (update method): Previously, any authenticated user could potentially modify library settings. This change adds a check at the beginning of the update method to ensure that only users with isAdminOrUp privileges can proceed with updating library settings.
• DELETE /api/libraries/:id/issues (removeLibraryItemsWithIssues method): Similarly, this endpoint for removing library items with issues was not properly protected. This PR adds an isAdminOrUp check at the start of the removeLibraryItemsWithIssues method to restrict its use to administrators.
• DELETE /api/libraries/:id (delete method): This PR also adds an isAdminOrUp check to the delete method to ensure that only admin users can delete libraries. Deleting a library is a highly sensitive operation that should be restricted to administrators.

These changes ensure that sensitive library management operations are only accessible to authorized admin users, enhancing the overall security and access control of the application.

How have you tested this?

Test Steps:

1. Manual testing was performed using curl commands to directly interact with the API endpoints.
2. For each endpoint, tests were executed using two types of JWTs:
   • Admin JWT: A valid JWT for a user with administrator privileges.
   • Non-Admin JWT: A valid JWT for a standard user (without administrator privileges).
3. The following endpoints were specifically tested to verify authorization:
   • PATCH /api/libraries/:id: curl commands were used with both Admin and Non-Admin JWTs to attempt updating library settings.
   • DELETE /api/libraries/:id/issues: curl commands were used with both Admin and Non-Admin JWTs to attempt removing library issues.
   • DELETE /api/libraries/:id: curl commands were used with both Admin and Non-Admin JWTs to attempt deleting a library.

[nichwall]

Instead of adding this to each function, I think this would be better added as part of the middleware for the LibraryController. So just adding

   if (req.path.includes('/narrators')) {
      // allow requests to narrators if user has access to library, uses `canUpdate` and `canDelete` permissions
    } else if (req.method == 'DELETE' && !req.user.isAdminOrUp) {
      Logger.warn(`[LibraryController] User "${req.user.username}" attempted to delete without permission`)
      return res.sendStatus(403)
    } else if ((req.method == 'PATCH' || req.method == 'POST') && !req.user.isAdminOrUp) {
      Logger.warn(`[LibraryController] User "${req.user.username}" attempted to update without permission`)
      return res.sendStatus(403)
    }

to the middleware function, similar to how it is done in the LibraryItemController here

audiobookshelf/server/controllers/LibraryItemController.js

Lines 1166 to 1196 in 19a65db

	async middleware(req, res, next) {
	req.libraryItem = await Database.libraryItemModel.getExpandedById(req.params.id)
	if (!req.libraryItem?.media) return res.sendStatus(404)
	// Check user can access this library item
	if (!req.user.checkCanAccessLibraryItem(req.libraryItem)) {
	return res.sendStatus(403)
	}
	// For library file routes, get the library file
	if (req.params.fileid) {
	req.libraryFile = req.libraryItem.getLibraryFileWithIno(req.params.fileid)
	if (!req.libraryFile) {
	Logger.error(`[LibraryItemController] Library file "${req.params.fileid}" does not exist for library item`)
	return res.sendStatus(404)
	}
	}
	if (req.path.includes('/play')) {
	// allow POST requests using /play and /play/:episodeId
	} else if (req.method == 'DELETE' && !req.user.canDelete) {
	Logger.warn(`[LibraryItemController] User "${req.user.username}" attempted to delete without permission`)
	return res.sendStatus(403)
	} else if ((req.method == 'PATCH' || req.method == 'POST') && !req.user.canUpdate) {
	Logger.warn(`[LibraryItemController] User "${req.user.username}" attempted to update without permission`)
	return res.sendStatus(403)
	}
	next()
	}
	}

[advplyr]

I think the middlewares should get refactored. Using req.path.includes like is done in LibraryItemController could cause issues like that recent security advisory.
Maybe we just add on another middleware for checking user access.

We'll put this in the next patch and work on better middleware after

[advplyr]

Thanks!