🛂 feat: Role as Permission Principal Type

WIP: Role as Permission Principal Type

WIP: add user role check optimization to user principal check, update type comparisons

WIP: cover edge cases for string vs ObjectId handling in permission granting and checking

chore: Update people picker access middleware to use PrincipalType constants

feat: Enhance people picker access control to include roles permissions

chore: add missing default role schema values for people picker perms, cleanup typing

feat: Enhance PeoplePicker component with role-specific UI and localization updates

chore: Add missing `VIEW_ROLES` permission to role schema
This commit is contained in:
Danny Avila 2025-08-03 19:24:40 -04:00
parent 28d63dab71
commit 39346d6b8e
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
49 changed files with 2879 additions and 258 deletions

View file

@ -89,4 +89,114 @@ describe('AppService interface configuration', () => {
expect(app.locals.interfaceConfig.bookmarks).toBe(false);
expect(loadDefaultInterface).toHaveBeenCalled();
});
it('should correctly configure peoplePicker permissions including roles', async () => {
mockLoadCustomConfig.mockResolvedValue({
interface: {
peoplePicker: {
admin: {
users: true,
groups: true,
roles: true,
},
user: {
users: false,
groups: false,
roles: false,
},
},
},
});
loadDefaultInterface.mockResolvedValue({
peoplePicker: {
admin: {
users: true,
groups: true,
roles: true,
},
user: {
users: false,
groups: false,
roles: false,
},
},
});
await AppService(app);
expect(app.locals.interfaceConfig.peoplePicker).toBeDefined();
expect(app.locals.interfaceConfig.peoplePicker.admin).toMatchObject({
users: true,
groups: true,
roles: true,
});
expect(app.locals.interfaceConfig.peoplePicker.user).toMatchObject({
users: false,
groups: false,
roles: false,
});
expect(loadDefaultInterface).toHaveBeenCalled();
});
it('should handle mixed peoplePicker permissions for roles', async () => {
mockLoadCustomConfig.mockResolvedValue({
interface: {
peoplePicker: {
admin: {
users: true,
groups: true,
roles: false,
},
user: {
users: true,
groups: false,
roles: true,
},
},
},
});
loadDefaultInterface.mockResolvedValue({
peoplePicker: {
admin: {
users: true,
groups: true,
roles: false,
},
user: {
users: true,
groups: false,
roles: true,
},
},
});
await AppService(app);
expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(false);
expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(true);
});
it('should set default peoplePicker roles permissions when not provided', async () => {
mockLoadCustomConfig.mockResolvedValue({});
loadDefaultInterface.mockResolvedValue({
peoplePicker: {
admin: {
users: true,
groups: true,
roles: true,
},
user: {
users: false,
groups: false,
roles: false,
},
},
});
await AppService(app);
expect(app.locals.interfaceConfig.peoplePicker).toBeDefined();
expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(true);
expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(false);
});
});

View file

@ -969,4 +969,59 @@ describe('AppService updating app.locals and issuing warnings', () => {
expect(app.locals.ocr.strategy).toEqual('mistral_ocr');
expect(app.locals.ocr.mistralModel).toEqual('mistral-medium');
});
it('should correctly configure peoplePicker with roles permission when specified', async () => {
const mockConfig = {
interface: {
peoplePicker: {
admin: {
users: true,
groups: true,
roles: true,
},
user: {
users: false,
groups: false,
roles: true,
},
},
},
};
require('./Config/loadCustomConfig').mockImplementationOnce(() => Promise.resolve(mockConfig));
const app = { locals: {} };
await AppService(app);
// Check that interface config includes the roles permission
expect(app.locals.interfaceConfig.peoplePicker).toBeDefined();
expect(app.locals.interfaceConfig.peoplePicker.admin).toMatchObject({
users: true,
groups: true,
roles: true,
});
expect(app.locals.interfaceConfig.peoplePicker.user).toMatchObject({
users: false,
groups: false,
roles: true,
});
});
it('should use default peoplePicker roles permissions when not specified', async () => {
const mockConfig = {
interface: {
// No peoplePicker configuration
},
};
require('./Config/loadCustomConfig').mockImplementationOnce(() => Promise.resolve(mockConfig));
const app = { locals: {} };
await AppService(app);
// Check that default roles permissions are applied
expect(app.locals.interfaceConfig.peoplePicker).toBeDefined();
expect(app.locals.interfaceConfig.peoplePicker.admin.roles).toBe(true);
expect(app.locals.interfaceConfig.peoplePicker.user.roles).toBe(false);
});
});

View file

@ -172,7 +172,12 @@ const primeFiles = async (options, apiKey) => {
// Filter by access if user and agent are provided
let dbFiles;
if (req?.user?.id && agentId) {
dbFiles = await filterFilesByAgentAccess(allFiles, req.user.id, agentId);
dbFiles = await filterFilesByAgentAccess({
files: allFiles,
userId: req.user.id,
role: req.user.role,
agentId,
});
} else {
dbFiles = allFiles;
}

View file

@ -5,12 +5,14 @@ const { getAgent } = require('~/models/Agent');
/**
* Checks if a user has access to multiple files through a shared agent (batch operation)
* @param {string} userId - The user ID to check access for
* @param {string[]} fileIds - Array of file IDs to check
* @param {string} agentId - The agent ID that might grant access
* @param {Object} params - Parameters object
* @param {string} params.userId - The user ID to check access for
* @param {string} [params.role] - Optional user role to avoid DB query
* @param {string[]} params.fileIds - Array of file IDs to check
* @param {string} params.agentId - The agent ID that might grant access
* @returns {Promise<Map<string, boolean>>} Map of fileId to access status
*/
const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId }) => {
const accessMap = new Map();
// Initialize all files as no access
@ -24,7 +26,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
}
// Check if user is the author - if so, grant access to all files
if (agent.author.toString() === userId) {
if (agent.author.toString() === userId.toString()) {
fileIds.forEach((fileId) => accessMap.set(fileId, true));
return accessMap;
}
@ -32,6 +34,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
// Check if user has at least VIEW permission on the agent
const hasViewPermission = await checkPermission({
userId,
role,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
requiredPermission: PermissionBits.VIEW,
@ -44,6 +47,7 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
// Check if user has EDIT permission (which would indicate collaborative access)
const hasEditPermission = await checkPermission({
userId,
role,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
requiredPermission: PermissionBits.EDIT,
@ -81,12 +85,14 @@ const hasAccessToFilesViaAgent = async (userId, fileIds, agentId) => {
/**
* Filter files based on user access through agents
* @param {Array<MongoFile>} files - Array of file documents
* @param {string} userId - User ID for access control
* @param {string} agentId - Agent ID that might grant access to files
* @param {Object} params - Parameters object
* @param {Array<MongoFile>} params.files - Array of file documents
* @param {string} params.userId - User ID for access control
* @param {string} [params.role] - Optional user role to avoid DB query
* @param {string} params.agentId - Agent ID that might grant access to files
* @returns {Promise<Array<MongoFile>>} Filtered array of accessible files
*/
const filterFilesByAgentAccess = async (files, userId, agentId) => {
const filterFilesByAgentAccess = async ({ files, userId, role, agentId }) => {
if (!userId || !agentId || !files || files.length === 0) {
return files;
}
@ -96,7 +102,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => {
const ownedFiles = [];
for (const file of files) {
if (file.user && file.user.toString() === userId) {
if (file.user && file.user.toString() === userId.toString()) {
ownedFiles.push(file);
} else {
filesToCheck.push(file);
@ -109,7 +115,7 @@ const filterFilesByAgentAccess = async (files, userId, agentId) => {
// Batch check access for all non-owned files
const fileIds = filesToCheck.map((f) => f.file_id);
const accessMap = await hasAccessToFilesViaAgent(userId, fileIds, agentId);
const accessMap = await hasAccessToFilesViaAgent({ userId, role, fileIds, agentId });
// Filter files based on access
const accessibleFiles = filesToCheck.filter((file) => accessMap.get(file.file_id));

View file

@ -1,7 +1,7 @@
const mongoose = require('mongoose');
const { isEnabled } = require('@librechat/api');
const { ResourceType, PrincipalType, PrincipalModel } = require('librechat-data-provider');
const { getTransactionSupport, logger } = require('@librechat/data-schemas');
const { ResourceType, PrincipalType, PrincipalModel } = require('librechat-data-provider');
const {
entraIdPrincipalFeatureEnabled,
getUserOwnedEntraGroups,
@ -46,8 +46,8 @@ const validateResourceType = (resourceType) => {
/**
* Grant a permission to a principal for a resource using a role
* @param {Object} params - Parameters for granting role-based permission
* @param {string} params.principalType - 'user', 'group', or 'public'
* @param {string|mongoose.Types.ObjectId|null} params.principalId - The ID of the principal (null for 'public')
* @param {string} params.principalType - PrincipalType.USER, PrincipalType.GROUP, or PrincipalType.PUBLIC
* @param {string|mongoose.Types.ObjectId|null} params.principalId - The ID of the principal (null for PrincipalType.PUBLIC)
* @param {string} params.resourceType - Type of resource (e.g., 'agent')
* @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource
* @param {string} params.accessRoleId - The ID of the role (e.g., AccessRoleIds.AGENT_VIEWER, AccessRoleIds.AGENT_EDITOR)
@ -70,10 +70,21 @@ const grantPermission = async ({
}
if (principalType !== PrincipalType.PUBLIC && !principalId) {
throw new Error('Principal ID is required for user and group principals');
throw new Error('Principal ID is required for user, group, and role principals');
}
if (principalId && !mongoose.Types.ObjectId.isValid(principalId)) {
// Validate principalId based on type
if (principalId && principalType === PrincipalType.ROLE) {
// Role IDs are strings (role names)
if (typeof principalId !== 'string' || principalId.trim().length === 0) {
throw new Error(`Invalid role ID: ${principalId}`);
}
} else if (
principalType &&
principalType !== PrincipalType.PUBLIC &&
!mongoose.Types.ObjectId.isValid(principalId)
) {
// User and Group IDs must be valid ObjectIds
throw new Error(`Invalid principal ID: ${principalId}`);
}
@ -115,12 +126,13 @@ const grantPermission = async ({
* Check if a user has specific permission bits on a resource
* @param {Object} params - Parameters for checking permissions
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user
* @param {string} [params.role] - Optional user role (if not provided, will query from DB)
* @param {string} params.resourceType - Type of resource (e.g., 'agent')
* @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource
* @param {number} params.requiredPermissions - The permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT)
* @returns {Promise<boolean>} Whether the user has the required permission bits
*/
const checkPermission = async ({ userId, resourceType, resourceId, requiredPermission }) => {
const checkPermission = async ({ userId, role, resourceType, resourceId, requiredPermission }) => {
try {
if (typeof requiredPermission !== 'number' || requiredPermission < 1) {
throw new Error('requiredPermission must be a positive number');
@ -129,7 +141,7 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi
validateResourceType(resourceType);
// Get all principals for the user (user + groups + public)
const principals = await getUserPrincipals(userId);
const principals = await getUserPrincipals({ userId, role });
if (principals.length === 0) {
return false;
@ -150,16 +162,17 @@ const checkPermission = async ({ userId, resourceType, resourceId, requiredPermi
* Get effective permission bitmask for a user on a resource
* @param {Object} params - Parameters for getting effective permissions
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user
* @param {string} [params.role] - Optional user role (if not provided, will query from DB)
* @param {string} params.resourceType - Type of resource (e.g., 'agent')
* @param {string|mongoose.Types.ObjectId} params.resourceId - The ID of the resource
* @returns {Promise<number>} Effective permission bitmask
*/
const getEffectivePermissions = async ({ userId, resourceType, resourceId }) => {
const getEffectivePermissions = async ({ userId, role, resourceType, resourceId }) => {
try {
validateResourceType(resourceType);
// Get all principals for the user (user + groups + public)
const principals = await getUserPrincipals(userId);
const principals = await getUserPrincipals({ userId, role });
if (principals.length === 0) {
return 0;
@ -175,11 +188,12 @@ const getEffectivePermissions = async ({ userId, resourceType, resourceId }) =>
* Find all resources of a specific type that a user has access to with specific permission bits
* @param {Object} params - Parameters for finding accessible resources
* @param {string|mongoose.Types.ObjectId} params.userId - The ID of the user
* @param {string} [params.role] - Optional user role (if not provided, will query from DB)
* @param {string} params.resourceType - Type of resource (e.g., 'agent')
* @param {number} params.requiredPermissions - The minimum permission bits required (e.g., 1 for VIEW, 3 for VIEW+EDIT)
* @returns {Promise<Array>} Array of resource IDs
*/
const findAccessibleResources = async ({ userId, resourceType, requiredPermissions }) => {
const findAccessibleResources = async ({ userId, role, resourceType, requiredPermissions }) => {
try {
if (typeof requiredPermissions !== 'number' || requiredPermissions < 1) {
throw new Error('requiredPermissions must be a positive number');
@ -188,7 +202,7 @@ const findAccessibleResources = async ({ userId, resourceType, requiredPermissio
validateResourceType(resourceType);
// Get all principals for the user (user + groups + public)
const principalsList = await getUserPrincipals(userId);
const principalsList = await getUserPrincipals({ userId, role });
if (principalsList.length === 0) {
return [];
@ -253,7 +267,7 @@ const getAvailableRoles = async ({ resourceType }) => {
* Ensures a principal exists in the database based on TPrincipal data
* Creates user if it doesn't exist locally (for Entra ID users)
* @param {Object} principal - TPrincipal object from frontend
* @param {string} principal.type - 'user', 'group', or 'public'
* @param {string} principal.type - PrincipalType.USER, PrincipalType.GROUP, or PrincipalType.PUBLIC
* @param {string} [principal.id] - Local database ID (null for Entra ID principals not yet synced)
* @param {string} principal.name - Display name
* @param {string} [principal.email] - Email address
@ -262,7 +276,7 @@ const getAvailableRoles = async ({ resourceType }) => {
* @returns {Promise<string|null>} Returns the principalId for database operations, null for public
*/
const ensurePrincipalExists = async function (principal) {
if (principal.type === 'public') {
if (principal.type === PrincipalType.PUBLIC) {
return null;
}
@ -270,7 +284,7 @@ const ensurePrincipalExists = async function (principal) {
return principal.id;
}
if (principal.type === 'user' && principal.source === 'entra') {
if (principal.type === PrincipalType.USER && principal.source === 'entra') {
if (!principal.email || !principal.idOnTheSource) {
throw new Error('Entra ID user principals must have email and idOnTheSource');
}
@ -303,7 +317,7 @@ const ensurePrincipalExists = async function (principal) {
return userId.toString();
}
if (principal.type === 'group') {
if (principal.type === PrincipalType.GROUP) {
throw new Error('Group principals should be handled by group-specific methods');
}
@ -315,7 +329,7 @@ const ensurePrincipalExists = async function (principal) {
* Creates group if it doesn't exist locally (for Entra ID groups)
* For Entra ID groups, always synchronizes member IDs when authentication context is provided
* @param {Object} principal - TPrincipal object from frontend
* @param {string} principal.type - Must be 'group'
* @param {string} principal.type - Must be PrincipalType.GROUP
* @param {string} [principal.id] - Local database ID (null for Entra ID principals not yet synced)
* @param {string} principal.name - Display name
* @param {string} [principal.email] - Email address
@ -328,8 +342,8 @@ const ensurePrincipalExists = async function (principal) {
* @returns {Promise<string>} Returns the groupId for database operations
*/
const ensureGroupPrincipalExists = async function (principal, authContext = null) {
if (principal.type !== 'group') {
throw new Error(`Invalid principal type: ${principal.type}. Expected 'group'`);
if (principal.type !== PrincipalType.GROUP) {
throw new Error(`Invalid principal type: ${principal.type}. Expected '${PrincipalType.GROUP}'`);
}
if (principal.source === 'entra') {
@ -612,10 +626,19 @@ const bulkUpdateResourcePermissions = async ({
resourceId,
};
if (principal.type !== 'public') {
query.principalId = principal.id;
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,
@ -628,9 +651,11 @@ const bulkUpdateResourcePermissions = async ({
resourceType,
resourceId,
...(principal.type !== PrincipalType.PUBLIC && {
principalId: principal.id,
principalModel:
principal.type === PrincipalType.USER ? PrincipalModel.USER : PrincipalModel.GROUP,
principalId:
principal.type === PrincipalType.ROLE
? principal.id
: new mongoose.Types.ObjectId(principal.id),
principalModel: principalModelMap[principal.type],
}),
},
};
@ -677,8 +702,11 @@ const bulkUpdateResourcePermissions = async ({
resourceId,
};
if (principal.type !== 'public') {
query.principalId = principal.id;
if (principal.type !== PrincipalType.PUBLIC) {
query.principalId =
principal.type === PrincipalType.ROLE
? principal.id
: new mongoose.Types.ObjectId(principal.id);
}
deleteQueries.push(query);

View file

@ -79,6 +79,7 @@ describe('PermissionService', () => {
const groupId = new mongoose.Types.ObjectId();
const resourceId = new mongoose.Types.ObjectId();
const grantedById = new mongoose.Types.ObjectId();
const roleResourceId = new mongoose.Types.ObjectId();
describe('grantPermission', () => {
test('should grant permission to a user with a role', async () => {
@ -171,7 +172,7 @@ describe('PermissionService', () => {
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: grantedById,
}),
).rejects.toThrow('Principal ID is required for user and group principals');
).rejects.toThrow('Principal ID is required for user, group, and role principals');
});
test('should throw error for non-existent role', async () => {
@ -1000,6 +1001,230 @@ describe('PermissionService', () => {
expect(publicEntry.roleId.accessRoleId).toBe(AccessRoleIds.AGENT_EDITOR);
});
test('should grant permission to a role', async () => {
const entry = await grantPermission({
principalType: PrincipalType.ROLE,
principalId: 'admin',
resourceType: ResourceType.AGENT,
resourceId: roleResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
expect(entry).toBeDefined();
expect(entry.principalType).toBe(PrincipalType.ROLE);
expect(entry.principalId).toBe('admin');
expect(entry.principalModel).toBe(PrincipalModel.ROLE);
expect(entry.resourceType).toBe(ResourceType.AGENT);
expect(entry.resourceId.toString()).toBe(roleResourceId.toString());
// Get the role to verify the permission bits are correctly set
const role = await findRoleByIdentifier(AccessRoleIds.AGENT_EDITOR);
expect(entry.permBits).toBe(role.permBits);
expect(entry.roleId.toString()).toBe(role._id.toString());
});
test('should check permissions for user with role', async () => {
// Grant permission to admin role
await grantPermission({
principalType: PrincipalType.ROLE,
principalId: 'admin',
resourceType: ResourceType.AGENT,
resourceId: roleResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return user with admin role
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: userId },
{ principalType: PrincipalType.ROLE, principalId: 'admin' },
{ principalType: PrincipalType.PUBLIC },
]);
const hasPermission = await checkPermission({
userId,
resourceType: ResourceType.AGENT,
resourceId: roleResourceId,
requiredPermission: 1, // VIEW
});
expect(hasPermission).toBe(true);
// Check that user without admin role cannot access
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: userId },
{ principalType: PrincipalType.PUBLIC },
]);
const hasNoPermission = await checkPermission({
userId,
resourceType: ResourceType.AGENT,
resourceId: roleResourceId,
requiredPermission: 1, // VIEW
});
expect(hasNoPermission).toBe(false);
});
test('should optimize permission checks when role is provided', async () => {
const testUserId = new mongoose.Types.ObjectId();
const testResourceId = new mongoose.Types.ObjectId();
// Create a user with EDITOR role
const User = mongoose.models.User;
await User.create({
_id: testUserId,
email: 'editor@test.com',
emailVerified: true,
provider: 'local',
role: 'EDITOR',
});
// Grant permission to EDITOR role
await grantPermission({
principalType: PrincipalType.ROLE,
principalId: 'EDITOR',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return user with EDITOR role when called
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: testUserId },
{ principalType: PrincipalType.ROLE, principalId: 'EDITOR' },
{ principalType: PrincipalType.PUBLIC },
]);
// Test 1: Check permission with role provided (optimization should be used)
const hasPermissionWithRole = await checkPermission({
userId: testUserId,
role: 'EDITOR',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 1, // VIEW
});
expect(hasPermissionWithRole).toBe(true);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' });
// Test 2: Check permission without role (should call getUserPrincipals)
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: testUserId },
{ principalType: PrincipalType.ROLE, principalId: 'EDITOR' },
{ principalType: PrincipalType.PUBLIC },
]);
const hasPermissionWithoutRole = await checkPermission({
userId: testUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 1, // VIEW
});
expect(hasPermissionWithoutRole).toBe(true);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: undefined });
// Test 3: Verify getEffectivePermissions also uses the optimization
getUserPrincipals.mockClear();
const effectiveWithRole = await getEffectivePermissions({
userId: testUserId,
role: 'EDITOR',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
expect(effectiveWithRole).toBe(3); // EDITOR = VIEW + EDIT
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' });
// Test 4: Verify findAccessibleResources also uses the optimization
getUserPrincipals.mockClear();
const accessibleWithRole = await findAccessibleResources({
userId: testUserId,
role: 'EDITOR',
resourceType: ResourceType.AGENT,
requiredPermissions: 1, // VIEW
});
expect(accessibleWithRole.map((id) => id.toString())).toContain(testResourceId.toString());
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' });
});
test('should handle role changes dynamically', async () => {
const testUserId = new mongoose.Types.ObjectId();
const testResourceId = new mongoose.Types.ObjectId();
// Grant permission to ADMIN role only
await grantPermission({
principalType: PrincipalType.ROLE,
principalId: 'ADMIN',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_OWNER,
grantedBy: grantedById,
});
// Test with ADMIN role - should have access
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: testUserId },
{ principalType: PrincipalType.ROLE, principalId: 'ADMIN' },
{ principalType: PrincipalType.PUBLIC },
]);
const hasAdminAccess = await checkPermission({
userId: testUserId,
role: 'ADMIN',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 7, // Full permissions
});
expect(hasAdminAccess).toBe(true);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'ADMIN' });
// Test with USER role - should NOT have access
getUserPrincipals.mockClear();
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: testUserId },
{ principalType: PrincipalType.ROLE, principalId: 'USER' },
{ principalType: PrincipalType.PUBLIC },
]);
const hasUserAccess = await checkPermission({
userId: testUserId,
role: 'USER',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 1, // Even VIEW
});
expect(hasUserAccess).toBe(false);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'USER' });
// Test with EDITOR role - should NOT have access
getUserPrincipals.mockClear();
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: testUserId },
{ principalType: PrincipalType.ROLE, principalId: 'EDITOR' },
{ principalType: PrincipalType.PUBLIC },
]);
const hasEditorAccess = await checkPermission({
userId: testUserId,
role: 'EDITOR',
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 1, // VIEW
});
expect(hasEditorAccess).toBe(false);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: testUserId, role: 'EDITOR' });
});
test('should work with different resource types', async () => {
// Test with promptGroup resources
const promptGroupResourceId = new mongoose.Types.ObjectId();
@ -1039,4 +1264,344 @@ describe('PermissionService', () => {
);
});
});
describe('String vs ObjectId Edge Cases', () => {
const stringUserId = new mongoose.Types.ObjectId().toString();
const objectIdUserId = new mongoose.Types.ObjectId();
const stringGroupId = new mongoose.Types.ObjectId().toString();
const objectIdGroupId = new mongoose.Types.ObjectId();
const testResourceId = new mongoose.Types.ObjectId();
beforeEach(async () => {
// Clear any existing ACL entries
await AclEntry.deleteMany({});
getUserPrincipals.mockReset();
});
test('should handle string userId in grantPermission', async () => {
const entry = await grantPermission({
principalType: PrincipalType.USER,
principalId: stringUserId, // Pass string
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: grantedById,
});
expect(entry).toBeDefined();
expect(entry.principalType).toBe(PrincipalType.USER);
// Should be stored as ObjectId
expect(entry.principalId).toBeInstanceOf(mongoose.Types.ObjectId);
expect(entry.principalId.toString()).toBe(stringUserId);
});
test('should handle string groupId in grantPermission', async () => {
const entry = await grantPermission({
principalType: PrincipalType.GROUP,
principalId: stringGroupId, // Pass string
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
expect(entry).toBeDefined();
expect(entry.principalType).toBe(PrincipalType.GROUP);
// Should be stored as ObjectId
expect(entry.principalId).toBeInstanceOf(mongoose.Types.ObjectId);
expect(entry.principalId.toString()).toBe(stringGroupId);
});
test('should handle string roleId in grantPermission for ROLE type', async () => {
const roleString = 'moderator';
const entry = await grantPermission({
principalType: PrincipalType.ROLE,
principalId: roleString,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: grantedById,
});
expect(entry).toBeDefined();
expect(entry.principalType).toBe(PrincipalType.ROLE);
// Should remain as string for ROLE type
expect(typeof entry.principalId).toBe('string');
expect(entry.principalId).toBe(roleString);
expect(entry.principalModel).toBe(PrincipalModel.ROLE);
});
test('should check permissions correctly when permission granted with string userId', async () => {
// Grant permission with string userId
await grantPermission({
principalType: PrincipalType.USER,
principalId: stringUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return ObjectId (as it should after our fix)
getUserPrincipals.mockResolvedValue([
{
principalType: PrincipalType.USER,
principalId: new mongoose.Types.ObjectId(stringUserId),
},
{ principalType: PrincipalType.PUBLIC },
]);
// Check permission with string userId
const hasPermission = await checkPermission({
userId: stringUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 1, // VIEW
});
expect(hasPermission).toBe(true);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: stringUserId, role: undefined });
});
test('should check permissions correctly when permission granted with ObjectId', async () => {
// Grant permission with ObjectId
await grantPermission({
principalType: PrincipalType.USER,
principalId: objectIdUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_OWNER,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return ObjectId
getUserPrincipals.mockResolvedValue([
{ principalType: PrincipalType.USER, principalId: objectIdUserId },
{ principalType: PrincipalType.PUBLIC },
]);
// Check permission with ObjectId
const hasPermission = await checkPermission({
userId: objectIdUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
requiredPermission: 7, // Full permissions
});
expect(hasPermission).toBe(true);
expect(getUserPrincipals).toHaveBeenCalledWith({ userId: objectIdUserId, role: undefined });
});
test('should handle bulkUpdateResourcePermissions with string IDs', async () => {
const updatedPrincipals = [
{
type: PrincipalType.USER,
id: stringUserId, // String ID
accessRoleId: AccessRoleIds.AGENT_VIEWER,
},
{
type: PrincipalType.GROUP,
id: stringGroupId, // String ID
accessRoleId: AccessRoleIds.AGENT_EDITOR,
},
{
type: PrincipalType.ROLE,
id: 'admin', // String ID (should remain string)
accessRoleId: AccessRoleIds.AGENT_OWNER,
},
];
const results = await bulkUpdateResourcePermissions({
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
updatedPrincipals,
grantedBy: grantedById,
});
expect(results.granted).toHaveLength(3);
expect(results.errors).toHaveLength(0);
// Verify USER entry has ObjectId
const userEntry = await AclEntry.findOne({
principalType: PrincipalType.USER,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
expect(userEntry.principalId).toBeInstanceOf(mongoose.Types.ObjectId);
expect(userEntry.principalId.toString()).toBe(stringUserId);
// Verify GROUP entry has ObjectId
const groupEntry = await AclEntry.findOne({
principalType: PrincipalType.GROUP,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
expect(groupEntry.principalId).toBeInstanceOf(mongoose.Types.ObjectId);
expect(groupEntry.principalId.toString()).toBe(stringGroupId);
// Verify ROLE entry has string
const roleEntry = await AclEntry.findOne({
principalType: PrincipalType.ROLE,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
expect(typeof roleEntry.principalId).toBe('string');
expect(roleEntry.principalId).toBe('admin');
});
test('should handle revoking permissions with string IDs in bulkUpdateResourcePermissions', async () => {
// First grant permissions with ObjectIds
await grantPermission({
principalType: PrincipalType.USER,
principalId: objectIdUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_OWNER,
grantedBy: grantedById,
});
await grantPermission({
principalType: PrincipalType.GROUP,
principalId: objectIdGroupId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Revoke using string IDs
const revokedPrincipals = [
{
type: PrincipalType.USER,
id: objectIdUserId.toString(), // String version of ObjectId
},
{
type: PrincipalType.GROUP,
id: objectIdGroupId.toString(), // String version of ObjectId
},
];
const results = await bulkUpdateResourcePermissions({
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
revokedPrincipals,
grantedBy: grantedById,
});
expect(results.revoked).toHaveLength(2);
expect(results.errors).toHaveLength(0);
// Verify permissions were actually revoked
const remainingEntries = await AclEntry.find({
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
expect(remainingEntries).toHaveLength(0);
});
test('should find accessible resources when permissions granted with mixed ID types', async () => {
const resource1 = new mongoose.Types.ObjectId();
const resource2 = new mongoose.Types.ObjectId();
const resource3 = new mongoose.Types.ObjectId();
// Grant with string userId
await grantPermission({
principalType: PrincipalType.USER,
principalId: stringUserId,
resourceType: ResourceType.AGENT,
resourceId: resource1,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: grantedById,
});
// Grant with ObjectId userId (same user)
await grantPermission({
principalType: PrincipalType.USER,
principalId: new mongoose.Types.ObjectId(stringUserId),
resourceType: ResourceType.AGENT,
resourceId: resource2,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Grant to role
await grantPermission({
principalType: PrincipalType.ROLE,
principalId: 'admin',
resourceType: ResourceType.AGENT,
resourceId: resource3,
accessRoleId: AccessRoleIds.AGENT_OWNER,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return user with admin role
getUserPrincipals.mockResolvedValue([
{
principalType: PrincipalType.USER,
principalId: new mongoose.Types.ObjectId(stringUserId),
},
{ principalType: PrincipalType.ROLE, principalId: 'admin' },
{ principalType: PrincipalType.PUBLIC },
]);
const accessibleResources = await findAccessibleResources({
userId: stringUserId,
role: 'admin',
resourceType: ResourceType.AGENT,
requiredPermissions: 1, // VIEW
});
// Should find all three resources
expect(accessibleResources).toHaveLength(3);
const resourceIds = accessibleResources.map((id) => id.toString());
expect(resourceIds).toContain(resource1.toString());
expect(resourceIds).toContain(resource2.toString());
expect(resourceIds).toContain(resource3.toString());
});
test('should get effective permissions with mixed ID types', async () => {
// Grant VIEW permission with string userId
await grantPermission({
principalType: PrincipalType.USER,
principalId: stringUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: grantedById,
});
// Grant EDIT permission to a group with string groupId
await grantPermission({
principalType: PrincipalType.GROUP,
principalId: stringGroupId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
accessRoleId: AccessRoleIds.AGENT_EDITOR,
grantedBy: grantedById,
});
// Mock getUserPrincipals to return ObjectIds (as it should after our fix)
getUserPrincipals.mockResolvedValue([
{
principalType: PrincipalType.USER,
principalId: new mongoose.Types.ObjectId(stringUserId),
},
{
principalType: PrincipalType.GROUP,
principalId: new mongoose.Types.ObjectId(stringGroupId),
},
{ principalType: PrincipalType.PUBLIC },
]);
const effectivePermissions = await getEffectivePermissions({
userId: stringUserId,
resourceType: ResourceType.AGENT,
resourceId: testResourceId,
});
// Should combine VIEW (1) and EDIT (3) permissions
expect(effectivePermissions).toBe(3); // EDITOR includes VIEW
});
});
});

View file

@ -57,10 +57,12 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol
admin: {
users: interfaceConfig?.peoplePicker?.admin?.users ?? defaults.peoplePicker?.admin.users,
groups: interfaceConfig?.peoplePicker?.admin?.groups ?? defaults.peoplePicker?.admin.groups,
roles: interfaceConfig?.peoplePicker?.admin?.roles ?? defaults.peoplePicker?.admin.roles,
},
user: {
users: interfaceConfig?.peoplePicker?.user?.users ?? defaults.peoplePicker?.user.users,
groups: interfaceConfig?.peoplePicker?.user?.groups ?? defaults.peoplePicker?.user.groups,
roles: interfaceConfig?.peoplePicker?.user?.roles ?? defaults.peoplePicker?.user.roles,
},
},
marketplace: {
@ -88,6 +90,7 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol
[PermissionTypes.PEOPLE_PICKER]: {
[Permissions.VIEW_USERS]: loadedInterface.peoplePicker.user?.users,
[Permissions.VIEW_GROUPS]: loadedInterface.peoplePicker.user?.groups,
[Permissions.VIEW_ROLES]: loadedInterface.peoplePicker.user?.roles,
},
[PermissionTypes.MARKETPLACE]: {
[Permissions.USE]: loadedInterface.marketplace.user?.use,
@ -110,6 +113,7 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol
[PermissionTypes.PEOPLE_PICKER]: {
[Permissions.VIEW_USERS]: loadedInterface.peoplePicker.admin?.users,
[Permissions.VIEW_GROUPS]: loadedInterface.peoplePicker.admin?.groups,
[Permissions.VIEW_ROLES]: loadedInterface.peoplePicker.admin?.roles,
},
[PermissionTypes.MARKETPLACE]: {
[Permissions.USE]: loadedInterface.marketplace.admin?.use,