mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 09:32:24 +00:00
📇 feat: Agent Contact Visibility with Owner Fallback (#13663)
* Shared Contract * Backend Resolution * Frontend Display * Contact Styling and more tests * fix contact flicker when saving an agent * fix display owner when contact deleted * simplification of the last fixes * github action fixes * fixes failing tests --------- Co-authored-by: Peter Rothlaender <peter.rothlaender@ginkgo.com>
This commit is contained in:
Peter
GitHub
parent
376370d610
commit
abf9fc307d
19 changed files with 894 additions and 111 deletions
|
|
@ -48,6 +48,7 @@ const {
|
|||
resolveConfigServers,
|
||||
userCanUseMCPServers,
|
||||
} = require('~/server/services/MCP');
|
||||
const { attachOwnerContacts } = require('~/server/services/Agents/ownerContact');
|
||||
const { getMCPServersRegistry } = require('~/config');
|
||||
const { getLogStores } = require('~/cache');
|
||||
const db = require('~/models');
|
||||
|
|
@ -517,13 +518,15 @@ const getAgentHandler = async (req, res, expandProperties = false) => {
|
|||
});
|
||||
agent.isPublic = isPublic;
|
||||
|
||||
await attachOwnerContacts([agent]);
|
||||
|
||||
if (agent.author !== author) {
|
||||
delete agent.author;
|
||||
}
|
||||
|
||||
if (!expandProperties) {
|
||||
// VIEW permission: Basic agent info only
|
||||
return res.status(200).json({
|
||||
const responseAgent = {
|
||||
_id: agent._id,
|
||||
id: agent.id,
|
||||
name: agent.name,
|
||||
|
|
@ -538,7 +541,16 @@ const getAgentHandler = async (req, res, expandProperties = false) => {
|
|||
// Safe metadata
|
||||
createdAt: agent.createdAt,
|
||||
updatedAt: agent.updatedAt,
|
||||
});
|
||||
};
|
||||
|
||||
if (agent.support_contact !== undefined) {
|
||||
responseAgent.support_contact = agent.support_contact;
|
||||
}
|
||||
if (agent.owner_contact !== undefined) {
|
||||
responseAgent.owner_contact = agent.owner_contact;
|
||||
}
|
||||
|
||||
return res.status(200).json(responseAgent);
|
||||
}
|
||||
|
||||
// EDIT permission: Full agent details including sensitive configuration
|
||||
|
|
@ -710,6 +722,8 @@ const updateAgentHandler = async (req, res) => {
|
|||
updatedAgent.author = updatedAgent.author.toString();
|
||||
}
|
||||
|
||||
await attachOwnerContacts([updatedAgent]);
|
||||
|
||||
if (updatedAgent.author !== req.user.id) {
|
||||
delete updatedAgent.author;
|
||||
}
|
||||
|
|
@ -1045,9 +1059,10 @@ const getListAgentsHandler = async (req, res) => {
|
|||
}
|
||||
|
||||
const publicSet = new Set(publiclyAccessibleIds.map((oid) => oid.toString()));
|
||||
const agentsWithContacts = await attachOwnerContacts(agents);
|
||||
|
||||
const urlCache = cachedRefresh?.urlCache;
|
||||
data.data = agents.map((agent) => {
|
||||
data.data = agentsWithContacts.map((agent) => {
|
||||
if (accessibleSkillSet) {
|
||||
sanitizeViewerSkillScope(agent, accessibleSkillSet);
|
||||
}
|
||||
|
|
@ -1153,6 +1168,7 @@ const uploadAgentAvatarHandler = async (req, res) => {
|
|||
const updatedAgent = await db.updateAgent({ id: agent_id }, data, {
|
||||
updatingUserId: req.user.id,
|
||||
});
|
||||
await attachOwnerContacts([updatedAgent]);
|
||||
|
||||
try {
|
||||
const avatarCache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL);
|
||||
|
|
@ -1257,6 +1273,8 @@ const revertAgentVersionHandler = async (req, res) => {
|
|||
updatedAgent.author = updatedAgent.author.toString();
|
||||
}
|
||||
|
||||
await attachOwnerContacts([updatedAgent]);
|
||||
|
||||
if (updatedAgent.author !== req.user.id) {
|
||||
delete updatedAgent.author;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,14 @@
|
|||
const mongoose = require('mongoose');
|
||||
const { nanoid } = require('nanoid');
|
||||
const { v4: uuidv4 } = require('uuid');
|
||||
const { agentSchema, fileSchema } = require('@librechat/data-schemas');
|
||||
const { FileSources, PermissionBits, ResourceType } = require('librechat-data-provider');
|
||||
const { agentSchema, aclEntrySchema, fileSchema, userSchema } = require('@librechat/data-schemas');
|
||||
const {
|
||||
FileSources,
|
||||
PermissionBits,
|
||||
PrincipalModel,
|
||||
PrincipalType,
|
||||
ResourceType,
|
||||
} = require('librechat-data-provider');
|
||||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
|
||||
// Only mock the dependencies that are not database-related
|
||||
|
|
@ -91,6 +97,32 @@ const { refreshS3Url } = require('@librechat/api');
|
|||
* @type {import('mongoose').Model<import('@librechat/data-schemas').IAgent>}
|
||||
*/
|
||||
let Agent;
|
||||
let AclEntry;
|
||||
let User;
|
||||
|
||||
const OWNER_PERMISSION_BITS =
|
||||
PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE;
|
||||
|
||||
const createOwner = (overrides = {}) =>
|
||||
User.create({
|
||||
name: 'Agent Owner',
|
||||
email: `owner-${nanoid(8)}@example.com`,
|
||||
provider: 'local',
|
||||
emailVerified: true,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const grantAgentOwner = ({ agent, owner, grantedAt = new Date() }) =>
|
||||
AclEntry.create({
|
||||
principalType: PrincipalType.USER,
|
||||
principalModel: PrincipalModel.USER,
|
||||
principalId: owner._id,
|
||||
resourceType: ResourceType.AGENT,
|
||||
resourceId: agent._id,
|
||||
permBits: OWNER_PERMISSION_BITS,
|
||||
grantedBy: owner._id,
|
||||
grantedAt,
|
||||
});
|
||||
|
||||
describe('Agent Controllers - Mass Assignment Protection', () => {
|
||||
let mongoServer;
|
||||
|
|
@ -102,6 +134,8 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
const mongoUri = mongoServer.getUri();
|
||||
await mongoose.connect(mongoUri);
|
||||
Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema);
|
||||
AclEntry = mongoose.models.AclEntry || mongoose.model('AclEntry', aclEntrySchema);
|
||||
User = mongoose.models.User || mongoose.model('User', userSchema);
|
||||
// Register File so orphan-pruning tests (and the tool_resources validation
|
||||
// test, which now needs real File docs for its ids) have a working model.
|
||||
mongoose.models.File || mongoose.model('File', fileSchema);
|
||||
|
|
@ -114,6 +148,8 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
|
||||
beforeEach(async () => {
|
||||
await Agent.deleteMany({});
|
||||
await AclEntry.deleteMany({});
|
||||
await User.deleteMany({});
|
||||
await mongoose.models.File.deleteMany({});
|
||||
|
||||
// Reset all mocks
|
||||
|
|
@ -510,6 +546,61 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(response.model_parameters.temperature).toBeUndefined();
|
||||
expect(response.model_parameters.apiKey).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should return owner_contact from the first ACL owner when support_contact is missing', async () => {
|
||||
const owner = await createOwner({
|
||||
name: 'Primary Owner',
|
||||
email: 'primary.owner@example.com',
|
||||
});
|
||||
const agent = await Agent.create({
|
||||
id: `agent_${uuidv4()}`,
|
||||
name: 'Owner Contact Agent',
|
||||
description: 'Uses owner fallback',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: owner._id,
|
||||
});
|
||||
await grantAgentOwner({ agent, owner });
|
||||
|
||||
mockReq.params = { id: agent.id };
|
||||
|
||||
await getAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(200);
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.owner_contact).toEqual({
|
||||
name: 'Primary Owner',
|
||||
email: 'primary.owner@example.com',
|
||||
});
|
||||
});
|
||||
|
||||
test('should not return owner_contact when support_contact is present', async () => {
|
||||
const owner = await createOwner({
|
||||
name: 'Primary Owner',
|
||||
email: 'primary.owner@example.com',
|
||||
});
|
||||
const agent = await Agent.create({
|
||||
id: `agent_${uuidv4()}`,
|
||||
name: 'Support Contact Agent',
|
||||
description: 'Uses support contact',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: owner._id,
|
||||
support_contact: { name: 'Support Team', email: 'support@example.com' },
|
||||
});
|
||||
await grantAgentOwner({ agent, owner });
|
||||
|
||||
mockReq.params = { id: agent.id };
|
||||
|
||||
await getAgentHandler(mockReq, mockRes);
|
||||
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.support_contact).toEqual({
|
||||
name: 'Support Team',
|
||||
email: 'support@example.com',
|
||||
});
|
||||
expect(response.owner_contact).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateAgentHandler', () => {
|
||||
|
|
@ -1303,6 +1394,71 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
expect(response.data[0].name).toBe('Agent A1');
|
||||
});
|
||||
|
||||
test('should return owner_contact for list agents missing support_contact', async () => {
|
||||
const owner = await createOwner({
|
||||
_id: userA,
|
||||
name: 'List Owner',
|
||||
email: 'list.owner@example.com',
|
||||
});
|
||||
await grantAgentOwner({ agent: agentA1, owner });
|
||||
|
||||
mockReq.user.id = userB.toString();
|
||||
findAccessibleResources.mockResolvedValue([agentA1._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.data[0].owner_contact).toEqual({
|
||||
name: 'List Owner',
|
||||
email: 'list.owner@example.com',
|
||||
});
|
||||
});
|
||||
|
||||
test('should use the first ACL owner when an agent has multiple owners', async () => {
|
||||
const firstOwner = await createOwner({
|
||||
name: 'First Owner',
|
||||
email: 'first.owner@example.com',
|
||||
});
|
||||
const secondOwner = await createOwner({
|
||||
name: 'Second Owner',
|
||||
email: 'second.owner@example.com',
|
||||
});
|
||||
await grantAgentOwner({
|
||||
agent: agentA1,
|
||||
owner: secondOwner,
|
||||
grantedAt: new Date('2024-02-01T00:00:00.000Z'),
|
||||
});
|
||||
await grantAgentOwner({
|
||||
agent: agentA1,
|
||||
owner: firstOwner,
|
||||
grantedAt: new Date('2024-01-01T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
mockReq.user.id = userB.toString();
|
||||
findAccessibleResources.mockResolvedValue([agentA1._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.data[0].owner_contact).toEqual({
|
||||
name: 'First Owner',
|
||||
email: 'first.owner@example.com',
|
||||
});
|
||||
});
|
||||
|
||||
test('should omit owner_contact when no owner user can be resolved', async () => {
|
||||
mockReq.user.id = userB.toString();
|
||||
findAccessibleResources.mockResolvedValue([agentA1._id]);
|
||||
findPubliclyAccessibleResources.mockResolvedValue([]);
|
||||
|
||||
await getListAgentsHandler(mockReq, mockRes);
|
||||
|
||||
const response = mockRes.json.mock.calls[0][0];
|
||||
expect(response.data[0].owner_contact).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should return only expected safe list fields for VIEW callers', async () => {
|
||||
const hiddenSkillId = new mongoose.Types.ObjectId();
|
||||
await Agent.findByIdAndUpdate(agentA1._id, {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue