diff --git a/api/server/middleware/checkPeoplePickerAccess.js b/api/server/middleware/checkPeoplePickerAccess.js index 9ed8e2939a..3f462362a1 100644 --- a/api/server/middleware/checkPeoplePickerAccess.js +++ b/api/server/middleware/checkPeoplePickerAccess.js @@ -7,7 +7,8 @@ const { logger } = require('~/config'); * Checks specific permission based on the 'type' query parameter: * - type=user: requires VIEW_USERS permission * - type=group: requires VIEW_GROUPS permission - * - no type (mixed search): requires either VIEW_USERS OR VIEW_GROUPS + * - type=role: requires VIEW_ROLES permission + * - no type (mixed search): requires either VIEW_USERS OR VIEW_GROUPS OR VIEW_ROLES */ const checkPeoplePickerAccess = async (req, res, next) => { try { @@ -31,29 +32,38 @@ const checkPeoplePickerAccess = async (req, res, next) => { const peoplePickerPerms = role.permissions[PermissionTypes.PEOPLE_PICKER] || {}; const canViewUsers = peoplePickerPerms[Permissions.VIEW_USERS] === true; const canViewGroups = peoplePickerPerms[Permissions.VIEW_GROUPS] === true; + const canViewRoles = peoplePickerPerms[Permissions.VIEW_ROLES] === true; - if (type === PrincipalType.USER) { - if (!canViewUsers) { - return res.status(403).json({ - error: 'Forbidden', - message: 'Insufficient permissions to search for users', - }); - } - } else if (type === PrincipalType.GROUP) { - if (!canViewGroups) { - return res.status(403).json({ - error: 'Forbidden', - message: 'Insufficient permissions to search for groups', - }); - } - } else { - if (!canViewUsers || !canViewGroups) { - return res.status(403).json({ - error: 'Forbidden', - message: 'Insufficient permissions to search for both users and groups', - }); - } + const permissionChecks = { + [PrincipalType.USER]: { + hasPermission: canViewUsers, + message: 'Insufficient permissions to search for users', + }, + [PrincipalType.GROUP]: { + hasPermission: canViewGroups, + message: 'Insufficient permissions to search for groups', + }, + [PrincipalType.ROLE]: { + hasPermission: canViewRoles, + message: 'Insufficient permissions to search for roles', + }, + }; + + const check = permissionChecks[type]; + if (check && !check.hasPermission) { + return res.status(403).json({ + error: 'Forbidden', + message: check.message, + }); } + + if (!type && !canViewUsers && !canViewGroups && !canViewRoles) { + return res.status(403).json({ + error: 'Forbidden', + message: 'Insufficient permissions to search for users, groups, or roles', + }); + } + next(); } catch (error) { logger.error( diff --git a/api/server/middleware/checkPeoplePickerAccess.spec.js b/api/server/middleware/checkPeoplePickerAccess.spec.js new file mode 100644 index 0000000000..ddbf6f86a9 --- /dev/null +++ b/api/server/middleware/checkPeoplePickerAccess.spec.js @@ -0,0 +1,250 @@ +const { PrincipalType, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { checkPeoplePickerAccess } = require('./checkPeoplePickerAccess'); +const { getRoleByName } = require('~/models/Role'); +const { logger } = require('~/config'); + +jest.mock('~/models/Role'); +jest.mock('~/config', () => ({ + logger: { + error: jest.fn(), + }, +})); + +describe('checkPeoplePickerAccess', () => { + let req, res, next; + + beforeEach(() => { + req = { + user: { id: 'user123', role: 'USER' }, + query: {}, + }; + res = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + next = jest.fn(); + jest.clearAllMocks(); + }); + + it('should return 401 if user is not authenticated', async () => { + req.user = null; + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ + error: 'Unauthorized', + message: 'Authentication required', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should return 403 if role has no permissions', async () => { + getRoleByName.mockResolvedValue(null); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'No permissions configured for user role', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should allow access when searching for users with VIEW_USERS permission', async () => { + req.query.type = PrincipalType.USER; + 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 access when searching for users without VIEW_USERS permission', async () => { + req.query.type = PrincipalType.USER; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: false, + [Permissions.VIEW_GROUPS]: true, + [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 users', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should allow access when searching for groups with VIEW_GROUPS permission', async () => { + req.query.type = PrincipalType.GROUP; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: false, + [Permissions.VIEW_GROUPS]: true, + [Permissions.VIEW_ROLES]: false, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should deny access when searching for groups without VIEW_GROUPS permission', async () => { + req.query.type = PrincipalType.GROUP; + 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 allow access when searching for roles with VIEW_ROLES permission', async () => { + req.query.type = PrincipalType.ROLE; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: false, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: true, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should deny access when searching for roles without VIEW_ROLES permission', async () => { + req.query.type = PrincipalType.ROLE; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: true, + [Permissions.VIEW_GROUPS]: true, + [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 mixed search when user has at least one permission', async () => { + // No type specified = mixed search + req.query.type = undefined; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.PEOPLE_PICKER]: { + [Permissions.VIEW_USERS]: false, + [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: true, + }, + }, + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should deny mixed search when user has no permissions', async () => { + // No type specified = mixed search + req.query.type = undefined; + 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 handle errors gracefully', async () => { + const error = new Error('Database error'); + getRoleByName.mockRejectedValue(error); + + await checkPeoplePickerAccess(req, res, next); + + expect(logger.error).toHaveBeenCalledWith( + '[checkPeoplePickerAccess][user123] checkPeoplePickerAccess error for req.query.type = undefined', + error, + ); + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + error: 'Internal Server Error', + message: 'Failed to check permissions', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should handle missing permissions object gracefully', async () => { + req.query.type = PrincipalType.USER; + getRoleByName.mockResolvedValue({ + permissions: {}, // No PEOPLE_PICKER permissions + }); + + await checkPeoplePickerAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'Insufficient permissions to search for users', + }); + expect(next).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/services/AppService.interface.spec.js b/api/server/services/AppService.interface.spec.js index a2e19ea91d..0b2266f3ec 100644 --- a/api/server/services/AppService.interface.spec.js +++ b/api/server/services/AppService.interface.spec.js @@ -89,4 +89,114 @@ describe('AppService interface configuration', () => { expect(app.locals.interfaceConfig.bookmarks).toBe(false); expect(loadDefaultInterface).toHaveBeenCalled(); }); + + it('should correctly configure peoplePicker permissions including roles', async () => { + mockLoadCustomConfig.mockResolvedValue({ + interface: { + peoplePicker: { + admin: { + users: true, + groups: true, + roles: true, + }, + user: { + users: false, + groups: false, + roles: false, + }, + }, + }, + }); + loadDefaultInterface.mockResolvedValue({ + peoplePicker: { + admin: { + users: true, + groups: true, + roles: true, + }, + user: { + users: false, + groups: false, + roles: false, + }, + }, + }); + + await AppService(app); + + expect(app.locals.interfaceConfig.peoplePicker).toBeDefined(); + expect(app.locals.interfaceConfig.peoplePicker.admin).toMatchObject({ + users: true, + groups: true, + roles: true, + }); + expect(app.locals.interfaceConfig.peoplePicker.user).toMatchObject({ + users: false, + groups: false, + roles: false, + }); + expect(loadDefaultInterface).toHaveBeenCalled(); + }); + + it('should handle mixed peoplePicker permissions for roles', async () => { + mockLoadCustomConfig.mockResolvedValue({ + interface: { + peoplePicker: { + admin: { + users: true, + groups: true, + roles: false, + }, + user: { + users: true, + groups: false, + roles: true, + }, + }, + }, + }); + loadDefaultInterface.mockResolvedValue({ + peoplePicker: { + admin: { + users: true, + groups: true, + roles: false, + }, + user: { + users: true, + groups: false, + roles: true, + }, + }, + }); + + await AppService(app); + + expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(false); + expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(true); + }); + + it('should set default peoplePicker roles permissions when not provided', async () => { + mockLoadCustomConfig.mockResolvedValue({}); + loadDefaultInterface.mockResolvedValue({ + peoplePicker: { + admin: { + users: true, + groups: true, + roles: true, + }, + user: { + users: false, + groups: false, + roles: false, + }, + }, + }); + + await AppService(app); + + expect(app.locals.interfaceConfig.peoplePicker).toBeDefined(); + expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(true); + expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(false); + }); }); diff --git a/api/server/services/AppService.spec.js b/api/server/services/AppService.spec.js index 15fbe1c555..f1c5c4076e 100644 --- a/api/server/services/AppService.spec.js +++ b/api/server/services/AppService.spec.js @@ -969,4 +969,59 @@ describe('AppService updating app.locals and issuing warnings', () => { expect(app.locals.ocr.strategy).toEqual('mistral_ocr'); expect(app.locals.ocr.mistralModel).toEqual('mistral-medium'); }); + + it('should correctly configure peoplePicker with roles permission when specified', async () => { + const mockConfig = { + interface: { + peoplePicker: { + admin: { + users: true, + groups: true, + roles: true, + }, + user: { + users: false, + groups: false, + roles: true, + }, + }, + }, + }; + + require('./Config/loadCustomConfig').mockImplementationOnce(() => Promise.resolve(mockConfig)); + + const app = { locals: {} }; + await AppService(app); + + // Check that interface config includes the roles permission + expect(app.locals.interfaceConfig.peoplePicker).toBeDefined(); + expect(app.locals.interfaceConfig.peoplePicker.admin).toMatchObject({ + users: true, + groups: true, + roles: true, + }); + expect(app.locals.interfaceConfig.peoplePicker.user).toMatchObject({ + users: false, + groups: false, + roles: true, + }); + }); + + it('should use default peoplePicker roles permissions when not specified', async () => { + const mockConfig = { + interface: { + // No peoplePicker configuration + }, + }; + + require('./Config/loadCustomConfig').mockImplementationOnce(() => Promise.resolve(mockConfig)); + + const app = { locals: {} }; + await AppService(app); + + // Check that default roles permissions are applied + expect(app.locals.interfaceConfig.peoplePicker).toBeDefined(); + expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(true); + expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(false); + }); }); diff --git a/api/server/services/start/interface.js b/api/server/services/start/interface.js index 554cfd56d1..a3cf5b295f 100644 --- a/api/server/services/start/interface.js +++ b/api/server/services/start/interface.js @@ -57,10 +57,12 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol admin: { users: interfaceConfig?.peoplePicker?.admin?.users ?? defaults.peoplePicker?.admin.users, groups: interfaceConfig?.peoplePicker?.admin?.groups ?? defaults.peoplePicker?.admin.groups, + roles: interfaceConfig?.peoplePicker?.admin?.roles ?? defaults.peoplePicker?.admin.roles, }, user: { users: interfaceConfig?.peoplePicker?.user?.users ?? defaults.peoplePicker?.user.users, groups: interfaceConfig?.peoplePicker?.user?.groups ?? defaults.peoplePicker?.user.groups, + roles: interfaceConfig?.peoplePicker?.user?.roles ?? defaults.peoplePicker?.user.roles, }, }, marketplace: { @@ -88,6 +90,7 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: loadedInterface.peoplePicker.user?.users, [Permissions.VIEW_GROUPS]: loadedInterface.peoplePicker.user?.groups, + [Permissions.VIEW_ROLES]: loadedInterface.peoplePicker.user?.roles, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: loadedInterface.marketplace.user?.use, @@ -110,6 +113,7 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: loadedInterface.peoplePicker.admin?.users, [Permissions.VIEW_GROUPS]: loadedInterface.peoplePicker.admin?.groups, + [Permissions.VIEW_ROLES]: loadedInterface.peoplePicker.admin?.roles, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: loadedInterface.marketplace.admin?.use, diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 29a453d6d3..ac251e6da9 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -538,12 +538,14 @@ export const interfaceSchema = z .object({ users: z.boolean().optional(), groups: z.boolean().optional(), + roles: z.boolean().optional(), }) .optional(), user: z .object({ users: z.boolean().optional(), groups: z.boolean().optional(), + roles: z.boolean().optional(), }) .optional(), }) @@ -583,10 +585,12 @@ export const interfaceSchema = z admin: { users: true, groups: true, + roles: true, }, user: { users: false, groups: false, + roles: false, }, }, marketplace: { diff --git a/packages/data-provider/src/permissions.ts b/packages/data-provider/src/permissions.ts index a62916c7a2..1621ea73e0 100644 --- a/packages/data-provider/src/permissions.ts +++ b/packages/data-provider/src/permissions.ts @@ -69,6 +69,7 @@ export enum Permissions { OPT_OUT = 'OPT_OUT', VIEW_USERS = 'VIEW_USERS', VIEW_GROUPS = 'VIEW_GROUPS', + VIEW_ROLES = 'VIEW_ROLES', } export const promptPermissionsSchema = z.object({ @@ -124,6 +125,7 @@ export type TWebSearchPermissions = z.infer; export const peoplePickerPermissionsSchema = z.object({ [Permissions.VIEW_USERS]: z.boolean().default(true), [Permissions.VIEW_GROUPS]: z.boolean().default(true), + [Permissions.VIEW_ROLES]: z.boolean().default(true), }); export type TPeoplePickerPermissions = z.infer;