mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-22 19:30:15 +01:00
feat: Enhance people picker access control to include roles permissions
This commit is contained in:
parent
4736a60c47
commit
8e003083dc
7 changed files with 457 additions and 22 deletions
|
|
@ -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(
|
||||
|
|
|
|||
250
api/server/middleware/checkPeoplePickerAccess.spec.js
Normal file
250
api/server/middleware/checkPeoplePickerAccess.spec.js
Normal file
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -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<typeof webSearchPermissionsSchema>;
|
|||
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<typeof peoplePickerPermissionsSchema>;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue