feat(core): Add project-scoped insights permissions by fabian8n · Pull Request #28331 · n8n-io/n8n

fabian8n · 2026-04-10T13:23:43Z

I have seen this code, I have run this code, and I take responsibility for this code.

Summary

Splits insights permissions from global-only to global + project-scoped, following the external secrets pattern.

Changes:

Adds insights:list scope to all 4 project roles (admin, personal owner, editor, viewer)
Extracts shared date filter logic from InsightsController into InsightsBaseController
Creates new InsightsProjectController at /insights/projects/:projectId/* with @ProjectScope('insights:list')
Registers the new controller in the insights module
Existing global controller (/insights/* with @GlobalScope) is unchanged

How it works:

Global owners/admins use /insights/* for instance-wide data (unchanged)
Project members use /insights/projects/:projectId/* for project-scoped data
@ProjectScope middleware handles authorization automatically via route params
A user with access to multiple projects can query each independently

No frontend changes — the UI continues using the existing global endpoints. Frontend integration (KPI cards, project filtering, dashboard) comes in a follow-up PR (LIGO-358).

How to test:

Existing insights tests pass (no regression)
New project controller tests verify projectId routing, date validation, license checks, and route param vs query param precedence

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/LIGO-358

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Docs updated or follow-up ticket created.
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…cope

…cope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CLAassistant · 2026-04-10T13:24:05Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

codecov · 2026-04-10T13:28:07Z

Codecov Report

❌ Patch coverage is 98.24561% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...i/src/modules/insights/insights-base.controller.ts	96.87%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov · 2026-04-10T13:37:23Z

Bundle Report

Changes will increase total bundle size by 72 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
editor-ui-esm	45.57MB	72 bytes (0.0%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: editor-ui-esm

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
`assets/worker-*.js`	3.14MB	3.16MB	17560.39% ⚠️
`assets/worker-*.js`	-3.14MB	17.9kB	-99.43%
`assets/constants-*.js`	72 bytes	3.14MB	0.0%

MarcL · 2026-04-10T16:23:12Z

 	'projectVariable:create',
 	'projectVariable:update',
 	'projectVariable:delete',
+	'insights:list',


All of these permission changes look good. 👍

MarcL · 2026-04-10T16:25:41Z

+
+import type { InsightsService } from './insights.service';
+
+export abstract class InsightsBaseController {


While it does make some sense, no other controller uses this inheritance pattern.
Most of them use a "dependency injection" (DI) pattern.

As we've also got similar logic in the public API, it would make sense to pull these functions with the business logic into a small service which could be injected into the controller and also used by the public API (which can't inherit from this controller). It'll avoid us having duplicated logic.

yeah it is literally just for not duplicating the logic (code). even though it is only a couple of lines. i saw that we don't do this anywhere else. but i thought it might be a better pattern?

The compositional pattern is usually better than inheritance as it's generally easier to test and has less coupling.

I think the only reason we've gone for the inheritance model here is because the insightsService is in use. I agree with @MarcL RE the inheritance vs composition 👍 - we need a "good reason" to reach for inheritance usually and I don't think this is a "good reason"...

Other than the checkDatesFiltersAgainstLicense these can be functional "utility" functions.

(being picky) I also think theres a bit of a "smell" in here 👀 .. prepareDateFilters is doing more than it suggests:

Validating dates and throwing an error

Extracting and defaulting dates (now - 7d) - I think the defaulting should happen higher up when you pluck the values out of the request payload as it's "specific" to each call.

Checking licensing information

MarcL · 2026-04-10T16:29:55Z

+});
+
+describe('InsightsProjectController', () => {
+	const insightsByPeriodRepository = mockInstance(InsightsByPeriodRepository);


The controller has the InsightsService injected into it but we're mocking the InsightsByPeriodRepository (which makes the DB calls). This is coupling the tests to the internals. If they change then the tests could break even though we've not changed the controller. Probably better to mock the InsightsService instead.

MarcL · 2026-04-10T16:32:32Z

+import { InsightsBaseController } from './insights-base.controller';
+import { InsightsService } from './insights.service';
+
+@RestController('/insights/projects')


We already have some API endpoints that exist on the /projects endpoint rather than this /insights endpoint. The pattern is /projects/:projectId/{resouce} e.g. /projects/:projectId/folders and /projects/:projectId/data-tables. It's worth following that pattern for consistency with other project endpoints.

We also don't check the validity of the projectId being passed in. The folder controller has a middleware pattern which returns a 404 if no project exists. We should consider doing something similar (or refactor that middleware out to a shared function if that makes sense).

See:

n8n/packages/cli/src/controllers/folder.controller.ts

Lines 42 to 56 in 9072365

@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;

}

}

MarcL · 2026-04-10T16:37:42Z

+		_req: AuthenticatedRequest,
+		_res: Response,
+		@Param('projectId') projectId: string,
+		@Query query: InsightsDateFilterDto = {},


The InsightsDateFilterDto for the incoming request query parameters contains an optional projectId parameter so this means we have 2 ways of passing in the projectId. I think the controller projectId in the path may take precedence but we should perhaps use another DTO here?

yes that's correct. i created a test for that actually. but yeah maybe making this explicit is better

geemanjs

Given the historic problems in insights - i'd suggest also writing an integration test around these additions.

geemanjs · 2026-04-13T08:39:33Z

+			projectId,
+			startDate,
+			endDate,
+		})) as InsightsByTime[];


If the response contains more/different fields than user expects we should pick out the expected fields rather than cast the type.

Here is another way:
https://www.notion.so/n8n/Request-Response-2265b6e0c94f8032a12ce711f5478a60?source=copy_link#2265b6e0c94f804581f5c4d68e9d5773

geemanjs · 2026-04-13T08:39:54Z

+			projectId,
+			startDate,
+			endDate,
+		})) as RestrictedInsightsByTime[];


Same here - The last thing we want is to send "data" back that we don't want in the response.

geemanjs · 2026-04-13T08:44:03Z

+	private checkDatesFiltersAgainstLicense(dateFilters: { startDate: Date; endDate: Date }) {
+		try {
+			this.insightsService.validateDateFiltersLicense(dateFilters);
+		} catch (error: unknown) {
+			if (error instanceof UserError) {
+				throw new ForbiddenError(error.message);
+			}
+
+			throw new InternalServerError();
+		}
+	}


Any reason why we map these errors? The ones coming back from the insight service seem to give the right information

geemanjs · 2026-04-13T09:14:17Z

+
+import type { InsightsService } from './insights.service';
+
+export abstract class InsightsBaseController {


I think the only reason we've gone for the inheritance model here is because the insightsService is in use. I agree with @MarcL RE the inheritance vs composition 👍 - we need a "good reason" to reach for inheritance usually and I don't think this is a "good reason"...

Other than the checkDatesFiltersAgainstLicense these can be functional "utility" functions.

(being picky) I also think theres a bit of a "smell" in here 👀 .. prepareDateFilters is doing more than it suggests:

Validating dates and throwing an error

Extracting and defaulting dates (now - 7d) - I think the defaulting should happen higher up when you pluck the values out of the request payload as it's "specific" to each call.

Checking licensing information

geemanjs · 2026-04-13T09:14:46Z

+		const today = new Date();
+
+		if (!query.startDate) {
+			return {
+				startDate: DateTime.now().minus({ days: 7 }).toJSDate(),
+				endDate: today,
+			};
+		}


We should use DateTime.utc() for all of these or we'll be using the "system time zone" which could lead to mistakes.

fabian8n and others added 6 commits April 7, 2026 09:47

feat(permissions): add insights:list scope to all project roles

60168d9

refactor(insights): extract date filter logic into base controller

fba4af6

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(insights): add project-scoped insights controller with @ProjectS…

7890311

…cope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(insights): register project controller in insights module

6e55364

fix(insights): use import type for InsightsService in base controller

c79d821

test(insights): add route param vs query param precedence test

05a3851

Merge branch 'master' into insights-permissions-split

9349411

fabian8n requested review from MarcL and geemanjs April 10, 2026 13:47

n8n-assistant Bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Apr 10, 2026

MarcL reviewed Apr 10, 2026

View reviewed changes

geemanjs reviewed Apr 13, 2026

View reviewed changes


		import type { InsightsService } from './insights.service';

		export abstract class InsightsBaseController {

	@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;
	}
	}

Conversation

fabian8n commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

Uh oh!

CLAassistant commented Apr 10, 2026

Uh oh!

codecov Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov Bot commented Apr 10, 2026

Bundle Report

Affected Assets, Files, and Routes:

Assets Changed:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geemanjs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fabian8n commented Apr 10, 2026 •

edited

Loading

codecov Bot commented Apr 10, 2026 •

edited

Loading