Field permission CRUD front-end implementation (#13524)
This PR implements what's needed to edit field permissions on a role. Field permissions that aren't useful are kept in the database to avoid the overhead of adding cleaning logics both in front end and back end. They just won't be taken into account if object permission doesn't allow it. In this PR we also handle the case were there's no object permission override but where there are only field permission overrides, which can happen if an object inherits from a "all object can read" but restricts read only on some fields. <img width="547" height="642" alt="image" src="https://github.com/user-attachments/assets/77d81f89-4af9-42b6-97f3-fae3a6ba1eeb" /> <img width="590" height="912" alt="image" src="https://github.com/user-attachments/assets/69fab8ee-7252-401a-bc6f-8a8b7c7f6bc4" /> Fixes https://github.com/twentyhq/core-team-issues/issues/1152 This PR lowers unit test coverage because the essential unit tests for this PR have been written and writing more tests would not be a great tradeoff for this feature which has already taken a lot of efforts.
This commit is contained in:
Lucas Bordeau
-100
@@ -451,106 +451,6 @@ describe('FieldPermissionService', () => {
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw error when object is not readable (permission wise)', async () => {
|
||||
const nonReadableObjectPermissions: ObjectRecordsPermissionsByRoleId = {
|
||||
[testRoleId]: {
|
||||
[testObjectMetadataId]: {
|
||||
canRead: false,
|
||||
canUpdate: false,
|
||||
canSoftDelete: false,
|
||||
canDestroy: false,
|
||||
restrictedFields: {},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
workspacePermissionsCacheService.getRolesPermissionsFromCache.mockResolvedValue(
|
||||
{
|
||||
version: '1',
|
||||
data: nonReadableObjectPermissions,
|
||||
},
|
||||
);
|
||||
|
||||
const input = createUpsertInput([
|
||||
{
|
||||
canUpdateFieldValue: false,
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
service.upsertFieldPermissions({
|
||||
workspaceId: testWorkspaceId,
|
||||
input,
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
new PermissionsException(
|
||||
PermissionsExceptionMessage.FIELD_RESTRICTION_ONLY_ALLOWED_ON_READABLE_OBJECT,
|
||||
PermissionsExceptionCode.FIELD_RESTRICTION_ONLY_ALLOWED_ON_READABLE_OBJECT,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw error when trying to restrict update on non-updatable object', async () => {
|
||||
const nonUpdatableObjectPermissions: ObjectRecordsPermissionsByRoleId =
|
||||
{
|
||||
[testRoleId]: {
|
||||
[testObjectMetadataId]: {
|
||||
canRead: true,
|
||||
canUpdate: false,
|
||||
canSoftDelete: false,
|
||||
canDestroy: false,
|
||||
restrictedFields: {},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
workspacePermissionsCacheService.getRolesPermissionsFromCache.mockResolvedValue(
|
||||
{
|
||||
version: '1',
|
||||
data: nonUpdatableObjectPermissions,
|
||||
},
|
||||
);
|
||||
|
||||
const input = createUpsertInput([
|
||||
{
|
||||
canUpdateFieldValue: false,
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
service.upsertFieldPermissions({
|
||||
workspaceId: testWorkspaceId,
|
||||
input,
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
new PermissionsException(
|
||||
PermissionsExceptionMessage.FIELD_RESTRICTION_ON_UPDATE_ONLY_ALLOWED_ON_UPDATABLE_OBJECT,
|
||||
PermissionsExceptionCode.FIELD_RESTRICTION_ON_UPDATE_ONLY_ALLOWED_ON_UPDATABLE_OBJECT,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw error when both canReadFieldValue and canUpdateFieldValue are null', async () => {
|
||||
const input = createUpsertInput([
|
||||
{
|
||||
canReadFieldValue: null,
|
||||
canUpdateFieldValue: null,
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
service.upsertFieldPermissions({
|
||||
workspaceId: testWorkspaceId,
|
||||
input,
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
new PermissionsException(
|
||||
PermissionsExceptionMessage.EMPTY_FIELD_PERMISSION_NOT_ALLOWED,
|
||||
PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED,
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('role validation errors', () => {
|
||||
|
||||
+17
-28
@@ -91,7 +91,23 @@ export class FieldPermissionService {
|
||||
workspaceId,
|
||||
}));
|
||||
|
||||
await this.fieldPermissionsRepository.upsert(fieldPermissions, {
|
||||
const existingFieldPermissionsToDelete = existingFieldPermissions.filter(
|
||||
(existingFieldPermissionToFilter) =>
|
||||
fieldPermissionsToDeleteIds.includes(
|
||||
existingFieldPermissionToFilter.id,
|
||||
),
|
||||
);
|
||||
|
||||
const fieldPermissionsToUpsert = fieldPermissions.filter(
|
||||
(fieldPermissionToUpsert) =>
|
||||
!existingFieldPermissionsToDelete.some(
|
||||
(existingFieldPermissionToDelete) =>
|
||||
existingFieldPermissionToDelete.fieldMetadataId ===
|
||||
fieldPermissionToUpsert.fieldMetadataId,
|
||||
),
|
||||
);
|
||||
|
||||
await this.fieldPermissionsRepository.upsert(fieldPermissionsToUpsert, {
|
||||
conflictPaths: ['fieldMetadataId', 'roleId'],
|
||||
});
|
||||
|
||||
@@ -182,33 +198,6 @@ export class FieldPermissionService {
|
||||
PermissionsExceptionCode.OBJECT_PERMISSION_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
if (rolePermissionOnObject.canRead === false) {
|
||||
throw new PermissionsException(
|
||||
PermissionsExceptionMessage.FIELD_RESTRICTION_ONLY_ALLOWED_ON_READABLE_OBJECT,
|
||||
PermissionsExceptionCode.FIELD_RESTRICTION_ONLY_ALLOWED_ON_READABLE_OBJECT,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
rolePermissionOnObject.canUpdate === false &&
|
||||
fieldPermission.canUpdateFieldValue === false
|
||||
) {
|
||||
throw new PermissionsException(
|
||||
PermissionsExceptionMessage.FIELD_RESTRICTION_ON_UPDATE_ONLY_ALLOWED_ON_UPDATABLE_OBJECT,
|
||||
PermissionsExceptionCode.FIELD_RESTRICTION_ON_UPDATE_ONLY_ALLOWED_ON_UPDATABLE_OBJECT,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
fieldPermission.canUpdateFieldValue === null &&
|
||||
fieldPermission.canReadFieldValue === null
|
||||
) {
|
||||
throw new PermissionsException(
|
||||
PermissionsExceptionMessage.EMPTY_FIELD_PERMISSION_NOT_ALLOWED,
|
||||
PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async getRoleOrThrow({
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Relation } from 'typeorm';
|
||||
|
||||
import { UUIDScalarType } from 'src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars';
|
||||
import { WorkspaceMember } from 'src/engine/core-modules/user/dtos/workspace-member.dto';
|
||||
import { FieldPermissionDTO } from 'src/engine/metadata-modules/object-permission/dtos/field-permission.dto';
|
||||
import { ObjectPermissionDTO } from 'src/engine/metadata-modules/object-permission/dtos/object-permission.dto';
|
||||
import { PermissionFlagDTO } from 'src/engine/metadata-modules/permission-flag/dtos/permission-flag.dto';
|
||||
import { RoleTargetsEntity } from 'src/engine/metadata-modules/role/role-targets.entity';
|
||||
@@ -54,4 +55,7 @@ export class RoleDTO {
|
||||
|
||||
@Field(() => [ObjectPermissionDTO], { nullable: true })
|
||||
objectPermissions?: ObjectPermissionDTO[];
|
||||
|
||||
@Field(() => [FieldPermissionDTO], { nullable: true })
|
||||
fieldPermissions?: FieldPermissionDTO[];
|
||||
}
|
||||
|
||||
@@ -40,7 +40,12 @@ export class RoleService {
|
||||
where: {
|
||||
workspaceId,
|
||||
},
|
||||
relations: ['roleTargets', 'permissionFlags', 'objectPermissions'],
|
||||
relations: {
|
||||
roleTargets: true,
|
||||
permissionFlags: true,
|
||||
objectPermissions: true,
|
||||
fieldPermissions: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -53,7 +58,12 @@ export class RoleService {
|
||||
id,
|
||||
workspaceId,
|
||||
},
|
||||
relations: ['roleTargets', 'permissionFlags'],
|
||||
relations: {
|
||||
roleTargets: true,
|
||||
permissionFlags: true,
|
||||
objectPermissions: true,
|
||||
fieldPermissions: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user