fix: honor agent rolePermissionConfig in record CRUD (#23248)
## Summary - Agent tools were built with the agent’s `rolePermissionConfig`, but record CRUD ignored it and re-resolved permissions from `authContext` (app `defaultRoleId`) - CRUD services now pass `rolePermissionConfig` through `CommonApiContextBuilder` and the common query runner, so repository access matches the agent role - Workflow/chat paths already use the same role for auth and `rolePermissionConfig`, so their behavior should be unchanged <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23248?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+7
-5
@@ -317,11 +317,13 @@ export abstract class CommonBaseQueryRunnerService<
|
||||
): Promise<Omit<CommonExtendedQueryRunnerContext, 'commonQueryParser'>> {
|
||||
const context = getWorkspaceContext();
|
||||
|
||||
const rolePermissionConfig = resolveRolePermissionConfig({
|
||||
authContext: context.authContext,
|
||||
userWorkspaceRoleMap: context.userWorkspaceRoleMap,
|
||||
apiKeyRoleMap: context.apiKeyRoleMap,
|
||||
});
|
||||
const rolePermissionConfig =
|
||||
queryRunnerContext.rolePermissionConfig ??
|
||||
resolveRolePermissionConfig({
|
||||
authContext: context.authContext,
|
||||
userWorkspaceRoleMap: context.userWorkspaceRoleMap,
|
||||
apiKeyRoleMap: context.apiKeyRoleMap,
|
||||
});
|
||||
|
||||
if (!rolePermissionConfig) {
|
||||
throw new CommonQueryRunnerException(
|
||||
|
||||
+2
@@ -3,6 +3,7 @@ import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/typ
|
||||
import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
|
||||
import { type FlatIndexMetadata } from 'src/engine/metadata-modules/flat-index-metadata/types/flat-index-metadata.type';
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
export type CommonBaseQueryRunnerContext = {
|
||||
authContext: WorkspaceAuthContext;
|
||||
@@ -11,4 +12,5 @@ export type CommonBaseQueryRunnerContext = {
|
||||
flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
|
||||
flatIndexMaps?: FlatEntityMaps<FlatIndexMetadata>;
|
||||
objectIdByNameSingular: Record<string, string>;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
};
|
||||
|
||||
+29
-9
@@ -22,6 +22,8 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
import { buildObjectIdByNameMaps } from 'src/engine/metadata-modules/flat-object-metadata/utils/build-object-id-by-name-maps.util';
|
||||
import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service';
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
import { getObjectsPermissionsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-objects-permissions-from-role-permission-config.util';
|
||||
import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';
|
||||
|
||||
export type CommonApiContext = {
|
||||
@@ -45,9 +47,11 @@ export class CommonApiContextBuilderService {
|
||||
async build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
}: {
|
||||
authContext: WorkspaceAuthContext;
|
||||
objectName: string;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
}): Promise<CommonApiContext> {
|
||||
const workspaceId = authContext.workspace.id;
|
||||
|
||||
@@ -94,7 +98,10 @@ export class CommonApiContextBuilderService {
|
||||
);
|
||||
}
|
||||
|
||||
const objectsPermissions = await this.getObjectsPermissions(authContext);
|
||||
const objectsPermissions = await this.getObjectsPermissions({
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
const restrictedFields =
|
||||
objectsPermissions[flatObjectMetadata.id]?.restrictedFields ?? {};
|
||||
@@ -113,6 +120,7 @@ export class CommonApiContextBuilderService {
|
||||
flatFieldMetadataMaps,
|
||||
flatIndexMaps,
|
||||
objectIdByNameSingular: idByNameSingular,
|
||||
rolePermissionConfig,
|
||||
},
|
||||
selectedFields,
|
||||
flatObjectMetadata,
|
||||
@@ -122,10 +130,27 @@ export class CommonApiContextBuilderService {
|
||||
};
|
||||
}
|
||||
|
||||
private async getObjectsPermissions(
|
||||
authContext: WorkspaceAuthContext,
|
||||
): Promise<ObjectsPermissions> {
|
||||
private async getObjectsPermissions({
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
}: {
|
||||
authContext: WorkspaceAuthContext;
|
||||
rolePermissionConfig?: RolePermissionConfig;
|
||||
}): Promise<ObjectsPermissions> {
|
||||
const workspaceId = authContext.workspace.id;
|
||||
|
||||
const { rolesPermissions } =
|
||||
await this.workspaceCacheService.getOrRecompute(workspaceId, [
|
||||
'rolesPermissions',
|
||||
]);
|
||||
|
||||
if (isDefined(rolePermissionConfig)) {
|
||||
return getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
}
|
||||
|
||||
let roleId: string;
|
||||
|
||||
if (isApiKeyAuthContext(authContext)) {
|
||||
@@ -160,11 +185,6 @@ export class CommonApiContextBuilderService {
|
||||
);
|
||||
}
|
||||
|
||||
const { rolesPermissions } =
|
||||
await this.workspaceCacheService.getOrRecompute(workspaceId, [
|
||||
'rolesPermissions',
|
||||
]);
|
||||
|
||||
return rolesPermissions[roleId] ?? {};
|
||||
}
|
||||
}
|
||||
|
||||
+3
-1
@@ -24,7 +24,8 @@ export class CreateManyRecordsService {
|
||||
) {}
|
||||
|
||||
async execute(params: CreateManyRecordsParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecords, authContext } = params;
|
||||
const { objectName, objectRecords, authContext, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
try {
|
||||
const {
|
||||
@@ -35,6 +36,7 @@ export class CreateManyRecordsService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+3
-1
@@ -24,7 +24,8 @@ export class CreateRecordService {
|
||||
) {}
|
||||
|
||||
async execute(params: CreateRecordParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecord, authContext } = params;
|
||||
const { objectName, objectRecord, authContext, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
try {
|
||||
const {
|
||||
@@ -35,6 +36,7 @@ export class CreateRecordService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+2
-1
@@ -21,7 +21,7 @@ export class DeleteManyRecordsService {
|
||||
) {}
|
||||
|
||||
async execute(params: DeleteManyRecordsParams): Promise<ToolOutput> {
|
||||
const { objectName, filter, authContext } = params;
|
||||
const { objectName, filter, authContext, rolePermissionConfig } = params;
|
||||
|
||||
if (!isDefined(filter) || isEmptyObject(filter)) {
|
||||
return {
|
||||
@@ -37,6 +37,7 @@ export class DeleteManyRecordsService {
|
||||
await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+8
-1
@@ -24,7 +24,13 @@ export class DeleteRecordService {
|
||||
) {}
|
||||
|
||||
async execute(params: DeleteRecordParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecordId, authContext, soft = true } = params;
|
||||
const {
|
||||
objectName,
|
||||
objectRecordId,
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
soft = true,
|
||||
} = params;
|
||||
|
||||
if (!isDefined(objectRecordId) || !isValidUuid(objectRecordId)) {
|
||||
return {
|
||||
@@ -39,6 +45,7 @@ export class DeleteRecordService {
|
||||
await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+2
@@ -34,6 +34,7 @@ export class FindRecordsService {
|
||||
limit,
|
||||
offset = 0,
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
select,
|
||||
shouldBuildEffectiveSelectFields,
|
||||
} = params;
|
||||
@@ -55,6 +56,7 @@ export class FindRecordsService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
const { effectiveSelectedFields, warnings } =
|
||||
|
||||
+2
@@ -41,6 +41,7 @@ export class GroupByRecordsService {
|
||||
orderBy = 'DESC',
|
||||
filter,
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
} = params;
|
||||
|
||||
try {
|
||||
@@ -52,6 +53,7 @@ export class GroupByRecordsService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
const availableAggregations =
|
||||
|
||||
+3
-1
@@ -23,7 +23,8 @@ export class UpdateManyRecordsService {
|
||||
) {}
|
||||
|
||||
async execute(params: UpdateManyRecordsParams): Promise<ToolOutput> {
|
||||
const { objectName, filter, data, authContext } = params;
|
||||
const { objectName, filter, data, authContext, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
try {
|
||||
const {
|
||||
@@ -34,6 +35,7 @@ export class UpdateManyRecordsService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+2
@@ -30,6 +30,7 @@ export class UpdateRecordService {
|
||||
objectRecord,
|
||||
fieldsToUpdate,
|
||||
authContext,
|
||||
rolePermissionConfig,
|
||||
} = params;
|
||||
|
||||
if (!isDefined(objectRecordId) || !isValidUuid(objectRecordId)) {
|
||||
@@ -49,6 +50,7 @@ export class UpdateRecordService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+3
-1
@@ -25,7 +25,8 @@ export class UpsertManyRecordsService {
|
||||
) {}
|
||||
|
||||
async execute(params: UpsertManyRecordsParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecords, authContext } = params;
|
||||
const { objectName, objectRecords, authContext, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
try {
|
||||
const {
|
||||
@@ -36,6 +37,7 @@ export class UpsertManyRecordsService {
|
||||
} = await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+3
-1
@@ -22,13 +22,15 @@ export class UpsertRecordService {
|
||||
) {}
|
||||
|
||||
async execute(params: UpsertRecordParams): Promise<ToolOutput> {
|
||||
const { objectName, objectRecord, authContext } = params;
|
||||
const { objectName, objectRecord, authContext, rolePermissionConfig } =
|
||||
params;
|
||||
|
||||
try {
|
||||
const { queryRunnerContext, selectedFields, flatObjectMetadata } =
|
||||
await this.commonApiContextBuilder.build({
|
||||
authContext,
|
||||
objectName,
|
||||
rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (
|
||||
|
||||
+5
-36
@@ -1,9 +1,5 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import {
|
||||
type ObjectsPermissions,
|
||||
type ObjectsPermissionsByRoleId,
|
||||
} from 'twenty-shared/types';
|
||||
import { camelToSnakeCase, isDefined } from 'twenty-shared/utils';
|
||||
import { canObjectBeManagedByAutomation } from 'twenty-shared/workflow';
|
||||
|
||||
@@ -32,7 +28,7 @@ import { type ToolIndexEntry } from 'src/engine/core-modules/tool-provider/types
|
||||
import { type ToolOutput } from 'src/engine/core-modules/tool/types/tool-output.type';
|
||||
import { getDatabaseCrudToolFlatObjects } from 'src/engine/metadata-modules/ai/ai-agent/utils/get-database-crud-tool-flat-objects.util';
|
||||
import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service';
|
||||
import { computePermissionIntersection } from 'src/engine/twenty-orm/utils/compute-permission-intersection.util';
|
||||
import { getObjectsPermissionsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-objects-permissions-from-role-permission-config.util';
|
||||
import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';
|
||||
import { ToolCategory } from 'twenty-shared/ai';
|
||||
|
||||
@@ -77,12 +73,12 @@ export class DatabaseToolProvider implements ToolProvider {
|
||||
'rolesPermissions',
|
||||
]);
|
||||
|
||||
const objectPermissions = this.getObjectPermissions(
|
||||
const objectPermissions = getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
context.rolePermissionConfig,
|
||||
);
|
||||
rolePermissionConfig: context.rolePermissionConfig,
|
||||
});
|
||||
|
||||
if (!objectPermissions) {
|
||||
if (Object.keys(objectPermissions).length === 0) {
|
||||
return descriptors;
|
||||
}
|
||||
|
||||
@@ -427,31 +423,4 @@ export class DatabaseToolProvider implements ToolProvider {
|
||||
toolNames.has(`upsert_many_${snakePlural}`)
|
||||
);
|
||||
}
|
||||
|
||||
private getObjectPermissions(
|
||||
rolesPermissions: ObjectsPermissionsByRoleId,
|
||||
rolePermissionConfig: ToolProviderContext['rolePermissionConfig'],
|
||||
): ObjectsPermissions | null {
|
||||
if ('intersectionOf' in rolePermissionConfig) {
|
||||
const allRolePermissions = rolePermissionConfig.intersectionOf.map(
|
||||
(roleId: string) => rolesPermissions[roleId],
|
||||
);
|
||||
|
||||
return allRolePermissions.length === 1
|
||||
? allRolePermissions[0]
|
||||
: computePermissionIntersection(allRolePermissions);
|
||||
}
|
||||
|
||||
if ('unionOf' in rolePermissionConfig) {
|
||||
if (rolePermissionConfig.unionOf.length === 1) {
|
||||
return rolesPermissions[rolePermissionConfig.unionOf[0]];
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
'Union permission logic for multiple roles not yet implemented',
|
||||
);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -132,7 +132,7 @@ describe('AgentAsyncExecutorService — workflow agent role-scoped tool resoluti
|
||||
service = module.get<AgentAsyncExecutorService>(AgentAsyncExecutorService);
|
||||
});
|
||||
|
||||
it('passes unionOf: [agentRoleId] when the agent has a role assigned', async () => {
|
||||
it('passes intersectionOf: [agentRoleId] when the agent has a role assigned', async () => {
|
||||
roleTargetRepository.findOne.mockResolvedValueOnce({ roleId: agentRoleId });
|
||||
|
||||
await service.executeAgent({
|
||||
@@ -145,7 +145,7 @@ describe('AgentAsyncExecutorService — workflow agent role-scoped tool resoluti
|
||||
expect(toolRegistry.getToolsByCategories).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
roleId: agentRoleId,
|
||||
rolePermissionConfig: { unionOf: [agentRoleId] },
|
||||
rolePermissionConfig: { intersectionOf: [agentRoleId] },
|
||||
workspaceId,
|
||||
}),
|
||||
expect.objectContaining({ wrapWithErrorContext: false }),
|
||||
|
||||
+1
-1
@@ -176,7 +176,7 @@ export class AgentAsyncExecutorService {
|
||||
// permission-tab role. No role means no registry tools.
|
||||
if (isDefined(agentRoleId)) {
|
||||
const agentRolePermissionConfig: RolePermissionConfig = {
|
||||
unionOf: [agentRoleId],
|
||||
intersectionOf: [agentRoleId],
|
||||
};
|
||||
|
||||
const toolProviderContext: ToolProviderContext = {
|
||||
|
||||
+83
@@ -0,0 +1,83 @@
|
||||
import { type ObjectsPermissionsByRoleId } from 'twenty-shared/types';
|
||||
|
||||
import { getObjectsPermissionsFromRolePermissionConfig } from 'src/engine/twenty-orm/utils/get-objects-permissions-from-role-permission-config.util';
|
||||
|
||||
const OBJECT_ID = 'object-1';
|
||||
|
||||
const agentRolePermissions = {
|
||||
[OBJECT_ID]: {
|
||||
canReadObjectRecords: true,
|
||||
canUpdateObjectRecords: false,
|
||||
canSoftDeleteObjectRecords: false,
|
||||
canDestroyObjectRecords: false,
|
||||
restrictedFields: {},
|
||||
rowLevelPermissionPredicates: [],
|
||||
rowLevelPermissionPredicateGroups: [],
|
||||
},
|
||||
};
|
||||
|
||||
const defaultRolePermissions = {
|
||||
[OBJECT_ID]: {
|
||||
canReadObjectRecords: false,
|
||||
canUpdateObjectRecords: false,
|
||||
canSoftDeleteObjectRecords: false,
|
||||
canDestroyObjectRecords: false,
|
||||
restrictedFields: {},
|
||||
rowLevelPermissionPredicates: [],
|
||||
rowLevelPermissionPredicateGroups: [],
|
||||
},
|
||||
};
|
||||
|
||||
const rolesPermissions: ObjectsPermissionsByRoleId = {
|
||||
'agent-role-id': agentRolePermissions,
|
||||
'default-role-id': defaultRolePermissions,
|
||||
};
|
||||
|
||||
describe('getObjectsPermissionsFromRolePermissionConfig', () => {
|
||||
it('should resolve a single union role', () => {
|
||||
expect(
|
||||
getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig: { unionOf: ['agent-role-id'] },
|
||||
}),
|
||||
).toEqual(agentRolePermissions);
|
||||
});
|
||||
|
||||
it('should resolve a single intersection role', () => {
|
||||
expect(
|
||||
getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig: { intersectionOf: ['default-role-id'] },
|
||||
}),
|
||||
).toEqual(defaultRolePermissions);
|
||||
});
|
||||
|
||||
it('should use the first role when multiple are provided', () => {
|
||||
expect(
|
||||
getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig: {
|
||||
intersectionOf: ['agent-role-id', 'default-role-id'],
|
||||
},
|
||||
}),
|
||||
).toEqual(agentRolePermissions);
|
||||
});
|
||||
|
||||
it('should return empty permissions when bypassing checks', () => {
|
||||
expect(
|
||||
getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig: { shouldBypassPermissionChecks: true },
|
||||
}),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
it('should return empty permissions when the role is missing from the cache', () => {
|
||||
expect(
|
||||
getObjectsPermissionsFromRolePermissionConfig({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig: { unionOf: ['missing-role-id'] },
|
||||
}),
|
||||
).toEqual({});
|
||||
});
|
||||
});
|
||||
+33
@@ -0,0 +1,33 @@
|
||||
import {
|
||||
type ObjectsPermissions,
|
||||
type ObjectsPermissionsByRoleId,
|
||||
} from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config';
|
||||
|
||||
// Multi-role union/intersection is not ready — use the first assigned role only.
|
||||
export const getObjectsPermissionsFromRolePermissionConfig = ({
|
||||
rolesPermissions,
|
||||
rolePermissionConfig,
|
||||
}: {
|
||||
rolesPermissions: ObjectsPermissionsByRoleId;
|
||||
rolePermissionConfig: RolePermissionConfig;
|
||||
}): ObjectsPermissions => {
|
||||
if ('shouldBypassPermissionChecks' in rolePermissionConfig) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const roleId =
|
||||
'intersectionOf' in rolePermissionConfig
|
||||
? rolePermissionConfig.intersectionOf[0]
|
||||
: 'unionOf' in rolePermissionConfig
|
||||
? rolePermissionConfig.unionOf[0]
|
||||
: undefined;
|
||||
|
||||
if (!isDefined(roleId)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return rolesPermissions[roleId] ?? {};
|
||||
};
|
||||
Reference in New Issue
Block a user