fix(core): Assign credential ownership correctly in source control import

packages/cli/src/environments/sourceControl/sourceControlExport.service.ee.ts

@@ -239,23 +239,28 @@ export class SourceControlExportService {
 				);
 			}
 			await Promise.all(
-				credentialsToBeExported.map(async (sharedCredential) => {
-					const { name, type, nodesAccess, data, id } = sharedCredential.credentials;
-					const credentialObject = new Credentials({ id, name }, type, nodesAccess, data);
-					const plainData = credentialObject.getData();
-					const sanitizedData = this.replaceCredentialData(plainData);
-					const fileName = this.getCredentialsPath(sharedCredential.credentials.id);
-					const sanitizedCredential: ExportableCredential = {
-						id: sharedCredential.credentials.id,
-						name: sharedCredential.credentials.name,
-						type: sharedCredential.credentials.type,
-						data: sanitizedData,
-						nodesAccess: sharedCredential.credentials.nodesAccess,
-					};
-					this.logger.debug(`Writing credential ${sharedCredential.credentials.id} to ${fileName}`);
-					return await fsWriteFile(fileName, JSON.stringify(sanitizedCredential, null, 2));
-				}),
+				credentialsToBeExported
+					.filter((sharing) => sharing.role === 'credential:owner')
+					.map(async (sharing) => {
+						const { name, type, nodesAccess, data, id } = sharing.credentials;
+						const credentials = new Credentials({ id, name }, type, nodesAccess, data);
+
+						const stub: ExportableCredential = {
+							id,
+							name,
+							type,
+							data: this.replaceCredentialData(credentials.getData()),
+							nodesAccess,
+							ownedBy: sharing.user.email,
+						};
+
+						const filePath = this.getCredentialsPath(id);
+						this.logger.debug(`Writing credentials stub "${name}" (ID ${id}) to: ${filePath}`);
+
+						return await fsWriteFile(filePath, JSON.stringify(stub, null, 2));
+					}),
 			);
+
 			return {
 				count: credentialsToBeExported.length,
 				folder: this.credentialExportFolder,

packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts

@@ -310,7 +310,10 @@ export class SourceControlImportService {
 		}>;
 	}
 
-	public async importCredentialsFromWorkFolder(candidates: SourceControlledFile[], userId: string) {
+	public async importCredentialsFromWorkFolder(
+		candidates: SourceControlledFile[],
+		importingUserId: string,
+	) {
 		const candidateIds = candidates.map((c) => c.id);
 		const existingCredentials = await Container.get(CredentialsRepository).find({
 			where: {
@@ -335,9 +338,6 @@ export class SourceControlImportService {
 				const existingCredential = existingCredentials.find(
 					(e) => e.id === credential.id && e.type === credential.type,
 				);
-				const sharedOwner = existingSharedCredentials.find(
-					(e) => e.credentialsId === credential.id,
-				);
 
 				const { name, type, data, id, nodesAccess } = credential;
 				const newCredentialObject = new Credentials({ id, name }, type, []);
@@ -351,10 +351,21 @@ export class SourceControlImportService {
 				this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`);
 				await Container.get(CredentialsRepository).upsert(newCredentialObject, ['id']);
 
-				if (!sharedOwner) {
+				const isOwnedLocally = existingSharedCredentials.some(
+					(c) => c.credentialsId === credential.id,
+				);
+
+				if (!isOwnedLocally) {
+					const remoteOwnerId = await Container.get(UserRepository)
+						.findOne({
+							where: { email: credential.ownedBy },
+							select: { id: true },
+						})
+						.then((user) => user?.id);
+
 					const newSharedCredential = new SharedCredentials();
 					newSharedCredential.credentialsId = newCredentialObject.id as string;
-					newSharedCredential.userId = userId;
+					newSharedCredential.userId = remoteOwnerId ?? importingUserId;
 					newSharedCredential.role = 'credential:owner';
 
 					await Container.get(SharedCredentialsRepository).upsert({ ...newSharedCredential }, [

packages/cli/src/environments/sourceControl/types/exportableCredential.ts

@@ -6,4 +6,10 @@ export interface ExportableCredential {
 	type: string;
 	data: ICredentialDataDecryptedObject;
 	nodesAccess: ICredentialNodeAccess[];
+
+	/**
+	 * Email of the user who owns this credential at the source instance.
+	 * Ownership is mirrored at target instance if user is also present there.
+	 */
+	ownedBy: string;
 }

packages/cli/src/environments/sourceControl/types/sourceControlPullWorkFolder.ts

@@ -19,6 +19,7 @@ export class SourceControlPullWorkFolder {
 }
 
 export class SourceControllPullOptions {
+	/** ID of user performing a source control pull. */
 	userId: string;
 
 	force?: boolean;

packages/cli/test/integration/environments/source-control-import.service.test.ts

@@ -0,0 +1,116 @@
+import fsp from 'node:fs/promises';
+import Container from 'typedi';
+import { mock } from 'jest-mock-extended';
+import * as utils from 'n8n-workflow';
+import { Cipher } from 'n8n-core';
+import { nanoid } from 'nanoid';
+import type { InstanceSettings } from 'n8n-core';
+
+import * as testDb from '../shared/testDb';
+import { SourceControlImportService } from '@/environments/sourceControl/sourceControlImport.service.ee';
+import { createMember, getGlobalOwner } from '../shared/db/users';
+import { SharedCredentialsRepository } from '@/databases/repositories/sharedCredentials.repository';
+import { mockInstance } from '../../shared/mocking';
+import type { SourceControlledFile } from '@/environments/sourceControl/types/sourceControlledFile';
+import type { ExportableCredential } from '@/environments/sourceControl/types/exportableCredential';
+
+describe('SourceControlImportService', () => {
+	let service: SourceControlImportService;
+	const cipher = mockInstance(Cipher);
+
+	beforeAll(async () => {
+		service = new SourceControlImportService(
+			mock(),
+			mock(),
+			mock(),
+			mock(),
+			mock<InstanceSettings>({ n8nFolder: '/some-path' }),
+		);
+
+		await testDb.init();
+	});
+
+	afterEach(async () => {
+		await testDb.truncate(['Credentials', 'SharedCredentials']);
+
+		jest.restoreAllMocks();
+	});
+
+	afterAll(async () => {
+		await testDb.terminate();
+	});
+
+	describe('importCredentialsFromWorkFolder()', () => {
+		describe('if user specified by `ownedBy` exists at target instance', () => {
+			it('should assign credential ownership to original user ', async () => {
+				const [importingUser, member] = await Promise.all([getGlobalOwner(), createMember()]);
+
+				fsp.readFile = jest.fn().mockResolvedValue(Buffer.from('some-content'));
+
+				const CREDENTIAL_ID = nanoid();
+
+				const stub: ExportableCredential = {
+					id: CREDENTIAL_ID,
+					name: 'My Credential',
+					type: 'someCredentialType',
+					data: {},
+					nodesAccess: [],
+					ownedBy: member.email, // user at source instance owns credential
+				};
+
+				jest.spyOn(utils, 'jsonParse').mockReturnValue(stub);
+
+				cipher.encrypt.mockReturnValue('some-encrypted-data');
+
+				await service.importCredentialsFromWorkFolder(
+					[mock<SourceControlledFile>({ id: CREDENTIAL_ID })],
+					importingUser.id,
+				);
+
+				const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
+					credentialsId: CREDENTIAL_ID,
+					userId: member.id,
+					role: 'credential:owner',
+				});
+
+				expect(sharing).toBeTruthy(); // same user at target instance owns credential
+			});
+		});
+
+		describe('if user specified by `ownedBy` does not exist at target instance', () => {
+			it('should assign credential ownership to the importing user', async () => {
+				const importingUser = await getGlobalOwner();
+
+				fsp.readFile = jest.fn().mockResolvedValue(Buffer.from('some-content'));
+
+				const CREDENTIAL_ID = nanoid();
+
+				const stub: ExportableCredential = {
+					id: CREDENTIAL_ID,
+					name: 'My Credential',
+					type: 'someCredentialType',
+					data: {},
+					nodesAccess: [],
+					ownedBy: '[email protected]', // user at source instance owns credential
+				};
+
+				jest.spyOn(utils, 'jsonParse').mockReturnValue(stub);
+
+				cipher.encrypt.mockReturnValue('some-encrypted-data');
+
+				await service.importCredentialsFromWorkFolder(
+					[mock<SourceControlledFile>({ id: CREDENTIAL_ID })],
+					importingUser.id,
+				);
+
+				const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
+					credentialsId: CREDENTIAL_ID,
+					userId: importingUser.id,
+					role: 'credential:owner',
+				});
+
+				expect(sharing).toBeTruthy(); // original user missing, so importing user owns credential
+			});
+		});
+	});
+});

packages/cli/test/integration/shared/db/users.ts

@@ -137,3 +137,7 @@ export const getLdapIdentities = async () =>
 		where: { providerType: 'ldap' },
 		relations: ['user'],
 	});
+
+export async function getGlobalOwner() {
+	return await Container.get(UserRepository).findOneByOrFail({ role: 'global:owner' });
+}