mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 17:44:11 +00:00
🪓 feat: Add Tenant-Scoped Agent Deletion (#15573)
* feat: add Agent Management delete endpoint * fix: scope agent deletion cleanup by tenant
This commit is contained in:
parent
6510908329
commit
5840e2f2ae
9 changed files with 485 additions and 17 deletions
|
|
@ -104,6 +104,7 @@ const {
|
|||
CONTENT_TRAVERSAL_MAX_DEPTH,
|
||||
createAgentManagementAuth,
|
||||
createAgentManagementCreateHandler,
|
||||
createAgentManagementDeleteHandler,
|
||||
createAgentManagementReadHandlers,
|
||||
createAgentManagementUpdateHandler,
|
||||
mergeDeploymentSkillIds,
|
||||
|
|
@ -436,12 +437,17 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
getRoleByName,
|
||||
createAgent: createAgentHandler,
|
||||
});
|
||||
const checkEditPermission = async ({ userId: accessibleUserId, resourceType, resourceId }) =>
|
||||
const checkAgentPermission = async ({
|
||||
userId: accessibleUserId,
|
||||
resourceType,
|
||||
resourceId,
|
||||
requiredPermission,
|
||||
}) =>
|
||||
(await AclEntry.exists({
|
||||
principalId: accessibleUserId,
|
||||
resourceType,
|
||||
resourceId,
|
||||
permBits: { $bitsAllSet: PermissionBits.EDIT },
|
||||
permBits: { $bitsAllSet: requiredPermission },
|
||||
})) != null;
|
||||
const hasCapability = jest.fn().mockResolvedValue(false);
|
||||
const reads = createAgentManagementReadHandlers({
|
||||
|
|
@ -454,16 +460,23 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
resourceType,
|
||||
permBits: { $bitsAllSet: PermissionBits.EDIT },
|
||||
}),
|
||||
checkPermission: checkEditPermission,
|
||||
checkPermission: checkAgentPermission,
|
||||
hasCapability,
|
||||
});
|
||||
const update = createAgentManagementUpdateHandler({
|
||||
getRoleByName,
|
||||
getAgentWithVersionCount: db.getAgentWithVersionCount,
|
||||
checkPermission: checkEditPermission,
|
||||
checkPermission: checkAgentPermission,
|
||||
hasCapability,
|
||||
updateAgent: updateAgentHandler,
|
||||
});
|
||||
const remove = createAgentManagementDeleteHandler({
|
||||
getRoleByName,
|
||||
getAgentWithVersionCount: db.getAgentWithVersionCount,
|
||||
checkPermission: checkAgentPermission,
|
||||
hasCapability,
|
||||
deleteAgent: db.deleteAgent,
|
||||
});
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use('/api/agents/v1/agents', auth);
|
||||
|
|
@ -477,6 +490,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
req.config = {};
|
||||
return update(req, res);
|
||||
});
|
||||
app.delete('/api/agents/v1/agents/:id', remove);
|
||||
|
||||
const createdResponse = await request(app)
|
||||
.post('/api/agents/v1/agents')
|
||||
|
|
@ -517,14 +531,47 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(listedResponse.body.data).toEqual([createdResponse.body]);
|
||||
|
||||
const otherTenantId = `tenant-${nanoid(8)}`;
|
||||
await tenantStorage.run({ tenantId: otherTenantId }, () =>
|
||||
Agent.create({
|
||||
id: createdResponse.body.id,
|
||||
name: 'Other tenant Agent',
|
||||
const currentTenantGraphId = `agent_${nanoid()}`;
|
||||
const otherTenantGraphId = `agent_${nanoid()}`;
|
||||
await tenantStorage.run({ tenantId }, async () => {
|
||||
await Agent.create({
|
||||
id: currentTenantGraphId,
|
||||
name: 'Current tenant graph',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: new mongoose.Types.ObjectId(),
|
||||
}),
|
||||
author: principal._id,
|
||||
edges: [{ from: '', to: createdResponse.body.id, edgeType: 'handoff' }],
|
||||
});
|
||||
await User.updateOne(
|
||||
{ _id: principal._id },
|
||||
{ $set: { favorites: [{ agentId: createdResponse.body.id }] } },
|
||||
);
|
||||
});
|
||||
const otherTenantPrincipal = await tenantStorage.run(
|
||||
{ tenantId: otherTenantId },
|
||||
async () => {
|
||||
const owner = await createOwner();
|
||||
await Agent.create({
|
||||
id: createdResponse.body.id,
|
||||
name: 'Other tenant Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: owner._id,
|
||||
});
|
||||
await Agent.create({
|
||||
id: otherTenantGraphId,
|
||||
name: 'Other tenant graph',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: owner._id,
|
||||
edges: [{ from: '', to: createdResponse.body.id, edgeType: 'handoff' }],
|
||||
});
|
||||
await User.updateOne(
|
||||
{ _id: owner._id },
|
||||
{ $set: { favorites: [{ agentId: createdResponse.body.id }] } },
|
||||
);
|
||||
return owner;
|
||||
},
|
||||
);
|
||||
|
||||
const updatedResponse = await request(app)
|
||||
|
|
@ -580,6 +627,55 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
resourceType: ResourceType.AGENT,
|
||||
}),
|
||||
);
|
||||
|
||||
const deletedResponse = await request(app)
|
||||
.delete(`/api/agents/v1/agents/${createdResponse.body.id}`)
|
||||
.set('Authorization', 'Bearer valid-token');
|
||||
expect(deletedResponse.status).toBe(200);
|
||||
expect(deletedResponse.body).toEqual({ id: createdResponse.body.id, deleted: true });
|
||||
|
||||
const [retrievedAfterDelete, repeatedDelete] = await Promise.all([
|
||||
request(app)
|
||||
.get(`/api/agents/v1/agents/${createdResponse.body.id}`)
|
||||
.set('Authorization', 'Bearer valid-token'),
|
||||
request(app)
|
||||
.delete(`/api/agents/v1/agents/${createdResponse.body.id}`)
|
||||
.set('Authorization', 'Bearer valid-token'),
|
||||
]);
|
||||
expect(retrievedAfterDelete.status).toBe(404);
|
||||
expect(repeatedDelete.status).toBe(404);
|
||||
await expect(Agent.exists({ id: createdResponse.body.id, tenantId })).resolves.toBeNull();
|
||||
await expect(
|
||||
Agent.exists({ id: createdResponse.body.id, tenantId: otherTenantId }),
|
||||
).resolves.not.toBeNull();
|
||||
await expect(
|
||||
Agent.exists({
|
||||
id: currentTenantGraphId,
|
||||
tenantId,
|
||||
'edges.to': createdResponse.body.id,
|
||||
}),
|
||||
).resolves.toBeNull();
|
||||
await expect(
|
||||
Agent.exists({
|
||||
id: otherTenantGraphId,
|
||||
tenantId: otherTenantId,
|
||||
'edges.to': createdResponse.body.id,
|
||||
}),
|
||||
).resolves.not.toBeNull();
|
||||
await expect(
|
||||
User.exists({
|
||||
_id: principal._id,
|
||||
tenantId,
|
||||
'favorites.agentId': createdResponse.body.id,
|
||||
}),
|
||||
).resolves.toBeNull();
|
||||
await expect(
|
||||
User.exists({
|
||||
_id: otherTenantPrincipal._id,
|
||||
tenantId: otherTenantId,
|
||||
'favorites.agentId': createdResponse.body.id,
|
||||
}),
|
||||
).resolves.not.toBeNull();
|
||||
});
|
||||
|
||||
test('management creation rejects caller-controlled ownership', async () => {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,14 @@ const mockCreateAgentManagementUpdateHandler = jest.fn((deps) => {
|
|||
mockUpdateDeps = deps;
|
||||
return mockUpdate;
|
||||
});
|
||||
const mockDelete = jest.fn((_req, res) =>
|
||||
res.status(200).json({ id: 'agent-deleted', deleted: true }),
|
||||
);
|
||||
let mockDeleteDeps;
|
||||
const mockCreateAgentManagementDeleteHandler = jest.fn((deps) => {
|
||||
mockDeleteDeps = deps;
|
||||
return mockDelete;
|
||||
});
|
||||
const mockBrowserCreate = jest.fn();
|
||||
const mockBrowserUpdate = jest.fn();
|
||||
const mockCheckBan = jest.fn((_req, _res, next) => next());
|
||||
|
|
@ -43,6 +51,7 @@ jest.mock('../middleware', () => ({
|
|||
jest.mock('@librechat/api', () => ({
|
||||
mapAgentManagementError: mockMapAgentManagementError,
|
||||
createAgentManagementCreateHandler: mockCreateAgentManagementCreateHandler,
|
||||
createAgentManagementDeleteHandler: mockCreateAgentManagementDeleteHandler,
|
||||
createAgentManagementReadHandlers: mockCreateAgentManagementReadHandlers,
|
||||
createAgentManagementUpdateHandler: mockCreateAgentManagementUpdateHandler,
|
||||
}));
|
||||
|
|
@ -64,6 +73,7 @@ jest.mock('~/models', () => ({
|
|||
getRoleByName: jest.fn(),
|
||||
getAgentWithVersionCount: jest.fn(),
|
||||
getAgentManagementListByAccess: jest.fn(),
|
||||
deleteAgent: jest.fn(),
|
||||
}));
|
||||
|
||||
const router = require('../management');
|
||||
|
|
@ -149,4 +159,19 @@ describe('Agent Management route boundary', () => {
|
|||
expect(mockUpdateDeps.hasCapability).toEqual(expect.any(Function));
|
||||
expect(mockUpdateDeps.updateAgent).toBe(mockBrowserUpdate);
|
||||
});
|
||||
|
||||
it('dispatches authenticated deletion requests with the shared Agent dependencies', async () => {
|
||||
const response = await request(app)
|
||||
.delete('/api/agents/v1/agents/agent-deleted')
|
||||
.set('Authorization', 'Bearer valid-token');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual({ id: 'agent-deleted', deleted: true });
|
||||
expect(mockDelete).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteDeps.getRoleByName).toEqual(expect.any(Function));
|
||||
expect(mockDeleteDeps.getAgentWithVersionCount).toEqual(expect.any(Function));
|
||||
expect(mockDeleteDeps.checkPermission).toEqual(expect.any(Function));
|
||||
expect(mockDeleteDeps.hasCapability).toEqual(expect.any(Function));
|
||||
expect(mockDeleteDeps.deleteAgent).toEqual(expect.any(Function));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
const express = require('express');
|
||||
const {
|
||||
createAgentManagementCreateHandler,
|
||||
createAgentManagementDeleteHandler,
|
||||
createAgentManagementReadHandlers,
|
||||
createAgentManagementUpdateHandler,
|
||||
mapAgentManagementError,
|
||||
|
|
@ -32,6 +33,13 @@ const updateHandler = createAgentManagementUpdateHandler({
|
|||
hasCapability,
|
||||
updateAgent: v1.updateAgent,
|
||||
});
|
||||
const deleteHandler = createAgentManagementDeleteHandler({
|
||||
getRoleByName: db.getRoleByName,
|
||||
getAgentWithVersionCount: db.getAgentWithVersionCount,
|
||||
checkPermission,
|
||||
hasCapability,
|
||||
deleteAgent: db.deleteAgent,
|
||||
});
|
||||
|
||||
router.use(requireAgentManagementAuth);
|
||||
router.use(checkBan);
|
||||
|
|
@ -40,6 +48,7 @@ router.post('/', configMiddleware, createHandler);
|
|||
router.get('/', readHandlers.list);
|
||||
router.get('/:id', readHandlers.get);
|
||||
router.patch('/:id', configMiddleware, updateHandler);
|
||||
router.delete('/:id', deleteHandler);
|
||||
|
||||
router.use((_req, res) => {
|
||||
const { status, body } = mapAgentManagementError('not_found');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue