diff --git a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts index a211da8018..c99f3c0cad 100644 --- a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts +++ b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts @@ -71,10 +71,18 @@ export default class DestinationsCreateForm extends Component { if (isValid) { try { - const verb = destination.isNew ? 'created' : 'updated'; - yield destination.save(); - this.flashMessages.success(`Successfully ${verb} the destination ${destination.name}`); - this.store.clearDataset('sync/destination'); + // we only want to save if there are changes + if (destination.dirtyType as unknown as string) { + const verb = destination.isNew ? 'created' : 'updated'; + yield destination.save(); + this.flashMessages.success(`Successfully ${verb} the destination ${destination.name}`); + // when saving a record the server returns all credentials as ****** + // Ember Data observes this as a change, marks the model as dirty and the field will be returned from changedAttributes + // if the user then attempts to update the record the credential will get overwritten with the masked placeholder value + // since the record will be fetched from the details route we can safely unload it to avoid the aforementioned issue + destination.unloadRecord(); + this.store.clearDataset('sync/destination'); + } this.router.transitionTo( 'vault.cluster.sync.secrets.destinations.destination.details', destination.type, diff --git a/ui/tests/acceptance/sync/secrets/destination-test.js b/ui/tests/acceptance/sync/secrets/destination-test.js index 03bfe3976c..4951b76049 100644 --- a/ui/tests/acceptance/sync/secrets/destination-test.js +++ b/ui/tests/acceptance/sync/secrets/destination-test.js @@ -57,4 +57,41 @@ module('Acceptance | enterprise | sync | destination', function (hooks) { await click(ts.confirmButton); assert.dom(ts.destinations.deleteBanner).exists('Delete banner renders'); }); + + test('it should not save placeholder values for credentials and only save when there are changes', async function (assert) { + assert.expect(2); + + const handler = this.server.patch( + '/sys/sync/destinations/vercel-project/destination-vercel', + (schema, req) => { + assert.deepEqual( + JSON.parse(req.requestBody), + { access_token: 'foobar' }, + 'Updated access token sent in patch request' + ); + const { deployment_environments, project_id, team_id, name, type, secret_name_template } = + this.server.create('sync-destination', 'vercel-project'); + return { + data: { + connection_details: { access_token: '*****', deployment_environments, project_id, team_id }, + name, + options: { custom_tags: {}, secret_name_template }, + type, + }, + }; + } + ); + + await visit('vault/sync/secrets/destinations/vercel-project/destination-vercel/edit'); + await click(ts.enableField('accessToken')); + await fillIn(ts.inputByAttr('accessToken'), 'foobar'); + await click(ts.saveButton); + await click(ts.toolbar('Edit destination')); + await click(ts.saveButton); + assert.strictEqual( + handler.numberOfCalls, + 1, + 'Model is not dirty after server returns masked value for credentials and save request is not made when there are no changes' + ); + }); }); diff --git a/ui/tests/helpers/general-selectors.js b/ui/tests/helpers/general-selectors.js index 18c4a20be1..9df84c1e07 100644 --- a/ui/tests/helpers/general-selectors.js +++ b/ui/tests/helpers/general-selectors.js @@ -44,6 +44,7 @@ export const SELECTORS = { infoRowValue: (label) => `[data-test-value-div="${label}"]`, inputByAttr: (attr) => `[data-test-input="${attr}"]`, fieldByAttr: (attr) => `[data-test-field="${attr}"]`, + enableField: (attr) => `[data-test-enable-field="${attr}"] button`, validation: (attr) => `[data-test-field-validation=${attr}]`, validationWarning: (attr) => `[data-test-validation-warning=${attr}]`, messageError: '[data-test-message-error]',