diff --git a/api/app/clients/tools/util/fileSearch.js b/api/app/clients/tools/util/fileSearch.js index 7221e9e3f4..c7e7c88336 100644 --- a/api/app/clients/tools/util/fileSearch.js +++ b/api/app/clients/tools/util/fileSearch.js @@ -30,7 +30,12 @@ const primeFiles = async (options) => { // Filter by access if user and agent are provided let dbFiles; if (req?.user?.id && agentId) { - dbFiles = await filterFilesByAgentAccess(allFiles, req.user.id, agentId); + dbFiles = await filterFilesByAgentAccess({ + files: allFiles, + userId: req.user.id, + role: req.user.role, + agentId, + }); } else { dbFiles = allFiles; } diff --git a/api/models/File.spec.js b/api/models/File.spec.js index a07d3721af..dd0981d1fe 100644 --- a/api/models/File.spec.js +++ b/api/models/File.spec.js @@ -2,7 +2,12 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); const { createModels } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); +const { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, +} = require('librechat-data-provider'); const { grantPermission } = require('~/server/services/PermissionService'); const { getFiles, createFile } = require('./File'); const { seedDefaultRoles } = require('~/models'); @@ -125,7 +130,12 @@ describe('File Access Control', () => { // Check access for all files const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); - const accessMap = await hasAccessToFilesViaAgent(userId.toString(), fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent({ + userId: userId, + role: SystemRoles.USER, + fileIds, + agentId: agent.id, // Use agent.id which is the custom UUID + }); // Should have access only to the first two files expect(accessMap.get(fileIds[0])).toBe(true); @@ -163,7 +173,12 @@ describe('File Access Control', () => { // Check access as the author const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); - const accessMap = await hasAccessToFilesViaAgent(authorId.toString(), fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent({ + userId: authorId, + role: SystemRoles.USER, + fileIds, + agentId, + }); // Author should have access to all files expect(accessMap.get(fileIds[0])).toBe(true); @@ -184,11 +199,12 @@ describe('File Access Control', () => { }); const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); - const accessMap = await hasAccessToFilesViaAgent( - userId.toString(), + const accessMap = await hasAccessToFilesViaAgent({ + userId: userId, + role: SystemRoles.USER, fileIds, - 'non-existent-agent', - ); + agentId: 'non-existent-agent', + }); // Should have no access to any files expect(accessMap.get(fileIds[0])).toBe(false); @@ -242,7 +258,12 @@ describe('File Access Control', () => { // Check access for files const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); - const accessMap = await hasAccessToFilesViaAgent(userId.toString(), fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent({ + userId: userId, + role: SystemRoles.USER, + fileIds, + agentId, + }); // Should have no access to any files when only VIEW permission expect(accessMap.get(fileIds[0])).toBe(false); @@ -336,7 +357,12 @@ describe('File Access Control', () => { // Then filter by access control const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); - const files = await filterFilesByAgentAccess(allFiles, userId.toString(), agentId); + const files = await filterFilesByAgentAccess({ + files: allFiles, + userId: userId, + role: SystemRoles.USER, + agentId, + }); expect(files).toHaveLength(2); expect(files.map((f) => f.file_id)).toContain(ownedFileId); @@ -371,4 +397,166 @@ describe('File Access Control', () => { expect(files).toHaveLength(2); }); }); + + describe('Role-based file permissions', () => { + it('should optimize permission checks when role is provided', async () => { + const userId = new mongoose.Types.ObjectId(); + const authorId = new mongoose.Types.ObjectId(); + const agentId = uuidv4(); + const fileIds = [uuidv4(), uuidv4()]; + + // Create users + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + role: 'ADMIN', // User has ADMIN role + }); + + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); + + // Create files + for (const fileId of fileIds) { + await createFile({ + file_id: fileId, + user: authorId, + filename: `${fileId}.txt`, + filepath: `/uploads/${fileId}.txt`, + type: 'text/plain', + bytes: 100, + }); + } + + // Create agent with files + const agent = await createAgent({ + id: agentId, + name: 'Test Agent', + author: authorId, + model: 'gpt-4', + provider: 'openai', + tool_resources: { + file_search: { + file_ids: fileIds, + }, + }, + }); + + // Grant permission to ADMIN role + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'ADMIN', + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: authorId, + }); + + // Check access with role provided (should avoid DB query) + const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); + const accessMapWithRole = await hasAccessToFilesViaAgent({ + userId: userId, + role: 'ADMIN', + fileIds, + agentId: agent.id, + }); + + // User should have access through their ADMIN role + expect(accessMapWithRole.get(fileIds[0])).toBe(true); + expect(accessMapWithRole.get(fileIds[1])).toBe(true); + + // Check access without role (will query DB to get user's role) + const accessMapWithoutRole = await hasAccessToFilesViaAgent({ + userId: userId, + fileIds, + agentId: agent.id, + }); + + // Should have same result + expect(accessMapWithoutRole.get(fileIds[0])).toBe(true); + expect(accessMapWithoutRole.get(fileIds[1])).toBe(true); + }); + + it('should deny access when user role changes', async () => { + const userId = new mongoose.Types.ObjectId(); + const authorId = new mongoose.Types.ObjectId(); + const agentId = uuidv4(); + const fileId = uuidv4(); + + // Create users + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + role: 'EDITOR', + }); + + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); + + // Create file + await createFile({ + file_id: fileId, + user: authorId, + filename: 'test.txt', + filepath: '/uploads/test.txt', + type: 'text/plain', + bytes: 100, + }); + + // Create agent + const agent = await createAgent({ + id: agentId, + name: 'Test Agent', + author: authorId, + model: 'gpt-4', + provider: 'openai', + tool_resources: { + file_search: { + file_ids: [fileId], + }, + }, + }); + + // Grant permission to EDITOR role only + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'EDITOR', + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: authorId, + }); + + const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); + + // Check with EDITOR role - should have access + const accessAsEditor = await hasAccessToFilesViaAgent({ + userId: userId, + role: 'EDITOR', + fileIds: [fileId], + agentId: agent.id, + }); + expect(accessAsEditor.get(fileId)).toBe(true); + + // Simulate role change to USER - should lose access + const accessAsUser = await hasAccessToFilesViaAgent({ + userId: userId, + role: SystemRoles.USER, + fileIds: [fileId], + agentId: agent.id, + }); + expect(accessAsUser.get(fileId)).toBe(false); + }); + }); }); diff --git a/api/models/Prompt.spec.js b/api/models/Prompt.spec.js index b3265bb340..e00a1a518c 100644 --- a/api/models/Prompt.spec.js +++ b/api/models/Prompt.spec.js @@ -334,10 +334,8 @@ describe('Prompt ACL Permissions', () => { productionId: new ObjectId(), }); - // Add editor to the editors group - await Group.findByIdAndUpdate(testGroups.editors._id, { - $push: { memberIds: testUsers.editor._id }, - }); + const { addUserToGroup } = require('~/models'); + await addUserToGroup(testUsers.editor._id, testGroups.editors._id); const prompt = await promptFns.savePrompt({ author: testUsers.owner._id, diff --git a/api/server/controllers/PermissionsController.js b/api/server/controllers/PermissionsController.js index 7562beb100..fab5593963 100644 --- a/api/server/controllers/PermissionsController.js +++ b/api/server/controllers/PermissionsController.js @@ -72,7 +72,7 @@ const updateResourcePermissions = async (req, res) => { // Add public permission if enabled if (isPublic && publicAccessRoleId) { updatedPrincipals.push({ - type: 'public', + type: PrincipalType.PUBLIC, id: null, accessRoleId: publicAccessRoleId, }); @@ -97,11 +97,13 @@ const updateResourcePermissions = async (req, res) => { try { let principalId; - if (principal.type === 'public') { + if (principal.type === PrincipalType.PUBLIC) { principalId = null; // Public principals don't need database records - } else if (principal.type === 'user') { + } else if (principal.type === PrincipalType.ROLE) { + principalId = principal.id; // Role principals use role name as ID + } else if (principal.type === PrincipalType.USER) { principalId = await ensurePrincipalExists(principal); - } else if (principal.type === 'group') { + } else if (principal.type === PrincipalType.GROUP) { // Pass authContext to enable member fetching for Entra ID groups when available principalId = await ensureGroupPrincipalExists(principal, authContext); } else { @@ -137,7 +139,7 @@ const updateResourcePermissions = async (req, res) => { // If public is disabled, add public to revoked list if (!isPublic) { revokedPrincipals.push({ - type: 'public', + type: PrincipalType.PUBLIC, id: null, }); } @@ -263,6 +265,16 @@ const getResourcePermissions = async (req, res) => { idOnTheSource: result.groupInfo.idOnTheSource || result.groupInfo._id.toString(), accessRoleId: result.accessRoleId, }); + } else if (result.principalType === PrincipalType.ROLE) { + principals.push({ + type: PrincipalType.ROLE, + /** Role name as ID */ + id: result.principalId, + /** Display the role name */ + name: result.principalId, + description: `System role: ${result.principalId}`, + accessRoleId: result.accessRoleId, + }); } } @@ -328,6 +340,7 @@ const getUserEffectivePermissions = async (req, res) => { const permissionBits = await getEffectivePermissions({ userId, + role: req.user.role, resourceType, resourceId, }); @@ -366,7 +379,9 @@ const searchPrincipals = async (req, res) => { } const searchLimit = Math.min(Math.max(1, parseInt(limit) || 10), 50); - const typeFilter = ['user', 'group'].includes(type) ? type : null; + const typeFilter = [PrincipalType.USER, PrincipalType.GROUP, PrincipalType.ROLE].includes(type) + ? type + : null; const localResults = await searchLocalPrincipals(query.trim(), searchLimit, typeFilter); let allPrincipals = [...localResults]; diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 7fae5ac78c..df009c0b0c 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -444,6 +444,7 @@ const getListAgentsHandler = async (req, res) => { // Get agent IDs the user has VIEW access to via ACL const accessibleIds = await findAccessibleResources({ userId, + role: req.user.role, resourceType: ResourceType.AGENT, requiredPermissions: requiredPermission, }); @@ -499,7 +500,7 @@ const uploadAgentAvatarHandler = async (req, res) => { return res.status(404).json({ error: 'Agent not found' }); } - const isAuthor = existingAgent.author.toString() === req.user.id; + const isAuthor = existingAgent.author.toString() === req.user.id.toString(); const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor; if (!hasEditPermission) { @@ -607,7 +608,7 @@ const revertAgentVersionHandler = async (req, res) => { return res.status(404).json({ error: 'Agent not found' }); } - const isAuthor = existingAgent.author.toString() === req.user.id; + const isAuthor = existingAgent.author.toString() === req.user.id.toString(); const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor; if (!hasEditPermission) { diff --git a/api/server/middleware/accessResources/canAccessAgentResource.spec.js b/api/server/middleware/accessResources/canAccessAgentResource.spec.js index c7f3efc6bb..e3dca73bd2 100644 --- a/api/server/middleware/accessResources/canAccessAgentResource.spec.js +++ b/api/server/middleware/accessResources/canAccessAgentResource.spec.js @@ -43,7 +43,7 @@ describe('canAccessAgentResource middleware', () => { }); req = { - user: { id: testUser._id.toString(), role: 'test-role' }, + user: { id: testUser._id, role: testUser.role }, params: {}, }; res = { diff --git a/api/server/middleware/accessResources/canAccessResource.js b/api/server/middleware/accessResources/canAccessResource.js index a236d3cffe..c8bd15ffc2 100644 --- a/api/server/middleware/accessResources/canAccessResource.js +++ b/api/server/middleware/accessResources/canAccessResource.js @@ -111,6 +111,7 @@ const canAccessResource = (options) => { // Check permissions using PermissionService with ObjectId const hasPermission = await checkPermission({ userId, + role: req.user.role, resourceType, resourceId, requiredPermission, diff --git a/api/server/middleware/accessResources/fileAccess.js b/api/server/middleware/accessResources/fileAccess.js index d2d084f3a5..2d588e623e 100644 --- a/api/server/middleware/accessResources/fileAccess.js +++ b/api/server/middleware/accessResources/fileAccess.js @@ -8,7 +8,7 @@ const { getFiles } = require('~/models/File'); * Checks if user has access to a file through agent permissions * Files inherit permissions from agents - if you can view the agent, you can access its files */ -const checkAgentBasedFileAccess = async (userId, fileId) => { +const checkAgentBasedFileAccess = async ({ userId, role, fileId }) => { try { // Find agents that have this file in their tool_resources const agentsWithFile = await getAgent({ @@ -35,6 +35,7 @@ const checkAgentBasedFileAccess = async (userId, fileId) => { try { const permissions = await getEffectivePermissions({ userId, + role, resourceType: ResourceType.AGENT, resourceId: agent._id || agent.id, }); @@ -67,7 +68,7 @@ const fileAccess = async (req, res, next) => { try { const fileId = req.params.file_id; const userId = req.user?.id; - + const userRole = req.user?.role; if (!fileId) { return res.status(400).json({ error: 'Bad Request', @@ -98,7 +99,7 @@ const fileAccess = async (req, res, next) => { } // Check agent-based access (file inherits agent permissions) - const hasAgentAccess = await checkAgentBasedFileAccess(userId, fileId); + const hasAgentAccess = await checkAgentBasedFileAccess({ userId, role: userRole, fileId }); if (hasAgentAccess) { req.fileAccess = { file }; return next(); diff --git a/api/server/middleware/checkPeoplePickerAccess.js b/api/server/middleware/checkPeoplePickerAccess.js index b2931608ff..3f462362a1 100644 --- a/api/server/middleware/checkPeoplePickerAccess.js +++ b/api/server/middleware/checkPeoplePickerAccess.js @@ -1,4 +1,4 @@ -const { PermissionTypes, Permissions } = require('librechat-data-provider'); +const { PrincipalType, PermissionTypes, Permissions } = require('librechat-data-provider'); const { getRoleByName } = require('~/models/Role'); const { logger } = require('~/config'); @@ -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 === 'user') { - if (!canViewUsers) { - return res.status(403).json({ - error: 'Forbidden', - message: 'Insufficient permissions to search for users', - }); - } - } else if (type === '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/routes/agents/actions.js b/api/server/routes/agents/actions.js index 8b7bf74e5e..c3a41bc771 100644 --- a/api/server/routes/agents/actions.js +++ b/api/server/routes/agents/actions.js @@ -37,6 +37,7 @@ router.get('/', async (req, res) => { const userId = req.user.id; const editableAgentObjectIds = await findAccessibleResources({ userId, + role: req.user.role, resourceType: ResourceType.AGENT, requiredPermissions: PermissionBits.EDIT, }); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index a5bda3100d..1708d99536 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -79,6 +79,7 @@ router.get('/agent/:agent_id', async (req, res) => { if (agent.author.toString() !== userId) { const hasEditPermission = await checkPermission({ userId, + role: req.user.role, resourceType: ResourceType.AGENT, resourceId: agent._id, requiredPermission: PermissionBits.EDIT, @@ -152,7 +153,7 @@ router.delete('/', async (req, res) => { const nonOwnedFiles = []; for (const file of dbFiles) { - if (file.user.toString() === req.user.id) { + if (file.user.toString() === req.user.id.toString()) { ownedFiles.push(file); } else { nonOwnedFiles.push(file); @@ -176,11 +177,12 @@ router.delete('/', async (req, res) => { if (req.body.agent_id && nonOwnedFiles.length > 0) { const nonOwnedFileIds = nonOwnedFiles.map((f) => f.file_id); - const accessMap = await hasAccessToFilesViaAgent( - req.user.id, - nonOwnedFileIds, - req.body.agent_id, - ); + const accessMap = await hasAccessToFilesViaAgent({ + userId: req.user.id, + role: req.user.role, + fileIds: nonOwnedFileIds, + agentId: req.body.agent_id, + }); for (const file of nonOwnedFiles) { if (accessMap.get(file.file_id)) { diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index fbbd289595..896542b6f4 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -4,7 +4,12 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); const { createMethods } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); +const { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, +} = require('librechat-data-provider'); const { createAgent } = require('~/models/Agent'); const { createFile } = require('~/models/File'); @@ -95,9 +100,11 @@ describe('File Routes - Delete with Agent Access', () => { app = express(); app.use(express.json()); - // Mock authentication middleware app.use((req, res, next) => { - req.user = { id: otherUserId ? otherUserId.toString() : 'default-user' }; + req.user = { + id: otherUserId || 'default-user', + role: SystemRoles.USER, + }; req.app = { locals: {} }; next(); }); diff --git a/api/server/routes/prompts.js b/api/server/routes/prompts.js index b5e5ea5039..6384ad1b1d 100644 --- a/api/server/routes/prompts.js +++ b/api/server/routes/prompts.js @@ -99,6 +99,7 @@ router.get('/all', async (req, res) => { // Get promptGroup IDs the user has VIEW access to via ACL const accessibleIds = await findAccessibleResources({ userId, + role: req.user.role, resourceType: ResourceType.PROMPTGROUP, requiredPermissions: PermissionBits.VIEW, }); @@ -130,6 +131,7 @@ router.get('/groups', async (req, res) => { // Get promptGroup IDs the user has VIEW access to via ACL const accessibleIds = await findAccessibleResources({ userId, + role: req.user.role, resourceType: ResourceType.PROMPTGROUP, requiredPermissions: PermissionBits.VIEW, }); @@ -334,6 +336,7 @@ router.get('/', async (req, res) => { if (groupId) { const permissions = await getEffectivePermissions({ userId: req.user.id, + role: req.user.role, resourceType: ResourceType.PROMPTGROUP, resourceId: groupId, }); diff --git a/api/server/routes/prompts.test.js b/api/server/routes/prompts.test.js index 812320fd6b..530d5d67fa 100644 --- a/api/server/routes/prompts.test.js +++ b/api/server/routes/prompts.test.js @@ -214,8 +214,6 @@ describe('Prompt Routes - ACL Permissions', () => { console.log('Console errors:', consoleErrorSpy.mock.calls); } - console.log('POST response:', response.body); - expect(response.status).toBe(200); expect(response.body.prompt).toBeDefined(); expect(response.body.prompt.prompt).toBe(promptData.prompt.prompt); @@ -303,8 +301,8 @@ describe('Prompt Routes - ACL Permissions', () => { grantedBy: testUsers.owner._id, }); - const response = await request(app).get(`/api/prompts/${testPrompt._id}`).expect(200); - + const response = await request(app).get(`/api/prompts/${testPrompt._id}`); + expect(response.status).toBe(200); expect(response.body._id).toBe(testPrompt._id.toString()); expect(response.body.prompt).toBe(testPrompt.prompt); }); 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/Files/Code/process.js b/api/server/services/Files/Code/process.js index f3f5687b53..420451ab6d 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -172,7 +172,12 @@ const primeFiles = async (options, apiKey) => { // Filter by access if user and agent are provided let dbFiles; if (req?.user?.id && agentId) { - dbFiles = await filterFilesByAgentAccess(allFiles, req.user.id, agentId); + dbFiles = await filterFilesByAgentAccess({ + files: allFiles, + userId: req.user.id, + role: req.user.role, + agentId, + }); } else { dbFiles = allFiles; } diff --git a/api/server/services/Files/permissions.js b/api/server/services/Files/permissions.js index 37eaf50cc6..6a7296bbde 100644 --- a/api/server/services/Files/permissions.js +++ b/api/server/services/Files/permissions.js @@ -5,12 +5,14 @@ const { getAgent } = require('~/models/Agent'); /** * Checks if a user has access to multiple files through a shared agent (batch operation) - * @param {string} userId - The user ID to check access for - * @param {string[]} fileIds - Array of file IDs to check - * @param {string} agentId - The agent ID that might grant access + * @param {Object} params - Parameters object + * @param {string} params.userId - The user ID to check access for + * @param {string} [params.role] - Optional user role to avoid DB query + * @param {string[]} params.fileIds - Array of file IDs to check + * @param {string} params.agentId - The agent ID that might grant access * @returns {Promise>} Map of fileId to access status */ -const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { +const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId }) => { const accessMap = new Map(); // Initialize all files as no access @@ -24,7 +26,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { } // Check if user is the author - if so, grant access to all files - if (agent.author.toString() === userId) { + if (agent.author.toString() === userId.toString()) { fileIds.forEach((fileId) => accessMap.set(fileId, true)); return accessMap; } @@ -32,6 +34,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { // Check if user has at least VIEW permission on the agent const hasViewPermission = await checkPermission({ userId, + role, resourceType: ResourceType.AGENT, resourceId: agent._id, requiredPermission: PermissionBits.VIEW, @@ -44,6 +47,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { // Check if user has EDIT permission (which would indicate collaborative access) const hasEditPermission = await checkPermission({ userId, + role, resourceType: ResourceType.AGENT, resourceId: agent._id, requiredPermission: PermissionBits.EDIT, @@ -81,12 +85,14 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { /** * Filter files based on user access through agents - * @param {Array} files - Array of file documents - * @param {string} userId - User ID for access control - * @param {string} agentId - Agent ID that might grant access to files + * @param {Object} params - Parameters object + * @param {Array} params.files - Array of file documents + * @param {string} params.userId - User ID for access control + * @param {string} [params.role] - Optional user role to avoid DB query + * @param {string} params.agentId - Agent ID that might grant access to files * @returns {Promise>} Filtered array of accessible files */ -const filterFilesByAgentAccess = async (files, userId, agentId) => { +const filterFilesByAgentAccess = async ({ files, userId, role, agentId }) => { if (!userId || !agentId || !files || files.length === 0) { return files; } @@ -96,7 +102,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => { const ownedFiles = []; for (const file of files) { - if (file.user && file.user.toString() === userId) { + if (file.user && file.user.toString() === userId.toString()) { ownedFiles.push(file); } else { filesToCheck.push(file); @@ -109,7 +115,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => { // Batch check access for all non-owned files const fileIds = filesToCheck.map((f) => f.file_id); - const accessMap = await hasAccessToFilesViaAgent(userId, fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent({ userId, role, fileIds, agentId }); // Filter files based on access const accessibleFiles = filesToCheck.filter((file) => accessMap.get(file.file_id)); diff --git a/api/server/services/PermissionService.js b/api/server/services/PermissionService.js index b525f82890..16374fc0dc 100644 --- a/api/server/services/PermissionService.js +++ b/api/server/services/PermissionService.js @@ -1,7 +1,7 @@ const mongoose = require('mongoose'); const { isEnabled } = require('@librechat/api'); -const { ResourceType, PrincipalType, PrincipalModel } = require('librechat-data-provider'); const { getTransactionSupport, logger } = require('@librechat/data-schemas'); +const { ResourceType, PrincipalType, PrincipalModel } = require('librechat-data-provider'); const { entraIdPrincipalFeatureEnabled, getUserOwnedEntraGroups, @@ -46,8 +46,8 @@ const validateResourceType = (resourceType) => { /** * Grant a permission to a principal for a resource using a role * @param {Object} params - Parameters for granting role-based permission - * @param {string} params.principalType - 'user', 'group', or 'public' - * @param {string|mongoose.Types.ObjectId|null} params.principalId - The ID of the principal (null for 'public') + * @param {string} params.principalType - PrincipalType.USER, PrincipalType.GROUP, or PrincipalType.PUBLIC + * @param {string|mongoose.Types.ObjectId|null} params.principalId - The ID of the principal (null for PrincipalType.PUBLIC) * @param {string} params.resourceType - Type of resource (e.g., 'agent') * @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource * @param {string} params.accessRoleId - The ID of the role (e.g., AccessRoleIds.AGENT_VIEWER, AccessRoleIds.AGENT_EDITOR) @@ -70,10 +70,21 @@ const grantPermission = async ({ } if (principalType !== PrincipalType.PUBLIC && !principalId) { - throw new Error('Principal ID is required for user and group principals'); + throw new Error('Principal ID is required for user, group, and role principals'); } - if (principalId && !mongoose.Types.ObjectId.isValid(principalId)) { + // Validate principalId based on type + if (principalId && principalType === PrincipalType.ROLE) { + // Role IDs are strings (role names) + if (typeof principalId !== 'string' || principalId.trim().length === 0) { + throw new Error(`Invalid role ID: ${principalId}`); + } + } else if ( + principalType && + principalType !== PrincipalType.PUBLIC && + !mongoose.Types.ObjectId.isValid(principalId) + ) { + // User and Group IDs must be valid ObjectIds throw new Error(`Invalid principal ID: ${principalId}`); } @@ -115,12 +126,13 @@ const grantPermission = async ({ * Check if a user has specific permission bits on a resource * @param {Object} params - Parameters for checking permissions * @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user + * @param {string} [params.role] - Optional user role (if not provided, will query from DB) * @param {string} params.resourceType - Type of resource (e.g., 'agent') * @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource * @param {number} params.requiredPermissions - The permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT) * @returns {Promise} Whether the user has the required permission bits */ -const checkPermission = async ({ userId, resourceType, resourceId, requiredPermission }) => { +const checkPermission = async ({ userId, role, resourceType, resourceId, requiredPermission }) => { try { if (typeof requiredPermission !== 'number' || requiredPermission < 1) { throw new Error('requiredPermission must be a positive number'); @@ -129,7 +141,7 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi validateResourceType(resourceType); // Get all principals for the user (user + groups + public) - const principals = await getUserPrincipals(userId); + const principals = await getUserPrincipals({ userId, role }); if (principals.length === 0) { return false; @@ -150,16 +162,17 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi * Get effective permission bitmask for a user on a resource * @param {Object} params - Parameters for getting effective permissions * @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user + * @param {string} [params.role] - Optional user role (if not provided, will query from DB) * @param {string} params.resourceType - Type of resource (e.g., 'agent') * @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource * @returns {Promise} Effective permission bitmask */ -const getEffectivePermissions = async ({ userId, resourceType, resourceId }) => { +const getEffectivePermissions = async ({ userId, role, resourceType, resourceId }) => { try { validateResourceType(resourceType); // Get all principals for the user (user + groups + public) - const principals = await getUserPrincipals(userId); + const principals = await getUserPrincipals({ userId, role }); if (principals.length === 0) { return 0; @@ -175,11 +188,12 @@ const getEffectivePermissions = async ({ userId, resourceType, resourceId }) => * Find all resources of a specific type that a user has access to with specific permission bits * @param {Object} params - Parameters for finding accessible resources * @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user + * @param {string} [params.role] - Optional user role (if not provided, will query from DB) * @param {string} params.resourceType - Type of resource (e.g., 'agent') * @param {number} params.requiredPermissions - The minimum permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT) * @returns {Promise} Array of resource IDs */ -const findAccessibleResources = async ({ userId, resourceType, requiredPermissions }) => { +const findAccessibleResources = async ({ userId, role, resourceType, requiredPermissions }) => { try { if (typeof requiredPermissions !== 'number' || requiredPermissions < 1) { throw new Error('requiredPermissions must be a positive number'); @@ -188,7 +202,7 @@ const findAccessibleResources = async ({ userId, resourceType, requiredPermissio validateResourceType(resourceType); // Get all principals for the user (user + groups + public) - const principalsList = await getUserPrincipals(userId); + const principalsList = await getUserPrincipals({ userId, role }); if (principalsList.length === 0) { return []; @@ -253,7 +267,7 @@ const getAvailableRoles = async ({ resourceType }) => { * Ensures a principal exists in the database based on TPrincipal data * Creates user if it doesn't exist locally (for Entra ID users) * @param {Object} principal - TPrincipal object from frontend - * @param {string} principal.type - 'user', 'group', or 'public' + * @param {string} principal.type - PrincipalType.USER, PrincipalType.GROUP, or PrincipalType.PUBLIC * @param {string} [principal.id] - Local database ID (null for Entra ID principals not yet synced) * @param {string} principal.name - Display name * @param {string} [principal.email] - Email address @@ -262,7 +276,7 @@ const getAvailableRoles = async ({ resourceType }) => { * @returns {Promise} Returns the principalId for database operations, null for public */ const ensurePrincipalExists = async function (principal) { - if (principal.type === 'public') { + if (principal.type === PrincipalType.PUBLIC) { return null; } @@ -270,7 +284,7 @@ const ensurePrincipalExists = async function (principal) { return principal.id; } - if (principal.type === 'user' && principal.source === 'entra') { + if (principal.type === PrincipalType.USER && principal.source === 'entra') { if (!principal.email || !principal.idOnTheSource) { throw new Error('Entra ID user principals must have email and idOnTheSource'); } @@ -303,7 +317,7 @@ const ensurePrincipalExists = async function (principal) { return userId.toString(); } - if (principal.type === 'group') { + if (principal.type === PrincipalType.GROUP) { throw new Error('Group principals should be handled by group-specific methods'); } @@ -315,7 +329,7 @@ const ensurePrincipalExists = async function (principal) { * Creates group if it doesn't exist locally (for Entra ID groups) * For Entra ID groups, always synchronizes member IDs when authentication context is provided * @param {Object} principal - TPrincipal object from frontend - * @param {string} principal.type - Must be 'group' + * @param {string} principal.type - Must be PrincipalType.GROUP * @param {string} [principal.id] - Local database ID (null for Entra ID principals not yet synced) * @param {string} principal.name - Display name * @param {string} [principal.email] - Email address @@ -328,8 +342,8 @@ const ensurePrincipalExists = async function (principal) { * @returns {Promise} Returns the groupId for database operations */ const ensureGroupPrincipalExists = async function (principal, authContext = null) { - if (principal.type !== 'group') { - throw new Error(`Invalid principal type: ${principal.type}. Expected 'group'`); + if (principal.type !== PrincipalType.GROUP) { + throw new Error(`Invalid principal type: ${principal.type}. Expected '${PrincipalType.GROUP}'`); } if (principal.source === 'entra') { @@ -612,10 +626,19 @@ const bulkUpdateResourcePermissions = async ({ resourceId, }; - if (principal.type !== 'public') { - query.principalId = principal.id; + if (principal.type !== PrincipalType.PUBLIC) { + query.principalId = + principal.type === PrincipalType.ROLE + ? principal.id + : new mongoose.Types.ObjectId(principal.id); } + const principalModelMap = { + [PrincipalType.USER]: PrincipalModel.USER, + [PrincipalType.GROUP]: PrincipalModel.GROUP, + [PrincipalType.ROLE]: PrincipalModel.ROLE, + }; + const update = { $set: { permBits: role.permBits, @@ -628,9 +651,11 @@ const bulkUpdateResourcePermissions = async ({ resourceType, resourceId, ...(principal.type !== PrincipalType.PUBLIC && { - principalId: principal.id, - principalModel: - principal.type === PrincipalType.USER ? PrincipalModel.USER : PrincipalModel.GROUP, + principalId: + principal.type === PrincipalType.ROLE + ? principal.id + : new mongoose.Types.ObjectId(principal.id), + principalModel: principalModelMap[principal.type], }), }, }; @@ -677,8 +702,11 @@ const bulkUpdateResourcePermissions = async ({ resourceId, }; - if (principal.type !== 'public') { - query.principalId = principal.id; + if (principal.type !== PrincipalType.PUBLIC) { + query.principalId = + principal.type === PrincipalType.ROLE + ? principal.id + : new mongoose.Types.ObjectId(principal.id); } deleteQueries.push(query); diff --git a/api/server/services/PermissionService.spec.js b/api/server/services/PermissionService.spec.js index 30b4460975..5772f3b909 100644 --- a/api/server/services/PermissionService.spec.js +++ b/api/server/services/PermissionService.spec.js @@ -79,6 +79,7 @@ describe('PermissionService', () => { const groupId = new mongoose.Types.ObjectId(); const resourceId = new mongoose.Types.ObjectId(); const grantedById = new mongoose.Types.ObjectId(); + const roleResourceId = new mongoose.Types.ObjectId(); describe('grantPermission', () => { test('should grant permission to a user with a role', async () => { @@ -171,7 +172,7 @@ describe('PermissionService', () => { accessRoleId: AccessRoleIds.AGENT_VIEWER, grantedBy: grantedById, }), - ).rejects.toThrow('Principal ID is required for user and group principals'); + ).rejects.toThrow('Principal ID is required for user, group, and role principals'); }); test('should throw error for non-existent role', async () => { @@ -1000,6 +1001,230 @@ describe('PermissionService', () => { expect(publicEntry.roleId.accessRoleId).toBe(AccessRoleIds.AGENT_EDITOR); }); + test('should grant permission to a role', async () => { + const entry = await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + resourceType: ResourceType.AGENT, + resourceId: roleResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + expect(entry).toBeDefined(); + expect(entry.principalType).toBe(PrincipalType.ROLE); + expect(entry.principalId).toBe('admin'); + expect(entry.principalModel).toBe(PrincipalModel.ROLE); + expect(entry.resourceType).toBe(ResourceType.AGENT); + expect(entry.resourceId.toString()).toBe(roleResourceId.toString()); + + // Get the role to verify the permission bits are correctly set + const role = await findRoleByIdentifier(AccessRoleIds.AGENT_EDITOR); + expect(entry.permBits).toBe(role.permBits); + expect(entry.roleId.toString()).toBe(role._id.toString()); + }); + + test('should check permissions for user with role', async () => { + // Grant permission to admin role + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + resourceType: ResourceType.AGENT, + resourceId: roleResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return user with admin role + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.ROLE, principalId: 'admin' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasPermission = await checkPermission({ + userId, + resourceType: ResourceType.AGENT, + resourceId: roleResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasPermission).toBe(true); + + // Check that user without admin role cannot access + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasNoPermission = await checkPermission({ + userId, + resourceType: ResourceType.AGENT, + resourceId: roleResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasNoPermission).toBe(false); + }); + + test('should optimize permission checks when role is provided', async () => { + const testUserId = new mongoose.Types.ObjectId(); + const testResourceId = new mongoose.Types.ObjectId(); + + // Create a user with EDITOR role + const User = mongoose.models.User; + await User.create({ + _id: testUserId, + email: 'editor@test.com', + emailVerified: true, + provider: 'local', + role: 'EDITOR', + }); + + // Grant permission to EDITOR role + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'EDITOR', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return user with EDITOR role when called + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: testUserId }, + { principalType: PrincipalType.ROLE, principalId: 'EDITOR' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + // Test 1: Check permission with role provided (optimization should be used) + const hasPermissionWithRole = await checkPermission({ + userId: testUserId, + role: 'EDITOR', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasPermissionWithRole).toBe(true); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' }); + + // Test 2: Check permission without role (should call getUserPrincipals) + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: testUserId }, + { principalType: PrincipalType.ROLE, principalId: 'EDITOR' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasPermissionWithoutRole = await checkPermission({ + userId: testUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasPermissionWithoutRole).toBe(true); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: undefined }); + + // Test 3: Verify getEffectivePermissions also uses the optimization + getUserPrincipals.mockClear(); + + const effectiveWithRole = await getEffectivePermissions({ + userId: testUserId, + role: 'EDITOR', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + + expect(effectiveWithRole).toBe(3); // EDITOR = VIEW + EDIT + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' }); + + // Test 4: Verify findAccessibleResources also uses the optimization + getUserPrincipals.mockClear(); + + const accessibleWithRole = await findAccessibleResources({ + userId: testUserId, + role: 'EDITOR', + resourceType: ResourceType.AGENT, + requiredPermissions: 1, // VIEW + }); + + expect(accessibleWithRole.map((id) => id.toString())).toContain(testResourceId.toString()); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' }); + }); + + test('should handle role changes dynamically', async () => { + const testUserId = new mongoose.Types.ObjectId(); + const testResourceId = new mongoose.Types.ObjectId(); + + // Grant permission to ADMIN role only + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'ADMIN', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_OWNER, + grantedBy: grantedById, + }); + + // Test with ADMIN role - should have access + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: testUserId }, + { principalType: PrincipalType.ROLE, principalId: 'ADMIN' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasAdminAccess = await checkPermission({ + userId: testUserId, + role: 'ADMIN', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 7, // Full permissions + }); + + expect(hasAdminAccess).toBe(true); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'ADMIN' }); + + // Test with USER role - should NOT have access + getUserPrincipals.mockClear(); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: testUserId }, + { principalType: PrincipalType.ROLE, principalId: 'USER' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasUserAccess = await checkPermission({ + userId: testUserId, + role: 'USER', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 1, // Even VIEW + }); + + expect(hasUserAccess).toBe(false); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'USER' }); + + // Test with EDITOR role - should NOT have access + getUserPrincipals.mockClear(); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: testUserId }, + { principalType: PrincipalType.ROLE, principalId: 'EDITOR' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const hasEditorAccess = await checkPermission({ + userId: testUserId, + role: 'EDITOR', + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasEditorAccess).toBe(false); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' }); + }); + test('should work with different resource types', async () => { // Test with promptGroup resources const promptGroupResourceId = new mongoose.Types.ObjectId(); @@ -1039,4 +1264,344 @@ describe('PermissionService', () => { ); }); }); + + describe('String vs ObjectId Edge Cases', () => { + const stringUserId = new mongoose.Types.ObjectId().toString(); + const objectIdUserId = new mongoose.Types.ObjectId(); + const stringGroupId = new mongoose.Types.ObjectId().toString(); + const objectIdGroupId = new mongoose.Types.ObjectId(); + const testResourceId = new mongoose.Types.ObjectId(); + + beforeEach(async () => { + // Clear any existing ACL entries + await AclEntry.deleteMany({}); + getUserPrincipals.mockReset(); + }); + + test('should handle string userId in grantPermission', async () => { + const entry = await grantPermission({ + principalType: PrincipalType.USER, + principalId: stringUserId, // Pass string + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: grantedById, + }); + + expect(entry).toBeDefined(); + expect(entry.principalType).toBe(PrincipalType.USER); + // Should be stored as ObjectId + expect(entry.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(entry.principalId.toString()).toBe(stringUserId); + }); + + test('should handle string groupId in grantPermission', async () => { + const entry = await grantPermission({ + principalType: PrincipalType.GROUP, + principalId: stringGroupId, // Pass string + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + expect(entry).toBeDefined(); + expect(entry.principalType).toBe(PrincipalType.GROUP); + // Should be stored as ObjectId + expect(entry.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(entry.principalId.toString()).toBe(stringGroupId); + }); + + test('should handle string roleId in grantPermission for ROLE type', async () => { + const roleString = 'moderator'; + + const entry = await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: roleString, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: grantedById, + }); + + expect(entry).toBeDefined(); + expect(entry.principalType).toBe(PrincipalType.ROLE); + // Should remain as string for ROLE type + expect(typeof entry.principalId).toBe('string'); + expect(entry.principalId).toBe(roleString); + expect(entry.principalModel).toBe(PrincipalModel.ROLE); + }); + + test('should check permissions correctly when permission granted with string userId', async () => { + // Grant permission with string userId + await grantPermission({ + principalType: PrincipalType.USER, + principalId: stringUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return ObjectId (as it should after our fix) + getUserPrincipals.mockResolvedValue([ + { + principalType: PrincipalType.USER, + principalId: new mongoose.Types.ObjectId(stringUserId), + }, + { principalType: PrincipalType.PUBLIC }, + ]); + + // Check permission with string userId + const hasPermission = await checkPermission({ + userId: stringUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 1, // VIEW + }); + + expect(hasPermission).toBe(true); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: stringUserId, role: undefined }); + }); + + test('should check permissions correctly when permission granted with ObjectId', async () => { + // Grant permission with ObjectId + await grantPermission({ + principalType: PrincipalType.USER, + principalId: objectIdUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_OWNER, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return ObjectId + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: objectIdUserId }, + { principalType: PrincipalType.PUBLIC }, + ]); + + // Check permission with ObjectId + const hasPermission = await checkPermission({ + userId: objectIdUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + requiredPermission: 7, // Full permissions + }); + + expect(hasPermission).toBe(true); + expect(getUserPrincipals).toHaveBeenCalledWith({ userId: objectIdUserId, role: undefined }); + }); + + test('should handle bulkUpdateResourcePermissions with string IDs', async () => { + const updatedPrincipals = [ + { + type: PrincipalType.USER, + id: stringUserId, // String ID + accessRoleId: AccessRoleIds.AGENT_VIEWER, + }, + { + type: PrincipalType.GROUP, + id: stringGroupId, // String ID + accessRoleId: AccessRoleIds.AGENT_EDITOR, + }, + { + type: PrincipalType.ROLE, + id: 'admin', // String ID (should remain string) + accessRoleId: AccessRoleIds.AGENT_OWNER, + }, + ]; + + const results = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + updatedPrincipals, + grantedBy: grantedById, + }); + + expect(results.granted).toHaveLength(3); + expect(results.errors).toHaveLength(0); + + // Verify USER entry has ObjectId + const userEntry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + expect(userEntry.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(userEntry.principalId.toString()).toBe(stringUserId); + + // Verify GROUP entry has ObjectId + const groupEntry = await AclEntry.findOne({ + principalType: PrincipalType.GROUP, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + expect(groupEntry.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(groupEntry.principalId.toString()).toBe(stringGroupId); + + // Verify ROLE entry has string + const roleEntry = await AclEntry.findOne({ + principalType: PrincipalType.ROLE, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + expect(typeof roleEntry.principalId).toBe('string'); + expect(roleEntry.principalId).toBe('admin'); + }); + + test('should handle revoking permissions with string IDs in bulkUpdateResourcePermissions', async () => { + // First grant permissions with ObjectIds + await grantPermission({ + principalType: PrincipalType.USER, + principalId: objectIdUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_OWNER, + grantedBy: grantedById, + }); + + await grantPermission({ + principalType: PrincipalType.GROUP, + principalId: objectIdGroupId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Revoke using string IDs + const revokedPrincipals = [ + { + type: PrincipalType.USER, + id: objectIdUserId.toString(), // String version of ObjectId + }, + { + type: PrincipalType.GROUP, + id: objectIdGroupId.toString(), // String version of ObjectId + }, + ]; + + const results = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + revokedPrincipals, + grantedBy: grantedById, + }); + + expect(results.revoked).toHaveLength(2); + expect(results.errors).toHaveLength(0); + + // Verify permissions were actually revoked + const remainingEntries = await AclEntry.find({ + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + expect(remainingEntries).toHaveLength(0); + }); + + test('should find accessible resources when permissions granted with mixed ID types', async () => { + const resource1 = new mongoose.Types.ObjectId(); + const resource2 = new mongoose.Types.ObjectId(); + const resource3 = new mongoose.Types.ObjectId(); + + // Grant with string userId + await grantPermission({ + principalType: PrincipalType.USER, + principalId: stringUserId, + resourceType: ResourceType.AGENT, + resourceId: resource1, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: grantedById, + }); + + // Grant with ObjectId userId (same user) + await grantPermission({ + principalType: PrincipalType.USER, + principalId: new mongoose.Types.ObjectId(stringUserId), + resourceType: ResourceType.AGENT, + resourceId: resource2, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Grant to role + await grantPermission({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + resourceType: ResourceType.AGENT, + resourceId: resource3, + accessRoleId: AccessRoleIds.AGENT_OWNER, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return user with admin role + getUserPrincipals.mockResolvedValue([ + { + principalType: PrincipalType.USER, + principalId: new mongoose.Types.ObjectId(stringUserId), + }, + { principalType: PrincipalType.ROLE, principalId: 'admin' }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const accessibleResources = await findAccessibleResources({ + userId: stringUserId, + role: 'admin', + resourceType: ResourceType.AGENT, + requiredPermissions: 1, // VIEW + }); + + // Should find all three resources + expect(accessibleResources).toHaveLength(3); + const resourceIds = accessibleResources.map((id) => id.toString()); + expect(resourceIds).toContain(resource1.toString()); + expect(resourceIds).toContain(resource2.toString()); + expect(resourceIds).toContain(resource3.toString()); + }); + + test('should get effective permissions with mixed ID types', async () => { + // Grant VIEW permission with string userId + await grantPermission({ + principalType: PrincipalType.USER, + principalId: stringUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: grantedById, + }); + + // Grant EDIT permission to a group with string groupId + await grantPermission({ + principalType: PrincipalType.GROUP, + principalId: stringGroupId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: grantedById, + }); + + // Mock getUserPrincipals to return ObjectIds (as it should after our fix) + getUserPrincipals.mockResolvedValue([ + { + principalType: PrincipalType.USER, + principalId: new mongoose.Types.ObjectId(stringUserId), + }, + { + principalType: PrincipalType.GROUP, + principalId: new mongoose.Types.ObjectId(stringGroupId), + }, + { principalType: PrincipalType.PUBLIC }, + ]); + + const effectivePermissions = await getEffectivePermissions({ + userId: stringUserId, + resourceType: ResourceType.AGENT, + resourceId: testResourceId, + }); + + // Should combine VIEW (1) and EDIT (3) permissions + expect(effectivePermissions).toBe(3); // EDITOR includes VIEW + }); + }); }); diff --git a/api/server/services/start/interface.js b/api/server/services/start/interface.js index fecd01508a..039c6376b5 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/client/src/components/Prompts/SharePrompt.tsx b/client/src/components/Prompts/SharePrompt.tsx index 9dda100668..ea5bfe62dc 100644 --- a/client/src/components/Prompts/SharePrompt.tsx +++ b/client/src/components/Prompts/SharePrompt.tsx @@ -4,8 +4,8 @@ import { SystemRoles, Permissions, ResourceType, - PermissionTypes, PermissionBits, + PermissionTypes, } from 'librechat-data-provider'; import { Button } from '@librechat/client'; import type { TPromptGroup } from 'librechat-data-provider'; diff --git a/client/src/components/Sharing/GenericGrantAccessDialog.tsx b/client/src/components/Sharing/GenericGrantAccessDialog.tsx index 8dfa7704ae..89225ea52e 100644 --- a/client/src/components/Sharing/GenericGrantAccessDialog.tsx +++ b/client/src/components/Sharing/GenericGrantAccessDialog.tsx @@ -36,7 +36,7 @@ export default function GenericGrantAccessDialog({ resourceId?: string | null; resourceName?: string; resourceType: ResourceType; - onGrantAccess?: (shares: TPrincipal[], isPublic: boolean, publicRole: AccessRoleIds) => void; + onGrantAccess?: (shares: TPrincipal[], isPublic: boolean, publicRole?: AccessRoleIds) => void; disabled?: boolean; children?: React.ReactNode; }) { diff --git a/client/src/components/Sharing/GenericManagePermissionsDialog.tsx b/client/src/components/Sharing/GenericManagePermissionsDialog.tsx index 1935f2e67d..c02795891c 100644 --- a/client/src/components/Sharing/GenericManagePermissionsDialog.tsx +++ b/client/src/components/Sharing/GenericManagePermissionsDialog.tsx @@ -30,7 +30,7 @@ export default function GenericManagePermissionsDialog({ onUpdatePermissions?: ( shares: TPrincipal[], isPublic: boolean, - publicRole: AccessRoleIds, + publicRole?: AccessRoleIds, ) => void; children?: React.ReactNode; }) { @@ -84,7 +84,7 @@ export default function GenericManagePermissionsDialog({ setHasChanges(true); }; - const handleRoleChange = (idOnTheSource: string, newRole: string) => { + const handleRoleChange = (idOnTheSource: string, newRole: AccessRoleIds) => { setManagedShares( managedShares.map((s) => s.idOnTheSource === idOnTheSource ? { ...s, accessRoleId: newRole } : s, @@ -162,7 +162,7 @@ export default function GenericManagePermissionsDialog({ setManagedPublicRole(config?.defaultViewerRoleId); } }; - const handlePublicRoleChange = (role: string) => { + const handlePublicRoleChange = (role: AccessRoleIds) => { setManagedPublicRole(role); setHasChanges(true); }; diff --git a/client/src/components/Sharing/GrantAccessDialog.tsx b/client/src/components/Sharing/GrantAccessDialog.tsx index 4f8bb7833a..61ab5f7f3e 100644 --- a/client/src/components/Sharing/GrantAccessDialog.tsx +++ b/client/src/components/Sharing/GrantAccessDialog.tsx @@ -1,6 +1,6 @@ -import React, { useState, useEffect, useMemo } from 'react'; +import React, { useState, useEffect } from 'react'; +import { ResourceType, AccessRoleIds } from 'librechat-data-provider'; import { Share2Icon, Users, Loader, Shield, Link, CopyCheck } from 'lucide-react'; -import { Permissions, ResourceType, PermissionTypes, AccessRoleIds } from 'librechat-data-provider'; import { useGetResourcePermissionsQuery, useUpdateResourcePermissionsMutation, @@ -15,7 +15,7 @@ import { useToastContext, } from '@librechat/client'; import type { TPrincipal } from 'librechat-data-provider'; -import { useLocalize, useCopyToClipboard, useHasAccess } from '~/hooks'; +import { useLocalize, useCopyToClipboard, usePeoplePickerPermissions } from '~/hooks'; import ManagePermissionsDialog from './ManagePermissionsDialog'; import PublicSharingToggle from './PublicSharingToggle'; import AccessRolesPicker from './AccessRolesPicker'; @@ -37,29 +37,7 @@ export default function GrantAccessDialog({ }) { const localize = useLocalize(); const { showToast } = useToastContext(); - - // Check if user has permission to access people picker - const canViewUsers = useHasAccess({ - permissionType: PermissionTypes.PEOPLE_PICKER, - permission: Permissions.VIEW_USERS, - }); - const canViewGroups = useHasAccess({ - permissionType: PermissionTypes.PEOPLE_PICKER, - permission: Permissions.VIEW_GROUPS, - }); - const hasPeoplePickerAccess = canViewUsers || canViewGroups; - - /** Type filter based on permissions */ - const peoplePickerTypeFilter = useMemo(() => { - if (canViewUsers && canViewGroups) { - return null; // Both types allowed - } else if (canViewUsers) { - return 'user' as const; - } else if (canViewGroups) { - return 'group' as const; - } - return null; - }, [canViewUsers, canViewGroups]); + const { hasPeoplePickerAccess, peoplePickerTypeFilter } = usePeoplePickerPermissions(); const { data: permissionsData, @@ -72,7 +50,7 @@ export default function GrantAccessDialog({ const updatePermissionsMutation = useUpdateResourcePermissionsMutation(); const [newShares, setNewShares] = useState([]); - const [defaultPermissionId, setDefaultPermissionId] = useState( + const [defaultPermissionId, setDefaultPermissionId] = useState( AccessRoleIds.AGENT_VIEWER, ); const [isModalOpen, setIsModalOpen] = useState(false); diff --git a/client/src/components/Sharing/PeoplePicker/PeoplePicker.tsx b/client/src/components/Sharing/PeoplePicker/PeoplePicker.tsx index 7c2dec5b90..4eaaf6966b 100644 --- a/client/src/components/Sharing/PeoplePicker/PeoplePicker.tsx +++ b/client/src/components/Sharing/PeoplePicker/PeoplePicker.tsx @@ -1,16 +1,17 @@ import React, { useState, useMemo } from 'react'; +import { PrincipalType } from 'librechat-data-provider'; import type { TPrincipal, PrincipalSearchParams } from 'librechat-data-provider'; import { useSearchPrincipalsQuery } from 'librechat-data-provider/react-query'; +import { useLocalize, usePeoplePickerPermissions } from '~/hooks'; import PeoplePickerSearchItem from './PeoplePickerSearchItem'; import SelectedPrincipalsList from './SelectedPrincipalsList'; import { SearchPicker } from './SearchPicker'; -import { useLocalize } from '~/hooks'; interface PeoplePickerProps { onSelectionChange: (principals: TPrincipal[]) => void; placeholder?: string; className?: string; - typeFilter?: 'user' | 'group' | null; + typeFilter?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE | null; } export default function PeoplePicker({ @@ -20,6 +21,7 @@ export default function PeoplePicker({ typeFilter = null, }: PeoplePickerProps) { const localize = useLocalize(); + const { canViewUsers, canViewGroups, canViewRoles } = usePeoplePickerPermissions(); const [searchQuery, setSearchQuery] = useState(''); const [selectedShares, setSelectedShares] = useState([]); @@ -54,6 +56,28 @@ export default function PeoplePicker({ console.error('Principal search error:', error); } + /** Get appropriate label based on permissions */ + const getSearchLabel = () => { + const permissions = [canViewUsers, canViewGroups, canViewRoles]; + const permissionCount = permissions.filter(Boolean).length; + + if (permissionCount === 3) { + return localize('com_ui_search_users_groups_roles'); + } else if (permissionCount === 2) { + if (canViewUsers && canViewGroups) { + return localize('com_ui_search_users_groups'); + } + } else if (canViewUsers) { + return localize('com_ui_search_users'); + } else if (canViewGroups) { + return localize('com_ui_search_groups'); + } else if (canViewRoles) { + return localize('com_ui_search_roles'); + } + + return localize('com_ui_search_users_groups'); + }; + return (
@@ -83,7 +107,7 @@ export default function PeoplePicker({ }); setSearchQuery(''); }} - label={localize('com_ui_search_users_groups')} + label={getSearchLabel()} isLoading={isLoading} />
diff --git a/client/src/components/Sharing/PeoplePicker/PeoplePickerSearchItem.tsx b/client/src/components/Sharing/PeoplePicker/PeoplePickerSearchItem.tsx index a4071d0be0..03166fcddb 100644 --- a/client/src/components/Sharing/PeoplePicker/PeoplePickerSearchItem.tsx +++ b/client/src/components/Sharing/PeoplePicker/PeoplePickerSearchItem.tsx @@ -1,8 +1,9 @@ import React, { forwardRef } from 'react'; +import { PrincipalType } from 'librechat-data-provider'; import type { TPrincipal } from 'librechat-data-provider'; -import { cn } from '~/utils'; +import PrincipalAvatar from '~/components/Sharing/PrincipalAvatar'; import { useLocalize } from '~/hooks'; -import PrincipalAvatar from '../PrincipalAvatar'; +import { cn } from '~/utils'; interface PeoplePickerSearchItemProps extends React.HTMLAttributes { principal: TPrincipal; @@ -16,10 +17,37 @@ const PeoplePickerSearchItem = forwardRef { + switch (type) { + case PrincipalType.USER: + return { + className: 'bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-300', + label: localize('com_ui_user'), + }; + case PrincipalType.GROUP: + return { + className: 'bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-300', + label: localize('com_ui_group'), + }; + case PrincipalType.ROLE: + return { + className: 'bg-purple-100 text-purple-800 dark:bg-purple-900 dark:text-purple-300', + label: localize('com_ui_role'), + }; + default: + return { + className: 'bg-gray-100 text-gray-800 dark:bg-gray-900 dark:text-gray-300', + label: type, + }; + } + }; + + const badgeConfig = getBadgeConfig(); + return (
- {type === 'user' ? localize('com_ui_user') : localize('com_ui_group')} + {badgeConfig.label}
diff --git a/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx b/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx index b323aceb35..4659dc6ec0 100644 --- a/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx +++ b/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx @@ -3,8 +3,8 @@ import * as Menu from '@ariakit/react/menu'; import { Button, DropdownPopup } from '@librechat/client'; import { Users, X, ExternalLink, ChevronDown } from 'lucide-react'; import type { TPrincipal, TAccessRole, AccessRoleIds } from 'librechat-data-provider'; +import PrincipalAvatar from '~/components/Sharing/PrincipalAvatar'; import { getRoleLocalizationKeys } from '~/utils'; -import PrincipalAvatar from '../PrincipalAvatar'; import { useLocalize } from '~/hooks'; interface SelectedPrincipalsListProps { @@ -36,7 +36,7 @@ export default function SelectedPrincipalsList({
-

{localize('com_ui_search_above_to_add')}

+

{localize('com_ui_search_above_to_add_all')}

); diff --git a/client/src/components/Sharing/PrincipalAvatar.tsx b/client/src/components/Sharing/PrincipalAvatar.tsx index 2914f2f821..a96a66b036 100644 --- a/client/src/components/Sharing/PrincipalAvatar.tsx +++ b/client/src/components/Sharing/PrincipalAvatar.tsx @@ -1,5 +1,6 @@ import React from 'react'; -import { Users, User } from 'lucide-react'; +import { Users, User, Shield } from 'lucide-react'; +import { PrincipalType } from 'librechat-data-provider'; import type { TPrincipal } from 'librechat-data-provider'; import { cn } from '~/utils'; @@ -17,7 +18,6 @@ export default function PrincipalAvatar({ const { avatar, type, name } = principal; const displayName = name || 'Unknown'; - // Size variants const sizeClasses = { sm: 'h-6 w-6', md: 'h-8 w-8', @@ -33,7 +33,38 @@ export default function PrincipalAvatar({ const avatarSizeClass = sizeClasses[size]; const iconSizeClass = iconSizeClasses[size]; - // Avatar or icon logic + /** Get icon component and styling based on type */ + const getIconConfig = () => { + switch (type) { + case PrincipalType.USER: + return { + Icon: User, + containerClass: 'bg-blue-100 dark:bg-blue-900', + iconClass: 'text-blue-600 dark:text-blue-400', + }; + case PrincipalType.GROUP: + return { + Icon: Users, + containerClass: 'bg-green-100 dark:bg-green-900', + iconClass: 'text-green-600 dark:text-green-400', + }; + case PrincipalType.ROLE: + return { + Icon: Shield, + containerClass: 'bg-purple-100 dark:bg-purple-900', + iconClass: 'text-purple-600 dark:text-purple-400', + }; + default: + return { + Icon: User, + containerClass: 'bg-gray-100 dark:bg-gray-900', + iconClass: 'text-gray-600 dark:text-gray-400', + }; + } + }; + + const { Icon, containerClass, iconClass } = getIconConfig(); + if (avatar) { return (
@@ -50,52 +81,31 @@ export default function PrincipalAvatar({ /> {/* Hidden fallback icon that shows if image fails */}
- {type === 'user' ? ( -
- -
- ) : ( -
- -
- )} +
+ +
); } - // Fallback icon based on type return (
- {type === 'user' ? ( -
- -
- ) : ( -
- -
- )} +
+ +
); } diff --git a/client/src/components/Sharing/PublicSharingToggle.tsx b/client/src/components/Sharing/PublicSharingToggle.tsx index 62f9d8e086..a48960d853 100644 --- a/client/src/components/Sharing/PublicSharingToggle.tsx +++ b/client/src/components/Sharing/PublicSharingToggle.tsx @@ -14,7 +14,7 @@ export default function PublicSharingToggle({ resourceType = ResourceType.AGENT, }: { isPublic: boolean; - publicRole: AccessRoleIds; + publicRole?: AccessRoleIds; onPublicToggle: (isPublic: boolean) => void; onPublicRoleChange: (role: AccessRoleIds) => void; resourceType?: ResourceType; diff --git a/client/src/components/SidePanel/Agents/AgentFooter.tsx b/client/src/components/SidePanel/Agents/AgentFooter.tsx index 4c63c94cda..5b070369ee 100644 --- a/client/src/components/SidePanel/Agents/AgentFooter.tsx +++ b/client/src/components/SidePanel/Agents/AgentFooter.tsx @@ -4,8 +4,8 @@ import { SystemRoles, Permissions, ResourceType, - PermissionTypes, PermissionBits, + PermissionTypes, } from 'librechat-data-provider'; import type { AgentForm, AgentPanelProps } from '~/common'; import { useLocalize, useAuthContext, useHasAccess, useResourcePermissions } from '~/hooks'; @@ -43,7 +43,7 @@ export default function AgentFooter({ permission: Permissions.SHARED_GLOBAL, }); const { hasPermission, isLoading: permissionsLoading } = useResourcePermissions( - 'agent', + ResourceType.AGENT, agent?._id || '', ); diff --git a/client/src/hooks/Sharing/usePeoplePickerPermissions.ts b/client/src/hooks/Sharing/usePeoplePickerPermissions.ts index 953f87f8f6..940439105f 100644 --- a/client/src/hooks/Sharing/usePeoplePickerPermissions.ts +++ b/client/src/hooks/Sharing/usePeoplePickerPermissions.ts @@ -1,5 +1,5 @@ import { useMemo } from 'react'; -import { PermissionTypes, Permissions } from 'librechat-data-provider'; +import { PermissionTypes, PrincipalType, Permissions } from 'librechat-data-provider'; import { useHasAccess } from '~/hooks'; /** @@ -17,21 +17,33 @@ export const usePeoplePickerPermissions = () => { permission: Permissions.VIEW_GROUPS, }); - const hasPeoplePickerAccess = canViewUsers || canViewGroups; + const canViewRoles = useHasAccess({ + permissionType: PermissionTypes.PEOPLE_PICKER, + permission: Permissions.VIEW_ROLES, + }); - const peoplePickerTypeFilter = useMemo(() => { - if (canViewUsers && canViewGroups) { - return null; // Both types allowed + const hasPeoplePickerAccess = canViewUsers || canViewGroups || canViewRoles; + + const peoplePickerTypeFilter: + | PrincipalType.USER + | PrincipalType.GROUP + | PrincipalType.ROLE + | null = useMemo(() => { + if (canViewUsers && canViewGroups && canViewRoles) { + return null; // All types allowed } else if (canViewUsers) { - return 'user' as const; + return PrincipalType.USER; } else if (canViewGroups) { - return 'group' as const; + return PrincipalType.GROUP; + } else if (canViewRoles) { + return PrincipalType.ROLE; } return null; - }, [canViewUsers, canViewGroups]); + }, [canViewUsers, canViewGroups, canViewRoles]); return { canViewUsers, + canViewRoles, canViewGroups, hasPeoplePickerAccess, peoplePickerTypeFilter, diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index ab09feb167..c81f45e9a5 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -700,10 +700,16 @@ "com_ui_grant_access": "Grant Access", "com_ui_granting": "Granting...", "com_ui_search_users_groups": "Search Users and Groups", + "com_ui_search_users_groups_roles": "Search Users, Groups, and Roles", + "com_ui_search_users": "Search Users", + "com_ui_search_groups": "Search Groups", + "com_ui_search_roles": "Search Roles", "com_ui_search_default_placeholder": "Search by name or email (min 2 chars)", "com_ui_user": "User", "com_ui_group": "Group", + "com_ui_role": "Role", "com_ui_search_above_to_add": "Search above to add users or groups", + "com_ui_search_above_to_add_all": "Search above to add users, groups, or roles", "com_ui_azure_ad": "Entra ID", "com_ui_remove_user": "Remove {{0}}", "com_ui_create": "Create", diff --git a/packages/data-provider/src/accessPermissions.ts b/packages/data-provider/src/accessPermissions.ts index 4ea1d5e4e3..3c5a232c65 100644 --- a/packages/data-provider/src/accessPermissions.ts +++ b/packages/data-provider/src/accessPermissions.ts @@ -17,6 +17,7 @@ export enum PrincipalType { USER = 'user', GROUP = 'group', PUBLIC = 'public', + ROLE = 'role', } /** @@ -25,6 +26,7 @@ export enum PrincipalType { export enum PrincipalModel { USER = 'User', GROUP = 'Group', + ROLE = 'Role', } /** @@ -74,16 +76,16 @@ export enum AccessRoleIds { // ===== ZOD SCHEMAS ===== /** - * Principal schema - represents a user, group, or public access + * Principal schema - represents a user, group, role, or public access */ export const principalSchema = z.object({ type: z.nativeEnum(PrincipalType), - id: z.string().optional(), // undefined for 'public' type + id: z.string().optional(), // undefined for 'public' type, role name for 'role' type name: z.string().optional(), email: z.string().optional(), // for user and group types source: z.enum(['local', 'entra']).optional(), avatar: z.string().optional(), // for user and group types - description: z.string().optional(), // for group type + description: z.string().optional(), // for group and role types idOnTheSource: z.string().optional(), // Entra ID for users/groups accessRoleId: z.nativeEnum(AccessRoleIds).optional(), // Access role ID for permissions memberCount: z.number().optional(), // for group type @@ -192,7 +194,7 @@ export type TUpdateResourcePermissionsResponse = z.infer< export type TPrincipalSearchParams = { q: string; // search query (required) limit?: number; // max results (1-50, default 10) - type?: 'user' | 'group'; // filter by type (optional) + type?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE; // filter by type (optional) }; /** @@ -200,7 +202,7 @@ export type TPrincipalSearchParams = { */ export type TPrincipalSearchResult = { id?: string | null; // null for Entra ID principals that don't exist locally yet - type: 'user' | 'group'; + type: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE; name: string; email?: string; // for users and groups username?: string; // for users @@ -218,7 +220,7 @@ export type TPrincipalSearchResult = { export type TPrincipalSearchResponse = { query: string; limit: number; - type?: 'user' | 'group'; + type?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE; results: TPrincipalSearchResult[]; count: number; sources: { diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 6258fd6a91..e86c159ba5 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; diff --git a/packages/data-provider/src/roles.ts b/packages/data-provider/src/roles.ts index d36ae72f03..22d20ebd5a 100644 --- a/packages/data-provider/src/roles.ts +++ b/packages/data-provider/src/roles.ts @@ -30,7 +30,6 @@ export enum SystemRoles { USER = 'USER', } -// The role schema now only needs to reference the permissions schema. export const roleSchema = z.object({ name: z.string(), permissions: permissionsSchema, @@ -38,7 +37,6 @@ export const roleSchema = z.object({ export type TRole = z.infer; -// Define default roles using the new structure. const defaultRolesSchema = z.object({ [SystemRoles.ADMIN]: roleSchema.extend({ name: z.literal(SystemRoles.ADMIN), @@ -80,6 +78,7 @@ const defaultRolesSchema = z.object({ [PermissionTypes.PEOPLE_PICKER]: peoplePickerPermissionsSchema.extend({ [Permissions.VIEW_USERS]: z.boolean().default(true), [Permissions.VIEW_GROUPS]: z.boolean().default(true), + [Permissions.VIEW_ROLES]: z.boolean().default(true), }), [PermissionTypes.MARKETPLACE]: z.object({ [Permissions.USE]: z.boolean().default(false), @@ -137,6 +136,7 @@ export const roleDefaults = defaultRolesSchema.parse({ [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: true, [Permissions.VIEW_GROUPS]: true, + [Permissions.VIEW_ROLES]: true, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: true, @@ -163,6 +163,7 @@ export const roleDefaults = defaultRolesSchema.parse({ [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: false, [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: false, diff --git a/packages/data-provider/src/types/queries.ts b/packages/data-provider/src/types/queries.ts index 44bfa9fece..f5ebebb2c0 100644 --- a/packages/data-provider/src/types/queries.ts +++ b/packages/data-provider/src/types/queries.ts @@ -1,5 +1,5 @@ import type { InfiniteData } from '@tanstack/react-query'; -import type { AccessRoleIds } from '../accessPermissions'; +import type * as p from '../accessPermissions'; import type * as a from '../types/agents'; import type * as s from '../schemas'; import type * as t from '../types'; @@ -129,28 +129,14 @@ export type MemoriesResponse = { export type PrincipalSearchParams = { q: string; limit?: number; - type?: 'user' | 'group'; -}; - -export type PrincipalSearchResult = { - id?: string | null; - type: 'user' | 'group'; - name: string; - email?: string; - username?: string; - avatar?: string; - provider?: string; - source: 'local' | 'entra'; - memberCount?: number; - description?: string; - idOnTheSource?: string; + type?: p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE; }; export type PrincipalSearchResponse = { query: string; limit: number; - type?: 'user' | 'group'; - results: PrincipalSearchResult[]; + type?: p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE; + results: p.TPrincipalSearchResult[]; count: number; sources: { local: number; @@ -159,7 +145,7 @@ export type PrincipalSearchResponse = { }; export type AccessRole = { - accessRoleId: AccessRoleIds; + accessRoleId: p.AccessRoleIds; name: string; description: string; permBits: number; diff --git a/packages/data-schemas/src/methods/aclEntry.spec.ts b/packages/data-schemas/src/methods/aclEntry.spec.ts index 69983f369f..be18a81c1b 100644 --- a/packages/data-schemas/src/methods/aclEntry.spec.ts +++ b/packages/data-schemas/src/methods/aclEntry.spec.ts @@ -394,6 +394,192 @@ describe('AclEntry Model Tests', () => { }); }); + describe('String vs ObjectId Edge Cases', () => { + test('should handle string userId in grantPermission', async () => { + const userIdString = userId.toString(); + + const entry = await methods.grantPermission( + PrincipalType.USER, + userIdString, // Pass string instead of ObjectId + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + expect(entry).toBeDefined(); + expect(entry?.principalType).toBe(PrincipalType.USER); + // Should be stored as ObjectId + expect(entry?.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(entry?.principalId?.toString()).toBe(userIdString); + }); + + test('should handle string groupId in grantPermission', async () => { + const groupIdString = groupId.toString(); + + const entry = await methods.grantPermission( + PrincipalType.GROUP, + groupIdString, // Pass string instead of ObjectId + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + expect(entry).toBeDefined(); + expect(entry?.principalType).toBe(PrincipalType.GROUP); + // Should be stored as ObjectId + expect(entry?.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(entry?.principalId?.toString()).toBe(groupIdString); + }); + + test('should handle string roleId in grantPermission for ROLE type', async () => { + const roleString = 'admin'; + + const entry = await methods.grantPermission( + PrincipalType.ROLE, + roleString, + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + expect(entry).toBeDefined(); + expect(entry?.principalType).toBe(PrincipalType.ROLE); + // Should remain as string for ROLE type + expect(typeof entry?.principalId).toBe('string'); + expect(entry?.principalId).toBe(roleString); + expect(entry?.principalModel).toBe(PrincipalModel.ROLE); + }); + + test('should handle string principalId in revokePermission', async () => { + // First grant permission with ObjectId + await methods.grantPermission( + PrincipalType.USER, + userId, + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + // Then revoke with string ID + const result = await methods.revokePermission( + PrincipalType.USER, + userId.toString(), // Pass string + ResourceType.AGENT, + resourceId, + ); + + expect(result.deletedCount).toBe(1); + + // Verify it's actually deleted + const entries = await methods.findEntriesByPrincipal(PrincipalType.USER, userId); + expect(entries).toHaveLength(0); + }); + + test('should handle string principalId in modifyPermissionBits', async () => { + // First grant permission with ObjectId + await methods.grantPermission( + PrincipalType.USER, + userId, + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + // Then modify with string ID + const updated = await methods.modifyPermissionBits( + PrincipalType.USER, + userId.toString(), // Pass string + ResourceType.AGENT, + resourceId, + PermissionBits.EDIT, + null, + ); + + expect(updated).toBeDefined(); + expect(updated?.permBits).toBe(PermissionBits.VIEW | PermissionBits.EDIT); + }); + + test('should handle mixed string and ObjectId in hasPermission', async () => { + // Grant permission with string ID + await methods.grantPermission( + PrincipalType.USER, + userId.toString(), + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + // Check permission with ObjectId in principals list + const hasPermWithObjectId = await methods.hasPermission( + [{ principalType: PrincipalType.USER, principalId: userId }], + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + ); + expect(hasPermWithObjectId).toBe(true); + + // Check permission with string in principals list + const hasPermWithString = await methods.hasPermission( + [{ principalType: PrincipalType.USER, principalId: userId.toString() }], + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + ); + expect(hasPermWithString).toBe(false); // This should fail because hasPermission doesn't convert + + // Check with converted ObjectId + const hasPermWithConvertedId = await methods.hasPermission( + [ + { + principalType: PrincipalType.USER, + principalId: new mongoose.Types.ObjectId(userId.toString()), + }, + ], + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + ); + expect(hasPermWithConvertedId).toBe(true); + }); + + test('should update existing permission when granting with string ID', async () => { + // First grant with ObjectId + await methods.grantPermission( + PrincipalType.USER, + userId, + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW, + grantedById, + ); + + // Grant again with string ID and different permissions + const updated = await methods.grantPermission( + PrincipalType.USER, + userId.toString(), + ResourceType.AGENT, + resourceId, + PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE, + grantedById, + ); + + expect(updated).toBeDefined(); + expect(updated?.permBits).toBe( + PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE, + ); + + // Should still only be one entry + const entries = await methods.findEntriesByPrincipal(PrincipalType.USER, userId); + expect(entries).toHaveLength(1); + }); + }); + describe('Resource Access Queries', () => { test('should find accessible resources', async () => { /** Create multiple resources with different permissions */ diff --git a/packages/data-schemas/src/methods/aclEntry.ts b/packages/data-schemas/src/methods/aclEntry.ts index 2284214259..67e91ad018 100644 --- a/packages/data-schemas/src/methods/aclEntry.ts +++ b/packages/data-schemas/src/methods/aclEntry.ts @@ -1,5 +1,6 @@ +import { Types } from 'mongoose'; import { PrincipalType, PrincipalModel } from 'librechat-data-provider'; -import type { Model, Types, DeleteResult, ClientSession } from 'mongoose'; +import type { Model, DeleteResult, ClientSession } from 'mongoose'; import type { IAclEntry } from '~/types'; export function createAclEntryMethods(mongoose: typeof import('mongoose')) { @@ -147,9 +148,17 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { }; if (principalType !== PrincipalType.PUBLIC) { - query.principalId = principalId; - query.principalModel = - principalType === PrincipalType.USER ? PrincipalModel.USER : PrincipalModel.GROUP; + query.principalId = + typeof principalId === 'string' && principalType !== PrincipalType.ROLE + ? new Types.ObjectId(principalId) + : principalId; + if (principalType === PrincipalType.USER) { + query.principalModel = PrincipalModel.USER; + } else if (principalType === PrincipalType.GROUP) { + query.principalModel = PrincipalModel.GROUP; + } else if (principalType === PrincipalType.ROLE) { + query.principalModel = PrincipalModel.ROLE; + } } const update = { @@ -194,7 +203,10 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { }; if (principalType !== PrincipalType.PUBLIC) { - query.principalId = principalId; + query.principalId = + typeof principalId === 'string' && principalType !== PrincipalType.ROLE + ? new Types.ObjectId(principalId) + : principalId; } const options = session ? { session } : {}; @@ -230,7 +242,10 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { }; if (principalType !== PrincipalType.PUBLIC) { - query.principalId = principalId; + query.principalId = + typeof principalId === 'string' && principalType !== PrincipalType.ROLE + ? new Types.ObjectId(principalId) + : principalId; } const update: Record = {}; diff --git a/packages/data-schemas/src/methods/userGroup.methods.spec.ts b/packages/data-schemas/src/methods/userGroup.methods.spec.ts new file mode 100644 index 0000000000..8a31544018 --- /dev/null +++ b/packages/data-schemas/src/methods/userGroup.methods.spec.ts @@ -0,0 +1,620 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import type * as t from '~/types'; +import { createUserGroupMethods } from './userGroup'; +import groupSchema from '~/schema/group'; +import userSchema from '~/schema/user'; + +/** Mocking logger */ +jest.mock('~/config/winston', () => ({ + error: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + +let mongoServer: MongoMemoryServer; +let Group: mongoose.Model; +let User: mongoose.Model; +let methods: ReturnType; + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + const mongoUri = mongoServer.getUri(); + await mongoose.connect(mongoUri); + + /** Register models */ + Group = mongoose.models.Group || mongoose.model('Group', groupSchema); + User = mongoose.models.User || mongoose.model('User', userSchema); + + /** Initialize methods */ + methods = createUserGroupMethods(mongoose); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await mongoose.connection.dropDatabase(); +}); + +describe('UserGroup Methods - Detailed Tests', () => { + describe('findGroupById', () => { + test('should find group by ObjectId', async () => { + const group = await Group.create({ + name: 'Test Group', + source: 'local', + memberIds: [], + }); + + const found = await methods.findGroupById(group._id as mongoose.Types.ObjectId); + + expect(found).toBeDefined(); + expect(found?._id.toString()).toBe(group._id.toString()); + expect(found?.name).toBe('Test Group'); + }); + + test('should find group by string ID', async () => { + const group = await Group.create({ + name: 'Test Group', + source: 'local', + memberIds: [], + }); + + const found = await methods.findGroupById(group._id as mongoose.Types.ObjectId); + + expect(found).toBeDefined(); + expect(found?._id.toString()).toBe(group._id.toString()); + }); + + test('should apply projection correctly', async () => { + const group = await Group.create({ + name: 'Test Group', + source: 'local', + description: 'Test Description', + memberIds: ['user1', 'user2'], + }); + + const found = await methods.findGroupById(group._id as mongoose.Types.ObjectId, { + name: 1, + }); + + expect(found).toBeDefined(); + expect(found?.name).toBe('Test Group'); + expect(found?.description).toBeUndefined(); + expect(found?.memberIds).toBeUndefined(); + }); + + test('should return null for non-existent group', async () => { + const fakeId = new mongoose.Types.ObjectId(); + const found = await methods.findGroupById(fakeId as mongoose.Types.ObjectId); + + expect(found).toBeNull(); + }); + }); + + describe('findGroupByExternalId', () => { + test('should find group by external ID and source', async () => { + await Group.create({ + name: 'Entra Group', + source: 'entra', + idOnTheSource: 'entra-123', + memberIds: [], + }); + + const found = await methods.findGroupByExternalId('entra-123', 'entra'); + + expect(found).toBeDefined(); + expect(found?.idOnTheSource).toBe('entra-123'); + expect(found?.source).toBe('entra'); + }); + + test('should not find group with wrong source', async () => { + await Group.create({ + name: 'Entra Group', + source: 'entra', + idOnTheSource: 'entra-123', + memberIds: [], + }); + + const found = await methods.findGroupByExternalId('entra-123', 'local'); + + expect(found).toBeNull(); + }); + + test('should handle multiple groups with same external ID but different sources', async () => { + const id = 'shared-id'; + + await Group.create({ + name: 'Entra Group', + source: 'entra', + idOnTheSource: id, + memberIds: [], + }); + + await Group.create({ + name: 'Local Group', + source: 'local', + memberIds: [], + }); + + const entraGroup = await methods.findGroupByExternalId(id, 'entra'); + const localGroup = await methods.findGroupByExternalId(id, 'local'); + + expect(entraGroup?.name).toBe('Entra Group'); + expect(localGroup).toBeNull(); // local groups don't use idOnTheSource by default + }); + }); + + describe('findGroupsByNamePattern', () => { + beforeEach(async () => { + await Group.create([ + { name: 'Engineering Team', source: 'local', memberIds: [] }, + { name: 'Engineering Managers', source: 'local', memberIds: [] }, + { name: 'Marketing Team', source: 'local', memberIds: [] }, + { + name: 'Remote Engineering', + source: 'entra', + idOnTheSource: 'entra-remote-eng', + memberIds: [], + }, + ]); + }); + + test('should find groups by name pattern', async () => { + const groups = await methods.findGroupsByNamePattern('Engineering'); + + expect(groups).toHaveLength(3); + expect(groups.every((g) => g.name.includes('Engineering'))).toBe(true); + }); + + test('should respect case insensitive search', async () => { + const groups = await methods.findGroupsByNamePattern('engineering'); + + expect(groups).toHaveLength(3); + }); + + test('should filter by source when provided', async () => { + const groups = await methods.findGroupsByNamePattern('Engineering', 'local'); + + expect(groups).toHaveLength(2); + expect(groups.every((g) => g.source === 'local')).toBe(true); + }); + + test('should respect limit parameter', async () => { + const groups = await methods.findGroupsByNamePattern('Engineering', null, 2); + + expect(groups).toHaveLength(2); + }); + + test('should return empty array for no matches', async () => { + const groups = await methods.findGroupsByNamePattern('NonExistent'); + + expect(groups).toEqual([]); + }); + }); + + describe('findGroupsByMemberId', () => { + let user1: mongoose.HydratedDocument; + + beforeEach(async () => { + user1 = await User.create({ + name: 'User 1', + email: 'user1@test.com', + provider: 'local', + }); + }); + + test('should find groups by member ObjectId', async () => { + await Group.create([ + { + name: 'Group 1', + source: 'local', + memberIds: [(user1._id as mongoose.Types.ObjectId).toString(), 'other-user'], + }, + { + name: 'Group 2', + source: 'local', + memberIds: [(user1._id as mongoose.Types.ObjectId).toString()], + }, + { + name: 'Group 3', + source: 'local', + memberIds: ['other-user'], + }, + ]); + + const groups = await methods.findGroupsByMemberId(user1._id as mongoose.Types.ObjectId); + + expect(groups).toHaveLength(2); + expect(groups.map((g) => g.name).sort()).toEqual(['Group 1', 'Group 2']); + }); + + test('should find groups by member string ID', async () => { + await Group.create([ + { + name: 'Group 1', + source: 'local', + memberIds: [(user1._id as mongoose.Types.ObjectId).toString()], + }, + ]); + + const groups = await methods.findGroupsByMemberId(user1._id as mongoose.Types.ObjectId); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Group 1'); + }); + + test('should return empty array for user with no groups', async () => { + const groups = await methods.findGroupsByMemberId(user1._id as mongoose.Types.ObjectId); + + expect(groups).toEqual([]); + }); + }); + + describe('createGroup', () => { + test('should create a group with all fields', async () => { + const groupData: Partial = { + name: 'New Group', + source: 'local', + description: 'A test group', + email: 'group@test.com', + avatar: 'avatar-url', + memberIds: ['user1', 'user2'], + }; + + const group = await methods.createGroup(groupData); + + expect(group).toBeDefined(); + expect(group.name).toBe(groupData.name); + expect(group.source).toBe(groupData.source); + expect(group.description).toBe(groupData.description); + expect(group.email).toBe(groupData.email); + expect(group.avatar).toBe(groupData.avatar); + expect(group.memberIds).toEqual(groupData.memberIds); + }); + + test('should create group with minimal data', async () => { + const group = await methods.createGroup({ + name: 'Minimal Group', + }); + + expect(group).toBeDefined(); + expect(group.name).toBe('Minimal Group'); + expect(group.source).toBe('local'); // default + expect(group.memberIds).toEqual([]); // default + }); + }); + + describe('upsertGroupByExternalId', () => { + test('should create new group when not exists', async () => { + const group = await methods.upsertGroupByExternalId('new-external-id', 'entra', { + name: 'New External Group', + description: 'Created by upsert', + }); + + expect(group).toBeDefined(); + expect(group?.idOnTheSource).toBe('new-external-id'); + expect(group?.source).toBe('entra'); + expect(group?.name).toBe('New External Group'); + expect(group?.description).toBe('Created by upsert'); + }); + + test('should update existing group', async () => { + // Create initial group + await Group.create({ + name: 'Original Name', + source: 'entra', + idOnTheSource: 'existing-id', + description: 'Original description', + memberIds: ['user1'], + }); + + // Upsert with updates + const updated = await methods.upsertGroupByExternalId('existing-id', 'entra', { + name: 'Updated Name', + description: 'Updated description', + memberIds: ['user1', 'user2'], + }); + + expect(updated).toBeDefined(); + expect(updated?.name).toBe('Updated Name'); + expect(updated?.description).toBe('Updated description'); + expect(updated?.memberIds).toEqual(['user1', 'user2']); + expect(updated?.idOnTheSource).toBe('existing-id'); // unchanged + }); + + test('should not update group from different source', async () => { + await Group.create({ + name: 'Entra Group', + source: 'entra', + idOnTheSource: 'shared-id', + }); + + const result = await methods.upsertGroupByExternalId('shared-id', 'local', { + name: 'Azure Group', + }); + + // Should create new group + expect(result?.name).toBe('Azure Group'); + expect(result?.source).toBe('local'); + + // Verify both exist + const groups = await Group.find({ idOnTheSource: 'shared-id' }); + expect(groups).toHaveLength(2); + }); + }); + + describe('addUserToGroup and removeUserFromGroup', () => { + let user: mongoose.HydratedDocument; + let userWithExternal: mongoose.HydratedDocument; + let group: mongoose.HydratedDocument; + + beforeEach(async () => { + user = await User.create({ + name: 'Test User', + email: 'user@test.com', + provider: 'local', + }); + + userWithExternal = await User.create({ + name: 'External User', + email: 'external@test.com', + provider: 'entra', + idOnTheSource: 'external-123', + }); + + group = await Group.create({ + name: 'Test Group', + source: 'local', + memberIds: [], + }); + }); + + test('should add user to group using user ID', async () => { + const result = await methods.addUserToGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + expect(result.user).toBeDefined(); + expect(result.group).toBeDefined(); + expect(result.group?.memberIds).toContain((user._id as mongoose.Types.ObjectId).toString()); + }); + + test('should add user to group using idOnTheSource if available', async () => { + const result = await methods.addUserToGroup( + userWithExternal._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + expect(result.group?.memberIds).toContain('external-123'); + expect(result.group?.memberIds).not.toContain( + (userWithExternal._id as mongoose.Types.ObjectId).toString(), + ); + }); + + test('should not duplicate user in group', async () => { + // Add user first time + await methods.addUserToGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + // Add same user again + const result = await methods.addUserToGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + expect(result.group?.memberIds).toHaveLength(1); + expect(result.group?.memberIds).toContain((user._id as mongoose.Types.ObjectId).toString()); + }); + + test('should remove user from group', async () => { + // First add user + await methods.addUserToGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + // Then remove + const result = await methods.removeUserFromGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + expect(result.group?.memberIds).toHaveLength(0); + expect(result.group?.memberIds).not.toContain( + (user._id as mongoose.Types.ObjectId).toString(), + ); + }); + + test('should handle removing user not in group', async () => { + const result = await methods.removeUserFromGroup( + user._id as mongoose.Types.ObjectId, + group._id as mongoose.Types.ObjectId, + ); + + expect(result.group?.memberIds).toHaveLength(0); + }); + }); + + describe('getUserGroups', () => { + let user: mongoose.HydratedDocument; + + beforeEach(async () => { + user = await User.create({ + name: 'Test User', + email: 'user@test.com', + provider: 'local', + }); + }); + + test('should get all groups for a user', async () => { + // Create groups with user as member + await Group.create([ + { + name: 'Group 1', + source: 'local', + memberIds: [(user._id as mongoose.Types.ObjectId).toString()], + }, + { + name: 'Group 2', + source: 'local', + memberIds: [(user._id as mongoose.Types.ObjectId).toString(), 'other-user'], + }, + { + name: 'Group 3', + source: 'local', + memberIds: ['other-user'], + }, + ]); + + const groups = await methods.getUserGroups(user._id as mongoose.Types.ObjectId); + + expect(groups).toHaveLength(2); + expect(groups.map((g) => g.name).sort()).toEqual(['Group 1', 'Group 2']); + }); + + test('should return empty array for user with no groups', async () => { + const groups = await methods.getUserGroups(user._id as mongoose.Types.ObjectId); + + expect(groups).toEqual([]); + }); + + test('should handle user with idOnTheSource', async () => { + const externalUser = await User.create({ + name: 'External User', + email: 'external@test.com', + provider: 'entra', + idOnTheSource: 'external-456', + }); + + await Group.create({ + name: 'External Group', + source: 'entra', + idOnTheSource: 'entra-external-group', + memberIds: ['external-456'], // Using idOnTheSource + }); + + const groups = await methods.getUserGroups(externalUser._id as mongoose.Types.ObjectId); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('External Group'); + }); + }); + + describe('syncUserEntraGroups', () => { + let user: mongoose.HydratedDocument; + + beforeEach(async () => { + user = await User.create({ + name: 'Entra User', + email: 'entra@test.com', + provider: 'entra', + idOnTheSource: 'entra-user-123', + }); + }); + + test('should create new groups and add user', async () => { + const entraGroups = [ + { id: 'group-1', name: 'Entra Group 1' }, + { id: 'group-2', name: 'Entra Group 2' }, + ]; + + const result = await methods.syncUserEntraGroups( + user._id as mongoose.Types.ObjectId, + entraGroups, + ); + + expect(result.user).toBeDefined(); + expect(result.addedGroups).toHaveLength(2); + expect(result.removedGroups).toHaveLength(0); + + // Verify groups were created + const groups = await Group.find({ source: 'entra' }); + expect(groups).toHaveLength(2); + + // Verify user is member of both + for (const group of groups) { + expect(group.memberIds).toContain('entra-user-123'); + } + }); + + test('should remove user from groups not in sync list', async () => { + // Create existing groups + const group1 = await Group.create({ + name: 'Keep Group', + source: 'entra', + idOnTheSource: 'keep-group', + memberIds: ['entra-user-123'], + }); + + const group2 = await Group.create({ + name: 'Remove Group', + source: 'entra', + idOnTheSource: 'remove-group', + memberIds: ['entra-user-123'], + }); + + // Sync with only one group + const result = await methods.syncUserEntraGroups(user._id as mongoose.Types.ObjectId, [ + { id: 'keep-group', name: 'Keep Group' }, + ]); + + expect(result.addedGroups).toHaveLength(0); + expect(result.removedGroups).toHaveLength(1); + + // Verify membership + const keepGroup = await Group.findById(group1._id); + const removeGroup = await Group.findById(group2._id); + + expect(keepGroup?.memberIds).toContain('entra-user-123'); + expect(removeGroup?.memberIds).not.toContain('entra-user-123'); + }); + + test('should not affect local groups', async () => { + // Create local group + const localGroup = await Group.create({ + name: 'Local Group', + source: 'local', + memberIds: ['entra-user-123'], + }); + + // Sync entra groups + await methods.syncUserEntraGroups(user._id as mongoose.Types.ObjectId, [ + { id: 'entra-group', name: 'Entra Group' }, + ]); + + // Verify local group unchanged + const savedLocalGroup = await Group.findById(localGroup._id); + expect(savedLocalGroup?.memberIds).toContain('entra-user-123'); + }); + + test('should throw error for non-existent user', async () => { + const fakeId = new mongoose.Types.ObjectId(); + + await expect(methods.syncUserEntraGroups(fakeId, [])).rejects.toThrow('User not found'); + }); + }); + + describe('sortPrincipalsByRelevance', () => { + test('should sort principals by relevance score', async () => { + const principals = [ + { id: '1', name: 'Test User', type: 'user' as const, source: 'local' as const }, + { id: '2', name: 'Admin Test', type: 'user' as const, source: 'local' as const }, + { id: '3', name: 'Test Group', type: 'group' as const, source: 'local' as const }, + ]; + + // Store original query in closure or pass it through + const sorted = methods.sortPrincipalsByRelevance(principals); + + // Since we can't pass the query directly, the method should maintain + // the original order or have been called in a context where it knows the query + expect(sorted).toBeDefined(); + expect(sorted).toHaveLength(3); + }); + }); +}); diff --git a/packages/data-schemas/src/methods/userGroup.roles.spec.ts b/packages/data-schemas/src/methods/userGroup.roles.spec.ts new file mode 100644 index 0000000000..fa50c6f4c4 --- /dev/null +++ b/packages/data-schemas/src/methods/userGroup.roles.spec.ts @@ -0,0 +1,383 @@ +import mongoose from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import type * as t from '~/types'; +import { createUserGroupMethods } from './userGroup'; +import groupSchema from '~/schema/group'; +import userSchema from '~/schema/user'; +import roleSchema from '~/schema/role'; + +/** Mocking logger */ +jest.mock('~/config/winston', () => ({ + error: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + +let mongoServer: MongoMemoryServer; +let Group: mongoose.Model; +let User: mongoose.Model; +let Role: mongoose.Model; +let methods: ReturnType; + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + const mongoUri = mongoServer.getUri(); + await mongoose.connect(mongoUri); + + /** Register models */ + Group = mongoose.models.Group || mongoose.model('Group', groupSchema); + User = mongoose.models.User || mongoose.model('User', userSchema); + Role = mongoose.models.Role || mongoose.model('Role', roleSchema); + + /** Initialize methods */ + methods = createUserGroupMethods(mongoose); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await mongoose.connection.dropDatabase(); +}); + +describe('Role-based Permissions Integration', () => { + describe('getUserPrincipals with roles', () => { + test('should include role principal for user with role', async () => { + const adminUser = await User.create({ + name: 'Admin User', + email: 'admin@test.com', + provider: 'local', + role: 'admin', + }); + + const principals = await methods.getUserPrincipals({ + userId: adminUser._id as mongoose.Types.ObjectId, + }); + + // Should have user, role, and public principals + expect(principals).toHaveLength(3); + + const userPrincipal = principals.find((p) => p.principalType === PrincipalType.USER); + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + const publicPrincipal = principals.find((p) => p.principalType === PrincipalType.PUBLIC); + + expect(userPrincipal).toBeDefined(); + expect(userPrincipal?.principalId?.toString()).toBe( + (adminUser._id as mongoose.Types.ObjectId).toString(), + ); + + expect(rolePrincipal).toBeDefined(); + expect(rolePrincipal?.principalType).toBe(PrincipalType.ROLE); + expect(rolePrincipal?.principalId).toBe('admin'); + + expect(publicPrincipal).toBeDefined(); + expect(publicPrincipal?.principalType).toBe(PrincipalType.PUBLIC); + expect(publicPrincipal?.principalId).toBeUndefined(); + }); + + test('should not include role principal for user without role', async () => { + const regularUser = await User.create({ + name: 'Regular User', + email: 'user@test.com', + provider: 'local', + role: null, // Explicitly set to null to override default + }); + + const principals = await methods.getUserPrincipals({ + userId: regularUser._id as mongoose.Types.ObjectId, + }); + + // Should only have user and public principals + expect(principals).toHaveLength(2); + + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal).toBeUndefined(); + }); + + test('should include all principal types for user with role and groups', async () => { + const user = await User.create({ + name: 'Complete User', + email: 'complete@test.com', + provider: 'local', + role: 'moderator', + }); + + // Add user to groups + const group1 = await Group.create({ + name: 'Group 1', + source: 'local', + memberIds: [(user._id as mongoose.Types.ObjectId).toString()], + }); + + const group2 = await Group.create({ + name: 'Group 2', + source: 'local', + memberIds: [(user._id as mongoose.Types.ObjectId).toString()], + }); + + const principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + + // Should have user, role, 2 groups, and public + expect(principals).toHaveLength(5); + + const principalTypes = principals.map((p) => p.principalType); + expect(principalTypes).toContain(PrincipalType.USER); + expect(principalTypes).toContain(PrincipalType.ROLE); + expect(principalTypes).toContain(PrincipalType.GROUP); + expect(principalTypes).toContain(PrincipalType.PUBLIC); + + // Check role principal + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal?.principalId).toBe('moderator'); + + // Check group principals + const groupPrincipals = principals.filter((p) => p.principalType === PrincipalType.GROUP); + expect(groupPrincipals).toHaveLength(2); + const groupIds = groupPrincipals.map((p) => p.principalId?.toString()); + expect(groupIds).toContain(group1._id.toString()); + expect(groupIds).toContain(group2._id.toString()); + }); + + test('should handle different role values', async () => { + const testCases = [ + { role: 'admin', expected: 'admin' }, + { role: 'moderator', expected: 'moderator' }, + { role: 'editor', expected: 'editor' }, + { role: 'viewer', expected: 'viewer' }, + { role: 'custom_role', expected: 'custom_role' }, + ]; + + for (const testCase of testCases) { + const user = await User.create({ + name: `User with ${testCase.role}`, + email: `${testCase.role}@test.com`, + provider: 'local', + role: testCase.role, + }); + + const principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + + expect(rolePrincipal).toBeDefined(); + expect(rolePrincipal?.principalId).toBe(testCase.expected); + } + }); + }); + + describe('searchPrincipals with role support', () => { + beforeEach(async () => { + // Create some roles in the database + await Role.create([ + { name: 'admin', description: 'Administrator role' }, + { name: 'moderator', description: 'Moderator role' }, + { name: 'editor', description: 'Editor role' }, + { name: 'viewer', description: 'Viewer role' }, + { name: 'guest', description: 'Guest role' }, + ]); + + // Create some users + await User.create([ + { + name: 'Admin User', + email: 'admin@test.com', + username: 'adminuser', + provider: 'local', + role: 'admin', + }, + { + name: 'Moderator User', + email: 'moderator@test.com', + username: 'moduser', + provider: 'local', + role: 'moderator', + }, + ]); + + // Create some groups + await Group.create([ + { + name: 'Admin Group', + source: 'local', + memberIds: [], + }, + { + name: 'Moderator Group', + source: 'local', + memberIds: [], + }, + ]); + }); + + test('should search for roles when Role model exists', async () => { + const results = await methods.searchPrincipals('admin'); + + const roleResults = results.filter((r) => r.type === PrincipalType.ROLE); + const userResults = results.filter((r) => r.type === PrincipalType.USER); + const groupResults = results.filter((r) => r.type === PrincipalType.GROUP); + + // Should find the admin role + expect(roleResults).toHaveLength(1); + expect(roleResults[0].id).toBe('admin'); + expect(roleResults[0].name).toBe('admin'); + expect(roleResults[0].type).toBe(PrincipalType.ROLE); + + // Should also find admin user and group + expect(userResults.some((u) => u.name === 'Admin User')).toBe(true); + expect(groupResults.some((g) => g.name === 'Admin Group')).toBe(true); + }); + + test('should filter search results by role type', async () => { + const results = await methods.searchPrincipals('mod', 10, PrincipalType.ROLE); + + expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true); + expect(results).toHaveLength(1); + expect(results[0].name).toBe('moderator'); + }); + + test('should respect limit for role search', async () => { + // Create many roles + for (let i = 0; i < 10; i++) { + await Role.create({ name: `testrole${i}` }); + } + + const results = await methods.searchPrincipals('testrole', 5, PrincipalType.ROLE); + + expect(results).toHaveLength(5); + expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true); + }); + + test('should search across all principal types', async () => { + const results = await methods.searchPrincipals('mod'); + + // Should find moderator role, user, and group + const types = new Set(results.map((r) => r.type)); + expect(types.has(PrincipalType.ROLE)).toBe(true); + expect(types.has(PrincipalType.USER)).toBe(true); + expect(types.has(PrincipalType.GROUP)).toBe(true); + + // Check specific results + expect(results.some((r) => r.type === PrincipalType.ROLE && r.name === 'moderator')).toBe( + true, + ); + expect( + results.some((r) => r.type === PrincipalType.USER && r.name === 'Moderator User'), + ).toBe(true); + expect( + results.some((r) => r.type === PrincipalType.GROUP && r.name === 'Moderator Group'), + ).toBe(true); + }); + + test('should handle case-insensitive role search', async () => { + const results = await methods.searchPrincipals('ADMIN', 10, PrincipalType.ROLE); + + expect(results).toHaveLength(1); + expect(results[0].name).toBe('admin'); + }); + + test('should return empty array for no role matches', async () => { + const results = await methods.searchPrincipals('nonexistentrole', 10, PrincipalType.ROLE); + + expect(results).toEqual([]); + }); + }); + + describe('Role principals in complex scenarios', () => { + test('should handle user role changes', async () => { + const user = await User.create({ + name: 'Changing User', + email: 'change@test.com', + provider: 'local', + role: 'viewer', + }); + + // Initial principals + let principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + let rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal?.principalId).toBe('viewer'); + + // Change role + user.role = 'editor'; + await user.save(); + + // Get principals again + principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal?.principalId).toBe('editor'); + }); + + test('should handle user role removal', async () => { + const user = await User.create({ + name: 'Demoted User', + email: 'demoted@test.com', + provider: 'local', + role: 'admin', + }); + + // Initial check + let principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + expect(principals).toHaveLength(3); // user, role, public + + // Remove role + user.role = undefined; + await user.save(); + + // Check again + principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + expect(principals).toHaveLength(2); // user, public + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal).toBeUndefined(); + }); + + test('should handle empty or null role values', async () => { + const testCases = [ + { role: '', expected: false }, + { role: null, expected: false }, + { role: undefined, expected: true, expectedRole: 'USER' }, // undefined gets default 'USER' + { role: ' ', expected: false }, // whitespace-only is not a valid role + { role: 'valid_role', expected: true, expectedRole: 'valid_role' }, + ]; + + for (const testCase of testCases) { + const userData: Partial = { + name: `User ${Math.random()}`, + email: `test${Math.random()}@test.com`, + provider: 'local', + }; + + // Only set role if it's not undefined (to test undefined case) + if (testCase.role !== undefined) { + userData.role = testCase.role as string; + } + + const user = await User.create(userData); + + const principals = await methods.getUserPrincipals({ + userId: user._id as mongoose.Types.ObjectId, + }); + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + + if (testCase.expected) { + expect(rolePrincipal).toBeDefined(); + expect(rolePrincipal?.principalId).toBe(testCase.expectedRole || testCase.role); + } else { + expect(rolePrincipal).toBeUndefined(); + } + } + }); + }); +}); diff --git a/packages/data-schemas/src/methods/userGroup.spec.ts b/packages/data-schemas/src/methods/userGroup.spec.ts index fb2dfd7ab0..9de8eaf912 100644 --- a/packages/data-schemas/src/methods/userGroup.spec.ts +++ b/packages/data-schemas/src/methods/userGroup.spec.ts @@ -21,8 +21,8 @@ let methods: ReturnType; beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); const mongoUri = mongoServer.getUri(); - Group = mongoose.models.Group || mongoose.model('Group', groupSchema); - User = mongoose.models.User || mongoose.model('User', userSchema); + Group = mongoose.models.Group || mongoose.model('Group', groupSchema); + User = mongoose.models.User || mongoose.model('User', userSchema); methods = createUserGroupMethods(mongoose); await mongoose.connect(mongoUri); }); @@ -325,10 +325,12 @@ describe('User Group Methods Tests', () => { ); /** Get user principals */ - const principals = await methods.getUserPrincipals(testUser1._id as mongoose.Types.ObjectId); + const principals = await methods.getUserPrincipals({ + userId: testUser1._id as mongoose.Types.ObjectId, + }); - /** Should include user, group, and public principals */ - expect(principals).toHaveLength(3); + /** Should include user, role (default USER), group, and public principals */ + expect(principals).toHaveLength(4); /** Check principal types */ const userPrincipal = principals.find((p) => p.principalType === PrincipalType.USER); @@ -349,7 +351,9 @@ describe('User Group Methods Tests', () => { test('should return user and public principals for non-existent user in getUserPrincipals', async () => { const nonExistentId = new mongoose.Types.ObjectId(); - const principals = await methods.getUserPrincipals(nonExistentId); + const principals = await methods.getUserPrincipals({ + userId: nonExistentId, + }); /** Should still return user and public principals even for non-existent user */ expect(principals).toHaveLength(2); @@ -358,6 +362,61 @@ describe('User Group Methods Tests', () => { expect(principals[1].principalType).toBe(PrincipalType.PUBLIC); expect(principals[1].principalId).toBeUndefined(); }); + + test('should convert string userId to ObjectId in getUserPrincipals', async () => { + /** Add user to a group */ + await methods.addUserToGroup( + testUser1._id as mongoose.Types.ObjectId, + testGroup._id as mongoose.Types.ObjectId, + ); + + /** Get user principals with string userId */ + const principals = await methods.getUserPrincipals({ + userId: (testUser1._id as mongoose.Types.ObjectId).toString(), + }); + + /** Should include user, role (default USER), group, and public principals */ + expect(principals).toHaveLength(4); + + /** Check that USER principal has ObjectId */ + const userPrincipal = principals.find((p) => p.principalType === PrincipalType.USER); + expect(userPrincipal).toBeDefined(); + expect(userPrincipal?.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(userPrincipal?.principalId?.toString()).toBe( + (testUser1._id as mongoose.Types.ObjectId).toString(), + ); + + /** Check that GROUP principal has ObjectId */ + const groupPrincipal = principals.find((p) => p.principalType === PrincipalType.GROUP); + expect(groupPrincipal).toBeDefined(); + expect(groupPrincipal?.principalId).toBeInstanceOf(mongoose.Types.ObjectId); + expect(groupPrincipal?.principalId?.toString()).toBe(testGroup._id.toString()); + }); + + test('should include role principal as string in getUserPrincipals', async () => { + /** Create user with specific role */ + const userWithRole = await User.create({ + name: 'Admin User', + email: 'admin@example.com', + password: 'password123', + provider: 'local', + role: 'ADMIN', + }); + + /** Get user principals */ + const principals = await methods.getUserPrincipals({ + userId: userWithRole._id as mongoose.Types.ObjectId, + }); + + /** Should include user, role, and public principals */ + expect(principals).toHaveLength(3); + + /** Check that ROLE principal has string ID */ + const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); + expect(rolePrincipal).toBeDefined(); + expect(typeof rolePrincipal?.principalId).toBe('string'); + expect(rolePrincipal?.principalId).toBe('ADMIN'); + }); }); describe('Entra ID Synchronization', () => { diff --git a/packages/data-schemas/src/methods/userGroup.ts b/packages/data-schemas/src/methods/userGroup.ts index 0545bc1fbc..2464e35c9e 100644 --- a/packages/data-schemas/src/methods/userGroup.ts +++ b/packages/data-schemas/src/methods/userGroup.ts @@ -1,7 +1,8 @@ +import { Types } from 'mongoose'; import { PrincipalType } from 'librechat-data-provider'; import type { TUser, TPrincipalSearchResult } from 'librechat-data-provider'; -import type { Model, Types, ClientSession } from 'mongoose'; -import type { IGroup, IUser } from '~/types'; +import type { Model, ClientSession } from 'mongoose'; +import type { IGroup, IRole, IUser } from '~/types'; export function createUserGroupMethods(mongoose: typeof import('mongoose')) { /** @@ -237,22 +238,47 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { /** * Get a list of all principal identifiers for a user (user ID + group IDs + public) * For use in permission checks - * @param userId - The user ID + * @param params - Parameters object + * @param params.userId - The user ID + * @param params.role - Optional user role (if not provided, will query from DB) * @param session - Optional MongoDB session for transactions * @returns Array of principal objects with type and id */ async function getUserPrincipals( - userId: string | Types.ObjectId, + params: { + userId: string | Types.ObjectId; + role?: string | null; + }, session?: ClientSession, ): Promise> { + const { userId, role } = params; + /** `userId` must be an `ObjectId` for USER principal since ACL entries store `ObjectId`s */ + const userObjectId = typeof userId === 'string' ? new Types.ObjectId(userId) : userId; const principals: Array<{ principalType: string; principalId?: string | Types.ObjectId }> = [ - { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.USER, principalId: userObjectId }, ]; + // If role is not provided, query user to get it + let userRole = role; + if (userRole === undefined) { + const User = mongoose.models.User as Model; + const query = User.findById(userId).select('role'); + if (session) { + query.session(session); + } + const user = await query.lean(); + userRole = user?.role; + } + + // Add role as a principal if user has one + if (userRole && userRole.trim()) { + principals.push({ principalType: PrincipalType.ROLE, principalId: userRole }); + } + const userGroups = await getUserGroups(userId, session); if (userGroups && userGroups.length > 0) { userGroups.forEach((group) => { - principals.push({ principalType: PrincipalType.GROUP, principalId: group._id.toString() }); + principals.push({ principalType: PrincipalType.GROUP, principalId: group._id }); }); } @@ -374,7 +400,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { /** Get searchable text based on type */ const searchableFields = - item.type === 'user' + item.type === PrincipalType.USER ? [item.name, item.email, item.username].filter(Boolean) : [item.name, item.email, item.description].filter(Boolean); @@ -418,7 +444,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { return (b._searchScore || 0) - (a._searchScore || 0); } if (a.type !== b.type) { - return a.type === 'user' ? -1 : 1; + return a.type === PrincipalType.USER ? -1 : 1; } const aName = a.name || a.email || ''; const bName = b.name || b.email || ''; @@ -434,7 +460,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { function transformUserToTPrincipalSearchResult(user: TUser): TPrincipalSearchResult { return { id: user.id, - type: 'user', + type: PrincipalType.USER, name: user.name || user.email, email: user.email, username: user.username, @@ -453,7 +479,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { function transformGroupToTPrincipalSearchResult(group: IGroup): TPrincipalSearchResult { return { id: group._id?.toString(), - type: 'group', + type: PrincipalType.GROUP, name: group.name, email: group.email, avatar: group.avatar, @@ -469,14 +495,14 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { * Returns combined results in TPrincipalSearchResult format without sorting * @param searchPattern - The pattern to search for * @param limitPerType - Maximum number of results to return - * @param typeFilter - Optional filter: 'user', 'group', or null for all + * @param typeFilter - Optional filter: PrincipalType.USER, PrincipalType.GROUP, or null for all * @param session - Optional MongoDB session for transactions * @returns Array of principals in TPrincipalSearchResult format */ async function searchPrincipals( searchPattern: string, limitPerType: number = 10, - typeFilter: 'user' | 'group' | null = null, + typeFilter: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE | null = null, session?: ClientSession, ): Promise { if (!searchPattern || searchPattern.trim().length === 0) { @@ -486,7 +512,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { const trimmedPattern = searchPattern.trim(); const promises: Promise[] = []; - if (!typeFilter || typeFilter === 'user') { + if (!typeFilter || typeFilter === PrincipalType.USER) { /** Note: searchUsers is imported from ~/models and needs to be passed in or implemented */ const userFields = 'name email username avatar provider idOnTheSource'; /** For now, we'll use a direct query instead of searchUsers */ @@ -521,7 +547,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { promises.push(Promise.resolve([])); } - if (!typeFilter || typeFilter === 'group') { + if (!typeFilter || typeFilter === PrincipalType.GROUP) { promises.push( findGroupsByNamePattern(trimmedPattern, null, limitPerType, session).then((groups) => groups.map(transformGroupToTPrincipalSearchResult), @@ -531,9 +557,34 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { promises.push(Promise.resolve([])); } - const [users, groups] = await Promise.all(promises); + if (!typeFilter || typeFilter === PrincipalType.ROLE) { + const Role = mongoose.models.Role as Model; + if (Role) { + const regex = new RegExp(trimmedPattern, 'i'); + const roleQuery = Role.find({ name: regex }).select('name').limit(limitPerType); - const combined = [...users, ...groups]; + if (session) { + roleQuery.session(session); + } + + promises.push( + roleQuery.lean().then((roles) => + roles.map((role) => ({ + /** Role name as ID */ + id: role.name, + type: PrincipalType.ROLE, + name: role.name, + source: 'local' as const, + })), + ), + ); + } + } else { + promises.push(Promise.resolve([])); + } + + const results = await Promise.all(promises); + const combined = results.flat(); return combined; } diff --git a/packages/data-schemas/src/schema/aclEntry.ts b/packages/data-schemas/src/schema/aclEntry.ts index 597e6d9d5c..dbaf73b466 100644 --- a/packages/data-schemas/src/schema/aclEntry.ts +++ b/packages/data-schemas/src/schema/aclEntry.ts @@ -10,7 +10,7 @@ const aclEntrySchema = new Schema( required: true, }, principalId: { - type: Schema.Types.ObjectId, + type: Schema.Types.Mixed, // Can be ObjectId for users/groups or String for roles refPath: 'principalModel', required: function (this: IAclEntry) { return this.principalType !== PrincipalType.PUBLIC; diff --git a/packages/data-schemas/src/schema/role.ts b/packages/data-schemas/src/schema/role.ts index 78f22f9b3a..6662bd47bf 100644 --- a/packages/data-schemas/src/schema/role.ts +++ b/packages/data-schemas/src/schema/role.ts @@ -42,6 +42,7 @@ const rolePermissionsSchema = new Schema( [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: { type: Boolean, default: false }, [Permissions.VIEW_GROUPS]: { type: Boolean, default: false }, + [Permissions.VIEW_ROLES]: { type: Boolean, default: false }, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: { type: Boolean, default: false }, @@ -85,6 +86,7 @@ const roleSchema: Schema = new Schema({ [PermissionTypes.PEOPLE_PICKER]: { [Permissions.VIEW_USERS]: false, [Permissions.VIEW_GROUPS]: false, + [Permissions.VIEW_ROLES]: false, }, [PermissionTypes.MARKETPLACE]: { [Permissions.USE]: false }, [PermissionTypes.FILE_SEARCH]: { [Permissions.USE]: true }, diff --git a/packages/data-schemas/src/types/aclEntry.ts b/packages/data-schemas/src/types/aclEntry.ts index 13fa69ad5f..026b852aa8 100644 --- a/packages/data-schemas/src/types/aclEntry.ts +++ b/packages/data-schemas/src/types/aclEntry.ts @@ -2,13 +2,13 @@ import type { Document, Types } from 'mongoose'; import { PrincipalType, PrincipalModel, ResourceType } from 'librechat-data-provider'; export type AclEntry = { - /** The type of principal ('user', 'group', 'public') */ + /** The type of principal (PrincipalType.USER, PrincipalType.GROUP, PrincipalType.PUBLIC) */ principalType: PrincipalType; - /** The ID of the principal (null for 'public') */ - principalId?: Types.ObjectId; - /** The model name for the principal ('User' or 'Group') */ + /** The ID of the principal (null for PrincipalType.PUBLIC, string for PrincipalType.ROLE) */ + principalId?: Types.ObjectId | string; + /** The model name for the principal (`PrincipalModel`) */ principalModel?: PrincipalModel; - /** The type of resource ('agent', 'project', 'file', 'promptGroup') */ + /** The type of resource (`ResourceType`) */ resourceType: ResourceType; /** The ID of the resource */ resourceId: Types.ObjectId; diff --git a/packages/data-schemas/src/types/role.ts b/packages/data-schemas/src/types/role.ts index 515b5236f8..281b8b4239 100644 --- a/packages/data-schemas/src/types/role.ts +++ b/packages/data-schemas/src/types/role.ts @@ -38,6 +38,7 @@ export interface IRole extends Document { [PermissionTypes.PEOPLE_PICKER]?: { [Permissions.VIEW_USERS]?: boolean; [Permissions.VIEW_GROUPS]?: boolean; + [Permissions.VIEW_ROLES]?: boolean; }; [PermissionTypes.MARKETPLACE]?: { [Permissions.USE]?: boolean;