diff --git a/package.json b/package.json index 25ced7f3..510198b3 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "scripts": { "preinstall": "node -e \"if(process.env.npm_execpath.indexOf('yarn') === -1) throw new Error('You must use Yarn to install, not NPM')\"", "clean": "rimraf dist/*", - "upgrade-form-control": "yarn upgrade bahmni-form-controls@0.94.0", + "upgrade-form-control": "yarn upgrade bahmni-form-controls@1.0.2", "copy": "copyfiles -f ./index.html ./dist", "build": "yarn run copy && webpack", "build-dev": "yarn run copy && webpack --config webpack.dev.config.js", @@ -81,7 +81,7 @@ "babel-loader": "^6.2.4", "babel-preset-es2015": "^6.9.0", "babel-preset-react": "^6.11.1", - "bahmni-form-controls": "^0.94.0", + "bahmni-form-controls": "^1.0.2", "classnames": "^2.2.5", "codemirror": "^5.51.0", "core-js": "^2.4.1", diff --git a/src/form-builder/actions/control.js b/src/form-builder/actions/control.js index 2df06e43..361b4495 100644 --- a/src/form-builder/actions/control.js +++ b/src/form-builder/actions/control.js @@ -53,3 +53,4 @@ export const formLoad = (controls) => ({ type: 'FORM_LOAD', controls }); export const deleteControl = (controlIds) => ({ type: 'DELETE_CONTROL', controlIds }); export const formDefVersionUpdate = (version) => ({ type: 'FORM_DEFINITION_VERSION_UPDATE', version }); +export const setAllowedDomains = (domains) => ({ type: 'SET_ALLOWED_DOMAINS', domains }); diff --git a/src/form-builder/components/ControlPropertiesContainer.jsx b/src/form-builder/components/ControlPropertiesContainer.jsx index 0285ce5e..521d213f 100644 --- a/src/form-builder/components/ControlPropertiesContainer.jsx +++ b/src/form-builder/components/ControlPropertiesContainer.jsx @@ -81,7 +81,7 @@ export class ControlPropertiesContainer extends Component { displayPropertyEditor() { const { selectedControl, selectedControl: { id, concept } } = this.props; - if (concept || selectedControl.type === 'section') { + if (concept || selectedControl.type === 'section' || selectedControl.type === 'label') { return ( 0) { + this.props.onValidationError( + `Import failed for form "${formName}": ${hyperlinkErrors.join('; ')}` + ); + return Promise.resolve(); + } return httpInterceptor.post(formBuilderConstants.formUrl, form).then((response) => { val.uuid = response.uuid; const formResource = { @@ -512,6 +520,7 @@ export default class FormBuilder extends Component { } FormBuilder.propTypes = { + allowedDomains: PropTypes.arrayOf(PropTypes.string), data: PropTypes.array.isRequired, dispatch: PropTypes.func.isRequired, match: PropTypes.shape({ diff --git a/src/form-builder/components/FormBuilderContainer.jsx b/src/form-builder/components/FormBuilderContainer.jsx index f27ecba6..fd88f16c 100644 --- a/src/form-builder/components/FormBuilderContainer.jsx +++ b/src/form-builder/components/FormBuilderContainer.jsx @@ -19,8 +19,9 @@ import get from 'lodash/get'; import map from 'lodash/map'; import sortBy from 'lodash/sortBy'; import formHelper from '../helpers/formHelper'; +import { fetchAllowedDomains } from '../helpers/hyperlinkValidationHelper'; import { connect } from 'react-redux'; -import { setDefaultLocale } from '../actions/control'; +import { setDefaultLocale, setAllowedDomains } from '../actions/control'; import { saveFormNameTranslations, saveTranslations } from 'common/apis/formTranslationApi'; @@ -36,6 +37,9 @@ export class FormBuilderContainer extends Component { this.getFormData().then(() => { this.getDefaultLocale(); }); + fetchAllowedDomains().then((allowedDomains) => { + this.props.dispatch(setAllowedDomains(allowedDomains)); + }); } onValidationError(message) { @@ -162,9 +166,7 @@ export class FormBuilderContainer extends Component { }); self.saveTranslations(updatedTranslations, formNameTranslationsResource); }) - .catch(() => { - this.setMessage('Error Importing Form', commonConstants.responseType.error); - }); + .catch((error) => this.showErrors(error)); } render() { @@ -175,6 +177,7 @@ export class FormBuilderContainer extends Component { notification={this.state.notification} /> { + this.props.dispatch(setAllowedDomains(allowedDomains)); + }); + } } componentWillUpdate(nextProps, nextState) { @@ -183,6 +190,14 @@ export class FormDetailContainer extends Component { onSave() { try { + const formResource = this.getFormResource(); + const hyperlinkErrors = validateFormHyperlinks( + JSON.parse(formResource.value), this.props.allowedDomains || [] + ); + if (hyperlinkErrors.length > 0) { + this.setErrorMessage({ message: hyperlinkErrors.join('; ') }); + return; + } const initialPrivileges = []; const formId = this.state.formData.id; const formVersion = this.state.formData.version; @@ -192,7 +207,6 @@ export class FormDetailContainer extends Component { initialPrivileges.push(privilege); }); this.setState({ formPrivileges: initialPrivileges, loading: false }); - const formResource = this.getFormResource(); this._saveFormResource(formResource); this._saveFormPrivileges(this.state.formData.id, this.state.formData.version, this.state.formPrivileges); @@ -246,6 +260,14 @@ export class FormDetailContainer extends Component { onPublish() { try { + const formJson = this.getFormResource(); + const hyperlinkErrors = validateFormHyperlinks( + JSON.parse(formJson.value), this.props.allowedDomains || [] + ); + if (hyperlinkErrors.length > 0) { + this.setErrorMessage({ message: hyperlinkErrors.join('; ') }); + return; + } const initialPrivileges = []; const formId = this.state.formData.id; const formVersion = this.state.formData.version; @@ -255,7 +277,6 @@ export class FormDetailContainer extends Component { initialPrivileges.push(privilege); }); this.setState({ formPrivileges: initialPrivileges, loading: false }); - const formJson = this.getFormResource(); httpInterceptor.post(formBuilderConstants.bahmniFormResourceUrl, formJson) .then((response) => { this.setFormData(response); @@ -515,6 +536,7 @@ export class FormDetailContainer extends Component { position="top center" > this.closePreview()} formJson={this.state.formPreviewJson} setErrorMessage={this.setErrorMessage} @@ -777,6 +799,7 @@ FormDetailContainer.contextTypes = { function mapStateToProps(state) { return { defaultLocale: state.formDetails && state.formDetails.defaultLocale, + allowedDomains: state.formDetails && state.formDetails.allowedDomains, translations: state.translations, formDetails: state.formDetails, formControlEvents: state.controlDetails.allObsControlEvents, diff --git a/src/form-builder/components/FormPreviewModal.jsx b/src/form-builder/components/FormPreviewModal.jsx index 963134c4..8cc9ea79 100644 --- a/src/form-builder/components/FormPreviewModal.jsx +++ b/src/form-builder/components/FormPreviewModal.jsx @@ -69,7 +69,9 @@ export default class FormPreviewModal extends React.Component { const container = React.createElement(Container, { metadata, observations, validate: true, validateForm: false, collapse: false, patient: null, locale: this.state.defaultLocale, translations: '', - onValueUpdated: this.onValueUpdated }); + onValueUpdated: this.onValueUpdated, + allowedDomains: this.props.allowedDomains || [], + showValidationErrors: true }); ReactDOM.render(container, document.getElementById('form-container')); } } @@ -100,6 +102,7 @@ export default class FormPreviewModal extends React.Component { } FormPreviewModal.propTypes = { + allowedDomains: PropTypes.arrayOf(PropTypes.string), close: PropTypes.func.isRequired, formJson: PropTypes.object, setErrorMessage: PropTypes.func.isRequired, diff --git a/src/form-builder/components/FormPrivilegeTable.jsx b/src/form-builder/components/FormPrivilegeTable.jsx index 12e67a9b..3a9c48fa 100644 --- a/src/form-builder/components/FormPrivilegeTable.jsx +++ b/src/form-builder/components/FormPrivilegeTable.jsx @@ -91,7 +91,7 @@ export default class FormPrivilegeTable extends Component { } fetchPrivileges() { - let initialPrivileges = []; + const initialPrivileges = []; let privileges = []; const queryParams = '?='; const optionsUrl = `${formBuilderConstants.formPrivilegeUrl}${queryParams}`; diff --git a/src/form-builder/components/Property.jsx b/src/form-builder/components/Property.jsx index 9839e5a1..b8d6827b 100644 --- a/src/form-builder/components/Property.jsx +++ b/src/form-builder/components/Property.jsx @@ -9,6 +9,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; +import { formBuilderConstants } from 'form-builder/constants'; export class Property extends Component { @@ -43,10 +44,9 @@ export class Property extends Component { ); case 'text': return ( this.updateProperty(e, elementType) } : { onChange: e => this.updateProperty(e, elementType) })} type="text" @@ -73,7 +73,7 @@ export class Property extends Component { render() { const { name, elementType } = this.props; return ( -
+
{this.getElement(elementType)}
diff --git a/src/form-builder/constants.js b/src/form-builder/constants.js index a241e3f9..313d94a1 100644 --- a/src/form-builder/constants.js +++ b/src/form-builder/constants.js @@ -39,4 +39,7 @@ export const formBuilderConstants = { pdfDownloadUrl: '/openmrs/ws/rest/v1/bahmniie/form/download/', dataLimit: 9999, formDefinitionVersion: 2.0, + hyperlinkUrlProperty: 'hyperlinkUrl', + allowedDomainsGPUrl: '/openmrs/ws/rest/v1/bahmnicore/sql/globalproperty?property=bahmni.forms.hyperlink.allowedDomains', + }; diff --git a/src/form-builder/helpers/hyperlinkValidationHelper.js b/src/form-builder/helpers/hyperlinkValidationHelper.js new file mode 100644 index 00000000..a919c929 --- /dev/null +++ b/src/form-builder/helpers/hyperlinkValidationHelper.js @@ -0,0 +1,32 @@ +import { validateHyperlink } from 'bahmni-form-controls'; +import { httpInterceptor } from 'common/utils/httpInterceptor'; +import { formBuilderConstants } from 'form-builder/constants'; + +function collectHyperlinkUrls(controls, acc, visited = new Set()) { + if (!controls) return acc; + controls.forEach((control) => { + if (visited.has(control)) return; + visited.add(control); + if (control.type === 'label' && control.properties && control.properties.hyperlinkUrl) { + acc.push(control.properties.hyperlinkUrl); + } + if (control.label) collectHyperlinkUrls([control.label], acc, visited); + if (control.controls) collectHyperlinkUrls(control.controls, acc, visited); + }); + return acc; +} + +export function validateFormHyperlinks(formJson, allowedDomains) { + const urls = collectHyperlinkUrls(formJson.controls || [], []); + return urls + .map((url) => validateHyperlink(url, allowedDomains)) + .filter((result) => !result.valid) + .map((result) => `Invalid hyperlink: ${result.error}`); +} + +export function fetchAllowedDomains() { + return httpInterceptor + .get(formBuilderConstants.allowedDomainsGPUrl, 'text') + .then((data) => (data || '').split(',').map((d) => d.trim()).filter(Boolean)) + .catch(() => []); +} diff --git a/src/form-builder/reducers/formDetails.js b/src/form-builder/reducers/formDetails.js index 906ab149..7c4b7719 100644 --- a/src/form-builder/reducers/formDetails.js +++ b/src/form-builder/reducers/formDetails.js @@ -33,6 +33,8 @@ const formDetails = (store = {}, action) => { return Object.assign({}, store, { defaultLocale: action.locale }); case 'FORM_DEFINITION_VERSION_UPDATE': return Object.assign({}, store, { formDefVersion: action.version }); + case 'SET_ALLOWED_DOMAINS': + return Object.assign({}, store, { allowedDomains: action.domains }); default: return store; } diff --git a/styles/common/_canvas.scss b/styles/common/_canvas.scss index 9304cb99..aea25c6d 100644 --- a/styles/common/_canvas.scss +++ b/styles/common/_canvas.scss @@ -83,3 +83,30 @@ min-height: 40px; } } + +[data-bahmni-hyperlink], +.form-builder-hyperlink-preview { + text-decoration: none; + + &:hover { + text-decoration: underline; + } +} + +.obs-attached-label { + display: block; + text-align: right; + min-height: 23px; + + .control-wrapper-content { + display: inline-flex; + align-items: center; + } + + .remove-control-button { + position: static; + margin-left: 4px; + top: auto; + right: auto; + } +} diff --git a/styles/common/_form.scss b/styles/common/_form.scss index ed33746d..cc263440 100644 --- a/styles/common/_form.scss +++ b/styles/common/_form.scss @@ -6,6 +6,22 @@ * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS * graphic logo is a trademark of OpenMRS Inc. */ +.property-text-row { + display: flex; + align-items: center; + padding: 2px 0; + + label { + flex-shrink: 0; + white-space: nowrap; + padding-right: 8px; + } + + input[type='text'] { + flex: 1; + min-width: 0; + } +} .form-field{ padding: 5px 0; diff --git a/test/form-builder/components/Property.spec.js b/test/form-builder/components/Property.spec.js index ecdf2b68..baebce4b 100755 --- a/test/form-builder/components/Property.spec.js +++ b/test/form-builder/components/Property.spec.js @@ -79,6 +79,34 @@ describe('Property', () => { expect(wrapper.find('input').props().defaultValue).to.eql('someText'); }); + it('should apply property-text-row class only for text type and not for others', () => { + const textWrapper = shallow( {}} + value="" + />); + expect(textWrapper.find('div').prop('className')).to.eql('property-text-row'); + + const checkboxWrapper = shallow( {}} + value={false} + />); + expect(checkboxWrapper.find('div').prop('className')).to.equal(undefined); + }); + + it('should not have className attribute on non-text wrapper div', () => { + wrapper = shallow( {}} + value={false} + />); + expect(wrapper.find('div').prop('className')).to.equal(undefined); + }); + it('should call update property on change of text box', () => { const spy = sinon.spy(); const type = 'text'; @@ -107,6 +135,20 @@ describe('Property', () => { sinon.assert.calledWith(spy, { url: 'someText' }); }); + it('should call update property on blur of text box if the name is hyperlinkUrl', () => { + const spy = sinon.spy(); + const type = 'text'; + + wrapper = shallow(); + wrapper.find('input').props().onBlur({ target: { value: 'https://example.com' } }, type); + sinon.assert.calledWith(spy, { hyperlinkUrl: 'https://example.com' }); + }); + it('should render select dropdown when given property with dropdown', () => { const type = 'dropdown'; diff --git a/test/form-builder/helpers/hyperlinkValidationHelper.spec.js b/test/form-builder/helpers/hyperlinkValidationHelper.spec.js new file mode 100644 index 00000000..fe051a25 --- /dev/null +++ b/test/form-builder/helpers/hyperlinkValidationHelper.spec.js @@ -0,0 +1,149 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at https://www.bahmni.org/license/mplv2hd. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ + +import { expect } from 'chai'; +import sinon from 'sinon'; +import { httpInterceptor } from 'common/utils/httpInterceptor'; +import { + validateFormHyperlinks, + fetchAllowedDomains, +} from 'form-builder/helpers/hyperlinkValidationHelper'; + +const ALLOWED_DOMAINS = ['who.int', 'usda.gov']; + +function labelControl(hyperlinkUrl) { + return { type: 'label', properties: { hyperlinkUrl } }; +} + +describe('hyperlinkValidationHelper', () => { + describe('validateFormHyperlinks', () => { + it('returns no errors for an empty form', () => { + const errors = validateFormHyperlinks({ controls: [] }, ALLOWED_DOMAINS); + expect(errors).to.eql([]); + }); + + it('returns no errors when form has no label controls', () => { + const formJson = { controls: [{ type: 'obsControl', properties: {} }] }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.eql([]); + }); + + it('returns no errors for a label with a valid allowed-domain URL', () => { + const formJson = { controls: [labelControl('https://who.int/some-page')] }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.eql([]); + }); + + it('returns no errors for a label with a valid internal URL', () => { + const formJson = { controls: [labelControl('/patient/summary')] }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.eql([]); + }); + + it('returns an error for a label with a disallowed external domain', () => { + const formJson = { controls: [labelControl('https://evil.com/page')] }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.have.length(1); + expect(errors[0]).to.include('Invalid hyperlink'); + }); + + it('returns an error for a javascript: URL', () => { + const formJson = { controls: [labelControl('java' + 'script:alert(1)')] }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.have.length(1); + expect(errors[0]).to.include('Invalid hyperlink'); + }); + + it('returns errors for all invalid labels in a flat list', () => { + const formJson = { + controls: [ + labelControl('https://evil.com'), + labelControl('https://who.int/ok'), + labelControl('java' + 'script:void(0)'), + ], + }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.have.length(2); + }); + + it('traverses nested controls via control.controls', () => { + const formJson = { + controls: [{ + type: 'section', + controls: [labelControl('https://evil.com')], + }], + }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.have.length(1); + }); + + it('traverses nested label via control.label', () => { + const formJson = { + controls: [{ + type: 'obsControl', + label: { type: 'label', properties: { hyperlinkUrl: 'https://evil.com' } }, + }], + }; + const errors = validateFormHyperlinks(formJson, ALLOWED_DOMAINS); + expect(errors).to.have.length(1); + }); + + it('returns errors for all external URLs when allowedDomains is empty', () => { + const formJson = { controls: [labelControl('https://who.int/page')] }; + const errors = validateFormHyperlinks(formJson, []); + expect(errors).to.have.length(1); + }); + + it('handles missing controls key on formJson gracefully', () => { + const errors = validateFormHyperlinks({}, ALLOWED_DOMAINS); + expect(errors).to.eql([]); + }); + + it('does not infinitely recurse on circular control references', () => { + const control = labelControl('https://evil.com'); + control.controls = [control]; + const formJson = { controls: [control] }; + expect(() => validateFormHyperlinks(formJson, ALLOWED_DOMAINS)).to.not.throw(); + }); + }); + + describe('fetchAllowedDomains', () => { + afterEach(() => { + if (httpInterceptor.get.restore) httpInterceptor.get.restore(); + }); + + it('returns parsed domain list on successful fetch', () => { + sinon.stub(httpInterceptor, 'get').returns(Promise.resolve('who.int, usda.gov')); + return fetchAllowedDomains().then((domains) => { + expect(domains).to.eql(['who.int', 'usda.gov']); + }); + }); + + it('filters out empty strings from domain list', () => { + sinon.stub(httpInterceptor, 'get').returns(Promise.resolve('who.int,,usda.gov,')); + return fetchAllowedDomains().then((domains) => { + expect(domains).to.eql(['who.int', 'usda.gov']); + }); + }); + + it('returns empty array on fetch failure', () => { + sinon.stub(httpInterceptor, 'get').returns(Promise.reject(new Error('network error'))); + return fetchAllowedDomains().then((domains) => { + expect(domains).to.eql([]); + }); + }); + + it('returns empty array when response is null', () => { + sinon.stub(httpInterceptor, 'get').returns(Promise.resolve(null)); + return fetchAllowedDomains().then((domains) => { + expect(domains).to.eql([]); + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 47a40be9..a509938d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1100,16 +1100,18 @@ backo2@1.0.2: resolved "https://registry.yarnpkg.com/backo2/-/backo2-1.0.2.tgz#31ab1ac8b129363463e35b3ebb69f4dfcfba7947" integrity sha1-MasayLEpNjRj41s+u2n038+6eUc= -bahmni-form-controls@^0.94.0: - version "0.94.0" - resolved "https://registry.npmjs.org/bahmni-form-controls/-/bahmni-form-controls-0.94.0.tgz#726dc8701bb1bc06c972f2bcc5d4d2f75f110fe9" - integrity sha512-CsSFIJNNKxu0Opfp/rYrMDYuVd+cOmsMulWyCfP21qvS8y2aY4hHo+b+oxmA58J3nF8ANJB20i60guW1TSOY8Q== +bahmni-form-controls@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/bahmni-form-controls/-/bahmni-form-controls-1.0.2.tgz#f46842bef1f77a82d0ab90b9912d94067d8dc946" + integrity sha512-/f8FGDmBSjl7AusttOuJlx+Y3cn4HhpU/KLJDmep5HwzHn+JlK2Ql2DgQyonKghpdo5gUlYrgSL1UE86JlYLww== dependencies: base64-inline-loader "^1.1.0" classnames "^2.2.5" enzyme-adapter-react-16 "^1.1.0" + html-entities "^2.6.0" immutable "3.8.1" lodash "4.17.12" + moment "^2.29.1" prop-types "^15.6.2" react-intl "^3.12.0" react-select "^1.0.0-rc.2" @@ -4333,6 +4335,11 @@ html-element-map@^1.2.0: dependencies: array-filter "^1.0.0" +html-entities@^2.6.0: + version "2.6.0" + resolved "https://registry.yarnpkg.com/html-entities/-/html-entities-2.6.0.tgz#7c64f1ea3b36818ccae3d3fb48b6974208e984f8" + integrity sha512-kig+rMn/QOVRvr7c86gQ8lWXq+Hkv6CbAH1hLu+RG338StTpE8Z0b44SDVaqVu7HGKf27frdmUYEs9hTUX/cLQ== + html-tags@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/html-tags/-/html-tags-2.0.0.tgz#10b30a386085f43cede353cc8fa7cb0deeea668b" @@ -6014,6 +6021,11 @@ moment@^2.14.1: resolved "https://registry.yarnpkg.com/moment/-/moment-2.24.0.tgz#0d055d53f5052aa653c9f6eb68bb5d12bf5c2b5b" integrity sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg== +moment@^2.29.1: + version "2.30.1" + resolved "https://registry.yarnpkg.com/moment/-/moment-2.30.1.tgz#f8c91c07b7a786e30c59926df530b4eac96974ae" + integrity sha512-uEmtNhbDOrWPFS+hdjFCBfy9f2YoyzRpwcl+DqpC6taX21FzsTLQVbMV/W7PzNSX6x/bhC1zA3c2UQ5NzH6how== + moo@^0.5.0: version "0.5.1" resolved "https://registry.yarnpkg.com/moo/-/moo-0.5.1.tgz#7aae7f384b9b09f620b6abf6f74ebbcd1b65dbc4"