diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b81ab1e..c36cb20 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -29,6 +29,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **Admin guard no longer depends on route names.** `PageLayoutKitAdminAccessSubscriber` only checked routes whose + name starts with `admin_page_layout` / `admin_page_blocks`; importing the bundle routes with a `name_prefix:` (or + renaming them) served the layout admin and block edit endpoints without the access check. The bundle's admin + controllers are now guarded by class, and the route-name prefixes still apply. + ## [1.8.0] - 2026-10-28 Security: empty access_roles deny instead of granting everyone; CSRF fails closed without form CSRF protection. See [UPGRADING](UPGRADING.md#to-180). diff --git a/src/EventSubscriber/PageLayoutKitAdminAccessSubscriber.php b/src/EventSubscriber/PageLayoutKitAdminAccessSubscriber.php index 4bf1334..beecbdd 100644 --- a/src/EventSubscriber/PageLayoutKitAdminAccessSubscriber.php +++ b/src/EventSubscriber/PageLayoutKitAdminAccessSubscriber.php @@ -10,13 +10,18 @@ use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use function is_array; +use function is_object; use function is_string; +use function str_starts_with; /** - * Enforces page layout admin access by route name prefix. + * Enforces page layout admin access on the bundle's admin controllers (by class) and on its route-name prefixes. */ final readonly class PageLayoutKitAdminAccessSubscriber implements EventSubscriberInterface { + private const ADMIN_CONTROLLER_NAMESPACE = 'Nowo\\PageLayoutKitBundle\\Controller\\Admin\\'; + public function __construct( private PageLayoutKitAccessCheckerInterface $accessChecker, ) { @@ -31,18 +36,31 @@ public static function getSubscribedEvents(): array public function onKernelController(ControllerEvent $event): void { - $route = $event->getRequest()->attributes->get('_route'); - if (!is_string($route)) { + if (!$this->isAdminRequest($event)) { return; } - if ( - str_starts_with($route, 'admin_page_layout') - || str_starts_with($route, 'admin_page_blocks') - ) { - if (!$this->accessChecker->canAccess()) { - throw new AccessDeniedException('Page layout admin requires an authorized user.'); - } + if (!$this->accessChecker->canAccess()) { + throw new AccessDeniedException('Page layout admin requires an authorized user.'); } } + + /** + * The bundle's admin controllers are guarded by class, whatever the route names are: a host that imports the + * routes with a `name_prefix` (or names its own routes for these controllers) must not bypass the check. The + * route-name prefixes still cover custom host controllers that reuse the bundle's route names. + */ + private function isAdminRequest(ControllerEvent $event): bool + { + $controller = $event->getController(); + $object = is_array($controller) ? $controller[0] : $controller; + if (is_object($object) && str_starts_with($object::class, self::ADMIN_CONTROLLER_NAMESPACE)) { + return true; + } + + $route = $event->getRequest()->attributes->get('_route'); + + return is_string($route) + && (str_starts_with($route, 'admin_page_layout') || str_starts_with($route, 'admin_page_blocks')); + } } diff --git a/tests/Unit/EventSubscriber/PageLayoutKitAdminAccessSubscriberTest.php b/tests/Unit/EventSubscriber/PageLayoutKitAdminAccessSubscriberTest.php index 8ef506f..68f3056 100644 --- a/tests/Unit/EventSubscriber/PageLayoutKitAdminAccessSubscriberTest.php +++ b/tests/Unit/EventSubscriber/PageLayoutKitAdminAccessSubscriberTest.php @@ -4,9 +4,12 @@ namespace Nowo\PageLayoutKitBundle\Tests\Unit\EventSubscriber; +use Nowo\PageLayoutKitBundle\Controller\Admin\PageBlockEditController; +use Nowo\PageLayoutKitBundle\Controller\Admin\PageLayoutController; use Nowo\PageLayoutKitBundle\EventSubscriber\PageLayoutKitAdminAccessSubscriber; use Nowo\PageLayoutKitBundle\Security\PageLayoutKitAccessCheckerInterface; use PHPUnit\Framework\TestCase; +use ReflectionClass; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\ControllerEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -62,14 +65,33 @@ public function testDeniesUnauthorizedAdminRoutes(): void $subscriber->onKernelController($this->createEvent('admin_page_blocks_edit')); } - private function createEvent(string|int|null $route): ControllerEvent + /** Routes imported with a `name_prefix` (or renamed by the host) must not bypass the guard. */ + public function testDeniesBundleAdminControllersWhateverTheRouteName(): void + { + $checker = $this->createMock(PageLayoutKitAccessCheckerInterface::class); + $checker->expects(self::exactly(2))->method('canAccess')->willReturn(false); + $subscriber = new PageLayoutKitAdminAccessSubscriber($checker); + + foreach ([PageLayoutController::class => 'layout', PageBlockEditController::class => 'editModal'] as $class => $method) { + $controller = (new ReflectionClass($class))->newInstanceWithoutConstructor(); + $callable = [$controller, $method]; + self::assertIsCallable($callable); + try { + $subscriber->onKernelController($this->createEvent('cms_admin_layout', $callable)); + self::fail($class . ' must be guarded'); + } catch (AccessDeniedException) { + } + } + } + + private function createEvent(string|int|null $route, ?callable $controller = null): ControllerEvent { $request = new Request(); $request->attributes->set('_route', $route); return new ControllerEvent( $this->createMock(HttpKernelInterface::class), - static fn (): null => null, + $controller ?? static fn (): null => null, $request, HttpKernelInterface::MAIN_REQUEST, );