fix(security): empty access_roles deny and CSRF fails closed

composer.json

@@ -45,6 +45,7 @@
         "symfony/http-kernel": "^7.4 || ^8.0",
         "symfony/routing": "^7.4 || ^8.0",
         "symfony/security-bundle": "^7.4 || ^8.0",
+        "symfony/security-csrf": "^7.4 || ^8.0",
         "symfony/translation": "^7.4 || ^8.0",
         "symfony/twig-bundle": "^7.4 || ^8.0",
         "symfony/yaml": "^7.4 || ^8.0",

docs/CHANGELOG.md

@@ -28,6 +28,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- **Empty role list denies (P1):** `ConfigurablePageLayoutKitAccessChecker` returned `true` for `security.access_roles: []`,
+  opening the layout admin and block edit endpoints to every user reaching them. An empty list now grants nothing, and
+  `NowoPageLayoutKitExtension` throws a `LogicException` at compile time when `access_roles` is empty while
+  `allow_unauthenticated` is `false` and no `access_checker` is set.
+- **CSRF fails closed (P2):** new `CsrfProtectionPass` refuses to compile without `security.csrf.token_manager` and
+  `form.type_extension.csrf`, or with `framework.form.csrf_protection: false`. Before, block reorder and block edit
+  accepted forged POSTs on such hosts. `symfony/security-csrf` is now a required dependency.
+  See [UPGRADING](UPGRADING.md#unreleased).
+
 ## [1.7.0] - 2026-10-22
 
 Dependency refresh, logged legacy page-block migration (psr/log), demo login via Symfony form and raw-form release gate. See [UPGRADING](UPGRADING.md#to-170).

docs/CONFIGURATION.md

@@ -63,12 +63,15 @@ Sanitization runs on Doctrine persist/update for text/compare/cta block translat
 
 | Key | Default | Description |
 | --- | --- | --- |
-| `access_roles` | `[ROLE_EDITOR]` | Any matching role grants admin access when no custom checker is configured. |
+| `access_roles` | `[ROLE_EDITOR]` | Any matching role grants admin access when no custom checker is configured. An empty list **denies** (never "everyone") and fails container compilation unless `access_checker` or `allow_unauthenticated` is set. |
 | `access_checker` | `null` | Optional service id implementing `PageLayoutKitAccessCheckerInterface`. |
 | `allow_unauthenticated` | `false` | When `true`, the bundle uses an allow-all checker. Intended only for trusted demos or internal tools. |
 
 The bundle enforces access on route names beginning with `admin_page_layout` and `admin_page_blocks`.
 
+The container also refuses to compile without form CSRF protection (`security.csrf.token_manager` +
+`form.type_extension.csrf`, `framework.form.csrf_protection` not `false`): block reorder and block edit rely on it.
+
 ## web_ui
 
 | Key | Default | Description |

docs/SECURITY.md

@@ -19,8 +19,9 @@ Security considerations for page layout editing, inline CMS modals, and public b
 | Risk | Mitigation |
 | --- | --- |
 | Unauthorized editors reach layout admin or block edit endpoints | `access_roles`, custom `access_checker`, and Symfony Security |
+| Misconfigured empty `access_roles` opens the admin to every user | An empty list grants nothing (`ConfigurablePageLayoutKitAccessChecker`); `access_roles: []` without `access_checker` / `allow_unauthenticated` → container compile error (`NowoPageLayoutKitExtension`) |
 | Stored XSS through rich text block content | Default Twig auto-escaping where possible; review templates that intentionally use `|raw` |
-| CSRF on reorder or modal save actions | Symfony forms and CSRF-protected admin flows |
+| CSRF on reorder or modal save actions | Symfony forms with CSRF tokens; `CsrfProtectionPass` fails compilation (fail closed) when `security.csrf.token_manager` / `form.type_extension.csrf` is missing or `framework.form.csrf_protection` is false |
 | Overly broad demo access | `allow_unauthenticated` defaults to `false`; enable only for trusted demos |
 | Shared-database table collisions | Optional `doctrine.table_prefix` |
 
@@ -59,7 +60,7 @@ The reorder form and modal update forms are submitted through Symfony forms, not
 Recommendations:
 
 - Do not expose admin routes outside authenticated editor sessions.
-- Do not remove CSRF handling from overridden forms.
+- Do not remove CSRF handling from overridden forms. The bundle refuses to compile when form CSRF protection is off (`DependencyInjection\Compiler\CsrfProtectionPass`).
 - Keep modal edit endpoints behind the same editor policy as the reorder UI.
 
 ## Rich text rendering

docs/UPGRADING.md

@@ -23,6 +23,19 @@
 
 ## Unreleased
 
+```bash
+composer update nowo-tech/page-layout-kit-bundle
+```
+
+**Behaviour changes (security):**
+
+- `nowo_page_layout_kit.security.access_roles: []` used to allow everyone; it now fails container compilation (and
+  an empty list denies). Configure a role, set `security.access_checker`, or — trusted demos only —
+  `security.allow_unauthenticated: true`.
+- The container no longer compiles without form CSRF protection. Keep `framework.csrf_protection` and
+  `framework.form.csrf_protection` enabled; `symfony/security-csrf` is now a required dependency.
+- Default configuration (`[ROLE_EDITOR]`, CSRF on) is unaffected.
+
 ## To 1.7.0
 
 From **1.6.1**:

src/DependencyInjection/Compiler/CsrfProtectionPass.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Nowo\PageLayoutKitBundle\DependencyInjection\Compiler;
+
+use LogicException;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * Fails closed when the layout admin would run without form CSRF protection (REQ-SEC-005).
+ *
+ * Block reorder and block edit are validated through Symfony forms whose types define their own CSRF options
+ * (`PageLayoutReorderType`, block edit forms). A host that disables `framework.form.csrf_protection` (or has no CSRF
+ * token manager) would silently accept forged POSTs, so the container refuses to compile instead.
+ */
+final class CsrfProtectionPass implements CompilerPassInterface
+{
+    /**
+     * Throws when the bundle is loaded and form CSRF protection or the CSRF token manager is unavailable.
+     */
+    public function process(ContainerBuilder $container): void
+    {
+        if (!$container->hasParameter('nowo_page_layout_kit.security.access_roles')) {
+            return;
+        }
+
+        $formCsrfDisabled = $container->hasParameter('form.type_extension.csrf.enabled')
+            && $container->getParameter('form.type_extension.csrf.enabled') === false;
+
+        if (
+            !$container->has('security.csrf.token_manager')
+            || !$container->hasDefinition('form.type_extension.csrf')
+            || $formCsrfDisabled
+        ) {
+            throw new LogicException('NowoPageLayoutKitBundle: the layout admin requires CSRF protection. Enable framework.csrf_protection and framework.form.csrf_protection (composer require symfony/security-csrf).');
+        }
+    }
+}

src/DependencyInjection/Configuration.php

@@ -51,6 +51,7 @@ public function getConfigTreeBuilder(): TreeBuilder
                         ->arrayNode('access_roles')
                             ->scalarPrototype()->end()
                             ->defaultValue(['ROLE_EDITOR'])
+                            ->info('Roles allowed to use the layout admin (user needs at least one). Empty list → grants nothing; fails container compilation unless access_checker or allow_unauthenticated is set.')
                         ->end()
                         ->scalarNode('access_checker')->defaultNull()->end()
                         ->booleanNode('allow_unauthenticated')->defaultFalse()->end()

src/DependencyInjection/NowoPageLayoutKitExtension.php

@@ -131,6 +131,10 @@ private function registerAccessChecker(ContainerBuilder $container, array $secur
             return;
         }
 
+        if ($security['access_roles'] === []) {
+            throw new LogicException('NowoPageLayoutKitBundle: security.access_roles is empty, so nobody could use the admin UI (an empty list denies). Configure at least one role, set security.access_checker, or set security.allow_unauthenticated for a trusted demo.');
+        }
+
         $id         = 'nowo_page_layout_kit.access_checker.default';
         $definition = new Definition(ConfigurablePageLayoutKitAccessChecker::class);
         $definition->setArgument('$accessRoles', $security['access_roles']);

src/NowoPageLayoutKitBundle.php

@@ -5,6 +5,7 @@
 namespace Nowo\PageLayoutKitBundle;
 
 use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\DoctrineOrmMappingsPass;
+use Nowo\PageLayoutKitBundle\DependencyInjection\Compiler\CsrfProtectionPass;
 use Nowo\PageLayoutKitBundle\DependencyInjection\Compiler\TwigPathsPass;
 use Nowo\PageLayoutKitBundle\DependencyInjection\NowoPageLayoutKitExtension;
 use Nowo\PageLayoutKitBundle\Locale\PageLocales;
@@ -19,6 +20,7 @@ class NowoPageLayoutKitBundle extends Bundle
 {
     public function build(ContainerBuilder $container): void
     {
+        $container->addCompilerPass(new CsrfProtectionPass());
         $container->addCompilerPass(new TwigPathsPass());
 
         $entityDir = __DIR__ . '/Entity';

src/Security/ConfigurablePageLayoutKitAccessChecker.php

@@ -6,6 +6,11 @@
 
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
 
+/**
+ * Role-based access driven by nowo_page_layout_kit.security.access_roles.
+ *
+ * An empty role list grants nothing (fail closed); the extension refuses to compile that configuration.
+ */
 final readonly class ConfigurablePageLayoutKitAccessChecker implements PageLayoutKitAccessCheckerInterface
 {
     /**
@@ -19,16 +24,6 @@ public function __construct(
 
     public function canAccess(): bool
     {
-        if ($this->accessRoles === []) {
-            return true;
-        }
-
-        foreach ($this->accessRoles as $role) {
-            if ($this->authorizationChecker->isGranted($role)) {
-                return true;
-            }
-        }
-
-        return false;
+        return array_any($this->accessRoles, fn (string $role): bool => $this->authorizationChecker->isGranted($role));
     }
 }

tests/Unit/DependencyInjection/Compiler/CsrfProtectionPassTest.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Nowo\PageLayoutKitBundle\Tests\Unit\DependencyInjection\Compiler;
+
+use LogicException;
+use Nowo\PageLayoutKitBundle\DependencyInjection\Compiler\CsrfProtectionPass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+
+final class CsrfProtectionPassTest extends TestCase
+{
+    public function testNoopWithoutBundleParameter(): void
+    {
+        $container = new ContainerBuilder();
+
+        (new CsrfProtectionPass())->process($container);
+
+        self::assertFalse($container->has('security.csrf.token_manager'));
+    }
+
+    public function testPassesWithTokenManagerAndFormExtension(): void
+    {
+        $container = $this->protectedContainer();
+        $container->setParameter('form.type_extension.csrf.enabled', true);
+
+        (new CsrfProtectionPass())->process($container);
+
+        self::assertTrue($container->hasDefinition('form.type_extension.csrf'));
+    }
+
+    public function testFailsWithoutTokenManager(): void
+    {
+        $container = $this->protectedContainer();
+        $container->removeDefinition('security.csrf.token_manager');
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage('requires CSRF protection');
+
+        (new CsrfProtectionPass())->process($container);
+    }
+
+    public function testFailsWithoutFormCsrfExtension(): void
+    {
+        $container = $this->protectedContainer();
+        $container->removeDefinition('form.type_extension.csrf');
+
+        $this->expectException(LogicException::class);
+
+        (new CsrfProtectionPass())->process($container);
+    }
+
+    public function testFailsWhenFormCsrfDisabled(): void
+    {
+        $container = $this->protectedContainer();
+        $container->setParameter('form.type_extension.csrf.enabled', false);
+
+        $this->expectException(LogicException::class);
+
+        (new CsrfProtectionPass())->process($container);
+    }
+
+    private function protectedContainer(): ContainerBuilder
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('nowo_page_layout_kit.security.access_roles', ['ROLE_EDITOR']);
+        $container->setDefinition('security.csrf.token_manager', new Definition());
+        $container->setDefinition('form.type_extension.csrf', new Definition());
+
+        return $container;
+    }
+}

tests/Unit/DependencyInjection/NowoPageLayoutKitExtensionTest.php

@@ -192,6 +192,35 @@ public function testLoadUsesCustomAccessCheckerAliasWhenConfigured(): void
         self::assertFalse($container->hasDefinition(TablePrefixListener::class));
     }
 
+    public function testLoadRejectsEmptyAccessRoles(): void
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('kernel.bundles', [
+            'SecurityBundle' => SecurityBundle::class,
+        ]);
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage('security.access_roles is empty');
+
+        (new NowoPageLayoutKitExtension())->load([
+            ['security' => ['allow_unauthenticated' => false, 'access_roles' => []]],
+        ], $container);
+    }
+
+    public function testLoadAcceptsEmptyAccessRolesWithCustomChecker(): void
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('kernel.bundles', [
+            'SecurityBundle' => SecurityBundle::class,
+        ]);
+
+        (new NowoPageLayoutKitExtension())->load([
+            ['security' => ['access_roles' => [], 'access_checker' => 'app.page_layout_checker']],
+        ], $container);
+
+        self::assertSame('app.page_layout_checker', (string) $container->getAlias(PageLayoutKitAccessCheckerInterface::class));
+    }
+
     public function testLoadRegistersDefaultConfigurableAccessCheckerWhenNeeded(): void
     {
         $container = new ContainerBuilder();

tests/Unit/NowoPageLayoutKitBundleTest.php

@@ -5,6 +5,7 @@
 namespace Nowo\PageLayoutKitBundle\Tests\Unit;
 
 use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\DoctrineOrmMappingsPass;
+use Nowo\PageLayoutKitBundle\DependencyInjection\Compiler\CsrfProtectionPass;
 use Nowo\PageLayoutKitBundle\DependencyInjection\Compiler\TwigPathsPass;
 use Nowo\PageLayoutKitBundle\DependencyInjection\NowoPageLayoutKitExtension;
 use Nowo\PageLayoutKitBundle\Locale\PageLocales;
@@ -25,6 +26,7 @@ public function testBuildRegistersTwigAndDoctrineCompilerPasses(): void
 
         $passes = $container->getCompilerPassConfig()->getPasses();
         self::assertNotEmpty(array_filter($passes, static fn (object $pass): bool => $pass instanceof TwigPathsPass));
+        self::assertNotEmpty(array_filter($passes, static fn (object $pass): bool => $pass instanceof CsrfProtectionPass));
         self::assertNotEmpty(array_filter($passes, static fn (object $pass): bool => $pass instanceof DoctrineOrmMappingsPass));
     }
 

tests/Unit/Security/PageLayoutKitAccessCheckerTest.php

@@ -16,14 +16,14 @@ public function testAllowAllCheckerAlwaysReturnsTrue(): void
         self::assertTrue((new AllowAllPageLayoutKitAccessChecker())->canAccess());
     }
 
-    public function testConfigurableCheckerAllowsWhenNoRolesConfigured(): void
+    public function testConfigurableCheckerDeniesWhenNoRolesConfigured(): void
     {
         $authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class);
         $authorizationChecker->expects(self::never())->method('isGranted');
 
         $checker = new ConfigurablePageLayoutKitAccessChecker($authorizationChecker, []);
 
-        self::assertTrue($checker->canAccess());
+        self::assertFalse($checker->canAccess());
     }
 
     public function testConfigurableCheckerReturnsTrueWhenAnyRoleIsGranted(): void