[permissions] Fix update of relation field permissions (#13755)
In this PR, we add some validation logic and rules in both FE and BE to ensure field permissions are handled correctly for relation fields. - (BE) Only one field permission per fieldMetadata is accepted per input. This is already guaranteed in the FE. It was added to help guarantee that, when looking within the field permissions input for a potential field permission on a relationTargetFieldMetadataId for a relation field, there can only be 0 or 1. - (FE) Only field permission with new values are sent to save, to avoid sending contradictory field permissions for related fields. E.g. let's say I have an existing field permission restricting read permission on company's people field. By definition I also have one on person's company field. If I update this field permission to enable the read permission by updating company's people field, in the previous logic I was also going to send for upsert the existing obsolete field permission on person's company. Thus the server does not know which is the right value so we should only send the new value. - (BE) If the server receives two contradictory field permissions on two related fields, e.g. on company's people with canRead = null and person's company with canRead = false, it throws an error.
This commit is contained in:
+10
-1
@@ -1,6 +1,7 @@
|
||||
import { GET_ROLES } from '@/settings/roles/graphql/queries/getRolesQuery';
|
||||
import { useUpdateWorkspaceMemberRole } from '@/settings/roles/hooks/useUpdateWorkspaceMemberRole';
|
||||
import { useRemoveFieldPermissionInDraftRole } from '@/settings/roles/role-permissions/object-level-permissions/field-permissions/hooks/useRemoveFieldPermissionInDraftRole';
|
||||
import { newFieldPermissionsFilter } from '@/settings/roles/role/hooks/utils/newFieldPermissionsFilter.util';
|
||||
import { settingsDraftRoleFamilyState } from '@/settings/roles/states/settingsDraftRoleFamilyState';
|
||||
import { settingsPersistedRoleFamilyState } from '@/settings/roles/states/settingsPersistedRoleFamilyState';
|
||||
import { SettingsPath } from '@/types/SettingsPath';
|
||||
@@ -72,7 +73,7 @@ export const useSaveDraftRoleToDB = ({
|
||||
);
|
||||
});
|
||||
|
||||
const fieldPermissionsToUpsert =
|
||||
const onlyMeaningfulFieldPermissions =
|
||||
dirtyFields.fieldPermissions?.filter(
|
||||
(dirtyFieldPermissionToFilter) =>
|
||||
!fieldPermissionsThatShouldntBeCreatedBecauseTheyAreUseless?.some(
|
||||
@@ -82,6 +83,14 @@ export const useSaveDraftRoleToDB = ({
|
||||
),
|
||||
) ?? [];
|
||||
|
||||
const fieldPermissionsToUpsert = onlyMeaningfulFieldPermissions.filter(
|
||||
(dirtyFieldPermission) =>
|
||||
newFieldPermissionsFilter(
|
||||
dirtyFieldPermission,
|
||||
settingsPersistedRole?.fieldPermissions,
|
||||
),
|
||||
);
|
||||
|
||||
const { removeFieldPermissionInDraftRole } =
|
||||
useRemoveFieldPermissionInDraftRole();
|
||||
|
||||
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
import { FieldPermission } from '~/generated/graphql';
|
||||
|
||||
export const newFieldPermissionsFilter = (
|
||||
dirtyFieldPermission: FieldPermission,
|
||||
existingFieldPermissions?: FieldPermission[] | null,
|
||||
) => {
|
||||
const existingFieldPermission = existingFieldPermissions?.find(
|
||||
(persistedFieldPermission) =>
|
||||
persistedFieldPermission.fieldMetadataId ===
|
||||
dirtyFieldPermission.fieldMetadataId,
|
||||
);
|
||||
|
||||
if (!existingFieldPermission) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return (
|
||||
dirtyFieldPermission.canReadFieldValue !==
|
||||
existingFieldPermission.canReadFieldValue ||
|
||||
dirtyFieldPermission.canUpdateFieldValue !==
|
||||
existingFieldPermission.canUpdateFieldValue
|
||||
);
|
||||
};
|
||||
+38
-6
@@ -7,7 +7,10 @@ import { In, Repository } from 'typeorm';
|
||||
|
||||
import { RelationType } from 'src/engine/metadata-modules/field-metadata/interfaces/relation-type.interface';
|
||||
|
||||
import { InternalServerError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import {
|
||||
InternalServerError,
|
||||
UserInputError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||
import { isFieldMetadataTypeRelation } from 'src/engine/metadata-modules/field-metadata/utils/is-field-metadata-type-relation.util';
|
||||
import { type UpsertFieldPermissionsInput } from 'src/engine/metadata-modules/object-permission/dtos/upsert-field-permissions.input';
|
||||
@@ -74,6 +77,7 @@ export class FieldPermissionService {
|
||||
|
||||
input.fieldPermissions.forEach((fieldPermission) => {
|
||||
this.validateFieldPermission({
|
||||
allFieldPermissions: input.fieldPermissions,
|
||||
fieldPermission,
|
||||
objectMetadataMapsById,
|
||||
rolesPermissions,
|
||||
@@ -169,16 +173,28 @@ export class FieldPermissionService {
|
||||
}
|
||||
|
||||
private validateFieldPermission({
|
||||
allFieldPermissions,
|
||||
fieldPermission,
|
||||
objectMetadataMapsById,
|
||||
rolesPermissions,
|
||||
role,
|
||||
}: {
|
||||
allFieldPermissions: UpsertFieldPermissionsInput['fieldPermissions'];
|
||||
fieldPermission: UpsertFieldPermissionsInput['fieldPermissions'][0];
|
||||
objectMetadataMapsById: ObjectMetadataMaps['byId'];
|
||||
rolesPermissions: ObjectsPermissionsByRoleIdDeprecated;
|
||||
role: RoleEntity;
|
||||
}) {
|
||||
const duplicateFieldPermissions = allFieldPermissions.filter(
|
||||
(permission) =>
|
||||
permission.fieldMetadataId === fieldPermission.fieldMetadataId,
|
||||
);
|
||||
|
||||
if (duplicateFieldPermissions.length > 1) {
|
||||
throw new UserInputError(
|
||||
`Cannot accept more than one fieldPermission for field ${fieldPermission.fieldMetadataId} in input.`,
|
||||
);
|
||||
}
|
||||
if (
|
||||
('canUpdateFieldValue' in fieldPermission &&
|
||||
fieldPermission.canUpdateFieldValue !== null &&
|
||||
@@ -402,16 +418,32 @@ export class FieldPermissionService {
|
||||
fieldMetadata.settings?.relationType === RelationType.ONE_TO_MANY ||
|
||||
fieldMetadata.settings?.relationType === RelationType.MANY_TO_ONE
|
||||
) {
|
||||
const fieldPermissionInputHasFieldPermissionOnRelationTargetFieldMetadata =
|
||||
const fieldPermissionsOnRelationTargetField =
|
||||
fieldPermissions.filter(
|
||||
(fieldPermissionInput) =>
|
||||
fieldPermissionInput.fieldMetadataId ===
|
||||
fieldMetadata.relationTargetFieldMetadataId,
|
||||
).length > 0;
|
||||
);
|
||||
|
||||
if (fieldPermissionsOnRelationTargetField.length > 0) {
|
||||
const firstFieldPermission =
|
||||
fieldPermissionsOnRelationTargetField[0]; // validation rules guarantee there can only be one
|
||||
|
||||
const hasConflictingPermissions =
|
||||
fieldPermission.canReadFieldValue !==
|
||||
firstFieldPermission.canReadFieldValue ||
|
||||
fieldPermission.canUpdateFieldValue !==
|
||||
firstFieldPermission.canUpdateFieldValue;
|
||||
|
||||
if (hasConflictingPermissions) {
|
||||
throw new UserInputError(
|
||||
'Conflicting field permissions found for relation target field',
|
||||
{
|
||||
userFriendlyMessage: `Contradicting field permissions have been detected on a relation field (${fieldMetadata.name}).`,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
fieldPermissionInputHasFieldPermissionOnRelationTargetFieldMetadata
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user