[permissions] Fix delete and soft-delete + enable fieldPermissions in devSeeds (#13646)

In this PR
1. Fix delete and soft-delete repository methods for repositories where
permission checks are NOT bypassed: they need to have a selection of
columns to return by default. To match what we did for insert I set it
to `'*'` by default. But I feel this may be bug prone as developers will
not necessarily think to fill the right values in. Maybe we should
change the api to use selectable fields by default. @charlesBochet
2. Add field permission seeds and enable field permission feature flag
in dev
This commit is contained in:
Marie
2025-08-07 10:43:28 +02:00
committed by GitHub
parent 9cfd62f1ef
commit 05c23a1297
10 changed files with 122 additions and 6 deletions
@@ -29,7 +29,9 @@ export class RestApiDeleteOneHandler extends RestApiBaseHandler {
select: selectOptions,
});
await repository.delete(recordId);
const columnsToReturnForDelete: string[] = [];
await repository.delete(recordId, undefined, columnsToReturnForDelete);
return this.formatResult({
operation: 'delete',
@@ -289,6 +289,9 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
fieldMetadataInput.workspaceId,
'view',
{
shouldBypassPermissionChecks: true,
},
);
await viewsRepository.delete({
@@ -116,6 +116,9 @@ export class ObjectMetadataRelatedRecordsService {
await this.twentyORMGlobalManager.getRepositoryForWorkspace<ViewWorkspaceEntity>(
workspaceId,
'view',
{
shouldBypassPermissionChecks: true,
},
);
await viewRepository.delete({
@@ -583,6 +583,7 @@ export class WorkspaceEntityManager extends EntityManager {
targetOrEntity: EntityTarget<Entity>,
criteria: unknown,
permissionOptions?: PermissionOptions,
selectedColumns: string[] | '*' = '*',
): Promise<DeleteResult> {
if (
criteria === undefined ||
@@ -611,6 +612,7 @@ export class WorkspaceEntityManager extends EntityManager {
.delete()
.from(targetOrEntity)
.whereInIds(criteria)
.returning(selectedColumns)
.execute();
} else {
return this.createQueryBuilder(
@@ -622,6 +624,7 @@ export class WorkspaceEntityManager extends EntityManager {
.delete()
.from(targetOrEntity)
.where(criteria)
.returning(selectedColumns)
.execute();
}
}
@@ -630,6 +633,7 @@ export class WorkspaceEntityManager extends EntityManager {
targetOrEntity: EntityTarget<Entity>,
criteria: unknown,
permissionOptions?: PermissionOptions,
selectedColumns: string[] | '*' = '*',
): Promise<UpdateResult> {
// if user passed empty criteria or empty list of criterias, then throw an error
if (
@@ -659,6 +663,7 @@ export class WorkspaceEntityManager extends EntityManager {
.softDelete()
.from(targetOrEntity)
.whereInIds(criteria)
.returning(selectedColumns)
.execute();
} else {
return this.createQueryBuilder(
@@ -670,6 +675,7 @@ export class WorkspaceEntityManager extends EntityManager {
.softDelete()
.from(targetOrEntity)
.where(criteria)
.returning(selectedColumns)
.execute();
}
}
@@ -678,6 +684,7 @@ export class WorkspaceEntityManager extends EntityManager {
targetOrEntity: EntityTarget<Entity>,
criteria: unknown,
permissionOptions?: PermissionOptions,
selectedColumns: string[] | '*' = '*',
): Promise<UpdateResult> {
// if user passed empty criteria or empty list of criterias, then throw an error
if (
@@ -707,6 +714,7 @@ export class WorkspaceEntityManager extends EntityManager {
.restore()
.from(targetOrEntity)
.whereInIds(criteria)
.returning(selectedColumns)
.execute();
} else {
return this.createQueryBuilder(
@@ -718,6 +726,7 @@ export class WorkspaceEntityManager extends EntityManager {
.restore()
.from(targetOrEntity)
.where(criteria)
.returning(selectedColumns)
.execute();
}
}
@@ -1237,6 +1246,10 @@ export class WorkspaceEntityManager extends EntityManager {
objectMetadataItem: ObjectMetadataItemWithFieldMaps;
permissionOptionsFromArgs: PermissionOptions | undefined;
}): Entity[] {
if (permissionOptionsFromArgs?.shouldBypassPermissionChecks === true) {
return formattedResult;
}
const restrictedFields =
permissionOptionsFromArgs?.objectRecordsPermissions?.[
objectMetadataItem.id
@@ -248,6 +248,7 @@ describe('WorkspaceRepository', () => {
shouldBypassPermissionChecks: false,
objectRecordsPermissions: mockObjectRecordsPermissions,
},
undefined,
);
expect(result).toEqual(expectedResult);
});
@@ -332,7 +332,7 @@ const getSelectedColumnsFromExpressionMap = ({
operationType,
)
) {
if (isEmpty(expressionMap.returning)) {
if (!isDefined(expressionMap.returning)) {
throw new InternalServerError(
'Returning columns are not set for update query',
);
@@ -348,6 +348,7 @@ export class WorkspaceRepository<
| ObjectId[]
| FindOptionsWhere<T>,
entityManager?: WorkspaceEntityManager,
selectedColumns?: string[] | '*',
): Promise<DeleteResult> {
const manager = entityManager || this.manager;
@@ -360,7 +361,12 @@ export class WorkspaceRepository<
objectRecordsPermissions: this.objectRecordsPermissions,
};
return manager.delete(this.target, criteria, permissionOptions);
return manager.delete(
this.target,
criteria,
permissionOptions,
selectedColumns,
);
}
override softRemove<U extends DeepPartial<T>>(
@@ -431,6 +437,7 @@ export class WorkspaceRepository<
| ObjectId[]
| FindOptionsWhere<T>,
entityManager?: WorkspaceEntityManager,
selectedColumns?: string[],
): Promise<UpdateResult> {
const manager = entityManager || this.manager;
@@ -443,7 +450,12 @@ export class WorkspaceRepository<
objectRecordsPermissions: this.objectRecordsPermissions,
};
return manager.softDelete(this.target, criteria, permissionOptions);
return manager.softDelete(
this.target,
criteria,
permissionOptions,
selectedColumns,
);
}
/**
@@ -517,6 +529,7 @@ export class WorkspaceRepository<
| ObjectId[]
| FindOptionsWhere<T>,
entityManager?: WorkspaceEntityManager,
selectedColumns?: string[],
): Promise<UpdateResult> {
const manager = entityManager || this.manager;
@@ -529,7 +542,12 @@ export class WorkspaceRepository<
objectRecordsPermissions: this.objectRecordsPermissions,
};
return manager.restore(this.target, criteria, permissionOptions);
return manager.restore(
this.target,
criteria,
permissionOptions,
selectedColumns,
);
}
/**
@@ -7,6 +7,7 @@ import { Repository } from 'typeorm';
import { TypeORMService } from 'src/database/typeorm/typeorm.service';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { FieldPermissionService } from 'src/engine/metadata-modules/object-permission/field-permission/field-permission.service';
import { ObjectPermissionService } from 'src/engine/metadata-modules/object-permission/object-permission.service';
import { RoleService } from 'src/engine/metadata-modules/role/role.service';
import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service';
@@ -30,6 +31,7 @@ export class DevSeederPermissionsService {
private readonly objectMetadataRepository: Repository<ObjectMetadataEntity>,
private readonly typeORMService: TypeORMService,
private readonly workspacePermissionsCacheService: WorkspacePermissionsCacheService,
private readonly fieldPermissionService: FieldPermissionService,
) {}
public async initPermissions(workspaceId: string) {
@@ -175,6 +177,28 @@ export class DevSeederPermissionsService {
},
});
const personObjectMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: {
nameSingular: 'person',
workspaceId,
},
relations: {
fields: true,
},
});
const companyObjectMetadata =
await this.objectMetadataRepository.findOneOrFail({
where: {
nameSingular: 'company',
workspaceId,
},
relations: {
fields: true,
},
});
await this.objectPermissionService.upsertObjectPermissions({
workspaceId,
input: {
@@ -198,6 +222,47 @@ export class DevSeederPermissionsService {
},
});
const personCityFieldMetadata = personObjectMetadata.fields.find(
(field) => field.name === 'city',
);
if (!personCityFieldMetadata) {
throw new Error('Person city field metadata not found');
}
const companyLinkedinLinkFieldMetadata = companyObjectMetadata.fields.find(
(field) => field.name === 'linkedinLink',
);
if (!companyLinkedinLinkFieldMetadata) {
throw new Error('Company linkedin link field metadata not found');
}
const readOnlyOnPersonCityFieldPermission = {
objectMetadataId: personObjectMetadata.id,
fieldMetadataId: personCityFieldMetadata.id,
canReadFieldValue: null,
canUpdateFieldValue: false,
};
const noReadOnCompanyLinkedinLinkFieldPermission = {
objectMetadataId: companyObjectMetadata.id,
fieldMetadataId: companyLinkedinLinkFieldMetadata.id,
canReadFieldValue: false,
canUpdateFieldValue: false,
};
await this.fieldPermissionService.upsertFieldPermissions({
workspaceId,
input: {
roleId: customRole.id,
fieldPermissions: [
readOnlyOnPersonCityFieldPermission,
noReadOnCompanyLinkedinLinkFieldPermission,
],
},
});
return customRole;
}
}
@@ -75,6 +75,11 @@ export const seedFeatureFlags = async (
workspaceId: workspaceId,
value: true,
},
{
key: FeatureFlagKey.IS_FIELDS_PERMISSIONS_ENABLED,
workspaceId: workspaceId,
value: true,
},
])
.execute();
};
@@ -92,7 +92,13 @@ export class DeleteRecordWorkflowAction implements WorkflowAction {
);
}
await repository.softDelete(workflowActionInput.objectRecordId);
const columnsToReturnForSoftDelete: string[] = [];
await repository.softDelete(
workflowActionInput.objectRecordId,
undefined,
columnsToReturnForSoftDelete,
);
return {
result: objectRecord,