Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Improve server side input validation #128

Merged
merged 4 commits into from
Oct 20, 2020

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented Oct 19, 2020

Issue #, if available:
#90

Description of changes:

  • input validation
    • url: pass relative url instead of full url, compose url on server side wiht localhost:5601

    • text fields: validate non-empty.

      • report Name: validate allowed characters using regex
      • email recipients: validate email address format
    • timestamp fields: validate positive number

    • cron-expression: validate unix cron-expression (5 digits)

    • time duration: validate ISO format for time duration. e.g. PT10M (10 mintues)

  • Decouple create visual report functions to another file, prepare for project layout re-arrangement. Will implement in future PRs.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -63,7 +63,7 @@ export const addReportsTableContent = (data) => {
//TODO: wrong name
timeCreated: report.time_created,
state: report.state,
url: report.query_url,
url: `${location.host}${report.query_url}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does location.host add?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the query_url is now relative, need to add the host as well for the link rendered in report table -> report source field to be accessible.

@@ -27,32 +29,56 @@ import {
} from '../routes/utils/constants';

export const dataReportSchema = schema.object({
base_url: schema.uri(),
base_url: schema.string({
validate(value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the validation fails will this still return an error to the client-side and trigger the error toasts in the UI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will, it will also include the error message

Copy link
Contributor

@davidcui1225 davidcui1225 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM- just have some minor questions

@zhongnansu zhongnansu merged commit 3b53da3 into opendistro-for-elasticsearch:dev Oct 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants