Fix field permission validation rejecting undefined optional fields (#19243)
The backend validation for field permissions was using !== null to check canReadFieldValue and canUpdateFieldValue, but these optional GraphQL fields can also be undefined when omitted from the input. This caused the validation to incorrectly reject legitimate requests (e.g., restricting only "update" without specifying "read") with the error "Field permissions can only be used to restrict access, not to grant additional permissions." Replaced !== null checks with isDefined() so that both null and undefined are treated as "no opinion" on that permission. To reproduce: - Create a single FieldPermission on an object with canEdit: false without any other rule on that same object
This commit is contained in:
+20
-9
@@ -130,8 +130,8 @@ export class FieldPermissionService {
|
||||
{
|
||||
objectMetadataId: fieldPermission.objectMetadataId,
|
||||
fieldMetadataId: fieldPermission.fieldMetadataId,
|
||||
canReadFieldValue: fieldPermission.canReadFieldValue ?? undefined,
|
||||
canUpdateFieldValue: fieldPermission.canUpdateFieldValue ?? undefined,
|
||||
canReadFieldValue: fieldPermission.canReadFieldValue,
|
||||
canUpdateFieldValue: fieldPermission.canUpdateFieldValue,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -183,9 +183,13 @@ export class FieldPermissionService {
|
||||
);
|
||||
} else {
|
||||
const effectiveCanRead =
|
||||
desired.canReadFieldValue ?? current.canReadFieldValue;
|
||||
desired.canReadFieldValue !== undefined
|
||||
? desired.canReadFieldValue
|
||||
: current.canReadFieldValue;
|
||||
const effectiveCanUpdate =
|
||||
desired.canUpdateFieldValue ?? current.canUpdateFieldValue;
|
||||
desired.canUpdateFieldValue !== undefined
|
||||
? desired.canUpdateFieldValue
|
||||
: current.canUpdateFieldValue;
|
||||
const changed =
|
||||
effectiveCanRead !== current.canReadFieldValue ||
|
||||
effectiveCanUpdate !== current.canUpdateFieldValue;
|
||||
@@ -201,8 +205,8 @@ export class FieldPermissionService {
|
||||
current.objectMetadataUniversalIdentifier,
|
||||
fieldMetadataUniversalIdentifier:
|
||||
current.fieldMetadataUniversalIdentifier,
|
||||
canReadFieldValue: effectiveCanRead ?? undefined,
|
||||
canUpdateFieldValue: effectiveCanUpdate ?? undefined,
|
||||
canReadFieldValue: effectiveCanRead,
|
||||
canUpdateFieldValue: effectiveCanUpdate,
|
||||
createdAt: current.createdAt,
|
||||
updatedAt: now,
|
||||
});
|
||||
@@ -210,9 +214,16 @@ export class FieldPermissionService {
|
||||
}
|
||||
}
|
||||
|
||||
const inputFieldKeys = new Set(
|
||||
input.fieldPermissions.map((fp) =>
|
||||
keyFrom(fp.objectMetadataId, fp.fieldMetadataId),
|
||||
),
|
||||
);
|
||||
|
||||
for (const current of currentFieldPermissionsForRole) {
|
||||
const key = keyFrom(current.objectMetadataId, current.fieldMetadataId);
|
||||
if (!desiredMap.has(key)) {
|
||||
|
||||
if (inputFieldKeys.has(key) && !desiredMap.has(key)) {
|
||||
flatEntityToDelete.push({
|
||||
universalIdentifier: current.universalIdentifier,
|
||||
applicationUniversalIdentifier:
|
||||
@@ -324,9 +335,9 @@ export class FieldPermissionService {
|
||||
}
|
||||
|
||||
if (
|
||||
(fieldPermission.canUpdateFieldValue !== null &&
|
||||
(isDefined(fieldPermission.canUpdateFieldValue) &&
|
||||
fieldPermission.canUpdateFieldValue !== false) ||
|
||||
(fieldPermission.canReadFieldValue !== null &&
|
||||
(isDefined(fieldPermission.canReadFieldValue) &&
|
||||
fieldPermission.canReadFieldValue !== false)
|
||||
) {
|
||||
throw new PermissionsException(
|
||||
|
||||
+2
-2
@@ -117,7 +117,7 @@ describe('Field permissions restrictions', () => {
|
||||
{
|
||||
objectMetadataId: companyObjectId,
|
||||
fieldMetadataId: restrictedCompanyFieldId,
|
||||
canReadFieldValue: false,
|
||||
canUpdateFieldValue: false,
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -134,7 +134,7 @@ describe('Field permissions restrictions', () => {
|
||||
{
|
||||
objectMetadataId: personObjectId,
|
||||
fieldMetadataId: restrictedPersonFieldId,
|
||||
canReadFieldValue: false,
|
||||
canUpdateFieldValue: false,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
+15
-47
@@ -14,6 +14,8 @@ import { updateWorkspaceMemberRole } from 'test/integration/graphql/utils/update
|
||||
import { upsertFieldPermissions } from 'test/integration/graphql/utils/upsert-field-permissions.util';
|
||||
import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util';
|
||||
|
||||
import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception';
|
||||
import { WORKSPACE_MEMBER_DATA_SEED_IDS } from 'src/engine/workspace-manager/dev-seeder/data/constants/workspace-member-data-seeds.constant';
|
||||
|
||||
const client = request(`http://localhost:${APP_PORT}`);
|
||||
@@ -28,33 +30,13 @@ const COMPANY_GQL_FIELDS_WITHOUT_EMPLOYEES = `
|
||||
id
|
||||
name
|
||||
`;
|
||||
const expectEmployeesIsAccessible = ({
|
||||
response,
|
||||
operationName,
|
||||
expectedEmployees,
|
||||
}: {
|
||||
response: any;
|
||||
operationName:
|
||||
| 'createCompanies'
|
||||
| 'createCompany'
|
||||
| 'updateCompanies'
|
||||
| 'updateCompany';
|
||||
expectedEmployees?: number;
|
||||
}) => {
|
||||
expect(response.body.errors).toBeUndefined();
|
||||
expect(response.body.data).toBeDefined();
|
||||
|
||||
const result =
|
||||
operationName === 'createCompany' || operationName === 'updateCompany'
|
||||
? response.body.data[operationName]
|
||||
: response.body.data[operationName]?.[0];
|
||||
|
||||
expect(result).toBeDefined();
|
||||
if (typeof expectedEmployees === 'number') {
|
||||
expect(result.employees).toBe(expectedEmployees);
|
||||
} else {
|
||||
expect(typeof result.employees).toBe('number');
|
||||
}
|
||||
const expectPermissionDeniedError = (response: any) => {
|
||||
expect(response.body.errors).toBeDefined();
|
||||
expect(response.body.errors.length).toBeGreaterThan(0);
|
||||
expect(response.body.errors[0].message).toBe(
|
||||
PermissionsExceptionMessage.PERMISSION_DENIED,
|
||||
);
|
||||
expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN);
|
||||
};
|
||||
|
||||
describe('Field update permissions restrictions', () => {
|
||||
@@ -314,7 +296,7 @@ describe('Field update permissions restrictions', () => {
|
||||
// });
|
||||
// });
|
||||
|
||||
describe('should allow employees field when creating', () => {
|
||||
describe('should block creating with update-restricted field in data', () => {
|
||||
beforeEach(async () => {
|
||||
await restrictUpdateAccessToCompanyEmployee(
|
||||
customRoleId,
|
||||
@@ -337,11 +319,7 @@ describe('Field update permissions restrictions', () => {
|
||||
const response =
|
||||
await makeGraphqlAPIRequestWithMemberRole(graphqlOperation);
|
||||
|
||||
expectEmployeesIsAccessible({
|
||||
response,
|
||||
operationName: 'createCompanies',
|
||||
expectedEmployees: 15,
|
||||
});
|
||||
expectPermissionDeniedError(response);
|
||||
});
|
||||
|
||||
it('2. createOne with restricted field', async () => {
|
||||
@@ -354,14 +332,10 @@ describe('Field update permissions restrictions', () => {
|
||||
const response =
|
||||
await makeGraphqlAPIRequestWithMemberRole(graphqlOperation);
|
||||
|
||||
expectEmployeesIsAccessible({
|
||||
response,
|
||||
operationName: 'createCompany',
|
||||
expectedEmployees: 25,
|
||||
});
|
||||
expectPermissionDeniedError(response);
|
||||
});
|
||||
});
|
||||
describe('should allow employees field in update operation responses', () => {
|
||||
describe('should block read-restricted field in update operation responses', () => {
|
||||
beforeEach(async () => {
|
||||
await restrictReadAccessToCompanyEmployee(
|
||||
customRoleId,
|
||||
@@ -382,10 +356,7 @@ describe('Field update permissions restrictions', () => {
|
||||
const response =
|
||||
await makeGraphqlAPIRequestWithMemberRole(graphqlOperation);
|
||||
|
||||
expectEmployeesIsAccessible({
|
||||
response,
|
||||
operationName: 'updateCompanies',
|
||||
});
|
||||
expectPermissionDeniedError(response);
|
||||
});
|
||||
|
||||
it('2. updateOne requesting restricted field in response', async () => {
|
||||
@@ -399,10 +370,7 @@ describe('Field update permissions restrictions', () => {
|
||||
const response =
|
||||
await makeGraphqlAPIRequestWithMemberRole(graphqlOperation);
|
||||
|
||||
expectEmployeesIsAccessible({
|
||||
response,
|
||||
operationName: 'updateCompany',
|
||||
});
|
||||
expectPermissionDeniedError(response);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user