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.
This commit is contained in:
Danny Avila 2025-07-14 20:24:40 -04:00
parent 4caac90909
commit 35c66b39c8
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
5 changed files with 512 additions and 270 deletions

View file

@ -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);
});
});
});

View file

@ -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([]);
}
}

View file

@ -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',
},
],
});