houseassassin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enhance role-check system with stricter checks (#15392)

Browse Source 
## Overview

This PR strengthens our permission system by introducing more granular
role-based access control across the platform.

## Changes

### New Permissions Added
- **Applications** - Control who can install and manage applications
- **Layouts** - Control who can customize page layouts and UI structure
- **AI** - Control access to AI features and agents
- **Upload File** - Separate permission for file uploads
- **Download File** - Separate permission for file downloads (frontend
visibility)

### Security Enhancements
- Implemented whitelist-based validation for workspace field updates
- Added explicit permission guards to core entity resolvers
- Enhanced ESLint rule to enforce permission checks on all mutations
- Created `CustomPermissionGuard` and `NoPermissionGuard` for better
code documentation

### Affected Components
- Core entity resolvers: webhooks, files, domains, applications,
layouts, postgres credentials
- Workspace update mutations now use whitelist validation
- Settings UI updated with new permission controls

### Developer Experience
- ESLint now catches missing permission guards during development
- Explicit guard markers make permission requirements clear in code
review
- Comprehensive test coverage for new permission logic

## Testing
- ✅ All TypeScript type checks pass
- ✅ ESLint validation passes
- ✅ New permission guards properly enforced
- ✅ Frontend UI displays new permissions correctly

## Migration Notes
Existing workspaces will need to assign the new permissions to roles as
needed. By default, all new permissions are set to `false` for non-admin
roles.
This commit is contained in:
Félix Malfait
2025-11-07 15:37:17 +01:00
committed by GitHub
parent 44d6ec2594
commit cff17db6cb
306 changed files with 13863 additions and 976 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.spec.ts
+65 -1
View File
@@ -86,6 +86,42 @@ ruleTester.run(RULE_NAME, rule, {
}
`,
},
{
code: `
@UseGuards(WorkspaceAuthGuard, SettingsPermissionsGuard(PermissionFlagType.WORKSPACE))
class TestResolver {
@Mutation(() => String)
async createSomething() {}
}
`,
},
{
code: `
class TestResolver {
@Mutation(() => String)
@UseGuards(WorkspaceAuthGuard, SettingsPermissionsGuard(PermissionFlagType.WORKSPACE))
async createSomething() {}
}
`,
},
{
code: `
class TestResolver {
@Mutation(() => String)
@UseGuards(WorkspaceAuthGuard, CustomPermissionGuard)
async createSomething() {}
}
`,
},
{
code: `
@UseGuards(WorkspaceAuthGuard, CustomPermissionGuard)
class TestResolver {
@Mutation(() => String)
async createSomething() {}
}
`,
},
],
invalid: [
{
@@ -155,5 +191,33 @@ ruleTester.run(RULE_NAME, rule, {
},
],
},
{
code: `
class TestResolver {
@Mutation(() => String)
@UseGuards(WorkspaceAuthGuard)
async createSomething() {}
}
`,
errors: [
{
messageId: 'graphqlResolversShouldBeGuarded',
},
],
},
{
code: `
@UseGuards(WorkspaceAuthGuard)
class TestResolver {
@Mutation(() => String)
async createSomething() {}
}
`,
errors: [
{
messageId: 'graphqlResolversShouldBeGuarded',
},
],
},
],
});
});
tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.ts
+32 -17
View File
@@ -6,17 +6,24 @@ import { typedTokenHelpers } from '../utils/typedTokenHelpers';
// NOTE: The rule will be available in ESLint configs as "@nx/workspace-graphql-resolvers-should-be-guarded"
export const RULE_NAME = 'graphql-resolvers-should-be-guarded';
export const graphqlResolversShouldBeGuarded = (node: TSESTree.MethodDefinition) => {
export const graphqlResolversShouldBeGuarded = (
node: TSESTree.MethodDefinition,
) => {
const hasGraphQLResolverDecorator = typedTokenHelpers.nodeHasDecoratorsNamed(
node,
['Query', 'Mutation', 'Subscription']
['Query', 'Mutation', 'Subscription'],
);
const hasAuthGuards = typedTokenHelpers.nodeHasAuthGuards(node);
const isMutation = typedTokenHelpers.nodeHasDecoratorsNamed(node, [
'Mutation',
]);
function findClassDeclaration(
node: TSESTree.Node
): TSESTree.ClassDeclaration | null {
const hasAuthGuards = typedTokenHelpers.nodeHasAuthGuards(node);
const hasPermissionsGuard = typedTokenHelpers.nodeHasPermissionsGuard(node);
const findClassDeclaration = (
node: TSESTree.Node,
): TSESTree.ClassDeclaration | null => {
if (node.type === TSESTree.AST_NODE_TYPES.ClassDeclaration) {
return node;
}
@@ -24,7 +31,7 @@ export const graphqlResolversShouldBeGuarded = (node: TSESTree.MethodDefinition)
return findClassDeclaration(node.parent);
}
return null;
}
};
const classNode = findClassDeclaration(node);
@@ -32,11 +39,19 @@ export const graphqlResolversShouldBeGuarded = (node: TSESTree.MethodDefinition)
? typedTokenHelpers.nodeHasAuthGuards(classNode)
: false;
return (
hasGraphQLResolverDecorator &&
!hasAuthGuards &&
!hasAuthGuardsOnResolver
);
const hasPermissionsGuardOnResolver = classNode
? typedTokenHelpers.nodeHasPermissionsGuard(classNode)
: false;
// Basic requirement: all resolvers need auth guards
const missingAuthGuard =
hasGraphQLResolverDecorator && !hasAuthGuards && !hasAuthGuardsOnResolver;
// Additional requirement: mutations need permission guards
const missingPermissionGuard =
isMutation && !hasPermissionsGuard && !hasPermissionsGuardOnResolver;
return missingAuthGuard || missingPermissionGuard;
};
export const rule = createRule<[], 'graphqlResolversShouldBeGuarded'>({
@@ -44,20 +59,20 @@ export const rule = createRule<[], 'graphqlResolversShouldBeGuarded'>({
meta: {
docs: {
description:
'GraphQL root resolvers (Query, Mutation, Subscription) should have authentication guards (UserAuthGuard or WorkspaceAuthGuard) or be explicitly marked as public (PublicEndpointGuard) to maintain our security model.',
'GraphQL root resolvers (Query, Mutation, Subscription) should have authentication guards (UserAuthGuard or WorkspaceAuthGuard) or be explicitly marked as public (PublicEndpointGuard) to maintain our security model. Mutations also require permission guards (SettingsPermissionsGuard or CustomPermissionGuard).',
},
messages: {
graphqlResolversShouldBeGuarded:
'All GraphQL root resolver methods (@Query, @Mutation, @Subscription) should have @UseGuards(UserAuthGuard), @UseGuards(WorkspaceAuthGuard), or @UseGuards(PublicEndpointGuard) decorators, or one decorating the root of the Resolver class.',
'All GraphQL resolvers must have authentication guards (@UseGuards(UserAuthGuard/WorkspaceAuthGuard)). Mutations also require permission guards (@UseGuards(..., SettingsPermissionsGuard(PermissionFlagType.XXX)), CustomPermissionGuard for custom logic, or NoPermissionGuard for special cases like onboarding).',
},
schema: [],
hasSuggestions: false,
type: 'suggestion',
},
defaultOptions: [],
create(context) {
create: (context) => {
return {
MethodDefinition(node: TSESTree.MethodDefinition): void {
MethodDefinition: (node: TSESTree.MethodDefinition): void => {
if (graphqlResolversShouldBeGuarded(node)) {
context.report({
node: node,
@@ -67,4 +82,4 @@ export const rule = createRule<[], 'graphqlResolversShouldBeGuarded'>({
},
};
},
});
});
tools/eslint-rules/utils/typedTokenHelpers.ts
+52 -13
View File
@@ -1,10 +1,10 @@
import { TSESTree } from '@typescript-eslint/utils';
export const typedTokenHelpers = {
nodeHasDecoratorsNamed(
nodeHasDecoratorsNamed: (
node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration,
decoratorNames: string[]
): boolean {
decoratorNames: string[],
): boolean => {
if (!node.decorators) {
return false;
}
@@ -13,8 +13,10 @@ export const typedTokenHelpers = {
if (decorator.expression.type === TSESTree.AST_NODE_TYPES.Identifier) {
return decoratorNames.includes(decorator.expression.name);
}
if (decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression) {
if (
decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression
) {
const callee = decorator.expression.callee;
if (callee.type === TSESTree.AST_NODE_TYPES.Identifier) {
return decoratorNames.includes(callee.name);
@@ -25,9 +27,9 @@ export const typedTokenHelpers = {
});
},
nodeHasAuthGuards(
node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration
): boolean {
nodeHasAuthGuards: (
node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration,
): boolean => {
if (!node.decorators) {
return false;
}
@@ -36,15 +38,18 @@ export const typedTokenHelpers = {
// Check for @UseGuards() call expression
if (
decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression &&
decorator.expression.callee.type === TSESTree.AST_NODE_TYPES.Identifier &&
decorator.expression.callee.type ===
TSESTree.AST_NODE_TYPES.Identifier &&
decorator.expression.callee.name === 'UseGuards'
) {
// Check the arguments for UserAuthGuard, WorkspaceAuthGuard, or PublicEndpoint
return decorator.expression.arguments.some((arg) => {
if (arg.type === TSESTree.AST_NODE_TYPES.Identifier) {
return arg.name === 'UserAuthGuard' ||
arg.name === 'WorkspaceAuthGuard' ||
arg.name === 'PublicEndpointGuard';
return (
arg.name === 'UserAuthGuard' ||
arg.name === 'WorkspaceAuthGuard' ||
arg.name === 'PublicEndpointGuard'
);
}
return false;
});
@@ -53,4 +58,38 @@ export const typedTokenHelpers = {
return false;
});
},
};
nodeHasPermissionsGuard: (
node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration,
): boolean => {
if (!node.decorators) {
return false;
}
return node.decorators.some((decorator) => {
if (
decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression &&
decorator.expression.callee.type ===
TSESTree.AST_NODE_TYPES.Identifier &&
decorator.expression.callee.name === 'UseGuards'
) {
// Check if any argument ends with PermissionGuard
return decorator.expression.arguments.some((arg) => {
// Factory-style guards: SettingsPermissionsGuard(PermissionFlagType.XXX)
if (arg.type === TSESTree.AST_NODE_TYPES.CallExpression) {
const callee = arg.callee;
if (callee.type === TSESTree.AST_NODE_TYPES.Identifier) {
return callee.name.endsWith('PermissionGuard');
}
}
// Identifier guards: CustomPermissionGuard, NoPermissionGuard, etc.
if (arg.type === TSESTree.AST_NODE_TYPES.Identifier) {
return arg.name.endsWith('PermissionGuard');
}
return false;
});
}
return false;
});
},
};