diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index 65b85d5971..f366c6d253 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -1317,6 +1317,7 @@ class BaseClient { context: 'api/app/clients/BaseClient.js - saveMessageToDatabase #saveConvo', unsetFields, noUpsert: req?._agentEventBindingParentConversationId != null, + initialAgentId: hasNonEphemeralAgent ? options.agent?.id : null, createdAtOnInsert: shouldSetCreatedAtOnInsert ? validCreatedAtOnInsert : undefined, ...(savedMessage?._id != null ? { appendMessageIds: [savedMessage._id] } : {}), }); diff --git a/api/app/clients/specs/BaseClient.test.js b/api/app/clients/specs/BaseClient.test.js index 139e9e9518..92a96eb649 100644 --- a/api/app/clients/specs/BaseClient.test.js +++ b/api/app/clients/specs/BaseClient.test.js @@ -1944,6 +1944,11 @@ describe('BaseClient', () => { test('saveMessageToDatabase uses snapshot of options, immune to mid-await disposal', async () => { const savedOptions = TestClient.options; + TestClient.options = { + ...savedOptions, + endpoint: 'agents', + agent: { id: 'agent_persisted' }, + }; saveMessage.mockClear(); saveConvo.mockClear(); @@ -1957,7 +1962,7 @@ describe('BaseClient', () => { const result = await TestClient.saveMessageToDatabase( { messageId: 'msg-1', conversationId: 'conv-1', isCreatedByUser: true, text: 'hi' }, - { endpoint: 'openAI' }, + { endpoint: 'agents', agent_id: 'agent_persisted' }, null, ); @@ -1965,6 +1970,11 @@ describe('BaseClient', () => { expect(result).toHaveProperty('message'); expect(result).toHaveProperty('conversation'); expect(saveMessage).toHaveBeenCalled(); + expect(saveConvo).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ initialAgentId: 'agent_persisted' }), + ); TestClient.options = savedOptions; saveMessage.mockReset(); diff --git a/api/server/controllers/PermissionsController.js b/api/server/controllers/PermissionsController.js index d112904d7a..6bfe9028f9 100644 --- a/api/server/controllers/PermissionsController.js +++ b/api/server/controllers/PermissionsController.js @@ -5,9 +5,18 @@ const mongoose = require('mongoose'); const { logger, getTenantId, SYSTEM_TENANT_ID } = require('@librechat/data-schemas'); const { ResourceType, PrincipalType, PermissionBits } = require('librechat-data-provider'); -const { enrichRemoteAgentPrincipals, backfillRemoteAgentPermissions } = require('@librechat/api'); +const { + enrichRemoteAgentPrincipals, + backfillRemoteAgentPermissions, + auditInsightsPermissionChanges, + getInsightsPrincipalState, + maskAgentInsightsBit, + sanitizeInsightsPermissionPrincipals, + validateInsightsPermissionUpdates, +} = require('@librechat/api'); const { bulkUpdateResourcePermissions, + restoreInsightsPermissionChanges, ensureGroupPrincipalExists, getResourcePermissionsMap, findAccessibleResources, @@ -65,14 +74,24 @@ const updateResourcePermissions = async (req, res) => { /** @type {TUpdateResourcePermissionsRequest} */ const { updated, removed, public: isPublic, publicAccessRoleId } = req.body; const { id: userId } = req.user; + const updatedList = Array.isArray(updated) ? updated : []; + const removedList = Array.isArray(removed) ? removed : []; + const insightsValidation = validateInsightsPermissionUpdates({ + resourceType, + userRole: req.user.role, + updatedPrincipals: updatedList, + }); + if (insightsValidation) { + return res.status(insightsValidation.status).json({ error: insightsValidation.error }); + } // Prepare principals for the service call const updatedPrincipals = []; const revokedPrincipals = []; // Add updated principals - if (updated && Array.isArray(updated)) { - updatedPrincipals.push(...updated); + if (updatedList.length > 0) { + updatedPrincipals.push(...updatedList); } // Add public permission if enabled @@ -138,8 +157,8 @@ const updateResourcePermissions = async (req, res) => { } // Add removed principals - if (removed && Array.isArray(removed)) { - revokedPrincipals.push(...removed); + if (removedList.length > 0) { + revokedPrincipals.push(...removedList); } // If public is explicitly disabled, add public to revoked list @@ -158,6 +177,24 @@ const updateResourcePermissions = async (req, res) => { grantedBy: userId, }); + await auditInsightsPermissionChanges({ + req, + resourceId, + changes: results.insightsChanges ?? [], + failClosed: process.env.AUDIT_LOG_FAIL_CLOSED === 'true', + deps: { + getAgent: db.getAgent, + recordAuditEntry: db.recordAuditEntry, + restoreInsightsPermissionChanges: (changes) => + restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes, + }), + logger, + }, + }); + if (resourceType === ResourceType.CODE_ENVIRONMENT) { await invalidateCodeEnvironmentConfigCache(req.user.tenantId).catch((error) => { // Cached environment metadata is authorization-filtered against the live ACL on every @@ -179,10 +216,15 @@ const updateResourcePermissions = async (req, res) => { } /** @type {TUpdateResourcePermissionsResponse} */ + const responsePrincipals = sanitizeInsightsPermissionPrincipals({ + resourceType, + userRole: req.user.role, + principals: results.granted, + }); const response = { message: 'Permissions updated successfully', results: { - principals: results.granted, + principals: responsePrincipals, ...(isPublic !== undefined ? { public: isPublic } : {}), publicAccessRoleId: isPublic ? publicAccessRoleId : undefined, }, @@ -191,7 +233,7 @@ const updateResourcePermissions = async (req, res) => { res.status(200).json(response); } catch (error) { logger.error('Error updating resource permissions:', error); - res.status(400).json({ + res.status(error.statusCode ?? 400).json({ error: 'Failed to update permissions', details: error.message, }); @@ -254,6 +296,7 @@ const getResourcePermissions = async (req, res) => { accessRoleId: { $arrayElemAt: ['$role.accessRoleId', 0] }, userInfo: { $arrayElemAt: ['$userInfo', 0] }, groupInfo: { $arrayElemAt: ['$groupInfo', 0] }, + permBits: 1, }, }, ]); @@ -281,6 +324,12 @@ const getResourcePermissions = async (req, res) => { source: !result.userInfo._id ? 'entra' : 'local', idOnTheSource: result.userInfo.idOnTheSource || result.userInfo._id.toString(), accessRoleId: result.accessRoleId, + ...getInsightsPrincipalState({ + principalType: PrincipalType.USER, + principalRole: result.userInfo.role, + requesterRole: req.user.role, + permBits: result.permBits, + }), }); } else if ( result.principalType === PrincipalType.GROUP && @@ -297,6 +346,11 @@ const getResourcePermissions = async (req, res) => { source: result.groupInfo.source || 'local', idOnTheSource: result.groupInfo.idOnTheSource || result.groupInfo._id.toString(), accessRoleId: result.accessRoleId, + ...getInsightsPrincipalState({ + principalType: PrincipalType.GROUP, + requesterRole: req.user.role, + permBits: result.permBits, + }), }); } else if (result.principalType === PrincipalType.ROLE) { principals.push({ @@ -307,6 +361,12 @@ const getResourcePermissions = async (req, res) => { name: result.principalId, description: `System role: ${result.principalId}`, accessRoleId: result.accessRoleId, + ...getInsightsPrincipalState({ + principalType: PrincipalType.ROLE, + principalRole: result.principalId, + requesterRole: req.user.role, + permBits: result.permBits, + }), }); } } @@ -391,7 +451,11 @@ const getUserEffectivePermissions = async (req, res) => { }); res.status(200).json({ - permissionBits, + permissionBits: maskAgentInsightsBit({ + resourceType, + userRole: req.user.role, + permBits: permissionBits, + }), }); } catch (error) { logger.error('Error getting user effective permissions:', error); @@ -556,7 +620,11 @@ const getAllEffectivePermissions = async (req, res) => { // Convert Map to plain object for JSON response const result = {}; for (const [resourceId, permBits] of permissionsMap) { - result[resourceId] = permBits; + result[resourceId] = maskAgentInsightsBit({ + resourceType, + userRole: req.user.role, + permBits, + }); } res.status(200).json(result); diff --git a/api/server/controllers/__tests__/PermissionsController.spec.js b/api/server/controllers/__tests__/PermissionsController.spec.js index 95236f2ca3..2a578b75e7 100644 --- a/api/server/controllers/__tests__/PermissionsController.spec.js +++ b/api/server/controllers/__tests__/PermissionsController.spec.js @@ -9,17 +9,21 @@ jest.mock('@librechat/data-schemas', () => ({ SYSTEM_TENANT_ID: '__SYSTEM__', })); -const { AccessRoleIds, ResourceType, PrincipalType } = +const { AccessRoleIds, ResourceType, PrincipalType, SystemRoles } = jest.requireActual('librechat-data-provider'); jest.mock('librechat-data-provider', () => ({ ...jest.requireActual('librechat-data-provider'), })); -jest.mock('@librechat/api', () => ({ - enrichRemoteAgentPrincipals: jest.fn(), - backfillRemoteAgentPermissions: jest.fn(), -})); +jest.mock('@librechat/api', () => { + const actual = jest.requireActual('@librechat/api'); + return { + ...actual, + enrichRemoteAgentPrincipals: jest.fn(), + backfillRemoteAgentPermissions: jest.fn(), + }; +}); const mockInvalidateCodeEnvironmentConfigCache = jest.fn().mockResolvedValue(undefined); jest.mock('~/server/services/Config', () => ({ @@ -28,9 +32,11 @@ jest.mock('~/server/services/Config', () => ({ })); const mockBulkUpdateResourcePermissions = jest.fn(); +const mockRestoreInsightsPermissionChanges = jest.fn(); jest.mock('~/server/services/PermissionService', () => ({ bulkUpdateResourcePermissions: (...args) => mockBulkUpdateResourcePermissions(...args), + restoreInsightsPermissionChanges: (...args) => mockRestoreInsightsPermissionChanges(...args), ensureGroupPrincipalExists: jest.fn(), getEffectivePermissions: jest.fn(), ensurePrincipalExists: jest.fn(), @@ -40,6 +46,7 @@ jest.mock('~/server/services/PermissionService', () => ({ })); const mockRemoveAgentFromUserFavorites = jest.fn(); +const mockRecordAuditEntry = jest.fn(); jest.mock('~/models', () => ({ aggregateAclEntries: jest.fn(), @@ -47,6 +54,8 @@ jest.mock('~/models', () => ({ sortPrincipalsByRelevance: jest.fn(), calculateRelevanceScore: jest.fn(), removeAgentFromUserFavorites: (...args) => mockRemoveAgentFromUserFavorites(...args), + getAgent: jest.fn(), + recordAuditEntry: (...args) => mockRecordAuditEntry(...args), })); jest.mock('~/server/services/GraphApiService', () => ({ @@ -221,6 +230,7 @@ describe('PermissionsController', () => { name: 'Current User', email: 'current-user@example.com', avatar: 'current-user.png', + role: SystemRoles.ADMIN, }, }, { @@ -247,6 +257,7 @@ describe('PermissionsController', () => { type: PrincipalType.USER, id: userId.toString(), email: 'current-user@example.com', + isAdmin: true, }), expect.objectContaining({ type: PrincipalType.GROUP, @@ -262,6 +273,7 @@ describe('PermissionsController', () => { const revokedUserId = new mongoose.Types.ObjectId().toString(); beforeEach(() => { + delete process.env.AUDIT_LOG_FAIL_CLOSED; mockBulkUpdateResourcePermissions.mockResolvedValue({ granted: [], updated: [], @@ -270,6 +282,210 @@ describe('PermissionsController', () => { }); mockRemoveAgentFromUserFavorites.mockResolvedValue(undefined); + db.getAgent.mockResolvedValue({ _id: agentObjectId, id: 'agent-a', name: 'Agent A' }); + mockRecordAuditEntry.mockResolvedValue({}); + mockRestoreInsightsPermissionChanges.mockResolvedValue(undefined); + }); + + it('rejects Insights permission changes from non-admin users', async () => { + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + body: { + updated: [ + { + type: PrincipalType.ROLE, + id: 'USER', + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + removed: [], + }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(res.status).toHaveBeenCalledWith(403); + expect(mockBulkUpdateResourcePermissions).not.toHaveBeenCalled(); + }); + + it('rejects non-boolean Insights permission values', async () => { + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + body: { + updated: [ + { + type: PrincipalType.ROLE, + id: 'USER', + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: null, + }, + ], + removed: [], + }, + user: { id: 'admin-id', role: SystemRoles.ADMIN }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(mockBulkUpdateResourcePermissions).not.toHaveBeenCalled(); + }); + + it('audits each actual Insights access transition made by an admin', async () => { + mockBulkUpdateResourcePermissions.mockResolvedValue({ + granted: [ + { + type: PrincipalType.ROLE, + id: 'USER', + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + updated: [], + revoked: [], + insightsChanges: [ + { action: 'assigned', principal: { type: PrincipalType.ROLE, id: 'USER' } }, + ], + errors: [], + }); + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + body: { + updated: [ + { + type: PrincipalType.ROLE, + id: 'USER', + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + removed: [], + }, + user: { + _id: new mongoose.Types.ObjectId(), + id: 'admin-id', + name: 'Admin', + role: SystemRoles.ADMIN, + tenantId: 'tenant-a', + }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(mockRecordAuditEntry).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'permission.insights_assigned', + tenantId: 'tenant-a', + target: expect.objectContaining({ id: 'agent-a' }), + metadata: { principalType: PrincipalType.ROLE, principalId: 'USER' }, + }), + { failClosed: false }, + ); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json.mock.calls[0][0].results.principals[0]).toEqual( + expect.objectContaining({ viewInsights: true }), + ); + }); + + it('restores unaudited Insights transitions when fail-closed auditing fails', async () => { + process.env.AUDIT_LOG_FAIL_CLOSED = 'true'; + const changes = [{ action: 'assigned', principal: { type: PrincipalType.ROLE, id: 'USER' } }]; + mockBulkUpdateResourcePermissions.mockResolvedValue({ + granted: [], + updated: [], + revoked: [], + insightsChanges: changes, + errors: [], + }); + mockRecordAuditEntry.mockRejectedValue(new Error('audit unavailable')); + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + body: { + updated: [ + { + type: PrincipalType.ROLE, + id: 'USER', + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + removed: [], + }, + user: { id: 'admin-id', role: SystemRoles.ADMIN, tenantId: 'tenant-a' }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(mockRestoreInsightsPermissionChanges).toHaveBeenCalledWith({ + resourceType: ResourceType.AGENT, + resourceId: agentObjectId, + changes, + }); + expect(res.status).toHaveBeenCalledWith(500); + }); + + it('keeps a successful permission response when audit target lookup fails open', async () => { + mockBulkUpdateResourcePermissions.mockResolvedValue({ + granted: [], + updated: [], + revoked: [], + insightsChanges: [ + { action: 'assigned', principal: { type: PrincipalType.ROLE, id: 'USER' } }, + ], + errors: [], + }); + db.getAgent.mockRejectedValue(new Error('agent lookup unavailable')); + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + user: { id: 'admin-id', role: SystemRoles.ADMIN, tenantId: 'tenant-a' }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(mockRestoreInsightsPermissionChanges).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('does not expose the protected bit in non-admin mutation responses', async () => { + mockBulkUpdateResourcePermissions.mockResolvedValue({ + granted: [ + { + type: PrincipalType.USER, + id: revokedUserId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + viewInsights: true, + }, + ], + updated: [], + revoked: [], + insightsChanges: [], + errors: [], + }); + const req = createMockReq({ + params: { resourceType: ResourceType.AGENT, resourceId: agentObjectId }, + body: { + updated: [ + { + type: PrincipalType.USER, + id: revokedUserId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + }, + ], + removed: [], + }, + }); + const res = createMockRes(); + + await updateResourcePermissions(req, res); + + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json.mock.calls[0][0].results.principals[0]).not.toHaveProperty('viewInsights'); }); it('removes agent from revoked users favorites on AGENT resource type', async () => { diff --git a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js index fd49e3bfd4..a0891694cb 100644 --- a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js +++ b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js @@ -3610,9 +3610,11 @@ describe('ResumableAgentController resume metadata', () => { const req = createFailedRequest({ conversationId: undefined, clientRequestId: 'failed-new-conversation', + agent_id: 'unverified-agent', parentMessageId: '00000000-0000-0000-0000-000000000000', endpointOption: { endpoint: 'azureOpenAI', + agent_id: 'unverified-agent', modelOptions: { model: 'gpt-4o' }, chatProjectId: '507f1f77bcf86cd799439011', }, @@ -3643,7 +3645,7 @@ describe('ResumableAgentController resume metadata', () => { model: 'gpt-4o', chatProjectId: '507f1f77bcf86cd799439011', }), - expect.any(Object), + expect.objectContaining({ initialAgentId: null }), ); }); }); diff --git a/api/server/controllers/agents/request.js b/api/server/controllers/agents/request.js index 7d9a22a908..b1bdd555cd 100644 --- a/api/server/controllers/agents/request.js +++ b/api/server/controllers/agents/request.js @@ -272,6 +272,7 @@ async function saveErrorTurn( liveUserMessage, liveResponseMessageId, sender, + initialAgentId, }, ) { try { @@ -413,7 +414,15 @@ async function saveErrorTurn( await saveConvo( reqCtx, { conversationId, ...convoFields }, - seedConvo ? { context } : { context, noUpsert: true }, + seedConvo + ? { + context, + initialAgentId: + typeof initialAgentId === 'string' && !isEphemeralAgentId(initialAgentId) + ? initialAgentId + : null, + } + : { context, noUpsert: true }, ); } catch (err) { logger.error('[AgentController] Failed to persist error turn', err); @@ -1371,6 +1380,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit }); let client = null; + let verifiedInitialAgentId = null; let jobCreatedAt; let providerExecutionId; let releaseEventChildLease; @@ -1763,6 +1773,12 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit }); startupTelemetry?.mark('client_initialized'); client = result.client; + if ( + typeof client?.options?.agent?.id === 'string' && + !isEphemeralAgentId(client.options.agent.id) + ) { + verifiedInitialAgentId = client.options.agent.id; + } /** Request-shape validation rejects every known edit/regenerate path, but * the client owns the final persistence decision. Fail closed if a future @@ -2997,6 +3013,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit liveUserMessage: userMessage, liveResponseMessageId, sender: client?.sender, + initialAgentId: verifiedInitialAgentId, }), })) === true; /** A true completion means this owner won the terminal CAS and @@ -3201,6 +3218,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit endpointOption, isNewConvo, errorText: initializationError, + initialAgentId: verifiedInitialAgentId, }), }) : GenerationJobManager.completeJob(streamId, initializationError, jobCreatedAt); diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 606f4733be..4903abf569 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -453,11 +453,14 @@ async function saveConversation(req, conversationId, agentId, agent) { { conversationId, endpoint: EModelEndpoint.agents, - agentId, + agent_id: agentId, ...(title != null && { title }), model: agent?.model, }, - { context: 'Responses API - save conversation' }, + { + context: 'Responses API - save conversation', + initialAgentId: agent?.id === agentId ? agentId : null, + }, ); } diff --git a/api/server/experimental.js b/api/server/experimental.js index 78ed711a79..102c6bbfed 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -634,7 +634,7 @@ if (cluster.isMaster) { /** Routes */ app.use('/oauth', preAuthTenantMiddleware, routes.oauth); app.use('/api/auth', preAuthTenantMiddleware, routes.auth); - app.use('/api/admin/insights', routes.insights); + app.use('/api/insights', routes.insights); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/skills', routes.adminSkills); app.use('/api/admin/code-environments', routes.adminCodeEnvironments); diff --git a/api/server/index.js b/api/server/index.js index 5d25eb248f..046004252d 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -380,7 +380,7 @@ const startServer = async () => { app.use('/oauth', preAuthTenantMiddleware, routes.oauth); /* API Endpoints */ app.use('/api/auth', preAuthTenantMiddleware, routes.auth); - app.use('/api/admin/insights', routes.insights); + app.use('/api/insights', routes.insights); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/config', routes.adminConfig); app.use('/api/admin/code-environments', routes.adminCodeEnvironments); diff --git a/api/server/middleware/checkPeoplePickerAccess.js b/api/server/middleware/checkPeoplePickerAccess.js index 50f137285e..83329f56a3 100644 --- a/api/server/middleware/checkPeoplePickerAccess.js +++ b/api/server/middleware/checkPeoplePickerAccess.js @@ -1,5 +1,10 @@ const { logger } = require('@librechat/data-schemas'); -const { PrincipalType, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { + PrincipalType, + PermissionTypes, + Permissions, + SystemRoles, +} = require('librechat-data-provider'); const { getRoleByName } = require('~/models'); const VALID_PRINCIPAL_TYPES = new Set([ @@ -27,6 +32,10 @@ const checkPeoplePickerAccess = async (req, res, next) => { }); } + if (user.role === SystemRoles.ADMIN) { + return next(); + } + const role = await getRoleByName(user.role); if (!role || !role.permissions) { return res.status(403).json({ diff --git a/api/server/middleware/checkPeoplePickerAccess.spec.js b/api/server/middleware/checkPeoplePickerAccess.spec.js index c394bbae65..45097ef290 100644 --- a/api/server/middleware/checkPeoplePickerAccess.spec.js +++ b/api/server/middleware/checkPeoplePickerAccess.spec.js @@ -1,5 +1,10 @@ const { logger } = require('@librechat/data-schemas'); -const { PrincipalType, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { + PrincipalType, + PermissionTypes, + Permissions, + SystemRoles, +} = require('librechat-data-provider'); const { checkPeoplePickerAccess } = require('./checkPeoplePickerAccess'); const { getRoleByName } = require('~/models'); @@ -53,6 +58,16 @@ describe('checkPeoplePickerAccess', () => { expect(next).not.toHaveBeenCalled(); }); + it('allows a literal admin regardless of configured people picker permissions', async () => { + req.user.role = SystemRoles.ADMIN; + + await checkPeoplePickerAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(getRoleByName).not.toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + it('should allow access when searching for users with VIEW_USERS permission', async () => { req.query.type = PrincipalType.USER; getRoleByName.mockResolvedValue({ diff --git a/api/server/routes/__tests__/insights.spec.js b/api/server/routes/__tests__/insights.spec.js index 3f754eb0e8..e173cb23eb 100644 --- a/api/server/routes/__tests__/insights.spec.js +++ b/api/server/routes/__tests__/insights.spec.js @@ -3,21 +3,27 @@ const request = require('supertest'); const mockCreateInsightsAccessHandler = jest.fn(() => (_req, res) => res.json({ access: true })); const mockCreateInsightsHandler = jest.fn(() => (_req, res) => res.json({ summary: {} })); -const mockGrantedCapabilities = new Set(['access:admin', 'read:insights']); +const mockGetAccessibleAgents = jest.fn(); const mockGetInsights = jest.fn(); -let mockUser = { id: 'admin-id', role: 'ADMIN' }; +let mockResolverCreateCount = 0; +let mockAccessHandlerDeps; +let mockDashboardHandlerDeps; +let mockUser = { id: 'user-id', role: 'USER' }; jest.mock('@librechat/api', () => ({ - createInsightsAccessHandler: (...args) => mockCreateInsightsAccessHandler(...args), - createInsightsHandler: (...args) => mockCreateInsightsHandler(...args), - isEnabled: (value) => value === 'true', -})); - -jest.mock('@librechat/data-schemas', () => ({ - SystemCapabilities: { - ACCESS_ADMIN: 'access:admin', - READ_INSIGHTS: 'read:insights', + createInsightsAccessHandler: (...args) => { + [mockAccessHandlerDeps] = args; + return mockCreateInsightsAccessHandler(...args); }, + createInsightsAgentAccessResolver: () => { + mockResolverCreateCount += 1; + return mockGetAccessibleAgents; + }, + createInsightsHandler: (...args) => { + [mockDashboardHandlerDeps] = args; + return mockCreateInsightsHandler(...args); + }, + isEnabled: (value) => value === 'true', })); jest.mock('~/server/middleware', () => ({ @@ -25,69 +31,49 @@ jest.mock('~/server/middleware', () => ({ req.user = mockUser; next(); }, - checkAdmin: (req, res, next) => { - if (req.user.role !== 'ADMIN') { - return res.status(403).json({ message: 'Forbidden' }); - } - next(); - }, -})); - -jest.mock('~/server/middleware/roles/capabilities', () => ({ - requireCapability: (capability) => (_req, res, next) => { - if (!mockGrantedCapabilities.has(capability)) { - return res.status(403).json({ message: 'Forbidden' }); - } - next(); - }, })); jest.mock('~/models', () => ({ + findAccessibleResources: jest.fn(), + getAgents: jest.fn(), getInsights: (...args) => mockGetInsights(...args), + getUserPrincipals: jest.fn(), + hasCapabilityForPrincipals: jest.fn(), })); const insightsRouter = require('../insights'); function createApp() { const app = express(); - app.use('/api/admin/insights', insightsRouter); + app.use('/api/insights', insightsRouter); return app; } describe('Insights routes', () => { beforeEach(() => { - mockUser = { id: 'admin-id', role: 'ADMIN' }; - mockGrantedCapabilities.clear(); - mockGrantedCapabilities.add('access:admin'); - mockGrantedCapabilities.add('read:insights'); + mockUser = { id: 'user-id', role: 'USER' }; }); - it('requires the ADMIN role', async () => { - mockUser = { id: 'delegated-admin-id', role: 'DELEGATED_ADMIN' }; - - const response = await request(createApp()).get('/api/admin/insights'); - - expect(response.status).toBe(403); - }); - - it('serves the access probe and dashboard', async () => { + it('serves the access probe and dashboard for authenticated users', async () => { const app = createApp(); - await expect(request(app).get('/api/admin/insights/access')).resolves.toMatchObject({ + await expect(request(app).get('/api/insights/access')).resolves.toMatchObject({ status: 200, body: { access: true }, }); - await expect(request(app).get('/api/admin/insights')).resolves.toMatchObject({ + await expect(request(app).get('/api/insights')).resolves.toMatchObject({ status: 200, body: { summary: {} }, }); }); - it.each(['access:admin', 'read:insights'])('requires %s', async (capability) => { - mockGrantedCapabilities.delete(capability); - - const response = await request(createApp()).get('/api/admin/insights'); - - expect(response.status).toBe(403); + it('uses one shared agent access resolver for both handlers', () => { + expect(mockResolverCreateCount).toBe(1); + expect(mockAccessHandlerDeps).toEqual( + expect.objectContaining({ getAccessibleAgents: mockGetAccessibleAgents }), + ); + expect(mockDashboardHandlerDeps).toEqual( + expect.objectContaining({ getAccessibleAgents: mockGetAccessibleAgents }), + ); }); }); diff --git a/api/server/routes/accessPermissions.js b/api/server/routes/accessPermissions.js index 311f059300..96aa8c8a3e 100644 --- a/api/server/routes/accessPermissions.js +++ b/api/server/routes/accessPermissions.js @@ -6,6 +6,7 @@ const { ResourceType, PermissionBits, } = require('librechat-data-provider'); +const { createAgentAdminPermissionAccess, isAgentPermissionsAdmin } = require('@librechat/api'); const { getUserEffectivePermissions, getAllEffectivePermissions, @@ -20,7 +21,8 @@ const { } = require('~/server/middleware/checkSharePublicAccess'); const { requireJwtAuth, checkBan, uaParser, canAccessResource } = require('~/server/middleware'); const { checkPeoplePickerAccess } = require('~/server/middleware/checkPeoplePickerAccess'); -const { findMCPServerByObjectId, getSkillById } = require('~/models'); +const db = require('~/models'); +const { findMCPServerByObjectId, getSkillById } = db; const router = express.Router(); @@ -52,63 +54,74 @@ router.get('/:resourceType/roles', getResourceRoles); * @param {string} requiredPermission - The permission bit required (e.g., SHARE) * @returns Express middleware function */ -const checkResourcePermissionAccess = (requiredPermission) => (req, res, next) => { - const { resourceType } = req.params; - let middleware; +const checkResourcePermissionAccess = (requiredPermission) => + createAgentAdminPermissionAccess({ + getAgent: db.getAgent, + fallback: (req, res, next) => { + const { resourceType } = req.params; + let middleware; - if (resourceType === ResourceType.AGENT) { - middleware = canAccessResource({ - resourceType: ResourceType.AGENT, - requiredPermission, - resourceIdParam: 'resourceId', - }); - } else if (resourceType === ResourceType.REMOTE_AGENT) { - middleware = canAccessResource({ - resourceType: ResourceType.REMOTE_AGENT, - requiredPermission, - resourceIdParam: 'resourceId', - }); - } else if (resourceType === ResourceType.PROMPTGROUP) { - middleware = canAccessResource({ - resourceType: ResourceType.PROMPTGROUP, - requiredPermission, - resourceIdParam: 'resourceId', - }); - } else if (resourceType === ResourceType.MCPSERVER) { - middleware = canAccessResource({ - resourceType: ResourceType.MCPSERVER, - requiredPermission, - resourceIdParam: 'resourceId', - idResolver: findMCPServerByObjectId, - }); - } else if (resourceType === ResourceType.SKILL) { - middleware = canAccessResource({ - resourceType: ResourceType.SKILL, - requiredPermission, - resourceIdParam: 'resourceId', - idResolver: getSkillById, - }); - } else if (resourceType === ResourceType.CODE_ENVIRONMENT) { - middleware = canAccessResource({ - resourceType: ResourceType.CODE_ENVIRONMENT, - requiredPermission, - resourceIdParam: 'resourceId', - }); - } else if (resourceType === ResourceType.SHARED_LINK) { - middleware = canAccessResource({ - resourceType: ResourceType.SHARED_LINK, - requiredPermission, - resourceIdParam: 'resourceId', - }); - } else { - return res.status(400).json({ - error: 'Bad Request', - message: `Unsupported resource type: ${resourceType}`, - }); + if (resourceType === ResourceType.AGENT) { + middleware = canAccessResource({ + resourceType: ResourceType.AGENT, + requiredPermission, + resourceIdParam: 'resourceId', + }); + } else if (resourceType === ResourceType.REMOTE_AGENT) { + middleware = canAccessResource({ + resourceType: ResourceType.REMOTE_AGENT, + requiredPermission, + resourceIdParam: 'resourceId', + }); + } else if (resourceType === ResourceType.PROMPTGROUP) { + middleware = canAccessResource({ + resourceType: ResourceType.PROMPTGROUP, + requiredPermission, + resourceIdParam: 'resourceId', + }); + } else if (resourceType === ResourceType.MCPSERVER) { + middleware = canAccessResource({ + resourceType: ResourceType.MCPSERVER, + requiredPermission, + resourceIdParam: 'resourceId', + idResolver: findMCPServerByObjectId, + }); + } else if (resourceType === ResourceType.SKILL) { + middleware = canAccessResource({ + resourceType: ResourceType.SKILL, + requiredPermission, + resourceIdParam: 'resourceId', + idResolver: getSkillById, + }); + } else if (resourceType === ResourceType.CODE_ENVIRONMENT) { + middleware = canAccessResource({ + resourceType: ResourceType.CODE_ENVIRONMENT, + requiredPermission, + resourceIdParam: 'resourceId', + }); + } else if (resourceType === ResourceType.SHARED_LINK) { + middleware = canAccessResource({ + resourceType: ResourceType.SHARED_LINK, + requiredPermission, + resourceIdParam: 'resourceId', + }); + } else { + return res.status(400).json({ + error: 'Bad Request', + message: `Unsupported resource type: ${resourceType}`, + }); + } + + // Execute the middleware + middleware(req, res, next); + }, + }); + +const checkShareAccessUnlessAgentAdmin = (req, res, next) => { + if (isAgentPermissionsAdmin(req)) { + return next(); } - - // Execute the middleware - middleware(req, res, next); + return checkShareAccess(req, res, next); }; const rejectSharedLinkOwnerPermissionChanges = async (req, res, next) => { @@ -182,7 +195,7 @@ router.get( router.put( '/:resourceType/:resourceId', checkResourcePermissionAccess(PermissionBits.SHARE), - checkShareAccess, + checkShareAccessUnlessAgentAdmin, checkSharePublicAccess, rejectSharedLinkOwnerPermissionChanges, updateResourcePermissions, diff --git a/api/server/routes/insights.js b/api/server/routes/insights.js index 2f58bb82df..6f0e13c9ae 100644 --- a/api/server/routes/insights.js +++ b/api/server/routes/insights.js @@ -1,17 +1,27 @@ const express = require('express'); -const { createInsightsAccessHandler, createInsightsHandler, isEnabled } = require('@librechat/api'); -const { SystemCapabilities } = require('@librechat/data-schemas'); -const { requireJwtAuth, checkAdmin } = require('~/server/middleware'); -const { requireCapability } = require('~/server/middleware/roles/capabilities'); +const { + createInsightsAccessHandler, + createInsightsAgentAccessResolver, + createInsightsHandler, + isEnabled, +} = require('@librechat/api'); +const { requireJwtAuth } = require('~/server/middleware'); const db = require('~/models'); const router = express.Router(); -const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); -const requireInsightsAccess = requireCapability(SystemCapabilities.READ_INSIGHTS); const isInsightsEnabled = () => isEnabled(process.env.ENABLE_INSIGHTS); +const getAccessibleAgents = createInsightsAgentAccessResolver({ + getAgents: db.getAgents, + getUserPrincipals: db.getUserPrincipals, + hasCapabilityForPrincipals: db.hasCapabilityForPrincipals, + findAccessibleResources: db.findAccessibleResources, +}); -router.use(requireJwtAuth, checkAdmin, requireAdminAccess, requireInsightsAccess); -router.get('/access', createInsightsAccessHandler({ isInsightsEnabled })); -router.get('/', createInsightsHandler({ isInsightsEnabled, getInsights: db.getInsights })); +router.use(requireJwtAuth); +router.get('/access', createInsightsAccessHandler({ isInsightsEnabled, getAccessibleAgents })); +router.get( + '/', + createInsightsHandler({ isInsightsEnabled, getAccessibleAgents, getInsights: db.getInsights }), +); module.exports = router; diff --git a/api/server/services/PermissionService.js b/api/server/services/PermissionService.js index 7d166505ca..41533ee0f2 100644 --- a/api/server/services/PermissionService.js +++ b/api/server/services/PermissionService.js @@ -1,13 +1,12 @@ const mongoose = require('mongoose'); -const { isEnabled } = require('@librechat/api'); +const { AccessControlService, isEnabled } = require('@librechat/api'); const { - getTransactionSupport, tenantStorage, getTenantId, logger, runAfterTransaction, } = require('@librechat/data-schemas'); -const { ResourceType, PrincipalType, PrincipalModel } = require('librechat-data-provider'); +const { ResourceType, PrincipalType } = require('librechat-data-provider'); const { entraIdPrincipalFeatureEnabled, getUserOwnedEntraGroups, @@ -18,9 +17,6 @@ const { } = require('~/server/services/GraphApiService'); const db = require('~/models'); -/** @type {boolean|null} */ -let transactionSupportCache = null; - /** * Validates that the resourceType is one of the supported enum values * @param {string} resourceType - The resource type to validate @@ -711,256 +707,12 @@ const hasPublicPermission = async ({ resourceType, resourceId, requiredPermissio } }; -/** - * Bulk update permissions for a resource (grant, update, revoke) - * Efficiently handles multiple permission changes in a single transaction - * - * @param {Object} params - Parameters for bulk permission update - * @param {string} params.resourceType - Type of resource (e.g., 'agent') - * @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource - * @param {Array} params.updatedPrincipals - Array of principals to grant/update permissions for - * @param {Array} params.revokedPrincipals - Array of principals to revoke permissions from - * @param {string|mongoose.Types.ObjectId} params.grantedBy - User ID making the changes - * @param {mongoose.ClientSession} [params.session] - Optional MongoDB session for transactions - * @returns {Promise} Results object with granted, updated, revoked arrays and error details - */ -const bulkUpdateResourcePermissions = async ({ - resourceType, - resourceId, - updatedPrincipals = [], - revokedPrincipals = [], - grantedBy, - session, -}) => { - const supportsTransactions = await getTransactionSupport(mongoose, transactionSupportCache); - transactionSupportCache = supportsTransactions; - let localSession = session; - let shouldEndSession = false; - - try { - if (!Array.isArray(updatedPrincipals)) { - throw new Error('updatedPrincipals must be an array'); - } - - if (!Array.isArray(revokedPrincipals)) { - throw new Error('revokedPrincipals must be an array'); - } - - if (!resourceId || !mongoose.Types.ObjectId.isValid(resourceId)) { - throw new Error(`Invalid resource ID: ${resourceId}`); - } - - if (!localSession && supportsTransactions) { - localSession = await mongoose.startSession(); - localSession.startTransaction(); - shouldEndSession = true; - } - - const sessionOptions = localSession ? { session: localSession } : {}; - - const roles = await db.findRolesByResourceType(resourceType); - const rolesMap = new Map(); - roles.forEach((role) => { - rolesMap.set(role.accessRoleId, role); - }); - - const results = { - granted: [], - updated: [], - revoked: [], - errors: [], - }; - - const bulkWrites = []; - - /** - * Tracks non-public principals granted in this same request so their revoke is skipped below. - * Grants are flushed before deletes, so a principal present in both `updatedPrincipals` and - * `revokedPrincipals` would be upserted and then deleted, stripping access the caller just set - * (e.g. a resource owner landing in both lists from a client `id`/`idOnTheSource` mismatch). - * Granting wins to make owner lockout impossible regardless of the client-side diff (#14316). - * - * PUBLIC is deliberately excluded: an explicit `public: false` disable adds the public principal - * to the revoke list, and disabling public access must always win over a stale/contradictory - * grant so a resource is never left public when the caller asked to make it private. - */ - const grantedPrincipalKeys = new Set(); - const principalKey = (principal) => `${principal.type}:${principal.id}`; - - for (const principal of updatedPrincipals) { - try { - if (!principal.accessRoleId) { - results.errors.push({ - principal, - error: 'accessRoleId is required for updated principals', - }); - continue; - } - - const role = rolesMap.get(principal.accessRoleId); - if (!role) { - results.errors.push({ - principal, - error: `Role ${principal.accessRoleId} not found`, - }); - continue; - } - - const query = { - principalType: principal.type, - resourceType, - resourceId, - }; - - if (principal.type !== PrincipalType.PUBLIC) { - query.principalId = - principal.type === PrincipalType.ROLE - ? principal.id - : new mongoose.Types.ObjectId(principal.id); - } - - const principalModelMap = { - [PrincipalType.USER]: PrincipalModel.USER, - [PrincipalType.GROUP]: PrincipalModel.GROUP, - [PrincipalType.ROLE]: PrincipalModel.ROLE, - }; - - const update = { - $set: { - permBits: role.permBits, - roleId: role._id, - grantedBy, - grantedAt: new Date(), - }, - $setOnInsert: { - principalType: principal.type, - resourceType, - resourceId, - ...(principal.type !== PrincipalType.PUBLIC && { - principalId: - principal.type === PrincipalType.ROLE - ? principal.id - : new mongoose.Types.ObjectId(principal.id), - principalModel: principalModelMap[principal.type], - }), - }, - }; - - bulkWrites.push({ - updateOne: { - filter: query, - update: update, - upsert: true, - }, - }); - - results.granted.push({ - type: principal.type, - id: principal.id, - name: principal.name, - email: principal.email, - source: principal.source, - avatar: principal.avatar, - description: principal.description, - idOnTheSource: principal.idOnTheSource, - accessRoleId: principal.accessRoleId, - memberCount: principal.memberCount, - memberIds: principal.memberIds, - }); - if (principal.type !== PrincipalType.PUBLIC) { - grantedPrincipalKeys.add(principalKey(principal)); - } - } catch (error) { - results.errors.push({ - principal, - error: error.message, - }); - } - } - - if (bulkWrites.length > 0) { - await db.bulkWriteAclEntries(bulkWrites, sessionOptions); - } - - const deleteQueries = []; - for (const principal of revokedPrincipals) { - try { - // Inside the try so a malformed revoke entry (e.g. a nullish principal) is recorded in - // results.errors and skipped, rather than throwing out after grants were already flushed. - if ( - principal.type !== PrincipalType.PUBLIC && - grantedPrincipalKeys.has(principalKey(principal)) - ) { - continue; - } - const query = { - principalType: principal.type, - resourceType, - resourceId, - }; - - if (principal.type !== PrincipalType.PUBLIC) { - query.principalId = - principal.type === PrincipalType.ROLE - ? principal.id - : new mongoose.Types.ObjectId(principal.id); - } - - deleteQueries.push(query); - - results.revoked.push({ - type: principal.type, - id: principal.id, - name: principal.name, - email: principal.email, - source: principal.source, - avatar: principal.avatar, - description: principal.description, - idOnTheSource: principal.idOnTheSource, - memberCount: principal.memberCount, - }); - } catch (error) { - results.errors.push({ - principal, - error: error.message, - }); - } - } - - if (deleteQueries.length > 0) { - await db.deleteAclEntries({ $or: deleteQueries }, sessionOptions); - } - - if (shouldEndSession && supportsTransactions) { - await localSession.commitTransaction(); - } - - if (resourceType === ResourceType.PROMPTGROUP) { - /** The caller's session may still be uncommitted; defer until it commits */ - await runAfterTransaction(localSession, () => db.invalidatePromptGroupAccessContext()); - } - - return results; - } catch (error) { - if (shouldEndSession && supportsTransactions) { - try { - await localSession.abortTransaction(); - } catch (transactionError) { - /** best-effort abort; may fail if commit already succeeded */ - logger.error( - `[PermissionService.bulkUpdateResourcePermissions] Error aborting transaction:`, - transactionError, - ); - } - } - logger.error(`[PermissionService.bulkUpdateResourcePermissions] Error: ${error.message}`); - throw error; - } finally { - if (shouldEndSession && localSession) { - localSession.endSession(); - } - } -}; +/** Typed implementation; this legacy module only binds the shared model methods. */ +const accessControlService = new AccessControlService(mongoose, db); +const bulkUpdateResourcePermissions = (params) => + accessControlService.bulkUpdateResourcePermissions(params); +const restoreInsightsPermissionChanges = (params) => + accessControlService.restoreInsightsPermissionChanges(params); /** * Remove all permissions for a resource (cleanup when resource is deleted) @@ -1003,6 +755,7 @@ module.exports = { hasPublicPermission, getAvailableRoles, bulkUpdateResourcePermissions, + restoreInsightsPermissionChanges, ensurePrincipalExists, ensureGroupPrincipalExists, syncUserEntraGroupMemberships, diff --git a/api/server/services/PermissionService.spec.js b/api/server/services/PermissionService.spec.js index 7729412c91..65680249d7 100644 --- a/api/server/services/PermissionService.spec.js +++ b/api/server/services/PermissionService.spec.js @@ -6,9 +6,11 @@ const { AccessRoleIds, PrincipalType, PrincipalModel, + PermissionBits, } = require('librechat-data-provider'); const { bulkUpdateResourcePermissions, + restoreInsightsPermissionChanges, syncUserEntraGroupMemberships, getEffectivePermissions, findAccessibleResources, @@ -18,7 +20,8 @@ const { ensurePrincipalExists, ensureGroupPrincipalExists, } = require('./PermissionService'); -const { findRoleByIdentifier, getUserPrincipals, seedDefaultRoles } = require('~/models'); +const db = require('~/models'); +const { findRoleByIdentifier, getUserPrincipals, seedDefaultRoles } = db; // Mock the getTransactionSupport function for testing jest.mock('@librechat/data-schemas', () => ({ @@ -881,6 +884,442 @@ describe('PermissionService', () => { expect(groupEntry.roleId.accessRoleId).toBe(AccessRoleIds.AGENT_OWNER); }); + test('adds, preserves, and removes agent Insights access independently of the role', async () => { + const grantResult = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + + let entry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }); + expect(entry.permBits & PermissionBits.VIEW_INSIGHTS).toBe(PermissionBits.VIEW_INSIGHTS); + expect(grantResult.insightsChanges).toEqual([ + expect.objectContaining({ action: 'assigned' }), + ]); + + const roleResult = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + }, + ], + grantedBy: grantedById, + }); + entry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }); + expect(entry.permBits & PermissionBits.VIEW_INSIGHTS).toBe(PermissionBits.VIEW_INSIGHTS); + expect(roleResult.insightsChanges).toEqual([]); + + const revokeResult = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + viewInsights: false, + }, + ], + grantedBy: grantedById, + }); + entry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }); + expect(entry.permBits & PermissionBits.VIEW_INSIGHTS).toBe(0); + expect(revokeResult.insightsChanges).toEqual([ + expect.objectContaining({ action: 'removed' }), + ]); + }); + + test.each([ + { + updates: [true, false], + expectedInsights: 0, + expectedChanges: [], + }, + { + updates: [false, true], + expectedInsights: PermissionBits.VIEW_INSIGHTS, + expectedChanges: [expect.objectContaining({ action: 'assigned' })], + }, + ])( + 'uses the last Insights value for duplicate principal updates', + async ({ updates, expectedInsights, expectedChanges }) => { + const results = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: updates.map((viewInsights) => ({ + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights, + })), + grantedBy: grantedById, + }); + + const entry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }); + expect(entry.permBits & PermissionBits.VIEW_INSIGHTS).toBe(expectedInsights); + expect(results.granted).toHaveLength(1); + expect(results.insightsChanges).toEqual(expectedChanges); + }, + ); + + test.each([ + { + concurrentChange: 'grant', + initialInsights: false, + concurrentBitUpdate: { or: PermissionBits.VIEW_INSIGHTS }, + expectedInsights: PermissionBits.VIEW_INSIGHTS, + }, + { + concurrentChange: 'revocation', + initialInsights: true, + concurrentBitUpdate: { and: ~PermissionBits.VIEW_INSIGHTS }, + expectedInsights: 0, + }, + ])( + 'atomically preserves a concurrent Insights $concurrentChange during a role-only update', + async ({ initialInsights, concurrentBitUpdate, expectedInsights }) => { + if (initialInsights) { + await AclEntry.updateOne( + { + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }, + { $bit: { permBits: { or: PermissionBits.VIEW_INSIGHTS } } }, + ); + } + + const originalBulkWriteAclEntries = db.bulkWriteAclEntries; + const bulkWriteSpy = jest + .spyOn(db, 'bulkWriteAclEntries') + .mockImplementationOnce(async (...args) => { + await AclEntry.updateOne( + { + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }, + { $bit: { permBits: concurrentBitUpdate } }, + ); + return originalBulkWriteAclEntries(...args); + }); + + try { + await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + }, + ], + grantedBy: grantedById, + }); + } finally { + bulkWriteSpy.mockRestore(); + } + + const entry = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }).populate('roleId', 'accessRoleId'); + expect(entry.roleId.accessRoleId).toBe(AccessRoleIds.AGENT_EDITOR); + expect(entry.permBits & PermissionBits.VIEW_INSIGHTS).toBe(expectedInsights); + }, + ); + + test('removes a stale-read upsert after a concurrent deletion and audit failure', async () => { + const originalBulkWriteAclEntries = db.bulkWriteAclEntries; + const bulkWriteSpy = jest + .spyOn(db, 'bulkWriteAclEntries') + .mockImplementationOnce(async (...args) => { + await AclEntry.deleteOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }); + return originalBulkWriteAclEntries(...args); + }); + + let result; + try { + result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + } finally { + bulkWriteSpy.mockRestore(); + } + + expect(result.insightsChanges[0].previousEntry).toBeNull(); + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + await expect( + AclEntry.exists({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }), + ).resolves.toBeNull(); + }); + + test('restores the prior ACL document after an Insights audit failure', async () => { + const before = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }).lean(); + const result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + const restored = await AclEntry.findById(before._id).lean(); + expect(restored).toEqual(before); + }); + + test('does not overwrite or resurrect an ACL document changed after the audited update', async () => { + const result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: userId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + const change = result.insightsChanges[0]; + const ownerRole = await findRoleByIdentifier(AccessRoleIds.AGENT_OWNER); + const newerGrantedAt = new Date(change.writtenEntry.grantedAt.getTime() + 1000); + + await AclEntry.updateOne( + { _id: change.previousEntry._id }, + { + $set: { + permBits: ownerRole.permBits, + roleId: ownerRole._id, + grantedBy: otherUserId, + grantedAt: newerGrantedAt, + }, + }, + ); + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + let current = await AclEntry.findById(change.previousEntry._id).lean(); + expect(current.permBits).toBe(ownerRole.permBits); + expect(current.grantedAt).toEqual(newerGrantedAt); + + await AclEntry.deleteOne({ _id: change.previousEntry._id }); + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + current = await AclEntry.findById(change.previousEntry._id).lean(); + expect(current).toBeNull(); + }); + + test('removes a new ACL document after an Insights audit failure', async () => { + const result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: otherUserId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + await expect( + AclEntry.exists({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId, + }), + ).resolves.toBeNull(); + }); + + test('does not remove a new ACL document changed after the audited insert', async () => { + const result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + updatedPrincipals: [ + { + type: PrincipalType.USER, + id: otherUserId, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + viewInsights: true, + }, + ], + grantedBy: grantedById, + }); + const change = result.insightsChanges[0]; + const editorRole = await findRoleByIdentifier(AccessRoleIds.AGENT_EDITOR); + const newerGrantedAt = new Date(change.writtenEntry.grantedAt.getTime() + 1000); + + await AclEntry.updateOne( + { + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId, + }, + { + $set: { + permBits: editorRole.permBits, + roleId: editorRole._id, + grantedAt: newerGrantedAt, + }, + }, + ); + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + const current = await AclEntry.findOne({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId, + }).lean(); + expect(current.permBits).toBe(editorRole.permBits); + expect(current.grantedAt).toEqual(newerGrantedAt); + }); + + test('does not replace a newer ACL grant when restoring an audited deletion', async () => { + await AclEntry.updateOne( + { + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }, + { $bit: { permBits: { or: PermissionBits.VIEW_INSIGHTS } } }, + ); + const result = await bulkUpdateResourcePermissions({ + resourceType: ResourceType.AGENT, + resourceId, + revokedPrincipals: [{ type: PrincipalType.USER, id: userId }], + grantedBy: grantedById, + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: otherUserId, + }); + await restoreInsightsPermissionChanges({ + resourceType: ResourceType.AGENT, + resourceId, + changes: result.insightsChanges, + }); + + const entries = await AclEntry.find({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + resourceId, + }) + .populate('roleId', 'accessRoleId') + .lean(); + expect(entries).toHaveLength(1); + expect(entries[0].roleId.accessRoleId).toBe(AccessRoleIds.AGENT_EDITOR); + }); + test('should revoke specified permissions', async () => { const revokedPrincipals = [ { diff --git a/client/src/common/types.ts b/client/src/common/types.ts index 1249bab3a2..eb3f0199ae 100644 --- a/client/src/common/types.ts +++ b/client/src/common/types.ts @@ -110,6 +110,7 @@ export type NavLink = { Component?: React.ComponentType; onClick?: (e?: React.MouseEvent) => void; variant?: 'default' | 'ghost'; + disabled?: boolean; id: string; }; diff --git a/client/src/components/Insights/InsightsView.tsx b/client/src/components/Insights/InsightsView.tsx index bc0d18a4b9..31c80bfdda 100644 --- a/client/src/components/Insights/InsightsView.tsx +++ b/client/src/components/Insights/InsightsView.tsx @@ -1,13 +1,19 @@ import { useEffect, useId, useMemo, useRef, useState } from 'react'; -import { Navigate } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { AlertCircle, Info, Search } from 'lucide-react'; -import { Button, Input, Spinner, TooltipAnchor, useMediaQuery } from '@librechat/client'; +import { Navigate, useSearchParams } from 'react-router-dom'; +import { + Button, + Input, + MultiSelect, + Spinner, + TooltipAnchor, + useMediaQuery, +} from '@librechat/client'; import { INSIGHTS_MAX_RANGE_DAYS, INSIGHTS_SEARCH_MAX_LENGTH, INSIGHTS_SEARCH_MIN_LENGTH, - SystemRoles, } from 'librechat-data-provider'; import type { InsightsRange, @@ -17,7 +23,8 @@ import type { TInsightsUser, } from 'librechat-data-provider'; import type { TranslationKeys } from '~/hooks'; -import { useGetStartupConfig, useInsightsAccessQuery, useInsightsQuery } from '~/data-provider'; +import { clearAgentFilters, shouldRecoverAgentFilters } from './agentFilters'; +import { useGetStartupConfig, useInsightsQuery } from '~/data-provider'; import { useAuthContext, useDocumentTitle, useLocalize } from '~/hooks'; import OpenSidebar from '~/components/Chat/Menus/OpenSidebar'; import { LocalizedDateRangePicker } from '~/components/ui'; @@ -32,6 +39,7 @@ type CustomDateRange = { startDate: Date; endDate: Date }; type KpiCardData = { id: 'conversations' | 'users' | 'messages' | 'tokens'; title: string; + description?: string; value: number; sparkline: SparklinePoint[]; }; @@ -242,7 +250,23 @@ function KpiCard({ card, locale }: { card: KpiCardData; locale: string }) { const localize = useLocalize(); return ( -

{card.title}

+

+ {card.title} + {card.description && ( + +

{formatValue(card.value, locale)}
@@ -450,11 +474,12 @@ function LatestConversations({
- +
+ +
{localize('com_insights_date')} {localize('com_insights_user')}{localize('com_insights_agent')} {localize('com_insights_first_message')} {localize('com_insights_messages')} @@ -476,6 +501,7 @@ function LatestConversations({ {conversation.agentName} {conversation.firstMessage || localize('com_insights_no_message')} @@ -528,6 +554,7 @@ function LatestConversations({ export default function InsightsView() { const localize = useLocalize(); + const [urlSearchParams, setUrlSearchParams] = useSearchParams(); const { i18n } = useTranslation(); const locale = i18n.resolvedLanguage ?? i18n.language ?? 'en'; const timeZone = Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; @@ -541,13 +568,18 @@ export default function InsightsView() { const dateRangeSelectionTimeout = useRef(); const isSmallScreen = useMediaQuery('(max-width: 768px)'); const insightsFeatureEnabled = startupConfig?.insightsEnabled === true; - const shouldCheckAccess = user?.role === SystemRoles.ADMIN && insightsFeatureEnabled; - const access = useInsightsAccessQuery(user?.id, { - enabled: shouldCheckAccess, - }); - const isAllowed = insightsFeatureEnabled && access.data?.access === true; + const selectedAgentIds = useMemo( + () => [...new Set(urlSearchParams.getAll('agentIds').filter(Boolean))].sort(), + [urlSearchParams], + ); const insightsParams = useMemo(() => { - const params: TInsightsParams = { page, pageSize: 10, search, timeZone }; + const params: TInsightsParams = { + page, + pageSize: 10, + search, + timeZone, + ...(selectedAgentIds.length > 0 ? { agentIds: selectedAgentIds } : {}), + }; if (customDateRange) { return { ...params, @@ -557,13 +589,38 @@ export default function InsightsView() { }; } return { ...params, range }; - }, [customDateRange, page, range, search, timeZone]); + }, [customDateRange, page, range, search, selectedAgentIds, timeZone]); const displayDateRange = useMemo( () => customDateRange ?? getShortcutDateRange(range), [customDateRange, range], ); - const insights = useInsightsQuery(insightsParams, { enabled: isAllowed }); + const insights = useInsightsQuery(insightsParams, { + enabled: !!user && insightsFeatureEnabled, + retry: false, + }); const data = insights.data; + const insightsStatus = responseStatus(insights.error); + const isRecoveringAgentFilters = shouldRecoverAgentFilters(insightsStatus, selectedAgentIds); + const agentItems = useMemo(() => { + if (!data) { + return []; + } + const duplicateNames = new Map(); + for (const agent of data.agents) { + duplicateNames.set(agent.name, (duplicateNames.get(agent.name) ?? 0) + 1); + } + return data.agents.map((agent) => ({ + value: agent.id, + label: + (duplicateNames.get(agent.name) ?? 0) > 1 + ? `${agent.name} (${agent.id.slice(-6)})` + : agent.name, + })); + }, [data]); + const effectiveAgentIds = useMemo( + () => (selectedAgentIds.length > 0 ? selectedAgentIds : agentItems.map((agent) => agent.value)), + [agentItems, selectedAgentIds], + ); useDocumentTitle(`${localize('com_insights_title')} | LibreChat`); @@ -589,6 +646,14 @@ export default function InsightsView() { return () => window.clearTimeout(timeout); }, [search, searchInput]); + useEffect(() => { + if (!isRecoveringAgentFilters) { + return; + } + setUrlSearchParams(clearAgentFilters(urlSearchParams), { replace: true }); + setPage(1); + }, [isRecoveringAgentFilters, setUrlSearchParams, urlSearchParams]); + const kpiCards = useMemo(() => { if (!data) { return []; @@ -597,6 +662,7 @@ export default function InsightsView() { { id: 'conversations', title: localize('com_insights_total_conversations'), + description: localize('com_insights_conversations_definition'), value: data.summary.totalConversations, sparkline: data.daily.map((row) => ({ date: row.date, value: row.conversations })), }, @@ -632,25 +698,37 @@ export default function InsightsView() { }, dateRangeSelectionDelayMs); }; - if (configLoading || (shouldCheckAccess && access.isLoading)) { + const handleAgentSelection = (agentIds: string[]) => { + if (agentIds.length === 0 || !data) { + return; + } + const normalizedIds = [...new Set(agentIds)].sort(); + const allAgentIds = data.agents.map((agent) => agent.id).sort(); + const nextParams = new URLSearchParams(urlSearchParams); + nextParams.delete('agentIds'); + if ( + normalizedIds.length !== allAgentIds.length || + normalizedIds.some((agentId, index) => agentId !== allAgentIds[index]) + ) { + for (const agentId of normalizedIds) { + nextParams.append('agentIds', agentId); + } + } + setUrlSearchParams(nextParams, { replace: true }); + setPage(1); + }; + + if ( + configLoading || + (insightsFeatureEnabled && (insights.isLoading || isRecoveringAgentFilters)) + ) { return (
); } - const accessStatus = responseStatus(access.error); - if (access.isError && accessStatus !== 403 && accessStatus !== 404) { - return ( -
- - - {localize('com_insights_load_error')} - -
- ); - } - if (!isAllowed) { + if (!insightsFeatureEnabled) { return ; } @@ -662,6 +740,42 @@ export default function InsightsView() {

{localize('com_insights_title')}

+ {data && ( + { + if (values.length === agentItems.length) { + return localize('com_insights_all_agents', { count: agentItems.length }); + } + if (values.length === 1) { + return agentItems.find((agent) => agent.value === values[0])?.label ?? values[0]; + } + return localize('com_insights_agents_selected', { count: values.length }); + }} + popoverHeader={ +
+ +
+ } + /> + )}
{ranges.map((item) => ( + } + /> +
+ )} {lockOwner ? ( {localize('com_ui_role_owner')} @@ -89,7 +128,7 @@ export default function SelectedPrincipalsList({ resourceType={resourceType} selectedRoleId={share.accessRoleId} onRoleChange={(newRole) => { - onRoleChange?.(share.idOnTheSource!, newRole); + onRoleChange?.(shareKey, newRole); }} className="min-w-0" /> @@ -98,7 +137,7 @@ export default function SelectedPrincipalsList({ {!lockOwner && (