Commit 89caf28e authored by solideogloria's avatar solideogloria
Browse files

Change route subscriber priority and use current route collection during rebuild

parent 821c18db

src/Routing/RouteSubscriber.php

+11 −1
Changes for src/Routing/RouteSubscriber.php: 11 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -3,6 +3,7 @@ 
namespace Drupal\restrict_route_by_ip\Routing;



use Drupal\Core\Routing\RouteSubscriberBase;

use Drupal\Core\Routing\RoutingEvents;

use Drupal\restrict_route_by_ip\Service\RestrictIpService;

use Symfony\Component\Routing\RouteCollection;
@@ -28,11 +29,20 @@ class RouteSubscriber extends RouteSubscriberBase { 
    $this->restrictRouteService = $restrict_route_service;

  }



  /**

   * {@inheritdoc}

   */

  public static function getSubscribedEvents(): array {

    $events = parent::getSubscribedEvents();

    $events[RoutingEvents::ALTER] = ['onAlterRoutes', -2048];

    return $events;

  }



  /**

   * {@inheritdoc}

   */

  protected function alterRoutes(RouteCollection $collection): void {

    $routes = $this->restrictRouteService->getAllRestrictedRoutes();

    $routes = $this->restrictRouteService->getAllRestrictedRoutes($collection);

    $route_names = array_column($routes, 'route_names');

    $route_names = array_reduce($route_names, 'array_merge', []);

src/Service/RestrictIpInterface.php

+11 −3
Changes for src/Service/RestrictIpInterface.php: 11 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ namespace Drupal\restrict_route_by_ip\Service; 


use Drupal\Core\Session\AccountInterface;

use Drupal\restrict_route_by_ip\Entity\RestrictRouteInterface;

use Symfony\Component\Routing\RouteCollection;



/**

 * Restrict Ip Interface.
@@ -37,6 +38,9 @@ interface RestrictIpInterface { 
  /**

   * Get all enabled restricted routes.

   *

   * @param \Symfony\Component\Routing\RouteCollection|null $collection

   *   The routes being rebuilt, or NULL to use the route provider.

   *

   * @return array

   *   List of all restricted routes. Each route is represented as:

   *   [
@@ -44,7 +48,7 @@ interface RestrictIpInterface { 
   *     'route_names' => An array of route names associated with the route,

   *   ]

   */

  public function getAllRestrictedRoutes(): array;

  public function getAllRestrictedRoutes(?RouteCollection $collection = NULL): array;



  /**

   * Get a restricted route by route name.
@@ -62,17 +66,21 @@ interface RestrictIpInterface { 
   *

   * @param \Drupal\restrict_route_by_ip\Entity\RestrictRouteInterface $entity

   *   A RestrictRoute entity.

   * @param \Symfony\Component\Routing\RouteCollection|null $collection

   *   The routes being rebuilt, or NULL to use the route provider.

   *

   * @return array

   *   Array of detail from RestrictRoute entity.

   */

  public function getRestrictedRouteDetail(RestrictRouteInterface $entity): array;

  public function getRestrictedRouteDetail(RestrictRouteInterface $entity, ?RouteCollection $collection = NULL): array;



  /**

   * Get all restricted route names and path from string.

   *

   * @param string $restricted_route_name

   *   A route name or a path or a regular expression.

   * @param \Symfony\Component\Routing\RouteCollection|null $collection

   *   The routes being rebuilt, or NULL to use the route provider.

   *

   * @return array

   *   Array of restricted route names and path. (ex: [
@@ -80,6 +88,6 @@ interface RestrictIpInterface { 
   *    'path' => [],

   *   ])

   */

  public function getAllRouteNamesAndPath(string $restricted_route_name): array;

  public function getAllRouteNamesAndPath(string $restricted_route_name, ?RouteCollection $collection = NULL): array;



}

src/Service/RestrictIpService.php

+17 −9
Changes for src/Service/RestrictIpService.php: 17 added lines, 9 removed lines.
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ use Drupal\restrict_route_by_ip\Form\RestrictRouteGlobalForm; 
use Psr\Log\LoggerInterface;

use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpFoundation\RequestStack;

use Symfony\Component\Routing\RouteCollection;



/**

 * Restrict Ip service.
@@ -253,7 +254,7 @@ class RestrictIpService implements RestrictIpInterface { 
  /**

   * {@inheritdoc}

   */

  public function getAllRestrictedRoutes(): array {

  public function getAllRestrictedRoutes(?RouteCollection $collection = NULL): array {

    $routes = [];

    $status = $this->globalConfig->get(RestrictRouteGlobalForm::FIELD_STATUS);
@@ -266,7 +267,7 @@ class RestrictIpService implements RestrictIpInterface { 


      /** @var \Drupal\restrict_route_by_ip\Entity\RestrictRouteInterface $entity */

      foreach ($entities as $entity) {

        $routes[] = $this->getRestrictedRouteDetail($entity);

        $routes[] = $this->getRestrictedRouteDetail($entity, $collection);

      }

    }

    return $routes;
@@ -275,9 +276,9 @@ class RestrictIpService implements RestrictIpInterface { 
  /**

   * {@inheritdoc}

   */

  public function getRestrictedRouteDetail(RestrictRouteInterface $entity): array {

  public function getRestrictedRouteDetail(RestrictRouteInterface $entity, ?RouteCollection $collection = NULL): array {

    $restricted_route_name = $entity->getRoute();

    $restricted_routes = $this->getAllRouteNamesAndPath($restricted_route_name);

    $restricted_routes = $this->getAllRouteNamesAndPath($restricted_route_name, $collection);

    return [

      'restricted_route_id' => $entity->id(),

      'route_names' => $restricted_routes['name'],
@@ -288,7 +289,7 @@ class RestrictIpService implements RestrictIpInterface { 
  /**

   * {@inheritdoc}

   */

  public function getAllRouteNamesAndPath(string $restricted_route_name): array {

  public function getAllRouteNamesAndPath(string $restricted_route_name, ?RouteCollection $collection = NULL): array {

    $route_names = [

      'name' => [],

      'path' => [],
@@ -306,14 +307,21 @@ class RestrictIpService implements RestrictIpInterface { 
      $regex = $this->getRegularExpressionFromPath($restricted_route_name);

    }



    // Load all routes.

    // During a rebuild, the stored routes may be missing or outdated. Do not

    // read or overwrite the provider cache with the collection being rebuilt.

    if ($collection !== NULL) {

      $all_routes = $collection->all();

    }

    else {

      if (empty($this->allRoutes)) {

        $this->allRoutes = iterator_to_array($this->routeProvider->getAllRoutes());

      }

      $all_routes = $this->allRoutes;

    }



    // Use regex to define impacted route names.

    if (!empty($regex)) {

      foreach ($this->allRoutes as $route_name => $route) {

      foreach ($all_routes as $route_name => $route) {

        $original_path = $route->getPath();

        $route_path = preg_replace('#(\{[_\-0-9a-zA-Z]+\})#', 'rrbip_replaced', $original_path);
@@ -331,9 +339,9 @@ class RestrictIpService implements RestrictIpInterface { 
        }

      }

    }

    elseif (isset($this->allRoutes[$restricted_route_name])) {

    elseif (isset($all_routes[$restricted_route_name])) {

      // If it's not a regex or a path, set the route name directly in array.

      $route = $this->allRoutes[$restricted_route_name];

      $route = $all_routes[$restricted_route_name];



      $route_names['name'][] = $restricted_route_name;

      $route_names['path'][] = $route->getPath();

tests/src/Unit/RestrictIpTest.php

+100 −0
Changes for tests/src/Unit/RestrictIpTest.php: 100 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -9,15 +9,24 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; 
use Drupal\Core\Entity\Query\QueryInterface;

use Drupal\Core\Logger\LoggerChannelFactoryInterface;

use Drupal\Core\Messenger\MessengerInterface;

use Drupal\Core\Routing\RouteBuildEvent;

use Drupal\Core\Routing\RouteMatchInterface;

use Drupal\Core\Routing\RouteProvider;

use Drupal\Core\Routing\RoutingEvents;

use Drupal\Core\Session\AccountInterface;

use Drupal\restrict_route_by_ip\Entity\RestrictRouteInterface;

use Drupal\restrict_route_by_ip\Form\RestrictRouteGlobalForm;

use Drupal\restrict_route_by_ip\Routing\RouteSubscriber;

use Drupal\restrict_route_by_ip\Service\RestrictIpInterface;

use Drupal\restrict_route_by_ip\Service\RestrictIpService;

use Drupal\Tests\UnitTestCase;

use Prophecy\PhpUnit\ProphecyTrait;

use Psr\Log\LoggerInterface;

use Symfony\Component\EventDispatcher\EventDispatcher;

use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpFoundation\RequestStack;

use Symfony\Component\Routing\Route;

use Symfony\Component\Routing\RouteCollection;



/**

 * Tests of the RestrictIpService class.
@@ -261,6 +270,97 @@ class RestrictIpTest extends UnitTestCase { 
    ], $name_and_path['path'], 'Check route paths return for regex.');

  }



  /**

   * Tests matching current routes without using or changing the provider cache.

   *

   * @covers ::getAllRouteNamesAndPath

   */

  public function testMatchingCurrentCollection(): void {

    // Prime the cache with routes from the previous router build.

    $stored = $this->restrictIpService->getAllRouteNamesAndPath('admin.test');

    $this->assertSame(['/admin/test'], $stored['path']);



    $collection = new RouteCollection();

    $collection->add('admin.test', new Route('/public/test'));

    $collection->add('admin.new', new Route('/admin/new/{item}'));



    // Regex, wildcard, path and route-name matching must see the new route.

    foreach (['#^/admin($|/)#', '/admin/%', '/admin/new/', 'admin.new'] as $pattern) {

      $result = $this->restrictIpService->getAllRouteNamesAndPath($pattern, $collection);

      $this->assertSame(['admin.new'], $result['name']);

      $this->assertSame(['/admin/new/{item}'], $result['path']);

    }



    // A changed path must come from the current collection.

    $result = $this->restrictIpService->getAllRouteNamesAndPath('admin.test', $collection);

    $this->assertSame(['/public/test'], $result['path']);



    // Removed routes and an empty collection must not fall back to old routes.

    $result = $this->restrictIpService->getAllRouteNamesAndPath('admin.test2', $collection);

    $this->assertSame(['name' => [], 'path' => []], $result);

    $result = $this->restrictIpService->getAllRouteNamesAndPath('#^/admin($|/)#', new RouteCollection());

    $this->assertSame(['name' => [], 'path' => []], $result);



    // Build-time matching must not replace the cache used by other callers.

    $this->assertSame($stored, $this->restrictIpService->getAllRouteNamesAndPath('admin.test'));

  }



  /**

   * Tests restrictions on routes added or replaced by later subscribers.

   *

   * @covers ::getAllRestrictedRoutes

   * @covers ::getRestrictedRouteDetail

   * @covers ::getAllRouteNamesAndPath

   * @covers \Drupal\restrict_route_by_ip\Routing\RouteSubscriber

   */

  public function testLateRouteAlterations(): void {

    $this->entityQueryExpects();

    $this->configObject->get(RestrictRouteGlobalForm::FIELD_STATUS)

      ->willReturn(RestrictRouteGlobalForm::STATUS_ENABLE);

    $restriction = $this->prophesize(RestrictRouteInterface::class);

    $restriction->getRoute()->willReturn('#^/admin($|/)#');

    $restriction->id()->willReturn('1');

    $this->entityStorage->loadMultiple(['1', '2'])

      ->willReturn([$restriction->reveal()]);



    $collection = new RouteCollection();

    $collection->add('system.admin', new Route('/admin'));

    $collection->add('system.admin_content', new Route('/admin/content'));

    $collection->add('public.page', new Route('/public', [], ['_permission' => 'access content']));



    $dispatcher = new EventDispatcher();

    $dispatcher->addSubscriber(new RouteSubscriber($this->restrictIpService));

    // Simulate Views replacing an existing route with a new route object.

    $dispatcher->addListener(RoutingEvents::ALTER, static function (RouteBuildEvent $event): void {

      $event->getRouteCollection()->add('system.admin_content', new Route('/admin/content', [], ['_permission' => 'access content overview']));

    }, -175);

    // This route does not exist in the stored router table at all.

    $dispatcher->addListener(RoutingEvents::ALTER, static function (RouteBuildEvent $event): void {

      $event->getRouteCollection()->add('admin.new', new Route('/admin/new'));

    }, -300);

    $dispatcher->dispatch(new RouteBuildEvent($collection), RoutingEvents::ALTER);



    foreach (['system.admin', 'system.admin_content', 'admin.new'] as $name) {

      $route = $collection->get($name);

      $this->assertSame('restrict_route_by_ip.services_access_checker::access', $route->getRequirement('_custom_access'));

      $this->assertTrue($route->getOption('no_cache'));

    }

    $this->assertSame('access content overview', $collection->get('system.admin_content')->getRequirement('_permission'));

    $this->assertSame(['_permission' => 'access content'], $collection->get('public.page')->getRequirements());

    $this->assertNull($collection->get('public.page')->getOption('no_cache'));



    // Disabling the module must leave a fresh collection unrestricted.

    $this->configObject->get(RestrictRouteGlobalForm::FIELD_STATUS)

      ->willReturn(RestrictRouteGlobalForm::STATUS_DISABLE);

    $disabled_collection = new RouteCollection();

    $disabled_collection->add('system.admin', new Route('/admin'));

    $dispatcher->dispatch(new RouteBuildEvent($disabled_collection), RoutingEvents::ALTER);

    foreach ($disabled_collection->all() as $route) {

      $this->assertFalse($route->hasRequirement('_custom_access'));

      $this->assertNull($route->getOption('no_cache'));

    }

  }



  /**

   * Tests the getRestrictedRouteDetail method.

   *