feat: multi role permission intersection (#15150)
Implements permission intersection (AND logic) to prevent permission escalation when agents act on behalf of users. ### Changes: - **Permission Intersection**: Operations requiring both user AND agent permissions - **RoleContext Type**: Unified type supporting single `roleId` or multiple `roleIds` for intersection - **CRUD Services**: Updated to accept `roleContext` for granular permission control - **Agent Integration**: Chat agents now use user + agent role intersection for all operations - **ORM Layer**: Enhanced `getRepository` to support multi-role permission checks ### Related: - Part 2 of ["Acting on behalf of user" concept PR](https://github.com/twentyhq/twenty/pull/15103) [Closes #1661](https://github.com/twentyhq/core-team-issues/issues/1661) --------- Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
This commit is contained in:
+1
-1
@@ -285,7 +285,7 @@ describe('McpService', () => {
|
||||
});
|
||||
|
||||
expect(toolService.listTools).toHaveBeenCalledWith(
|
||||
mockAdminRoleId,
|
||||
{ unionOf: [mockAdminRoleId] },
|
||||
mockWorkspace.id,
|
||||
);
|
||||
expect(mockTool.execute).toHaveBeenCalledWith(
|
||||
|
||||
+11
-6
@@ -9,6 +9,7 @@ import { type ToolInput } from 'src/engine/core-modules/tool/types/tool-input.ty
|
||||
import { type Tool } from 'src/engine/core-modules/tool/types/tool.type';
|
||||
import { PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants';
|
||||
import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
const createMockToolRegistry = () => ({
|
||||
getAllToolTypes: jest.fn(),
|
||||
@@ -18,7 +19,7 @@ const createMockToolRegistry = () => ({
|
||||
const createMockPermissions = () => ({
|
||||
hasToolPermission: jest.fn<
|
||||
Promise<boolean>,
|
||||
[string, string, PermissionFlagType]
|
||||
[RolePermissionConfig, string, PermissionFlagType]
|
||||
>(),
|
||||
});
|
||||
|
||||
@@ -89,7 +90,9 @@ describe('ToolAdapterService', () => {
|
||||
|
||||
expect(Object.keys(toolsNoContext)).toContain('http_request');
|
||||
|
||||
const toolsWithPartialContext = await service.getTools('role-1');
|
||||
const toolsWithPartialContext = await service.getTools({
|
||||
unionOf: ['role-1'],
|
||||
});
|
||||
|
||||
expect(Object.keys(toolsWithPartialContext)).toContain('http_request');
|
||||
});
|
||||
@@ -99,7 +102,9 @@ describe('ToolAdapterService', () => {
|
||||
|
||||
expect(Object.keys(toolsNoContext)).not.toContain('send_email');
|
||||
|
||||
const toolsRoleOnly = await service.getTools('role-1');
|
||||
const toolsRoleOnly = await service.getTools({
|
||||
unionOf: ['role-1'],
|
||||
});
|
||||
|
||||
expect(Object.keys(toolsRoleOnly)).not.toContain('send_email');
|
||||
|
||||
@@ -111,10 +116,10 @@ describe('ToolAdapterService', () => {
|
||||
it('should include flagged tools when permission is granted', async () => {
|
||||
mockPermissions.hasToolPermission.mockResolvedValueOnce(true);
|
||||
|
||||
const tools = await service.getTools('role-1', 'ws-1');
|
||||
const tools = await service.getTools({ unionOf: ['role-1'] }, 'ws-1');
|
||||
|
||||
expect(mockPermissions.hasToolPermission).toHaveBeenCalledWith(
|
||||
'role-1',
|
||||
{ unionOf: ['role-1'] },
|
||||
'ws-1',
|
||||
PermissionFlagType.SEND_EMAIL_TOOL,
|
||||
);
|
||||
@@ -125,7 +130,7 @@ describe('ToolAdapterService', () => {
|
||||
it('should exclude flagged tools when permission is denied', async () => {
|
||||
mockPermissions.hasToolPermission.mockResolvedValueOnce(false);
|
||||
|
||||
const tools = await service.getTools('role-1', 'ws-1');
|
||||
const tools = await service.getTools({ unionOf: ['role-1'] }, 'ws-1');
|
||||
|
||||
expect(Object.keys(tools)).not.toContain('send_email');
|
||||
});
|
||||
|
||||
+5
-5
@@ -69,10 +69,10 @@ describe('ToolService', () => {
|
||||
data: {
|
||||
[roleId]: {
|
||||
[testObject.id]: {
|
||||
canRead: true,
|
||||
canUpdate: true,
|
||||
canSoftDelete: true,
|
||||
canDestroy: false,
|
||||
canReadObjectRecords: true,
|
||||
canUpdateObjectRecords: true,
|
||||
canSoftDeleteObjectRecords: true,
|
||||
canDestroyObjectRecords: false,
|
||||
restrictedFields: {},
|
||||
},
|
||||
},
|
||||
@@ -128,7 +128,7 @@ describe('ToolService', () => {
|
||||
|
||||
describe('listTools', () => {
|
||||
it('should return tools based on role permissions', async () => {
|
||||
const tools = await service.listTools(roleId, workspaceId);
|
||||
const tools = await service.listTools({ unionOf: [roleId] }, workspaceId);
|
||||
|
||||
expect(
|
||||
permissionsCacheService.getRolesPermissionsFromCache,
|
||||
|
||||
@@ -124,7 +124,10 @@ export class McpService {
|
||||
apiKey,
|
||||
);
|
||||
|
||||
const toolSet = await this.toolService.listTools(roleId, workspace.id);
|
||||
const toolSet = await this.toolService.listTools(
|
||||
{ unionOf: [roleId] },
|
||||
workspace.id,
|
||||
);
|
||||
|
||||
if (method === 'tools/call' && params) {
|
||||
return await this.handleToolCall(id, toolSet, params);
|
||||
|
||||
@@ -7,6 +7,7 @@ import { type ToolInput } from 'src/engine/core-modules/tool/types/tool-input.ty
|
||||
import { type Tool } from 'src/engine/core-modules/tool/types/tool.type';
|
||||
import { type PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants';
|
||||
import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
@Injectable()
|
||||
export class ToolAdapterService {
|
||||
@@ -15,7 +16,10 @@ export class ToolAdapterService {
|
||||
private readonly permissionsService: PermissionsService,
|
||||
) {}
|
||||
|
||||
async getTools(roleId?: string, workspaceId?: string): Promise<ToolSet> {
|
||||
async getTools(
|
||||
rolePermissionConfig?: RolePermissionConfig,
|
||||
workspaceId?: string,
|
||||
): Promise<ToolSet> {
|
||||
const tools: ToolSet = {};
|
||||
|
||||
for (const toolType of this.toolRegistry.getAllToolTypes()) {
|
||||
@@ -23,9 +27,9 @@ export class ToolAdapterService {
|
||||
|
||||
if (!tool.flag) {
|
||||
tools[toolType.toLowerCase()] = this.createToolSet(tool);
|
||||
} else if (roleId && workspaceId) {
|
||||
} else if (rolePermissionConfig && workspaceId) {
|
||||
const hasPermission = await this.permissionsService.hasToolPermission(
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
workspaceId,
|
||||
tool.flag as PermissionFlagType,
|
||||
);
|
||||
|
||||
@@ -18,6 +18,8 @@ import { type ActorMetadata } from 'src/engine/metadata-modules/field-metadata/c
|
||||
import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metadata/object-metadata.service';
|
||||
import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
import { computePermissionIntersection } from 'src/engine/twenty-orm/utils/compute-permission-intersection.util';
|
||||
|
||||
@Injectable()
|
||||
export class ToolService {
|
||||
@@ -32,7 +34,7 @@ export class ToolService {
|
||||
) {}
|
||||
|
||||
async listTools(
|
||||
roleId: string,
|
||||
rolePermissionConfig: RolePermissionConfig,
|
||||
workspaceId: string,
|
||||
actorContext?: ActorMetadata,
|
||||
): Promise<ToolSet> {
|
||||
@@ -43,7 +45,29 @@ export class ToolService {
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
const objectPermissions = rolesPermissions[roleId];
|
||||
let objectPermissions;
|
||||
|
||||
if ('unionOf' in rolePermissionConfig) {
|
||||
if (rolePermissionConfig.unionOf.length === 1) {
|
||||
objectPermissions = rolesPermissions[rolePermissionConfig.unionOf[0]];
|
||||
} else {
|
||||
// TODO: Implement union logic for multiple roles
|
||||
throw new Error(
|
||||
'Union permission logic for multiple roles not yet implemented',
|
||||
);
|
||||
}
|
||||
} else if ('intersectionOf' in rolePermissionConfig) {
|
||||
const allRolePermissions = rolePermissionConfig.intersectionOf.map(
|
||||
(roleId: string) => rolesPermissions[roleId],
|
||||
);
|
||||
|
||||
objectPermissions =
|
||||
allRolePermissions.length === 1
|
||||
? allRolePermissions[0]
|
||||
: computePermissionIntersection(allRolePermissions);
|
||||
} else {
|
||||
return tools;
|
||||
}
|
||||
|
||||
const allObjectMetadata =
|
||||
await this.objectMetadataService.findManyWithinWorkspace(workspaceId, {
|
||||
@@ -65,7 +89,7 @@ export class ToolService {
|
||||
return;
|
||||
}
|
||||
|
||||
if (objectPermission.canUpdate) {
|
||||
if (objectPermission.canUpdateObjectRecords) {
|
||||
tools[`create_${objectMetadata.nameSingular}`] = {
|
||||
description: `Create a new ${objectMetadata.labelSingular} record. Provide all required fields and any optional fields you want to set. The system will automatically handle timestamps and IDs. Returns the created record with all its data.`,
|
||||
inputSchema: getRecordInputSchema(objectMetadata),
|
||||
@@ -74,7 +98,7 @@ export class ToolService {
|
||||
objectName: objectMetadata.nameSingular,
|
||||
objectRecord: parameters.input,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
createdBy: actorContext,
|
||||
});
|
||||
},
|
||||
@@ -91,13 +115,13 @@ export class ToolService {
|
||||
objectRecordId: id,
|
||||
objectRecord,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
if (objectPermission.canRead) {
|
||||
if (objectPermission.canReadObjectRecords) {
|
||||
tools[`find_${objectMetadata.nameSingular}`] = {
|
||||
description: `Search for ${objectMetadata.labelSingular} records using flexible filtering criteria. Supports exact matches, pattern matching, ranges, and null checks. Use limit/offset for pagination. Returns an array of matching records with their full data.`,
|
||||
inputSchema: generateFindToolSchema(objectMetadata),
|
||||
@@ -110,7 +134,7 @@ export class ToolService {
|
||||
limit,
|
||||
offset,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
},
|
||||
};
|
||||
@@ -124,13 +148,13 @@ export class ToolService {
|
||||
filter: { id: { eq: parameters.input.id } },
|
||||
limit: 1,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
if (objectPermission.canSoftDelete) {
|
||||
if (objectPermission.canSoftDeleteObjectRecords) {
|
||||
tools[`soft_delete_${objectMetadata.nameSingular}`] = {
|
||||
description: `Soft delete a ${objectMetadata.labelSingular} record by marking it as deleted. The record remains in the database but is hidden from normal queries. This is reversible and preserves all data. Use this for temporary removal.`,
|
||||
inputSchema: generateSoftDeleteToolSchema(),
|
||||
@@ -139,7 +163,7 @@ export class ToolService {
|
||||
objectName: objectMetadata.nameSingular,
|
||||
objectRecordId: parameters.input.id,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
soft: true,
|
||||
});
|
||||
},
|
||||
@@ -153,7 +177,7 @@ export class ToolService {
|
||||
objectMetadata.nameSingular,
|
||||
parameters.input,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
);
|
||||
},
|
||||
};
|
||||
@@ -167,14 +191,14 @@ export class ToolService {
|
||||
objectName: string,
|
||||
parameters: Record<string, unknown>,
|
||||
workspaceId: string,
|
||||
roleId: string,
|
||||
rolePermissionConfig: RolePermissionConfig,
|
||||
) {
|
||||
try {
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectName,
|
||||
{ roleId },
|
||||
rolePermissionConfig,
|
||||
);
|
||||
|
||||
const { filter } = parameters;
|
||||
|
||||
+3
-2
@@ -28,7 +28,8 @@ export class CreateRecordService {
|
||||
) {}
|
||||
|
||||
async execute(params: CreateRecordParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecord, workspaceId, roleId } = params;
|
||||
const { objectName, objectRecord, workspaceId, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
if (!workspaceId) {
|
||||
return {
|
||||
@@ -43,7 +44,7 @@ export class CreateRecordService {
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectName,
|
||||
roleId ? { roleId } : { shouldBypassPermissionChecks: true },
|
||||
rolePermissionConfig,
|
||||
);
|
||||
|
||||
const { objectMetadataItemWithFieldsMaps } =
|
||||
|
||||
+2
-2
@@ -27,7 +27,7 @@ export class DeleteRecordService {
|
||||
objectName,
|
||||
objectRecordId,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
soft = true,
|
||||
} = params;
|
||||
|
||||
@@ -52,7 +52,7 @@ export class DeleteRecordService {
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectName,
|
||||
roleId ? { roleId } : { shouldBypassPermissionChecks: true },
|
||||
rolePermissionConfig,
|
||||
);
|
||||
|
||||
const { objectMetadataItemWithFieldsMaps } =
|
||||
|
||||
+2
-2
@@ -37,7 +37,7 @@ export class FindRecordsService {
|
||||
limit,
|
||||
offset = 0,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
} = params;
|
||||
|
||||
if (!workspaceId) {
|
||||
@@ -53,7 +53,7 @@ export class FindRecordsService {
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectName,
|
||||
roleId ? { roleId } : { shouldBypassPermissionChecks: true },
|
||||
rolePermissionConfig,
|
||||
);
|
||||
|
||||
const { objectMetadataItemWithFieldsMaps, objectMetadataMaps } =
|
||||
|
||||
+2
-2
@@ -32,7 +32,7 @@ export class UpdateRecordService {
|
||||
objectRecord,
|
||||
fieldsToUpdate,
|
||||
workspaceId,
|
||||
roleId,
|
||||
rolePermissionConfig,
|
||||
} = params;
|
||||
|
||||
if (!workspaceId) {
|
||||
@@ -56,7 +56,7 @@ export class UpdateRecordService {
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectName,
|
||||
roleId ? { roleId } : { shouldBypassPermissionChecks: true },
|
||||
rolePermissionConfig,
|
||||
);
|
||||
|
||||
const previousObjectRecord = await repository.findOne({
|
||||
|
||||
+2
-1
@@ -1,10 +1,11 @@
|
||||
import { type ObjectRecordProperties } from 'src/engine/core-modules/record-crud/types/object-record-properties.type';
|
||||
import { type ActorMetadata } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
export type CreateRecordParams = {
|
||||
objectName: string;
|
||||
objectRecord: ObjectRecordProperties;
|
||||
workspaceId: string;
|
||||
roleId?: string;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
createdBy?: ActorMetadata;
|
||||
};
|
||||
|
||||
+3
-1
@@ -1,7 +1,9 @@
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
export type DeleteRecordParams = {
|
||||
objectName: string;
|
||||
objectRecordId: string;
|
||||
workspaceId: string;
|
||||
roleId?: string;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
soft?: boolean;
|
||||
};
|
||||
|
||||
+3
-1
@@ -3,6 +3,8 @@ import {
|
||||
type ObjectRecordOrderBy,
|
||||
} from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';
|
||||
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
export type FindRecordsParams = {
|
||||
objectName: string;
|
||||
filter?:
|
||||
@@ -14,5 +16,5 @@ export type FindRecordsParams = {
|
||||
limit?: number;
|
||||
offset?: number;
|
||||
workspaceId: string;
|
||||
roleId?: string;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
};
|
||||
|
||||
+2
-1
@@ -1,4 +1,5 @@
|
||||
import { type ObjectRecordProperties } from 'src/engine/core-modules/record-crud/types/object-record-properties.type';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
export type UpdateRecordParams = {
|
||||
objectName: string;
|
||||
@@ -6,5 +7,5 @@ export type UpdateRecordParams = {
|
||||
objectRecord: ObjectRecordProperties;
|
||||
fieldsToUpdate?: string[];
|
||||
workspaceId: string;
|
||||
roleId?: string;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user