From 48ba2b5cbbc83b03e4103aa65b1ec07a2b648d28 Mon Sep 17 00:00:00 2001
From: David Cui <53581635+davidcui-amzn@users.noreply.github.com>
Date: Wed, 25 Nov 2020 16:36:15 -0800
Subject: [PATCH] Lock Edit report source and Input Validation (#225)
---
.../create/create_report_definition.tsx | 90 +++-------------
.../edit/edit_report_definition.tsx | 78 +++++++++-----
.../report_settings.test.tsx.snap | 9 ++
.../report_settings/report_settings.tsx | 1 +
.../report_settings/time_range.tsx | 2 +-
.../report_definitions/utils/utils.tsx | 102 ++++++++++++++++++
6 files changed, 178 insertions(+), 104 deletions(-)
create mode 100644 kibana-reports/public/components/report_definitions/utils/utils.tsx
diff --git a/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx b/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx
index a07100eb..7f406370 100644
--- a/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx
+++ b/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx
@@ -28,13 +28,12 @@ import { ReportSettings } from '../report_settings';
import { ReportDelivery } from '../delivery';
import { ReportTrigger } from '../report_trigger';
import { generateReportFromDefinitionId } from '../../main/main_utils';
-import { isValidCron } from 'cron-validator';
import { converter } from '../utils';
-import moment from 'moment';
import {
permissionsMissingToast,
permissionsMissingActions,
} from '../../utils/utils';
+import { definitionInputValidation } from '../utils/utils';
interface reportParamsType {
report_name: string;
@@ -209,80 +208,6 @@ export function CreateReport(props) {
timeTo: new Date(),
};
- const definitionInputValidation = async (metadata, error) => {
- // check report name
- // allow a-z, A-Z, 0-9, (), [], ',' - and _ and spaces
- let regexp = /^[\w\-\s\(\)\[\]\,\_\-+]+$/;
- if (metadata.report_params.report_name.search(regexp) === -1) {
- setShowSettingsReportNameError(true);
- if (metadata.report_params.report_name === '') {
- setSettingsReportNameErrorMessage('Name must not be empty.');
- } else {
- setSettingsReportNameErrorMessage('Invalid characters in report name.');
- }
- error = true;
- }
-
- // if recurring by interval and input is not a number
- if (
- metadata.trigger.trigger_type === 'Schedule' &&
- metadata.trigger.trigger_params.schedule_type === 'Recurring'
- ) {
- let interval = parseInt(
- metadata.trigger.trigger_params.schedule.interval.period
- );
- if (isNaN(interval)) {
- setShowTriggerIntervalNaNError(true);
- error = true;
- }
- }
-
- // if time range is invalid
- const nowDate = new Date(moment.now());
- if (timeRange.timeFrom > timeRange.timeTo || timeRange.timeTo > nowDate) {
- setShowTimeRangeError(true);
- error = true;
- }
-
- // if cron based and cron input is invalid
- if (
- metadata.trigger.trigger_type === 'Schedule' &&
- metadata.trigger.trigger_params.schedule_type === 'Cron based'
- ) {
- if (
- !isValidCron(metadata.trigger.trigger_params.schedule.cron.expression)
- ) {
- setShowCronError(true);
- error = true;
- }
- }
- // if email delivery
- if (metadata.delivery.delivery_type === 'Channel') {
- // no recipients are listed
- if (metadata.delivery.delivery_params.recipients.length === 0) {
- setShowEmailRecipientsError(true);
- setEmailRecipientsErrorMessage(
- 'Email recipients list cannot be empty.'
- );
- error = true;
- }
- // recipients have invalid email addresses: regexp checks format xxxxx@yyyy.zzz
- let emailRegExp = /\S+@\S+\.\S+/;
- let index;
- let recipients = metadata.delivery.delivery_params.recipients;
- for (index = 0; index < recipients.length; ++index) {
- if (recipients[0].search(emailRegExp) === -1) {
- setShowEmailRecipientsError(true);
- setEmailRecipientsErrorMessage(
- 'Invalid email addresses in recipients list.'
- );
- error = true;
- }
- }
- }
- return error;
- };
-
const createNewReportDefinition = async (
metadata: reportDefinitionParams,
timeRange: timeRangeParams
@@ -298,7 +223,18 @@ export function CreateReport(props) {
}
let error = false;
- await definitionInputValidation(metadata, error).then((response) => {
+ await definitionInputValidation(
+ metadata,
+ error,
+ setShowSettingsReportNameError,
+ setSettingsReportNameErrorMessage,
+ setShowTriggerIntervalNaNError,
+ timeRange,
+ setShowTimeRangeError,
+ setShowCronError,
+ setShowEmailRecipientsError,
+ setEmailRecipientsErrorMessage
+ ).then((response) => {
error = response;
});
if (error) {
diff --git a/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx b/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx
index cc0a211b..c609ce9d 100644
--- a/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx
+++ b/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx
@@ -34,12 +34,36 @@ import {
permissionsMissingToast,
permissionsMissingActions,
} from '../../utils/utils';
+import { definitionInputValidation } from '../utils/utils';
export function EditReportDefinition(props) {
const [toasts, setToasts] = useState([]);
const [comingFromError, setComingFromError] = useState(false);
const [preErrorData, setPreErrorData] = useState({});
+ const [
+ showSettingsReportNameError,
+ setShowSettingsReportNameError,
+ ] = useState(false);
+ const [
+ settingsReportNameErrorMessage,
+ setSettingsReportNameErrorMessage,
+ ] = useState('');
+ const [
+ showTriggerIntervalNaNError,
+ setShowTriggerIntervalNaNError,
+ ] = useState(false);
+ const [showCronError, setShowCronError] = useState(false);
+ const [
+ showEmailRecipientsError,
+ setShowEmailRecipientsError
+ ] = useState(false);
+ const [
+ emailRecipientsErrorMessage,
+ setEmailRecipientsErrorMessage,
+ ] = useState('');
+ const [showTimeRangeError, setShowTimeRangeError] = useState(false);
+
const addPermissionsMissingViewEditPageToastHandler = (errorType: string) => {
let toast = {};
if (errorType === 'permissions') {
@@ -151,7 +175,6 @@ export function EditReportDefinition(props) {
const callUpdateAPI = async (metadata) => {
const { httpClient } = props;
-
httpClient
.put(`../api/reporting/reportDefinitions/${reportDefinitionId}`, {
body: JSON.stringify(metadata),
@@ -175,7 +198,6 @@ export function EditReportDefinition(props) {
};
const editReportDefinition = async (metadata) => {
- const { httpClient } = props;
if ('header' in metadata.report_params.core_params) {
metadata.report_params.core_params.header = converter.makeHtml(
metadata.report_params.core_params.header
@@ -186,30 +208,27 @@ export function EditReportDefinition(props) {
metadata.report_params.core_params.footer
);
}
- /*
- we check if this editing updates the trigger type from Schedule to On demand.
- If so, need to first delete the reportDefinition along with the scheduled job first, by calling the delete
- report definition API
- */
- const {
- trigger: { trigger_type: triggerType },
- } = reportDefinition;
- if (
- triggerType !== 'On demand' &&
- metadata.trigger.trigger_type === 'On demand'
- ) {
- httpClient
- .delete(`../api/reporting/reportDefinitions/${reportDefinitionId}`)
- .then(async () => {
- await callUpdateAPI(metadata);
- })
- .catch((error) => {
- console.log(
- 'error when deleting old scheduled report definition:',
- error
- );
- handleErrorDeletingReportDefinitionToast();
- });
+
+ // client-side input validation
+ let error = false;
+ await definitionInputValidation(
+ metadata,
+ error,
+ setShowSettingsReportNameError,
+ setSettingsReportNameErrorMessage,
+ setShowTriggerIntervalNaNError,
+ timeRange,
+ setShowTimeRangeError,
+ setShowCronError,
+ setShowEmailRecipientsError,
+ setEmailRecipientsErrorMessage
+ ).then((response) => {
+ error = response;
+ });
+ if (error) {
+ handleInputValidationErrorToast();
+ setPreErrorData(metadata);
+ setComingFromError(true);
} else {
await callUpdateAPI(metadata);
}
@@ -273,6 +292,9 @@ export function EditReportDefinition(props) {
reportDefinitionRequest={editReportDefinitionRequest}
httpClientProps={props['httpClient']}
timeRange={timeRange}
+ showSettingsReportNameError={showSettingsReportNameError}
+ settingsReportNameErrorMessage={settingsReportNameErrorMessage}
+ showTimeRangeError={showTimeRangeError}
/>
diff --git a/kibana-reports/public/components/report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap b/kibana-reports/public/components/report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap
index dbf342a3..41cfd11c 100644
--- a/kibana-reports/public/components/report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap
+++ b/kibana-reports/public/components/report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap
@@ -2748,6 +2748,7 @@ exports[` panel render edit, dashboard source 1`] = `
panel render edit, dashboard source 1`] = `
>
panel render edit, dashboard source 1`] = `
>
panel render edit, saved search source 1`] = `
panel render edit, saved search source 1`] = `
>
panel render edit, saved search source 1`] = `
>
panel render edit, visualization source 1`] = `
panel render edit, visualization source 1`] = `
>
panel render edit, visualization source 1`] = `
>
diff --git a/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx b/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx
index 53604bf3..badf2424 100644
--- a/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx
+++ b/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx
@@ -71,7 +71,7 @@ export function TimeRangeSelect(props) {
if (
!timeRangeMoment ||
!timeRangeMoment.isValid() ||
- timeRangeMoment > moment()
+ timeRangeMoment > moment.now()
) {
handleInvalidTimeRangeToast();
}
diff --git a/kibana-reports/public/components/report_definitions/utils/utils.tsx b/kibana-reports/public/components/report_definitions/utils/utils.tsx
new file mode 100644
index 00000000..ee3c5839
--- /dev/null
+++ b/kibana-reports/public/components/report_definitions/utils/utils.tsx
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+import { isValidCron } from "cron-validator";
+import moment from "moment";
+
+ export const definitionInputValidation = async (
+ metadata,
+ error,
+ setShowSettingsReportNameError,
+ setSettingsReportNameErrorMessage,
+ setShowTriggerIntervalNaNError,
+ timeRange,
+ setShowTimeRangeError,
+ setShowCronError,
+ setShowEmailRecipientsError,
+ setEmailRecipientsErrorMessage
+ ) => {
+ // check report name
+ // allow a-z, A-Z, 0-9, (), [], ',' - and _ and spaces
+ let regexp = /^[\w\-\s\(\)\[\]\,\_\-+]+$/;
+ if (metadata.report_params.report_name.search(regexp) === -1) {
+ setShowSettingsReportNameError(true);
+ if (metadata.report_params.report_name === '') {
+ setSettingsReportNameErrorMessage('Name must not be empty.');
+ } else {
+ setSettingsReportNameErrorMessage('Invalid characters in report name.');
+ }
+ error = true;
+ }
+
+ // if recurring by interval and input is not a number
+ if (
+ metadata.trigger.trigger_type === 'Schedule' &&
+ metadata.trigger.trigger_params.schedule_type === 'Recurring'
+ ) {
+ let interval = parseInt(
+ metadata.trigger.trigger_params.schedule.interval.period
+ );
+ if (isNaN(interval)) {
+ setShowTriggerIntervalNaNError(true);
+ error = true;
+ }
+ }
+
+ // if time range is invalid
+ const nowDate = new Date(moment.now());
+ if (timeRange.timeFrom > timeRange.timeTo || timeRange.timeTo > nowDate) {
+ setShowTimeRangeError(true);
+ error = true;
+ }
+
+ // if cron based and cron input is invalid
+ if (
+ metadata.trigger.trigger_type === 'Schedule' &&
+ metadata.trigger.trigger_params.schedule_type === 'Cron based'
+ ) {
+ if (
+ !isValidCron(metadata.trigger.trigger_params.schedule.cron.expression)
+ ) {
+ setShowCronError(true);
+ error = true;
+ }
+ }
+ // if email delivery
+ if (metadata.delivery.delivery_type === 'Channel') {
+ // no recipients are listed
+ if (metadata.delivery.delivery_params.recipients.length === 0) {
+ setShowEmailRecipientsError(true);
+ setEmailRecipientsErrorMessage(
+ 'Email recipients list cannot be empty.'
+ );
+ error = true;
+ }
+ // recipients have invalid email addresses: regexp checks format xxxxx@yyyy.zzz
+ let emailRegExp = /\S+@\S+\.\S+/;
+ let index;
+ let recipients = metadata.delivery.delivery_params.recipients;
+ for (index = 0; index < recipients.length; ++index) {
+ if (recipients[0].search(emailRegExp) === -1) {
+ setShowEmailRecipientsError(true);
+ setEmailRecipientsErrorMessage(
+ 'Invalid email addresses in recipients list.'
+ );
+ error = true;
+ }
+ }
+ }
+ return error;
+ };
\ No newline at end of file