feat(core): Handle scoped resource not existing with 404 (#16022)

packages/cli/src/controller.registry.ts

@@ -18,6 +18,8 @@ import { userHasScopes } from '@/permissions.ee/check-access';
 import type { AuthenticatedRequest } from '@/requests';
 import { send } from '@/response-helper'; // TODO: move `ResponseHelper.send` to this file
 
+import { NotFoundError } from './errors/response-errors/not-found.error';
+
 @Service()
 export class ControllerRegistry {
 	constructor(
@@ -125,12 +127,20 @@ export class ControllerRegistry {
 
 			const { scope, globalOnly } = accessScope;
 
-			if (!(await userHasScopes(req.user, [scope], globalOnly, req.params))) {
-				res.status(403).json({
-					status: 'error',
-					message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE,
-				});
-				return;
+			try {
+				if (!(await userHasScopes(req.user, [scope], globalOnly, req.params))) {
+					res.status(403).json({
+						status: 'error',
+						message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE,
+					});
+					return;
+				}
+			} catch (error) {
+				if (error instanceof NotFoundError) {
+					res.status(404).json({ status: 'error', message: error.message });
+					return;
+				}
+				throw error;
 			}
 
 			next();

packages/cli/src/permissions.ee/__tests__/check-access.test.ts

@@ -0,0 +1,158 @@
+import {
+	ProjectRepository,
+	SharedCredentialsRepository,
+	SharedWorkflowRepository,
+	type User,
+} from '@n8n/db';
+import { Container } from '@n8n/di';
+import type { Scope } from '@n8n/permissions';
+import { mock } from 'jest-mock-extended';
+
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+
+import { userHasScopes } from '../check-access';
+
+describe('userHasScopes', () => {
+	const findByWorkflowMock = jest.fn();
+	const findByCredentialMock = jest.fn();
+
+	beforeAll(() => {
+		Container.set(
+			SharedWorkflowRepository,
+			mock<SharedWorkflowRepository>({
+				findBy: findByWorkflowMock,
+			}),
+		);
+
+		Container.set(
+			SharedCredentialsRepository,
+			mock<SharedCredentialsRepository>({
+				findBy: findByCredentialMock,
+			}),
+		);
+
+		Container.set(
+			ProjectRepository,
+			mock<ProjectRepository>({
+				find: jest.fn().mockResolvedValue([
+					{
+						id: 'projectId',
+						projectRelations: [{ userId: 'userId', role: 'project:admin' }],
+					},
+				]),
+			}),
+		);
+	});
+
+	beforeEach(() => {
+		findByWorkflowMock.mockReset();
+		findByCredentialMock.mockReset();
+	});
+
+	it.each<{ type: 'workflow' | 'credential'; id: string }>([
+		{
+			type: 'workflow',
+			id: 'workflowId',
+		},
+		{
+			type: 'credential',
+			id: 'credentialId',
+		},
+	])('should return 404 if the resource is not found', async ({ type, id }) => {
+		findByWorkflowMock.mockResolvedValueOnce([]);
+		findByCredentialMock.mockResolvedValueOnce([]);
+
+		const user = { id: 'userId', scopes: [], role: 'global:member' } as unknown as User;
+		const scopes = ['workflow:read', 'credential:read'] as Scope[];
+
+		const params: { credentialId?: string; workflowId?: string; projectId?: string } = {
+			projectId: 'projectId',
+		};
+		if (type === 'credential') {
+			params.credentialId = id;
+		} else {
+			params.workflowId = id;
+		}
+		await expect(userHasScopes(user, scopes, false, params)).rejects.toThrow(NotFoundError);
+	});
+
+	test.each<{
+		type: 'workflow' | 'credential';
+		id: string;
+		role: string;
+		scope: Scope;
+		userScopes: Scope[];
+		expected: boolean;
+	}>([
+		{
+			type: 'workflow',
+			id: 'workflowId',
+			role: 'workflow:member',
+			scope: 'workflow:delete',
+			userScopes: [],
+			expected: false,
+		},
+		{
+			type: 'credential',
+			id: 'credentialId',
+			role: 'credential:member',
+			scope: 'credential:delete',
+			userScopes: [],
+			expected: false,
+		},
+		{
+			type: 'workflow',
+			id: 'workflowId',
+			role: 'workflow:editor',
+			scope: 'workflow:read',
+			userScopes: ['workflow:read'],
+			expected: true,
+		},
+		{
+			type: 'credential',
+			id: 'credentialId',
+			role: 'credential:user',
+			scope: 'credential:read',
+			userScopes: ['credential:read'],
+			expected: true,
+		},
+	])(
+		'should return $expected if the user has the required scopes for a $type',
+		async ({ type, id, role, scope, userScopes, expected }) => {
+			if (type === 'workflow') {
+				findByWorkflowMock.mockResolvedValueOnce([
+					{
+						workflowId: id,
+						projectId: 'projectId',
+						role,
+					},
+				]);
+			} else {
+				findByCredentialMock.mockResolvedValueOnce([
+					{
+						credentialId: id,
+						projectId: 'projectId',
+						role,
+					},
+				]);
+			}
+
+			const user = {
+				id: 'userId',
+				scopes: userScopes,
+				role: 'global:member',
+			} as unknown as User;
+			const scopes = [scope] as Scope[];
+			const params: { credentialId?: string; workflowId?: string; projectId?: string } = {
+				projectId: 'projectId',
+			};
+			if (type === 'credential') {
+				params.credentialId = id;
+			} else {
+				params.workflowId = id;
+			}
+			const result = await userHasScopes(user, scopes, false, params);
+			expect(result).toBe(expected);
+		},
+	);
+});

packages/cli/src/permissions.ee/check-access.ts

@@ -6,6 +6,8 @@ import { hasGlobalScope, rolesWithScope, type Scope } from '@n8n/permissions';
 import { In } from '@n8n/typeorm';
 import { UnexpectedError } from 'n8n-workflow';
 
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+
 /**
  * Check if a user has the required scopes. The check can be:
  *
@@ -49,19 +51,33 @@ export async function userHasScopes(
 	// those resource roles over the resource being checked.
 
 	if (credentialId) {
-		return await Container.get(SharedCredentialsRepository).existsBy({
+		const credentials = await Container.get(SharedCredentialsRepository).findBy({
 			credentialsId: credentialId,
-			projectId: In(userProjectIds),
-			role: In(rolesWithScope('credential', scopes)),
 		});
+		if (!credentials.length) {
+			throw new NotFoundError(`Credential with ID "${credentialId}" not found.`);
+		}
+
+		return credentials.some(
+			(c) =>
+				userProjectIds.includes(c.projectId) &&
+				rolesWithScope('credential', scopes).includes(c.role),
+		);
 	}
 
 	if (workflowId) {
-		return await Container.get(SharedWorkflowRepository).existsBy({
+		const workflows = await Container.get(SharedWorkflowRepository).findBy({
 			workflowId,
-			projectId: In(userProjectIds),
-			role: In(rolesWithScope('workflow', scopes)),
 		});
+
+		if (!workflows.length) {
+			throw new NotFoundError(`Workflow with ID "${workflowId}" not found.`);
+		}
+
+		return workflows.some(
+			(w) =>
+				userProjectIds.includes(w.projectId) && rolesWithScope('workflow', scopes).includes(w.role),
+		);
 	}
 
 	if (projectId) return userProjectIds.includes(projectId);

packages/cli/src/public-api/v1/shared/middlewares/global.middleware.ts

@@ -6,6 +6,7 @@ import type express from 'express';
 import type { NextFunction } from 'express';
 
 import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { License } from '@/license';
 import { userHasScopes } from '@/permissions.ee/check-access';
 import type { AuthenticatedRequest } from '@/requests';
@@ -36,8 +37,16 @@ const buildScopeMiddleware = (
 				params.credentialId = req.params.id;
 			}
 		}
-		if (!(await userHasScopes(req.user, scopes, globalOnly, params))) {
-			return res.status(403).json({ message: 'Forbidden' });
+
+		try {
+			if (!(await userHasScopes(req.user, scopes, globalOnly, params))) {
+				return res.status(403).json({ message: 'Forbidden' });
+			}
+		} catch (error) {
+			if (error instanceof NotFoundError) {
+				return res.status(404).json({ message: error.message });
+			}
+			throw error;
 		}
 
 		return next();

packages/cli/test/integration/credentials/credentials.api.test.ts

@@ -1260,12 +1260,12 @@ describe('PATCH /credentials/:id', () => {
 		expect(response.statusCode).toBe(404);
 	});
 
-	test('should fail with a 403 if the credential does not exist and the actor does not have the global credential:update scope', async () => {
+	test('should fail with a 404 if the credential does not exist and the actor does not have the global credential:update scope', async () => {
 		const response = await authMemberAgent
 			.patch('/credentials/123')
 			.send(randomCredentialPayload());
 
-		expect(response.statusCode).toBe(403);
+		expect(response.statusCode).toBe(404);
 	});
 
 	test('should fail with a 400 is credential is managed', async () => {