From 2e73f4abd04ba7ab929b0fce57bf12651a0a2e49 Mon Sep 17 00:00:00 2001
From: Omar Ajoue <krynble@gmail.com>
Date: Fri, 28 Apr 2023 12:02:28 +0200
Subject: [PATCH] fix: Correctly allow sharees to test credential when opening
 the modal (#6111)

* fix: Prevent incorrect error message when sharee opens a credential

* test: Add testing for automated credential testing

* chore: Remove unnecessary comments
---
 cypress/e2e/17-sharing.cy.ts                              | 8 ++++++++
 cypress/pages/modals/credentials-modal.ts                 | 5 ++---
 cypress/support/commands.ts                               | 8 ++++----
 .../src/components/CredentialEdit/CredentialConfig.vue    | 1 +
 .../src/components/CredentialEdit/CredentialEdit.vue      | 8 +++++++-
 5 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/cypress/e2e/17-sharing.cy.ts b/cypress/e2e/17-sharing.cy.ts
index 1a8b0a3d00..ea61b5a67e 100644
--- a/cypress/e2e/17-sharing.cy.ts
+++ b/cypress/e2e/17-sharing.cy.ts
@@ -164,4 +164,12 @@ describe('Sharing', () => {
 		workflowsPage.getters.workflowCard('Workflow W2').click();
 		workflowPage.actions.executeWorkflow();
 	});
+
+	it('should automatically test C2 when opened by U2 sharee', () => {
+		cy.signin(users[0]);
+
+		cy.visit(credentialsPage.url);
+		credentialsPage.getters.credentialCard('Credential C2').click();
+		credentialsModal.getters.testSuccessTag().should('be.visible');
+	});
 });
diff --git a/cypress/pages/modals/credentials-modal.ts b/cypress/pages/modals/credentials-modal.ts
index 2f5fd298aa..99d712e7e3 100644
--- a/cypress/pages/modals/credentials-modal.ts
+++ b/cypress/pages/modals/credentials-modal.ts
@@ -10,9 +10,7 @@ export class CredentialsModal extends BasePage {
 		newCredentialTypeButton: () => cy.getByTestId('new-credential-type-button'),
 		connectionParameters: () => cy.getByTestId('credential-connection-parameter'),
 		connectionParameter: (fieldName: string) =>
-			this.getters
-				.connectionParameters()
-				.find(`:contains('${fieldName}') .n8n-input input`),
+			this.getters.connectionParameters().find(`:contains('${fieldName}') .n8n-input input`),
 		name: () => cy.getByTestId('credential-name'),
 		nameInput: () => cy.getByTestId('credential-name').find('input'),
 		// Saving of the credentials takes a while on the CI so we need to increase the timeout
@@ -27,6 +25,7 @@ export class CredentialsModal extends BasePage {
 		menu: () => this.getters.editCredentialModal().get('.menu-container'),
 		menuItem: (name: string) => this.getters.menu().get('.n8n-menu-item').contains(name),
 		usersSelect: () => cy.getByTestId('credential-sharing-modal-users-select'),
+		testSuccessTag: () => cy.getByTestId('credentials-config-container-test-success'),
 	};
 	actions = {
 		addUser: (email: string) => {
diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts
index 132745510c..70d8a19882 100644
--- a/cypress/support/commands.ts
+++ b/cypress/support/commands.ts
@@ -58,8 +58,8 @@ Cypress.Commands.add('waitForLoad', (waitForIntercepts = true) => {
 	// These aliases are set-up before each test in cypress/support/e2e.ts
 	// we can't set them up here because at this point it would be too late
 	// and the requests would already have been made
-	if(waitForIntercepts) {
-		cy.wait(['@loadSettings', '@loadLogin'])
+	if (waitForIntercepts) {
+		cy.wait(['@loadSettings', '@loadLogin']);
 	}
 	cy.getByTestId('node-view-loader', { timeout: 20000 }).should('not.exist');
 	cy.get('.el-loading-mask', { timeout: 20000 }).should('not.exist');
@@ -243,8 +243,8 @@ Cypress.Commands.add('drag', (selector, pos, options) => {
 	element.trigger('mousedown');
 	element.trigger('mousemove', {
 		which: 1,
-		pageX: options?.abs? xDiff: originalLocation.right + xDiff,
-		pageY: options?.abs? yDiff: originalLocation.top + yDiff,
+		pageX: options?.abs ? xDiff : originalLocation.right + xDiff,
+		pageY: options?.abs ? yDiff : originalLocation.top + yDiff,
 		force: true,
 	});
 	element.trigger('mouseup', { force: true });
diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue
index 8d173f4506..306de0582d 100644
--- a/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue
+++ b/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue
@@ -58,6 +58,7 @@
 			:buttonTitle="$locale.baseText('credentialEdit.credentialConfig.retryCredentialTest')"
 			:buttonLoading="isRetesting"
 			@click="$emit('retest')"
+			data-test-id="credentials-config-container-test-success"
 		/>
 
 		<template v-if="credentialPermissions.updateConnection">
diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue
index f26b98036b..6614c32a4c 100644
--- a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue
+++ b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue
@@ -256,7 +256,9 @@ export default mixins(showMessage, nodeHelpers).extend({
 
 		setTimeout(() => {
 			if (this.credentialId) {
-				if (!this.requiredPropertiesFilled) {
+				if (!this.requiredPropertiesFilled && this.credentialPermissions.isOwner === true) {
+					// sharees can't see properties, so this check would always fail for them
+					// if the credential contains required fields.
 					this.showValidationWarning = true;
 				} else {
 					this.retestCredential();
@@ -347,6 +349,10 @@ export default mixins(showMessage, nodeHelpers).extend({
 			};
 		},
 		isCredentialTestable(): boolean {
+			// Sharees can always test since they can't see the data.
+			if (this.credentialPermissions.isOwner === false) {
+				return true;
+			}
 			if (this.isOAuthType || !this.requiredPropertiesFilled) {
 				return false;
 			}