WIP: add user role check optimization to user principal check, update type comparisons

This commit is contained in:
Danny Avila 2025-08-03 21:53:06 -04:00
parent 89f0a4e02f
commit ecd7bf0d51
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
19 changed files with 481 additions and 71 deletions

View file

@ -30,7 +30,12 @@ const primeFiles = async (options) => {
// Filter by access if user and agent are provided // Filter by access if user and agent are provided
let dbFiles; let dbFiles;
if (req?.user?.id && agentId) { 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 { } else {
dbFiles = allFiles; dbFiles = allFiles;
} }

View file

@ -2,7 +2,12 @@ const mongoose = require('mongoose');
const { v4: uuidv4 } = require('uuid'); const { v4: uuidv4 } = require('uuid');
const { createModels } = require('@librechat/data-schemas'); const { createModels } = require('@librechat/data-schemas');
const { MongoMemoryServer } = require('mongodb-memory-server'); 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 { grantPermission } = require('~/server/services/PermissionService');
const { getFiles, createFile } = require('./File'); const { getFiles, createFile } = require('./File');
const { seedDefaultRoles } = require('~/models'); const { seedDefaultRoles } = require('~/models');
@ -125,7 +130,12 @@ describe('File Access Control', () => {
// Check access for all files // Check access for all files
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); 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 // Should have access only to the first two files
expect(accessMap.get(fileIds[0])).toBe(true); expect(accessMap.get(fileIds[0])).toBe(true);
@ -163,7 +173,12 @@ describe('File Access Control', () => {
// Check access as the author // Check access as the author
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); 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 // Author should have access to all files
expect(accessMap.get(fileIds[0])).toBe(true); expect(accessMap.get(fileIds[0])).toBe(true);
@ -184,11 +199,12 @@ describe('File Access Control', () => {
}); });
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
const accessMap = await hasAccessToFilesViaAgent( const accessMap = await hasAccessToFilesViaAgent({
userId.toString(), userId: userId,
role: SystemRoles.USER,
fileIds, fileIds,
'non-existent-agent', agentId: 'non-existent-agent',
); });
// Should have no access to any files // Should have no access to any files
expect(accessMap.get(fileIds[0])).toBe(false); expect(accessMap.get(fileIds[0])).toBe(false);
@ -242,7 +258,12 @@ describe('File Access Control', () => {
// Check access for files // Check access for files
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions'); 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 // Should have no access to any files when only VIEW permission
expect(accessMap.get(fileIds[0])).toBe(false); expect(accessMap.get(fileIds[0])).toBe(false);
@ -336,7 +357,12 @@ describe('File Access Control', () => {
// Then filter by access control // Then filter by access control
const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); 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).toHaveLength(2);
expect(files.map((f) => f.file_id)).toContain(ownedFileId); expect(files.map((f) => f.file_id)).toContain(ownedFileId);
@ -371,4 +397,166 @@ describe('File Access Control', () => {
expect(files).toHaveLength(2); 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);
});
});
}); });

View file

@ -334,10 +334,8 @@ describe('Prompt ACL Permissions', () => {
productionId: new ObjectId(), productionId: new ObjectId(),
}); });
// Add editor to the editors group const { addUserToGroup } = require('~/models');
await Group.findByIdAndUpdate(testGroups.editors._id, { await addUserToGroup(testUsers.editor._id, testGroups.editors._id);
$push: { memberIds: testUsers.editor._id },
});
const prompt = await promptFns.savePrompt({ const prompt = await promptFns.savePrompt({
author: testUsers.owner._id, author: testUsers.owner._id,

View file

@ -340,6 +340,7 @@ const getUserEffectivePermissions = async (req, res) => {
const permissionBits = await getEffectivePermissions({ const permissionBits = await getEffectivePermissions({
userId, userId,
role: req.user.role,
resourceType, resourceType,
resourceId, resourceId,
}); });

View file

@ -444,6 +444,7 @@ const getListAgentsHandler = async (req, res) => {
// Get agent IDs the user has VIEW access to via ACL // Get agent IDs the user has VIEW access to via ACL
const accessibleIds = await findAccessibleResources({ const accessibleIds = await findAccessibleResources({
userId, userId,
role: req.user.role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
requiredPermissions: requiredPermission, requiredPermissions: requiredPermission,
}); });
@ -499,7 +500,7 @@ const uploadAgentAvatarHandler = async (req, res) => {
return res.status(404).json({ error: 'Agent not found' }); 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; const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor;
if (!hasEditPermission) { if (!hasEditPermission) {
@ -607,7 +608,7 @@ const revertAgentVersionHandler = async (req, res) => {
return res.status(404).json({ error: 'Agent not found' }); 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; const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor;
if (!hasEditPermission) { if (!hasEditPermission) {

View file

@ -43,7 +43,7 @@ describe('canAccessAgentResource middleware', () => {
}); });
req = { req = {
user: { id: testUser._id.toString(), role: 'test-role' }, user: { id: testUser._id, role: testUser.role },
params: {}, params: {},
}; };
res = { res = {

View file

@ -111,6 +111,7 @@ const canAccessResource = (options) => {
// Check permissions using PermissionService with ObjectId // Check permissions using PermissionService with ObjectId
const hasPermission = await checkPermission({ const hasPermission = await checkPermission({
userId, userId,
role: req.user.role,
resourceType, resourceType,
resourceId, resourceId,
requiredPermission, requiredPermission,

View file

@ -8,7 +8,7 @@ const { getFiles } = require('~/models/File');
* Checks if user has access to a file through agent permissions * 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 * 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 { try {
// Find agents that have this file in their tool_resources // Find agents that have this file in their tool_resources
const agentsWithFile = await getAgent({ const agentsWithFile = await getAgent({
@ -35,6 +35,7 @@ const checkAgentBasedFileAccess = async (userId, fileId) => {
try { try {
const permissions = await getEffectivePermissions({ const permissions = await getEffectivePermissions({
userId, userId,
role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
resourceId: agent._id || agent.id, resourceId: agent._id || agent.id,
}); });
@ -67,7 +68,7 @@ const fileAccess = async (req, res, next) => {
try { try {
const fileId = req.params.file_id; const fileId = req.params.file_id;
const userId = req.user?.id; const userId = req.user?.id;
const userRole = req.user?.role;
if (!fileId) { if (!fileId) {
return res.status(400).json({ return res.status(400).json({
error: 'Bad Request', error: 'Bad Request',
@ -98,7 +99,7 @@ const fileAccess = async (req, res, next) => {
} }
// Check agent-based access (file inherits agent permissions) // Check agent-based access (file inherits agent permissions)
const hasAgentAccess = await checkAgentBasedFileAccess(userId, fileId); const hasAgentAccess = await checkAgentBasedFileAccess({ userId, role: userRole, fileId });
if (hasAgentAccess) { if (hasAgentAccess) {
req.fileAccess = { file }; req.fileAccess = { file };
return next(); return next();

View file

@ -37,6 +37,7 @@ router.get('/', async (req, res) => {
const userId = req.user.id; const userId = req.user.id;
const editableAgentObjectIds = await findAccessibleResources({ const editableAgentObjectIds = await findAccessibleResources({
userId, userId,
role: req.user.role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
requiredPermissions: PermissionBits.EDIT, requiredPermissions: PermissionBits.EDIT,
}); });

View file

@ -79,6 +79,7 @@ router.get('/agent/:agent_id', async (req, res) => {
if (agent.author.toString() !== userId) { if (agent.author.toString() !== userId) {
const hasEditPermission = await checkPermission({ const hasEditPermission = await checkPermission({
userId, userId,
role: req.user.role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
resourceId: agent._id, resourceId: agent._id,
requiredPermission: PermissionBits.EDIT, requiredPermission: PermissionBits.EDIT,
@ -152,7 +153,7 @@ router.delete('/', async (req, res) => {
const nonOwnedFiles = []; const nonOwnedFiles = [];
for (const file of dbFiles) { for (const file of dbFiles) {
if (file.user.toString() === req.user.id) { if (file.user.toString() === req.user.id.toString()) {
ownedFiles.push(file); ownedFiles.push(file);
} else { } else {
nonOwnedFiles.push(file); nonOwnedFiles.push(file);
@ -176,11 +177,12 @@ router.delete('/', async (req, res) => {
if (req.body.agent_id && nonOwnedFiles.length > 0) { if (req.body.agent_id && nonOwnedFiles.length > 0) {
const nonOwnedFileIds = nonOwnedFiles.map((f) => f.file_id); const nonOwnedFileIds = nonOwnedFiles.map((f) => f.file_id);
const accessMap = await hasAccessToFilesViaAgent( const accessMap = await hasAccessToFilesViaAgent({
req.user.id, userId: req.user.id,
nonOwnedFileIds, role: req.user.role,
req.body.agent_id, fileIds: nonOwnedFileIds,
); agentId: req.body.agent_id,
});
for (const file of nonOwnedFiles) { for (const file of nonOwnedFiles) {
if (accessMap.get(file.file_id)) { if (accessMap.get(file.file_id)) {

View file

@ -4,7 +4,12 @@ const mongoose = require('mongoose');
const { v4: uuidv4 } = require('uuid'); const { v4: uuidv4 } = require('uuid');
const { createMethods } = require('@librechat/data-schemas'); const { createMethods } = require('@librechat/data-schemas');
const { MongoMemoryServer } = require('mongodb-memory-server'); 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 { createAgent } = require('~/models/Agent');
const { createFile } = require('~/models/File'); const { createFile } = require('~/models/File');
@ -95,9 +100,11 @@ describe('File Routes - Delete with Agent Access', () => {
app = express(); app = express();
app.use(express.json()); app.use(express.json());
// Mock authentication middleware
app.use((req, res, next) => { 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: {} }; req.app = { locals: {} };
next(); next();
}); });

View file

@ -99,6 +99,7 @@ router.get('/all', async (req, res) => {
// Get promptGroup IDs the user has VIEW access to via ACL // Get promptGroup IDs the user has VIEW access to via ACL
const accessibleIds = await findAccessibleResources({ const accessibleIds = await findAccessibleResources({
userId, userId,
role: req.user.role,
resourceType: ResourceType.PROMPTGROUP, resourceType: ResourceType.PROMPTGROUP,
requiredPermissions: PermissionBits.VIEW, requiredPermissions: PermissionBits.VIEW,
}); });
@ -130,6 +131,7 @@ router.get('/groups', async (req, res) => {
// Get promptGroup IDs the user has VIEW access to via ACL // Get promptGroup IDs the user has VIEW access to via ACL
const accessibleIds = await findAccessibleResources({ const accessibleIds = await findAccessibleResources({
userId, userId,
role: req.user.role,
resourceType: ResourceType.PROMPTGROUP, resourceType: ResourceType.PROMPTGROUP,
requiredPermissions: PermissionBits.VIEW, requiredPermissions: PermissionBits.VIEW,
}); });
@ -334,6 +336,7 @@ router.get('/', async (req, res) => {
if (groupId) { if (groupId) {
const permissions = await getEffectivePermissions({ const permissions = await getEffectivePermissions({
userId: req.user.id, userId: req.user.id,
role: req.user.role,
resourceType: ResourceType.PROMPTGROUP, resourceType: ResourceType.PROMPTGROUP,
resourceId: groupId, resourceId: groupId,
}); });

View file

@ -172,7 +172,12 @@ const primeFiles = async (options, apiKey) => {
// Filter by access if user and agent are provided // Filter by access if user and agent are provided
let dbFiles; let dbFiles;
if (req?.user?.id && agentId) { 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 { } else {
dbFiles = allFiles; dbFiles = allFiles;
} }

View file

@ -5,12 +5,14 @@ const { getAgent } = require('~/models/Agent');
/** /**
* Checks if a user has access to multiple files through a shared agent (batch operation) * 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 {Object} params - Parameters object
* @param {string[]} fileIds - Array of file IDs to check * @param {string} params.userId - The user ID to check access for
* @param {string} agentId - The agent ID that might grant access * @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<string, boolean>>} Map of fileId to access status * @returns {Promise<Map<string, boolean>>} Map of fileId to access status
*/ */
const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => { const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId }) => {
const accessMap = new Map(); const accessMap = new Map();
// Initialize all files as no access // 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 // 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)); fileIds.forEach((fileId) => accessMap.set(fileId, true));
return accessMap; return accessMap;
} }
@ -32,6 +34,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
// Check if user has at least VIEW permission on the agent // Check if user has at least VIEW permission on the agent
const hasViewPermission = await checkPermission({ const hasViewPermission = await checkPermission({
userId, userId,
role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
resourceId: agent._id, resourceId: agent._id,
requiredPermission: PermissionBits.VIEW, requiredPermission: PermissionBits.VIEW,
@ -44,6 +47,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
// Check if user has EDIT permission (which would indicate collaborative access) // Check if user has EDIT permission (which would indicate collaborative access)
const hasEditPermission = await checkPermission({ const hasEditPermission = await checkPermission({
userId, userId,
role,
resourceType: ResourceType.AGENT, resourceType: ResourceType.AGENT,
resourceId: agent._id, resourceId: agent._id,
requiredPermission: PermissionBits.EDIT, requiredPermission: PermissionBits.EDIT,
@ -81,12 +85,14 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
/** /**
* Filter files based on user access through agents * Filter files based on user access through agents
* @param {Array<MongoFile>} files - Array of file documents * @param {Object} params - Parameters object
* @param {string} userId - User ID for access control * @param {Array<MongoFile>} params.files - Array of file documents
* @param {string} agentId - Agent ID that might grant access to files * @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<Array<MongoFile>>} Filtered array of accessible files * @returns {Promise<Array<MongoFile>>} 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) { if (!userId || !agentId || !files || files.length === 0) {
return files; return files;
} }
@ -96,7 +102,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => {
const ownedFiles = []; const ownedFiles = [];
for (const file of files) { for (const file of files) {
if (file.user && file.user.toString() === userId) { if (file.user && file.user.toString() === userId.toString()) {
ownedFiles.push(file); ownedFiles.push(file);
} else { } else {
filesToCheck.push(file); filesToCheck.push(file);
@ -109,7 +115,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => {
// Batch check access for all non-owned files // Batch check access for all non-owned files
const fileIds = filesToCheck.map((f) => f.file_id); 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 // Filter files based on access
const accessibleFiles = filesToCheck.filter((file) => accessMap.get(file.file_id)); const accessibleFiles = filesToCheck.filter((file) => accessMap.get(file.file_id));

View file

@ -126,12 +126,13 @@ const grantPermission = async ({
* Check if a user has specific permission bits on a resource * Check if a user has specific permission bits on a resource
* @param {Object} params - Parameters for checking permissions * @param {Object} params - Parameters for checking permissions
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user * @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} params.resourceType - Type of resource (e.g., 'agent')
* @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource * @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) * @param {number} params.requiredPermissions - The permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT)
* @returns {Promise<boolean>} Whether the user has the required permission bits * @returns {Promise<boolean>} Whether the user has the required permission bits
*/ */
const checkPermission = async ({ userId, resourceType, resourceId, requiredPermission }) => { const checkPermission = async ({ userId, role, resourceType, resourceId, requiredPermission }) => {
try { try {
if (typeof requiredPermission !== 'number' || requiredPermission < 1) { if (typeof requiredPermission !== 'number' || requiredPermission < 1) {
throw new Error('requiredPermission must be a positive number'); throw new Error('requiredPermission must be a positive number');
@ -140,7 +141,7 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi
validateResourceType(resourceType); validateResourceType(resourceType);
// Get all principals for the user (user + groups + public) // Get all principals for the user (user + groups + public)
const principals = await getUserPrincipals(userId); const principals = await getUserPrincipals({ userId, role });
if (principals.length === 0) { if (principals.length === 0) {
return false; return false;
@ -161,16 +162,17 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi
* Get effective permission bitmask for a user on a resource * Get effective permission bitmask for a user on a resource
* @param {Object} params - Parameters for getting effective permissions * @param {Object} params - Parameters for getting effective permissions
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user * @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} params.resourceType - Type of resource (e.g., 'agent')
* @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource * @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource
* @returns {Promise<number>} Effective permission bitmask * @returns {Promise<number>} Effective permission bitmask
*/ */
const getEffectivePermissions = async ({ userId, resourceType, resourceId }) => { const getEffectivePermissions = async ({ userId, role, resourceType, resourceId }) => {
try { try {
validateResourceType(resourceType); validateResourceType(resourceType);
// Get all principals for the user (user + groups + public) // Get all principals for the user (user + groups + public)
const principals = await getUserPrincipals(userId); const principals = await getUserPrincipals({ userId, role });
if (principals.length === 0) { if (principals.length === 0) {
return 0; return 0;
@ -186,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 * 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 {Object} params - Parameters for finding accessible resources
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user * @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} 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) * @param {number} params.requiredPermissions - The minimum permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT)
* @returns {Promise<Array>} Array of resource IDs * @returns {Promise<Array>} Array of resource IDs
*/ */
const findAccessibleResources = async ({ userId, resourceType, requiredPermissions }) => { const findAccessibleResources = async ({ userId, role, resourceType, requiredPermissions }) => {
try { try {
if (typeof requiredPermissions !== 'number' || requiredPermissions < 1) { if (typeof requiredPermissions !== 'number' || requiredPermissions < 1) {
throw new Error('requiredPermissions must be a positive number'); throw new Error('requiredPermissions must be a positive number');
@ -199,7 +202,7 @@ const findAccessibleResources = async ({ userId, resourceType, requiredPermissio
validateResourceType(resourceType); validateResourceType(resourceType);
// Get all principals for the user (user + groups + public) // Get all principals for the user (user + groups + public)
const principalsList = await getUserPrincipals(userId); const principalsList = await getUserPrincipals({ userId, role });
if (principalsList.length === 0) { if (principalsList.length === 0) {
return []; return [];

View file

@ -1067,6 +1067,164 @@ describe('PermissionService', () => {
expect(hasNoPermission).toBe(false); 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('should work with different resource types', async () => {
// Test with promptGroup resources // Test with promptGroup resources
const promptGroupResourceId = new mongoose.Types.ObjectId(); const promptGroupResourceId = new mongoose.Types.ObjectId();

View file

@ -53,7 +53,9 @@ describe('Role-based Permissions Integration', () => {
role: 'admin', role: 'admin',
}); });
const principals = await methods.getUserPrincipals(adminUser._id as mongoose.Types.ObjectId); const principals = await methods.getUserPrincipals({
userId: adminUser._id as mongoose.Types.ObjectId,
});
// Should have user, role, and public principals // Should have user, role, and public principals
expect(principals).toHaveLength(3); expect(principals).toHaveLength(3);
@ -84,9 +86,9 @@ describe('Role-based Permissions Integration', () => {
role: null, // Explicitly set to null to override default role: null, // Explicitly set to null to override default
}); });
const principals = await methods.getUserPrincipals( const principals = await methods.getUserPrincipals({
regularUser._id as mongoose.Types.ObjectId, userId: regularUser._id as mongoose.Types.ObjectId,
); });
// Should only have user and public principals // Should only have user and public principals
expect(principals).toHaveLength(2); expect(principals).toHaveLength(2);
@ -116,7 +118,9 @@ describe('Role-based Permissions Integration', () => {
memberIds: [(user._id as mongoose.Types.ObjectId).toString()], memberIds: [(user._id as mongoose.Types.ObjectId).toString()],
}); });
const principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); const principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
// Should have user, role, 2 groups, and public // Should have user, role, 2 groups, and public
expect(principals).toHaveLength(5); expect(principals).toHaveLength(5);
@ -156,7 +160,9 @@ describe('Role-based Permissions Integration', () => {
role: testCase.role, role: testCase.role,
}); });
const principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); const principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE);
expect(rolePrincipal).toBeDefined(); expect(rolePrincipal).toBeDefined();
@ -292,7 +298,9 @@ describe('Role-based Permissions Integration', () => {
}); });
// Initial principals // Initial principals
let principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); let principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
let rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); let rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE);
expect(rolePrincipal?.principalId).toBe('viewer'); expect(rolePrincipal?.principalId).toBe('viewer');
@ -301,7 +309,9 @@ describe('Role-based Permissions Integration', () => {
await user.save(); await user.save();
// Get principals again // Get principals again
principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE);
expect(rolePrincipal?.principalId).toBe('editor'); expect(rolePrincipal?.principalId).toBe('editor');
}); });
@ -315,7 +325,9 @@ describe('Role-based Permissions Integration', () => {
}); });
// Initial check // Initial check
let principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); let principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
expect(principals).toHaveLength(3); // user, role, public expect(principals).toHaveLength(3); // user, role, public
// Remove role // Remove role
@ -323,7 +335,9 @@ describe('Role-based Permissions Integration', () => {
await user.save(); await user.save();
// Check again // Check again
principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
expect(principals).toHaveLength(2); // user, public expect(principals).toHaveLength(2); // user, public
const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE);
expect(rolePrincipal).toBeUndefined(); expect(rolePrincipal).toBeUndefined();
@ -352,7 +366,9 @@ describe('Role-based Permissions Integration', () => {
const user = await User.create(userData); const user = await User.create(userData);
const principals = await methods.getUserPrincipals(user._id as mongoose.Types.ObjectId); const principals = await methods.getUserPrincipals({
userId: user._id as mongoose.Types.ObjectId,
});
const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE); const rolePrincipal = principals.find((p) => p.principalType === PrincipalType.ROLE);
if (testCase.expected) { if (testCase.expected) {

View file

@ -325,7 +325,9 @@ describe('User Group Methods Tests', () => {
); );
/** Get user principals */ /** 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, role (default USER), group, and public principals */ /** Should include user, role (default USER), group, and public principals */
expect(principals).toHaveLength(4); expect(principals).toHaveLength(4);
@ -349,7 +351,9 @@ describe('User Group Methods Tests', () => {
test('should return user and public principals for non-existent user in getUserPrincipals', async () => { test('should return user and public principals for non-existent user in getUserPrincipals', async () => {
const nonExistentId = new mongoose.Types.ObjectId(); 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 */ /** Should still return user and public principals even for non-existent user */
expect(principals).toHaveLength(2); expect(principals).toHaveLength(2);

View file

@ -237,35 +237,45 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
/** /**
* Get a list of all principal identifiers for a user (user ID + group IDs + public) * Get a list of all principal identifiers for a user (user ID + group IDs + public)
* For use in permission checks * 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 * @param session - Optional MongoDB session for transactions
* @returns Array of principal objects with type and id * @returns Array of principal objects with type and id
*/ */
async function getUserPrincipals( async function getUserPrincipals(
userId: string | Types.ObjectId, params: {
userId: string | Types.ObjectId;
role?: string | null;
},
session?: ClientSession, session?: ClientSession,
): Promise<Array<{ principalType: string; principalId?: string | Types.ObjectId }>> { ): Promise<Array<{ principalType: string; principalId?: string | Types.ObjectId }>> {
const { userId, role } = params;
const principals: Array<{ principalType: string; principalId?: string | Types.ObjectId }> = [ const principals: Array<{ principalType: string; principalId?: string | Types.ObjectId }> = [
{ principalType: PrincipalType.USER, principalId: userId }, { principalType: PrincipalType.USER, principalId: userId },
]; ];
// Get user to check their role // If role is not provided, query user to get it
let userRole = role;
if (userRole === undefined) {
const User = mongoose.models.User as Model<IUser>; const User = mongoose.models.User as Model<IUser>;
const query = User.findById(userId).select('role'); const query = User.findById(userId).select('role');
if (session) { if (session) {
query.session(session); query.session(session);
} }
const user = await query.lean(); const user = await query.lean();
userRole = user?.role;
}
// Add role as a principal if user has one // Add role as a principal if user has one
if (user?.role && user.role.trim()) { if (userRole && userRole.trim()) {
principals.push({ principalType: PrincipalType.ROLE, principalId: user.role }); principals.push({ principalType: PrincipalType.ROLE, principalId: userRole });
} }
const userGroups = await getUserGroups(userId, session); const userGroups = await getUserGroups(userId, session);
if (userGroups && userGroups.length > 0) { if (userGroups && userGroups.length > 0) {
userGroups.forEach((group) => { userGroups.forEach((group) => {
principals.push({ principalType: PrincipalType.GROUP, principalId: group._id.toString() }); principals.push({ principalType: PrincipalType.GROUP, principalId: group._id });
}); });
} }