From e923955efc9558a47b8e4440f1a0204a5a4c9efb Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 11:10:26 -0500 Subject: [PATCH 01/16] Update add-to-array and remove-from-array helpers --- ui/app/adapters/database/role.js | 2 +- ui/app/helpers/add-to-array.js | 15 +++++++++++-- ui/app/helpers/remove-from-array.js | 19 +++++++++++----- .../integration/helpers/add-to-array-test.js | 8 +++---- .../helpers/remove-from-array-test.js | 22 +++++++++++++------ 5 files changed, 47 insertions(+), 19 deletions(-) diff --git a/ui/app/adapters/database/role.js b/ui/app/adapters/database/role.js index 35f6ea3c4c44..964d716edbfb 100644 --- a/ui/app/adapters/database/role.js +++ b/ui/app/adapters/database/role.js @@ -144,7 +144,7 @@ export default ApplicationAdapter.extend({ async _updateAllowedRoles(store, { role, backend, db, type = 'add' }) { const connection = await store.queryRecord('database/connection', { backend, id: db }); const roles = [...(connection.allowed_roles || [])]; - const allowedRoles = type === 'add' ? addToArray([roles, role]) : removeFromArray([roles, role]); + const allowedRoles = type === 'add' ? addToArray(roles, role) : removeFromArray(roles, role); connection.allowed_roles = allowedRoles; return connection.save(); }, diff --git a/ui/app/helpers/add-to-array.js b/ui/app/helpers/add-to-array.js index b89a12f2b569..5ab02792ccd7 100644 --- a/ui/app/helpers/add-to-array.js +++ b/ui/app/helpers/add-to-array.js @@ -10,7 +10,13 @@ function dedupe(items) { return items.filter((v, i) => items.indexOf(v) === i); } -export function addToArray([array, string]) { +export function addManyToArray(array, otherArray) { + assert(`Both values must be an array`, Array.isArray(array) && Array.isArray(otherArray)); + const newArray = [...array].concat(otherArray); + return dedupe(newArray); +} + +export function addToArray(array, string) { if (!Array.isArray(array)) { assert(`Value provided is not an array`, false); } @@ -19,4 +25,9 @@ export function addToArray([array, string]) { return dedupe(newArray); } -export default buildHelper(addToArray); +export default buildHelper(function ([array, string]) { + if (Array.isArray(string)) { + return addManyToArray(array, string); + } + return addToArray(array, string); +}); diff --git a/ui/app/helpers/remove-from-array.js b/ui/app/helpers/remove-from-array.js index ea62da6de064..0fbc758817e5 100644 --- a/ui/app/helpers/remove-from-array.js +++ b/ui/app/helpers/remove-from-array.js @@ -10,10 +10,14 @@ function dedupe(items) { return items.filter((v, i) => items.indexOf(v) === i); } -export function removeFromArray([array, string]) { - if (!Array.isArray(array)) { - assert(`Value provided is not an array`, false); - } +export function removeManyFromArray(array, toRemove) { + assert(`Both values must be an array`, Array.isArray(array) && Array.isArray(toRemove)); + const a = [...(array || [])]; + return a.filter((v) => !toRemove.includes(v)); +} + +export function removeFromArray(array, string) { + assert(`Value provided is not an array`, Array.isArray(array)); const newArray = [...array]; const idx = newArray.indexOf(string); if (idx >= 0) { @@ -22,4 +26,9 @@ export function removeFromArray([array, string]) { return dedupe(newArray); } -export default buildHelper(removeFromArray); +export default buildHelper(function ([array, string]) { + if (Array.isArray(string)) { + return removeManyFromArray(array, string); + } + return removeFromArray(array, string); +}); diff --git a/ui/tests/integration/helpers/add-to-array-test.js b/ui/tests/integration/helpers/add-to-array-test.js index 3bce33fee067..8d4110b5df89 100644 --- a/ui/tests/integration/helpers/add-to-array-test.js +++ b/ui/tests/integration/helpers/add-to-array-test.js @@ -12,7 +12,7 @@ module('Integration | Helper | add-to-array', function (hooks) { test('it correctly adds a value to an array without mutating the original', function (assert) { const ARRAY = ['horse', 'cow', 'chicken']; - const result = addToArray([ARRAY, 'pig']); + const result = addToArray(ARRAY, 'pig'); assert.deepEqual(result, [...ARRAY, 'pig'], 'Result has additional item'); assert.deepEqual(ARRAY, ['horse', 'cow', 'chicken'], 'original array is not mutated'); }); @@ -20,7 +20,7 @@ module('Integration | Helper | add-to-array', function (hooks) { test('it fails if the first value is not an array', function (assert) { let result; try { - result = addToArray(['not-array', 'string']); + result = addToArray('not-array', 'string'); } catch (e) { result = e.message; } @@ -29,13 +29,13 @@ module('Integration | Helper | add-to-array', function (hooks) { test('it works with non-string arrays', function (assert) { const ARRAY = ['five', 6, '7']; - const result = addToArray([ARRAY, 10]); + const result = addToArray(ARRAY, 10); assert.deepEqual(result, ['five', 6, '7', 10], 'added number value'); }); test('it de-dupes the result', function (assert) { const ARRAY = ['horse', 'cow', 'chicken']; - const result = addToArray([ARRAY, 'horse']); + const result = addToArray(ARRAY, 'horse'); assert.deepEqual(result, ['horse', 'cow', 'chicken']); }); }); diff --git a/ui/tests/integration/helpers/remove-from-array-test.js b/ui/tests/integration/helpers/remove-from-array-test.js index 3e87cd5ea6da..b536f42f6dba 100644 --- a/ui/tests/integration/helpers/remove-from-array-test.js +++ b/ui/tests/integration/helpers/remove-from-array-test.js @@ -5,28 +5,28 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { removeFromArray } from '../../../helpers/remove-from-array'; +import { removeManyFromArray, removeFromArray } from 'vault/helpers/remove-from-array'; module('Integration | Helper | remove-from-array', function (hooks) { setupRenderingTest(hooks); test('it correctly removes a value from an array without mutating the original', function (assert) { const ARRAY = ['horse', 'cow', 'chicken']; - const result = removeFromArray([ARRAY, 'horse']); + const result = removeFromArray(ARRAY, 'horse'); assert.deepEqual(result, ['cow', 'chicken'], 'Result does not have removed item'); assert.deepEqual(ARRAY, ['horse', 'cow', 'chicken'], 'original array is not mutated'); }); test('it returns the same value if the item is not found', function (assert) { const ARRAY = ['horse', 'cow', 'chicken']; - const result = removeFromArray([ARRAY, 'pig']); + const result = removeFromArray(ARRAY, 'pig'); assert.deepEqual(result, ARRAY, 'Results are the same as original array'); }); test('it fails if the first value is not an array', function (assert) { let result; try { - result = removeFromArray(['not-array', 'string']); + result = removeFromArray('not-array', 'string'); } catch (e) { result = e.message; } @@ -35,15 +35,23 @@ module('Integration | Helper | remove-from-array', function (hooks) { test('it works with non-string arrays', function (assert) { const ARRAY = ['five', 6, '7']; - const result1 = removeFromArray([ARRAY, 6]); - const result2 = removeFromArray([ARRAY, 7]); + const result1 = removeFromArray(ARRAY, 6); + const result2 = removeFromArray(ARRAY, 7); assert.deepEqual(result1, ['five', '7'], 'removed number value'); assert.deepEqual(result2, ARRAY, 'did not match on different types'); }); test('it de-dupes the result', function (assert) { const ARRAY = ['horse', 'cow', 'chicken', 'cow']; - const result = removeFromArray([ARRAY, 'horse']); + const result = removeFromArray(ARRAY, 'horse'); assert.deepEqual(result, ['cow', 'chicken']); }); + + test('it works with two arrays', function (assert) { + const ARRAY = ['five', 6, '7']; + const result1 = removeManyFromArray(ARRAY, [6, '7']); + const result2 = removeManyFromArray(ARRAY, ['foo', 'five']); + assert.deepEqual(result1, ['five'], 'removed multiple values'); + assert.deepEqual(result2, [6, '7'], 'did nothing with values that were not in the array'); + }); }); From 95bd2c27e928bf9d7fe332a98b614a644c572e70 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 11:11:34 -0500 Subject: [PATCH 02/16] remove search-select-has-many, moved logic directly into mfa-login-enforcement-form (see #16470) --- .../mfa/mfa-login-enforcement-form.js | 24 +++++++++--- .../components/search-select-with-modal.js | 2 +- ui/lib/core/addon/components/search-select.js | 2 +- .../addon/utils/search-select-has-many.js | 38 ------------------- 4 files changed, 20 insertions(+), 46 deletions(-) delete mode 100644 ui/lib/core/addon/utils/search-select-has-many.js diff --git a/ui/app/components/mfa/mfa-login-enforcement-form.js b/ui/app/components/mfa/mfa-login-enforcement-form.js index f12f9d279c75..6935d6503aee 100644 --- a/ui/app/components/mfa/mfa-login-enforcement-form.js +++ b/ui/app/components/mfa/mfa-login-enforcement-form.js @@ -8,7 +8,8 @@ import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { service } from '@ember/service'; import { task } from 'ember-concurrency'; -import handleHasManySelection from 'core/utils/search-select-has-many'; +import { addManyToArray, addToArray } from 'vault/helpers/add-to-array'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; /** * @module MfaLoginEnforcementForm @@ -64,7 +65,7 @@ export default class MfaLoginEnforcementForm extends Component { for (const { label, key } of this.targetTypes) { const targetArray = await this.args.model[key]; const targets = targetArray.map((value) => ({ label, key, value })); - this.targets.addObjects(targets); + this.targets = addManyToArray(this.targets, targets); // [...this.targets, ...targets]; } } async resetTargetState() { @@ -102,7 +103,6 @@ export default class MfaLoginEnforcementForm extends Component { updateModelForKey(key) { const newValue = this.targets.filter((t) => t.key === key).map((t) => t.value); - // Set the model value directly instead of using Array methods (like .addObject) this.args.model[key] = newValue; } @@ -126,8 +126,18 @@ export default class MfaLoginEnforcementForm extends Component { @action async onMethodChange(selectedIds) { + // first make sure the async relationship is loaded const methods = await this.args.model.mfa_methods; - handleHasManySelection(selectedIds, methods, this.store, 'mfa-method'); + // then remove items that are no longer selected + const updatedList = methods.filter((model) => { + return selectedIds.includes(model.id); + }); + // then add selected items that don't exist in the list already + const modelIds = updatedList.map((model) => model.id); + const toAdd = selectedIds + .filter((id) => !modelIds.includes(id)) + .map((id) => this.store.peekRecord('mfa-method', id)); + this.args.model.mfa_methods = addManyToArray(updatedList, toAdd); } @action @@ -150,7 +160,7 @@ export default class MfaLoginEnforcementForm extends Component { addTarget() { const { label, key } = this.selectedTarget; const value = this.selectedTargetValue; - this.targets.addObject({ label, value, key }); + this.targets = addToArray(this.targets, { label, value, key }); // recalculate value for appropriate model property this.updateModelForKey(key); this.selectedTargetValue = null; @@ -158,7 +168,9 @@ export default class MfaLoginEnforcementForm extends Component { } @action removeTarget(target) { - this.targets.removeObject(target); + // const targets = [...this.targets]; + // targets.splice(targets.indexOf(target), 1); + this.targets = removeFromArray(this.targets, target); // recalculate value for appropriate model property this.updateModelForKey(target.key); } diff --git a/ui/lib/core/addon/components/search-select-with-modal.js b/ui/lib/core/addon/components/search-select-with-modal.js index 85e66a872a4c..fc8c762ed4f9 100644 --- a/ui/lib/core/addon/components/search-select-with-modal.js +++ b/ui/lib/core/addon/components/search-select-with-modal.js @@ -31,7 +31,7 @@ import { filterOptions, defaultMatcher } from 'ember-power-select/utils/group-ut * /> * // * component functionality - * @param {function} onChange - The onchange action for this form field. ** SEE UTIL ** search-select-has-many.js if selecting models from a hasMany relationship + * @param {function} onChange - The onchange action for this form field. ** SEE EXAMPLE ** mfa-login-enforcement-form.js (onMethodChange) for example when selecting models from a hasMany relationship * @param {array} [inputValue] - Array of strings corresponding to the input's initial value, e.g. an array of model ids that on edit will appear as selected items below the input * @param {boolean} [shouldRenderName=false] - By default an item's id renders in the dropdown, `true` displays the name with its id in smaller text beside it *NOTE: the boolean flips automatically with 'identity' models * @param {array} [excludeOptions] - array of strings containing model ids to filter from the dropdown (ex: ['allow_all']) diff --git a/ui/lib/core/addon/components/search-select.js b/ui/lib/core/addon/components/search-select.js index 555941ef8b9a..660298d3f8cf 100644 --- a/ui/lib/core/addon/components/search-select.js +++ b/ui/lib/core/addon/components/search-select.js @@ -28,7 +28,7 @@ import { filterOptions, defaultMatcher } from 'ember-power-select/utils/group-ut * /> * // * component functionality - * @param {function} onChange - The onchange action for this form field. ** SEE UTIL ** search-select-has-many.js if selecting models from a hasMany relationship + * @param {function} onChange - The onchange action for this form field. ** SEE EXAMPLE ** mfa-login-enforcement-form.js (onMethodChange) for example when selecting models from a hasMany relationship * @param {array} [inputValue] - Array of strings corresponding to the input's initial value, e.g. an array of model ids that on edit will appear as selected items below the input * @param {boolean} [disallowNewItems=false] - Controls whether or not the user can add a new item if none found * @param {boolean} [shouldRenderName=false] - By default an item's id renders in the dropdown, `true` displays the name with its id in smaller text beside it *NOTE: the boolean flips automatically with 'identity' models or if this.idKey !== 'id' diff --git a/ui/lib/core/addon/utils/search-select-has-many.js b/ui/lib/core/addon/utils/search-select-has-many.js deleted file mode 100644 index e30ec358cd22..000000000000 --- a/ui/lib/core/addon/utils/search-select-has-many.js +++ /dev/null @@ -1,38 +0,0 @@ -/** - * Copyright (c) HashiCorp, Inc. - * SPDX-License-Identifier: BUSL-1.1 - */ - -/** - * Util to add/remove models in a hasMany relationship via the search-select component - * - * @example of using the util within an action in a component - *```js - * @action - * async onSearchSelectChange(selectedIds) { - * const methods = await this.args.model.mfa_methods; - * handleHasManySelection(selectedIds, methods, this.store, 'mfa-method'); - * } - * - * @param selectedIds array of selected options from search-select component - * @param modelCollection array-like, list of models from the hasMany relationship - * @param store the store so we can call peekRecord() - * @param modelRecord string passed to peekRecord - */ - -export default function handleHasManySelection(selectedIds, modelCollection, store, modelRecord) { - // first check for existing models that have been removed from selection - modelCollection.forEach((model) => { - if (!selectedIds.includes(model.id)) { - modelCollection.removeObject(model); - } - }); - // now check for selected items that don't exist and add them to the model - const modelIds = modelCollection.map((model) => model.id); - selectedIds.forEach((id) => { - if (!modelIds.includes(id)) { - const model = store.peekRecord(modelRecord, id); - modelCollection.addObject(model); - } - }); -} From b0d2358eeb97786a55467b399e999ae282b85efb Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 11:23:24 -0500 Subject: [PATCH 03/16] Replace add/remove object in MFA files - All MFA tests pass --- .../vault/cluster/access/mfa/methods/create.js | 3 ++- ui/app/models/mfa-login-enforcement.js | 10 ++++++---- ui/mirage/handlers/mfa-config.js | 2 +- ui/tests/acceptance/mfa-method-test.js | 2 +- .../components/mfa-login-enforcement-form-test.js | 12 ++++++------ 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ui/app/controllers/vault/cluster/access/mfa/methods/create.js b/ui/app/controllers/vault/cluster/access/mfa/methods/create.js index 93cc7fea3061..063500cbd270 100644 --- a/ui/app/controllers/vault/cluster/access/mfa/methods/create.js +++ b/ui/app/controllers/vault/cluster/access/mfa/methods/create.js @@ -9,6 +9,7 @@ import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { capitalize } from '@ember/string'; import { task } from 'ember-concurrency'; +import { addToArray } from 'vault/helpers/add-to-array'; export default class MfaMethodCreateController extends Controller { @service store; @@ -95,7 +96,7 @@ export default class MfaMethodCreateController extends Controller { // first save method yield this.method.save(); if (this.enforcement) { - this.enforcement.mfa_methods.addObject(this.method); + this.enforcement.mfa_methods = addToArray(this.enforcement.mfa_methods.slice(), this.method); try { // now save enforcement and catch error separately yield this.enforcement.save(); diff --git a/ui/app/models/mfa-login-enforcement.js b/ui/app/models/mfa-login-enforcement.js index a78576cd5200..51c357fe993c 100644 --- a/ui/app/models/mfa-login-enforcement.js +++ b/ui/app/models/mfa-login-enforcement.js @@ -10,6 +10,7 @@ import { methods } from 'vault/helpers/mountable-auth-methods'; import { withModelValidations } from 'vault/decorators/model-validations'; import { isPresent } from '@ember/utils'; import { service } from '@ember/service'; +import { addManyToArray, addToArray } from 'vault/helpers/add-to-array'; const validations = { name: [{ type: 'presence', message: 'Name is required' }], @@ -52,7 +53,7 @@ export default class MfaLoginEnforcementModel extends Model { async prepareTargets() { let authMethods; - const targets = []; + let targets = []; if (this.auth_method_accessors.length || this.auth_method_types.length) { // fetch all auth methods and lookup by accessor to get mount path and type @@ -68,7 +69,8 @@ export default class MfaLoginEnforcementModel extends Model { const selectedAuthMethods = authMethods.filter((model) => { return this.auth_method_accessors.includes(model.accessor); }); - targets.addObjects( + targets = addManyToArray( + targets, selectedAuthMethods.map((method) => ({ icon: this.iconForMount(method.type), link: 'vault.cluster.access.method', @@ -82,7 +84,7 @@ export default class MfaLoginEnforcementModel extends Model { this.auth_method_types.forEach((type) => { const icon = this.iconForMount(type); const mountCount = authMethods.filterBy('type', type).length; - targets.addObject({ + targets = addToArray(targets, { key: 'auth_method_types', icon, title: type, @@ -92,7 +94,7 @@ export default class MfaLoginEnforcementModel extends Model { for (const key of ['identity_entities', 'identity_groups']) { (await this[key]).forEach((model) => { - targets.addObject({ + targets = addToArray(targets, { key, icon: 'user', link: 'vault.cluster.access.identity.show', diff --git a/ui/mirage/handlers/mfa-config.js b/ui/mirage/handlers/mfa-config.js index 2563f9b5d820..0b5f87ecadcb 100644 --- a/ui/mirage/handlers/mfa-config.js +++ b/ui/mirage/handlers/mfa-config.js @@ -38,7 +38,7 @@ export default function (server) { let records = []; if (isMethod) { methods.forEach((method) => { - records.addObjects(schema.db[dbKeyFromType(method)].where({})); + records = [...records, ...schema.db[dbKeyFromType(method)].where({})]; }); } else { records = schema.db.mfaLoginEnforcements.where({}); diff --git a/ui/tests/acceptance/mfa-method-test.js b/ui/tests/acceptance/mfa-method-test.js index 52a82dd5781b..dbe65eb487f5 100644 --- a/ui/tests/acceptance/mfa-method-test.js +++ b/ui/tests/acceptance/mfa-method-test.js @@ -21,7 +21,7 @@ module('Acceptance | mfa-method', function (hooks) { this.store = this.owner.lookup('service:store'); this.getMethods = () => ['Totp', 'Duo', 'Okta', 'Pingid'].reduce((methods, type) => { - methods.addObjects(this.server.db[`mfa${type}Methods`].where({})); + methods = [...methods, ...this.server.db[`mfa${type}Methods`].where({})]; return methods; }, []); return authPage.login(); diff --git a/ui/tests/integration/components/mfa-login-enforcement-form-test.js b/ui/tests/integration/components/mfa-login-enforcement-form-test.js index e593f2d19e08..3498035a42ef 100644 --- a/ui/tests/integration/components/mfa-login-enforcement-form-test.js +++ b/ui/tests/integration/components/mfa-login-enforcement-form-test.js @@ -153,8 +153,8 @@ module('Integration | Component | mfa-login-enforcement-form', function (hooks) test('it should populate fields with model data', async function (assert) { this.model.name = 'foo'; const [method] = await this.store.query('mfa-method', {}); - this.model.mfa_methods.addObject(method); - this.model.auth_method_accessors.addObject('auth_userpass_1234'); + this.model.mfa_methods = [method]; + this.model.auth_method_accessors = ['auth_userpass_1234']; await render(hbs` Date: Wed, 13 Mar 2024 12:51:40 -0500 Subject: [PATCH 04/16] Replace in PKI components (pki tests all passing) --- .../pki/addon/components/page/pki-configuration-edit.ts | 3 ++- ui/lib/pki/addon/components/page/pki-issuer-edit.ts | 9 +++++++-- ui/lib/pki/addon/components/pki-issuer-cross-sign.js | 7 ++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ui/lib/pki/addon/components/page/pki-configuration-edit.ts b/ui/lib/pki/addon/components/page/pki-configuration-edit.ts index 704b77d15401..3545006208f5 100644 --- a/ui/lib/pki/addon/components/page/pki-configuration-edit.ts +++ b/ui/lib/pki/addon/components/page/pki-configuration-edit.ts @@ -18,6 +18,7 @@ import type PkiConfigClusterModel from 'vault/models/pki/config/cluster'; import type PkiConfigCrlModel from 'vault/models/pki/config/crl'; import type PkiConfigUrlsModel from 'vault/models/pki/config/urls'; import type { FormField, TtlEvent } from 'vault/app-types'; +import { addToArray } from 'vault/helpers/add-to-array'; interface Args { acme: PkiConfigAcmeModel; @@ -76,7 +77,7 @@ export default class PkiConfigurationEditComponent extends Component { message: errorMessage(error), }; this.flashMessages.danger(`Error updating config/${modelName}`, { sticky: true }); - this.errors.pushObject(errorObject); + this.errors = addToArray(this.errors, errorObject); } } diff --git a/ui/lib/pki/addon/components/page/pki-issuer-edit.ts b/ui/lib/pki/addon/components/page/pki-issuer-edit.ts index ab233a2ebdef..1f456efb09c1 100644 --- a/ui/lib/pki/addon/components/page/pki-issuer-edit.ts +++ b/ui/lib/pki/addon/components/page/pki-issuer-edit.ts @@ -13,6 +13,8 @@ import errorMessage from 'vault/utils/error-message'; import type RouterService from '@ember/routing/router-service'; import type FlashMessageService from 'vault/services/flash-messages'; import type PkiIssuerModel from 'vault/models/pki/issuer'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; +import { addToArray } from 'vault/helpers/add-to-array'; interface Args { model: PkiIssuerModel; @@ -36,8 +38,11 @@ export default class PkiIssuerEditComponent extends Component { @action setUsage(value: string) { - const method = this.usageValues.includes(value) ? 'removeObject' : 'addObject'; - this.usageValues[method](value); + if (this.usageValues.includes(value)) { + this.usageValues = removeFromArray(this.usageValues, value); + } else { + this.usageValues = addToArray(this.usageValues, value); + } this.args.model.usage = this.usageValues.join(','); } diff --git a/ui/lib/pki/addon/components/pki-issuer-cross-sign.js b/ui/lib/pki/addon/components/pki-issuer-cross-sign.js index eac1b49d4787..947cff70c38e 100644 --- a/ui/lib/pki/addon/components/pki-issuer-cross-sign.js +++ b/ui/lib/pki/addon/components/pki-issuer-cross-sign.js @@ -11,6 +11,7 @@ import { tracked } from '@glimmer/tracking'; import errorMessage from 'vault/utils/error-message'; import { waitFor } from '@ember/test-waiters'; import { parseCertificate } from 'vault/utils/parse-pki-cert'; +import { addToArray } from 'vault/helpers/add-to-array'; /** * @module PkiIssuerCrossSign * PkiIssuerCrossSign components render from a parent issuer's details page to cross-sign an intermediate issuer (from a different mount). @@ -92,7 +93,7 @@ export default class PkiIssuerCrossSign extends Component { // for cross-signing error handling we want to record the list of issuers before the process starts this.intermediateIssuers[intermediateMount] = issuers; - this.validationErrors.addObject({ + this.validationErrors = addToArray(this.validationErrors, { newCrossSignedIssuer: this.nameValidation(newCrossSignedIssuer, issuers), }); } @@ -109,9 +110,9 @@ export default class PkiIssuerCrossSign extends Component { intermediateIssuer, newCrossSignedIssuer ); - this.signedIssuers.addObject({ ...data, hasError: false }); + this.signedIssuers = addToArray(this.signedIssuers, { ...data, hasError: false }); } catch (error) { - this.signedIssuers.addObject({ + this.signedIssuers = addToArray(this.signedIssuers, { ...this.formData[row], hasError: errorMessage(error), hasUnsupportedParams: error.cause ? error.cause.map((e) => e.message).join(', ') : null, From c3a57a98bdb6ad56824cd410362bc24a1154ed83 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 13:08:52 -0500 Subject: [PATCH 05/16] Replace in core addon where applicable --- ui/lib/core/addon/components/form-field.js | 14 ++++++++++---- ui/lib/core/addon/components/kv-object-editor.js | 1 + ui/lib/core/addon/components/object-list-input.js | 3 ++- .../addon/components/search-select-with-modal.js | 14 ++++++++------ ui/lib/core/addon/components/search-select.js | 14 ++++++++------ ui/lib/core/addon/components/string-list.js | 1 + 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/ui/lib/core/addon/components/form-field.js b/ui/lib/core/addon/components/form-field.js index 0349d10ba265..d6ff6c6ea6c2 100644 --- a/ui/lib/core/addon/components/form-field.js +++ b/ui/lib/core/addon/components/form-field.js @@ -10,6 +10,8 @@ import { capitalize } from 'vault/helpers/capitalize'; import { humanize } from 'vault/helpers/humanize'; import { dasherize } from 'vault/helpers/dasherize'; import { assert } from '@ember/debug'; +import { addToArray } from 'vault/helpers/add-to-array'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; /** * @module FormField @@ -193,9 +195,13 @@ export default class FormFieldComponent extends Component { @action handleChecklist(event) { - const valueArray = this.args.model[this.valuePath]; - const method = event.target.checked ? 'addObject' : 'removeObject'; - valueArray[method](event.target.value); - this.setAndBroadcast(valueArray); + let updatedValue = this.args.model[this.valuePath]; + if (event.target.checked) { + updatedValue = addToArray(updatedValue, event.target.value); + } else { + updatedValue = removeFromArray(updatedValue, event.target.value); + } + this.args.model[this.valuePath] = updatedValue; + this.setAndBroadcast(updatedValue); } } diff --git a/ui/lib/core/addon/components/kv-object-editor.js b/ui/lib/core/addon/components/kv-object-editor.js index 084d3cc4b385..385718bba15f 100644 --- a/ui/lib/core/addon/components/kv-object-editor.js +++ b/ui/lib/core/addon/components/kv-object-editor.js @@ -40,6 +40,7 @@ import KVObject from 'vault/lib/kv-object'; */ export default class KvObjectEditor extends Component { + // kvData is type ArrayProxy, so addObject etc are fine here @tracked kvData; get placeholders() { diff --git a/ui/lib/core/addon/components/object-list-input.js b/ui/lib/core/addon/components/object-list-input.js index e63824358700..e4655901ee01 100644 --- a/ui/lib/core/addon/components/object-list-input.js +++ b/ui/lib/core/addon/components/object-list-input.js @@ -7,6 +7,7 @@ import Component from '@glimmer/component'; import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; import { assert } from '@ember/debug'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; /** * @module ObjectListInput @@ -80,7 +81,7 @@ export default class ObjectListInput extends Component { @action removeRow(idx) { const row = this.inputList.objectAt(idx); - this.inputList.removeObject(row); + this.inputList = removeFromArray(this.inputList, row); this.handleChange(); } diff --git a/ui/lib/core/addon/components/search-select-with-modal.js b/ui/lib/core/addon/components/search-select-with-modal.js index fc8c762ed4f9..eb41bd893585 100644 --- a/ui/lib/core/addon/components/search-select-with-modal.js +++ b/ui/lib/core/addon/components/search-select-with-modal.js @@ -9,6 +9,8 @@ import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; import { task } from 'ember-concurrency'; import { filterOptions, defaultMatcher } from 'ember-power-select/utils/group-utils'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; +import { addToArray } from 'vault/helpers/add-to-array'; /** * @module SearchSelectWithModal @@ -81,7 +83,7 @@ export default class SearchSelectWithModal extends Component { return inputValues.map((option) => { const matchingOption = this.dropdownOptions.find((opt) => opt.id === option); // remove any matches from dropdown list - this.dropdownOptions.removeObject(matchingOption); + this.dropdownOptions = removeFromArray(this.dropdownOptions, matchingOption); return { id: option, name: matchingOption ? matchingOption.name : option, @@ -168,8 +170,8 @@ export default class SearchSelectWithModal extends Component { // ----- @action discardSelection(selected) { - this.selectedOptions.removeObject(selected); - this.dropdownOptions.pushObject(selected); + this.selectedOptions = removeFromArray(this.selectedOptions, selected); + this.dropdownOptions = addToArray(this.dropdownOptions, selected); this.handleChange(); } @@ -196,8 +198,8 @@ export default class SearchSelectWithModal extends Component { this.showModal = true; } else { // user has selected an existing item, handleChange immediately - this.selectedOptions.pushObject(selection); - this.dropdownOptions.removeObject(selection); + this.selectedOptions = addToArray(this.selectedOptions, selection); + this.dropdownOptions = removeFromArray(this.dropdownOptions, selection); this.handleChange(); } } @@ -209,7 +211,7 @@ export default class SearchSelectWithModal extends Component { this.showModal = false; if (model && model.currentState.isSaved) { const { name } = model; - this.selectedOptions.pushObject({ name, id: name }); + this.selectedOptions = addToArray(this.selectedOptions, { name, id: name }); this.handleChange(); } this.nameInput = null; diff --git a/ui/lib/core/addon/components/search-select.js b/ui/lib/core/addon/components/search-select.js index 660298d3f8cf..adb05d1b25f8 100644 --- a/ui/lib/core/addon/components/search-select.js +++ b/ui/lib/core/addon/components/search-select.js @@ -10,6 +10,8 @@ import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; import { resolve } from 'rsvp'; import { filterOptions, defaultMatcher } from 'ember-power-select/utils/group-utils'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; +import { addToArray } from 'vault/helpers/add-to-array'; /** * @module SearchSelect * The `SearchSelect` is an implementation of the [ember-power-select](https://github.com/cibernox/ember-power-select) used for form elements where options come dynamically from the API. @@ -118,7 +120,7 @@ export default class SearchSelect extends Component { : false; // remove any matches from dropdown list - this.dropdownOptions.removeObject(matchingOption); + this.dropdownOptions = removeFromArray(this.dropdownOptions, matchingOption); return { id: option, name: matchingOption ? matchingOption[this.nameKey] : option, @@ -263,9 +265,9 @@ export default class SearchSelect extends Component { @action discardSelection(selected) { - this.selectedOptions.removeObject(selected); + this.selectedOptions = removeFromArray(this.selectedOptions, selected); if (!selected.new) { - this.dropdownOptions.pushObject(selected); + this.dropdownOptions = addToArray(this.dropdownOptions, selected); } this.handleChange(); } @@ -291,10 +293,10 @@ export default class SearchSelect extends Component { selectOrCreate(selection) { if (selection && selection.__isSuggestion__) { const name = selection.__value__; - this.selectedOptions.pushObject({ name, id: name, new: true }); + this.selectedOptions = addToArray(this.selectedOptions, { name, id: name, new: true }); } else { - this.selectedOptions.pushObject(selection); - this.dropdownOptions.removeObject(selection); + this.selectedOptions = addToArray(this.selectedOptions, selection); + this.dropdownOptions = removeFromArray(this.dropdownOptions, selection); } this.handleChange(); } diff --git a/ui/lib/core/addon/components/string-list.js b/ui/lib/core/addon/components/string-list.js index 0a356dd30bc0..cba6e0c1338b 100644 --- a/ui/lib/core/addon/components/string-list.js +++ b/ui/lib/core/addon/components/string-list.js @@ -33,6 +33,7 @@ export default class StringList extends Component { constructor() { super(...arguments); + // inputList is type ArrayProxy, so addObject etc are fine here this.inputList = ArrayProxy.create({ // trim the `value` when accessing objects content: [], From de9946a8ca1da5ee7e6dc190677712435c8b167e Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 14:10:48 -0500 Subject: [PATCH 06/16] glimmerize console service -- console tests pass --- ui/app/services/console.js | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/ui/app/services/console.js b/ui/app/services/console.js index 96ecc257d4ca..060ac6aab503 100644 --- a/ui/app/services/console.js +++ b/ui/app/services/console.js @@ -6,12 +6,12 @@ // Low level service that allows users to input paths to make requests to vault // this service provides the UI synecdote to the cli commands read, write, delete, and list import Service from '@ember/service'; - import { getOwner } from '@ember/application'; -import { computed } from '@ember/object'; import { shiftCommandIndex } from 'vault/lib/console-helpers'; import { encodePath } from 'vault/utils/path-encoding-helpers'; import { sanitizePath, ensureTrailingSlash } from 'core/utils/sanitize-path'; +import { tracked } from '@glimmer/tracking'; +import { addManyToArray } from 'vault/helpers/add-to-array'; const VERBS = { read: 'GET', @@ -20,51 +20,51 @@ const VERBS = { delete: 'DELETE', }; -export default Service.extend({ - isOpen: false, +export default class ConsoleService extends Service { + @tracked isOpen = false; + @tracked commandIndex = null; + @tracked log = []; - adapter() { - return getOwner(this).lookup('adapter:console'); - }, get commandHistory() { return this.log.filter((log) => log.type === 'command'); - }, - log: computed(function () { - return []; - }), - commandIndex: null, + } + + // Not a getter so it can be stubbed in tests + adapter() { + return getOwner(this).lookup('adapter:console'); + } shiftCommandIndex(keyCode, setCommandFn = () => {}) { const [newIndex, newCommand] = shiftCommandIndex(keyCode, this.commandHistory, this.commandIndex); if (newCommand !== undefined && newIndex !== undefined) { - this.set('commandIndex', newIndex); + this.commandIndex = newIndex; setCommandFn(newCommand); } - }, + } clearLog(clearAll = false) { - const log = this.log; let history; if (!clearAll) { history = this.commandHistory.slice(); history.setEach('hidden', true); } - log.clear(); + this.log = []; if (history) { - log.addObjects(history); + this.log = addManyToArray(this.log, history); } - }, + } logAndOutput(command, logContent) { - const log = this.log; + const log = this.log.slice(); if (command) { - log.pushObject({ type: 'command', content: command }); - this.set('commandIndex', null); + log.push({ type: 'command', content: command }); } if (logContent) { - log.pushObject(logContent); + log.push(logContent); } - }, + this.log = log; + this.commandIndex = null; + } ajax(operation, path, options = {}) { const verb = VERBS[operation]; @@ -75,7 +75,7 @@ export default Service.extend({ data, wrapTTL, }); - }, + } kvGet(path, data, flags = {}) { const { wrapTTL, metadata } = flags; @@ -84,21 +84,21 @@ export default Service.extend({ const [backend, secretPath] = path.split(/\/(.+)?/); const kvPath = `${backend}/${pathSegment}/${secretPath}`; return this.ajax('read', sanitizePath(kvPath), { wrapTTL }); - }, + } read(path, data, flags) { const wrapTTL = flags?.wrapTTL; return this.ajax('read', sanitizePath(path), { wrapTTL }); - }, + } write(path, data, flags) { const wrapTTL = flags?.wrapTTL; return this.ajax('write', sanitizePath(path), { data, wrapTTL }); - }, + } delete(path) { return this.ajax('delete', sanitizePath(path)); - }, + } list(path, data, flags) { const wrapTTL = flags?.wrapTTL; @@ -109,5 +109,5 @@ export default Service.extend({ }, wrapTTL, }); - }, -}); + } +} From a1ca39e398f743669707fc052a03e6f23444d5af Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Mar 2024 14:18:31 -0500 Subject: [PATCH 07/16] more replacements --- ui/app/adapters/ldap/role.js | 5 +++-- ui/app/components/clients/horizontal-bar-chart.js | 8 +++++--- ui/app/components/keymgmt/provider-edit.js | 3 ++- ui/app/initializers/deprecation-filter.js | 2 +- ui/app/models/kmip/role.js | 15 +++++++++------ ui/app/serializers/sync/destination.js | 2 +- ui/app/services/auth.js | 5 ++--- ui/app/services/wizard.js | 5 +++-- 8 files changed, 26 insertions(+), 19 deletions(-) diff --git a/ui/app/adapters/ldap/role.js b/ui/app/adapters/ldap/role.js index 436f275a5a54..f964eb392fc3 100644 --- a/ui/app/adapters/ldap/role.js +++ b/ui/app/adapters/ldap/role.js @@ -7,6 +7,7 @@ import NamedPathAdapter from 'vault/adapters/named-path'; import { encodePath } from 'vault/utils/path-encoding-helpers'; import { service } from '@ember/service'; import AdapterError from '@ember-data/adapter/error'; +import { addManyToArray } from 'vault/helpers/add-to-array'; export default class LdapRoleAdapter extends NamedPathAdapter { @service flashMessages; @@ -33,7 +34,7 @@ export default class LdapRoleAdapter extends NamedPathAdapter { async query(store, type, query, recordArray, options) { const { showPartialError } = options.adapterOptions || {}; const { backend } = query; - const roles = []; + let roles = []; const errors = []; for (const roleType of ['static', 'dynamic']) { @@ -42,7 +43,7 @@ export default class LdapRoleAdapter extends NamedPathAdapter { const models = await this.ajax(url, 'GET', { data: { list: true } }).then((resp) => { return resp.data.keys.map((name) => ({ id: name, name, backend, type: roleType })); }); - roles.addObjects(models); + roles = addManyToArray(roles, models); } catch (error) { if (error.httpStatus !== 404) { errors.push(error); diff --git a/ui/app/components/clients/horizontal-bar-chart.js b/ui/app/components/clients/horizontal-bar-chart.js index b21d651610ae..ad57f2ba6ac4 100644 --- a/ui/app/components/clients/horizontal-bar-chart.js +++ b/ui/app/components/clients/horizontal-bar-chart.js @@ -14,6 +14,7 @@ import { max, maxIndex } from 'd3-array'; import { GREY, BLUE_PALETTE } from 'vault/utils/chart-helpers'; import { tracked } from '@glimmer/tracking'; import { formatNumber } from 'core/helpers/format-number'; +import { addManyToArray } from 'vault/helpers/add-to-array'; /** * @module HorizontalBarChart @@ -174,9 +175,10 @@ export default class HorizontalBarChart extends Component { this.tooltipTarget = hoveredElement; this.isLabel = false; this.tooltipText = []; // clear stats - this.args.chartLegend.forEach(({ key, label }) => { - this.tooltipText.pushObject(`${formatNumber([data[key]])} ${label}`); - }); + this.tooltipText = addManyToArray( + this.tooltipText, + this.args.chartLegend.map(({ key, label }) => `${formatNumber([data[key]])} ${label}`) + ); select(hoveredElement).style('opacity', 1); }) diff --git a/ui/app/components/keymgmt/provider-edit.js b/ui/app/components/keymgmt/provider-edit.js index d8929e69b964..79756257aead 100644 --- a/ui/app/components/keymgmt/provider-edit.js +++ b/ui/app/components/keymgmt/provider-edit.js @@ -9,6 +9,7 @@ import { action } from '@ember/object'; import { tracked } from '@glimmer/tracking'; import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; /** * @module KeymgmtProviderEdit @@ -95,7 +96,7 @@ export default class KeymgmtProviderEdit extends Component { async onDeleteKey(model) { try { await model.destroyRecord(); - this.args.model.keys.removeObject(model); + this.args.model.keys = removeFromArray(this.args.model.keys, model); } catch (error) { this.flashMessages.danger(error.errors.join('. ')); } diff --git a/ui/app/initializers/deprecation-filter.js b/ui/app/initializers/deprecation-filter.js index ec2347caba3b..f9731115bd3f 100644 --- a/ui/app/initializers/deprecation-filter.js +++ b/ui/app/initializers/deprecation-filter.js @@ -11,7 +11,7 @@ export function initialize() { // filter deprecations that are scheduled to be removed in a specific version // when upgrading or addressing deprecation warnings be sure to update this or remove if not needed if (options?.until.includes('5.0')) { - return; + // return; } next(message, options); }); diff --git a/ui/app/models/kmip/role.js b/ui/app/models/kmip/role.js index c0379ab4d2a8..e094cb04d659 100644 --- a/ui/app/models/kmip/role.js +++ b/ui/app/models/kmip/role.js @@ -8,6 +8,7 @@ import { computed } from '@ember/object'; import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs'; import apiPath from 'vault/utils/api-path'; import lazyCapabilities from 'vault/macros/lazy-capabilities'; +import { removeManyFromArray } from 'vault/helpers/remove-from-array'; export const COMPUTEDS = { operationFields: computed('newFields', function () { @@ -15,7 +16,7 @@ export const COMPUTEDS = { }), operationFieldsWithoutSpecial: computed('operationFields', function () { - return this.operationFields.slice().removeObjects(['operationAll', 'operationNone']); + return removeManyFromArray(this.operationFields, ['operationAll', 'operationNone']); }), tlsFields: computed(function () { @@ -25,12 +26,12 @@ export const COMPUTEDS = { // For rendering on the create/edit pages defaultFields: computed('newFields', 'operationFields', 'tlsFields', function () { const excludeFields = ['role'].concat(this.operationFields, this.tlsFields); - return this.newFields.slice().removeObjects(excludeFields); + return removeManyFromArray(this.newFields, excludeFields); }), // For adapter/serializer nonOperationFields: computed('newFields', 'operationFields', function () { - return this.newFields.slice().removeObjects(this.operationFields); + return removeManyFromArray(this.newFields, this.operationFields); }), }; @@ -64,9 +65,11 @@ export default Model.extend(COMPUTEDS, { const attributes = ['operationAddAttribute', 'operationGetAttributes']; const server = ['operationDiscoverVersions']; - const others = this.operationFieldsWithoutSpecial - .slice() - .removeObjects(objects.concat(attributes, server)); + const others = removeManyFromArray(this.operationFieldsWithoutSpecial, [ + ...objects, + ...attributes, + ...server, + ]); const groups = [ { 'Managed Cryptographic Objects': objects }, { 'Object Attributes': attributes }, diff --git a/ui/app/serializers/sync/destination.js b/ui/app/serializers/sync/destination.js index 4d40e74ee323..7f06474ea531 100644 --- a/ui/app/serializers/sync/destination.js +++ b/ui/app/serializers/sync/destination.js @@ -50,7 +50,7 @@ export default class SyncDestinationSerializer extends ApplicationSerializer { const type = key.replace(/\/$/, ''); const id = `${type}/${name}`; // create object with destination's id and attributes, add to payload - transformedPayload.pushObject({ id, name, type }); + transformedPayload.push({ id, name, type }); }); } return transformedPayload; diff --git a/ui/app/services/auth.js b/ui/app/services/auth.js index 136f34dc384c..632f513f7573 100644 --- a/ui/app/services/auth.js +++ b/ui/app/services/auth.js @@ -17,6 +17,7 @@ import { resolve, reject } from 'rsvp'; import getStorage from 'vault/lib/token-storage'; import ENV from 'vault/config/environment'; import { allSupportedAuthBackends } from 'vault/helpers/supported-auth-backends'; +import { addToArray } from 'vault/helpers/add-to-array'; const TOKEN_SEPARATOR = '☃'; const TOKEN_PREFIX = 'vault-'; @@ -250,7 +251,6 @@ export default Service.extend({ persistAuthData() { const [firstArg, resp] = arguments; - const tokens = this.tokens; const currentNamespace = this.namespaceService.path || ''; // dropdown vs tab format const mountPath = firstArg?.data?.path || firstArg?.selectedAuth; @@ -303,8 +303,7 @@ export default Service.extend({ if (!data.displayName) { data.displayName = (this.getTokenData(tokenName) || {}).displayName; } - tokens.addObject(tokenName); - this.set('tokens', tokens); + this.set('tokens', addToArray(this.tokens, tokenName)); this.set('allowExpiration', false); this.setTokenData(tokenName, data); return resolve({ diff --git a/ui/app/services/wizard.js b/ui/app/services/wizard.js index b38b3af713ce..14311fa6b012 100644 --- a/ui/app/services/wizard.js +++ b/ui/app/services/wizard.js @@ -11,6 +11,7 @@ import { capitalize } from '@ember/string'; import getStorage from 'vault/lib/token-storage'; import { STORAGE_KEYS, DEFAULTS, MACHINES } from 'vault/helpers/wizard-constants'; +import { addToArray } from 'vault/helpers/add-to-array'; const { TUTORIAL_STATE, COMPONENT_STATE, @@ -101,7 +102,7 @@ export default Service.extend(DEFAULTS, { } else { if (this.featureMachineHistory) { if (!this.featureMachineHistory.includes(state)) { - const newHistory = this.featureMachineHistory.addObject(state); + const newHistory = addToArray(this.featureMachineHistory, state); this.set('featureMachineHistory', newHistory); } else { //we're repeating steps @@ -337,7 +338,7 @@ export default Service.extend(DEFAULTS, { completed.push(done); this.saveExtState(COMPLETED_FEATURES, completed); } else { - this.saveExtState(COMPLETED_FEATURES, this.getExtState(COMPLETED_FEATURES).addObject(done)); + this.saveExtState(COMPLETED_FEATURES, addToArray(this.getExtState(COMPLETED_FEATURES), done)); } this.saveExtState(FEATURE_LIST, features.length ? features : null); From a68d556a0913e81d569219be01a09a9eef4d6064 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 14 Mar 2024 10:50:05 -0500 Subject: [PATCH 08/16] update string-list, add comment to vertical-bar-chart --- ui/app/components/clients/vertical-bar-chart.js | 4 +++- ui/lib/core/addon/components/string-list.js | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ui/app/components/clients/vertical-bar-chart.js b/ui/app/components/clients/vertical-bar-chart.js index ac6af2dd8676..cdf82a54fb3a 100644 --- a/ui/app/components/clients/vertical-bar-chart.js +++ b/ui/app/components/clients/vertical-bar-chart.js @@ -155,7 +155,9 @@ export default class VerticalBarChart extends Component { this.tooltipStats = []; // clear stats this.args.chartLegend.forEach(({ key, label }) => { stackedNumbers.push(data[key]); - this.tooltipStats.pushObject(`${formatNumber([data[key]])} ${label}`); + // since we're relying on D3 not ember reactivity, + // pushing directly to this.tooltipStats updates the DOM + this.tooltipStats.push(`${formatNumber([data[key]])} ${label}`); }); this.tooltipTotal = `${formatNumber([calculateSum(stackedNumbers)])} ${ data.new_clients ? 'total' : 'new' diff --git a/ui/lib/core/addon/components/string-list.js b/ui/lib/core/addon/components/string-list.js index cba6e0c1338b..43f798d3504c 100644 --- a/ui/lib/core/addon/components/string-list.js +++ b/ui/lib/core/addon/components/string-list.js @@ -10,6 +10,8 @@ import { action } from '@ember/object'; import { set } from '@ember/object'; import { next } from '@ember/runloop'; import { tracked } from '@glimmer/tracking'; +import { addToArray } from 'vault/helpers/add-to-array'; +import { removeFromArray } from 'vault/helpers/remove-from-array'; /** * @module StringList @@ -91,9 +93,11 @@ export default class StringList extends Component { @action inputChanged(idx, event) { if (event.target.value.includes(',') && !this.indicesWithComma.includes(idx)) { - this.indicesWithComma.pushObject(idx); + this.indicesWithComma = addToArray(this.indicesWithComma, idx); + } + if (!event.target.value.includes(',')) { + this.indicesWithComma = removeFromArray(this.indicesWithComma, idx); } - if (!event.target.value.includes(',')) this.indicesWithComma.removeObject(idx); const inputObj = this.inputList.objectAt(idx); set(inputObj, 'value', event.target.value); From 1438dca51b0e59870dead088d740a9a561dc8a18 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 14 Mar 2024 11:09:10 -0500 Subject: [PATCH 09/16] Refactor CSP Event service - only used one place (auth-form) so simplified that usage - glimmerize and refactor so that the tests work --- ui/app/components/auth-form.js | 11 +++--- ui/app/services/csp-event.js | 35 ++++++++++--------- ui/app/templates/components/auth-form.hbs | 2 +- .../integration/components/auth-form-test.js | 25 +++++++------ 4 files changed, 40 insertions(+), 33 deletions(-) diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index 7e5248186add..cf7e93102c95 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -6,7 +6,7 @@ import Ember from 'ember'; import { next } from '@ember/runloop'; import { service } from '@ember/service'; -import { match, alias, or } from '@ember/object/computed'; +import { match, or } from '@ember/object/computed'; import { dasherize } from '@ember/string'; import Component from '@ember/component'; import { computed } from '@ember/object'; @@ -166,9 +166,12 @@ export default Component.extend(DEFAULTS, { return templateName; }), - hasCSPError: alias('csp.connectionViolations'), - - cspErrorText: `This is a standby Vault node but can't communicate with the active node via request forwarding. Sign in at the active node to use the Vault UI.`, + cspError: computed('csp.connectionViolations.length', function () { + if (this.csp.connectionViolations.length) { + return `This is a standby Vault node but can't communicate with the active node via request forwarding. Sign in at the active node to use the Vault UI.`; + } + return ''; + }), allSupportedMethods: computed('methodsToShow', 'hasMethodsWithPath', 'authMethods', function () { const hasMethodsWithPath = this.hasMethodsWithPath; diff --git a/ui/app/services/csp-event.js b/ui/app/services/csp-event.js index b53ccb968178..e889cb311254 100644 --- a/ui/app/services/csp-event.js +++ b/ui/app/services/csp-event.js @@ -3,34 +3,35 @@ * SPDX-License-Identifier: BUSL-1.1 */ -/*eslint-disable no-constant-condition*/ -import { computed } from '@ember/object'; - import Service from '@ember/service'; +import { tracked } from '@glimmer/tracking'; import { task, waitForEvent } from 'ember-concurrency'; +import { addToArray } from 'vault/helpers/add-to-array'; -export default Service.extend({ - events: computed(function () { - return []; - }), - connectionViolations: computed('events.@each.violatedDirective', function () { - return this.events.some((e) => e.violatedDirective.startsWith('connect-src')); - }), +export default class CspEventService extends Service { + @tracked connectionViolations = []; attach() { this.monitor.perform(); - }, + } remove() { this.monitor.cancelAll(); - }, + } + + handleEvent(event) { + if (event.violatedDirective.startsWith('connect-src')) { + this.connectionViolations = addToArray(this.connectionViolations, event); + } + } - monitor: task(function* () { - this.events.clear(); + @task + *monitor() { + this.connectionViolations = []; while (true) { const event = yield waitForEvent(window.document, 'securitypolicyviolation'); - this.events.addObject(event); + this.handleEvent(event); } - }), -}); + } +} diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs index d7c1c35e7ca8..66f135f2e940 100644 --- a/ui/app/templates/components/auth-form.hbs +++ b/ui/app/templates/components/auth-form.hbs @@ -50,7 +50,7 @@ {{/if}}
- + {{#if this.selectedAuthBackend.path}}

{{this.selectedAuthBackend.path}}

diff --git a/ui/tests/integration/components/auth-form-test.js b/ui/tests/integration/components/auth-form-test.js index 8e1dd06f3a31..dd1409ff2320 100644 --- a/ui/tests/integration/components/auth-form-test.js +++ b/ui/tests/integration/components/auth-form-test.js @@ -51,9 +51,9 @@ module('Integration | Component | auth form', function (hooks) { assert.expect(2); this.set('cluster', EmberObject.create({ standby: true })); this.set('selectedAuth', 'token'); - await render(hbs`{{auth-form cluster=this.cluster selectedAuth=this.selectedAuth}}`); + await render(hbs``); assert.false(component.errorMessagePresent, false); - this.owner.lookup('service:csp-event').events.addObject({ violatedDirective: 'connect-src' }); + this.owner.lookup('service:csp-event').handleEvent({ violatedDirective: 'connect-src' }); await settled(); assert.strictEqual(component.errorText, CSP_ERR_TEXT); }); @@ -75,7 +75,7 @@ module('Integration | Component | auth form', function (hooks) { this.set('cluster', EmberObject.create({})); this.set('selectedAuth', 'token'); - await render(hbs`{{auth-form cluster=this.cluster selectedAuth=this.selectedAuth}}`); + await render(hbs``); return component.login().then(() => { assert.strictEqual(component.errorText, 'Error Authentication failed: Not allowed'); server.shutdown(); @@ -86,17 +86,20 @@ module('Integration | Component | auth form', function (hooks) { assert.expect(1); const server = new Pretender(function () { this.get('/v1/auth/**', () => { - return [400, { 'Content-Type': 'application/json' }]; + return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors: ['API Error here'] })]; }); this.get('/v1/sys/internal/ui/mounts', this.passthrough); }); this.set('cluster', EmberObject.create({})); this.set('selectedAuth', 'token'); - await render(hbs`{{auth-form cluster=this.cluster selectedAuth=this.selectedAuth}}`); - // returns null because test does not return details of failed network request. On the app it will return the details of the error instead of null. + await render(hbs``); return component.login().then(() => { - assert.strictEqual(component.errorText, 'Error Authentication failed: null'); + assert.strictEqual( + component.errorText, + 'Error Authentication failed: API Error here', + 'shows the error from the API' + ); server.shutdown(); }); }); @@ -134,7 +137,7 @@ module('Integration | Component | auth form', function (hooks) { }); this.set('cluster', EmberObject.create({})); - await render(hbs`{{auth-form cluster=this.cluster }}`); + await render(hbs``); assert.strictEqual(component.tabs.length, 2, 'renders a tab for userpass and Other'); assert.strictEqual(component.tabs.objectAt(0).name, 'foo', 'uses the path in the label'); @@ -155,7 +158,7 @@ module('Integration | Component | auth form', function (hooks) { }); }); this.set('cluster', EmberObject.create({})); - await render(hbs`{{auth-form cluster=this.cluster }}`); + await render(hbs``); assert.strictEqual( component.descriptionText, @@ -183,7 +186,7 @@ module('Integration | Component | auth form', function (hooks) { this.set('cluster', EmberObject.create({})); this.set('selectedAuth', 'foo/'); - await render(hbs`{{auth-form cluster=this.cluster selectedAuth=this.selectedAuth}}`); + await render(hbs``); await component.login(); await settled(); @@ -267,7 +270,7 @@ module('Integration | Component | auth form', function (hooks) { }); this.set('wrappedToken', '54321'); - await render(hbs`{{auth-form cluster=this.cluster wrappedToken=this.wrappedToken}}`); + await render(hbs``); later(() => cancelTimers(), 50); await settled(); From 50932924f3f22c2a9df116579f1ef95149db66ff Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 14 Mar 2024 11:40:44 -0500 Subject: [PATCH 10/16] small updates --- ui/app/components/secret-create-or-update.js | 1 + ui/lib/core/addon/components/string-list.js | 3 +-- ui/lib/kubernetes/addon/components/page/roles.js | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ui/app/components/secret-create-or-update.js b/ui/app/components/secret-create-or-update.js index 298ab3ff1eaf..1544b5bb5ddf 100644 --- a/ui/app/components/secret-create-or-update.js +++ b/ui/app/components/secret-create-or-update.js @@ -195,6 +195,7 @@ export default class SecretCreateOrUpdate extends Component { if (isBlank(item.name)) { return; } + // secretData is a KVObject/ArrayProxy so removeObject is fine here data.removeObject(item); this.checkRows(); this.handleChange(); diff --git a/ui/lib/core/addon/components/string-list.js b/ui/lib/core/addon/components/string-list.js index 43f798d3504c..a95e3961e000 100644 --- a/ui/lib/core/addon/components/string-list.js +++ b/ui/lib/core/addon/components/string-list.js @@ -114,8 +114,7 @@ export default class StringList extends Component { @action removeInput(idx) { - const inputs = this.inputList; - inputs.removeObject(inputs.objectAt(idx)); + this.inputList.removeObject(this.inputs.objectAt(idx)); this.args.onChange(this.toVal()); } } diff --git a/ui/lib/kubernetes/addon/components/page/roles.js b/ui/lib/kubernetes/addon/components/page/roles.js index 78eeda01a479..e3ef2eca9033 100644 --- a/ui/lib/kubernetes/addon/components/page/roles.js +++ b/ui/lib/kubernetes/addon/components/page/roles.js @@ -32,7 +32,6 @@ export default class RolesPageComponent extends Component { try { const message = `Successfully deleted role ${model.name}`; await model.destroyRecord(); - this.args.roles.removeObject(model); this.flashMessages.success(message); } catch (error) { const message = errorMessage(error, 'Error deleting role. Please try again or contact support'); From de5984da96606c14a2a2c0089cf75ce0c0ff49ba Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 14 Mar 2024 12:13:39 -0500 Subject: [PATCH 11/16] more cleanup --- ui/app/components/clients/horizontal-bar-chart.js | 10 +++++----- ui/lib/core/addon/components/object-list-input.js | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ui/app/components/clients/horizontal-bar-chart.js b/ui/app/components/clients/horizontal-bar-chart.js index ad57f2ba6ac4..3fae0b9ca6be 100644 --- a/ui/app/components/clients/horizontal-bar-chart.js +++ b/ui/app/components/clients/horizontal-bar-chart.js @@ -14,7 +14,6 @@ import { max, maxIndex } from 'd3-array'; import { GREY, BLUE_PALETTE } from 'vault/utils/chart-helpers'; import { tracked } from '@glimmer/tracking'; import { formatNumber } from 'core/helpers/format-number'; -import { addManyToArray } from 'vault/helpers/add-to-array'; /** * @module HorizontalBarChart @@ -175,10 +174,11 @@ export default class HorizontalBarChart extends Component { this.tooltipTarget = hoveredElement; this.isLabel = false; this.tooltipText = []; // clear stats - this.tooltipText = addManyToArray( - this.tooltipText, - this.args.chartLegend.map(({ key, label }) => `${formatNumber([data[key]])} ${label}`) - ); + this.args.chartLegend.forEach(({ key, label }) => { + // since we're relying on D3 not ember reactivity, + // pushing directly to this.tooltipText updates the DOM + this.tooltipText.pushObject(`${formatNumber([data[key]])} ${label}`); + }); select(hoveredElement).style('opacity', 1); }) diff --git a/ui/lib/core/addon/components/object-list-input.js b/ui/lib/core/addon/components/object-list-input.js index e4655901ee01..53df710340e5 100644 --- a/ui/lib/core/addon/components/object-list-input.js +++ b/ui/lib/core/addon/components/object-list-input.js @@ -66,7 +66,7 @@ export default class ObjectListInput extends Component { @action handleInput(idx, { target }) { - const inputObj = this.inputList.objectAt(idx); + const inputObj = this.inputList[idx]; inputObj[target.name] = target.value; this.handleChange(); } @@ -80,7 +80,7 @@ export default class ObjectListInput extends Component { @action removeRow(idx) { - const row = this.inputList.objectAt(idx); + const row = this.inputList[idx]; this.inputList = removeFromArray(this.inputList, row); this.handleChange(); } From 24a80301b312f22785fdcba5d1342ac3d17f94de Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Tue, 19 Mar 2024 16:51:17 -0500 Subject: [PATCH 12/16] Fix tests --- ui/app/services/wizard.js | 6 +++--- ui/lib/core/addon/components/string-list.js | 3 ++- .../components/ldap/page/library/create-and-edit-test.js | 2 +- .../integration/components/path-filter-config-list-test.js | 1 + 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ui/app/services/wizard.js b/ui/app/services/wizard.js index 14311fa6b012..8f34e1aa3b0f 100644 --- a/ui/app/services/wizard.js +++ b/ui/app/services/wizard.js @@ -294,7 +294,7 @@ export default Service.extend(DEFAULTS, { return; } this.startFeature(); - const nextFeature = this.featureList.length > 1 ? capitalize(this.featureList.objectAt(1)) : 'Finish'; + const nextFeature = this.featureList.length > 1 ? capitalize(this.featureList[1]) : 'Finish'; this.set('nextFeature', nextFeature); let next; if (this.currentMachine === 'secrets' && this.featureState === 'display') { @@ -312,9 +312,9 @@ export default Service.extend(DEFAULTS, { }, startFeature() { - const FeatureMachineConfig = MACHINES[this.featureList.objectAt(0)]; + const FeatureMachineConfig = MACHINES[this.featureList[0]]; FeatureMachine = Machine(FeatureMachineConfig); - this.set('currentMachine', this.featureList.objectAt(0)); + this.set('currentMachine', this.featureList[0]); if (this.storageHasKey(FEATURE_STATE)) { this.saveState('featureState', this.getExtState(FEATURE_STATE)); } else { diff --git a/ui/lib/core/addon/components/string-list.js b/ui/lib/core/addon/components/string-list.js index a95e3961e000..cb9f610bc924 100644 --- a/ui/lib/core/addon/components/string-list.js +++ b/ui/lib/core/addon/components/string-list.js @@ -114,7 +114,8 @@ export default class StringList extends Component { @action removeInput(idx) { - this.inputList.removeObject(this.inputs.objectAt(idx)); + const itemToRemove = this.inputList.objectAt(idx); + this.inputList.removeObject(itemToRemove); this.args.onChange(this.toVal()); } } diff --git a/ui/tests/integration/components/ldap/page/library/create-and-edit-test.js b/ui/tests/integration/components/ldap/page/library/create-and-edit-test.js index 2466a2cbf2bb..28e1e5af0d50 100644 --- a/ui/tests/integration/components/ldap/page/library/create-and-edit-test.js +++ b/ui/tests/integration/components/ldap/page/library/create-and-edit-test.js @@ -147,7 +147,7 @@ module('Integration | Component | ldap | Page::Library::CreateAndEdit', function await this.renderComponent(); - await click('[data-test-string-list-button="delete"]'); + await click('[data-test-string-list-row="0"] [data-test-string-list-button="delete"]'); await click('[data-test-input="disable_check_in_enforcement"] input#Disabled'); await click('[data-test-save]'); diff --git a/ui/tests/integration/components/path-filter-config-list-test.js b/ui/tests/integration/components/path-filter-config-list-test.js index 5666fa36a194..4f17cf78fa85 100644 --- a/ui/tests/integration/components/path-filter-config-list-test.js +++ b/ui/tests/integration/components/path-filter-config-list-test.js @@ -159,6 +159,7 @@ module('Integration | Component | path filter config list', function (hooks) { await clickTrigger(); await searchSelect.deleteButtons.objectAt(1).click(); await clickTrigger(); + await typeInSearch('ns1'); assert.dom('.ember-power-select-group').hasText('Namespaces ns1', 'puts ns back within group'); await clickTrigger(); }); From 1a6659cdb10063f347c04929cfbd9cfb42878046 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Tue, 19 Mar 2024 16:51:38 -0500 Subject: [PATCH 13/16] Remove objectAt from console-helpers --- ui/app/lib/console-helpers.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/app/lib/console-helpers.ts b/ui/app/lib/console-helpers.ts index 1a76f4775d4b..5bee858d9587 100644 --- a/ui/app/lib/console-helpers.ts +++ b/ui/app/lib/console-helpers.ts @@ -241,7 +241,8 @@ export function shiftCommandIndex(keyCode: number, history: CommandLog[], index: } if (newInputValue !== '') { - newInputValue = history.objectAt(index)?.content; + const objAt = history[index]; + newInputValue = objAt?.content; } return [index, newInputValue]; From 16e6ce97270bf657d38672026519224a462f9cfb Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 21 Mar 2024 10:25:04 -0500 Subject: [PATCH 14/16] Address PR comments --- ui/app/components/clients/horizontal-bar-chart.js | 2 +- ui/app/components/keymgmt/provider-edit.js | 3 ++- ui/app/components/mfa/mfa-login-enforcement-form.js | 4 +--- ui/app/controllers/vault/cluster/access/mfa/methods/create.js | 1 + ui/app/initializers/deprecation-filter.js | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ui/app/components/clients/horizontal-bar-chart.js b/ui/app/components/clients/horizontal-bar-chart.js index 3fae0b9ca6be..63c6b74e34cb 100644 --- a/ui/app/components/clients/horizontal-bar-chart.js +++ b/ui/app/components/clients/horizontal-bar-chart.js @@ -177,7 +177,7 @@ export default class HorizontalBarChart extends Component { this.args.chartLegend.forEach(({ key, label }) => { // since we're relying on D3 not ember reactivity, // pushing directly to this.tooltipText updates the DOM - this.tooltipText.pushObject(`${formatNumber([data[key]])} ${label}`); + this.tooltipText.push(`${formatNumber([data[key]])} ${label}`); }); select(hoveredElement).style('opacity', 1); diff --git a/ui/app/components/keymgmt/provider-edit.js b/ui/app/components/keymgmt/provider-edit.js index 79756257aead..85ed0f7374f1 100644 --- a/ui/app/components/keymgmt/provider-edit.js +++ b/ui/app/components/keymgmt/provider-edit.js @@ -95,8 +95,9 @@ export default class KeymgmtProviderEdit extends Component { @action async onDeleteKey(model) { try { + const providerKeys = removeFromArray(this.args.model.keys, model); await model.destroyRecord(); - this.args.model.keys = removeFromArray(this.args.model.keys, model); + this.args.model.keys = providerKeys; } catch (error) { this.flashMessages.danger(error.errors.join('. ')); } diff --git a/ui/app/components/mfa/mfa-login-enforcement-form.js b/ui/app/components/mfa/mfa-login-enforcement-form.js index 6935d6503aee..d16603f823ed 100644 --- a/ui/app/components/mfa/mfa-login-enforcement-form.js +++ b/ui/app/components/mfa/mfa-login-enforcement-form.js @@ -65,7 +65,7 @@ export default class MfaLoginEnforcementForm extends Component { for (const { label, key } of this.targetTypes) { const targetArray = await this.args.model[key]; const targets = targetArray.map((value) => ({ label, key, value })); - this.targets = addManyToArray(this.targets, targets); // [...this.targets, ...targets]; + this.targets = addManyToArray(this.targets, targets); } } async resetTargetState() { @@ -168,8 +168,6 @@ export default class MfaLoginEnforcementForm extends Component { } @action removeTarget(target) { - // const targets = [...this.targets]; - // targets.splice(targets.indexOf(target), 1); this.targets = removeFromArray(this.targets, target); // recalculate value for appropriate model property this.updateModelForKey(target.key); diff --git a/ui/app/controllers/vault/cluster/access/mfa/methods/create.js b/ui/app/controllers/vault/cluster/access/mfa/methods/create.js index 063500cbd270..4f5eea0d093a 100644 --- a/ui/app/controllers/vault/cluster/access/mfa/methods/create.js +++ b/ui/app/controllers/vault/cluster/access/mfa/methods/create.js @@ -96,6 +96,7 @@ export default class MfaMethodCreateController extends Controller { // first save method yield this.method.save(); if (this.enforcement) { + // mfa_methods is type PromiseManyArray so slice in necessary to convert it to an Array this.enforcement.mfa_methods = addToArray(this.enforcement.mfa_methods.slice(), this.method); try { // now save enforcement and catch error separately diff --git a/ui/app/initializers/deprecation-filter.js b/ui/app/initializers/deprecation-filter.js index f9731115bd3f..ec2347caba3b 100644 --- a/ui/app/initializers/deprecation-filter.js +++ b/ui/app/initializers/deprecation-filter.js @@ -11,7 +11,7 @@ export function initialize() { // filter deprecations that are scheduled to be removed in a specific version // when upgrading or addressing deprecation warnings be sure to update this or remove if not needed if (options?.until.includes('5.0')) { - // return; + return; } next(message, options); }); From 4fadb8be6dc3a8a39dac868643d431333f266ad7 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 21 Mar 2024 10:49:30 -0500 Subject: [PATCH 15/16] move commandIndex clearing back --- ui/app/services/console.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/services/console.js b/ui/app/services/console.js index 060ac6aab503..664e49e329a2 100644 --- a/ui/app/services/console.js +++ b/ui/app/services/console.js @@ -58,12 +58,12 @@ export default class ConsoleService extends Service { const log = this.log.slice(); if (command) { log.push({ type: 'command', content: command }); + this.commandIndex = null; } if (logContent) { log.push(logContent); } this.log = log; - this.commandIndex = null; } ajax(operation, path, options = {}) { From d7a2b8347bc43853221febacef70c3bb77de94ba Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Mon, 25 Mar 2024 13:14:23 -0500 Subject: [PATCH 16/16] Remove extra model set --- ui/lib/core/addon/components/form-field.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/lib/core/addon/components/form-field.js b/ui/lib/core/addon/components/form-field.js index d6ff6c6ea6c2..b9e61ef02bc2 100644 --- a/ui/lib/core/addon/components/form-field.js +++ b/ui/lib/core/addon/components/form-field.js @@ -201,7 +201,6 @@ export default class FormFieldComponent extends Component { } else { updatedValue = removeFromArray(updatedValue, event.target.value); } - this.args.model[this.valuePath] = updatedValue; this.setAndBroadcast(updatedValue); } }