From 49d1cefe71da712a526d4e38e5156c8a41eade7f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 2 Aug 2025 16:02:56 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20refactor:=20Add=20and=20use=20Pr?= =?UTF-8?q?incipalType=20Enum?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replaced string literals for principal types ('user', 'group', 'public') with the new PrincipalType enum across various models, services, and tests for improved type safety and consistency. - Updated permission handling in multiple files to utilize the PrincipalType enum, enhancing maintainability and reducing potential errors. - Ensured all relevant tests reflect these changes to maintain coverage and functionality. --- api/models/Agent.spec.js | 4 +- api/models/File.spec.js | 8 +- api/models/Prompt.spec.js | 17 ++- api/models/PromptGroupMigration.spec.js | 11 +- .../controllers/PermissionsController.js | 12 +- api/server/controllers/agents/v1.js | 7 +- .../canAccessAgentResource.spec.js | 16 +- api/server/routes/files/files.agents.test.js | 8 +- api/server/routes/files/files.test.js | 10 +- api/server/routes/prompts.js | 5 +- api/server/routes/prompts.test.js | 17 ++- .../services/Files/processFiles.test.js | 5 + api/server/services/PermissionService.js | 10 +- api/server/services/PermissionService.spec.js | 144 ++++++++++-------- config/migrate-agent-permissions.js | 10 +- config/migrate-prompt-permissions.js | 10 +- .../data-provider/src/accessPermissions.ts | 10 +- .../data-schemas/src/methods/aclEntry.spec.ts | 121 ++++++++------- packages/data-schemas/src/methods/aclEntry.ts | 15 +- .../src/methods/userGroup.spec.ts | 13 +- .../data-schemas/src/methods/userGroup.ts | 9 +- packages/data-schemas/src/schema/aclEntry.ts | 7 +- packages/data-schemas/src/types/aclEntry.ts | 3 +- 23 files changed, 253 insertions(+), 219 deletions(-) diff --git a/api/models/Agent.spec.js b/api/models/Agent.spec.js index 0c67d9238..65a31c49a 100644 --- a/api/models/Agent.spec.js +++ b/api/models/Agent.spec.js @@ -14,7 +14,7 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); const { agentSchema } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { getAgent, loadAgent, @@ -500,7 +500,7 @@ describe('models/Agent', () => { // Grant permissions (simulating sharing) await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: authorId, resourceType: ResourceType.AGENT, resourceId: agent._id, diff --git a/api/models/File.spec.js b/api/models/File.spec.js index 30de61bab..a07d3721a 100644 --- a/api/models/File.spec.js +++ b/api/models/File.spec.js @@ -2,7 +2,7 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); const { createModels } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { grantPermission } = require('~/server/services/PermissionService'); const { getFiles, createFile } = require('./File'); const { seedDefaultRoles } = require('~/models'); @@ -115,7 +115,7 @@ describe('File Access Control', () => { // Grant EDIT permission to user on the agent await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -232,7 +232,7 @@ describe('File Access Control', () => { // Grant only VIEW permission to user on the agent await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -290,7 +290,7 @@ describe('File Access Control', () => { // Grant EDIT permission to user on the agent await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: agent._id, diff --git a/api/models/Prompt.spec.js b/api/models/Prompt.spec.js index d7e8da575..b3265bb34 100644 --- a/api/models/Prompt.spec.js +++ b/api/models/Prompt.spec.js @@ -6,6 +6,7 @@ const { SystemRoles, ResourceType, AccessRoleIds, + PrincipalType, PermissionBits, } = require('librechat-data-provider'); @@ -151,7 +152,7 @@ describe('Prompt ACL Permissions', () => { // Manually grant permissions as would happen in the route await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -163,7 +164,7 @@ describe('Prompt ACL Permissions', () => { const aclEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, }); @@ -195,7 +196,7 @@ describe('Prompt ACL Permissions', () => { // Grant owner permissions await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testPromptGroup._id, @@ -233,7 +234,7 @@ describe('Prompt ACL Permissions', () => { it('user with viewer role should only have view access', async () => { // Grant viewer permissions await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.viewer._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testPromptGroup._id, @@ -355,7 +356,7 @@ describe('Prompt ACL Permissions', () => { // Grant edit permissions to the group await permissionService.grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: testGroups.editors._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testPromptGroup._id, @@ -423,7 +424,7 @@ describe('Prompt ACL Permissions', () => { // Grant public view access to publicPromptGroup await permissionService.grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.PROMPTGROUP, resourceId: publicPromptGroup._id, @@ -433,7 +434,7 @@ describe('Prompt ACL Permissions', () => { // Grant only owner access to privatePromptGroup await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: privatePromptGroup._id, @@ -504,7 +505,7 @@ describe('Prompt ACL Permissions', () => { // Grant permission await permissionService.grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testPromptGroup._id, diff --git a/api/models/PromptGroupMigration.spec.js b/api/models/PromptGroupMigration.spec.js index 3cb3d62e1..4214b684d 100644 --- a/api/models/PromptGroupMigration.spec.js +++ b/api/models/PromptGroupMigration.spec.js @@ -6,6 +6,7 @@ const { Constants, ResourceType, AccessRoleIds, + PrincipalType, PermissionBits, } = require('librechat-data-provider'); @@ -158,7 +159,7 @@ describe('PromptGroup Migration Script', () => { const globalOwnerEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: globalPromptGroup._id, - principalType: 'user', + principalType: PrincipalType.USER, principalId: testOwner._id, }); expect(globalOwnerEntry).toBeTruthy(); @@ -167,7 +168,7 @@ describe('PromptGroup Migration Script', () => { const globalPublicEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: globalPromptGroup._id, - principalType: 'public', + principalType: PrincipalType.PUBLIC, }); expect(globalPublicEntry).toBeTruthy(); expect(globalPublicEntry.permBits).toBe(viewerRole.permBits); @@ -176,7 +177,7 @@ describe('PromptGroup Migration Script', () => { const privateOwnerEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: privatePromptGroup._id, - principalType: 'user', + principalType: PrincipalType.USER, principalId: testOwner._id, }); expect(privateOwnerEntry).toBeTruthy(); @@ -185,7 +186,7 @@ describe('PromptGroup Migration Script', () => { const privatePublicEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: privatePromptGroup._id, - principalType: 'public', + principalType: PrincipalType.PUBLIC, }); expect(privatePublicEntry).toBeNull(); }); @@ -208,7 +209,7 @@ describe('PromptGroup Migration Script', () => { // Grant permission to one promptGroup manually (simulating it already has ACL) await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testOwner._id, principalModel: 'User', resourceType: ResourceType.PROMPTGROUP, diff --git a/api/server/controllers/PermissionsController.js b/api/server/controllers/PermissionsController.js index 1dfadeecd..7562beb10 100644 --- a/api/server/controllers/PermissionsController.js +++ b/api/server/controllers/PermissionsController.js @@ -4,7 +4,7 @@ const mongoose = require('mongoose'); const { logger } = require('@librechat/data-schemas'); -const { ResourceType } = require('librechat-data-provider'); +const { ResourceType, PrincipalType } = require('librechat-data-provider'); const { bulkUpdateResourcePermissions, ensureGroupPrincipalExists, @@ -235,14 +235,14 @@ const getResourcePermissions = async (req, res) => { // Process aggregation results for (const result of results) { - if (result.principalType === 'public') { + if (result.principalType === PrincipalType.PUBLIC) { publicPermission = { public: true, publicAccessRoleId: result.accessRoleId, }; - } else if (result.principalType === 'user' && result.userInfo) { + } else if (result.principalType === PrincipalType.USER && result.userInfo) { principals.push({ - type: 'user', + type: PrincipalType.USER, id: result.userInfo._id.toString(), name: result.userInfo.name || result.userInfo.username, email: result.userInfo.email, @@ -251,9 +251,9 @@ const getResourcePermissions = async (req, res) => { idOnTheSource: result.userInfo.idOnTheSource || result.userInfo._id.toString(), accessRoleId: result.accessRoleId, }); - } else if (result.principalType === 'group' && result.groupInfo) { + } else if (result.principalType === PrincipalType.GROUP && result.groupInfo) { principals.push({ - type: 'group', + type: PrincipalType.GROUP, id: result.groupInfo._id.toString(), name: result.groupInfo.name, email: result.groupInfo.email, diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index acac6298c..7fae5ac78 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -9,9 +9,10 @@ const { FileSources, ResourceType, AccessRoleIds, + PrincipalType, EToolResources, - actionDelimiter, PermissionBits, + actionDelimiter, removeNullishValues, } = require('librechat-data-provider'); const { @@ -80,7 +81,7 @@ const createAgentHandler = async (req, res) => { // Automatically grant owner permissions to the creator try { await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -346,7 +347,7 @@ const duplicateAgentHandler = async (req, res) => { // Automatically grant owner permissions to the duplicator try { await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: newAgent._id, diff --git a/api/server/middleware/accessResources/canAccessAgentResource.spec.js b/api/server/middleware/accessResources/canAccessAgentResource.spec.js index 4656f9735..2f6d5b0ed 100644 --- a/api/server/middleware/accessResources/canAccessAgentResource.spec.js +++ b/api/server/middleware/accessResources/canAccessAgentResource.spec.js @@ -1,5 +1,5 @@ const mongoose = require('mongoose'); -const { ResourceType } = require('librechat-data-provider'); +const { ResourceType, PrincipalType } = require('librechat-data-provider'); const { MongoMemoryServer } = require('mongodb-memory-server'); const { canAccessAgentResource } = require('./canAccessAgentResource'); const { User, Role, AclEntry } = require('~/db/models'); @@ -97,7 +97,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry for the author (owner permissions) await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -134,7 +134,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry for the other user (owner) await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -175,7 +175,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry granting view permission to test user await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -212,7 +212,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry granting only view permission await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -259,7 +259,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry for the author await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -295,7 +295,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry with all permissions for the owner await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -355,7 +355,7 @@ describe('canAccessAgentResource middleware', () => { // Create ACL entry for the author await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUser._id, principalModel: 'User', resourceType: ResourceType.AGENT, diff --git a/api/server/routes/files/files.agents.test.js b/api/server/routes/files/files.agents.test.js index 25e298aa5..fbccf62a2 100644 --- a/api/server/routes/files/files.agents.test.js +++ b/api/server/routes/files/files.agents.test.js @@ -2,9 +2,9 @@ const express = require('express'); const request = require('supertest'); const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); -const { MongoMemoryServer } = require('mongodb-memory-server'); const { createMethods } = require('@librechat/data-schemas'); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { MongoMemoryServer } = require('mongodb-memory-server'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { createAgent } = require('~/models/Agent'); const { createFile } = require('~/models/File'); @@ -185,7 +185,7 @@ describe('File Routes - Agent Files Endpoint', () => { // Grant EDIT permission to user on the agent using PermissionService const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -240,7 +240,7 @@ describe('File Routes - Agent Files Endpoint', () => { // Grant only VIEW permission to user on the agent const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index 350bb3281..fbbd28959 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -4,7 +4,7 @@ const mongoose = require('mongoose'); const { v4: uuidv4 } = require('uuid'); const { createMethods } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { createAgent } = require('~/models/Agent'); const { createFile } = require('~/models/File'); @@ -227,7 +227,7 @@ describe('File Routes - Delete with Agent Access', () => { // Grant EDIT permission to user on the agent const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -281,7 +281,7 @@ describe('File Routes - Delete with Agent Access', () => { // Grant EDIT permission to user on the agent const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -347,7 +347,7 @@ describe('File Routes - Delete with Agent Access', () => { // Grant EDIT permission to user on the agent const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -390,7 +390,7 @@ describe('File Routes - Delete with Agent Access', () => { // Grant only VIEW permission to user on the agent const { grantPermission } = require('~/server/services/PermissionService'); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: otherUserId, resourceType: ResourceType.AGENT, resourceId: agent._id, diff --git a/api/server/routes/prompts.js b/api/server/routes/prompts.js index ec14bfd14..b5e5ea503 100644 --- a/api/server/routes/prompts.js +++ b/api/server/routes/prompts.js @@ -6,8 +6,9 @@ const { SystemRoles, ResourceType, AccessRoleIds, - PermissionTypes, + PrincipalType, PermissionBits, + PermissionTypes, } = require('librechat-data-provider'); const { makePromptProduction, @@ -189,7 +190,7 @@ const createNewPromptGroup = async (req, res) => { if (result.prompt && result.prompt._id && result.prompt.groupId) { try { await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: req.user.id, resourceType: ResourceType.PROMPTGROUP, resourceId: result.prompt.groupId, diff --git a/api/server/routes/prompts.test.js b/api/server/routes/prompts.test.js index 5654f64fd..812320fd6 100644 --- a/api/server/routes/prompts.test.js +++ b/api/server/routes/prompts.test.js @@ -7,6 +7,7 @@ const { SystemRoles, ResourceType, AccessRoleIds, + PrincipalType, PermissionBits, } = require('librechat-data-provider'); @@ -223,7 +224,7 @@ describe('Prompt Routes - ACL Permissions', () => { const aclEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: response.body.prompt.groupId, - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, }); @@ -253,7 +254,7 @@ describe('Prompt Routes - ACL Permissions', () => { const aclEntry = await AclEntry.findOne({ resourceType: ResourceType.PROMPTGROUP, resourceId: response.body.group._id, - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, }); @@ -294,7 +295,7 @@ describe('Prompt Routes - ACL Permissions', () => { it('should retrieve prompt when user has view permissions', async () => { // Grant view permissions on the promptGroup await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -379,7 +380,7 @@ describe('Prompt Routes - ACL Permissions', () => { // Grant owner permissions on the promptGroup await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -426,7 +427,7 @@ describe('Prompt Routes - ACL Permissions', () => { // Grant only viewer permissions to viewer user on the promptGroup await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.viewer._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -493,7 +494,7 @@ describe('Prompt Routes - ACL Permissions', () => { it('should make prompt production when user has edit permissions', async () => { // Grant edit permissions on the promptGroup await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.owner._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -531,7 +532,7 @@ describe('Prompt Routes - ACL Permissions', () => { it('should deny making production when user lacks edit permissions', async () => { // Grant only view permissions to viewer on the promptGroup await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: testUsers.viewer._id, resourceType: ResourceType.PROMPTGROUP, resourceId: testGroup._id, @@ -588,7 +589,7 @@ describe('Prompt Routes - ACL Permissions', () => { // Grant public viewer access on the promptGroup await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.PROMPTGROUP, resourceId: publicGroup._id, diff --git a/api/server/services/Files/processFiles.test.js b/api/server/services/Files/processFiles.test.js index 060af22fd..5da8b862d 100644 --- a/api/server/services/Files/processFiles.test.js +++ b/api/server/services/Files/processFiles.test.js @@ -17,6 +17,11 @@ jest.mock('~/config', () => ({ jest.mock('librechat-data-provider', () => ({ isUUID: { parse: jest.fn() }, megabyte: 1024 * 1024, + PrincipalType: { + USER: 'user', + GROUP: 'group', + PUBLIC: 'public', + }, FileContext: { message_attachment: 'message_attachment' }, FileSources: { local: 'local' }, EModelEndpoint: { assistants: 'assistants' }, diff --git a/api/server/services/PermissionService.js b/api/server/services/PermissionService.js index 31e91dc4d..5fd068d13 100644 --- a/api/server/services/PermissionService.js +++ b/api/server/services/PermissionService.js @@ -1,6 +1,6 @@ const mongoose = require('mongoose'); const { isEnabled } = require('@librechat/api'); -const { ResourceType } = require('librechat-data-provider'); +const { ResourceType, PrincipalType } = require('librechat-data-provider'); const { getTransactionSupport, logger } = require('@librechat/data-schemas'); const { entraIdPrincipalFeatureEnabled, @@ -65,11 +65,11 @@ const grantPermission = async ({ session, }) => { try { - if (!['user', 'group', 'public'].includes(principalType)) { + if (!Object.values(PrincipalType).includes(principalType)) { throw new Error(`Invalid principal type: ${principalType}`); } - if (principalType !== 'public' && !principalId) { + if (principalType !== PrincipalType.PUBLIC && !principalId) { throw new Error('Principal ID is required for user and group principals'); } @@ -221,7 +221,7 @@ const findPubliclyAccessibleResources = async ({ resourceType, requiredPermissio // Find all public ACL entries where the public principal has at least the required permission bits const entries = await AclEntry.find({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, resourceType, permBits: { $bitsAllSet: requiredPermissions }, }).distinct('resourceId'); @@ -505,7 +505,7 @@ const hasPublicPermission = async ({ resourceType, resourceId, requiredPermissio validateResourceType(resourceType); // Use public principal to check permissions - const publicPrincipal = [{ principalType: 'public' }]; + const publicPrincipal = [{ principalType: PrincipalType.PUBLIC }]; const entries = await findEntriesByPrincipalsAndResource( publicPrincipal, diff --git a/api/server/services/PermissionService.spec.js b/api/server/services/PermissionService.spec.js index 53a0e5e4a..93e53d4e6 100644 --- a/api/server/services/PermissionService.spec.js +++ b/api/server/services/PermissionService.spec.js @@ -1,7 +1,7 @@ const mongoose = require('mongoose'); const { RoleBits, createModels } = require('@librechat/data-schemas'); const { MongoMemoryServer } = require('mongodb-memory-server'); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { bulkUpdateResourcePermissions, getEffectivePermissions, @@ -78,7 +78,7 @@ describe('PermissionService', () => { describe('grantPermission', () => { test('should grant permission to a user with a role', async () => { const entry = await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -103,7 +103,7 @@ describe('PermissionService', () => { test('should grant permission to a group with a role', async () => { const entry = await grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId, @@ -112,7 +112,7 @@ describe('PermissionService', () => { }); expect(entry).toBeDefined(); - expect(entry.principalType).toBe('group'); + expect(entry.principalType).toBe(PrincipalType.GROUP); expect(entry.principalId.toString()).toBe(groupId.toString()); expect(entry.principalModel).toBe('Group'); @@ -124,7 +124,7 @@ describe('PermissionService', () => { test('should grant public permission with a role', async () => { const entry = await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.AGENT, resourceId, @@ -133,7 +133,7 @@ describe('PermissionService', () => { }); expect(entry).toBeDefined(); - expect(entry.principalType).toBe('public'); + expect(entry.principalType).toBe(PrincipalType.PUBLIC); expect(entry.principalId).toBeUndefined(); expect(entry.principalModel).toBeUndefined(); @@ -159,7 +159,7 @@ describe('PermissionService', () => { test('should throw error for missing principalId with user type', async () => { await expect( grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: null, resourceType: ResourceType.AGENT, resourceId, @@ -172,7 +172,7 @@ describe('PermissionService', () => { test('should throw error for non-existent role', async () => { await expect( grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -185,7 +185,7 @@ describe('PermissionService', () => { test('should throw error for role-resource type mismatch', async () => { await expect( grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -198,7 +198,7 @@ describe('PermissionService', () => { test('should update existing permission when granting to same principal and resource', async () => { // First grant with viewer role await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -208,7 +208,7 @@ describe('PermissionService', () => { // Then update to editor role const updated = await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -222,7 +222,7 @@ describe('PermissionService', () => { // Verify there's only one entry const entries = await AclEntry.find({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -240,7 +240,7 @@ describe('PermissionService', () => { // Setup test data await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -250,7 +250,7 @@ describe('PermissionService', () => { otherResourceId = new mongoose.Types.ObjectId(); await grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId: otherResourceId, @@ -261,7 +261,9 @@ describe('PermissionService', () => { test('should check permission for user principal', async () => { // Mock getUserPrincipals to return just the user principal - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); const hasViewPermission = await checkPermission({ userId, @@ -286,8 +288,8 @@ describe('PermissionService', () => { test('should check permission for user and group principals', async () => { // Mock getUserPrincipals to return both user and group principals getUserPrincipals.mockResolvedValue([ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]); // Check original resource (user has access) @@ -317,7 +319,7 @@ describe('PermissionService', () => { // Grant public access to a resource await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.AGENT, resourceId: publicResourceId, @@ -327,9 +329,9 @@ describe('PermissionService', () => { // Mock getUserPrincipals to return user, group, and public principals getUserPrincipals.mockResolvedValue([ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, - { principalType: 'public' }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, + { principalType: PrincipalType.PUBLIC }, ]); const hasPublicAccess = await checkPermission({ @@ -343,7 +345,9 @@ describe('PermissionService', () => { }); test('should return false for invalid permission bits', async () => { - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); await expect( checkPermission({ @@ -385,7 +389,7 @@ describe('PermissionService', () => { // Setup test data with multiple permissions from different sources await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -394,7 +398,7 @@ describe('PermissionService', () => { }); await grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId, @@ -405,7 +409,7 @@ describe('PermissionService', () => { // Create another resource with public permission const publicResourceId = new mongoose.Types.ObjectId(); await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.AGENT, resourceId: publicResourceId, @@ -418,7 +422,7 @@ describe('PermissionService', () => { const childResourceId = new mongoose.Types.ObjectId(); await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.PROMPTGROUP, resourceId: parentResourceId, @@ -427,7 +431,7 @@ describe('PermissionService', () => { }); await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -443,8 +447,8 @@ describe('PermissionService', () => { test('should get effective permissions from multiple sources', async () => { // Mock getUserPrincipals to return both user and group principals getUserPrincipals.mockResolvedValue([ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]); const effective = await getEffectivePermissions({ @@ -464,7 +468,9 @@ describe('PermissionService', () => { const childResourceId = inheritedEntry.resourceId; // Mock getUserPrincipals to return user principal - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); const effective = await getEffectivePermissions({ userId, @@ -516,7 +522,7 @@ describe('PermissionService', () => { // User can view resource 1 await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: resource1, @@ -526,7 +532,7 @@ describe('PermissionService', () => { // User can edit resource 2 await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId: resource2, @@ -536,7 +542,7 @@ describe('PermissionService', () => { // Group can view resource 3 await grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId: resource3, @@ -547,7 +553,9 @@ describe('PermissionService', () => { test('should find resources user can view', async () => { // Mock getUserPrincipals to return user principal - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); const viewableResources = await findAccessibleResources({ userId, @@ -561,7 +569,9 @@ describe('PermissionService', () => { test('should find resources user can edit', async () => { // Mock getUserPrincipals to return user principal - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); const editableResources = await findAccessibleResources({ userId, @@ -576,8 +586,8 @@ describe('PermissionService', () => { test('should find resources accessible via group membership', async () => { // Mock getUserPrincipals to return user and group principals getUserPrincipals.mockResolvedValue([ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]); const viewableResources = await findAccessibleResources({ @@ -591,7 +601,9 @@ describe('PermissionService', () => { }); test('should return empty array for invalid permissions', async () => { - getUserPrincipals.mockResolvedValue([{ principalType: 'user', principalId: userId }]); + getUserPrincipals.mockResolvedValue([ + { principalType: PrincipalType.USER, principalId: userId }, + ]); await expect( findAccessibleResources({ @@ -652,7 +664,7 @@ describe('PermissionService', () => { await seedDefaultRoles(); // Setup existing permissions for testing await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -661,7 +673,7 @@ describe('PermissionService', () => { }); await grantPermission({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId, @@ -670,7 +682,7 @@ describe('PermissionService', () => { }); await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.AGENT, resourceId, @@ -683,17 +695,17 @@ describe('PermissionService', () => { const newResourceId = new mongoose.Types.ObjectId(); const updatedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, accessRoleId: AccessRoleIds.AGENT_VIEWER, }, { - type: 'user', + type: PrincipalType.USER, id: otherUserId, accessRoleId: AccessRoleIds.AGENT_EDITOR, }, { - type: 'group', + type: PrincipalType.GROUP, id: groupId, accessRoleId: AccessRoleIds.AGENT_OWNER, }, @@ -722,17 +734,17 @@ describe('PermissionService', () => { test('should update existing permissions in bulk', async () => { const updatedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, accessRoleId: AccessRoleIds.AGENT_EDITOR, // Upgrade from viewer to editor }, { - type: 'group', + type: PrincipalType.GROUP, id: groupId, accessRoleId: AccessRoleIds.AGENT_OWNER, // Upgrade from editor to owner }, { - type: 'public', + type: PrincipalType.PUBLIC, accessRoleId: AccessRoleIds.AGENT_VIEWER, // Keep same role }, ]; @@ -752,7 +764,7 @@ describe('PermissionService', () => { // Verify updates const userEntry = await AclEntry.findOne({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, resourceType: ResourceType.AGENT, resourceId, @@ -760,7 +772,7 @@ describe('PermissionService', () => { expect(userEntry.roleId.accessRoleId).toBe(AccessRoleIds.AGENT_EDITOR); const groupEntry = await AclEntry.findOne({ - principalType: 'group', + principalType: PrincipalType.GROUP, principalId: groupId, resourceType: ResourceType.AGENT, resourceId, @@ -771,11 +783,11 @@ describe('PermissionService', () => { test('should revoke specified permissions', async () => { const revokedPrincipals = [ { - type: 'group', + type: PrincipalType.GROUP, id: groupId, }, { - type: 'public', + type: PrincipalType.PUBLIC, }, ]; @@ -797,19 +809,19 @@ describe('PermissionService', () => { resourceId, }); expect(remainingEntries).toHaveLength(1); - expect(remainingEntries[0].principalType).toBe('user'); + expect(remainingEntries[0].principalType).toBe(PrincipalType.USER); expect(remainingEntries[0].principalId.toString()).toBe(userId.toString()); }); test('should handle mixed operations (grant, update, revoke)', async () => { const updatedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, accessRoleId: AccessRoleIds.AGENT_OWNER, // Update existing }, { - type: 'user', + type: PrincipalType.USER, id: otherUserId, accessRoleId: AccessRoleIds.AGENT_VIEWER, // New permission }, @@ -817,11 +829,11 @@ describe('PermissionService', () => { const revokedPrincipals = [ { - type: 'group', + type: PrincipalType.GROUP, id: groupId, }, { - type: 'public', + type: PrincipalType.PUBLIC, }, ]; @@ -858,17 +870,17 @@ describe('PermissionService', () => { test('should handle errors for invalid roles gracefully', async () => { const updatedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, accessRoleId: AccessRoleIds.AGENT_VIEWER, // Valid }, { - type: 'user', + type: PrincipalType.USER, id: otherUserId, accessRoleId: 'non_existent_role', // Invalid }, { - type: 'group', + type: PrincipalType.GROUP, id: groupId, accessRoleId: AccessRoleIds.PROMPTGROUP_VIEWER, // Wrong resource type }, @@ -938,11 +950,11 @@ describe('PermissionService', () => { test('should handle public permissions correctly', async () => { const updatedPrincipals = [ { - type: 'public', + type: PrincipalType.PUBLIC, accessRoleId: AccessRoleIds.AGENT_EDITOR, // Update public permission }, { - type: 'user', + type: PrincipalType.USER, id: otherUserId, accessRoleId: AccessRoleIds.AGENT_VIEWER, // New user permission }, @@ -950,11 +962,11 @@ describe('PermissionService', () => { const revokedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, }, { - type: 'group', + type: PrincipalType.GROUP, id: groupId, }, ]; @@ -974,7 +986,7 @@ describe('PermissionService', () => { // Verify public permission was updated const publicEntry = await AclEntry.findOne({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, resourceType: ResourceType.AGENT, resourceId, }).populate('roleId', 'accessRoleId'); @@ -988,12 +1000,12 @@ describe('PermissionService', () => { const promptGroupResourceId = new mongoose.Types.ObjectId(); const updatedPrincipals = [ { - type: 'user', + type: PrincipalType.USER, id: userId, accessRoleId: AccessRoleIds.PROMPTGROUP_VIEWER, }, { - type: 'group', + type: PrincipalType.GROUP, id: groupId, accessRoleId: AccessRoleIds.PROMPTGROUP_EDITOR, }, diff --git a/config/migrate-agent-permissions.js b/config/migrate-agent-permissions.js index fffb39f2b..cf23bf5f5 100644 --- a/config/migrate-agent-permissions.js +++ b/config/migrate-agent-permissions.js @@ -4,7 +4,7 @@ const path = require('path'); const { logger } = require('@librechat/data-schemas'); require('module-alias')({ base: path.resolve(__dirname, '..', 'api') }); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { GLOBAL_PROJECT_NAME } = require('librechat-data-provider').Constants; const connect = require('./connect'); @@ -51,8 +51,8 @@ async function migrateAgentPermissionsEnhanced({ dryRun = true, batchSize = 100 as: 'aclEntry', cond: { $and: [ - { $eq: ['$$aclEntry.resourceType', 'agent'] }, - { $eq: ['$$aclEntry.principalType', 'user'] }, + { $eq: ['$$aclEntry.resourceType', ResourceType.AGENT] }, + { $eq: ['$$aclEntry.principalType', PrincipalType.USER] }, ], }, }, @@ -163,7 +163,7 @@ async function migrateAgentPermissionsEnhanced({ dryRun = true, batchSize = 100 // Always grant owner permission to author await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: agent.author, resourceType: ResourceType.AGENT, resourceId: agent._id, @@ -191,7 +191,7 @@ async function migrateAgentPermissionsEnhanced({ dryRun = true, batchSize = 100 // Grant public permission await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.AGENT, resourceId: agent._id, diff --git a/config/migrate-prompt-permissions.js b/config/migrate-prompt-permissions.js index 07c633d97..c16406ce4 100644 --- a/config/migrate-prompt-permissions.js +++ b/config/migrate-prompt-permissions.js @@ -2,7 +2,7 @@ const path = require('path'); const { logger } = require('@librechat/data-schemas'); require('module-alias')({ base: path.resolve(__dirname, '..', 'api') }); -const { AccessRoleIds, ResourceType } = require('librechat-data-provider'); +const { AccessRoleIds, ResourceType, PrincipalType } = require('librechat-data-provider'); const { GLOBAL_PROJECT_NAME } = require('librechat-data-provider').Constants; const connect = require('./connect'); @@ -51,8 +51,8 @@ async function migrateToPromptGroupPermissions({ dryRun = true, batchSize = 100 as: 'aclEntry', cond: { $and: [ - { $eq: ['$$aclEntry.resourceType', 'promptGroup'] }, - { $eq: ['$$aclEntry.principalType', 'user'] }, + { $eq: ['$$aclEntry.resourceType', ResourceType.PROMPTGROUP] }, + { $eq: ['$$aclEntry.principalType', PrincipalType.USER] }, ], }, }, @@ -145,7 +145,7 @@ async function migrateToPromptGroupPermissions({ dryRun = true, batchSize = 100 // Always grant owner permission to author await grantPermission({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: group.author, resourceType: ResourceType.PROMPTGROUP, resourceId: group._id, @@ -157,7 +157,7 @@ async function migrateToPromptGroupPermissions({ dryRun = true, batchSize = 100 // Grant public view permissions for promptGroups in global project if (isGlobalGroup) { await grantPermission({ - principalType: 'public', + principalType: PrincipalType.PUBLIC, principalId: null, resourceType: ResourceType.PROMPTGROUP, resourceId: group._id, diff --git a/packages/data-provider/src/accessPermissions.ts b/packages/data-provider/src/accessPermissions.ts index 566cdce64..4b6961a8e 100644 --- a/packages/data-provider/src/accessPermissions.ts +++ b/packages/data-provider/src/accessPermissions.ts @@ -13,7 +13,11 @@ import { z } from 'zod'; /** * Principal types for permission system */ -export type TPrincipalType = 'user' | 'group' | 'public'; +export enum PrincipalType { + USER = 'user', + GROUP = 'group', + PUBLIC = 'public', +} /** * Source of the principal (local LibreChat or external Entra ID) @@ -65,7 +69,7 @@ export enum AccessRoleIds { * Principal schema - represents a user, group, or public access */ export const principalSchema = z.object({ - type: z.enum(['user', 'group', 'public']), + type: z.nativeEnum(PrincipalType), id: z.string().optional(), // undefined for 'public' type name: z.string().optional(), email: z.string().optional(), // for user and group types @@ -93,7 +97,7 @@ export const accessRoleSchema = z.object({ */ export const permissionEntrySchema = z.object({ id: z.string(), - principalType: z.enum(['user', 'group', 'public']), + principalType: z.nativeEnum(PrincipalType), principalId: z.string().optional(), // undefined for 'public' principalName: z.string().optional(), role: accessRoleSchema, diff --git a/packages/data-schemas/src/methods/aclEntry.spec.ts b/packages/data-schemas/src/methods/aclEntry.spec.ts index 418642270..25bb215b3 100644 --- a/packages/data-schemas/src/methods/aclEntry.spec.ts +++ b/packages/data-schemas/src/methods/aclEntry.spec.ts @@ -1,5 +1,5 @@ import mongoose from 'mongoose'; -import { ResourceType, PermissionBits } from 'librechat-data-provider'; +import { ResourceType, PermissionBits, PrincipalType } from 'librechat-data-provider'; import { MongoMemoryServer } from 'mongodb-memory-server'; import type * as t from '~/types'; import { createAclEntryMethods } from './aclEntry'; @@ -36,7 +36,7 @@ describe('AclEntry Model Tests', () => { describe('Permission Grant and Query', () => { test('should grant permission to a user', async () => { const entry = await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -45,10 +45,10 @@ describe('AclEntry Model Tests', () => { ); expect(entry).toBeDefined(); - expect(entry?.principalType).toBe('user'); + expect(entry?.principalType).toBe(PrincipalType.USER); expect(entry?.principalId?.toString()).toBe(userId.toString()); expect(entry?.principalModel).toBe('User'); - expect(entry?.resourceType).toBe('agent'); + expect(entry?.resourceType).toBe(ResourceType.AGENT); expect(entry?.resourceId.toString()).toBe(resourceId.toString()); expect(entry?.permBits).toBe(PermissionBits.VIEW); expect(entry?.grantedBy?.toString()).toBe(grantedById.toString()); @@ -57,7 +57,7 @@ describe('AclEntry Model Tests', () => { test('should grant permission to a group', async () => { const entry = await methods.grantPermission( - 'group', + PrincipalType.GROUP, groupId, ResourceType.AGENT, resourceId, @@ -66,7 +66,7 @@ describe('AclEntry Model Tests', () => { ); expect(entry).toBeDefined(); - expect(entry?.principalType).toBe('group'); + expect(entry?.principalType).toBe(PrincipalType.GROUP); expect(entry?.principalId?.toString()).toBe(groupId.toString()); expect(entry?.principalModel).toBe('Group'); expect(entry?.permBits).toBe(PermissionBits.VIEW | PermissionBits.EDIT); @@ -74,7 +74,7 @@ describe('AclEntry Model Tests', () => { test('should grant public permission', async () => { const entry = await methods.grantPermission( - 'public', + PrincipalType.PUBLIC, null, ResourceType.AGENT, resourceId, @@ -83,7 +83,7 @@ describe('AclEntry Model Tests', () => { ); expect(entry).toBeDefined(); - expect(entry?.principalType).toBe('public'); + expect(entry?.principalType).toBe(PrincipalType.PUBLIC); expect(entry?.principalId).toBeUndefined(); expect(entry?.principalModel).toBeUndefined(); }); @@ -91,7 +91,7 @@ describe('AclEntry Model Tests', () => { test('should find entries by principal', async () => { /** Create two different permissions for the same user */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -99,7 +99,7 @@ describe('AclEntry Model Tests', () => { grantedById, ); await methods.grantPermission( - 'user', + PrincipalType.USER, userId, 'project', new mongoose.Types.ObjectId(), @@ -108,19 +108,23 @@ describe('AclEntry Model Tests', () => { ); /** Find all entries for the user */ - const entries = await methods.findEntriesByPrincipal('user', userId); + const entries = await methods.findEntriesByPrincipal(PrincipalType.USER, userId); expect(entries).toHaveLength(2); /** Find entries filtered by resource type */ - const agentEntries = await methods.findEntriesByPrincipal('user', userId, 'agent'); + const agentEntries = await methods.findEntriesByPrincipal( + PrincipalType.USER, + userId, + ResourceType.AGENT, + ); expect(agentEntries).toHaveLength(1); - expect(agentEntries[0].resourceType).toBe('agent'); + expect(agentEntries[0].resourceType).toBe(ResourceType.AGENT); }); test('should find entries by resource', async () => { /** Grant permissions to different principals for the same resource */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -128,7 +132,7 @@ describe('AclEntry Model Tests', () => { grantedById, ); await methods.grantPermission( - 'group', + PrincipalType.GROUP, groupId, ResourceType.AGENT, resourceId, @@ -136,7 +140,7 @@ describe('AclEntry Model Tests', () => { grantedById, ); await methods.grantPermission( - 'public', + PrincipalType.PUBLIC, null, ResourceType.AGENT, resourceId, @@ -144,7 +148,7 @@ describe('AclEntry Model Tests', () => { grantedById, ); - const entries = await methods.findEntriesByResource('agent', resourceId); + const entries = await methods.findEntriesByResource(ResourceType.AGENT, resourceId); expect(entries).toHaveLength(3); }); }); @@ -153,7 +157,7 @@ describe('AclEntry Model Tests', () => { beforeEach(async () => { /** Setup test data with various permissions */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -161,7 +165,7 @@ describe('AclEntry Model Tests', () => { grantedById, ); await methods.grantPermission( - 'group', + PrincipalType.GROUP, groupId, ResourceType.AGENT, resourceId, @@ -171,7 +175,7 @@ describe('AclEntry Model Tests', () => { const otherResourceId = new mongoose.Types.ObjectId(); await methods.grantPermission( - 'public', + PrincipalType.PUBLIC, null, ResourceType.AGENT, otherResourceId, @@ -182,8 +186,8 @@ describe('AclEntry Model Tests', () => { test('should find entries by principals and resource', async () => { const principalsList = [ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]; const entries = await methods.findEntriesByPrincipalsAndResource( @@ -195,7 +199,7 @@ describe('AclEntry Model Tests', () => { }); test('should check if user has permission', async () => { - const principalsList = [{ principalType: 'user', principalId: userId }]; + const principalsList = [{ principalType: PrincipalType.USER, principalId: userId }]; /** User has VIEW permission */ const hasViewPermission = await methods.hasPermission( @@ -217,7 +221,7 @@ describe('AclEntry Model Tests', () => { }); test('should check if group has permission', async () => { - const principalsList = [{ principalType: 'group', principalId: groupId }]; + const principalsList = [{ principalType: PrincipalType.GROUP, principalId: groupId }]; /** Group has EDIT permission */ const hasEditPermission = await methods.hasPermission( @@ -231,8 +235,8 @@ describe('AclEntry Model Tests', () => { test('should check permission for multiple principals', async () => { const principalsList = [ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]; /** User has VIEW and group has EDIT, together they should have both */ @@ -264,11 +268,15 @@ describe('AclEntry Model Tests', () => { test('should get effective permissions', async () => { const principalsList = [ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ]; - const effective = await methods.getEffectivePermissions(principalsList, 'agent', resourceId); + const effective = await methods.getEffectivePermissions( + principalsList, + ResourceType.AGENT, + resourceId, + ); /** Combined permissions should be VIEW | EDIT */ expect(effective).toBe(PermissionBits.VIEW | PermissionBits.EDIT); @@ -279,7 +287,7 @@ describe('AclEntry Model Tests', () => { test('should revoke permission', async () => { /** Grant permission first */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -288,22 +296,27 @@ describe('AclEntry Model Tests', () => { ); /** Check it exists */ - const entriesBefore = await methods.findEntriesByPrincipal('user', userId); + const entriesBefore = await methods.findEntriesByPrincipal(PrincipalType.USER, userId); expect(entriesBefore).toHaveLength(1); /** Revoke it */ - const result = await methods.revokePermission('user', userId, ResourceType.AGENT, resourceId); + const result = await methods.revokePermission( + PrincipalType.USER, + userId, + ResourceType.AGENT, + resourceId, + ); expect(result.deletedCount).toBe(1); /** Verify it's gone */ - const entriesAfter = await methods.findEntriesByPrincipal('user', userId); + const entriesAfter = await methods.findEntriesByPrincipal(PrincipalType.USER, userId); expect(entriesAfter).toHaveLength(0); }); test('should modify permission bits - add permissions', async () => { /** Start with VIEW permission */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -313,7 +326,7 @@ describe('AclEntry Model Tests', () => { /** Add EDIT permission */ const updated = await methods.modifyPermissionBits( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -328,7 +341,7 @@ describe('AclEntry Model Tests', () => { test('should modify permission bits - remove permissions', async () => { /** Start with VIEW | EDIT permissions */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -338,7 +351,7 @@ describe('AclEntry Model Tests', () => { /** Remove EDIT permission */ const updated = await methods.modifyPermissionBits( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -353,7 +366,7 @@ describe('AclEntry Model Tests', () => { test('should modify permission bits - add and remove at once', async () => { /** Start with VIEW permission */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId, @@ -363,9 +376,9 @@ describe('AclEntry Model Tests', () => { /** Add EDIT and remove VIEW in one operation */ const updated = await methods.modifyPermissionBits( - 'user', + PrincipalType.USER, userId, - 'agent', + ResourceType.AGENT, resourceId, PermissionBits.EDIT, PermissionBits.VIEW, @@ -385,7 +398,7 @@ describe('AclEntry Model Tests', () => { /** User can view resource 1 */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId1, @@ -395,7 +408,7 @@ describe('AclEntry Model Tests', () => { /** User can view and edit resource 2 */ await methods.grantPermission( - 'user', + PrincipalType.USER, userId, ResourceType.AGENT, resourceId2, @@ -405,7 +418,7 @@ describe('AclEntry Model Tests', () => { /** Group can view resource 3 */ await methods.grantPermission( - 'group', + PrincipalType.GROUP, groupId, ResourceType.AGENT, resourceId3, @@ -415,7 +428,7 @@ describe('AclEntry Model Tests', () => { /** Find resources with VIEW permission for user */ const userViewableResources = await methods.findAccessibleResources( - [{ principalType: 'user', principalId: userId }], + [{ principalType: PrincipalType.USER, principalId: userId }], ResourceType.AGENT, PermissionBits.VIEW, ); @@ -428,8 +441,8 @@ describe('AclEntry Model Tests', () => { /** Find resources with VIEW permission for user or group */ const allViewableResources = await methods.findAccessibleResources( [ - { principalType: 'user', principalId: userId }, - { principalType: 'group', principalId: groupId }, + { principalType: PrincipalType.USER, principalId: userId }, + { principalType: PrincipalType.GROUP, principalId: groupId }, ], ResourceType.AGENT, PermissionBits.VIEW, @@ -439,7 +452,7 @@ describe('AclEntry Model Tests', () => { /** Find resources with EDIT permission for user */ const editableResources = await methods.findAccessibleResources( - [{ principalType: 'user', principalId: userId }], + [{ principalType: PrincipalType.USER, principalId: userId }], ResourceType.AGENT, PermissionBits.EDIT, ); @@ -452,19 +465,9 @@ describe('AclEntry Model Tests', () => { const projectId = new mongoose.Types.ObjectId(); const childResourceId = new mongoose.Types.ObjectId(); - /** Grant permission on project */ - await methods.grantPermission( - 'user', - userId, - 'project', - projectId, - PermissionBits.VIEW, - grantedById, - ); - /** Grant inherited permission on child resource */ await AclEntry.create({ - principalType: 'user', + principalType: PrincipalType.USER, principalId: userId, principalModel: 'User', resourceType: ResourceType.AGENT, @@ -476,7 +479,7 @@ describe('AclEntry Model Tests', () => { /** Get effective permissions */ const effective = await methods.getEffectivePermissions( - [{ principalType: 'user', principalId: userId }], + [{ principalType: PrincipalType.USER, principalId: userId }], ResourceType.AGENT, childResourceId, ); diff --git a/packages/data-schemas/src/methods/aclEntry.ts b/packages/data-schemas/src/methods/aclEntry.ts index a5bea9058..237e23864 100644 --- a/packages/data-schemas/src/methods/aclEntry.ts +++ b/packages/data-schemas/src/methods/aclEntry.ts @@ -1,3 +1,4 @@ +import { PrincipalType } from 'librechat-data-provider'; import type { Model, Types, DeleteResult, ClientSession } from 'mongoose'; import type { IAclEntry } from '~/types'; @@ -51,7 +52,7 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { const AclEntry = mongoose.models.AclEntry as Model; const principalsQuery = principalsList.map((p) => ({ principalType: p.principalType, - ...(p.principalType !== 'public' && { principalId: p.principalId }), + ...(p.principalType !== PrincipalType.PUBLIC && { principalId: p.principalId }), })); return await AclEntry.find({ @@ -78,7 +79,7 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { const AclEntry = mongoose.models.AclEntry as Model; const principalsQuery = principalsList.map((p) => ({ principalType: p.principalType, - ...(p.principalType !== 'public' && { principalId: p.principalId }), + ...(p.principalType !== PrincipalType.PUBLIC && { principalId: p.principalId }), })); const entry = await AclEntry.findOne({ @@ -145,9 +146,9 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { resourceId, }; - if (principalType !== 'public') { + if (principalType !== PrincipalType.PUBLIC) { query.principalId = principalId; - query.principalModel = principalType === 'user' ? 'User' : 'Group'; + query.principalModel = principalType === PrincipalType.USER ? 'User' : 'Group'; } const update = { @@ -191,7 +192,7 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { resourceId, }; - if (principalType !== 'public') { + if (principalType !== PrincipalType.PUBLIC) { query.principalId = principalId; } @@ -227,7 +228,7 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { resourceId, }; - if (principalType !== 'public') { + if (principalType !== PrincipalType.PUBLIC) { query.principalId = principalId; } @@ -266,7 +267,7 @@ export function createAclEntryMethods(mongoose: typeof import('mongoose')) { const AclEntry = mongoose.models.AclEntry as Model; const principalsQuery = principalsList.map((p) => ({ principalType: p.principalType, - ...(p.principalType !== 'public' && { principalId: p.principalId }), + ...(p.principalType !== PrincipalType.PUBLIC && { principalId: p.principalId }), })); const entries = await AclEntry.find({ diff --git a/packages/data-schemas/src/methods/userGroup.spec.ts b/packages/data-schemas/src/methods/userGroup.spec.ts index d05430769..fb2dfd7ab 100644 --- a/packages/data-schemas/src/methods/userGroup.spec.ts +++ b/packages/data-schemas/src/methods/userGroup.spec.ts @@ -1,9 +1,10 @@ import mongoose from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; import { MongoMemoryServer } from 'mongodb-memory-server'; +import type * as t from '~/types'; import { createUserGroupMethods } from './userGroup'; import groupSchema from '~/schema/group'; import userSchema from '~/schema/user'; -import type * as t from '~/types'; /** Mocking logger */ jest.mock('~/config/winston', () => ({ @@ -330,9 +331,9 @@ describe('User Group Methods Tests', () => { expect(principals).toHaveLength(3); /** Check principal types */ - const userPrincipal = principals.find((p) => p.principalType === 'user'); - const groupPrincipal = principals.find((p) => p.principalType === 'group'); - const publicPrincipal = principals.find((p) => p.principalType === 'public'); + const userPrincipal = principals.find((p) => p.principalType === PrincipalType.USER); + const groupPrincipal = principals.find((p) => p.principalType === PrincipalType.GROUP); + const publicPrincipal = principals.find((p) => p.principalType === PrincipalType.PUBLIC); expect(userPrincipal).toBeDefined(); expect(userPrincipal?.principalId?.toString()).toBe( @@ -352,9 +353,9 @@ describe('User Group Methods Tests', () => { /** Should still return user and public principals even for non-existent user */ expect(principals).toHaveLength(2); - expect(principals[0].principalType).toBe('user'); + expect(principals[0].principalType).toBe(PrincipalType.USER); expect(principals[0].principalId?.toString()).toBe(nonExistentId.toString()); - expect(principals[1].principalType).toBe('public'); + expect(principals[1].principalType).toBe(PrincipalType.PUBLIC); expect(principals[1].principalId).toBeUndefined(); }); }); diff --git a/packages/data-schemas/src/methods/userGroup.ts b/packages/data-schemas/src/methods/userGroup.ts index 034f59ad3..0545bc1fb 100644 --- a/packages/data-schemas/src/methods/userGroup.ts +++ b/packages/data-schemas/src/methods/userGroup.ts @@ -1,5 +1,6 @@ -import type { Model, Types, ClientSession } from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; import type { TUser, TPrincipalSearchResult } from 'librechat-data-provider'; +import type { Model, Types, ClientSession } from 'mongoose'; import type { IGroup, IUser } from '~/types'; export function createUserGroupMethods(mongoose: typeof import('mongoose')) { @@ -245,17 +246,17 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { session?: ClientSession, ): Promise> { const principals: Array<{ principalType: string; principalId?: string | Types.ObjectId }> = [ - { principalType: 'user', principalId: userId }, + { principalType: PrincipalType.USER, principalId: userId }, ]; const userGroups = await getUserGroups(userId, session); if (userGroups && userGroups.length > 0) { userGroups.forEach((group) => { - principals.push({ principalType: 'group', principalId: group._id.toString() }); + principals.push({ principalType: PrincipalType.GROUP, principalId: group._id.toString() }); }); } - principals.push({ principalType: 'public' }); + principals.push({ principalType: PrincipalType.PUBLIC }); return principals; } diff --git a/packages/data-schemas/src/schema/aclEntry.ts b/packages/data-schemas/src/schema/aclEntry.ts index 3f3f1012e..b3c3d4b05 100644 --- a/packages/data-schemas/src/schema/aclEntry.ts +++ b/packages/data-schemas/src/schema/aclEntry.ts @@ -1,18 +1,19 @@ import { Schema } from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; import type { IAclEntry } from '~/types'; const aclEntrySchema = new Schema( { principalType: { type: String, - enum: ['user', 'group', 'public'], + enum: Object.values(PrincipalType), required: true, }, principalId: { type: Schema.Types.ObjectId, refPath: 'principalModel', required: function (this: IAclEntry) { - return this.principalType !== 'public'; + return this.principalType !== PrincipalType.PUBLIC; }, index: true, }, @@ -20,7 +21,7 @@ const aclEntrySchema = new Schema( type: String, enum: ['User', 'Group'], required: function (this: IAclEntry) { - return this.principalType !== 'public'; + return this.principalType !== PrincipalType.PUBLIC; }, }, resourceType: { diff --git a/packages/data-schemas/src/types/aclEntry.ts b/packages/data-schemas/src/types/aclEntry.ts index 55bbe5196..ff03b23e8 100644 --- a/packages/data-schemas/src/types/aclEntry.ts +++ b/packages/data-schemas/src/types/aclEntry.ts @@ -1,8 +1,9 @@ import type { Document, Types } from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; export type AclEntry = { /** The type of principal ('user', 'group', 'public') */ - principalType: 'user' | 'group' | 'public'; + principalType: PrincipalType; /** The ID of the principal (null for 'public') */ principalId?: Types.ObjectId; /** The model name for the principal ('User' or 'Group') */