From 33b7c75d6517a984c867366314482b9c4bf4f036 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Thu, 25 May 2023 16:56:45 -0500 Subject: [PATCH] Backport UI: Fix id fields not allowing update (#19117) (#20794) --- ui/app/adapters/pki/pki-role.js | 4 ++-- ui/app/adapters/role-aws.js | 15 ++++++++++----- ui/app/adapters/role-ssh.js | 11 ++++++++--- ui/app/adapters/transform.js | 10 +++++++--- ui/app/adapters/transform/base.js | 17 ++++++++++++----- ui/app/components/role-aws-edit.js | 3 ++- ui/app/components/role-edit.js | 3 ++- ui/app/components/transformation-edit.js | 2 +- ui/app/models/pki/pki-role.js | 2 +- ui/app/models/role-aws.js | 3 +-- ui/app/models/role-ssh.js | 2 +- ui/app/models/transform.js | 2 -- ui/app/models/transform/alphabet.js | 1 - ui/app/models/transform/role.js | 1 - ui/app/models/transform/template.js | 1 - ui/app/models/transit-key.js | 1 - ui/app/serializers/role-aws.js | 5 ++++- ui/app/serializers/role.js | 7 +++++-- ui/app/serializers/transform.js | 3 ++- ui/app/serializers/transform/alphabet.js | 6 ++---- ui/app/serializers/transform/role.js | 2 ++ ui/app/serializers/transform/template.js | 8 +++++--- ui/lib/core/addon/components/form-field.js | 5 +++++ 23 files changed, 72 insertions(+), 42 deletions(-) diff --git a/ui/app/adapters/pki/pki-role.js b/ui/app/adapters/pki/pki-role.js index 80e2c06fed34..199ca0804ff3 100644 --- a/ui/app/adapters/pki/pki-role.js +++ b/ui/app/adapters/pki/pki-role.js @@ -6,10 +6,10 @@ export default ApplicationAdapter.extend({ namespace: 'v1', createOrUpdate(store, type, snapshot, requestType) { + const { name, backend } = snapshot.record; const serializer = store.serializerFor(type.modelName); const data = serializer.serialize(snapshot, requestType); - const { id } = snapshot; - const url = this.urlForRole(snapshot.record.get('backend'), id); + const url = this.urlForRole(backend, name); return this.ajax(url, 'POST', { data }); }, diff --git a/ui/app/adapters/role-aws.js b/ui/app/adapters/role-aws.js index 44a5fc774ad3..de64be4e1120 100644 --- a/ui/app/adapters/role-aws.js +++ b/ui/app/adapters/role-aws.js @@ -1,4 +1,3 @@ -import { assign } from '@ember/polyfills'; import ApplicationAdapter from './application'; import { encodePath } from 'vault/utils/path-encoding-helpers'; @@ -6,12 +5,18 @@ export default ApplicationAdapter.extend({ namespace: 'v1', createOrUpdate(store, type, snapshot, requestType) { + const { name, backend } = snapshot.record; const serializer = store.serializerFor(type.modelName); const data = serializer.serialize(snapshot, requestType); - const { id } = snapshot; - const url = this.urlForRole(snapshot.record.get('backend'), id); + const url = this.urlForRole(backend, name); - return this.ajax(url, 'POST', { data }); + return this.ajax(url, 'POST', { data }).then((resp) => { + // Ember data doesn't like 204 responses except for DELETE method + const response = resp || { data: {} }; + response.data.name = name; + response.data.backend = name; + return response; + }); }, createRecord() { @@ -56,7 +61,7 @@ export default ApplicationAdapter.extend({ backend, }; - return assign({}, resp, data); + return { ...resp, ...data }; }); }, diff --git a/ui/app/adapters/role-ssh.js b/ui/app/adapters/role-ssh.js index 3d62279ddb51..a7e86ffdebe6 100644 --- a/ui/app/adapters/role-ssh.js +++ b/ui/app/adapters/role-ssh.js @@ -7,12 +7,17 @@ export default ApplicationAdapter.extend({ namespace: 'v1', createOrUpdate(store, type, snapshot, requestType) { + const { name, backend } = snapshot.record; const serializer = store.serializerFor(type.modelName); const data = serializer.serialize(snapshot, requestType); - const { id } = snapshot; - const url = this.urlForRole(snapshot.record.get('backend'), id); + const url = this.urlForRole(backend, name); - return this.ajax(url, 'POST', { data }); + return this.ajax(url, 'POST', { data }).then((resp) => { + // Ember data doesn't like 204 responses except for DELETE method + const response = resp || { data: {} }; + response.data.name = name; + return response; + }); }, createRecord() { diff --git a/ui/app/adapters/transform.js b/ui/app/adapters/transform.js index 5c82cdf72f28..31df25afa4df 100644 --- a/ui/app/adapters/transform.js +++ b/ui/app/adapters/transform.js @@ -7,12 +7,16 @@ export default ApplicationAdapter.extend({ namespace: 'v1', createOrUpdate(store, type, snapshot) { + const { backend, name } = snapshot.record; const serializer = store.serializerFor(type.modelName); const data = serializer.serialize(snapshot); - const { id } = snapshot; - const url = this.urlForTransformations(snapshot.record.get('backend'), id); + const url = this.urlForTransformations(backend, name); - return this.ajax(url, 'POST', { data }); + return this.ajax(url, 'POST', { data }).then((resp) => { + const response = resp || {}; + response.id = name; + return response; + }); }, createRecord() { diff --git a/ui/app/adapters/transform/base.js b/ui/app/adapters/transform/base.js index dfdaeb486ffd..5e9ea52afb05 100644 --- a/ui/app/adapters/transform/base.js +++ b/ui/app/adapters/transform/base.js @@ -9,12 +9,16 @@ export default ApplicationAdapter.extend({ }, createOrUpdate(store, type, snapshot) { + const { backend, name } = snapshot.record; const serializer = store.serializerFor(type.modelName); const data = serializer.serialize(snapshot); - const { id } = snapshot; - const url = this.url(snapshot.record.get('backend'), type.modelName, id); - - return this.ajax(url, 'POST', { data }); + const url = this.url(backend, type.modelName, name); + return this.ajax(url, 'POST', { data }).then((resp) => { + // Ember data doesn't like 204 responses except for DELETE method + const response = resp || { data: {} }; + response.data.name = name; + return response; + }); }, createRecord() { @@ -42,9 +46,12 @@ export default ApplicationAdapter.extend({ fetchByQuery(query) { const { backend, modelName, id } = query; return this.ajax(this.url(backend, modelName, id), 'GET').then((resp) => { + // The API response doesn't explicitly include the name/id, so add it here return { ...resp, backend, + id, + name: id, }; }); }, @@ -55,9 +62,9 @@ export default ApplicationAdapter.extend({ queryRecord(store, type, query) { return this.ajax(this.url(query.backend, type.modelName, query.id), 'GET').then((result) => { - // CBS TODO: Add name to response and unmap name <> id on models return { id: query.id, + name: query.id, ...result, }; }); diff --git a/ui/app/components/role-aws-edit.js b/ui/app/components/role-aws-edit.js index 4d0ad5cf4a9b..1dfc62a0e0cd 100644 --- a/ui/app/components/role-aws-edit.js +++ b/ui/app/components/role-aws-edit.js @@ -8,7 +8,8 @@ export default RoleEdit.extend({ createOrUpdate(type, event) { event.preventDefault(); - const modelId = this.model.id; + // all of the attributes with fieldValue:'id' are called `name` + const modelId = this.model.id || this.model.name; // prevent from submitting if there's no key // maybe do something fancier later if (type === 'create' && isBlank(modelId)) { diff --git a/ui/app/components/role-edit.js b/ui/app/components/role-edit.js index aead8aaeabec..c24fb3ee0166 100644 --- a/ui/app/components/role-edit.js +++ b/ui/app/components/role-edit.js @@ -64,7 +64,8 @@ export default Component.extend(FocusOnInsertMixin, { createOrUpdate(type, event) { event.preventDefault(); - const modelId = this.model.id; + // all of the attributes with fieldValue:'id' are called `name` + const modelId = this.model.id || this.model.name; // prevent from submitting if there's no key // maybe do something fancier later if (type === 'create' && isBlank(modelId)) { diff --git a/ui/app/components/transformation-edit.js b/ui/app/components/transformation-edit.js index 430208ebc94f..0e736bc264bf 100644 --- a/ui/app/components/transformation-edit.js +++ b/ui/app/components/transformation-edit.js @@ -97,7 +97,7 @@ export default TransformBase.extend({ event.preventDefault(); this.applyChanges('save', () => { - const transformationId = this.model.id; + const transformationId = this.model.id || this.model.name; const newModelRoles = this.model.allowed_roles || []; const initialRoles = this.initialRoles || []; diff --git a/ui/app/models/pki/pki-role.js b/ui/app/models/pki/pki-role.js index e3025b76f01f..265037a30dce 100644 --- a/ui/app/models/pki/pki-role.js +++ b/ui/app/models/pki/pki-role.js @@ -11,7 +11,7 @@ export default Model.extend({ }), name: attr('string', { label: 'Role name', - fieldValue: 'id', + fieldValue: 'name', readOnly: true, }), useOpenAPI: true, diff --git a/ui/app/models/role-aws.js b/ui/app/models/role-aws.js index 9ddef405f2bf..8d2496739bf8 100644 --- a/ui/app/models/role-aws.js +++ b/ui/app/models/role-aws.js @@ -24,10 +24,8 @@ export default Model.extend({ }), name: attr('string', { label: 'Role name', - fieldValue: 'id', readOnly: true, }), - useOpenAPI: false, // credentialTypes are for backwards compatibility. // we use this to populate "credentialType" in // the serializer. if there is more than one, the @@ -51,6 +49,7 @@ export default Model.extend({ editType: 'json', helpText: 'A policy is an object in AWS that, when associated with an identity or resource, defines their permissions.', + defaultValue: '{\n}', }), fields: computed('credentialType', function () { const credentialType = this.credentialType; diff --git a/ui/app/models/role-ssh.js b/ui/app/models/role-ssh.js index 148d92f08c4d..cff2c4c67673 100644 --- a/ui/app/models/role-ssh.js +++ b/ui/app/models/role-ssh.js @@ -53,7 +53,7 @@ export default Model.extend({ }), name: attr('string', { label: 'Role Name', - fieldValue: 'id', + fieldValue: 'name', readOnly: true, }), keyType: attr('string', { diff --git a/ui/app/models/transform.js b/ui/app/models/transform.js index e4b307a6c79f..893c5a09a063 100644 --- a/ui/app/models/transform.js +++ b/ui/app/models/transform.js @@ -34,11 +34,9 @@ const TWEAK_SOURCE = [ ]; const ModelExport = Model.extend({ - useOpenAPI: false, name: attr('string', { // CBS TODO: make this required for making a transformation label: 'Name', - fieldValue: 'id', readOnly: true, subText: 'The name for your transformation. This cannot be edited later.', }), diff --git a/ui/app/models/transform/alphabet.js b/ui/app/models/transform/alphabet.js index f881c69dba8c..2d9530b6238b 100644 --- a/ui/app/models/transform/alphabet.js +++ b/ui/app/models/transform/alphabet.js @@ -12,7 +12,6 @@ const M = Model.extend({ }), name: attr('string', { - fieldValue: 'id', readOnly: true, subText: 'The alphabet name. Keep in mind that spaces are not allowed and this cannot be edited later.', }), diff --git a/ui/app/models/transform/role.js b/ui/app/models/transform/role.js index b59f9b39e3fa..f750586fef6c 100644 --- a/ui/app/models/transform/role.js +++ b/ui/app/models/transform/role.js @@ -16,7 +16,6 @@ const ModelExport = Model.extend({ name: attr('string', { // TODO: make this required for making a transformation label: 'Name', - fieldValue: 'id', readOnly: true, subText: 'The name for your role. This cannot be edited later.', }), diff --git a/ui/app/models/transform/template.js b/ui/app/models/transform/template.js index 39a3c4e68439..f88438e305bf 100644 --- a/ui/app/models/transform/template.js +++ b/ui/app/models/transform/template.js @@ -12,7 +12,6 @@ const M = Model.extend({ }), name: attr('string', { - fieldValue: 'id', readOnly: true, subText: 'Templates allow Vault to determine what and how to capture the value to be transformed. This cannot be edited later.', diff --git a/ui/app/models/transit-key.js b/ui/app/models/transit-key.js index e87b64e8c175..d50f56451457 100644 --- a/ui/app/models/transit-key.js +++ b/ui/app/models/transit-key.js @@ -53,7 +53,6 @@ export default Model.extend({ }), name: attr('string', { label: 'Name', - fieldValue: 'id', readOnly: true, }), autoRotatePeriod: attr({ diff --git a/ui/app/serializers/role-aws.js b/ui/app/serializers/role-aws.js index 1a769aad6e18..9cb84f622109 100644 --- a/ui/app/serializers/role-aws.js +++ b/ui/app/serializers/role-aws.js @@ -1,9 +1,12 @@ import ApplicationSerializer from './application'; + export default ApplicationSerializer.extend({ + primaryKey: 'name', + extractLazyPaginatedData(payload) { return payload.data.keys.map((key) => { const model = { - id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; diff --git a/ui/app/serializers/role.js b/ui/app/serializers/role.js index 603f97f49ae6..19d1dda55481 100644 --- a/ui/app/serializers/role.js +++ b/ui/app/serializers/role.js @@ -1,6 +1,9 @@ import ApplicationSerializer from './application'; export default ApplicationSerializer.extend({ + primaryKey: 'name', + + // Used for both pki-role (soon to be deprecated) and role-ssh extractLazyPaginatedData(payload) { if (payload.zero_address_roles) { payload.zero_address_roles.forEach((role) => { @@ -11,7 +14,7 @@ export default ApplicationSerializer.extend({ if (!payload.data.key_info) { return payload.data.keys.map((key) => { const model = { - id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; @@ -22,7 +25,7 @@ export default ApplicationSerializer.extend({ const ret = payload.data.keys.map((key) => { const model = { - id: key, + name: key, key_type: payload.data.key_info[key].key_type, zero_address: payload.data.key_info[key].zero_address, }; diff --git a/ui/app/serializers/transform.js b/ui/app/serializers/transform.js index 3a1b2e173a3f..2b4a1bc237c7 100644 --- a/ui/app/serializers/transform.js +++ b/ui/app/serializers/transform.js @@ -2,7 +2,7 @@ import ApplicationSerializer from './application'; export default ApplicationSerializer.extend({ normalizeResponse(store, primaryModelClass, payload, id, requestType) { - if (payload.data.masking_character) { + if (payload.data?.masking_character) { payload.data.masking_character = String.fromCharCode(payload.data.masking_character); } return this._super(store, primaryModelClass, payload, id, requestType); @@ -21,6 +21,7 @@ export default ApplicationSerializer.extend({ return payload.data.keys.map((key) => { const model = { id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; diff --git a/ui/app/serializers/transform/alphabet.js b/ui/app/serializers/transform/alphabet.js index e44accb06e64..a29a3a5c1dfc 100644 --- a/ui/app/serializers/transform/alphabet.js +++ b/ui/app/serializers/transform/alphabet.js @@ -1,15 +1,13 @@ import ApplicationSerializer from '../application'; export default ApplicationSerializer.extend({ - normalizeResponse(store, primaryModelClass, payload, id, requestType) { - payload.data.name = payload.id; - return this._super(store, primaryModelClass, payload, id, requestType); - }, + primaryKey: 'name', extractLazyPaginatedData(payload) { return payload.data.keys.map((key) => { const model = { id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; diff --git a/ui/app/serializers/transform/role.js b/ui/app/serializers/transform/role.js index 2153ca8361bc..48e199f8dca6 100644 --- a/ui/app/serializers/transform/role.js +++ b/ui/app/serializers/transform/role.js @@ -1,10 +1,12 @@ import ApplicationSerializer from '../application'; export default ApplicationSerializer.extend({ + primaryKey: 'name', extractLazyPaginatedData(payload) { return payload.data.keys.map((key) => { const model = { id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; diff --git a/ui/app/serializers/transform/template.js b/ui/app/serializers/transform/template.js index 31bc57186367..c94c1157f4d0 100644 --- a/ui/app/serializers/transform/template.js +++ b/ui/app/serializers/transform/template.js @@ -1,13 +1,14 @@ import ApplicationSerializer from '../application'; export default ApplicationSerializer.extend({ + primaryKey: 'name', + normalizeResponse(store, primaryModelClass, payload, id, requestType) { - payload.data.name = payload.id; - if (payload.data.alphabet) { + if (payload.data?.alphabet) { payload.data.alphabet = [payload.data.alphabet]; } // strip out P character from any named capture groups - if (payload.data.pattern) { + if (payload.data?.pattern) { this._formatNamedCaptureGroups(payload.data, '?P', '?'); } return this._super(store, primaryModelClass, payload, id, requestType); @@ -43,6 +44,7 @@ export default ApplicationSerializer.extend({ return payload.data.keys.map((key) => { const model = { id: key, + name: key, }; if (payload.backend) { model.backend = payload.backend; diff --git a/ui/lib/core/addon/components/form-field.js b/ui/lib/core/addon/components/form-field.js index 9c6d5c160044..dfad85ada089 100644 --- a/ui/lib/core/addon/components/form-field.js +++ b/ui/lib/core/addon/components/form-field.js @@ -4,6 +4,7 @@ import { action } from '@ember/object'; import { capitalize } from 'vault/helpers/capitalize'; import { humanize } from 'vault/helpers/humanize'; import { dasherize } from 'vault/helpers/dasherize'; +import { assert } from '@ember/debug'; /** * @module FormField * `FormField` components are field elements associated with a particular model. @@ -61,6 +62,10 @@ export default class FormFieldComponent extends Component { super(...arguments); const { attr, model } = this.args; const valuePath = attr.options?.fieldValue || attr.name; + assert( + 'Form is attempting to modify an ID. Ember-data does not allow this.', + valuePath.toLowerCase() !== 'id' + ); const modelValue = model[valuePath]; this.showInput = !!modelValue; }