From aa275751b2db11e6581bb312d45ecb87f47712cb Mon Sep 17 00:00:00 2001 From: Mike Stone Date: Fri, 21 Oct 2016 17:58:13 -0400 Subject: [PATCH] Client side query validation (#335) * validate query text * Update structure of submitted SaveQueryForm data * form calls correct prop function when invalid query text * Lowercase directory names --- .../Queries/NewQuery/NewQuery.tests.jsx | 53 ----------- .../queries/SaveQueryForm/SaveQueryForm.jsx | 2 +- .../SaveQueryForm/SaveQueryForm.tests.jsx | 32 +++---- .../NewQuery/NewQuery.jsx | 24 +++-- .../queries/NewQuery/NewQuery.tests.jsx | 92 +++++++++++++++++++ .../NewQuery/SaveQuerySection/index.jsx | 0 .../NewQuery/SaveQuerySection/styles.js | 0 .../NewQuery/ThemeDropdown/index.jsx | 0 .../NewQuery/ThemeDropdown/styles.js | 0 .../components/queries/NewQuery/helpers.js | 29 ++++++ .../queries/NewQuery/helpers.tests.js | 39 ++++++++ .../{Queries => queries}/NewQuery/index.js | 0 .../{Queries => queries}/NewQuery/mode.js | 0 .../{Queries => queries}/NewQuery/styles.js | 0 .../{Queries => queries}/NewQuery/theme.js | 0 .../QueryPageWrapper/QueryPageWrapper.jsx | 0 .../QueryPageWrapper/index.js | 0 .../NewQueryPage/NewQueryPage.jsx | 27 ++++-- .../NewQueryPage/NewQueryPage.tests.js | 0 .../NewQueryPage/index.js | 0 frontend/router/index.jsx | 4 +- package.json | 1 + 22 files changed, 214 insertions(+), 89 deletions(-) delete mode 100644 frontend/components/Queries/NewQuery/NewQuery.tests.jsx rename frontend/components/{Queries => queries}/NewQuery/NewQuery.jsx (89%) create mode 100644 frontend/components/queries/NewQuery/NewQuery.tests.jsx rename frontend/components/{Queries => queries}/NewQuery/SaveQuerySection/index.jsx (100%) rename frontend/components/{Queries => queries}/NewQuery/SaveQuerySection/styles.js (100%) rename frontend/components/{Queries => queries}/NewQuery/ThemeDropdown/index.jsx (100%) rename frontend/components/{Queries => queries}/NewQuery/ThemeDropdown/styles.js (100%) create mode 100644 frontend/components/queries/NewQuery/helpers.js create mode 100644 frontend/components/queries/NewQuery/helpers.tests.js rename frontend/components/{Queries => queries}/NewQuery/index.js (100%) rename frontend/components/{Queries => queries}/NewQuery/mode.js (100%) rename frontend/components/{Queries => queries}/NewQuery/styles.js (100%) rename frontend/components/{Queries => queries}/NewQuery/theme.js (100%) rename frontend/components/{Queries => queries}/QueryPageWrapper/QueryPageWrapper.jsx (100%) rename frontend/components/{Queries => queries}/QueryPageWrapper/index.js (100%) rename frontend/pages/{Queries => queries}/NewQueryPage/NewQueryPage.jsx (78%) rename frontend/pages/{Queries => queries}/NewQueryPage/NewQueryPage.tests.js (100%) rename frontend/pages/{Queries => queries}/NewQueryPage/index.js (100%) diff --git a/frontend/components/Queries/NewQuery/NewQuery.tests.jsx b/frontend/components/Queries/NewQuery/NewQuery.tests.jsx deleted file mode 100644 index 5e88c829aa..0000000000 --- a/frontend/components/Queries/NewQuery/NewQuery.tests.jsx +++ /dev/null @@ -1,53 +0,0 @@ -import React from 'react'; -import expect, { restoreSpies } from 'expect'; -import { mount } from 'enzyme'; -import { noop } from 'lodash'; - -import { createAceSpy } from '../../../test/helpers'; -import NewQuery from './index'; - -describe('NewQuery - component', () => { - beforeEach(() => { - createAceSpy(); - }); - afterEach(restoreSpies); - - it('renders the SaveQuerySection', () => { - const component = mount( - - ); - - expect(component.find('SaveQuerySection').length).toEqual(1); - }); - - it('renders the ThemeDropdown', () => { - const component = mount( - - ); - - expect(component.find('ThemeDropdown').length).toEqual(1); - }); - - it('renders the SaveQueryForm', () => { - const component = mount( - - ); - - component.find('Slider').simulate('click'); - - expect(component.find('SaveQueryForm').length).toEqual(1); - }); -}); - diff --git a/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.jsx b/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.jsx index 760bbb3dce..efda1102aa 100644 --- a/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.jsx +++ b/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.jsx @@ -81,7 +81,7 @@ class SaveQueryForm extends Component { const { validate } = this; if (validate(runType)) { - return onSubmit({ formData, runType }); + return onSubmit({ ...formData, runType }); } return false; diff --git a/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.tests.jsx b/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.tests.jsx index d135b04f21..890dc4833d 100644 --- a/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.tests.jsx +++ b/frontend/components/forms/queries/SaveQueryForm/SaveQueryForm.tests.jsx @@ -55,16 +55,14 @@ describe('SaveQueryForm - component', () => { form.simulate('submit'); expect(onSubmit).toHaveBeenCalledWith({ + description: null, + duration: 'short', + hosts: 'all', + hostsPercentage: null, + name: queryName, + platforms: 'all', runType: 'RUN_AND_SAVE', - formData: { - description: null, - duration: 'short', - hosts: 'all', - hostsPercentage: null, - name: queryName, - platforms: 'all', - scanInterval: 0, - }, + scanInterval: 0, }); }); @@ -77,16 +75,14 @@ describe('SaveQueryForm - component', () => { form.simulate('submit'); expect(onSubmit).toHaveBeenCalledWith({ + description: null, + duration: 'short', + hosts: 'all', + hostsPercentage: null, + name: null, + platforms: 'all', runType: 'RUN', - formData: { - description: null, - duration: 'short', - hosts: 'all', - hostsPercentage: null, - name: null, - platforms: 'all', - scanInterval: 0, - }, + scanInterval: 0, }); }); }); diff --git a/frontend/components/Queries/NewQuery/NewQuery.jsx b/frontend/components/queries/NewQuery/NewQuery.jsx similarity index 89% rename from frontend/components/Queries/NewQuery/NewQuery.jsx rename to frontend/components/queries/NewQuery/NewQuery.jsx index 7558dcb4f2..203ece6290 100644 --- a/frontend/components/Queries/NewQuery/NewQuery.jsx +++ b/frontend/components/queries/NewQuery/NewQuery.jsx @@ -8,12 +8,15 @@ import 'react-select/dist/react-select.css'; import './mode'; import './theme'; import componentStyles from './styles'; +import debounce from '../../../utilities/debounce'; import SaveQueryForm from '../../forms/queries/SaveQueryForm'; import SaveQuerySection from './SaveQuerySection'; import ThemeDropdown from './ThemeDropdown'; +import { validateQuery } from './helpers'; class NewQuery extends Component { static propTypes = { + onInvalidQuerySubmit: PropTypes.func, onNewQueryFormSubmit: PropTypes.func, onOsqueryTableSelect: PropTypes.func, onTextEditorInputChange: PropTypes.func, @@ -67,17 +70,26 @@ class NewQuery extends Component { }); } - onSaveQueryFormSubmit = (formData) => { - const { onNewQueryFormSubmit } = this.props; + onSaveQueryFormSubmit = debounce((formData) => { + const { + onInvalidQuerySubmit, + onNewQueryFormSubmit, + textEditorText, + } = this.props; const { selectedTargets } = this.state; - onNewQueryFormSubmit({ + const { error } = validateQuery(textEditorText); + + if (error) { + return onInvalidQuerySubmit(error); + } + + return onNewQueryFormSubmit({ ...formData, + query: textEditorText, selectedTargets, }); - - return false; - } + }) onTargetSelect = (selectedTargets) => { this.setState({ selectedTargets }); diff --git a/frontend/components/queries/NewQuery/NewQuery.tests.jsx b/frontend/components/queries/NewQuery/NewQuery.tests.jsx new file mode 100644 index 0000000000..afb4067ebb --- /dev/null +++ b/frontend/components/queries/NewQuery/NewQuery.tests.jsx @@ -0,0 +1,92 @@ +import React from 'react'; +import expect, { createSpy, restoreSpies } from 'expect'; +import { mount } from 'enzyme'; +import { noop } from 'lodash'; + +import { createAceSpy } from '../../../test/helpers'; +import NewQuery from './index'; + +describe('NewQuery - component', () => { + beforeEach(() => { + createAceSpy(); + }); + afterEach(restoreSpies); + + it('renders the SaveQuerySection', () => { + const component = mount( + + ); + + expect(component.find('SaveQuerySection').length).toEqual(1); + }); + + it('renders the ThemeDropdown', () => { + const component = mount( + + ); + + expect(component.find('ThemeDropdown').length).toEqual(1); + }); + + it('renders the SaveQueryForm', () => { + const component = mount( + + ); + + component.find('Slider').simulate('click'); + + expect(component.find('SaveQueryForm').length).toEqual(1); + }); + + describe('Query string validations', () => { + const invalidQuery = 'CREATE TABLE users (LastName varchar(255))'; + const validQuery = 'SELECT * FROM users'; + + it('calls onInvalidQuerySubmit when invalid', () => { + const invalidQuerySubmitSpy = createSpy(); + const component = mount( + + ); + const form = component.find('SaveQueryForm'); + + form.simulate('submit'); + + expect(invalidQuerySubmitSpy).toHaveBeenCalledWith('Cannot INSERT or CREATE in osquery queries'); + }); + + it('calls onNewQueryFormSubmit when valid', () => { + const onNewQueryFormSubmitSpy = createSpy(); + const component = mount( + + ); + const form = component.find('SaveQueryForm'); + + form.simulate('submit'); + + expect(onNewQueryFormSubmitSpy).toHaveBeenCalled(); + }); + }); +}); + diff --git a/frontend/components/Queries/NewQuery/SaveQuerySection/index.jsx b/frontend/components/queries/NewQuery/SaveQuerySection/index.jsx similarity index 100% rename from frontend/components/Queries/NewQuery/SaveQuerySection/index.jsx rename to frontend/components/queries/NewQuery/SaveQuerySection/index.jsx diff --git a/frontend/components/Queries/NewQuery/SaveQuerySection/styles.js b/frontend/components/queries/NewQuery/SaveQuerySection/styles.js similarity index 100% rename from frontend/components/Queries/NewQuery/SaveQuerySection/styles.js rename to frontend/components/queries/NewQuery/SaveQuerySection/styles.js diff --git a/frontend/components/Queries/NewQuery/ThemeDropdown/index.jsx b/frontend/components/queries/NewQuery/ThemeDropdown/index.jsx similarity index 100% rename from frontend/components/Queries/NewQuery/ThemeDropdown/index.jsx rename to frontend/components/queries/NewQuery/ThemeDropdown/index.jsx diff --git a/frontend/components/Queries/NewQuery/ThemeDropdown/styles.js b/frontend/components/queries/NewQuery/ThemeDropdown/styles.js similarity index 100% rename from frontend/components/Queries/NewQuery/ThemeDropdown/styles.js rename to frontend/components/queries/NewQuery/ThemeDropdown/styles.js diff --git a/frontend/components/queries/NewQuery/helpers.js b/frontend/components/queries/NewQuery/helpers.js new file mode 100644 index 0000000000..822e9487c5 --- /dev/null +++ b/frontend/components/queries/NewQuery/helpers.js @@ -0,0 +1,29 @@ +import sqliteParser from 'sqlite-parser'; +import { includes, some } from 'lodash'; + +const BLACKLISTED_ACTIONS = ['insert', 'create']; +const invalidQueryErrorMessage = 'Cannot INSERT or CREATE in osquery queries'; +const invalidQueryResponse = (message) => { + return { valid: false, error: message }; +}; +const validQueryResponse = { valid: true, error: null }; + +export const validateQuery = (queryText) => { + try { + const ast = sqliteParser(queryText); + const { statement } = ast; + const invalidQuery = some(statement, (obj) => { + return includes(BLACKLISTED_ACTIONS, obj.variant.toLowerCase()); + }); + + if (invalidQuery) { + return invalidQueryResponse(invalidQueryErrorMessage); + } + + return validQueryResponse; + } catch (error) { + return invalidQueryResponse(error.message); + } +}; + +export default { validateQuery }; diff --git a/frontend/components/queries/NewQuery/helpers.tests.js b/frontend/components/queries/NewQuery/helpers.tests.js new file mode 100644 index 0000000000..8ecdab1da6 --- /dev/null +++ b/frontend/components/queries/NewQuery/helpers.tests.js @@ -0,0 +1,39 @@ +import expect from 'expect'; + +import helpers from './helpers'; + +const malformedQuery = 'this is not a thing'; +const validQuery = 'SELECT * FROM users'; +const createQuery = 'CREATE TABLE users (LastName varchar(255))'; +const insertQuery = 'INSERT INTO users (name) values ("Mike")'; + +describe('NewQuery - helpers', () => { + describe('#validateQuery', () => { + const { validateQuery } = helpers; + + it('rejects malformed queries', () => { + const { error, valid } = validateQuery(malformedQuery); + + expect(valid).toEqual(false); + expect(error).toEqual('Syntax error found near WITH Clause (Statement)'); + }); + + it('rejects create queries', () => { + const { error, valid } = validateQuery(createQuery); + expect(valid).toEqual(false); + expect(error).toEqual('Cannot INSERT or CREATE in osquery queries'); + }); + + it('rejects insert queries', () => { + const { error, valid } = validateQuery(insertQuery); + expect(valid).toEqual(false); + expect(error).toEqual('Cannot INSERT or CREATE in osquery queries'); + }); + + it('accepts valid queries', () => { + const { error, valid } = validateQuery(validQuery); + expect(valid).toEqual(true); + expect(error).toNotExist(); + }); + }); +}); diff --git a/frontend/components/Queries/NewQuery/index.js b/frontend/components/queries/NewQuery/index.js similarity index 100% rename from frontend/components/Queries/NewQuery/index.js rename to frontend/components/queries/NewQuery/index.js diff --git a/frontend/components/Queries/NewQuery/mode.js b/frontend/components/queries/NewQuery/mode.js similarity index 100% rename from frontend/components/Queries/NewQuery/mode.js rename to frontend/components/queries/NewQuery/mode.js diff --git a/frontend/components/Queries/NewQuery/styles.js b/frontend/components/queries/NewQuery/styles.js similarity index 100% rename from frontend/components/Queries/NewQuery/styles.js rename to frontend/components/queries/NewQuery/styles.js diff --git a/frontend/components/Queries/NewQuery/theme.js b/frontend/components/queries/NewQuery/theme.js similarity index 100% rename from frontend/components/Queries/NewQuery/theme.js rename to frontend/components/queries/NewQuery/theme.js diff --git a/frontend/components/Queries/QueryPageWrapper/QueryPageWrapper.jsx b/frontend/components/queries/QueryPageWrapper/QueryPageWrapper.jsx similarity index 100% rename from frontend/components/Queries/QueryPageWrapper/QueryPageWrapper.jsx rename to frontend/components/queries/QueryPageWrapper/QueryPageWrapper.jsx diff --git a/frontend/components/Queries/QueryPageWrapper/index.js b/frontend/components/queries/QueryPageWrapper/index.js similarity index 100% rename from frontend/components/Queries/QueryPageWrapper/index.js rename to frontend/components/queries/QueryPageWrapper/index.js diff --git a/frontend/pages/Queries/NewQueryPage/NewQueryPage.jsx b/frontend/pages/queries/NewQueryPage/NewQueryPage.jsx similarity index 78% rename from frontend/pages/Queries/NewQueryPage/NewQueryPage.jsx rename to frontend/pages/queries/NewQueryPage/NewQueryPage.jsx index f0d6efa426..44e8fcb3ee 100644 --- a/frontend/pages/Queries/NewQueryPage/NewQueryPage.jsx +++ b/frontend/pages/queries/NewQueryPage/NewQueryPage.jsx @@ -2,10 +2,11 @@ import React, { Component, PropTypes } from 'react'; import { connect } from 'react-redux'; import { find } from 'lodash'; -import NewQuery from '../../../components/Queries/NewQuery'; +import NewQuery from '../../../components/queries/NewQuery'; +import { osqueryTables } from '../../../utilities/osquery_tables'; import QuerySidePanel from '../../../components/side_panels/QuerySidePanel'; import { showRightSidePanel, removeRightSidePanel } from '../../../redux/nodes/app/actions'; -import { osqueryTables } from '../../../utilities/osquery_tables'; +import { renderFlash } from '../../../redux/nodes/notifications/actions'; class NewQueryPage extends Component { static propTypes = { @@ -35,13 +36,15 @@ class NewQueryPage extends Component { } onNewQueryFormSubmit = (formData) => { - const { textEditorText } = this.state; - const data = { - queryText: textEditorText, - ...formData, - }; + console.log('New Query Form submitted', formData); + } - console.log('New Query Form submitted', data); + onInvalidQuerySubmit = (errorMessage) => { + const { dispatch } = this.props; + + dispatch(renderFlash('error', errorMessage)); + + return false; } onOsqueryTableSelect = (tableName) => { @@ -59,12 +62,18 @@ class NewQueryPage extends Component { render () { const { selectedOsqueryTable, textEditorText } = this.state; - const { onNewQueryFormSubmit, onOsqueryTableSelect, onTextEditorInputChange } = this; + const { + onNewQueryFormSubmit, + onInvalidQuerySubmit, + onOsqueryTableSelect, + onTextEditorInputChange, + } = this; return (