Add comprehensive permission guard coverage across GraphQL and REST endpoints (#15739)
This PR enhances our security model by ensuring all GraphQL resolvers and REST API endpoints have appropriate permission guards. ## Changes ### ESLint Rules - Enhanced `graphql-resolvers-should-be-guarded` to require permission guards on all resolvers (Query, Mutation, Subscription), not just mutations - Enhanced `rest-api-methods-should-be-guarded` to require permission guards on all REST endpoints (GET, POST, PUT, PATCH, DELETE), not just mutating methods - Both rules now enforce consistent security: authentication guards + permission guards for all endpoints ### Permission Guards Added **Public Endpoints** - Added `NoPermissionGuard`: - Auth-related queries (checkUserExists, findWorkspaceFromInviteHash, validatePasswordResetToken) - Billing webhooks (Stripe callbacks) - SSO callbacks (SAML authentication) - Workflow webhooks - Cloudflare webhooks - Route trigger endpoints - GraphQL subscriptions - Current workspace queries - Geo-map address autocomplete - View-related read operations (view-field, view-filter, view-group, view-sort, view-filter-group) **Settings Permission Guards** - Added `SettingsPermissionGuard`: - API Keys management: `PermissionFlagType.API_KEYS_AND_WEBHOOKS` - Webhooks management: `PermissionFlagType.API_KEYS_AND_WEBHOOKS` - Page Layouts (write operations): `PermissionFlagType.LAYOUTS` - REST Metadata API: `PermissionFlagType.DATA_MODEL` - Agent operations: `PermissionFlagType.AI` - Remote servers: `PermissionFlagType.DATA_MODEL` - Remote tables: `PermissionFlagType.DATA_MODEL` - Serverless functions: `PermissionFlagType.WORKFLOWS` **Custom Permission Guards** - Added `CustomPermissionGuard`: - REST Core API (permissions checked at query execution layer) - Timeline calendar events (permission checks in service layer) - Timeline messaging (permission checks in service layer) - Search operations (permission checks in service layer) - View operations (permission checks via dedicated view permission guards) ### View Permission Guards - Created dedicated `FindManyViewsPermissionGuard` and `FindOneViewPermissionGuard` for reading views - Created `CreateViewPermissionGuard` for view creation with visibility-based permission checks - All view child entities (view-field, view-filter, view-sort, view-group, view-filter-group) use `NoPermissionGuard` for reads - Write operations on view child entities use dedicated permission guards that check parent view access ### Page Layout Permissions - Read operations (GET/Query) now use `NoPermissionGuard` - users can view layouts without LAYOUTS permission - Write operations (POST/PATCH/DELETE/Mutation) require `SettingsPermissionGuard(PermissionFlagType.LAYOUTS)` - Applied consistently across page-layout, page-layout-tab, and page-layout-widget endpoints ## Security Model All endpoints now follow a consistent pattern: 1. **Authentication**: `UserAuthGuard`, `WorkspaceAuthGuard`, or `PublicEndpointGuard` 2. **Authorization**: One of: - `SettingsPermissionGuard(PermissionFlagType.XXX)` - for settings/admin operations - `CustomPermissionGuard` - when permissions are checked in service/data layer - `NoPermissionGuard` - for public or non-sensitive read operations The ESLint rules automatically enforce this pattern going forward. ## Stats - 47 files changed - 603 insertions, 163 deletions - 3 new guard files created
This commit is contained in:
Félix Malfait
+87
@@ -382,6 +382,93 @@ describe('ViewService', () => {
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('should re-allocate view to current user when changing from WORKSPACE to UNLISTED visibility', async () => {
|
||||
const id = 'view-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
const userWorkspaceId = 'current-user-workspace-id';
|
||||
const workspaceView = {
|
||||
...mockView,
|
||||
visibility: ViewVisibility.WORKSPACE,
|
||||
createdByUserWorkspaceId: null,
|
||||
} as ViewEntity;
|
||||
const updateData = { visibility: ViewVisibility.UNLISTED };
|
||||
const expectedSaveData = {
|
||||
id,
|
||||
visibility: ViewVisibility.UNLISTED,
|
||||
createdByUserWorkspaceId: userWorkspaceId,
|
||||
};
|
||||
const updatedView = {
|
||||
...workspaceView,
|
||||
...expectedSaveData,
|
||||
};
|
||||
|
||||
jest.spyOn(viewService, 'findById').mockResolvedValue(workspaceView);
|
||||
jest.spyOn(viewRepository, 'save').mockResolvedValue(updatedView);
|
||||
|
||||
const result = await viewService.update(
|
||||
id,
|
||||
workspaceId,
|
||||
updateData,
|
||||
userWorkspaceId,
|
||||
);
|
||||
|
||||
expect(viewService.findById).toHaveBeenCalledWith(id, workspaceId);
|
||||
expect(viewRepository.save).toHaveBeenCalledWith(expectedSaveData);
|
||||
expect(result.createdByUserWorkspaceId).toBe(userWorkspaceId);
|
||||
});
|
||||
|
||||
it('should not change createdByUserWorkspaceId when visibility is not changing to UNLISTED', async () => {
|
||||
const id = 'view-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
const userWorkspaceId = 'current-user-workspace-id';
|
||||
const updateData = { name: 'Updated Name' };
|
||||
const updatedView = { ...mockView, ...updateData };
|
||||
|
||||
jest.spyOn(viewService, 'findById').mockResolvedValue(mockView);
|
||||
jest.spyOn(viewRepository, 'save').mockResolvedValue(updatedView);
|
||||
|
||||
await viewService.update(id, workspaceId, updateData, userWorkspaceId);
|
||||
|
||||
expect(viewRepository.save).toHaveBeenCalledWith({
|
||||
id,
|
||||
...updateData,
|
||||
});
|
||||
expect(viewRepository.save).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
createdByUserWorkspaceId: userWorkspaceId,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not change createdByUserWorkspaceId when view is already UNLISTED', async () => {
|
||||
const id = 'view-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
const userWorkspaceId = 'current-user-workspace-id';
|
||||
const originalOwner = 'original-owner-workspace-id';
|
||||
const unlistedView = {
|
||||
...mockView,
|
||||
visibility: ViewVisibility.UNLISTED,
|
||||
createdByUserWorkspaceId: originalOwner,
|
||||
} as ViewEntity;
|
||||
const updateData = { visibility: ViewVisibility.UNLISTED };
|
||||
const updatedView = { ...unlistedView, ...updateData };
|
||||
|
||||
jest.spyOn(viewService, 'findById').mockResolvedValue(unlistedView);
|
||||
jest.spyOn(viewRepository, 'save').mockResolvedValue(updatedView);
|
||||
|
||||
await viewService.update(id, workspaceId, updateData, userWorkspaceId);
|
||||
|
||||
expect(viewRepository.save).toHaveBeenCalledWith({
|
||||
id,
|
||||
...updateData,
|
||||
});
|
||||
expect(viewRepository.save).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
createdByUserWorkspaceId: userWorkspaceId,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('delete', () => {
|
||||
|
||||
@@ -100,9 +100,11 @@ export class ViewV2Service {
|
||||
async updateOne({
|
||||
updateViewInput,
|
||||
workspaceId,
|
||||
userWorkspaceId,
|
||||
}: {
|
||||
updateViewInput: UpdateViewInput;
|
||||
workspaceId: string;
|
||||
userWorkspaceId?: string;
|
||||
}): Promise<ViewDTO> {
|
||||
const {
|
||||
flatViewMaps: existingFlatViewMaps,
|
||||
@@ -121,6 +123,21 @@ export class ViewV2Service {
|
||||
flatViewMaps: existingFlatViewMaps,
|
||||
});
|
||||
|
||||
const existingFlatView = existingFlatViewMaps.byId[updateViewInput.id];
|
||||
|
||||
// If changing visibility from WORKSPACE to UNLISTED, ensure createdByUserWorkspaceId is set
|
||||
// This prevents the view from disappearing for the user making the change
|
||||
if (
|
||||
isDefined(existingFlatView) &&
|
||||
isDefined(updateViewInput.visibility) &&
|
||||
updateViewInput.visibility === 'UNLISTED' &&
|
||||
existingFlatView.visibility === 'WORKSPACE' &&
|
||||
isDefined(userWorkspaceId)
|
||||
) {
|
||||
// Re-allocate the view to the current user
|
||||
flatViewFromUpdateInput.createdByUserWorkspaceId = userWorkspaceId;
|
||||
}
|
||||
|
||||
const validateAndBuildResult =
|
||||
await this.workspaceMigrationValidateBuildAndRunService.validateBuildAndRunWorkspaceMigration(
|
||||
{
|
||||
|
||||
@@ -193,6 +193,7 @@ export class ViewService {
|
||||
id: string,
|
||||
workspaceId: string,
|
||||
updateData: Partial<ViewEntity>,
|
||||
userWorkspaceId?: string,
|
||||
): Promise<ViewEntity> {
|
||||
const existingView = await this.findById(id, workspaceId);
|
||||
|
||||
@@ -206,9 +207,23 @@ export class ViewService {
|
||||
);
|
||||
}
|
||||
|
||||
// If changing visibility from WORKSPACE to UNLISTED, ensure createdByUserWorkspaceId is set
|
||||
// This prevents the view from disappearing for the user making the change
|
||||
const dataToUpdate = { ...updateData };
|
||||
|
||||
if (
|
||||
isDefined(updateData.visibility) &&
|
||||
updateData.visibility === ViewVisibility.UNLISTED &&
|
||||
existingView.visibility === ViewVisibility.WORKSPACE &&
|
||||
isDefined(userWorkspaceId)
|
||||
) {
|
||||
// Re-allocate the view to the current user if it has no owner or a different owner
|
||||
dataToUpdate.createdByUserWorkspaceId = userWorkspaceId;
|
||||
}
|
||||
|
||||
const updatedView = await this.viewRepository.save({
|
||||
id,
|
||||
...updateData,
|
||||
...dataToUpdate,
|
||||
});
|
||||
|
||||
await this.flushGraphQLCache(workspaceId);
|
||||
|
||||
Reference in New Issue
Block a user