diff --git a/api/server/routes/__tests__/roles.spec.js b/api/server/routes/__tests__/roles.spec.js new file mode 100644 index 0000000000..e78965d460 --- /dev/null +++ b/api/server/routes/__tests__/roles.spec.js @@ -0,0 +1,155 @@ +const express = require('express'); +const request = require('supertest'); +const { SystemRoles, roleDefaults } = require('librechat-data-provider'); + +const mockGetRoleByName = jest.fn(); +const mockHasCapability = jest.fn(); + +jest.mock('~/server/middleware', () => ({ + requireJwtAuth: (_req, _res, next) => next(), +})); + +jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: (...args) => mockHasCapability(...args), + requireCapability: () => (_req, _res, next) => next(), +})); + +jest.mock('~/models', () => ({ + getRoleByName: (...args) => mockGetRoleByName(...args), + updateRoleByName: jest.fn(), +})); + +const rolesRouter = require('../roles'); + +function createApp(user) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.user = user; + next(); + }); + app.use('/api/roles', rolesRouter); + return app; +} + +const staffRole = { + name: 'STAFF', + permissions: { + PROMPTS: { USE: true, CREATE: false }, + }, +}; + +const userRole = roleDefaults[SystemRoles.USER]; +const adminRole = roleDefaults[SystemRoles.ADMIN]; + +beforeEach(() => { + jest.clearAllMocks(); + mockHasCapability.mockResolvedValue(false); + mockGetRoleByName.mockResolvedValue(null); +}); + +describe('GET /api/roles/:roleName — isOwnRole authorization', () => { + it('allows a custom role user to fetch their own role', async () => { + mockGetRoleByName.mockResolvedValue(staffRole); + const app = createApp({ id: 'u1', role: 'STAFF' }); + + const res = await request(app).get('/api/roles/STAFF'); + + expect(res.status).toBe(200); + expect(res.body.name).toBe('STAFF'); + expect(mockGetRoleByName).toHaveBeenCalledWith('STAFF', '-_id -__v'); + }); + + it('returns 403 when a custom role user requests a different custom role', async () => { + const app = createApp({ id: 'u1', role: 'STAFF' }); + + const res = await request(app).get('/api/roles/MANAGER'); + + expect(res.status).toBe(403); + expect(mockGetRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 403 when a custom role user requests ADMIN', async () => { + const app = createApp({ id: 'u1', role: 'STAFF' }); + + const res = await request(app).get('/api/roles/ADMIN'); + + expect(res.status).toBe(403); + expect(mockGetRoleByName).not.toHaveBeenCalled(); + }); + + it('allows USER to fetch the USER role (roleDefaults key)', async () => { + mockGetRoleByName.mockResolvedValue(userRole); + const app = createApp({ id: 'u1', role: SystemRoles.USER }); + + const res = await request(app).get(`/api/roles/${SystemRoles.USER}`); + + expect(res.status).toBe(200); + }); + + it('returns 403 when USER requests the ADMIN role', async () => { + const app = createApp({ id: 'u1', role: SystemRoles.USER }); + + const res = await request(app).get(`/api/roles/${SystemRoles.ADMIN}`); + + expect(res.status).toBe(403); + }); + + it('allows ADMIN user to fetch their own ADMIN role via isOwnRole', async () => { + mockHasCapability.mockResolvedValue(false); + mockGetRoleByName.mockResolvedValue(adminRole); + const app = createApp({ id: 'u1', role: SystemRoles.ADMIN }); + + const res = await request(app).get(`/api/roles/${SystemRoles.ADMIN}`); + + expect(res.status).toBe(200); + }); + + it('allows any user with READ_ROLES capability to fetch any role', async () => { + mockHasCapability.mockResolvedValue(true); + mockGetRoleByName.mockResolvedValue(staffRole); + const app = createApp({ id: 'u1', role: SystemRoles.USER }); + + const res = await request(app).get('/api/roles/STAFF'); + + expect(res.status).toBe(200); + expect(res.body.name).toBe('STAFF'); + }); + + it('returns 404 when the requested role does not exist', async () => { + mockGetRoleByName.mockResolvedValue(null); + const app = createApp({ id: 'u1', role: 'GHOST' }); + + const res = await request(app).get('/api/roles/GHOST'); + + expect(res.status).toBe(404); + }); + + it('returns 500 when getRoleByName throws', async () => { + mockGetRoleByName.mockRejectedValue(new Error('db error')); + const app = createApp({ id: 'u1', role: SystemRoles.USER }); + + const res = await request(app).get(`/api/roles/${SystemRoles.USER}`); + + expect(res.status).toBe(500); + }); + + it('returns 403 for prototype property names like constructor (no prototype pollution)', async () => { + const app = createApp({ id: 'u1', role: 'STAFF' }); + + const res = await request(app).get('/api/roles/constructor'); + + expect(res.status).toBe(403); + expect(mockGetRoleByName).not.toHaveBeenCalled(); + }); + + it('treats hasCapability failure as no capability (does not 500)', async () => { + mockHasCapability.mockRejectedValue(new Error('capability check failed')); + const app = createApp({ id: 'u1', role: 'STAFF' }); + mockGetRoleByName.mockResolvedValue(staffRole); + + const res = await request(app).get('/api/roles/STAFF'); + + expect(res.status).toBe(200); + }); +}); diff --git a/api/server/routes/roles.js b/api/server/routes/roles.js index 25ee47854d..456d144af1 100644 --- a/api/server/routes/roles.js +++ b/api/server/routes/roles.js @@ -71,9 +71,7 @@ const createPermissionUpdateHandler = (permissionKey) => { const config = permissionConfigs[permissionKey]; return async (req, res) => { - const { roleName: _r } = req.params; - // TODO: TEMP, use a better parsing for roleName - const roleName = _r.toUpperCase(); + const { roleName } = req.params; const updates = req.body; try { @@ -110,9 +108,7 @@ const createPermissionUpdateHandler = (permissionKey) => { * Get a specific role by name */ router.get('/:roleName', async (req, res) => { - const { roleName: _r } = req.params; - // TODO: TEMP, use a better parsing for roleName - const roleName = _r.toUpperCase(); + const { roleName } = req.params; try { let hasReadRoles = false; @@ -121,7 +117,9 @@ router.get('/:roleName', async (req, res) => { } catch (err) { logger.warn(`[GET /roles/:roleName] capability check failed: ${err.message}`); } - if (!hasReadRoles && (roleName === SystemRoles.ADMIN || !roleDefaults[roleName])) { + const isOwnRole = req.user?.role === roleName; + const isDefaultRole = Object.hasOwn(roleDefaults, roleName); + if (!hasReadRoles && !isOwnRole && (roleName === SystemRoles.ADMIN || !isDefaultRole)) { return res.status(403).send({ message: 'Unauthorized' }); } diff --git a/client/src/components/Sharing/PeoplePickerAdminSettings.tsx b/client/src/components/Sharing/PeoplePickerAdminSettings.tsx index 38d44ad311..5f334f3764 100644 --- a/client/src/components/Sharing/PeoplePickerAdminSettings.tsx +++ b/client/src/components/Sharing/PeoplePickerAdminSettings.tsx @@ -1,8 +1,8 @@ -import { useMemo, useEffect, useState } from 'react'; +import { useEffect, useState } from 'react'; import * as Ariakit from '@ariakit/react'; import { ShieldEllipsis } from 'lucide-react'; import { useForm, Controller } from 'react-hook-form'; -import { Permissions, SystemRoles, roleDefaults, PermissionTypes } from 'librechat-data-provider'; +import { Permissions, SystemRoles, PermissionTypes } from 'librechat-data-provider'; import { Button, Switch, @@ -15,7 +15,7 @@ import { } from '@librechat/client'; import type { Control, UseFormSetValue, UseFormGetValues } from 'react-hook-form'; import { useUpdatePeoplePickerPermissionsMutation } from '~/data-provider'; -import { useLocalize, useAuthContext } from '~/hooks'; +import { useLocalize, useAuthContext, useRoleSelector } from '~/hooks'; type FormValues = { [Permissions.VIEW_USERS]: boolean; @@ -70,7 +70,7 @@ const LabelController: React.FC = ({ const PeoplePickerAdminSettings = () => { const localize = useLocalize(); const { showToast } = useToastContext(); - const { user, roles } = useAuthContext(); + const { user } = useAuthContext(); const { mutate, isLoading } = useUpdatePeoplePickerPermissionsMutation({ onSuccess: () => { showToast({ status: 'success', message: localize('com_ui_saved') }); @@ -81,15 +81,14 @@ const PeoplePickerAdminSettings = () => { }); const [isRoleMenuOpen, setIsRoleMenuOpen] = useState(false); - const [selectedRole, setSelectedRole] = useState(SystemRoles.USER); - - const defaultValues = useMemo(() => { - const rolePerms = roles?.[selectedRole]?.permissions; - if (rolePerms) { - return rolePerms[PermissionTypes.PEOPLE_PICKER]; - } - return roleDefaults[selectedRole].permissions[PermissionTypes.PEOPLE_PICKER]; - }, [roles, selectedRole]); + const { + selectedRole, + isSelectedCustomRole, + isCustomRoleLoading, + isCustomRoleError, + defaultValues, + roleDropdownItems, + } = useRoleSelector(PermissionTypes.PEOPLE_PICKER); const { reset, @@ -100,17 +99,15 @@ const PeoplePickerAdminSettings = () => { formState: { isSubmitting }, } = useForm({ mode: 'onChange', - defaultValues, + defaultValues: defaultValues as FormValues, }); useEffect(() => { - const value = roles?.[selectedRole]?.permissions?.[PermissionTypes.PEOPLE_PICKER]; - if (value) { - reset(value); - } else { - reset(roleDefaults[selectedRole].permissions[PermissionTypes.PEOPLE_PICKER]); + if (isSelectedCustomRole && (isCustomRoleLoading || isCustomRoleError)) { + return; } - }, [roles, selectedRole, reset]); + reset(defaultValues as FormValues); + }, [isSelectedCustomRole, isCustomRoleLoading, isCustomRoleError, defaultValues, reset]); if (user?.role !== SystemRoles.ADMIN) { return null; @@ -138,21 +135,6 @@ const PeoplePickerAdminSettings = () => { mutate({ roleName: selectedRole, updates: data }); }; - const roleDropdownItems = [ - { - label: SystemRoles.USER, - onClick: () => { - setSelectedRole(SystemRoles.USER); - }, - }, - { - label: SystemRoles.ADMIN, - onClick: () => { - setSelectedRole(SystemRoles.ADMIN); - }, - }, - ]; - return ( @@ -179,7 +161,7 @@ const PeoplePickerAdminSettings = () => { isOpen={isRoleMenuOpen} setIsOpen={setIsRoleMenuOpen} trigger={ - + {selectedRole} } @@ -207,7 +189,11 @@ const PeoplePickerAdminSettings = () => {