fix(core): Don't allow creating more projects than allowed by exploiting a race condition (#15218)

packages/@n8n/config/src/configs/__tests__/database.config.test.ts

@@ -0,0 +1,39 @@
+import { Container } from '@n8n/di';
+
+import { DatabaseConfig } from '../database.config';
+
+describe('DatabaseConfig', () => {
+	beforeEach(() => {
+		Container.reset();
+		jest.clearAllMocks();
+	});
+
+	test('`isLegacySqlite` defaults to true', () => {
+		const databaseConfig = Container.get(DatabaseConfig);
+		expect(databaseConfig.isLegacySqlite).toBe(true);
+	});
+
+	test.each(['mariadb', 'mysqldb', 'postgresdb'] satisfies Array<DatabaseConfig['type']>)(
+		'`isLegacySqlite` returns false if dbType is `%s`',
+		(dbType) => {
+			const databaseConfig = Container.get(DatabaseConfig);
+			databaseConfig.sqlite.poolSize = 0;
+			databaseConfig.type = dbType;
+			expect(databaseConfig.isLegacySqlite).toBe(false);
+		},
+	);
+
+	test('`isLegacySqlite` returns false if dbType is `sqlite` and `poolSize` > 0', () => {
+		const databaseConfig = Container.get(DatabaseConfig);
+		databaseConfig.sqlite.poolSize = 1;
+		databaseConfig.type = 'sqlite';
+		expect(databaseConfig.isLegacySqlite).toBe(false);
+	});
+
+	test('`isLegacySqlite` returns true if dbType is `sqlite` and `poolSize` is 0', () => {
+		const databaseConfig = Container.get(DatabaseConfig);
+		databaseConfig.sqlite.poolSize = 0;
+		databaseConfig.type = 'sqlite';
+		expect(databaseConfig.isLegacySqlite).toBe(true);
+	});
+});

packages/@n8n/config/src/configs/database.config.ts

@@ -145,6 +145,15 @@ export class DatabaseConfig {
 	@Env('DB_TYPE', dbTypeSchema)
 	type: DbType = 'sqlite';
 
+	/**
+	 * Is true if the default sqlite data source of TypeORM is used, as opposed
+	 * to any other (e.g. postgres)
+	 * This also returns false if n8n's new pooled sqlite data source is used.
+	 */
+	get isLegacySqlite() {
+		return this.type === 'sqlite' && this.sqlite.poolSize === 0;
+	}
+
 	/** Prefix for table names */
 	@Env('DB_TABLE_PREFIX')
 	tablePrefix: string = '';

packages/@n8n/config/test/config.test.ts

@@ -71,6 +71,7 @@ describe('GlobalConfig', () => {
 			},
 			tablePrefix: '',
 			type: 'sqlite',
+			isLegacySqlite: true,
 		},
 		credentials: {
 			defaultName: 'My credentials',
@@ -309,7 +310,10 @@ describe('GlobalConfig', () => {
 	it('should use all default values when no env variables are defined', () => {
 		process.env = {};
 		const config = Container.get(GlobalConfig);
-		expect(structuredClone(config)).toEqual(defaultConfig);
+		// Makes sure the objects are structurally equal while respecting getters,
+		// which `toEqual` and `toBe` does not do.
+		expect(defaultConfig).toMatchObject(config);
+		expect(config).toMatchObject(defaultConfig);
 		expect(mockFs.readFileSync).not.toHaveBeenCalled();
 	});
 
@@ -365,7 +369,7 @@ describe('GlobalConfig', () => {
 		mockFs.readFileSync.calledWith(passwordFile, 'utf8').mockReturnValueOnce('password-from-file');
 
 		const config = Container.get(GlobalConfig);
-		expect(structuredClone(config)).toEqual({
+		const expected = {
 			...defaultConfig,
 			database: {
 				...defaultConfig.database,
@@ -374,7 +378,11 @@ describe('GlobalConfig', () => {
 					password: 'password-from-file',
 				},
 			},
-		});
+		};
+		// Makes sure the objects are structurally equal while respecting getters,
+		// which `toEqual` and `toBe` does not do.
+		expect(config).toMatchObject(expected);
+		expect(expected).toMatchObject(config);
 		expect(mockFs.readFileSync).toHaveBeenCalled();
 	});
 

packages/@n8n/config/tsconfig.build.json

@@ -7,5 +7,5 @@
 		"tsBuildInfoFile": "dist/build.tsbuildinfo"
 	},
 	"include": ["src/**/*.ts"],
-	"exclude": ["test/**"]
+	"exclude": ["test/**", "src/**/__tests__/**"]
 }

packages/cli/src/services/project.service.ee.ts

@@ -1,4 +1,6 @@
 import type { CreateProjectDto, ProjectType, UpdateProjectDto } from '@n8n/api-types';
+import { LicenseState } from '@n8n/backend-common';
+import { DatabaseConfig } from '@n8n/config';
 import { UNLIMITED_LICENSE_QUOTA } from '@n8n/constants';
 import type { User } from '@n8n/db';
 import {
@@ -20,7 +22,6 @@ import { UserError } from 'n8n-workflow';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
-import { License } from '@/license';
 
 import { CacheService } from './cache/cache.service';
 
@@ -46,7 +47,8 @@ export class ProjectService {
 		private readonly projectRelationRepository: ProjectRelationRepository,
 		private readonly sharedCredentialsRepository: SharedCredentialsRepository,
 		private readonly cacheService: CacheService,
-		private readonly license: License,
+		private readonly licenseState: LicenseState,
+		private readonly databaseConfig: DatabaseConfig,
 	) {}
 
 	private get workflowService() {
@@ -181,25 +183,48 @@ export class ProjectService {
 		return await this.projectRelationRepository.getPersonalProjectOwners(projectIds);
 	}
 
-	async createTeamProject(adminUser: User, data: CreateProjectDto): Promise<Project> {
+	private async createTeamProjectWithEntityManager(
-		const limit = this.license.getTeamProjectLimit();
+		adminUser: User,
-		if (
+		data: CreateProjectDto,
-			limit !== UNLIMITED_LICENSE_QUOTA &&
+		trx: EntityManager,
-			limit <= (await this.projectRepository.count({ where: { type: 'team' } }))
+	) {
-		) {
+		const limit = this.licenseState.getMaxTeamProjects();
-			throw new TeamProjectOverQuotaError(limit);
+		if (limit !== UNLIMITED_LICENSE_QUOTA) {
+			const teamProjectCount = await trx.count(Project, { where: { type: 'team' } });
+			if (teamProjectCount >= limit) {
+				throw new TeamProjectOverQuotaError(limit);
+			}
 		}
 
-		const project = await this.projectRepository.save(
+		const project = await trx.save(
+			Project,
 			this.projectRepository.create({ ...data, type: 'team' }),
 		);
 
 		// Link admin
-		await this.addUser(project.id, adminUser.id, 'project:admin');
+		await this.addUser(project.id, adminUser.id, 'project:admin', trx);
 
 		return project;
 	}
 
+	async createTeamProject(adminUser: User, data: CreateProjectDto): Promise<Project> {
+		if (this.databaseConfig.isLegacySqlite) {
+			// Using transaction in the sqlite legacy driver can cause data loss, so
+			// we avoid this here.
+			return await this.createTeamProjectWithEntityManager(
+				adminUser,
+				data,
+				this.projectRepository.manager,
+			);
+		} else {
+			// This needs to be SERIALIZABLE otherwise the count would not block a
+			// concurrent transaction and we could insert multiple projects.
+			return await this.projectRepository.manager.transaction('SERIALIZABLE', async (trx) => {
+				return await this.createTeamProjectWithEntityManager(adminUser, data, trx);
+			});
+		}
+	}
+
 	async updateProject(
 		projectId: string,
 		data: Pick<UpdateProjectDto, 'name' | 'icon'>,
@@ -252,11 +277,11 @@ export class ProjectService {
 	private isProjectRoleLicensed(role: ProjectRole) {
 		switch (role) {
 			case 'project:admin':
-				return this.license.isProjectRoleAdminLicensed();
+				return this.licenseState.isProjectRoleAdminLicensed();
 			case 'project:editor':
-				return this.license.isProjectRoleEditorLicensed();
+				return this.licenseState.isProjectRoleEditorLicensed();
 			case 'project:viewer':
-				return this.license.isProjectRoleViewerLicensed();
+				return this.licenseState.isProjectRoleViewerLicensed();
 			default:
 				return true;
 		}
@@ -326,8 +351,9 @@ export class ProjectService {
 		});
 	}
 
-	async addUser(projectId: string, userId: string, role: ProjectRole) {
+	async addUser(projectId: string, userId: string, role: ProjectRole, trx?: EntityManager) {
-		return await this.projectRelationRepository.save({
+		trx = trx ?? this.projectRelationRepository.manager;
+		return await trx.save(ProjectRelation, {
 			projectId,
 			userId,
 			role,

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

@@ -1,3 +1,4 @@
+import { GlobalConfig } from '@n8n/config';
 import type { Project } from '@n8n/db';
 import { FolderRepository } from '@n8n/db';
 import { ProjectRelationRepository } from '@n8n/db';
@@ -432,6 +433,34 @@ describe('POST /projects/', () => {
 
 		expect(await Container.get(ProjectRepository).count({ where: { type: 'team' } })).toBe(2);
 	});
+
+	const globalConfig = Container.get(GlobalConfig);
+	// Preventing this relies on transactions and we can't use them with the
+	// sqlite legacy driver due to data loss risks.
+	if (!globalConfig.database.isLegacySqlite) {
+		test('should respect the quota when trying to create multiple projects in parallel (no race conditions)', async () => {
+			expect(await Container.get(ProjectRepository).count({ where: { type: 'team' } })).toBe(0);
+			testServer.license.setQuota('quota:maxTeamProjects', 3);
+			const ownerUser = await createOwner();
+			const ownerAgent = testServer.authAgentFor(ownerUser);
+			await expect(
+				Container.get(ProjectRepository).count({ where: { type: 'team' } }),
+			).resolves.toBe(0);
+
+			await Promise.all([
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 1' }),
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 2' }),
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 3' }),
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 4' }),
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 5' }),
+				ownerAgent.post('/projects/').send({ name: 'Test Team Project 6' }),
+			]);
+
+			await expect(
+				Container.get(ProjectRepository).count({ where: { type: 'team' } }),
+			).resolves.toBe(3);
+		});
+	}
 });
 
 describe('PATCH /projects/:projectId', () => {

packages/cli/test/integration/workflows/workflow-sharing.service.test.ts

@@ -1,13 +1,13 @@
+import { LicenseState } from '@n8n/backend-common';
 import type { User } from '@n8n/db';
 import { Container } from '@n8n/di';
+import { mock } from 'jest-mock-extended';
 
-import { License } from '@/license';
 import { ProjectService } from '@/services/project.service.ee';
 import { WorkflowSharingService } from '@/workflows/workflow-sharing.service';
 
 import { createUser } from '../shared/db/users';
 import { createWorkflow, shareWorkflowWithUsers } from '../shared/db/workflows';
-import { LicenseMocker } from '../shared/license';
 import * as testDb from '../shared/test-db';
 
 let owner: User;
@@ -21,11 +21,10 @@ beforeAll(async () => {
 	owner = await createUser({ role: 'global:owner' });
 	member = await createUser({ role: 'global:member' });
 	anotherMember = await createUser({ role: 'global:member' });
-	let license: LicenseMocker;
+	const licenseMock = mock<LicenseState>();
-	license = new LicenseMocker();
+	licenseMock.isSharingLicensed.mockReturnValue(true);
-	license.mock(Container.get(License));
+	licenseMock.getMaxTeamProjects.mockReturnValue(-1);
-	license.enable('feat:sharing');
+	Container.set(LicenseState, licenseMock);
-	license.setQuota('quota:maxTeamProjects', -1);
 	workflowSharingService = Container.get(WorkflowSharingService);
 	projectService = Container.get(ProjectService);
 });