Skip to content
Jump to bottom

Sanitize HTML output and increase size of Body textarea input fields (CFM-62) #293

Merged
merged 2 commits into from Feb 13, 2025
+61 −20
Merged

Sanitize HTML output and increase size of Body textarea input fields (CFM-62) #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f7c91b
Sanitize HTML output and increase size of Body textarea input fields …
arlen Feb 12, 2025
0d8b841
Ensure Mostly Static Page body is wrapped with a default div element …
arlen Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion app/src/Controller/PagesController.php
View file
Open in desktop
Expand Up @@ -23,6 +23,8 @@
use Cake\ORM\TableRegistry;
use Cake\View\Exception\MissingTemplateException;
use \App\Lib\Enum\SuspendableStatusEnum;
use Symfony\Component\HtmlSanitizer\HtmlSanitizer;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;

/**
* Static content controller
Expand Down Expand Up @@ -130,7 +132,17 @@ public function show(string $coid, string $name) {
$this->set('vv_bc_skip', true); // this doesn't do anything?
$this->set('vv_title', $msp->title);
$this->set('vv_body', $msp->body);

// Mostly Static Pages allow HTML input. Pass this through the Symfony HTML Sanitizer to
// disallow dom elements like <script> and <style>.
// XXX We may need to write a plugin (as in v4) if we want to allow <style> tags
$htmlSanitizer = new HtmlSanitizer(
// Allow all elements from the W3C Sanitizer API. This is more permissive than "allowSafeElements()".
// See: https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/HtmlSanitizer/Reference/W3CReference.php
(new HtmlSanitizerConfig())->allowStaticElements()
);
$sanitizedBody = $htmlSanitizer->sanitize($msp->body);
$this->set('vv_body', $sanitizedBody);

return $this->render('/MostlyStaticPages/display');
}
Expand Down
9 changes: 3 additions & 6 deletions app/src/Lib/Traits/ValidationTrait.php
View file
Open in desktop
Expand Up @@ -33,8 +33,6 @@
use Cake\Database\Schema\TableSchemaInterface;
use Cake\ORM\TableRegistry;
use Cake\Validation\Validator;
use Symfony\Component\HtmlSanitizer\HtmlSanitizer;
use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;

trait ValidationTrait {
/**
Expand Down Expand Up @@ -241,11 +239,10 @@ public function validateInput($value, array $context) {
switch($context['type']) {
case 'html':
// We are accepting HTML input. We will mostly pass it all through and ensure
// properly sanitized output. However, to help warn users about entering tags that
// will be stripped, we can do some very rudimentary checking for script and
// style tags. (An informational note should be placed below these fields as well.)
// properly sanitized output. However, we can do some very rudimentary checking for script tags.
// (An informational note should be placed below these fields as well.)
$lowercaseVal = strtolower($value);
if(str_contains($lowercaseVal, '<script') || str_contains($lowercaseVal, '<style')) {
if(str_contains($lowercaseVal, '<script')) {
// Disallowed HTML is in the input, so warn the user.
return __d('error', 'input.invalid.html');
}
Expand Down
28 changes: 18 additions & 10 deletions app/templates/MessageTemplates/fields.inc
View file
Open in desktop
Expand Up @@ -72,15 +72,20 @@ if($vv_action == 'add' || $vv_action == 'edit') {
]
]);

foreach ([
'subject',
'body_text'
] as $field) {
print $this->element('form/listItem', [
'arguments' => [
'fieldName' => $field
]]);
}
print $this->element('form/listItem', [
'arguments' => [
'fieldName' => 'subject'
]
]);

print $this->element('form/listItem', [
'arguments' => [
'fieldName' => 'body_text',
'fieldOptions' => [
'class' => 'big-textarea'
]
]
]);

$afterField =
'<div class="alert alert-info small p-2 d-flex gap-1" role="alert">' .
Expand All @@ -90,7 +95,10 @@ if($vv_action == 'add' || $vv_action == 'edit') {

print $this->element('form/listItem', [
'arguments' => [
'fieldName' => 'body_html'
'fieldName' => 'body_html',
'fieldOptions' => [
'class' => 'big-textarea'
]
],
'afterField' => $afterField
]);
Expand Down
5 changes: 4 additions & 1 deletion app/templates/MostlyStaticPages/display.php
View file
Open in desktop
Expand Up @@ -51,4 +51,7 @@
<?php endif; // $banners ?>
</div>

<?= $vv_body ?>
<div class="page-body">
<?= $vv_body ?>
</div>

19 changes: 17 additions & 2 deletions app/templates/MostlyStaticPages/fields.inc
View file
Open in desktop
Expand Up @@ -72,12 +72,27 @@ if($vv_action == 'add' || $vv_action == 'edit') {
foreach ([
'description',
'status',
'context',
'body'
'context'
] as $field) {
print $this->element('form/listItem', [
'arguments' => [
'fieldName' => $field
]]);
}

$afterField =
Copy link
Contributor

@Ioannis Ioannis Feb 13, 2025 •
edited

Choose a reason for hiding this comment

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

Why don't you make the afterField a simple element? If this is specific to the MostlyStaticPages, then the element could be under the folder mostlyStaticPages and be named as afterField.php. Since this is static, we can also cache it. The way we do with other static elements as well.
Currenlty we have several static elements already. The footer, form/requiredSpan, notSetDiv, etc. We could also create a directory call staticElements and move all of these element under that directory.

      // Add a hidden field
      $element = $this->getView()->element('form/notSetDiv', [], [
        'cache' => '_html_elements',
      ]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not the purpose of afterField and beforeField: the purpose of these is to supply arbitrary markup before or after any field. This allows for arbitrary dom elements (for display) and arbitrary JavaScript (for interactivity). This provides flexibility that elements don't provide on a per-field basis.

(Note: these features need to get into the documentation.)

'<div class="alert alert-info small p-2 d-flex gap-1" role="alert">' .
' <em class="material-symbols-outlined" aria-hidden="true">info</em>' .
' <span class="alert-text">' . __d('information','html.sanitization') . '</span>' .
'</div>';

print $this->element('form/listItem', [
'arguments' => [
'fieldName' => 'body',
'fieldOptions' => [
'class' => 'big-textarea'
]
],
'afterField' => $afterField
]);
}
6 changes: 6 additions & 0 deletions app/webroot/css/co-base.css
View file
Open in desktop
Expand Up @@ -1618,6 +1618,9 @@ ul.form-list textarea {
border: 1px solid var(--cmg-color-bg-007);
padding: 0.5em;
}
ul.form-list textarea.big-textarea {
height: 24em;
}
ul.form-list li.field-stack textarea {
margin: 0;
width: 100%;
Expand Down Expand Up @@ -2185,6 +2188,9 @@ body.pages.logged-out #breadcrumbs {
body.pages.logged-out .page-title-container {
margin: 0;
}
.page-body {
margin-top: 1em;
}
/* GENERAL */
.hidden,
.invisible,
Expand Down