diff --git a/api/server/middleware/checkPeoplePickerAccess.js b/api/server/middleware/checkPeoplePickerAccess.js index 0e604272db..af2154dbba 100644 --- a/api/server/middleware/checkPeoplePickerAccess.js +++ b/api/server/middleware/checkPeoplePickerAccess.js @@ -2,13 +2,20 @@ const { logger } = require('@librechat/data-schemas'); const { PrincipalType, PermissionTypes, Permissions } = require('librechat-data-provider'); const { getRoleByName } = require('~/models/Role'); +const VALID_PRINCIPAL_TYPES = new Set([ + PrincipalType.USER, + PrincipalType.GROUP, + PrincipalType.ROLE, +]); + /** - * Middleware to check if user has permission to access people picker functionality - * Checks specific permission based on the 'type' query parameter: - * - type=user: requires VIEW_USERS permission - * - type=group: requires VIEW_GROUPS permission - * - type=role: requires VIEW_ROLES permission - * - no type (mixed search): requires either VIEW_USERS OR VIEW_GROUPS OR VIEW_ROLES + * Middleware to check if user has permission to access people picker functionality. + * Validates requested principal types via `type` (singular) and `types` (comma-separated or array) + * query parameters against the caller's role permissions: + * - user: requires VIEW_USERS permission + * - group: requires VIEW_GROUPS permission + * - role: requires VIEW_ROLES permission + * - no type filter (mixed search): requires at least one of the above */ const checkPeoplePickerAccess = async (req, res, next) => { try { @@ -28,7 +35,7 @@ const checkPeoplePickerAccess = async (req, res, next) => { }); } - const { type } = req.query; + const { type, types } = req.query; const peoplePickerPerms = role.permissions[PermissionTypes.PEOPLE_PICKER] || {}; const canViewUsers = peoplePickerPerms[Permissions.VIEW_USERS] === true; const canViewGroups = peoplePickerPerms[Permissions.VIEW_GROUPS] === true; @@ -49,15 +56,32 @@ const checkPeoplePickerAccess = async (req, res, next) => { }, }; - const check = permissionChecks[type]; - if (check && !check.hasPermission) { - return res.status(403).json({ - error: 'Forbidden', - message: check.message, - }); + const requestedTypes = new Set(); + + if (type && VALID_PRINCIPAL_TYPES.has(type)) { + requestedTypes.add(type); } - if (!type && !canViewUsers && !canViewGroups && !canViewRoles) { + if (types) { + const typesArray = Array.isArray(types) ? types : types.split(','); + for (const t of typesArray) { + if (VALID_PRINCIPAL_TYPES.has(t)) { + requestedTypes.add(t); + } + } + } + + for (const requested of requestedTypes) { + const check = permissionChecks[requested]; + if (!check.hasPermission) { + return res.status(403).json({ + error: 'Forbidden', + message: check.message, + }); + } + } + + if (requestedTypes.size === 0 && !canViewUsers && !canViewGroups && !canViewRoles) { return res.status(403).json({ error: 'Forbidden', message: 'Insufficient permissions to search for users, groups, or roles', @@ -67,7 +91,7 @@ const checkPeoplePickerAccess = async (req, res, next) => { next(); } catch (error) { logger.error( - `[checkPeoplePickerAccess][${req.user?.id}] checkPeoplePickerAccess error for req.query.type = ${req.query.type}`, + `[checkPeoplePickerAccess][${req.user?.id}] error for type=${req.query.type}, types=${req.query.types}`, error, ); return res.status(500).json({ diff --git a/api/server/middleware/checkPeoplePickerAccess.spec.js b/api/server/middleware/checkPeoplePickerAccess.spec.js index 52bf0e6724..9a229610de 100644 --- a/api/server/middleware/checkPeoplePickerAccess.spec.js +++ b/api/server/middleware/checkPeoplePickerAccess.spec.js @@ -173,6 +173,171 @@ describe('checkPeoplePickerAccess', () => { expect(next).not.toHaveBeenCalled(); }); + it('should deny access when using types param to bypass type-specific check', async () => { + req.query.types = PrincipalType.GROUP; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for groups', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should deny access when types contains any unpermitted type', async () => { + req.query.types = `${PrincipalType.USER},${PrincipalType.ROLE}`; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for roles', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should allow access when all requested types are permitted', async () => { + req.query.types = `${PrincipalType.USER},${PrincipalType.GROUP}`; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: true, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should validate types when provided as array (Express qs parsing)', async () => { + req.query.types = [PrincipalType.GROUP, PrincipalType.ROLE]; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: true, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for groups', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should enforce permissions for combined type and types params', async () => { + req.query.type = PrincipalType.USER; + req.query.types = PrincipalType.GROUP; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for groups', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should treat all-invalid types values as mixed search', async () => { + req.query.types = 'foobar'; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should deny when types is empty string and user has no permissions', async () => { + req.query.types = ''; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: false, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for users, groups, or roles', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should treat types=public as mixed search since PUBLIC is not a searchable principal type', async () => { + req.query.types = PrincipalType.PUBLIC; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + it('should allow mixed search when user has at least one permission', async () => { // No type specified = mixed search req.query.type = undefined; @@ -222,7 +387,7 @@ describe('checkPeoplePickerAccess', () => { await checkPeoplePickerAccess(req, res, next); expect(logger.error).toHaveBeenCalledWith( - '[checkPeoplePickerAccess][user123] checkPeoplePickerAccess error for req.query.type = undefined', + '[checkPeoplePickerAccess][user123] error for type=undefined, types=undefined', error, ); expect(res.status).toHaveBeenCalledWith(500); diff --git a/packages/data-provider/src/accessPermissions.ts b/packages/data-provider/src/accessPermissions.ts index f2431fcf9a..bc97458076 100644 --- a/packages/data-provider/src/accessPermissions.ts +++ b/packages/data-provider/src/accessPermissions.ts @@ -200,9 +200,9 @@ export type TUpdateResourcePermissionsResponse = z.infer< * Principal search request parameters */ export type TPrincipalSearchParams = { - q: string; // search query (required) - limit?: number; // max results (1-50, default 10) - type?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE; // filter by type (optional) + q: string; + limit?: number; + types?: Array; }; /** @@ -228,7 +228,7 @@ export type TPrincipalSearchResult = { export type TPrincipalSearchResponse = { query: string; limit: number; - type?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE; + types?: Array | null; results: TPrincipalSearchResult[]; count: number; sources: {