github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-06-01 01:07:04 +02:00
Code Issues Actions 12 Packages Projects Releases Wiki Activity Graph

fix(core): Grant admins full data table access (#21316)

Browse Source
This commit is contained in:
Charlie Kolb 2025-10-31 11:14:14 +01:00 committed by GitHub
parent c4b9470427
commit 18012d98c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 91 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts
View File

@ -90,7 +90,14 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [
'folder:list',
'oidc:manage',
'provisioning:manage',
'dataTable:create',
'dataTable:delete',
'dataTable:read',
'dataTable:update',
'dataTable:list',
'dataTable:listProject',
'dataTable:readRow',
'dataTable:writeRow',
'role:manage',
'mcp:manage',
'mcpApiKey:create',

21
packages/cli/src/modules/data-table/__tests__/data-table-aggregate.controller.integration.test.ts
View File

@ -65,21 +65,28 @@ describe('GET /data-tables-global', () => {
expect(response.body.data.data).toHaveLength(0);
});
test('should not list data tables from projects admin has no access to', async () => {
test('should list data tables from projects admin is not explicitly a part of', async () => {
const project = await createTeamProject('test project', owner);
await createDataTable(project, { name: 'Test Data Table' });
const response = await authAdminAgent.get('/data-tables-global').expect(200);
expect(response.body.data.count).toBe(0);
expect(response.body.data.data).toHaveLength(0);
expect(response.body.data.count).toBe(1);
expect(response.body.data.data[0].name).toEqual('Test Data Table');
});
test("should not list data tables from another user's personal project", async () => {
test("should only return data tables from another user's personal project for admins", async () => {
await createDataTable(ownerProject, { name: 'Personal Data Table' });
const response = await authAdminAgent.get('/data-tables-global').expect(200);
expect(response.body.data.count).toBe(0);
expect(response.body.data.data).toHaveLength(0);
{
const response = await authAdminAgent.get('/data-tables-global').expect(200);
expect(response.body.data.count).toBe(1);
expect(response.body.data.data[0].name).toBe('Personal Data Table');
}
{
const response = await authMemberAgent.get('/data-tables-global').expect(200);
expect(response.body.data.count).toBe(0);
expect(response.body.data.data).toHaveLength(0);
}
});
test('should list data tables from team projects where user has project:viewer role', async () => {

24
packages/cli/src/modules/data-table/__tests__/data-table-aggregate.service.integration.test.ts
View File

@ -7,6 +7,7 @@ import {
type Project,
type User,
PROJECT_ADMIN_ROLE,
GLOBAL_ADMIN_ROLE,
} from '@n8n/db';
import { Container } from '@n8n/di';
import type { EntityManager } from '@n8n/typeorm';
@ -135,6 +136,29 @@ describe('dataTableAggregate', () => {
expect(result.count).toBe(0);
});
it('should list all tables for owners and admins', async () => {
// ARRANGE
const currentUser = await createUser({ role: GLOBAL_ADMIN_ROLE });
const dt1 = await dataTableService.createDataTable(project1.id, {
name: 'dataTable1',
columns: [],
});
projectRelationRepository.find.mockResolvedValueOnce([]);
// ACT
const result = await dataTableAggregateService.getManyAndCount(currentUser, {
skip: 0,
take: 10,
});
// ASSERT
expect(result.data).toEqual(
expect.arrayContaining([expect.objectContaining({ id: dt1.id, name: dt1.name })]),
);
expect(result.count).toBe(1);
});
it('should return only the data table matching the given data table id filter', async () => {
// ARRANGE
await dataTableService.createDataTable(project1.id, {

34
packages/cli/src/modules/data-table/__tests__/data-table.controller.integration.test.ts
View File

@ -78,8 +78,8 @@ describe('POST /projects/:projectId/data-tables', () => {
};
await authMemberAgent.post('/projects/non-existing-id/data-tables').send(payload).expect(403);
await authAdminAgent.post('/projects/non-existing-id/data-tables').send(payload).expect(403);
await authOwnerAgent.post('/projects/non-existing-id/data-tables').send(payload).expect(403);
await authAdminAgent.post('/projects/non-existing-id/data-tables').send(payload).expect(404);
await authOwnerAgent.post('/projects/non-existing-id/data-tables').send(payload).expect(404);
});
test('should not create data table when name is empty', async () => {
@ -229,8 +229,8 @@ describe('POST /projects/:projectId/data-tables', () => {
describe('GET /projects/:projectId/data-tables', () => {
test('should not list data tables when project does not exist', async () => {
await authMemberAgent.get('/projects/non-existing-id/data-tables').expect(403);
await authAdminAgent.get('/projects/non-existing-id/data-tables').expect(403);
await authOwnerAgent.get('/projects/non-existing-id/data-tables').expect(403);
await authAdminAgent.get('/projects/non-existing-id/data-tables').expect(404);
await authOwnerAgent.get('/projects/non-existing-id/data-tables').expect(404);
});
test('should not list data tables if user has no access to project', async () => {
@ -239,10 +239,10 @@ describe('GET /projects/:projectId/data-tables', () => {
await authMemberAgent.get(`/projects/${project.id}/data-tables`).expect(403);
});
test('should not list data tables if admin has no access to project', async () => {
test('should list data tables for admins', async () => {
const project = await createTeamProject('test project', owner);
await authAdminAgent.get(`/projects/${project.id}/data-tables`).expect(403);
await authAdminAgent.get(`/projects/${project.id}/data-tables`).expect(200);
});
test("should not list data tables from another user's personal project", async () => {
@ -520,7 +520,7 @@ describe('PATCH /projects/:projectId/data-tables/:dataTableId', () => {
await authOwnerAgent
.patch('/projects/non-existing-id/data-tables/some-data-table-id')
.send(payload)
.expect(403);
.expect(404);
});
test('should not update data table when data table does not exist', async () => {
@ -663,7 +663,7 @@ describe('DELETE /projects/:projectId/data-tables/:dataTableId', () => {
await authOwnerAgent
.delete('/projects/non-existing-id/data-tables/some-data-table-id')
.send({})
.expect(403);
.expect(404);
});
test('should not delete data table when data table does not exist', async () => {
@ -790,7 +790,7 @@ describe('GET /projects/:projectId/data-tables/:dataTableId/columns', () => {
test('should not list columns when project does not exist', async () => {
await authOwnerAgent
.get('/projects/non-existing-id/data-tables/non-existing-id/columns')
.expect(403);
.expect(404);
});
test('should not list columns if user has no access to project', async () => {
@ -889,7 +889,7 @@ describe('POST /projects/:projectId/data-tables/:dataTableId/columns', () => {
await authOwnerAgent
.post('/projects/non-existing-id/data-tables/some-data-table-id/columns')
.send(payload)
.expect(403);
.expect(404);
});
test('should not create column when data table does not exist', async () => {
@ -1115,7 +1115,7 @@ describe('DELETE /projects/:projectId/data-tables/:dataTableId/columns/:columnId
await authOwnerAgent
.delete('/projects/non-existing-id/data-tables/some-data-table-id/columns/some-column-id')
.send({})
.expect(403);
.expect(404);
});
test('should not delete column when data table does not exist', async () => {
@ -1302,7 +1302,7 @@ describe('PATCH /projects/:projectId/data-tables/:dataTableId/columns/:columnId/
await authOwnerAgent
.patch('/projects/non-existing-id/data-tables/some-data-table-id/columns/some-column-id/move')
.send(payload)
.expect(403);
.expect(404);
});
test('should not move column when data table does not exist', async () => {
@ -1526,7 +1526,7 @@ describe('GET /projects/:projectId/data-tables/:dataTableId/rows', () => {
test('should not list rows when project does not exist', async () => {
await authOwnerAgent
.get('/projects/non-existing-id/data-tables/some-data-table-id/rows')
.expect(403);
.expect(404);
});
test('should not list rows when data table does not exist', async () => {
@ -1907,7 +1907,7 @@ describe('POST /projects/:projectId/data-tables/:dataTableId/insert', () => {
await authOwnerAgent
.post('/projects/non-existing-id/data-tables/some-data-table-id/insert')
.send(payload)
.expect(403);
.expect(404);
});
test('should not insert rows when data table does not exist', async () => {
@ -2508,7 +2508,7 @@ describe('DELETE /projects/:projectId/data-tables/:dataTableId/rows', () => {
filters: [{ columnName: 'first', condition: 'eq', value: 'test value' }],
}),
})
.expect(403);
.expect(404);
});
test('should not delete rows when data table does not exist', async () => {
@ -2874,7 +2874,7 @@ describe('POST /projects/:projectId/data-tables/:dataTableId/upsert', () => {
await authOwnerAgent
.post('/projects/non-existing-id/data-tables/some-data-table-id/upsert')
.send(payload)
.expect(403);
.expect(404);
});
test('should not upsert rows when data table does not exist', async () => {
@ -3121,7 +3121,7 @@ describe('PATCH /projects/:projectId/data-tables/:dataTableId/rows', () => {
await authOwnerAgent
.patch('/projects/non-existing-id/data-tables/some-data-table-id/rows')
.send(payload)
.expect(403);
.expect(404);
});
test('should not update row when data table does not exist', async () => {

6
packages/cli/src/modules/data-table/data-table-aggregate.service.ts
View File

@ -6,6 +6,7 @@ import { Service } from '@n8n/di';
import { ProjectService } from '@/services/project.service.ee';
import { DataTableRepository } from './data-table.repository';
import { hasGlobalScope } from '@n8n/permissions';
@Service()
export class DataTableAggregateService {
@ -20,7 +21,12 @@ export class DataTableAggregateService {
async shutdown() {}
async getManyAndCount(user: User, options: ListDataTableQueryDto) {
if (hasGlobalScope(user, 'dataTable:listProject')) {
return await this.dataTableRepository.getManyAndCount(options);
}
const projects = await this.projectService.getProjectRelationsForUser(user);
let projectIds = projects.map((x) => x.projectId);
if (options.filter?.projectId) {
const mask = [options.filter?.projectId].flat();

24
packages/cli/src/modules/data-table/data-table.controller.ts
View File

@ -15,6 +15,7 @@ import {
Body,
Delete,
Get,
Middleware,
Param,
Patch,
Post,
@ -22,6 +23,7 @@ import {
Query,
RestController,
} from '@n8n/decorators';
import { NextFunction, Response } from 'express';
import { DataTableRowReturn } from 'n8n-workflow';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
@ -36,10 +38,30 @@ import { DataTableNameConflictError } from './errors/data-table-name-conflict.er
import { DataTableNotFoundError } from './errors/data-table-not-found.error';
import { DataTableSystemColumnNameConflictError } from './errors/data-table-system-column-name-conflict.error';
import { DataTableValidationError } from './errors/data-table-validation.error';
import { ProjectService } from '@/services/project.service.ee';
@RestController('/projects/:projectId/data-tables')
export class DataTableController {
constructor(private readonly dataTableService: DataTableService) {}
constructor(
private readonly dataTableService: DataTableService,
private readonly projectService: ProjectService,
) {}
@Middleware()
async validateProjectExists(
req: AuthenticatedRequest<{ projectId: string }>,
res: Response,
next: NextFunction,
) {
try {
const { projectId } = req.params;
await this.projectService.getProject(projectId);
next();
} catch (e) {
res.status(404).send('Project not found');
return;
}
}
@Post('/')
@ProjectScope('dataTable:create')