From 5ac3710efab44b8eaaebeef3a8994f0a909d7f27 Mon Sep 17 00:00:00 2001 From: Nicolas Chaulet Date: Wed, 27 May 2020 08:46:52 -0400 Subject: [PATCH] [Ingest Manager] Better validation of settings form (#67297) --- .../components/settings_flyout.tsx | 60 ++++++++++++++---- .../ingest_manager/hooks/use_input.ts | 63 ++++++++++++++++--- .../server/types/rest_spec/output.ts | 2 +- .../server/types/rest_spec/settings.ts | 2 +- 4 files changed, 106 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/ingest_manager/public/applications/ingest_manager/components/settings_flyout.tsx b/x-pack/plugins/ingest_manager/public/applications/ingest_manager/components/settings_flyout.tsx index cbd0b056eaaf18..47d723dd9a1ac6 100644 --- a/x-pack/plugins/ingest_manager/public/applications/ingest_manager/components/settings_flyout.tsx +++ b/x-pack/plugins/ingest_manager/public/applications/ingest_manager/components/settings_flyout.tsx @@ -27,37 +27,71 @@ import { EuiText } from '@elastic/eui'; import { useInput, useComboInput, useCore, useGetSettings, sendPutSettings } from '../hooks'; import { useGetOutputs, sendPutOutput } from '../hooks/use_request/outputs'; +const URL_REGEX = /^(https?):\/\/[^\s$.?#].[^\s]*$/gm; + interface Props { onClose: () => void; } -function useSettingsForm(outputId: string | undefined) { +function useSettingsForm(outputId: string | undefined, onSuccess: () => void) { + const [isLoading, setIsloading] = React.useState(false); const { notifications } = useCore(); - const kibanaUrlInput = useInput(); - const elasticsearchUrlInput = useComboInput([]); + const kibanaUrlInput = useInput('', (value) => { + if (!value.match(URL_REGEX)) { + return [ + i18n.translate('xpack.ingestManager.settings.kibanaUrlError', { + defaultMessage: 'Invalid URL', + }), + ]; + } + }); + const elasticsearchUrlInput = useComboInput([], (value) => { + if (value.some((v) => !v.match(URL_REGEX))) { + return [ + i18n.translate('xpack.ingestManager.settings.elasticHostError', { + defaultMessage: 'Invalid URL', + }), + ]; + } + }); return { + isLoading, onSubmit: async () => { + if (!kibanaUrlInput.validate() || !elasticsearchUrlInput.validate()) { + return; + } + try { + setIsloading(true); if (!outputId) { throw new Error('Unable to load outputs'); } - await sendPutOutput(outputId, { + const outputResponse = await sendPutOutput(outputId, { hosts: elasticsearchUrlInput.value, }); - await sendPutSettings({ + if (outputResponse.error) { + throw outputResponse.error; + } + const settingsResponse = await sendPutSettings({ kibana_url: kibanaUrlInput.value, }); + if (settingsResponse.error) { + throw settingsResponse.error; + } + notifications.toasts.addSuccess( + i18n.translate('xpack.ingestManager.settings.success.message', { + defaultMessage: 'Settings saved', + }) + ); + setIsloading(false); + onSuccess(); } catch (error) { + setIsloading(false); notifications.toasts.addError(error, { title: 'Error', }); } - notifications.toasts.addSuccess( - i18n.translate('xpack.ingestManager.settings.success.message', { - defaultMessage: 'Settings saved', - }) - ); }, inputs: { kibanaUrl: kibanaUrlInput, @@ -72,7 +106,7 @@ export const SettingFlyout: React.FunctionComponent = ({ onClose }) => { const settings = settingsRequest?.data?.item; const outputsRequest = useGetOutputs(); const output = outputsRequest.data?.items?.[0]; - const { inputs, onSubmit } = useSettingsForm(output?.id); + const { inputs, onSubmit, isLoading } = useSettingsForm(output?.id, onClose); useEffect(() => { if (output) { @@ -185,6 +219,7 @@ export const SettingFlyout: React.FunctionComponent = ({ onClose }) => { label={i18n.translate('xpack.ingestManager.settings.kibanaUrlLabel', { defaultMessage: 'Kibana URL', })} + {...inputs.kibanaUrl.formRowProps} > @@ -195,6 +230,7 @@ export const SettingFlyout: React.FunctionComponent = ({ onClose }) => { label={i18n.translate('xpack.ingestManager.settings.elasticsearchUrlLabel', { defaultMessage: 'Elasticsearch URL', })} + {...inputs.elasticsearchUrl.formRowProps} > @@ -226,7 +262,7 @@ export const SettingFlyout: React.FunctionComponent = ({ onClose }) => { - + string[] | undefined) { const [value, setValue] = React.useState(defaultValue); + const [errors, setErrors] = React.useState(); + + const onChange = (e: React.ChangeEvent) => { + const newValue = e.target.value; + setValue(newValue); + if (errors && validate && validate(newValue) === undefined) { + setErrors(undefined); + } + }; + + const isInvalid = errors !== undefined; return { value, + errors, props: { - onChange: (e: React.ChangeEvent) => { - setValue(e.target.value); - }, + onChange, value, + isInvalid, + }, + formRowProps: { + error: errors, + isInvalid, }, clear: () => { setValue(''); }, + validate: () => { + if (validate) { + const newErrors = validate(value); + setErrors(newErrors); + return newErrors === undefined; + } + + return true; + }, setValue, }; } -export function useComboInput(defaultValue = []) { +export function useComboInput( + defaultValue = [], + validate?: (value: string[]) => string[] | undefined +) { const [value, setValue] = React.useState(defaultValue); + const [errors, setErrors] = React.useState(); + + const isInvalid = errors !== undefined; return { props: { @@ -33,14 +63,33 @@ export function useComboInput(defaultValue = []) { onCreateOption: (newVal: any) => { setValue([...value, newVal]); }, - onChange: (newVals: any[]) => { - setValue(newVals.map((val) => val.label)); + onChange: (newSelectedOptions: any[]) => { + const newValues = newSelectedOptions.map((option) => option.label); + setValue(newValues); + if (errors && validate && validate(newValues) === undefined) { + setErrors(undefined); + } }, + isInvalid, + }, + formRowProps: { + error: errors, + isInvalid, }, value, clear: () => { setValue([]); }, setValue, + validate: () => { + if (validate) { + const newErrors = validate(value); + setErrors(newErrors); + + return newErrors === undefined; + } + + return true; + }, }; } diff --git a/x-pack/plugins/ingest_manager/server/types/rest_spec/output.ts b/x-pack/plugins/ingest_manager/server/types/rest_spec/output.ts index 79a7c444dacdb9..315923fd9f4017 100644 --- a/x-pack/plugins/ingest_manager/server/types/rest_spec/output.ts +++ b/x-pack/plugins/ingest_manager/server/types/rest_spec/output.ts @@ -18,7 +18,7 @@ export const PutOutputRequestSchema = { outputId: schema.string(), }), body: schema.object({ - hosts: schema.maybe(schema.arrayOf(schema.string())), + hosts: schema.maybe(schema.arrayOf(schema.uri({ scheme: ['http', 'https'] }))), ca_sha256: schema.maybe(schema.string()), }), }; diff --git a/x-pack/plugins/ingest_manager/server/types/rest_spec/settings.ts b/x-pack/plugins/ingest_manager/server/types/rest_spec/settings.ts index 8b7500e4a9bd95..f6e5fcbba7976b 100644 --- a/x-pack/plugins/ingest_manager/server/types/rest_spec/settings.ts +++ b/x-pack/plugins/ingest_manager/server/types/rest_spec/settings.ts @@ -11,7 +11,7 @@ export const PutSettingsRequestSchema = { body: schema.object({ agent_auto_upgrade: schema.maybe(schema.boolean()), package_auto_upgrade: schema.maybe(schema.boolean()), - kibana_url: schema.maybe(schema.string()), + kibana_url: schema.maybe(schema.uri({ scheme: ['http', 'https'] })), kibana_ca_sha256: schema.maybe(schema.string()), }), };