Fix TWENTY-SERVER-60Y: register permissions exception filter globally (#23104)
## Context Sentry [TWENTY-SERVER-60Y](https://twenty-v7.sentry.io/issues/6633503406) ("Permission Denied: Entity performing the request does not have permission") has ~12.9k occurrences / 151 users. It groups by the shared throw site `settings-permission.guard.ts:57`, so it's a **catch-all bucket** for settings-permission denials across many resolvers, not a single operation. Sampled events include `findOneApplication` (app runtimes reading their own `applicationVariables`), `uploadFilesFieldFileByUniversalIdentifier`, `UpdatePageLayoutWithTabsAndWidgets`, `CreateFileUpload`, etc. ## Root cause Settings-permission denials are only converted to a client-appropriate `FORBIDDEN` when a resolver manually attaches `PermissionsGraphqlApiExceptionFilter`. **28** GraphQL resolvers do; **~35** guarded resolvers do not. On those, the guard-thrown `PermissionsException` (a `CustomException`, not a `BaseGraphQLError`) falls through to the Yoga error hook, is serialized as `INTERNAL_SERVER_ERROR`, and `shouldCaptureException` reports it to Sentry as a 500-class error. REST is not affected: every `SettingsPermissionGuard` controller already carries `PermissionsRestApiExceptionFilter` (15/15). ## Fix Register `PermissionsGraphqlApiExceptionFilter` globally via `APP_FILTER` in the core and metadata engine modules, mirroring the existing global GraphQL filters `BillingGraphqlApiExceptionFilter` and `FlatEntityMapsGraphqlApiExceptionFilter`. The filter is guarded on the GraphQL context (`host.getType()`), so REST keeps its existing per-controller filter untouched and no request-scoped dependency is pulled into a global provider. Every settings-guarded resolver now returns `FORBIDDEN` for denials, which is both the correct client error code and excluded from Sentry. Existing per-resolver `@UseFilters(PermissionsGraphqlApiExceptionFilter)` entries remain valid (handler-scoped takes precedence, identical result); collapsing them into the global registration is a possible follow-up. ## Test Added a case to `granular-settings-permissions.integration-spec.ts`: a member without the `APPLICATIONS` flag calling `findOneApplication{applicationVariables{key value}}` (the exact Sentry query, and a resolver with **no** resolver-scoped filter) must receive `FORBIDDEN`, exercising the global filter. Verified against the local test DB: - With the global filter: passes (`FORBIDDEN`). - Without it: fails with `INTERNAL_SERVER_ERROR`, reproducing the leak. - The existing roles / workspace-members / api-keys denial tests (per-resolver filters) still pass, confirming no precedence conflict and no boot issue from dual `APP_FILTER` registration. Fixes TWENTY-SERVER-60Y
This commit is contained in:
@@ -20,6 +20,7 @@ import { BillingWebhookModule } from 'src/engine/core-modules/billing-webhook/bi
|
||||
import { AppBillingModule } from 'src/engine/core-modules/billing/app-billing/app-billing.module';
|
||||
import { BillingModule } from 'src/engine/core-modules/billing/billing.module';
|
||||
import { BillingGraphqlApiExceptionFilter } from 'src/engine/core-modules/billing/filters/billing-graphql-api-exception.filter';
|
||||
import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
|
||||
import { CacheStorageModule } from 'src/engine/core-modules/cache-storage/cache-storage.module';
|
||||
import { TimelineCalendarEventModule } from 'src/engine/core-modules/calendar/timeline-calendar-event.module';
|
||||
import { CaptchaModule } from 'src/engine/core-modules/captcha/captcha.module';
|
||||
@@ -174,6 +175,10 @@ import { FileModule } from './file/file.module';
|
||||
provide: APP_FILTER,
|
||||
useClass: BillingGraphqlApiExceptionFilter,
|
||||
},
|
||||
{
|
||||
provide: APP_FILTER,
|
||||
useClass: PermissionsGraphqlApiExceptionFilter,
|
||||
},
|
||||
],
|
||||
exports: [
|
||||
EventLogsViewerModule,
|
||||
|
||||
@@ -13,6 +13,7 @@ import { ConnectedAccountMetadataModule } from 'src/engine/metadata-modules/conn
|
||||
import { CommandMenuItemModule } from 'src/engine/metadata-modules/command-menu-item/command-menu-item.module';
|
||||
import { FieldMetadataModule } from 'src/engine/metadata-modules/field-metadata/field-metadata.module';
|
||||
import { FlatEntityMapsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/flat-entity/filters/flat-entity-maps-graphql-api-exception.filter';
|
||||
import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
|
||||
import { FrontComponentModule } from 'src/engine/metadata-modules/front-component/front-component.module';
|
||||
import { LogicFunctionLayerModule } from 'src/engine/metadata-modules/logic-function-layer/logic-function-layer.module';
|
||||
import { LogicFunctionModule } from 'src/engine/metadata-modules/logic-function/logic-function.module';
|
||||
@@ -69,6 +70,10 @@ import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/work
|
||||
provide: APP_FILTER,
|
||||
useClass: FlatEntityMapsGraphqlApiExceptionFilter,
|
||||
},
|
||||
{
|
||||
provide: APP_FILTER,
|
||||
useClass: PermissionsGraphqlApiExceptionFilter,
|
||||
},
|
||||
],
|
||||
exports: [
|
||||
FieldMetadataModule,
|
||||
|
||||
+11
-2
@@ -1,11 +1,20 @@
|
||||
import { Catch, type ExceptionFilter } from '@nestjs/common';
|
||||
import {
|
||||
type ArgumentsHost,
|
||||
Catch,
|
||||
type ExceptionFilter,
|
||||
} from '@nestjs/common';
|
||||
import { type GqlContextType } from '@nestjs/graphql';
|
||||
|
||||
import { PermissionsException } from 'src/engine/metadata-modules/permissions/permissions.exception';
|
||||
import { permissionGraphqlApiExceptionHandler } from 'src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util';
|
||||
|
||||
@Catch(PermissionsException)
|
||||
export class PermissionsGraphqlApiExceptionFilter implements ExceptionFilter {
|
||||
catch(exception: PermissionsException) {
|
||||
catch(exception: PermissionsException, host: ArgumentsHost) {
|
||||
if (host.getType<GqlContextType>() !== 'graphql') {
|
||||
throw exception;
|
||||
}
|
||||
|
||||
return permissionGraphqlApiExceptionHandler(exception);
|
||||
}
|
||||
}
|
||||
|
||||
+28
@@ -300,6 +300,34 @@ describe('Granular settings permissions', () => {
|
||||
expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN);
|
||||
});
|
||||
|
||||
it('should deny access to applications operations when user does not have APPLICATIONS setting permission', async () => {
|
||||
const findOneApplicationQuery = {
|
||||
query: `
|
||||
query FindOneApplication {
|
||||
findOneApplication(id: "20202020-1c25-4d02-bf25-6aeccf7ea419") {
|
||||
applicationVariables {
|
||||
key
|
||||
value
|
||||
}
|
||||
}
|
||||
}
|
||||
`,
|
||||
};
|
||||
|
||||
const response = await client
|
||||
.post('/metadata')
|
||||
.set('Authorization', `Bearer ${APPLE_JONY_MEMBER_ACCESS_TOKEN}`)
|
||||
.send(findOneApplicationQuery);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body.data).toBeNull();
|
||||
expect(response.body.errors).toBeDefined();
|
||||
expect(response.body.errors[0].message).toBe(
|
||||
PermissionsExceptionMessage.PERMISSION_DENIED,
|
||||
);
|
||||
expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN);
|
||||
});
|
||||
|
||||
it('should deny access to API keys operations when user does not have API_KEYS_AND_WEBHOOKS setting permission', async () => {
|
||||
// Test creating an API key (requires API_KEYS_AND_WEBHOOKS permission)
|
||||
const createApiKeyQuery = {
|
||||
|
||||
Reference in New Issue
Block a user