From a6ab9bed1d4ac8544136d2d295e0a0fad5a8c341 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Thu, 19 Jun 2025 10:03:09 -0600 Subject: [PATCH] [UI] Ember Data Migration - Form Type Updates (#31011) * updates type handling for forms * fixes validation in aws and ssh config forms * fixes issue with missing sync destination name --- .../components/secret-engine/configure-wif.ts | 15 +++++---- ui/app/forms/custom-message.ts | 4 +-- ui/app/forms/form.ts | 21 +++++++------ ui/app/forms/secrets/aws-config.ts | 9 +++--- ui/app/forms/secrets/azure-config.ts | 4 ++- ui/app/forms/secrets/engine.ts | 4 +-- ui/app/forms/secrets/gcp-config.ts | 4 ++- ui/app/forms/secrets/ssh-config.ts | 12 +++---- ui/app/forms/secrets/wif-config.ts | 2 +- ui/app/forms/sync/aws-sm.ts | 11 ++++--- ui/app/forms/sync/azure-kv.ts | 11 ++++--- ui/app/forms/sync/gcp-sm.ts | 11 ++++--- ui/app/forms/sync/gh.ts | 11 ++++--- ui/app/forms/sync/shared.ts | 11 ++++--- ui/app/forms/sync/vercel-project.ts | 11 ++++--- .../cluster/settings/mount-secret-backend.ts | 2 +- .../page/destinations/create-and-edit.ts | 31 +++++++++---------- ui/types/vault/secrets/engine.d.ts | 24 +++++++++++--- 18 files changed, 110 insertions(+), 88 deletions(-) diff --git a/ui/app/components/secret-engine/configure-wif.ts b/ui/app/components/secret-engine/configure-wif.ts index 42f2deb3d4..fc09c33fce 100644 --- a/ui/app/components/secret-engine/configure-wif.ts +++ b/ui/app/components/secret-engine/configure-wif.ts @@ -56,7 +56,6 @@ export default class ConfigureWif extends Component { @service declare readonly version: VersionService; @service declare readonly flashMessages: FlashMessageService; - @tracked accessType = 'account'; // for community users they will not be able to change this. for enterprise users, they will have the option to select "wif". @tracked errorMessage = ''; @tracked invalidFormAlert = ''; @tracked saveIssuerWarning = ''; @@ -68,7 +67,7 @@ export default class ConfigureWif extends Component { constructor(owner: Owner, args: Args) { super(owner, args); // the following checks are only relevant to existing enterprise configurations - const { isNew, isWifPluginConfigured, isAccountPluginConfigured } = this.args.configForm; + const { isNew, data, isWifPluginConfigured, isAccountPluginConfigured } = this.args.configForm; if (this.version.isEnterprise && !isNew) { assert( @@ -83,7 +82,7 @@ export default class ConfigureWif extends Component { } // cache the issuer to check if it has been changed later - this.originalIssuer = this.args.configForm.data['issuer'] as string | undefined; + this.originalIssuer = data.issuer; } @action continueSubmitForm() { @@ -105,7 +104,7 @@ export default class ConfigureWif extends Component { return; } - if (this.originalIssuer !== data['issuer']) { + if (this.originalIssuer !== data.issuer) { // if the issuer has changed show modal with warning that the config will change // if the modal is shown, the user has to click confirm to continue saving this.saveIssuerWarning = `You are updating the global issuer config. This will overwrite Vault's current issuer ${ @@ -178,19 +177,19 @@ export default class ConfigureWif extends Component { @action onChangeAccessType(accessType: 'account' | 'wif') { const { configForm, type } = this.args; - configForm['accessType'] = accessType; + configForm.accessType = accessType; if (accessType === 'account') { // reset all "wif" attributes that are mutually exclusive with "account" attributes // these attributes are the same for each engine - configForm['identityTokenAudience'] = configForm['identityTokenTtl'] = undefined; + configForm.data.identityTokenAudience = configForm.data.identityTokenTtl = undefined; } else if (accessType === 'wif') { // reset all "account" attributes that are mutually exclusive with "wif" attributes // these attributes are different for each engine if (type === 'azure') { - configForm['clientSecret'] = undefined; + (configForm as AzureConfigForm).data.clientSecret = undefined; } else if (type === 'aws') { - configForm['accessKey'] = undefined; + (configForm as AwsConfigForm).data.accessKey = undefined; } } } diff --git a/ui/app/forms/custom-message.ts b/ui/app/forms/custom-message.ts index b702852432..8d25f5c267 100644 --- a/ui/app/forms/custom-message.ts +++ b/ui/app/forms/custom-message.ts @@ -12,9 +12,7 @@ import { Validations } from 'vault/vault/app-types'; type CustomMessageFormData = Partial; -export default class CustomMessageForm extends Form { - declare data: CustomMessageFormData; - +export default class CustomMessageForm extends Form { formFields = [ new FormField('authenticated', undefined, { label: 'Where should we display this message?', diff --git a/ui/app/forms/form.ts b/ui/app/forms/form.ts index 0927eea3dd..712074a716 100644 --- a/ui/app/forms/form.ts +++ b/ui/app/forms/form.ts @@ -13,14 +13,13 @@ export type FormOptions = { isNew?: boolean; }; -export default class Form { - [key: string]: unknown; // Add an index signature to allow dynamic property assignment for set shim - declare data: Record; +export default class Form { + declare data: T; declare validations: Validations; declare isNew: boolean; - constructor(data = {}, options: FormOptions = {}, validations?: Validations) { - this.data = { ...data }; + constructor(data: Partial = {}, options: FormOptions = {}, validations?: Validations) { + this.data = { ...data } as T; this.isNew = options.isNew || false; // typically this would be defined on the subclass // if validations are conditional, it may be preferable to define them during instantiation @@ -31,16 +30,20 @@ export default class Form { // this allows for form field properties to be accessed directly on the class rather than form.data.someField const proxyTarget = (target: this, prop: string) => { // check if the property that is being accessed is a form field - const formFields = Array.isArray(target['formFields']) ? target['formFields'] : []; + const { formFields, formFieldGroups } = target as { + formFields?: FormField[]; + formFieldGroups?: FormFieldGroup[]; + }; + const fields = Array.isArray(formFields) ? formFields : []; // in the case of formFieldGroups we need extract the fields out into a flat array - const formGroupFields = Array.isArray(target['formFieldGroups']) - ? target['formFieldGroups'].reduce((arr: FormField[], group: FormFieldGroup) => { + const groupFields = Array.isArray(formFieldGroups) + ? formFieldGroups.reduce((arr: FormField[], group) => { const values = Object.values(group)[0] || []; return [...arr, ...values]; }, []) : []; // combine the formFields and formGroupFields into a single array - const allFields = [...formFields, ...formGroupFields]; + const allFields = [...fields, ...groupFields]; const formDataKeys = allFields.map((field) => field.name) || []; // if the property is a form field return the data object as the target, otherwise return the original target (this) // account for nested form data properties like 'config.maxLeaseTtl' when accessing the object like this.config diff --git a/ui/app/forms/secrets/aws-config.ts b/ui/app/forms/secrets/aws-config.ts index f66e0ed556..837afe8cb8 100644 --- a/ui/app/forms/secrets/aws-config.ts +++ b/ui/app/forms/secrets/aws-config.ts @@ -9,13 +9,14 @@ import FormFieldGroup from 'vault/utils/forms/field-group'; import { regions } from 'vault/helpers/aws-regions'; import type { Validations } from 'vault/app-types'; +import type { AwsConfigFormData } from 'vault/secrets/engine'; -export default class AwsConfigForm extends WifConfigForm { +export default class AwsConfigForm extends WifConfigForm { validations: Validations = { lease: [ { - validator(form: AwsConfigForm) { - const { lease, leaseMax } = form; + validator(data: AwsConfigForm['data']) { + const { lease, leaseMax } = data; return (lease && leaseMax) || (!lease && !leaseMax) ? true : false; }, message: 'Lease TTL and Max Lease TTL are both required if one of them is set.', @@ -24,7 +25,7 @@ export default class AwsConfigForm extends WifConfigForm { }; get isAccountPluginConfigured() { - return !!this.data['accessKey']; + return !!this.data.accessKey; } get isWifPluginConfigured() { diff --git a/ui/app/forms/secrets/azure-config.ts b/ui/app/forms/secrets/azure-config.ts index a320b5cdde..99dc5a0898 100644 --- a/ui/app/forms/secrets/azure-config.ts +++ b/ui/app/forms/secrets/azure-config.ts @@ -7,7 +7,9 @@ import WifConfigForm from './wif-config'; import FormField from 'vault/utils/forms/field'; import FormFieldGroup from 'vault/utils/forms/field-group'; -export default class AzureConfigForm extends WifConfigForm { +import type { AzureConfigFormData } from 'vault/vault/secrets/engine'; + +export default class AzureConfigForm extends WifConfigForm { // the "clientSecret" param is not checked because it's never returned by the API. // thus we can never say for sure if the account accessType has been configured so we always return false isAccountPluginConfigured = false; diff --git a/ui/app/forms/secrets/engine.ts b/ui/app/forms/secrets/engine.ts index 9c9cde48c9..9df40d6ab2 100644 --- a/ui/app/forms/secrets/engine.ts +++ b/ui/app/forms/secrets/engine.ts @@ -13,8 +13,7 @@ import { tracked } from '@glimmer/tracking'; import type { SecretsEngineFormData } from 'vault/secrets/engine'; import type { Validations } from 'vault/app-types'; -export default class SecretsEngineForm extends Form { - declare data: Partial; +export default class SecretsEngineForm extends Form { @tracked declare type: string; validations: Validations = { @@ -179,6 +178,7 @@ export default class SecretsEngineForm extends Form { ...this.data, config: { ...(config || {}), + forceNoCache: config?.forceNoCache ?? false, listingVisibility: config?.listingVisibility ? 'unauth' : 'hidden', }, }; diff --git a/ui/app/forms/secrets/gcp-config.ts b/ui/app/forms/secrets/gcp-config.ts index 54109a224b..c0638965fa 100644 --- a/ui/app/forms/secrets/gcp-config.ts +++ b/ui/app/forms/secrets/gcp-config.ts @@ -7,7 +7,9 @@ import WifConfigForm from './wif-config'; import FormField from 'vault/utils/forms/field'; import FormFieldGroup from 'vault/utils/forms/field-group'; -export default class AzureConfigForm extends WifConfigForm { +import type { GcpConfigFormData } from 'vault/secrets/engine'; + +export default class AzureConfigForm extends WifConfigForm { // the "credentials" param is not checked for "isAccountPluginConfigured" because it's never return by the API // additionally credentials can be set via GOOGLE_APPLICATION_CREDENTIALS env var so we cannot call it a required field in the ui. // thus we can never say for sure if the account accessType has been configured so we always return false diff --git a/ui/app/forms/secrets/ssh-config.ts b/ui/app/forms/secrets/ssh-config.ts index f68b1f2501..bf18549a6e 100644 --- a/ui/app/forms/secrets/ssh-config.ts +++ b/ui/app/forms/secrets/ssh-config.ts @@ -9,14 +9,12 @@ import FormField from 'vault/utils/forms/field'; import type { Validations } from 'vault/app-types'; import type { SshConfigureCaRequest } from '@hashicorp/vault-client-typescript'; -export default class SshConfigForm extends Form { - declare data: Partial; - +export default class SshConfigForm extends Form { validations: Validations = { generateSigningKey: [ { - validator(form: SshConfigForm) { - const { publicKey, privateKey, generateSigningKey } = form; + validator(data: SshConfigForm['data']) { + const { publicKey, privateKey, generateSigningKey } = data; // if generateSigningKey is false, both public and private keys are required if (!generateSigningKey && (!publicKey || !privateKey)) { return false; @@ -28,8 +26,8 @@ export default class SshConfigForm extends Form { ], publicKey: [ { - validator(form: SshConfigForm) { - const { publicKey, privateKey } = form; + validator(data: SshConfigForm['data']) { + const { publicKey, privateKey } = data; // regardless of generateSigningKey, if one key is set they both need to be set. return publicKey || privateKey ? !!(publicKey && privateKey) : true; }, diff --git a/ui/app/forms/secrets/wif-config.ts b/ui/app/forms/secrets/wif-config.ts index 0a62cb2703..bb75b0c7be 100644 --- a/ui/app/forms/secrets/wif-config.ts +++ b/ui/app/forms/secrets/wif-config.ts @@ -7,7 +7,7 @@ import Form from 'vault/forms/form'; import FormField from 'vault/utils/forms/field'; import { tracked } from '@glimmer/tracking'; -export default class WifConfigForm extends Form { +export default class WifConfigForm extends Form { // for community users they will not be able to change this. for enterprise users, they will have the option to select "wif". @tracked accessType: 'account' | 'wif' = 'account'; diff --git a/ui/app/forms/sync/aws-sm.ts b/ui/app/forms/sync/aws-sm.ts index 9f8d4d5322..87ed24c08b 100644 --- a/ui/app/forms/sync/aws-sm.ts +++ b/ui/app/forms/sync/aws-sm.ts @@ -10,11 +10,11 @@ import { commonFields, getPayload } from './shared'; import type { SystemWriteSyncDestinationsAwsSmNameRequest } from '@hashicorp/vault-client-typescript'; -type AwsSmFormData = Partial; - -export default class AwsSmForm extends Form { - declare data: AwsSmFormData; +type AwsSmFormData = SystemWriteSyncDestinationsAwsSmNameRequest & { + name: string; +}; +export default class AwsSmForm extends Form { formFieldGroups = [ new FormFieldGroup('default', [ commonFields.name, @@ -59,6 +59,7 @@ export default class AwsSmForm extends Form { toJSON() { const formState = super.toJSON(); - return { ...formState, data: getPayload('aws-sm', this.data, this.isNew) }; + const data = getPayload('aws-sm', this.data, this.isNew); + return { ...formState, data }; } } diff --git a/ui/app/forms/sync/azure-kv.ts b/ui/app/forms/sync/azure-kv.ts index 60c0e484fb..385f0c46f5 100644 --- a/ui/app/forms/sync/azure-kv.ts +++ b/ui/app/forms/sync/azure-kv.ts @@ -10,11 +10,11 @@ import { commonFields, getPayload } from './shared'; import type { SystemWriteSyncDestinationsAzureKvNameRequest } from '@hashicorp/vault-client-typescript'; -type AzureKvFormData = Partial; - -export default class AzureKvForm extends Form { - declare data: AzureKvFormData; +type AzureKvFormData = SystemWriteSyncDestinationsAzureKvNameRequest & { + name: string; +}; +export default class AzureKvForm extends Form { formFieldGroups = [ new FormFieldGroup('default', [ commonFields.name, @@ -57,6 +57,7 @@ export default class AzureKvForm extends Form { toJSON() { const formState = super.toJSON(); - return { ...formState, data: getPayload('azure-kv', this.data, this.isNew) }; + const data = getPayload('azure-kv', this.data, this.isNew); + return { ...formState, data }; } } diff --git a/ui/app/forms/sync/gcp-sm.ts b/ui/app/forms/sync/gcp-sm.ts index a318fc59f8..a274f2a5f1 100644 --- a/ui/app/forms/sync/gcp-sm.ts +++ b/ui/app/forms/sync/gcp-sm.ts @@ -10,11 +10,11 @@ import { commonFields, getPayload } from './shared'; import type { SystemWriteSyncDestinationsGcpSmNameRequest } from '@hashicorp/vault-client-typescript'; -type GcpSmFormData = Partial; - -export default class GcpSmForm extends Form { - declare data: GcpSmFormData; +type GcpSmFormData = SystemWriteSyncDestinationsGcpSmNameRequest & { + name: string; +}; +export default class GcpSmForm extends Form { formFieldGroups = [ new FormFieldGroup('default', [ commonFields.name, @@ -42,6 +42,7 @@ export default class GcpSmForm extends Form { toJSON() { const formState = super.toJSON(); - return { ...formState, data: getPayload('gcp-sm', this.data, this.isNew) }; + const data = getPayload('gcp-sm', this.data, this.isNew); + return { ...formState, data }; } } diff --git a/ui/app/forms/sync/gh.ts b/ui/app/forms/sync/gh.ts index b3ec8eec29..9c4d4b7b18 100644 --- a/ui/app/forms/sync/gh.ts +++ b/ui/app/forms/sync/gh.ts @@ -10,11 +10,11 @@ import { commonFields, getPayload } from './shared'; import type { SystemWriteSyncDestinationsGhNameRequest } from '@hashicorp/vault-client-typescript'; -type GhFormData = Partial; - -export default class GcpSmForm extends Form { - declare data: GhFormData; +type GhFormData = SystemWriteSyncDestinationsGhNameRequest & { + name: string; +}; +export default class GcpSmForm extends Form { formFieldGroups = [ new FormFieldGroup('default', [ commonFields.name, @@ -42,6 +42,7 @@ export default class GcpSmForm extends Form { toJSON() { const formState = super.toJSON(); - return { ...formState, data: getPayload('gh', this.data, this.isNew) }; + const data = getPayload('gh', this.data, this.isNew); + return { ...formState, data }; } } diff --git a/ui/app/forms/sync/shared.ts b/ui/app/forms/sync/shared.ts index 650349d75d..e28dc36022 100644 --- a/ui/app/forms/sync/shared.ts +++ b/ui/app/forms/sync/shared.ts @@ -45,26 +45,27 @@ export const commonFields = { }), }; -export function getPayload(type: DestinationType, data: Record, isNew: boolean) { +export function getPayload(type: DestinationType, data: T, isNew: boolean) { const { maskedParams, readonlyParams } = findDestination(type); - const payload = { ...data }; + const payload: T = { ...data }; // the server returns ****** for sensitive fields // these are represented as maskedParams in the sync-destinations helper // when editing, remove these fields from the payload if they haven't been changed if (!isNew) { maskedParams.forEach((maskedParam) => { - const value = (payload[maskedParam] as string) || ''; + const key = maskedParam as keyof T; + const value = (payload[key] as string) || ''; // if the value is asterisks, remove it from the payload if (value.match(/^\*+$/)) { - delete payload[maskedParam]; + delete payload[key]; } }); // to preserve the original Ember Data payload structure, remove fields that are not editable // since editing is disabled in the form the value will not change so this is mostly to satisfy existing test conditions readonlyParams.forEach((readonlyParam) => { - delete payload[readonlyParam]; + delete payload[readonlyParam as keyof T]; }); } diff --git a/ui/app/forms/sync/vercel-project.ts b/ui/app/forms/sync/vercel-project.ts index 4f6f780c4d..f3222b48da 100644 --- a/ui/app/forms/sync/vercel-project.ts +++ b/ui/app/forms/sync/vercel-project.ts @@ -10,11 +10,11 @@ import { commonFields, getPayload } from './shared'; import type { SystemWriteSyncDestinationsVercelProjectNameRequest } from '@hashicorp/vault-client-typescript'; -type VercelProjectFormData = Partial; - -export default class GcpSmForm extends Form { - declare data: VercelProjectFormData; +type VercelProjectFormData = SystemWriteSyncDestinationsVercelProjectNameRequest & { + name: string; +}; +export default class VercelProjectForm extends Form { formFieldGroups = [ new FormFieldGroup('default', [ commonFields.name, @@ -45,6 +45,7 @@ export default class GcpSmForm extends Form { toJSON() { const formState = super.toJSON(); - return { ...formState, data: getPayload('vercel-project', this.data, this.isNew) }; + const data = getPayload('vercel-project', this.data, this.isNew); + return { ...formState, data }; } } diff --git a/ui/app/routes/vault/cluster/settings/mount-secret-backend.ts b/ui/app/routes/vault/cluster/settings/mount-secret-backend.ts index 997fe02235..13f491c43f 100644 --- a/ui/app/routes/vault/cluster/settings/mount-secret-backend.ts +++ b/ui/app/routes/vault/cluster/settings/mount-secret-backend.ts @@ -21,7 +21,7 @@ export default class VaultClusterSettingsMountSecretBackendRoute extends Route { kvConfig: { maxVersions: 0, casRequired: false, - deleteVersionAfter: 0, + deleteVersionAfter: undefined, }, options: { version: 2 }, }; 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 6d9404e9ce..76384980b1 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 @@ -39,15 +39,17 @@ export default class DestinationsCreateForm extends Component { super(owner, args); // cache initial custom tags value to compare against updates // tags that are removed when editing need to be added to the payload - if (args.form['customTags']) { - this.initialCustomTags = { ...args.form['customTags'] }; + // cast type here since not all types have customTags + const { customTags } = args.form.data as unknown as Record; + if (customTags) { + this.initialCustomTags = { ...customTags }; } } get header() { const { type, form } = this.args; const { name: typeDisplayName } = findDestination(type); - const { name } = form; + const { name } = form.data; return form.isNew ? { @@ -87,18 +89,18 @@ export default class DestinationsCreateForm extends Component { } } - diffCustomTags(customTags?: Record) { + diffCustomTags(payload: Record) { // if tags were removed we need to add them to the payload - const { form } = this.args; - if (!form.isNew && customTags && this.initialCustomTags) { + const { isNew } = this.args.form; + const { customTags } = payload; + if (!isNew && customTags && this.initialCustomTags) { // compare the new and old keys of customTags object to determine which need to be removed const oldKeys = Object.keys(this.initialCustomTags).filter((k) => !Object.keys(customTags).includes(k)); // add tagsToRemove to the payload if there is a diff if (oldKeys.length > 0) { - return { tagsToRemove: oldKeys }; + payload['tagsToRemove'] = oldKeys; } } - return {}; } save = task( @@ -109,21 +111,18 @@ export default class DestinationsCreateForm extends Component { this.modelValidations = null; const { form, type } = this.args; + const { name } = form.data; const { isValid, state, invalidFormMessage, data } = form.toJSON(); - const name = form['name'] as string; this.modelValidations = isValid ? null : state; this.invalidFormMessage = isValid ? '' : invalidFormMessage; if (isValid) { try { - const { tagsToRemove } = this.diffCustomTags(data['customTags'] as Record); - if (tagsToRemove) { - data['tagsToRemove'] = tagsToRemove; - } - - const method = apiMethodResolver(form.isNew ? 'write' : 'patch', this.args.type); - await this.api.sys[method](name, data); + const payload = data as unknown as Record; + this.diffCustomTags(payload); + const method = apiMethodResolver(form.isNew ? 'write' : 'patch', type); + await this.api.sys[method](name, payload); this.router.transitionTo('vault.cluster.sync.secrets.destinations.destination.details', type, name); } catch (error) { diff --git a/ui/types/vault/secrets/engine.d.ts b/ui/types/vault/secrets/engine.d.ts index f56b955756..b1f6d28051 100644 --- a/ui/types/vault/secrets/engine.d.ts +++ b/ui/types/vault/secrets/engine.d.ts @@ -3,13 +3,19 @@ * SPDX-License-Identifier: BUSL-1.1 */ -import type { MountsEnableSecretsEngineRequest } from '@hashicorp/vault-client-typescript'; +import type { + MountsEnableSecretsEngineRequest, + AwsConfigureRootIamCredentialsRequest, + AwsConfigureLeaseRequest, + AzureConfigureRequest, + GoogleCloudConfigureRequest, +} from '@hashicorp/vault-client-typescript'; export type EngineConfig = { - forceNoCache: boolean; - listingVisibility: string; - defaultLeaseTtl: number; - maxLeaseTtl: number; + forceNoCache?: boolean; + listingVisibility?: string | boolean; + defaultLeaseTtl?: number; + maxLeaseTtl?: number; allowedManagedKeys?: string[]; auditNonHmacRequestKeys?: string[]; auditNonHmacResponseKeys?: string[]; @@ -91,3 +97,11 @@ export type SecretsEngineFormData = MountsEnableSecretsEngineRequest & { deleteVersionAfter?: string; }; }; + +type Issuer = { + issuer?: string; +}; + +export type AwsConfigFormData = AwsConfigureRootIamCredentialsRequest & AwsConfigureLeaseRequest & Issuer; +export type AzureConfigFormData = AzureConfigureRequest & Issuer; +export type GcpConfigFormData = GoogleCloudConfigureRequest & Issuer;