fix: Project Viewer always seeing a connection error when testing credentials (#10417)

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

@@ -127,11 +127,11 @@ export class CredentialsController {
 		const mergedCredentials = deepCopy(credentials);
 		const decryptedData = this.credentialsService.decrypt(storedCredential);
 
-		// When a sharee opens a credential, the fields and the credential data are missing
+		// When a sharee (or project viewer) opens a credential, the fields and the
-		// so the payload will be empty
+		// credential data are missing so the payload will be empty
 		// We need to replace the credential contents with the db version if that's the case
 		// So the credential can be tested properly
-		this.credentialsService.replaceCredentialContentsForSharee(
+		await this.credentialsService.replaceCredentialContentsForSharee(
 			req.user,
 			storedCredential,
 			decryptedData,

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

@@ -38,6 +38,7 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import type { ProjectRelation } from '@/databases/entities/ProjectRelation';
 import { RoleService } from '@/services/role.service';
 import { UserRepository } from '@/databases/repositories/user.repository';
+import { userHasScope } from '@/permissions/checkAccess';
 
 export type CredentialsGetSharedOptions =
 	| { allowGlobalScope: true; globalScope: Scope }
@@ -599,28 +600,20 @@ export class CredentialsService {
 		);
 	}
 
-	replaceCredentialContentsForSharee(
+	async replaceCredentialContentsForSharee(
 		user: User,
 		credential: CredentialsEntity,
 		decryptedData: ICredentialDataDecryptedObject,
 		mergedCredentials: ICredentialsDecrypted,
 	) {
-		credential.shared.forEach((sharedCredentials) => {
+		// We may want to change this to 'credential:decrypt' if that gets added, but this
-			if (sharedCredentials.role === 'credential:owner') {
+		// works for now. The only time we wouldn't want to do this is if the user
-				if (sharedCredentials.project.type === 'personal') {
+		// could actually be testing the credential before saving it, so this should cover
-					// Find the owner of this personal project
+		// the cases we need it for.
-					sharedCredentials.project.projectRelations.forEach((projectRelation) => {
+		if (
-						if (
+			!(await userHasScope(user, ['credential:update'], false, { credentialId: credential.id }))
-							projectRelation.role === 'project:personalOwner' &&
+		) {
-							projectRelation.user.id !== user.id
+			mergedCredentials.data = decryptedData;
-						) {
+		}
-							// If we realize that the current user does not own this credential
-							// We replace the payload with the stored decrypted data
-							mergedCredentials.data = decryptedData;
-						}
-					});
-				}
-			}
-		});
 	}
 }

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

@@ -7,6 +7,7 @@ import { SharedCredentialsRepository } from '@/databases/repositories/sharedCred
 import Container from 'typedi';
 import { CredentialsService } from '@/credentials/credentials.service';
 import * as testDb from '../shared/testDb';
+import { createTeamProject, linkUserToProject } from '@test-integration/db/projects';
 
 const credentialPayload = randomCredentialPayload();
 let memberWhoOwnsCredential: User;
@@ -42,7 +43,7 @@ describe('credentials service', () => {
 				data: { accessToken: '' },
 			};
 
-			Container.get(CredentialsService).replaceCredentialContentsForSharee(
+			await Container.get(CredentialsService).replaceCredentialContentsForSharee(
 				memberWhoDoesNotOwnCredential,
 				storedCredential!,
 				decryptedData,
@@ -51,5 +52,68 @@ describe('credentials service', () => {
 
 			expect(mergedCredentials.data).toEqual({ accessToken: credentialPayload.data.accessToken });
 		});
+
+		it('should replace the contents of the credential for project viewer', async () => {
+			const [project, viewerMember] = await Promise.all([createTeamProject(), createMember()]);
+			await linkUserToProject(viewerMember, project, 'project:viewer');
+			const projectCredential = await saveCredential(credentialPayload, {
+				project,
+				role: 'credential:owner',
+			});
+
+			const storedProjectCredential = await Container.get(
+				SharedCredentialsRepository,
+			).findCredentialForUser(projectCredential.id, viewerMember, ['credential:read']);
+
+			const decryptedData = Container.get(CredentialsService).decrypt(storedProjectCredential!);
+
+			const mergedCredentials = {
+				id: projectCredential.id,
+				name: projectCredential.name,
+				type: projectCredential.type,
+				data: { accessToken: '' },
+			};
+
+			await Container.get(CredentialsService).replaceCredentialContentsForSharee(
+				viewerMember,
+				storedProjectCredential!,
+				decryptedData,
+				mergedCredentials,
+			);
+
+			expect(mergedCredentials.data).toEqual({ accessToken: credentialPayload.data.accessToken });
+		});
+
+		it('should not replace the contents of the credential for project editor', async () => {
+			const [project, editorMember] = await Promise.all([createTeamProject(), createMember()]);
+			await linkUserToProject(editorMember, project, 'project:editor');
+			const projectCredential = await saveCredential(credentialPayload, {
+				project,
+				role: 'credential:owner',
+			});
+
+			const storedProjectCredential = await Container.get(
+				SharedCredentialsRepository,
+			).findCredentialForUser(projectCredential.id, editorMember, ['credential:read']);
+
+			const decryptedData = Container.get(CredentialsService).decrypt(storedProjectCredential!);
+
+			const originalData = { accessToken: '' };
+			const mergedCredentials = {
+				id: projectCredential.id,
+				name: projectCredential.name,
+				type: projectCredential.type,
+				data: originalData,
+			};
+
+			await Container.get(CredentialsService).replaceCredentialContentsForSharee(
+				editorMember,
+				storedProjectCredential!,
+				decryptedData,
+				mergedCredentials,
+			);
+
+			expect(mergedCredentials.data).toBe(originalData);
+		});
 	});
 });