Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Add new SafeInnerHTML component #89

Open
cwardzala wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add new SafeInnerHTML component #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b0009d
Add new SafeInnerHTML component
cwardzala Jan 6, 2023
c45b44b
Add info about DOMPurify to forms page
cwardzala Jan 6, 2023
bc84b38
Pass through props to new component
cwardzala Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
"color": "^4.2.3",
"formik": "^2.2.9",
"highlight.js": "^11.5.0",
"isomorphic-dompurify": "^0.25.0",
"prop-types": "^15.8.1",
"react": "^17.0.2",
"react-app-polyfill": "^3.0.0",
"react-dom": "^17.0.2",
"react-icons": "^4.3.1",
"react-quill": "^2.0.0",
"react-router-dom": "^6.3.0",
"react-scripts": "5.0.0",
"react-select": "^5.2.2",
Expand Down
23 changes: 23 additions & 0 deletions src/components/SafeInnerHTML.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import React from 'react';
import DOMPurify from 'isomorphic-dompurify';
import PropTypes from 'prop-types';

export const purifySettings = {
KEEP_CONTENT: false,
FORBID_TAGS: ['style', 'img'], // In most cases we don't need style or img tags
FORBID_ATTR: ['style'] // In most cases we don't want inline css
};

const SafeInnerHTML = ({ as = 'div', html = '', ...props }) => {
const Comp = as;
if (!html) return null;

return <Comp {...props} dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(html, purifySettings) }} />;
};

SafeInnerHTML.propTypes = {
as: PropTypes.elementType,
html: PropTypes.string
};
Comment on lines +18 to +21
Copy link
Collaborator

@nsmedira nsmedira Jan 6, 2023

Choose a reason for hiding this comment

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

if we merged in the work on TS in this PR, we could write this in TS from the start and replace propTypes with Props interface

Copy link
Member Author

@cwardzala cwardzala Jan 6, 2023

Choose a reason for hiding this comment

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

Yea, seems like that PR is not quite ready for prime time.

Copy link
Contributor

@nclayt0n nclayt0n Jan 17, 2023

Choose a reason for hiding this comment

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

now that the TS PR has been approved, can we refactor this to typescript


export default SafeInnerHTML;
48 changes: 30 additions & 18 deletions src/components/form/WysiwygEditor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import React from 'react';
import classnames from 'classnames';

import ReactQuill from 'react-quill';
import SafeInnerHTML, { purifySettings } from '../SafeInnerHTML';
import DOMPurify from 'isomorphic-dompurify';

import 'react-quill/dist/quill.snow.css';

const WysiwygEditor = ({
id = 'wysiwyg1',
Expand All @@ -14,6 +18,7 @@ const WysiwygEditor = ({
required = false,
showRequired = false,
className = null,
hideLabel = false,

errors = null,
touched = null,
Expand All @@ -33,48 +38,55 @@ const WysiwygEditor = ({
let isRequired = required || showRequired;

const labelClassName = classnames({
'required': isRequired
required: isRequired
});

const cleanHTML = DOMPurify.sanitize(html, purifySettings);

return (
<div className={classnames('form-group', className)}>
<div className="label-wrapper">
<label htmlFor={id} className={labelClassName}>{label} {isRequired && <small className="text-muted font-weight-normal font-italic text-danger">(required)</small>}</label>
{labelHelp}
</div>
{readOnly && <div dangerouslySetInnerHTML={{__html: html}} />}
{!readOnly &&
{!!label && !hideLabel && (
<div className="label-wrapper">
<label htmlFor={id} className={labelClassName}>
{label}{' '}
{isRequired && (
<small className="text-muted font-weight-normal font-italic text-danger">(required)</small>
)}
</label>
{labelHelp}
</div>
)}
{readOnly && <SafeInnerHTML html={cleanHTML} />}
{!readOnly && (
<ReactQuill
className={classnames('wysiwyg-editor', {
'error': errors
error: errors
})}
placeholder={placeholder}
defaultValue={html}
defaultValue={cleanHTML}
formats={['bold', 'italic', 'underline', 'blockquote', 'header', 'list']}
modules={{
toolbar: [
[{ 'header': [1, 2, false] }],
[{ header: [1, 2, false] }],
['bold', 'italic', 'underline'],
[{'list': 'ordered'}, {'list': 'bullet'}]
[{ list: 'ordered' }, { list: 'bullet' }]
]
}}
onChange={onQuillChange}
onFocus={onFocus}
onBlur={onBlur}
/>
}
)}

{help &&
<small className="form-text text-muted">{help}</small>
}
{help && <small className="form-text text-muted">{help}</small>}

{!!errors && touched &&
{!!errors && touched && (
<div className="form-error">
<span>{errors}</span>
</div>
}
)}
</div>
);
}
};

export default WysiwygEditor;
27 changes: 26 additions & 1 deletion src/pages/Components/FormsPage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import DisplayFormikState from '../../components/DisplayFormikState';
import { BreadcrumbItem, Breadcrumbs } from '../../components/Breadcrumbs';
import { Link } from 'react-router-dom';
import ReactSelect from '../../components/form/ReactSelect';
import WysiwygEditor from '../../components/form/WysiwygEditor';

import { FaExternalLinkAlt } from 'react-icons/fa';

const FormsPage = () => {
let fruits = ['apple', 'banana', 'orange', 'avocado'];
Expand Down Expand Up @@ -48,7 +51,11 @@ const FormsPage = () => {
</Breadcrumbs>
<h2>Forms</h2>
<p>
Forms should be built with <a href="https://jaredpalmer.com/formik/">Formik</a>.
Forms should be built with{' '}
<a href="https://jaredpalmer.com/formik/" target="_blank" rel="noopener noreferrer">
Formik <FaExternalLinkAlt style={{ fontSize: '75%' }} />
Copy link
Contributor

@nclayt0n nclayt0n Jan 17, 2023

Choose a reason for hiding this comment

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

either add title, aria-label, or aria-hidden. As of now the screen reader has no context for this icon.

</a>
.
</p>
<p>Form markup is derrived from Bootstrap.</p>
<hr />
Expand Down Expand Up @@ -136,6 +143,24 @@ const FormsPage = () => {
<hr />
<TextArea id="text_area" label="Text Area" />

<h3>WYSIWYG Editor</h3>
<hr />
<p>
The HTML content passed into and generated from <code>WysiwygEditor</code> is sanitized with{' '}
<a href="https://github.com/cure53/DOMPurify" target="_blank" rel="noopener noreferrer">
DOMPurify <FaExternalLinkAlt style={{ fontSize: '75%' }} />
Copy link
Contributor

@nclayt0n nclayt0n Jan 17, 2023

Choose a reason for hiding this comment

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

class instead of inline style

Copy link
Contributor

@nclayt0n nclayt0n Jan 17, 2023

Choose a reason for hiding this comment

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

either add title, aria-label, or aria-hidden. As of now the screen reader has no context for this icon.

</a>
</p>
<WysiwygEditor
label="WYSIWYG Editor"
html={`<p>Pellentesque habitant <strong style="color: red">morbi tristique senectus</strong> <img src=x onerror=alert('this should never get shown') /> fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies mi vitae est. Mauris placerat eleifend leo.</p>`}
/>
<WysiwygEditor
label="Read Only WYSIWYG Editor"
html="<p>Pellentesque habitant <strong>morbi tristique senectus</strong> <img src=x onerror=alert('this should never get shown') /> et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies mi vitae est. Mauris placerat eleifend leo.</p>"
readOnly
/>
Comment on lines +154 to +162
Copy link
Collaborator

@nsmedira nsmedira Jan 6, 2023

Choose a reason for hiding this comment

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

would the intent of this code (i.e. presumably to demonstrate that alert('this should never get shown') is never executed) be better expressed as a jest test?

e.g. pseudo

describe('WysiwygEditor', () => {
    test('html prop is sanitized', () => {
        render(<WysiwygEditor html={<img src=x onerror=alert('this should never get shown') />)

        expect(wysiwygEditorDiv).toHaveTextContent('');
    })
})

Copy link
Member Author

@cwardzala cwardzala Jan 6, 2023

Choose a reason for hiding this comment

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

yes, 100%


<h3>Checkbox</h3>
<hr />
<Checkbox label="Checkbox" id="checkbox_1" />
Expand Down
20 changes: 20 additions & 0 deletions src/scss/components/_forms.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,23 @@ label.error,
height: auto;
padding: rem(2) 0;
}

// React Quill
.wysiwyg-editor,
.quill {
background: $white;
.ql-toolbar {
border-top-left-radius: $border-radius;
border-top-right-radius: $border-radius;
border-color: $border-color;
}

.ql-container {
font-size: 1rem;
border-bottom-left-radius: $border-radius;
border-bottom-right-radius: $border-radius;
border-color: $border-color;
}
}


Loading