Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions api/server/controllers/PermissionsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

const mongoose = require('mongoose');
const { logger } = require('@librechat/data-schemas');
const { logger, getTenantId, SYSTEM_TENANT_ID } = require('@librechat/data-schemas');
const { ResourceType, PrincipalType, PermissionBits } = require('librechat-data-provider');
const { enrichRemoteAgentPrincipals, backfillRemoteAgentPermissions } = require('@librechat/api');
const {
Expand All @@ -21,6 +21,13 @@ const {
} = require('~/server/services/GraphApiService');
const db = require('~/models');

const matchesCurrentTenant = (principal, tenantId) => {
if (!tenantId || tenantId === SYSTEM_TENANT_ID) {
return true;
}
return principal?.tenantId === tenantId;
};

/**
* Generic controller for resource permission endpoints
* Delegates validation and logic to PermissionService
Expand Down Expand Up @@ -191,6 +198,7 @@ const getResourcePermissions = async (req, res) => {
try {
const { resourceType, resourceId } = req.params;
validateResourceType(resourceType);
const tenantId = getTenantId();

const results = await db.aggregateAclEntries([
// Match ACL entries for this resource
Expand Down Expand Up @@ -244,14 +252,17 @@ const getResourcePermissions = async (req, res) => {
let principals = [];
let publicPermission = null;

// Process aggregation results
for (const result of results) {
if (result.principalType === PrincipalType.PUBLIC) {
publicPermission = {
public: true,
publicAccessRoleId: result.accessRoleId,
};
} else if (result.principalType === PrincipalType.USER && result.userInfo) {
} else if (
result.principalType === PrincipalType.USER &&
result.userInfo &&
matchesCurrentTenant(result.userInfo, tenantId)
) {
principals.push({
type: PrincipalType.USER,
id: result.userInfo._id.toString(),
Expand All @@ -262,7 +273,11 @@ const getResourcePermissions = async (req, res) => {
idOnTheSource: result.userInfo.idOnTheSource || result.userInfo._id.toString(),
accessRoleId: result.accessRoleId,
});
} else if (result.principalType === PrincipalType.GROUP && result.groupInfo) {
} else if (
result.principalType === PrincipalType.GROUP &&
result.groupInfo &&
matchesCurrentTenant(result.groupInfo, tenantId)
) {
principals.push({
type: PrincipalType.GROUP,
id: result.groupInfo._id.toString(),
Expand Down
116 changes: 114 additions & 2 deletions api/server/controllers/__tests__/PermissionsController.spec.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
const mongoose = require('mongoose');

const mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() };
const mockGetTenantId = jest.fn();

jest.mock('@librechat/data-schemas', () => ({
logger: mockLogger,
getTenantId: mockGetTenantId,
SYSTEM_TENANT_ID: '__SYSTEM__',
}));

const { ResourceType, PrincipalType } = jest.requireActual('librechat-data-provider');
const { AccessRoleIds, ResourceType, PrincipalType } =
jest.requireActual('librechat-data-provider');

jest.mock('librechat-data-provider', () => ({
...jest.requireActual('librechat-data-provider'),
Expand All @@ -32,6 +36,7 @@ jest.mock('~/server/services/PermissionService', () => ({
const mockRemoveAgentFromUserFavorites = jest.fn();

jest.mock('~/models', () => ({
aggregateAclEntries: jest.fn(),
searchPrincipals: jest.fn(),
sortPrincipalsByRelevance: jest.fn(),
calculateRelevanceScore: jest.fn(),
Expand All @@ -44,7 +49,11 @@ jest.mock('~/server/services/GraphApiService', () => ({
}));

const db = require('~/models');
const { updateResourcePermissions, searchPrincipals } = require('../PermissionsController');
const {
updateResourcePermissions,
searchPrincipals,
getResourcePermissions,
} = require('../PermissionsController');

const createMockReq = (overrides = {}) => ({
params: { resourceType: ResourceType.AGENT, resourceId: '507f1f77bcf86cd799439011' },
Expand All @@ -66,6 +75,7 @@ const flushPromises = () => new Promise((resolve) => setImmediate(resolve));
describe('PermissionsController', () => {
beforeEach(() => {
jest.clearAllMocks();
mockGetTenantId.mockReturnValue(undefined);
});

describe('searchPrincipals', () => {
Expand Down Expand Up @@ -139,6 +149,108 @@ describe('PermissionsController', () => {
});
});

describe('getResourcePermissions — principal details', () => {
const currentTenantId = 'tenant-a';
const otherTenantId = 'tenant-b';
const userId = new mongoose.Types.ObjectId();
const groupId = new mongoose.Types.ObjectId();

it('omits joined user and group details outside the current request context', async () => {
mockGetTenantId.mockReturnValue(currentTenantId);
db.aggregateAclEntries.mockResolvedValue([
{
principalType: PrincipalType.USER,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
userInfo: {
_id: userId,
tenantId: otherTenantId,
name: 'Outside User',
email: 'outside-user@example.com',
avatar: 'outside-user.png',
},
},
{
principalType: PrincipalType.GROUP,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
groupInfo: {
_id: groupId,
tenantId: otherTenantId,
name: 'Outside Group',
email: 'outside-group@example.com',
avatar: 'outside-group.png',
},
},
{
principalType: PrincipalType.PUBLIC,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
},
]);

const req = createMockReq();
const res = createMockRes();

await getResourcePermissions(req, res);

expect(res.status).toHaveBeenCalledWith(200);
expect(res.json).toHaveBeenCalledWith({
resourceType: ResourceType.AGENT,
resourceId: req.params.resourceId,
principals: [],
public: true,
publicAccessRoleId: AccessRoleIds.AGENT_VIEWER,
});
expect(JSON.stringify(res.json.mock.calls[0][0])).not.toContain('outside-user@example.com');
expect(JSON.stringify(res.json.mock.calls[0][0])).not.toContain('outside-group@example.com');
});

it('includes joined user and group details in the current request context', async () => {
mockGetTenantId.mockReturnValue(currentTenantId);
db.aggregateAclEntries.mockResolvedValue([
{
principalType: PrincipalType.USER,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
userInfo: {
_id: userId,
tenantId: currentTenantId,
name: 'Current User',
email: 'current-user@example.com',
avatar: 'current-user.png',
},
},
{
principalType: PrincipalType.GROUP,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
groupInfo: {
_id: groupId,
tenantId: currentTenantId,
name: 'Current Group',
email: 'current-group@example.com',
avatar: 'current-group.png',
},
},
]);

const req = createMockReq();
const res = createMockRes();

await getResourcePermissions(req, res);

expect(res.status).toHaveBeenCalledWith(200);
expect(res.json.mock.calls[0][0].principals).toEqual([
expect.objectContaining({
type: PrincipalType.USER,
id: userId.toString(),
email: 'current-user@example.com',
}),
expect.objectContaining({
type: PrincipalType.GROUP,
id: groupId.toString(),
email: 'current-group@example.com',
}),
]);
});
});

describe('updateResourcePermissions — favorites cleanup', () => {
const agentObjectId = new mongoose.Types.ObjectId().toString();
const revokedUserId = new mongoose.Types.ObjectId().toString();
Expand Down
24 changes: 22 additions & 2 deletions api/server/services/PermissionService.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,22 @@ const validateResourceType = (resourceType) => {
}
};

const ensureLocalUserPrincipalExists = async (principalId) => {
const user = await db.findUser({ _id: principalId }, '_id');
if (!user) {
throw new Error('User principal not found');
}
return user._id.toString();
};

const ensureLocalGroupPrincipalExists = async (principalId) => {
const group = await db.findGroupById(principalId, { _id: 1 });
if (!group) {
throw new Error('Group principal not found');
}
return group._id.toString();
};

/**
* @import { TPrincipal } from 'librechat-data-provider'
*/
Expand Down Expand Up @@ -300,8 +316,8 @@ const ensurePrincipalExists = async function (principal) {
return null;
}

if (principal.id) {
return principal.id;
if (principal.type === PrincipalType.USER && principal.id) {
return await ensureLocalUserPrincipalExists(principal.id);
}

if (principal.type === PrincipalType.USER && principal.source === 'entra') {
Expand Down Expand Up @@ -366,6 +382,10 @@ const ensureGroupPrincipalExists = async function (principal, authContext = null
throw new Error(`Invalid principal type: ${principal.type}. Expected '${PrincipalType.GROUP}'`);
}

if (principal.id && principal.source !== 'entra') {
return await ensureLocalGroupPrincipalExists(principal.id);
}

if (principal.source === 'entra') {
if (!principal.name || !principal.idOnTheSource) {
throw new Error('Entra ID group principals must have name and idOnTheSource');
Expand Down
71 changes: 70 additions & 1 deletion api/server/services/PermissionService.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const mongoose = require('mongoose');
const { RoleBits, createModels } = require('@librechat/data-schemas');
const { RoleBits, createModels, tenantStorage } = require('@librechat/data-schemas');
const { MongoMemoryServer } = require('mongodb-memory-server');
const {
ResourceType,
Expand All @@ -15,6 +15,8 @@ const {
getAvailableRoles,
grantPermission,
checkPermission,
ensurePrincipalExists,
ensureGroupPrincipalExists,
} = require('./PermissionService');
const { findRoleByIdentifier, getUserPrincipals, seedDefaultRoles } = require('~/models');

Expand Down Expand Up @@ -44,6 +46,8 @@ jest.mock('~/config', () => ({

let mongoServer;
let AclEntry;
let User;
let Group;

beforeAll(async () => {
mongoServer = await MongoMemoryServer.create();
Expand All @@ -58,6 +62,8 @@ beforeAll(async () => {
Object.assign(mongoose.models, dbModels);

AclEntry = dbModels.AclEntry;
User = dbModels.User;
Group = dbModels.Group;

// Seed default roles
await seedDefaultRoles();
Expand Down Expand Up @@ -243,6 +249,69 @@ describe('PermissionService', () => {
});
});

describe('principal validation for ACL writes', () => {
beforeEach(async () => {
await User.deleteMany({ email: /acl-principal/i });
await Group.deleteMany({ name: /ACL Principal/i });
});

test('rejects a local user id outside the current request context', async () => {
const outsideUser = await User.create({
name: 'ACL Principal Outside User',
email: 'acl-principal-outside-user@example.com',
tenantId: 'tenant-b',
});

await expect(
tenantStorage.run({ tenantId: 'tenant-a' }, async () =>
ensurePrincipalExists({
type: PrincipalType.USER,
id: outsideUser._id.toString(),
name: 'Outside User',
source: 'local',
}),
),
).rejects.toThrow('User principal not found');
});

test('accepts a local user id in the current request context', async () => {
const currentUser = await User.create({
name: 'ACL Principal Current User',
email: 'acl-principal-current-user@example.com',
tenantId: 'tenant-a',
});

const principalId = await tenantStorage.run({ tenantId: 'tenant-a' }, async () =>
ensurePrincipalExists({
type: PrincipalType.USER,
id: currentUser._id.toString(),
name: 'Current User',
source: 'local',
}),
);

expect(principalId).toBe(currentUser._id.toString());
});

test('rejects a local group id outside the current request context', async () => {
const outsideGroup = await Group.create({
name: 'ACL Principal Outside Group',
tenantId: 'tenant-b',
});

await expect(
tenantStorage.run({ tenantId: 'tenant-a' }, async () =>
ensureGroupPrincipalExists({
type: PrincipalType.GROUP,
id: outsideGroup._id.toString(),
name: 'Outside Group',
source: 'local',
}),
),
).rejects.toThrow('Group principal not found');
});
});

describe('checkPermission', () => {
let otherResourceId;

Expand Down
Loading
Loading