From 35c66b39c892ec37dfc37edd28bd6b5807a6dedd Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 14 Jul 2025 20:24:40 -0400 Subject: [PATCH] refactor: Implement permission checks for file access via agents - Updated `hasAccessToFilesViaAgent` to utilize permission checks for VIEW and EDIT access. - Replaced project-based access validation with permission-based checks. - Enhanced tests to cover new permission logic and ensure proper access control for files associated with agents. - Cleaned up imports and initialized models in test files for consistency. --- api/models/File.js | 39 ++- api/models/File.spec.js | 178 +++++++++--- api/server/routes/files/files.agents.test.js | 290 +++++++++++-------- api/server/routes/files/files.js | 19 +- api/server/routes/files/files.test.js | 256 +++++++++++----- 5 files changed, 512 insertions(+), 270 deletions(-) diff --git a/api/models/File.js b/api/models/File.js index 3d735434e3..1703e5bba0 100644 --- a/api/models/File.js +++ b/api/models/File.js @@ -1,6 +1,6 @@ const { logger } = require('@librechat/data-schemas'); -const { EToolResources, FileContext, Constants } = require('librechat-data-provider'); -const { getProjectByName } = require('./Project'); +const { EToolResources, FileContext, PERMISSION_BITS } = require('librechat-data-provider'); +const { checkPermission } = require('~/server/services/PermissionService'); const { getAgent } = require('./Agent'); const { File } = require('~/db/models'); @@ -40,26 +40,33 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId, checkCollabora return accessMap; } - // Check if agent is shared with the user via projects - if (!agent.projectIds || agent.projectIds.length === 0) { + // Check if user has at least VIEW permission on the agent + const hasViewPermission = await checkPermission({ + userId, + resourceType: 'agent', + resourceId: agent._id, + requiredPermission: PERMISSION_BITS.VIEW, + }); + + if (!hasViewPermission) { return accessMap; } - // Check if agent is in global project - const globalProject = await getProjectByName(Constants.GLOBAL_PROJECT_NAME, '_id'); - if ( - !globalProject || - !agent.projectIds.some((pid) => pid.toString() === globalProject._id.toString()) - ) { + // Check if user has EDIT permission (which would indicate collaborative access) + const hasEditPermission = await checkPermission({ + userId, + resourceType: 'agent', + resourceId: agent._id, + requiredPermission: PERMISSION_BITS.EDIT, + }); + + // If user only has VIEW permission, they can't access files + // Only users with EDIT permission or higher can access agent files + if (!hasEditPermission) { return accessMap; } - // Agent is globally shared - check if it's collaborative - if (checkCollaborative && !agent.isCollaborative) { - return accessMap; - } - - // Check which files are actually attached + // User has edit permissions - check which files are actually attached const attachedFileIds = new Set(); if (agent.tool_resources) { for (const [_resourceType, resource] of Object.entries(agent.tool_resources)) { diff --git a/api/models/File.spec.js b/api/models/File.spec.js index 9861571e33..5eb39ce3d9 100644 --- a/api/models/File.spec.js +++ b/api/models/File.spec.js @@ -1,17 +1,17 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); -const { fileSchema } = require('@librechat/data-schemas'); -const { agentSchema } = require('@librechat/data-schemas'); -const { projectSchema } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { GLOBAL_PROJECT_NAME } = require('librechat-data-provider').Constants; +const { createModels } = require('@librechat/data-schemas'); const { getFiles, createFile } = require('./File'); -const { getProjectByName } = require('./Project'); const { createAgent } = require('./Agent'); +const { grantPermission } = require('~/server/services/PermissionService'); +const { seedDefaultRoles } = require('~/models'); let File; let Agent; -let Project; +let AclEntry; +let User; +let AccessRole; describe('File Access Control', () => { let mongoServer; @@ -19,10 +19,23 @@ describe('File Access Control', () => { beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); const mongoUri = mongoServer.getUri(); - File = mongoose.models.File || mongoose.model('File', fileSchema); - Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema); - Project = mongoose.models.Project || mongoose.model('Project', projectSchema); await mongoose.connect(mongoUri); + + // Initialize all models + createModels(mongoose); + + // Register models on mongoose.models so methods can access them + const models = require('~/db/models'); + Object.assign(mongoose.models, models); + + File = models.File; + Agent = models.Agent; + AclEntry = models.AclEntry; + User = models.User; + AccessRole = models.AccessRole; + + // Seed default roles + await seedDefaultRoles(); }); afterAll(async () => { @@ -33,16 +46,32 @@ describe('File Access Control', () => { beforeEach(async () => { await File.deleteMany({}); await Agent.deleteMany({}); - await Project.deleteMany({}); + await AclEntry.deleteMany({}); + await User.deleteMany({}); }); describe('hasAccessToFilesViaAgent', () => { it('should efficiently check access for multiple files at once', async () => { - const userId = new mongoose.Types.ObjectId().toString(); - const authorId = new mongoose.Types.ObjectId().toString(); + const userId = new mongoose.Types.ObjectId(); + const authorId = new mongoose.Types.ObjectId(); const agentId = uuidv4(); const fileIds = [uuidv4(), uuidv4(), uuidv4(), uuidv4()]; + // Create users + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + }); + + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); + // Create files for (const fileId of fileIds) { await createFile({ @@ -54,13 +83,12 @@ describe('File Access Control', () => { } // Create agent with only first two files attached - await createAgent({ + const agent = await createAgent({ id: agentId, name: 'Test Agent', author: authorId, model: 'gpt-4', provider: 'openai', - isCollaborative: true, tool_resources: { file_search: { file_ids: [fileIds[0], fileIds[1]], @@ -68,15 +96,19 @@ describe('File Access Control', () => { }, }); - // Get or create global project - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, '_id'); - - // Share agent globally - await Agent.updateOne({ id: agentId }, { $push: { projectIds: globalProject._id } }); + // Grant EDIT permission to user on the agent + await grantPermission({ + principalType: 'user', + principalId: userId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); // Check access for all files const { hasAccessToFilesViaAgent } = require('./File'); - const accessMap = await hasAccessToFilesViaAgent(userId, fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent(userId.toString(), fileIds, agentId); // Should have access only to the first two files expect(accessMap.get(fileIds[0])).toBe(true); @@ -86,12 +118,20 @@ describe('File Access Control', () => { }); it('should grant access to all files when user is the agent author', async () => { - const authorId = new mongoose.Types.ObjectId().toString(); + const authorId = new mongoose.Types.ObjectId(); const agentId = uuidv4(); const fileIds = [uuidv4(), uuidv4(), uuidv4()]; + // Create author user + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); + // Create agent - await createAgent({ + const agent = await createAgent({ id: agentId, name: 'Test Agent', author: authorId, @@ -106,7 +146,7 @@ describe('File Access Control', () => { // Check access as the author const { hasAccessToFilesViaAgent } = require('./File'); - const accessMap = await hasAccessToFilesViaAgent(authorId, fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent(authorId.toString(), fileIds, agentId); // Author should have access to all files expect(accessMap.get(fileIds[0])).toBe(true); @@ -115,31 +155,57 @@ describe('File Access Control', () => { }); it('should handle non-existent agent gracefully', async () => { - const userId = new mongoose.Types.ObjectId().toString(); + const userId = new mongoose.Types.ObjectId(); const fileIds = [uuidv4(), uuidv4()]; + // Create user + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + }); + const { hasAccessToFilesViaAgent } = require('./File'); - const accessMap = await hasAccessToFilesViaAgent(userId, fileIds, 'non-existent-agent'); + const accessMap = await hasAccessToFilesViaAgent( + userId.toString(), + fileIds, + 'non-existent-agent', + ); // Should have no access to any files expect(accessMap.get(fileIds[0])).toBe(false); expect(accessMap.get(fileIds[1])).toBe(false); }); - it('should deny access when agent is not collaborative', async () => { - const userId = new mongoose.Types.ObjectId().toString(); - const authorId = new mongoose.Types.ObjectId().toString(); + it('should deny access when user only has VIEW permission', async () => { + const userId = new mongoose.Types.ObjectId(); + const authorId = new mongoose.Types.ObjectId(); const agentId = uuidv4(); const fileIds = [uuidv4(), uuidv4()]; - // Create agent with files but isCollaborative: false - await createAgent({ + // Create users + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + }); + + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); + + // Create agent with files + const agent = await createAgent({ id: agentId, - name: 'Non-Collaborative Agent', + name: 'View-Only Agent', author: authorId, model: 'gpt-4', provider: 'openai', - isCollaborative: false, tool_resources: { file_search: { file_ids: fileIds, @@ -147,17 +213,21 @@ describe('File Access Control', () => { }, }); - // Get or create global project - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, '_id'); - - // Share agent globally - await Agent.updateOne({ id: agentId }, { $push: { projectIds: globalProject._id } }); + // Grant only VIEW permission to user on the agent + await grantPermission({ + principalType: 'user', + principalId: userId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_viewer', + grantedBy: authorId, + }); // Check access for files const { hasAccessToFilesViaAgent } = require('./File'); - const accessMap = await hasAccessToFilesViaAgent(userId, fileIds, agentId); + const accessMap = await hasAccessToFilesViaAgent(userId.toString(), fileIds, agentId); - // Should have no access to any files when isCollaborative is false + // Should have no access to any files when only VIEW permission expect(accessMap.get(fileIds[0])).toBe(false); expect(accessMap.get(fileIds[1])).toBe(false); }); @@ -172,18 +242,28 @@ describe('File Access Control', () => { const sharedFileId = `file_${uuidv4()}`; const inaccessibleFileId = `file_${uuidv4()}`; - // Create/get global project using getProjectByName which will upsert - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME); + // Create users + await User.create({ + _id: userId, + email: 'user@example.com', + emailVerified: true, + provider: 'local', + }); + + await User.create({ + _id: authorId, + email: 'author@example.com', + emailVerified: true, + provider: 'local', + }); // Create agent with shared file - await createAgent({ + const agent = await createAgent({ id: agentId, name: 'Shared Agent', provider: 'test', model: 'test-model', author: authorId, - projectIds: [globalProject._id], - isCollaborative: true, tool_resources: { file_search: { file_ids: [sharedFileId], @@ -191,6 +271,16 @@ describe('File Access Control', () => { }, }); + // Grant EDIT permission to user on the agent + await grantPermission({ + principalType: 'user', + principalId: userId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); + // Create files await createFile({ file_id: ownedFileId, diff --git a/api/server/routes/files/files.agents.test.js b/api/server/routes/files/files.agents.test.js index a669559982..4e6c5ca457 100644 --- a/api/server/routes/files/files.agents.test.js +++ b/api/server/routes/files/files.agents.test.js @@ -2,10 +2,12 @@ const express = require('express'); const request = require('supertest'); const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); +const { createMethods } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { GLOBAL_PROJECT_NAME } = require('librechat-data-provider').Constants; +const { createAgent } = require('~/models/Agent'); +const { createFile } = require('~/models/File'); -// Mock dependencies +// Only mock the external dependencies that we don't want to test jest.mock('~/server/services/Files/process', () => ({ processDeleteRequest: jest.fn().mockResolvedValue({}), filterFile: jest.fn(), @@ -25,31 +27,8 @@ jest.mock('~/server/services/Tools/credentials', () => ({ loadAuthValues: jest.fn(), })); -jest.mock('~/server/services/Files/S3/crud', () => ({ - refreshS3FileUrls: jest.fn(), -})); - -jest.mock('~/cache', () => ({ - getLogStores: jest.fn(() => ({ - get: jest.fn(), - set: jest.fn(), - })), -})); - -jest.mock('~/config', () => ({ - logger: { - error: jest.fn(), - warn: jest.fn(), - debug: jest.fn(), - }, -})); - -const { createFile } = require('~/models/File'); -const { createAgent } = require('~/models/Agent'); -const { getProjectByName } = require('~/models/Project'); - -// Import the router after mocks -const router = require('./files'); +// Import the router +const router = require('~/server/routes/files/files'); describe('File Routes - Agent Files Endpoint', () => { let app; @@ -60,13 +39,38 @@ describe('File Routes - Agent Files Endpoint', () => { let fileId1; let fileId2; let fileId3; + let File; + let User; + let Agent; + let methods; + let AclEntry; + // eslint-disable-next-line no-unused-vars + let AccessRole; beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); - await mongoose.connect(mongoServer.getUri()); + const mongoUri = mongoServer.getUri(); + await mongoose.connect(mongoUri); - // Initialize models - require('~/db/models'); + // Initialize all models using createModels + const { createModels } = require('@librechat/data-schemas'); + const models = createModels(mongoose); + + // Register models on mongoose.models so methods can access them + Object.assign(mongoose.models, models); + + // Create methods with our test mongoose instance + methods = createMethods(mongoose); + + // Now we can access models from the db/models + File = models.File; + Agent = models.Agent; + AclEntry = models.AclEntry; + User = models.User; + AccessRole = models.AccessRole; + + // Seed default roles using our methods + await methods.seedDefaultRoles(); app = express(); app.use(express.json()); @@ -87,83 +91,101 @@ describe('File Routes - Agent Files Endpoint', () => { }); beforeEach(async () => { - jest.clearAllMocks(); + await File.deleteMany({}); + await Agent.deleteMany({}); + await User.deleteMany({}); + await AclEntry.deleteMany({}); - // Clear database - const collections = mongoose.connection.collections; - for (const key in collections) { - await collections[key].deleteMany({}); - } - - authorId = new mongoose.Types.ObjectId().toString(); - otherUserId = new mongoose.Types.ObjectId().toString(); + // Create test users + authorId = new mongoose.Types.ObjectId(); + otherUserId = new mongoose.Types.ObjectId(); agentId = uuidv4(); fileId1 = uuidv4(); fileId2 = uuidv4(); fileId3 = uuidv4(); + // Create users in database + await User.create({ + _id: authorId, + username: 'author', + email: 'author@test.com', + }); + + await User.create({ + _id: otherUserId, + username: 'other', + email: 'other@test.com', + }); + // Create files await createFile({ user: authorId, file_id: fileId1, - filename: 'agent-file1.txt', - filepath: `/uploads/${authorId}/${fileId1}`, - bytes: 1024, + filename: 'file1.txt', + filepath: '/uploads/file1.txt', + bytes: 100, type: 'text/plain', }); await createFile({ user: authorId, file_id: fileId2, - filename: 'agent-file2.txt', - filepath: `/uploads/${authorId}/${fileId2}`, - bytes: 2048, + filename: 'file2.txt', + filepath: '/uploads/file2.txt', + bytes: 200, type: 'text/plain', }); await createFile({ user: otherUserId, file_id: fileId3, - filename: 'user-file.txt', - filepath: `/uploads/${otherUserId}/${fileId3}`, - bytes: 512, + filename: 'file3.txt', + filepath: '/uploads/file3.txt', + bytes: 300, type: 'text/plain', }); - - // Create an agent with files attached - await createAgent({ - id: agentId, - name: 'Test Agent', - author: authorId, - model: 'gpt-4', - provider: 'openai', - isCollaborative: true, - tool_resources: { - file_search: { - file_ids: [fileId1, fileId2], - }, - }, - }); - - // Share the agent globally - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, '_id'); - if (globalProject) { - const { updateAgent } = require('~/models/Agent'); - await updateAgent({ id: agentId }, { projectIds: [globalProject._id] }); - } }); describe('GET /files/agent/:agent_id', () => { - it('should return files accessible through the agent for non-author', async () => { + it('should return files accessible through the agent for non-author with EDIT permission', async () => { + // Create an agent with files attached + const agent = await createAgent({ + id: agentId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId1, fileId2], + }, + }, + }); + + // Grant EDIT permission to user on the agent using PermissionService + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); + + // Mock req.user for this request + app.use((req, res, next) => { + req.user = { id: otherUserId.toString() }; + next(); + }); + const response = await request(app).get(`/files/agent/${agentId}`); expect(response.status).toBe(200); - expect(response.body).toHaveLength(2); // Only agent files, not user-owned files - - const fileIds = response.body.map((f) => f.file_id); - expect(fileIds).toContain(fileId1); - expect(fileIds).toContain(fileId2); - expect(fileIds).not.toContain(fileId3); // User's own file not included + expect(Array.isArray(response.body)).toBe(true); + expect(response.body).toHaveLength(2); + expect(response.body.map((f) => f.file_id)).toContain(fileId1); + expect(response.body.map((f) => f.file_id)).toContain(fileId2); }); it('should return 400 when agent_id is not provided', async () => { @@ -176,45 +198,63 @@ describe('File Routes - Agent Files Endpoint', () => { const response = await request(app).get('/files/agent/non-existent-agent'); expect(response.status).toBe(200); - expect(response.body).toEqual([]); // Empty array for non-existent agent + expect(Array.isArray(response.body)).toBe(true); + expect(response.body).toEqual([]); }); - it('should return empty array when agent is not collaborative', async () => { - // Create a non-collaborative agent - const nonCollabAgentId = uuidv4(); - await createAgent({ - id: nonCollabAgentId, - name: 'Non-Collaborative Agent', - author: authorId, - model: 'gpt-4', + it('should return empty array when user only has VIEW permission', async () => { + // Create an agent with files attached + const agent = await createAgent({ + id: agentId, + name: 'Test Agent', provider: 'openai', - isCollaborative: false, + model: 'gpt-4', + author: authorId, tool_resources: { file_search: { - file_ids: [fileId1], + file_ids: [fileId1, fileId2], }, }, }); - // Share it globally - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, '_id'); - if (globalProject) { - const { updateAgent } = require('~/models/Agent'); - await updateAgent({ id: nonCollabAgentId }, { projectIds: [globalProject._id] }); - } + // Grant only VIEW permission to user on the agent + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_viewer', + grantedBy: authorId, + }); - const response = await request(app).get(`/files/agent/${nonCollabAgentId}`); + const response = await request(app).get(`/files/agent/${agentId}`); expect(response.status).toBe(200); - expect(response.body).toEqual([]); // Empty array when not collaborative + expect(Array.isArray(response.body)).toBe(true); + expect(response.body).toEqual([]); }); it('should return agent files for agent author', async () => { + // Create an agent with files attached + await createAgent({ + id: agentId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId1, fileId2], + }, + }, + }); + // Create a new app instance with author authentication const authorApp = express(); authorApp.use(express.json()); authorApp.use((req, res, next) => { - req.user = { id: authorId }; + req.user = { id: authorId.toString() }; req.app = { locals: {} }; next(); }); @@ -223,46 +263,48 @@ describe('File Routes - Agent Files Endpoint', () => { const response = await request(authorApp).get(`/files/agent/${agentId}`); expect(response.status).toBe(200); - expect(response.body).toHaveLength(2); // Agent files for author - - const fileIds = response.body.map((f) => f.file_id); - expect(fileIds).toContain(fileId1); - expect(fileIds).toContain(fileId2); - expect(fileIds).not.toContain(fileId3); // User's own file not included + expect(Array.isArray(response.body)).toBe(true); + expect(response.body).toHaveLength(2); }); it('should return files uploaded by other users to shared agent for author', async () => { - // Create a file uploaded by another user + const anotherUserId = new mongoose.Types.ObjectId(); const otherUserFileId = uuidv4(); - const anotherUserId = new mongoose.Types.ObjectId().toString(); + + await User.create({ + _id: anotherUserId, + username: 'another', + email: 'another@test.com', + }); await createFile({ user: anotherUserId, file_id: otherUserFileId, filename: 'other-user-file.txt', - filepath: `/uploads/${anotherUserId}/${otherUserFileId}`, - bytes: 4096, + filepath: '/uploads/other-user-file.txt', + bytes: 400, type: 'text/plain', }); - // Update agent to include the file uploaded by another user - const { updateAgent } = require('~/models/Agent'); - await updateAgent( - { id: agentId }, - { - tool_resources: { - file_search: { - file_ids: [fileId1, fileId2, otherUserFileId], - }, + // Create agent to include the file uploaded by another user + await createAgent({ + id: agentId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId1, otherUserFileId], }, }, - ); + }); - // Create app instance with author authentication + // Create a new app instance with author authentication const authorApp = express(); authorApp.use(express.json()); authorApp.use((req, res, next) => { - req.user = { id: authorId }; + req.user = { id: authorId.toString() }; req.app = { locals: {} }; next(); }); @@ -271,12 +313,10 @@ describe('File Routes - Agent Files Endpoint', () => { const response = await request(authorApp).get(`/files/agent/${agentId}`); expect(response.status).toBe(200); - expect(response.body).toHaveLength(3); // Including file from another user - - const fileIds = response.body.map((f) => f.file_id); - expect(fileIds).toContain(fileId1); - expect(fileIds).toContain(fileId2); - expect(fileIds).toContain(otherUserFileId); // File uploaded by another user + expect(Array.isArray(response.body)).toBe(true); + expect(response.body).toHaveLength(2); + expect(response.body.map((f) => f.file_id)).toContain(fileId1); + expect(response.body.map((f) => f.file_id)).toContain(otherUserFileId); }); }); }); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index e0b354ef6e..5b92abcbe9 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -5,8 +5,8 @@ const { Time, isUUID, CacheKeys, - Constants, FileSources, + PERMISSION_BITS, EModelEndpoint, isAgentsEndpoint, checkOpenAIStorage, @@ -20,9 +20,9 @@ const { const { getFiles, batchUpdateFiles, hasAccessToFilesViaAgent } = require('~/models/File'); const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { getOpenAIClient } = require('~/server/controllers/assistants/helpers'); +const { checkPermission } = require('~/server/services/PermissionService'); const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { refreshS3FileUrls } = require('~/server/services/Files/S3/crud'); -const { getProjectByName } = require('~/models/Project'); const { getAssistant } = require('~/models/Assistant'); const { getAgent } = require('~/models/Agent'); const { getLogStores } = require('~/cache'); @@ -77,14 +77,15 @@ router.get('/agent/:agent_id', async (req, res) => { // Check if user has access to the agent if (agent.author.toString() !== userId) { - // Non-authors need the agent to be globally shared and collaborative - const globalProject = await getProjectByName(Constants.GLOBAL_PROJECT_NAME, '_id'); + // Non-authors need at least EDIT permission to view agent files + const hasEditPermission = await checkPermission({ + userId, + resourceType: 'agent', + resourceId: agent._id, + requiredPermission: PERMISSION_BITS.EDIT, + }); - if ( - !globalProject || - !agent.projectIds.some((pid) => pid.toString() === globalProject._id.toString()) || - !agent.isCollaborative - ) { + if (!hasEditPermission) { return res.status(200).json([]); } } diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index 3f5ae9fc41..59aefa0d11 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -2,10 +2,12 @@ const express = require('express'); const request = require('supertest'); const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); +const { createMethods } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { GLOBAL_PROJECT_NAME } = require('librechat-data-provider').Constants; +const { createAgent } = require('~/models/Agent'); +const { createFile } = require('~/models/File'); -// Mock dependencies +// Only mock the external dependencies that we don't want to test jest.mock('~/server/services/Files/process', () => ({ processDeleteRequest: jest.fn().mockResolvedValue({}), filterFile: jest.fn(), @@ -44,9 +46,6 @@ jest.mock('~/config', () => ({ }, })); -const { createFile } = require('~/models/File'); -const { createAgent } = require('~/models/Agent'); -const { getProjectByName } = require('~/models/Project'); const { processDeleteRequest } = require('~/server/services/Files/process'); // Import the router after mocks @@ -57,22 +56,48 @@ describe('File Routes - Delete with Agent Access', () => { let mongoServer; let authorId; let otherUserId; - let agentId; let fileId; + let File; + let Agent; + let AclEntry; + let User; + let methods; + // eslint-disable-next-line no-unused-vars + let agentId; + // eslint-disable-next-line no-unused-vars + let AccessRole; beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); - await mongoose.connect(mongoServer.getUri()); + const mongoUri = mongoServer.getUri(); + await mongoose.connect(mongoUri); - // Initialize models - require('~/db/models'); + // Initialize all models using createModels + const { createModels } = require('@librechat/data-schemas'); + const models = createModels(mongoose); + + // Register models on mongoose.models so methods can access them + Object.assign(mongoose.models, models); + + // Create methods with our test mongoose instance + methods = createMethods(mongoose); + + // Now we can access models from the db/models + File = models.File; + Agent = models.Agent; + AclEntry = models.AclEntry; + User = models.User; + AccessRole = models.AccessRole; + + // Seed default roles using our methods + await methods.seedDefaultRoles(); app = express(); app.use(express.json()); // Mock authentication middleware app.use((req, res, next) => { - req.user = { id: otherUserId || 'default-user' }; + req.user = { id: otherUserId ? otherUserId.toString() : 'default-user' }; req.app = { locals: {} }; next(); }); @@ -89,47 +114,39 @@ describe('File Routes - Delete with Agent Access', () => { jest.clearAllMocks(); // Clear database - const collections = mongoose.connection.collections; - for (const key in collections) { - await collections[key].deleteMany({}); - } + await File.deleteMany({}); + await Agent.deleteMany({}); + await User.deleteMany({}); + await AclEntry.deleteMany({}); - authorId = new mongoose.Types.ObjectId().toString(); - otherUserId = new mongoose.Types.ObjectId().toString(); + // Create test data + authorId = new mongoose.Types.ObjectId(); + otherUserId = new mongoose.Types.ObjectId(); + agentId = uuidv4(); fileId = uuidv4(); + // Create users in database + await User.create({ + _id: authorId, + username: 'author', + email: 'author@test.com', + }); + + await User.create({ + _id: otherUserId, + username: 'other', + email: 'other@test.com', + }); + // Create a file owned by the author await createFile({ user: authorId, file_id: fileId, filename: 'test.txt', - filepath: `/uploads/${authorId}/${fileId}`, - bytes: 1024, + filepath: '/uploads/test.txt', + bytes: 100, type: 'text/plain', }); - - // Create an agent with the file attached - const agent = await createAgent({ - id: uuidv4(), - name: 'Test Agent', - author: authorId, - model: 'gpt-4', - provider: 'openai', - isCollaborative: true, - tool_resources: { - file_search: { - file_ids: [fileId], - }, - }, - }); - agentId = agent.id; - - // Share the agent globally - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, '_id'); - if (globalProject) { - const { updateAgent } = require('~/models/Agent'); - await updateAgent({ id: agentId }, { projectIds: [globalProject._id] }); - } }); describe('DELETE /files', () => { @@ -140,8 +157,8 @@ describe('File Routes - Delete with Agent Access', () => { user: otherUserId, file_id: userFileId, filename: 'user-file.txt', - filepath: `/uploads/${otherUserId}/${userFileId}`, - bytes: 1024, + filepath: '/uploads/user-file.txt', + bytes: 200, type: 'text/plain', }); @@ -151,7 +168,7 @@ describe('File Routes - Delete with Agent Access', () => { files: [ { file_id: userFileId, - filepath: `/uploads/${otherUserId}/${userFileId}`, + filepath: '/uploads/user-file.txt', }, ], }); @@ -168,7 +185,7 @@ describe('File Routes - Delete with Agent Access', () => { files: [ { file_id: fileId, - filepath: `/uploads/${authorId}/${fileId}`, + filepath: '/uploads/test.txt', }, ], }); @@ -180,14 +197,39 @@ describe('File Routes - Delete with Agent Access', () => { }); it('should allow deleting files accessible through shared agent', async () => { + // Create an agent with the file attached + const agent = await createAgent({ + id: uuidv4(), + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId], + }, + }, + }); + + // Grant EDIT permission to user on the agent + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); + const response = await request(app) .delete('/files') .send({ - agent_id: agentId, + agent_id: agent.id, files: [ { file_id: fileId, - filepath: `/uploads/${authorId}/${fileId}`, + filepath: '/uploads/test.txt', }, ], }); @@ -204,19 +246,44 @@ describe('File Routes - Delete with Agent Access', () => { user: authorId, file_id: unattachedFileId, filename: 'unattached.txt', - filepath: `/uploads/${authorId}/${unattachedFileId}`, - bytes: 1024, + filepath: '/uploads/unattached.txt', + bytes: 300, type: 'text/plain', }); + // Create an agent without the unattached file + const agent = await createAgent({ + id: uuidv4(), + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId], // Only fileId, not unattachedFileId + }, + }, + }); + + // Grant EDIT permission to user on the agent + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); + const response = await request(app) .delete('/files') .send({ - agent_id: agentId, + agent_id: agent.id, files: [ { file_id: unattachedFileId, - filepath: `/uploads/${authorId}/${unattachedFileId}`, + filepath: '/uploads/unattached.txt', }, ], }); @@ -224,6 +291,7 @@ describe('File Routes - Delete with Agent Access', () => { expect(response.status).toBe(403); expect(response.body.message).toBe('You can only delete files you have access to'); expect(response.body.unauthorizedFiles).toContain(unattachedFileId); + expect(processDeleteRequest).not.toHaveBeenCalled(); }); it('should handle mixed authorized and unauthorized files', async () => { @@ -233,8 +301,8 @@ describe('File Routes - Delete with Agent Access', () => { user: otherUserId, file_id: userFileId, filename: 'user-file.txt', - filepath: `/uploads/${otherUserId}/${userFileId}`, - bytes: 1024, + filepath: '/uploads/user-file.txt', + bytes: 200, type: 'text/plain', }); @@ -244,51 +312,87 @@ describe('File Routes - Delete with Agent Access', () => { user: authorId, file_id: unauthorizedFileId, filename: 'unauthorized.txt', - filepath: `/uploads/${authorId}/${unauthorizedFileId}`, - bytes: 1024, + filepath: '/uploads/unauthorized.txt', + bytes: 400, type: 'text/plain', }); + // Create an agent with only fileId attached + const agent = await createAgent({ + id: uuidv4(), + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId], + }, + }, + }); + + // Grant EDIT permission to user on the agent + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_editor', + grantedBy: authorId, + }); + const response = await request(app) .delete('/files') .send({ - agent_id: agentId, + agent_id: agent.id, files: [ - { - file_id: fileId, // Authorized through agent - filepath: `/uploads/${authorId}/${fileId}`, - }, - { - file_id: userFileId, // Owned by user - filepath: `/uploads/${otherUserId}/${userFileId}`, - }, - { - file_id: unauthorizedFileId, // Not authorized - filepath: `/uploads/${authorId}/${unauthorizedFileId}`, - }, + { file_id: userFileId, filepath: '/uploads/user-file.txt' }, + { file_id: fileId, filepath: '/uploads/test.txt' }, + { file_id: unauthorizedFileId, filepath: '/uploads/unauthorized.txt' }, ], }); expect(response.status).toBe(403); expect(response.body.message).toBe('You can only delete files you have access to'); expect(response.body.unauthorizedFiles).toContain(unauthorizedFileId); - expect(response.body.unauthorizedFiles).not.toContain(fileId); - expect(response.body.unauthorizedFiles).not.toContain(userFileId); + expect(processDeleteRequest).not.toHaveBeenCalled(); }); - it('should prevent deleting files when agent is not collaborative', async () => { - // Update the agent to be non-collaborative - const { updateAgent } = require('~/models/Agent'); - await updateAgent({ id: agentId }, { isCollaborative: false }); + it('should prevent deleting files when user lacks EDIT permission on agent', async () => { + // Create an agent with the file attached + const agent = await createAgent({ + id: uuidv4(), + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + tool_resources: { + file_search: { + file_ids: [fileId], + }, + }, + }); + + // Grant only VIEW permission to user on the agent + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: 'user', + principalId: otherUserId, + resourceType: 'agent', + resourceId: agent._id, + accessRoleId: 'agent_viewer', + grantedBy: authorId, + }); const response = await request(app) .delete('/files') .send({ - agent_id: agentId, + agent_id: agent.id, files: [ { file_id: fileId, - filepath: `/uploads/${authorId}/${fileId}`, + filepath: '/uploads/test.txt', }, ], });