Skip to content

Commit

Permalink
fix: Correctly allow sharees to test credential when opening the modal (
Browse files Browse the repository at this point in the history
#6111)

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

* test: Add testing for automated credential testing

* chore: Remove unnecessary comments
  • Loading branch information
krynble authored and netroy committed May 2, 2023
1 parent 2f015c0 commit 240bb47
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 8 deletions.
8 changes: 8 additions & 0 deletions cypress/e2e/17-sharing.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
5 changes: 2 additions & 3 deletions cypress/pages/modals/credentials-modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) => {
Expand Down
8 changes: 4 additions & 4 deletions cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 240bb47

Please sign in to comment.