Skip to content

Commit 4ffe492

Browse files
committed
Improve validation handling
1 parent 43c748e commit 4ffe492

File tree

2 files changed

+47
-38
lines changed

2 files changed

+47
-38
lines changed

application/forms/Config/General/ApplicationConfigForm.php

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Icinga\Web\Form;
1010
use Icinga\Util\Csp;
1111
use Icinga\Web\Notification;
12+
use Zend_Validate_Callback;
1213

1314
/**
1415
* Configuration form for general application options
@@ -63,6 +64,7 @@ public function createElements(array $formData)
6364
'security_use_strict_csp',
6465
[
6566
'label' => $this->translate('Enable strict content security policy'),
67+
'autosubmit' => true,
6668
'description' => $this->translate(
6769
'Set whether to use strict content security policy (CSP).'
6870
. ' This setting helps to protect from cross-site scripting (XSS).'
@@ -71,21 +73,51 @@ public function createElements(array $formData)
7173
);
7274

7375
$securityImageSourceWhitelistHidden =
74-
!(boolean) Icinga::app()->getConfig()->get("security", "use_strict_csp", false);
75-
$securityImageSourceWhitelistHiddenClass = $securityImageSourceWhitelistHidden ? 'input-hidden' : '';
76-
$this->addElement(
77-
'text',
78-
'security_image_source_whitelist',
79-
[
80-
'label' => $this->translate('Domains whitelist for the CSP image source'),
81-
'placeholder' => $this->translate('No external domains whitelisted'),
82-
'class' => $securityImageSourceWhitelistHiddenClass,
83-
'description' => $this->translate(
84-
'This whitelist allows you to specify a comma separated list of trusted hosts, '
85-
. 'from which images for the icingaweb2 UI can be loaded.'
86-
)
87-
]
88-
);
76+
! (boolean) Icinga::app()->getConfig()->get("security", "use_strict_csp", false)
77+
&& ! $formData['security_use_strict_csp'];
78+
79+
if (! $securityImageSourceWhitelistHidden) {
80+
$callbackValidator = new Zend_Validate_Callback(function ($value) {
81+
foreach (explode(",", $value) as $imageSource) {
82+
try {
83+
Csp::validateImageSourceWhitelistItem($imageSource);
84+
}catch (SecurityException $e) {
85+
Notification::error($e->getMessage());
86+
return false;
87+
}
88+
}
89+
90+
return true;
91+
});
92+
93+
$this->addElement(
94+
'text',
95+
'security_image_source_whitelist',
96+
[
97+
'label' => $this->translate('Domains whitelist for the CSP image source'),
98+
'placeholder' => $this->translate('No external domains whitelisted'),
99+
'validators' => [$callbackValidator],
100+
'description' => $this->translate(
101+
'This whitelist allows you to specify a comma separated list of trusted hosts, '
102+
. 'from which images for the icingaweb2 UI can be loaded.'
103+
)
104+
]
105+
);
106+
} else {
107+
$this->addElement(
108+
'hidden',
109+
'security_image_source_whitelist'
110+
[
111+
'validators' => [new Zend_Validate_Callback(function ($value) {
112+
foreach (explode(",", $value) as $imageSource) {
113+
// This can only be triggered by a malicious actor.
114+
// If any url is invalid, it will throw an expection.
115+
Csp::validateImageSourceWhitelistItem($imageSource);
116+
}
117+
})]
118+
]
119+
);
120+
}
89121

90122
$this->addElement(
91123
'text',
@@ -122,23 +154,4 @@ public function createElements(array $formData)
122154

123155
return $this;
124156
}
125-
126-
/**
127-
* @inheritDoc
128-
*/
129-
public function isValid($formData)
130-
{
131-
$imageSourceWhitelist = $formData["security_image_source_whitelist"] ?? "";
132-
$valid = true;
133-
foreach (explode(",", $imageSourceWhitelist) as $imageSource) {
134-
try {
135-
Csp::validateImageSourceWhitelistItem($imageSource);
136-
}catch (SecurityException $e) {
137-
Notification::error($e->getMessage());
138-
$valid = false;
139-
}
140-
}
141-
142-
return $valid && parent::isValid($formData);
143-
}
144157
}

public/css/icinga/forms.less

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ form.icinga-form {
3333
margin-bottom: 0;
3434
}
3535
}
36-
37-
&:has(.input-hidden) {
38-
display: none;
39-
}
4036
}
4137

4238
.control-group > :not(.control-label-group) {

0 commit comments

Comments
 (0)