Block filters on restricted fields (#22873)
## Summary
- Reject filter conditions that target fields without read access,
including relation traversals
- Add unit and integration coverage for denied field filter access
The issue is an information leak through filtering: a user who cannot
read a field can still infer its values from totalCount
### Manual reproduction
- Create or use a non-admin role.
- Give it read access to People records.
- Disable read access to the Person jobTitle field.
- Assign a test member to that role.
- Authenticate as that member.
Run:
```gql
query People($filter: PersonFilterInput) {
people(filter: $filter, first: 0) {
totalCount
}
}
Variables:
{
"filter": {
"jobTitle": {
"like": "Par%"
}
}
}
```
Ensure at least one Person has a matching jobTitle.
Before the fix, the request succeeds:
```gql
{
"data": {
"people": {
"totalCount": 1
}
}
}
```
The caller can probe restricted values using different filters.
After the fix, it returns a permission error:
```gql
{
"errors": [
{
"message": "Permission denied"
}
]
}
```
The same should happen through a relation filter, for example filtering
Companies by a restricted Person field:
```gql
query Companies($filter: CompanyFilterInput) {
companies(filter: $filter, first: 0) {
totalCount
}
}
{
"filter": {
"people": {
"jobTitle": {
"like": "Par%"
}
}
}
}
```
This commit is contained in:
+15
-7
@@ -4,6 +4,7 @@ import {
|
||||
CompositeFieldSubFieldName,
|
||||
PartialFieldMetadataItemOption,
|
||||
RecordFilterGroupLogicalOperator,
|
||||
type RestrictedFieldsPermissions,
|
||||
} from 'twenty-shared/types';
|
||||
import {
|
||||
assertIsDefinedOrThrow,
|
||||
@@ -183,12 +184,14 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic
|
||||
args,
|
||||
flatObjectMetadata,
|
||||
flatFieldMetadataMaps,
|
||||
restrictedFields,
|
||||
appliedFilters,
|
||||
workspaceId,
|
||||
}: {
|
||||
args: GroupByQueryArgs;
|
||||
flatObjectMetadata: FlatObjectMetadata;
|
||||
flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
|
||||
restrictedFields: RestrictedFieldsPermissions;
|
||||
appliedFilters: ObjectRecordFilter;
|
||||
workspaceId: string;
|
||||
}): Promise<ObjectRecordFilter> {
|
||||
@@ -244,13 +247,15 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic
|
||||
const fields = getFlatFieldsFromFlatObjectMetadata(
|
||||
flatObjectMetadata,
|
||||
flatFieldMetadataMaps,
|
||||
).map((field) => ({
|
||||
id: field.id,
|
||||
name: field.name,
|
||||
type: field.type,
|
||||
label: field.label,
|
||||
options: field.options as PartialFieldMetadataItemOption[],
|
||||
}));
|
||||
)
|
||||
.filter((field) => restrictedFields[field.id]?.canRead !== false)
|
||||
.map((field) => ({
|
||||
id: field.id,
|
||||
name: field.name,
|
||||
type: field.type,
|
||||
label: field.label,
|
||||
options: field.options as PartialFieldMetadataItemOption[],
|
||||
}));
|
||||
|
||||
const filtersFromView = computeRecordGqlOperationFilter({
|
||||
recordFilters,
|
||||
@@ -311,6 +316,9 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic
|
||||
args,
|
||||
flatObjectMetadata,
|
||||
flatFieldMetadataMaps,
|
||||
restrictedFields:
|
||||
queryBuilder.objectRecordsPermissions[flatObjectMetadata.id]
|
||||
?.restrictedFields ?? {},
|
||||
appliedFilters,
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
+20
@@ -23,6 +23,11 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m
|
||||
import { buildFieldMapsFromFlatObjectMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util';
|
||||
import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util';
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
import {
|
||||
PermissionsException,
|
||||
PermissionsExceptionCode,
|
||||
PermissionsExceptionMessage,
|
||||
} from 'src/engine/metadata-modules/permissions/permissions.exception';
|
||||
import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder';
|
||||
|
||||
import { GraphqlQueryFilterConditionParser } from './graphql-query-filter-condition.parser';
|
||||
@@ -30,6 +35,7 @@ import { GraphqlQueryFilterConditionParser } from './graphql-query-filter-condit
|
||||
const ARRAY_OPERATORS = ['in', 'contains', 'notContains'];
|
||||
|
||||
export class GraphqlQueryFilterFieldParser {
|
||||
private flatObjectMetadata: FlatObjectMetadata;
|
||||
private flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
|
||||
private flatObjectMetadataMaps?: FlatEntityMaps<FlatObjectMetadata>;
|
||||
private fieldIdByName: Record<string, string>;
|
||||
@@ -42,6 +48,7 @@ export class GraphqlQueryFilterFieldParser {
|
||||
flatObjectMetadataMaps?: FlatEntityMaps<FlatObjectMetadata>,
|
||||
depth = 0,
|
||||
) {
|
||||
this.flatObjectMetadata = flatObjectMetadata;
|
||||
this.flatFieldMetadataMaps = flatFieldMetadataMaps;
|
||||
this.flatObjectMetadataMaps = flatObjectMetadataMaps;
|
||||
this.depth = depth;
|
||||
@@ -78,6 +85,19 @@ export class GraphqlQueryFilterFieldParser {
|
||||
throw new Error(`Field metadata not found for field: ${key}`);
|
||||
}
|
||||
|
||||
const objectPermissions =
|
||||
outerQueryBuilder.objectRecordsPermissions[this.flatObjectMetadata.id];
|
||||
|
||||
if (
|
||||
objectPermissions?.canReadObjectRecords === false ||
|
||||
objectPermissions?.restrictedFields[fieldMetadata.id]?.canRead === false
|
||||
) {
|
||||
throw new PermissionsException(
|
||||
PermissionsExceptionMessage.PERMISSION_DENIED,
|
||||
PermissionsExceptionCode.PERMISSION_DENIED,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
isFilterKeyARelation &&
|
||||
isMorphOrRelationFlatFieldMetadata(fieldMetadata) &&
|
||||
|
||||
+32
@@ -449,6 +449,38 @@ describe('Field permissions restrictions', () => {
|
||||
expect(response.body.data.companies.edges[0].node.id).toBeDefined();
|
||||
});
|
||||
|
||||
it('should reject filters on fields without read permission', async () => {
|
||||
await upsertFieldPermissions({
|
||||
roleId: customRoleId,
|
||||
fieldPermissions: [
|
||||
{
|
||||
objectMetadataId: personObjectId,
|
||||
fieldMetadataId: restrictedPersonFieldId,
|
||||
canReadFieldValue: false,
|
||||
canUpdateFieldValue: null,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const graphqlOperation = {
|
||||
query: gql`
|
||||
query People($filter: PersonFilterInput) {
|
||||
people(filter: $filter, first: 0) {
|
||||
totalCount
|
||||
}
|
||||
}
|
||||
`,
|
||||
variables: {
|
||||
filter: { jobTitle: { like: 'Par%' } },
|
||||
},
|
||||
};
|
||||
|
||||
const response =
|
||||
await makeGraphqlAPIRequestWithMemberRole(graphqlOperation);
|
||||
|
||||
expectPermissionDeniedError(response);
|
||||
});
|
||||
|
||||
describe('Aggregate operations', () => {
|
||||
it('1. should allow aggregate over a restricted field', async () => {
|
||||
await restrictAccessToCompanyEmployee(
|
||||
|
||||
Reference in New Issue
Block a user