From de3d55d25df98214658e6dbf47325352bbe35ba5 Mon Sep 17 00:00:00 2001 From: Frank Gonnello Date: Sun, 21 Dec 2025 18:21:03 -0500 Subject: [PATCH 1/2] fix(permissions): allow admins to disable auto-approve for their own requests Admins/owners can now disable their auto-approve permissions, allowing their requests to enter the pending queue like regular users. Changes: - Modified hasPermission() to not auto-bypass for AUTO_APPROVE* permissions - Removed MANAGE_REQUESTS from auto-approve permission checks - Allow owner (ID 1) to modify their own auto-approve settings via API - Updated UI to enable owner to toggle auto-approve permissions This enables admins to opt into manual approval workflows when needed. Fixes #4031 --- server/entity/MediaRequest.ts | 11 ++-- server/lib/permissions.ts | 68 ++++++++++++++++------- server/routes/user/usersettings.ts | 24 +++++++- src/components/PermissionOption/index.tsx | 11 ++-- 4 files changed, 83 insertions(+), 31 deletions(-) diff --git a/server/entity/MediaRequest.ts b/server/entity/MediaRequest.ts index fe31fbfd49..e9096c80fd 100644 --- a/server/entity/MediaRequest.ts +++ b/server/entity/MediaRequest.ts @@ -192,7 +192,8 @@ export class MediaRequest { type: MediaType.MOVIE, media, requestedBy: requestUser, - // If the user is an admin or has the "auto approve" permission, automatically approve the request + // If the user has explicit "auto approve" permission, automatically approve the request + // Note: MANAGE_REQUESTS removed so admins can opt out of auto-approval status: user.hasPermission( [ requestBody.is4k @@ -201,7 +202,6 @@ export class MediaRequest { requestBody.is4k ? Permission.AUTO_APPROVE_4K_MOVIE : Permission.AUTO_APPROVE_MOVIE, - Permission.MANAGE_REQUESTS, ], { type: 'or' } ) @@ -215,7 +215,6 @@ export class MediaRequest { requestBody.is4k ? Permission.AUTO_APPROVE_4K_MOVIE : Permission.AUTO_APPROVE_MOVIE, - Permission.MANAGE_REQUESTS, ], { type: 'or' } ) @@ -298,7 +297,8 @@ export class MediaRequest { type: MediaType.TV, media, requestedBy: requestUser, - // If the user is an admin or has the "auto approve" permission, automatically approve the request + // If the user has explicit "auto approve" permission, automatically approve the request + // Note: MANAGE_REQUESTS removed so admins can opt out of auto-approval status: user.hasPermission( [ requestBody.is4k @@ -307,7 +307,6 @@ export class MediaRequest { requestBody.is4k ? Permission.AUTO_APPROVE_4K_TV : Permission.AUTO_APPROVE_TV, - Permission.MANAGE_REQUESTS, ], { type: 'or' } ) @@ -321,7 +320,6 @@ export class MediaRequest { requestBody.is4k ? Permission.AUTO_APPROVE_4K_TV : Permission.AUTO_APPROVE_TV, - Permission.MANAGE_REQUESTS, ], { type: 'or' } ) @@ -345,7 +343,6 @@ export class MediaRequest { requestBody.is4k ? Permission.AUTO_APPROVE_4K_TV : Permission.AUTO_APPROVE_TV, - Permission.MANAGE_REQUESTS, ], { type: 'or' } ) diff --git a/server/lib/permissions.ts b/server/lib/permissions.ts index 98c81a4983..0be05f094d 100644 --- a/server/lib/permissions.ts +++ b/server/lib/permissions.ts @@ -32,40 +32,70 @@ export interface PermissionCheckOptions { type: 'and' | 'or'; } -/** - * Takes a Permission and the users permission value and determines - * if the user has access to the permission provided. If the user has - * the admin permission, true will always be returned from this check! - * - * @param permissions Single permission or array of permissions - * @param value users current permission value - * @param options Extra options to control permission check behavior (mainly for arrays) - */ +function isAutoApprovePermission(perm: Permission): boolean { + return ( + perm === Permission.AUTO_APPROVE || + perm === Permission.AUTO_APPROVE_MOVIE || + perm === Permission.AUTO_APPROVE_TV || + perm === Permission.AUTO_APPROVE_4K || + perm === Permission.AUTO_APPROVE_4K_MOVIE || + perm === Permission.AUTO_APPROVE_4K_TV + ); +} + export const hasPermission = ( permissions: Permission | Permission[], - value: number, + userPermissionValue: number, options: PermissionCheckOptions = { type: 'and' } ): boolean => { - let total = 0; + // 1) Normalize permissions to an array + const requiredPermissions: Permission[] = Array.isArray(permissions) + ? permissions + : [permissions]; - // If we are not checking any permissions, bail out and return true - if (permissions === 0) { + // 2) If we're not checking any permissions at all, return true + if (requiredPermissions.length === 0) { return true; } + // 3) If it’s an array of permissions, handle "and"/"or" if (Array.isArray(permissions)) { - if (value & Permission.ADMIN) { + // Check if this array includes ANY auto-approve permission + const includesAutoApprove = requiredPermissions.some((perm) => + isAutoApprovePermission(perm) + ); + + if (!includesAutoApprove && userPermissionValue & Permission.ADMIN) { + // If there's NO auto-approve permission in the list, then + // "admin = true" as usual return true; } + + // Otherwise, we do the normal bit checks for each required permission switch (options.type) { case 'and': - return permissions.every((permission) => !!(value & permission)); + // "and": user must have *all* required permissions + return requiredPermissions.every( + (perm) => !!(userPermissionValue & perm) + ); case 'or': - return permissions.some((permission) => !!(value & permission)); + // "or": user must have at least one required permission + return requiredPermissions.some( + (perm) => !!(userPermissionValue & perm) + ); } - } else { - total = permissions; } - return !!(value & Permission.ADMIN) || !!(value & total); + // 4) If it's a single permission (not an array) + const singlePerm = requiredPermissions[0]; + // If it's NOT an auto-approve permission, let admin pass automatically + if ( + !isAutoApprovePermission(singlePerm) && + userPermissionValue & Permission.ADMIN + ) { + return true; + } + + // Otherwise, must explicitly match the permission bit + return !!(userPermissionValue & singlePerm); }; diff --git a/server/routes/user/usersettings.ts b/server/routes/user/usersettings.ts index c8b3f50bd2..b0eacbb24f 100644 --- a/server/routes/user/usersettings.ts +++ b/server/routes/user/usersettings.ts @@ -381,6 +381,15 @@ userSettingsRoutes.get<{ id: string }, { permissions?: number }>( } ); +// Auto-approve permission bits that the owner can modify on themselves +const AUTO_APPROVE_BITS = + Permission.AUTO_APPROVE | + Permission.AUTO_APPROVE_MOVIE | + Permission.AUTO_APPROVE_TV | + Permission.AUTO_APPROVE_4K | + Permission.AUTO_APPROVE_4K_MOVIE | + Permission.AUTO_APPROVE_4K_TV; + userSettingsRoutes.post< { id: string }, { permissions?: number }, @@ -400,7 +409,20 @@ userSettingsRoutes.post< return next({ status: 404, message: 'User not found.' }); } - // "Owner" user permissions cannot be modified, and users cannot set their own permissions + // Special case: Owner (ID 1) can modify their OWN auto-approve permissions + // This allows admins to opt out of auto-approval if desired + if (user.id === 1 && req.user?.id === 1) { + // Only allow changes to auto-approve bits, preserve all other permissions + const currentNonAutoApprove = user.permissions & ~AUTO_APPROVE_BITS; + const newAutoApprove = req.body.permissions & AUTO_APPROVE_BITS; + user.permissions = currentNonAutoApprove | newAutoApprove; + + await userRepository.save(user); + return res.status(200).json({ permissions: user.permissions }); + } + + // "Owner" user permissions cannot be modified by others, + // and users cannot set their own permissions (except owner for auto-approve above) if (user.id === 1 || req.user?.id === user.id) { return next({ status: 403, diff --git a/src/components/PermissionOption/index.tsx b/src/components/PermissionOption/index.tsx index 43d5128daf..2f057b5b9a 100644 --- a/src/components/PermissionOption/index.tsx +++ b/src/components/PermissionOption/index.tsx @@ -48,15 +48,18 @@ const PermissionOption = ({ let disabled = false; let checked = hasPermission(option.permission, currentPermission); + // Check if this is an auto-approve permission + const isAutoApprove = autoApprovePermissions.includes(option.permission); + if ( // Permissions for user ID 1 (Plex server owner) cannot be changed - (currentUser && currentUser.id === 1) || + // EXCEPT for auto-approve permissions (admins can opt out of auto-approval) + (currentUser && currentUser.id === 1 && !isAutoApprove) || // Admin permission automatically bypasses/grants all other permissions + // EXCEPT for auto-approve permissions (admins must explicitly have these) (option.permission !== Permission.ADMIN && + !isAutoApprove && hasPermission(Permission.ADMIN, currentPermission)) || - // Manage Requests permission automatically grants all Auto-Approve permissions - (autoApprovePermissions.includes(option.permission) && - hasPermission(Permission.MANAGE_REQUESTS, currentPermission)) || // Selecting a parent permission automatically selects all children (!!parent?.permission && hasPermission(parent.permission, currentPermission)) From 093cc57c3308ea76961633713ad4724ce2f4a2cf Mon Sep 17 00:00:00 2001 From: Frank Gonnello Date: Sun, 21 Dec 2025 23:28:00 -0500 Subject: [PATCH 2/2] fix: restore permissions === 0 check for non-admin login When hasPermission is called with 0 (no permission required), return true immediately. This was accidentally removed during the admin auto-approve fix, breaking /auth/me for non-admin users. --- server/lib/permissions.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/lib/permissions.ts b/server/lib/permissions.ts index 0be05f094d..4d4bafd1a0 100644 --- a/server/lib/permissions.ts +++ b/server/lib/permissions.ts @@ -48,12 +48,18 @@ export const hasPermission = ( userPermissionValue: number, options: PermissionCheckOptions = { type: 'and' } ): boolean => { + // If we are not checking any permissions, bail out and return true + // This handles isAuthenticated() called with no arguments (any logged-in user) + if (permissions === 0) { + return true; + } + // 1) Normalize permissions to an array const requiredPermissions: Permission[] = Array.isArray(permissions) ? permissions : [permissions]; - // 2) If we're not checking any permissions at all, return true + // 2) If we're checking an empty array, return true if (requiredPermissions.length === 0) { return true; }