Fix config form value & Simplify CspConfigForm

application/controllers/ConfigController.php

@@ -8,6 +8,7 @@
 use Exception;
 use GuzzleHttp\Psr7\ServerRequest;
 use Icinga\Application\Version;
+use Icinga\Web\Session;
 use InvalidArgumentException;
 use Icinga\Application\Config;
 use Icinga\Application\Icinga;
@@ -132,24 +133,14 @@ public function securityAction(): void
 
         $this->view->title = $this->translate('Security');
 
-        $config = Config::app();
-        $cspForm = new CspConfigForm($config);
-        // Keep the auto-generation options off by default for installations that already
-        // had CSP enabled, so their existing behavior isn't changed. For installations
-        // enabling CSP for the first time, default them on as the recommended setting.
-        $defaultValue = $cspForm->isCspEnabled() ? '0' : '1';
-        $cspForm->populate([
-            'security__csp_enable_modules'    => $config->get('security', 'csp_enable_modules', $defaultValue),
-            'security__csp_enable_dashboards' => $config->get('security', 'csp_enable_dashboards', $defaultValue),
-            'security__csp_enable_navigation' => $config->get('security', 'csp_enable_navigation', $defaultValue),
-        ]);
-
-        $cspForm->on(ContractForm::ON_SUBMIT, function (CspConfigForm $form) {
-            if ($form->hasConfigChanged()) {
-                $this->getResponse()->setReloadWindow(true);
-            }
-        });
-        $cspForm->handleRequest(ServerRequest::fromGlobals());
+        $cspForm = (new CspConfigForm(Config::app()))
+            ->setCsrfCounterMeasureId(Session::getSession()->getId())
+            ->on(ContractForm::ON_SUBMIT, function (CspConfigForm $form) {
+                if ($form->hasConfigChanged()) {
+                    $this->getResponse()->setReloadWindow(true);
+                }
+            })
+            ->handleRequest(ServerRequest::fromGlobals());
         $this->view->cspForm = $cspForm;
         $this->createApplicationTabs()->activate('security');
     }

application/forms/Config/Security/CspConfigForm.php

@@ -17,12 +17,12 @@
 use Icinga\Security\Csp\Reason\StaticCspReason;
 use Icinga\Util\Csp;
 use Icinga\Web\Form\ConfigForm;
-use Icinga\Web\Session;
 use ipl\Html\Attributes;
 use ipl\Html\BaseHtmlElement;
 use ipl\Html\HtmlElement;
 use ipl\Html\Table;
 use ipl\Html\Text;
+use ipl\Stdlib\Str;
 use ipl\Validator\CallbackValidator;
 use ipl\Web\Common\CalloutType;
 use ipl\Web\Common\Csp as CspInstance;
@@ -130,7 +130,7 @@ protected function assemble(): void
 
         $this->addElement($this->createUidElement());
 
-        $this->addCsrfCounterMeasure(Session::getSession()->getId());
+        $this->addCsrfCounterMeasure();
 
         $this->addElement('checkbox', 'security__use_strict_csp', [
             'label'          => $this->translate('Send CSP header'),
@@ -194,7 +194,7 @@ function (StaticCspReason $reason, string $directive, string $expression) {
                     'Should module defined CSP directives be enabled?'
                     . ' Modules can define or change csp directives at any point.',
                 ),
-                'security__csp_enable_modules',
+                'csp_enable_modules',
                 ! $useCustomCsp,
             );
 
@@ -219,7 +219,7 @@ function (ModuleCspReason $reason, string $directive, string $expression) {
                     . ' header that is sent to the user will only contain the subset of directives that actually'
                     . ' matters to them.',
                 ),
-                'security__csp_enable_dashboards',
+                'csp_enable_dashboards',
                 ! $useCustomCsp,
             );
 
@@ -252,7 +252,7 @@ function (DashboardCspReason $reason, string $directive, string $expression) {
                     . ' all users. The actual header that is sent to the user will only contain the subset of'
                     . ' directives that actually matters to them.',
                 ),
-                'security__csp_enable_navigation',
+                'csp_enable_navigation',
                 ! $useCustomCsp,
             );
 
@@ -307,6 +307,7 @@ function (NavigationCspReason $reason, string $directive, string $expression) {
                     'class'          => 'autosubmit csp-form-content-aligned csp-label-header-h3 csp-form-header',
                     'checkedValue'   => '1',
                     'uncheckedValue' => '0',
+                    'value'          => '0',
                 ],
             );
 
@@ -399,15 +400,15 @@ public function isCustomCspEnabled(): bool
      *
      * @param string $label The label of the checkbox
      * @param string $description The description of the checkbox
-     * @param string $field The name of the checkbox field
+     * @param string $key The key of the checkbox field in the config
      * @param bool $enabled Whether the checkbox should be checked and enabled
      *
      * @return void
      */
     protected function addDirectiveCheckboxElement(
         string $label,
         string $description,
-        string $field,
+        string $key,
         bool $enabled,
     ): void {
         $classList = [
@@ -420,14 +421,24 @@ protected function addDirectiveCheckboxElement(
             $classList[] = 'csp-disabled';
         }
 
+        $field = 'security' . $this->sectionKeyDelimiter . $key;
+
+        // Keep the auto-generation options off by default for installations that already
+        // had CSP enabled, so their existing behavior isn't changed. For installations
+        // enabling CSP for the first time, default them on as the recommended setting.
+        $cspWasEnabled = $this->config->get('security', 'use_strict_csp') === '1';
+        $defaultValue = $cspWasEnabled ? '0' : '1';
+        if (Str::isEmpty($this->getPopulatedValue($field)) && $this->config->get('security', $key) === null) {
+            $this->populate([$field => $defaultValue]);
+        }
+
         $this->addElement('checkbox', $field, [
             'label'          => $label,
             'description'    => $description,
             'class'          => $classList,
             'checkedValue'   => '1',
             'uncheckedValue' => '0',
             'disabled'       => ! $enabled,
-            'value'          => $this->getPopulatedValue($field),
         ]);
     }
 

library/Icinga/Web/Form/ConfigForm.php

@@ -8,14 +8,14 @@
 use Exception;
 use Icinga\Application\Config;
 use ipl\Html\Contract\FormElement;
-use ipl\Html\Contract\ValueCandidates;
 use ipl\Html\HtmlElement;
 use ipl\Html\ValidHtml;
 use ipl\Stdlib\Str;
 use ipl\Web\Compat\CompatForm;
 use ipl\Web\Compat\DisplayFormElement;
 use ipl\Web\Widget\CopyToClipboard;
 use LogicException;
+use stdClass;
 
 /**
  * Form base-class providing standard functionality for configuration forms
@@ -42,6 +42,16 @@ class ConfigForm extends CompatForm
      */
     protected string $sectionKeyDelimiter = '__';
 
+    /**
+     * The original values of the form elements
+     *
+     * This is used to determine whether an element's submitted value differs from
+     * the original value.
+     *
+     * @var array<string, mixed>
+     */
+    protected array $originalValues = [];
+
     /**
      * Create a new configuration form
      *
@@ -50,34 +60,30 @@ class ConfigForm extends CompatForm
     public function __construct(
         protected Config $config,
     ) {
-        $this->on(static::ON_ELEMENT_REGISTERED, function (FormElement $element) {
-            [$section, $key] = Str::symmetricSplit($element->getName(), $this->sectionKeyDelimiter, 2);
-            if ($key === null || $element->hasValue()) {
-                return;
-            }
-
-            $configValue = $this->config->get($section, $key);
-            if ($configValue === null) {
-                return;
-            }
-
-            if (! ($element instanceof ValueCandidates)) {
-                $element->setValue($configValue);
+    }
 
-                return;
+    /**
+     * Register the given element, seeding it with its configuration value first
+     *
+     * @param FormElement $element
+     *
+     * @return $this
+     */
+    public function registerElement(FormElement $element): static
+    {
+        $name = $element->getName();
+        [$section, $key] = Str::symmetricSplit($name, $this->sectionKeyDelimiter, 2);
+        if ($key !== null) {
+            $this->originalValues[$name] = $element->getValue();
+
+            $sentinel = new stdClass();
+            $configValue = $this->config->get($section, $key, $this->originalValues[$name] ?? $sentinel);
+            if ($configValue !== $sentinel) {
+                $this->populatedValues[$name] = array_merge([$configValue], $this->populatedValues[$name] ?? []);
             }
+        }
 
-            $candidates = $element->getValueCandidates();
-            if (empty($candidates)) {
-                // Initial render: no prior submission, set value and seed candidates
-                $element->setValue($configValue);
-                $element->setValueCandidates([$configValue]);
-            } else {
-                // POST: candidates were set by registerElement; append config value so
-                // PasswordElement's (count > 1) condition resolves DUMMYPASSWORD on re-render
-                $element->setValueCandidates(array_merge($candidates, [$configValue]));
-            }
-        });
+        return parent::registerElement($element);
     }
 
     /**
@@ -160,8 +166,11 @@ protected function save(): void
                 throw new LogicException(sprintf('Cannot save element "%s": array values are not supported', $element));
             }
 
+            $originalValue = $this->originalValues[$element] ?? '';
             $configSection = $this->config->getSection($section);
-            if (Str::isEmpty($value)) {
+            if ($originalValue !== '' && Str::isEmpty($value)) {
+                $configSection[$key] = '';
+            } elseif (Str::isEmpty($value) || $originalValue === $value) {
                 unset($configSection[$key]);
             } else {
                 $configSection[$key] = $value;

test/php/application/forms/Config/Security/CspConfigFormTest.php

@@ -0,0 +1,73 @@
+<?php
+
+// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+namespace Tests\Icinga\Forms\Config\Security;
+
+use Icinga\Application\Config;
+use Icinga\Forms\Config\Security\CspConfigForm;
+use Icinga\Test\BaseTestCase;
+
+class CspConfigFormTest extends BaseTestCase
+{
+    public function testDirectiveCheckboxesDefaultOnWhenStrictCspIsUnconfigured(): void
+    {
+        $this->assertDirectiveDefaults([], '1');
+    }
+
+    public function testDirectiveCheckboxesDefaultOnWhenStrictCspWasDisabled(): void
+    {
+        $this->assertDirectiveDefaults(['security' => ['use_strict_csp' => '0']], '1');
+    }
+
+    public function testDirectiveCheckboxesDefaultOffWhenStrictCspWasEnabled(): void
+    {
+        $this->assertDirectiveDefaults(['security' => ['use_strict_csp' => '1']], '0');
+    }
+
+    public function testConfiguredDirectiveCheckboxValuesOverrideDefaults(): void
+    {
+        $form = $this->createDirectiveForm([
+            'security' => [
+                'use_strict_csp'        => '1',
+                'csp_enable_modules'    => '1',
+                'csp_enable_dashboards' => '0',
+                'csp_enable_navigation' => '1',
+            ],
+        ]);
+
+        $this->assertSame('1', $form->getValue('security__csp_enable_modules'));
+        $this->assertSame('0', $form->getValue('security__csp_enable_dashboards'));
+        $this->assertSame('1', $form->getValue('security__csp_enable_navigation'));
+    }
+
+    private function assertDirectiveDefaults(array $configData, string $expected): void
+    {
+        $form = $this->createDirectiveForm($configData);
+
+        foreach (['csp_enable_modules', 'csp_enable_dashboards', 'csp_enable_navigation'] as $key) {
+            $this->assertSame($expected, $form->getValue('security__' . $key), $key);
+        }
+    }
+
+    private function createDirectiveForm(array $configData): CspConfigForm
+    {
+        $form = new class (Config::fromArray($configData)) extends CspConfigForm {
+            protected function assemble(): void
+            {
+            }
+
+            public function addDirectiveCheckboxForTest(string $key): void
+            {
+                $this->addDirectiveCheckboxElement($key, $key, $key, true);
+            }
+        };
+
+        foreach (['csp_enable_modules', 'csp_enable_dashboards', 'csp_enable_navigation'] as $key) {
+            $form->addDirectiveCheckboxForTest($key);
+        }
+
+        return $form;
+    }
+}