✅ add tests for fetching single credential

packages/cli/src/credentials/credentials.controller.ee.ts

@@ -1,8 +1,7 @@
 /* eslint-disable import/no-cycle */
-/* eslint-disable no-param-reassign */
 import express from 'express';
 import { LoggerProxy } from 'n8n-workflow';
-import { Db, ICredentialsDb, ResponseHelper } from '..';
+import { Db, ResponseHelper } from '..';
 import type { CredentialsEntity } from '../databases/entities/CredentialsEntity';
 
 import type { CredentialRequest } from '../requests';
@@ -28,38 +27,13 @@ EECredentialsController.use((_req, _res, next) => {
 EECredentialsController.get(
 	'/',
 	ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise<CredentialWithSharings[]> => {
-		let allCredentials: ICredentialsDb[] | undefined;
-
 		try {
-			allCredentials = await EECredentials.getAll(req.user, {
+			const allCredentials = await EECredentials.getAll(req.user, {
 				relations: ['shared', 'shared.role', 'shared.user'],
 			});
 
-			return allCredentials.map((credential: CredentialsEntity & CredentialWithSharings) => {
-				credential.ownedBy = null;
-				credential.sharedWith = [];
-
-				credential.shared?.forEach((sharing) => {
-					const { id, email, firstName, lastName } = sharing.user;
-
-					if (sharing.role.name === 'owner') {
-						credential.ownedBy = { id, email, firstName, lastName };
-						return;
-					}
-
-					if (sharing.role.name !== 'owner') {
-						credential.sharedWith?.push({ id, email, firstName, lastName });
-					}
-				});
-
-				// @ts-ignore
-				delete credential.shared;
-
-				// @ts-ignore @TODO_TECH_DEBT: Stringify `id` with entity field transformer
-				credential.id = credential.id.toString();
-
-				return credential;
-			});
+			// eslint-disable-next-line @typescript-eslint/unbound-method
+			return allCredentials.map(EECredentials.addOwnerAndSharings);
 		} catch (error) {
 			LoggerProxy.error('Request to list credentials failed', error as Error);
 			throw error;
@@ -72,11 +46,16 @@ EECredentialsController.get(
  */
 EECredentialsController.get(
 	'/:id',
+	(req, _, next) => (req.params.id === 'new' ? next('router') : next()), // skip ee router and use free one for naming
 	ResponseHelper.send(async (req: CredentialRequest.Get) => {
 		const { id: credentialId } = req.params;
 		const includeDecryptedData = req.query.includeData === 'true';
 
-		const credential = (await EECredentials.get(
+		if (Number.isNaN(Number(credentialId))) {
+			throw new ResponseHelper.ResponseError(`Credential ID must be a number.`, undefined, 400);
+		}
+
+		let credential = (await EECredentials.get(
 			{ id: credentialId },
 			{ relations: ['shared', 'shared.role', 'shared.user'] },
 		)) as CredentialsEntity & CredentialWithSharings;
@@ -95,35 +74,20 @@ EECredentialsController.get(
 			throw new ResponseHelper.ResponseError(`Forbidden.`, undefined, 403);
 		}
 
-		credential.ownedBy = null;
-		credential.sharedWith = [];
-
-		credential.shared?.forEach((sharing) => {
-			const { id, email, firstName, lastName } = sharing.user;
-
-			if (sharing.role.name === 'owner') {
-				credential.ownedBy = { id, email, firstName, lastName };
-				return;
-			}
-
-			if (sharing.role.name !== 'owner') {
-				credential.sharedWith?.push({ id, email, firstName, lastName });
-			}
-		});
-
-		// @ts-ignore
-		delete credential.shared;
+		credential = EECredentials.addOwnerAndSharings(credential);
 
 		// @ts-ignore @TODO_TECH_DEBT: Stringify `id` with entity field transformer
 		credential.id = credential.id.toString();
 
 		if (!includeDecryptedData || !userSharing || userSharing.role.name !== 'owner') {
+			// eslint-disable-next-line @typescript-eslint/no-unused-vars
 			const { id, data: _, ...rest } = credential;
 
 			// @TODO_TECH_DEBT: Stringify `id` with entity field transformer
 			return { id: id.toString(), ...rest };
 		}
 
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars
 		const { id, data: _, ...rest } = credential;
 
 		const key = await EECredentials.getEncryptionKey();

packages/cli/src/credentials/credentials.controller.ts

@@ -73,6 +73,10 @@ credentialsController.get(
 		const { id: credentialId } = req.params;
 		const includeDecryptedData = req.query.includeData === 'true';
 
+		if (Number.isNaN(Number(credentialId))) {
+			throw new ResponseHelper.ResponseError(`Credential ID must be a number.`, undefined, 400);
+		}
+
 		const sharing = await CredentialsService.getSharing(req.user, credentialId, ['credentials']);
 
 		if (!sharing) {

packages/cli/src/credentials/credentials.service.ee.ts

@@ -1,4 +1,5 @@
 /* eslint-disable import/no-cycle */
+/* eslint-disable no-param-reassign */
 import { DeleteResult, EntityManager, In, Not } from 'typeorm';
 import { Db } from '..';
 import { RoleService } from '../role/role.service';
@@ -8,6 +9,7 @@ import { CredentialsEntity } from '../databases/entities/CredentialsEntity';
 import { SharedCredentials } from '../databases/entities/SharedCredentials';
 import { User } from '../databases/entities/User';
 import { UserService } from '../user/user.service';
+import type { CredentialWithSharings } from './credentials.types';
 
 export class EECredentialsService extends CredentialsService {
 	static async isOwned(
@@ -68,4 +70,29 @@ export class EECredentialsService extends CredentialsService {
 
 		return transaction.save(newSharedCredentials);
 	}
+
+	static addOwnerAndSharings(
+		credential: CredentialsEntity & CredentialWithSharings,
+	): CredentialsEntity & CredentialWithSharings {
+		credential.ownedBy = null;
+		credential.sharedWith = [];
+
+		credential.shared?.forEach((sharing) => {
+			const { id, email, firstName, lastName } = sharing.user;
+
+			if (sharing.role.name === 'owner') {
+				credential.ownedBy = { id, email, firstName, lastName };
+				return;
+			}
+
+			if (sharing.role.name !== 'owner') {
+				credential.sharedWith?.push({ id, email, firstName, lastName });
+			}
+		});
+
+		// @ts-ignore
+		delete credential.shared;
+
+		return credential;
+	}
 }

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

@@ -4,9 +4,11 @@ import { In } from 'typeorm';
 
 import { Db } from '../../src';
 import { RESPONSE_ERROR_MESSAGES } from '../../src/constants';
-import type { UserSharingsDetails } from '../../src/credentials/credentials.types';
+import type {
+	CredentialWithSharings,
+	UserSharingsDetails,
+} from '../../src/credentials/credentials.types';
 import * as CredentialHelpers from '../../src/credentials/helpers';
-import type { CredentialsEntity } from '../../src/databases/entities/CredentialsEntity';
 import type { Role } from '../../src/databases/entities/Role';
 import { randomCredentialPayload } from './shared/random';
 import * as testDb from './shared/testDb';
@@ -127,7 +129,10 @@ test('GET /credentials should return all creds for owner', async () => {
 	const [ownerCredential, memberCredential] = response.body.data;
 
 	validateMainCredentialData(ownerCredential);
+	expect(ownerCredential.data).toBeUndefined();
+
 	validateMainCredentialData(memberCredential);
+	expect(memberCredential.data).toBeUndefined();
 
 	expect(ownerCredential.ownedBy).toMatchObject({
 		id: owner.id,
@@ -177,6 +182,7 @@ test('GET /credentials should return only relevant creds for member', async () =
 	const [member1Credential] = response.body.data;
 
 	validateMainCredentialData(member1Credential);
+	expect(member1Credential.data).toBeUndefined();
 
 	expect(member1Credential.ownedBy).toMatchObject({
 		id: member1.id,
@@ -211,46 +217,105 @@ test('GET /credentials/:id should retrieve owned cred for owner', async () => {
 
 	expect(firstResponse.statusCode).toBe(200);
 
-	expect(typeof firstResponse.body.data.name).toBe('string');
-	expect(typeof firstResponse.body.data.type).toBe('string');
-	expect(typeof firstResponse.body.data.nodesAccess[0].nodeType).toBe('string');
-	expect(firstResponse.body.data.data).toBeUndefined();
+	const { data: firstCredential } = firstResponse.body;
+	validateMainCredentialData(firstCredential);
+	expect(firstCredential.data).toBeUndefined();
+	expect(firstCredential.ownedBy).toMatchObject({
+		id: ownerShell.id,
+		email: ownerShell.email,
+		firstName: ownerShell.firstName,
+		lastName: ownerShell.lastName,
+	});
+	expect(firstCredential.sharedWith.length).toBe(0);
 
 	const secondResponse = await authOwnerAgent
 		.get(`/credentials/${savedCredential.id}`)
 		.query({ includeData: true });
 
 	expect(secondResponse.statusCode).toBe(200);
-	expect(typeof secondResponse.body.data.name).toBe('string');
-	expect(typeof secondResponse.body.data.type).toBe('string');
-	expect(typeof secondResponse.body.data.nodesAccess[0].nodeType).toBe('string');
-	expect(secondResponse.body.data.data).toBeDefined();
+
+	const { data: secondCredential } = secondResponse.body;
+	validateMainCredentialData(secondCredential);
+	expect(secondCredential.data).toBeDefined();
+});
+
+test('GET /credentials/:id should retrieve non-owned cred for owner', async () => {
+	const owner = await testDb.createUser({ globalRole: globalOwnerRole });
+	const authOwnerAgent = authAgent(owner);
+	const [member1, member2] = await testDb.createManyUsers(2, {
+		globalRole: globalMemberRole,
+	});
+
+	const savedCredential = await saveCredential(randomCredentialPayload(), { user: member1 });
+	await testDb.shareCredentialWithUsers(savedCredential, [member2]);
+
+	const response1 = await authOwnerAgent.get(`/credentials/${savedCredential.id}`);
+
+	expect(response1.statusCode).toBe(200);
+
+	validateMainCredentialData(response1.body.data);
+	expect(response1.body.data.data).toBeUndefined();
+	expect(response1.body.data.ownedBy).toMatchObject({
+		id: member1.id,
+		email: member1.email,
+		firstName: member1.firstName,
+		lastName: member1.lastName,
+	});
+	expect(response1.body.data.sharedWith.length).toBe(1);
+	expect(response1.body.data.sharedWith[0]).toMatchObject({
+		id: member2.id,
+		email: member2.email,
+		firstName: member2.firstName,
+		lastName: member2.lastName,
+	});
+
+	const response2 = await authOwnerAgent
+		.get(`/credentials/${savedCredential.id}`)
+		.query({ includeData: true });
+
+	expect(response2.statusCode).toBe(200);
+
+	validateMainCredentialData(response2.body.data);
+	expect(response2.body.data.data).toBeUndefined();
+	expect(response2.body.data.sharedWith.length).toBe(1);
 });
 
 test('GET /credentials/:id should retrieve owned cred for member', async () => {
-	const member = await testDb.createUser({ globalRole: globalMemberRole });
-	const authMemberAgent = authAgent(member);
-	const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });
+	const [member1, member2, member3] = await testDb.createManyUsers(3, {
+		globalRole: globalMemberRole,
+	});
+	const authMemberAgent = authAgent(member1);
+	const savedCredential = await saveCredential(randomCredentialPayload(), { user: member1 });
+	await testDb.shareCredentialWithUsers(savedCredential, [member2, member3]);
 
 	const firstResponse = await authMemberAgent.get(`/credentials/${savedCredential.id}`);
 
 	expect(firstResponse.statusCode).toBe(200);
 
-	expect(typeof firstResponse.body.data.name).toBe('string');
-	expect(typeof firstResponse.body.data.type).toBe('string');
-	expect(typeof firstResponse.body.data.nodesAccess[0].nodeType).toBe('string');
-	expect(firstResponse.body.data.data).toBeUndefined();
+	const { data: firstCredential } = firstResponse.body;
+	validateMainCredentialData(firstCredential);
+	expect(firstCredential.data).toBeUndefined();
+	expect(firstCredential.ownedBy).toMatchObject({
+		id: member1.id,
+		email: member1.email,
+		firstName: member1.firstName,
+		lastName: member1.lastName,
+	});
+	expect(firstCredential.sharedWith.length).toBe(2);
+	firstCredential.sharedWith.forEach((sharee: UserSharingsDetails, idx: number) => {
+		expect([member2.id, member3.id]).toContain(sharee.id);
+	});
 
 	const secondResponse = await authMemberAgent
 		.get(`/credentials/${savedCredential.id}`)
 		.query({ includeData: true });
 
 	expect(secondResponse.statusCode).toBe(200);
 
-	expect(typeof secondResponse.body.data.name).toBe('string');
-	expect(typeof secondResponse.body.data.type).toBe('string');
-	expect(typeof secondResponse.body.data.nodesAccess[0].nodeType).toBe('string');
-	expect(secondResponse.body.data.data).toBeDefined();
+	const { data: secondCredential } = secondResponse.body;
+	validateMainCredentialData(secondCredential);
+	expect(secondCredential.data).toBeDefined();
+	expect(firstCredential.sharedWith.length).toBe(2);
 });
 
 test('GET /credentials/:id should not retrieve non-owned cred for member', async () => {
@@ -284,8 +349,14 @@ test('GET /credentials/:id should return 404 if cred not found', async () => {
 	const ownerShell = await testDb.createUserShell(globalOwnerRole);
 
 	const response = await authAgent(ownerShell).get('/credentials/789');
-
 	expect(response.statusCode).toBe(404);
+
+	const responseAbc = await authAgent(ownerShell).get('/credentials/abc');
+	expect(responseAbc.statusCode).toBe(400);
+
+	// because EE router has precedence, check if forwards this route
+	const responseNew = await authAgent(ownerShell).get('/credentials/new');
+	expect(responseNew.statusCode).toBe(200);
 });
 
 // ----------------------------------------
@@ -474,9 +545,10 @@ test('PUT /credentials/:id/share should unshare the credential', async () => {
 	expect(sharedCredentials[0].userId).toBe(owner.id);
 });
 
-function validateMainCredentialData(ownerCredential: CredentialsEntity) {
-	expect(typeof ownerCredential.name).toBe('string');
-	expect(typeof ownerCredential.type).toBe('string');
-	expect(typeof ownerCredential.nodesAccess[0].nodeType).toBe('string');
-	expect(ownerCredential.data).toBeUndefined();
+function validateMainCredentialData(credential: CredentialWithSharings) {
+	expect(typeof credential.name).toBe('string');
+	expect(typeof credential.type).toBe('string');
+	expect(typeof credential.nodesAccess[0].nodeType).toBe('string');
+	expect(credential.ownedBy).toBeDefined();
+	expect(Array.isArray(credential.sharedWith)).toBe(true);
 }