refactor(core): Store projectId on additionalData independent of dataStore context (no-changelog) (#19093)

packages/cli/src/controllers/dynamic-node-parameters.controller.ts

@@ -10,7 +10,6 @@ import type { INodePropertyOptions, NodeParameterValueType } from 'n8n-workflow'
 
 import { DynamicNodeParametersService } from '@/services/dynamic-node-parameters.service';
 import { getBase } from '@/workflow-execute-additional-data';
-import { userHasScopes } from '@/permissions.ee/check-access';
 
 @RestController('/dynamic-node-parameters')
 export class DynamicNodeParametersController {
@@ -22,6 +21,8 @@ export class DynamicNodeParametersController {
 		_res: Response,
 		@Body payload: OptionsRequestDto,
 	): Promise<INodePropertyOptions[]> {
+		await this.service.scrubInaccessibleProjectId(req.user, payload);
+
 		const {
 			credentials,
 			currentNodeParameters,
@@ -33,17 +34,7 @@ export class DynamicNodeParametersController {
 		} = payload;
 
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
-
+		additionalData.dataTableProjectId = projectId;
-		if (projectId) {
-			if (await userHasScopes(req.user, ['dataStore:listProject'], false, { projectId })) {
-				// Project ID is currently only added on the additionalData if the user
-				// has data store listing permission for that project. We should consider
-				// turning this into a more general check, but as of now data stores are
-				// the only nodes with project specific resource locators where we want to ensure
-				// that only data stores belonging to their respective projects are shown.
-				additionalData.dataStoreProjectId = projectId;
-			}
-		}
 
 		if (methodName) {
 			return await this.service.getOptionsViaMethodName(
@@ -75,6 +66,8 @@ export class DynamicNodeParametersController {
 		_res: Response,
 		@Body payload: ResourceLocatorRequestDto,
 	) {
+		await this.service.scrubInaccessibleProjectId(req.user, payload);
+
 		const {
 			path,
 			methodName,
@@ -87,17 +80,7 @@ export class DynamicNodeParametersController {
 		} = payload;
 
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
-
+		additionalData.dataTableProjectId = projectId;
-		if (projectId) {
-			if (await userHasScopes(req.user, ['dataStore:listProject'], false, { projectId })) {
-				// Project ID is currently only added on the additionalData if the user
-				// has data store listing permission for that project. We should consider
-				// turning this into a more general check, but as of now data stores are
-				// the only nodes with project specific resource locators where we want to ensure
-				// that only data stores belonging to their respective projects are shown.
-				additionalData.dataStoreProjectId = projectId;
-			}
-		}
 
 		return await this.service.getResourceLocatorResults(
 			methodName,
@@ -117,21 +100,13 @@ export class DynamicNodeParametersController {
 		_res: Response,
 		@Body payload: ResourceMapperFieldsRequestDto,
 	) {
+		await this.service.scrubInaccessibleProjectId(req.user, payload);
+
 		const { path, methodName, credentials, currentNodeParameters, nodeTypeAndVersion, projectId } =
 			payload;
 
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
-
+		additionalData.dataTableProjectId = projectId;
-		if (projectId) {
-			if (await userHasScopes(req.user, ['dataStore:listProject'], false, { projectId })) {
-				// Project ID is currently only added on the additionalData if the user
-				// has data store listing permission for that project. We should consider
-				// turning this into a more general check, but as of now data stores are
-				// the only nodes with project specific resource locators where we want to ensure
-				// that only data stores belonging to their respective projects are shown.
-				additionalData.dataStoreProjectId = projectId;
-			}
-		}
 
 		return await this.service.getResourceMappingFields(
 			methodName,

packages/cli/src/modules/data-table/data-store-proxy.service.ts

@@ -49,10 +49,10 @@ export class DataStoreProxyService implements DataStoreProxyProvider {
 	async getDataStoreAggregateProxy(
 		workflow: Workflow,
 		node: INode,
-		dataStoreProjectId?: string,
+		projectId?: string,
 	): Promise<IDataStoreProjectAggregateService> {
 		this.validateRequest(node);
-		const projectId = dataStoreProjectId ?? (await this.getProjectId(workflow));
+		projectId = projectId ?? (await this.getProjectId(workflow));
 
 		return this.makeAggregateOperations(projectId);
 	}
@@ -61,10 +61,10 @@ export class DataStoreProxyService implements DataStoreProxyProvider {
 		workflow: Workflow,
 		node: INode,
 		dataStoreId: string,
-		dataStoreProjectId?: string,
+		projectId?: string,
 	): Promise<IDataStoreProjectService> {
 		this.validateRequest(node);
-		const projectId = dataStoreProjectId ?? (await this.getProjectId(workflow));
+		projectId = projectId ?? (await this.getProjectId(workflow));
 
 		return this.makeDataStoreOperations(projectId, dataStoreId);
 	}

packages/cli/src/services/dynamic-node-parameters.service.ts

@@ -26,6 +26,9 @@ import { Workflow, UnexpectedError } from 'n8n-workflow';
 import { NodeTypes } from '@/node-types';
 
 import { WorkflowLoaderService } from './workflow-loader.service';
+import { User } from '@n8n/db';
+import { userHasScopes } from '@/permissions.ee/check-access';
+import { Logger } from '@n8n/backend-common';
 
 type LocalResourceMappingMethod = (
 	this: ILocalLoadOptionsFunctions,
@@ -52,10 +55,29 @@ type NodeMethod =
 @Service()
 export class DynamicNodeParametersService {
 	constructor(
+		private logger: Logger,
 		private nodeTypes: NodeTypes,
 		private workflowLoaderService: WorkflowLoaderService,
 	) {}
 
+	async scrubInaccessibleProjectId(user: User, payload: { projectId?: string }) {
+		// We want to avoid relying on generic project:read permissions to enable
+		// a future with fine-grained permission control dependent on the respective resource
+		// For now we use the dataStore:listProject scope as this is the existing consumer of
+		// the project id
+		if (
+			payload.projectId &&
+			!(await userHasScopes(user, ['dataStore:listProject'], false, {
+				projectId: payload.projectId,
+			}))
+		) {
+			this.logger.warn(
+				`Scrubbed inaccessible projectId ${payload.projectId} from DynamicNodeParameters request`,
+			);
+			payload.projectId = undefined;
+		}
+	}
+
 	/** Returns the available options via a predefined method */
 	async getOptionsViaMethodName(
 		methodName: string,

packages/core/src/execution-engine/index.ts

@@ -8,7 +8,10 @@ declare module 'n8n-workflow' {
 		hooks?: ExecutionLifecycleHooks;
 		externalSecretsProxy: ExternalSecretsProxy;
 		'data-table'?: { dataStoreProxyProvider: DataStoreProxyProvider };
-		dataStoreProjectId?: string;
+		// Project ID is currently only added on the additionalData if the user
+		// has data table listing permission for that project. We should consider
+		// that only data tables belonging to their respective projects are shown.
+		dataTableProjectId?: string;
 	}
 }
 

packages/core/src/execution-engine/node-execution-context/utils/data-store-helper-functions.ts

@@ -17,14 +17,14 @@ export function getDataStoreHelperFunctions(
 			await dataStoreProxyProvider.getDataStoreAggregateProxy(
 				workflow,
 				node,
-				additionalData.dataStoreProjectId,
+				additionalData.dataTableProjectId,
 			),
 		getDataStoreProxy: async (dataStoreId: string) =>
 			await dataStoreProxyProvider.getDataStoreProxy(
 				workflow,
 				node,
 				dataStoreId,
-				additionalData.dataStoreProjectId,
+				additionalData.dataTableProjectId,
 			),
 	};
 }

packages/workflow/src/interfaces.ts

@@ -924,13 +924,13 @@ export type DataStoreProxyProvider = {
 	getDataStoreAggregateProxy(
 		workflow: Workflow,
 		node: INode,
-		dataStoreProjectId?: string,
+		projectId?: string,
 	): Promise<IDataStoreProjectAggregateService>;
 	getDataStoreProxy(
 		workflow: Workflow,
 		node: INode,
 		dataStoreId: string,
-		dataStoreProjectId?: string,
+		projectId?: string,
 	): Promise<IDataStoreProjectService>;
 };