From 4119eec52322b5b5358166ab3367b723d0a15775 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 10 Jan 2025 10:37:55 +0000
Subject: [PATCH 1/7] Issue #3498739 - Allow returning a 403 response when
blocking
---
config/install/restrict_ip.settings.yml | 3 +++
config/schema/restrict_ip.schema.yml | 3 +++
restrict_ip.services.yml | 1 +
src/EventSubscriber/RestrictIpEventSubscriber.php | 14 ++++++++++++++
src/Form/ConfigForm.php | 13 +++++++++++++
5 files changed, 34 insertions(+)
diff --git a/config/install/restrict_ip.settings.yml b/config/install/restrict_ip.settings.yml
index 71b69c1..0b8915e 100644
--- a/config/install/restrict_ip.settings.yml
+++ b/config/install/restrict_ip.settings.yml
@@ -18,6 +18,9 @@ allow_role_bypass: false
# A key indicating what action should be performed when bypasses are allowed by role, but users are not logged in
bypass_action: 'provide_link_login_page'
+# A boolean indicating whether to return a 403 access denied response when blocking.
+return_403: false
+
# D7 variable - restrict_ip_white_black_list
# An integer indicating how to check bypass restrictions.
# 0 = check access on all paths
diff --git a/config/schema/restrict_ip.schema.yml b/config/schema/restrict_ip.schema.yml
index 8489931..f36df7e 100644
--- a/config/schema/restrict_ip.schema.yml
+++ b/config/schema/restrict_ip.schema.yml
@@ -23,6 +23,9 @@ restrict_ip.settings:
Choice:
- provide_link_login_page
- redirect_login_page
+ return_403:
+ type: boolean
+ label: 'Return a 403 access denied response when blocking'
white_black_list:
type: integer
label: 'Whether to use a path whitelist, blacklist, or check all pages'
diff --git a/restrict_ip.services.yml b/restrict_ip.services.yml
index a027553..8d95d7c 100644
--- a/restrict_ip.services.yml
+++ b/restrict_ip.services.yml
@@ -31,6 +31,7 @@ services:
- '@module_handler'
- '@url_generator'
- '@messenger'
+ - '@current_route_match'
tags:
- {name: event_subscriber}
diff --git a/src/EventSubscriber/RestrictIpEventSubscriber.php b/src/EventSubscriber/RestrictIpEventSubscriber.php
index 6bc282a..202343f 100644
--- a/src/EventSubscriber/RestrictIpEventSubscriber.php
+++ b/src/EventSubscriber/RestrictIpEventSubscriber.php
@@ -7,6 +7,7 @@ use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Messenger\Messenger;
+use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Routing\UrlGeneratorInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\Url;
@@ -15,6 +16,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpKernel\Event\RequestEvent;
+use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpKernel\KernelEvents;
/**
@@ -38,6 +40,8 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
* The Url Generator service.
* @param \Drupal\Core\Messenger\Messenger $messenger
* The messenger service object.
+ * @param Drupal\Core\Routing\RouteMatchInterface $routeMatch
+ * The route match handler.
*/
public function __construct(
protected RestrictIpServiceInterface $restrictIpService,
@@ -46,6 +50,7 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
protected ModuleHandlerInterface $moduleHandler,
protected UrlGeneratorInterface $urlGenerator,
protected Messenger $messenger,
+ protected RouteMatchInterface $routeMatch,
) {
}
@@ -60,6 +65,7 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
$container->get('module_handler'),
$container->get('url_generator'),
$container->get('messenger'),
+ $container->get('current_route_match'),
);
}
@@ -77,6 +83,11 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
* Check if response needs to be restricted.
*/
public function checkIpRestricted(RequestEvent $event): void {
+ // If we are on the system 403 route, bypass the inner logic.
+ if ($this->routeMatch->getRouteName() === 'system.403') {
+ return;
+ }
+
unset($_SESSION['restrict_ip']);
$this->restrictIpService->testForBlock();
@@ -95,6 +106,9 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
$url = Url::fromRoute('user.login');
$event->setResponse(new RedirectResponse($url->toString()));
}
+ elseif ($config->get('return_403')) {
+ throw new AccessDeniedHttpException();
+ }
elseif (in_array($config->get('white_black_list'), [0, 1])) {
$url = Url::fromRoute('restrict_ip.access_denied_page');
$event->setResponse(new RedirectResponse($url->toString()));
diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php
index 5dda5e6..2135893 100644
--- a/src/Form/ConfigForm.php
+++ b/src/Form/ConfigForm.php
@@ -180,6 +180,18 @@ class ConfigForm extends ConfigFormBase {
],
];
+ $form['return_403'] = [
+ '#title' => $this->t('Return a 403 access denied response'),
+ '#type' => 'checkbox',
+ '#default_value' => $config->get('return_403'),
+ '#description' => $this->t('When this box is checked, a 403 access denied response will be returned instead of the access denied page or front page.'),
+ '#states' => [
+ 'visible' => [
+ '#edit-allow-role-bypass' => ['checked' => FALSE],
+ ]
+ ]
+ ];
+
$form['white_black_list'] = [
'#type' => 'radios',
'#options' => [
@@ -394,6 +406,7 @@ class ConfigForm extends ConfigFormBase {
->set('dblog', (bool) $form_state->getValue('dblog'))
->set('allow_role_bypass', (bool) $form_state->getValue('allow_role_bypass'))
->set('bypass_action', (string) $form_state->getValue('bypass_action'))
+ ->set('return_403', (bool) $form_state->getValue('return_403'))
->set('white_black_list', (int) $form_state->getValue('white_black_list'))
->set('country_white_black_list', (int) $form_state->getValue('country_white_black_list'))
->set('country_list', $country_list)
--
GitLab
From 0bc094538e6b4b3105d4a8f8fa649d1ac08e8977 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 17 Jan 2025 14:28:39 +0000
Subject: [PATCH 2/7] #3498739 - Add test to check the new return_403 option
works as expected.
---
tests/src/Functional/RestrictIpAccessTest.php | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tests/src/Functional/RestrictIpAccessTest.php b/tests/src/Functional/RestrictIpAccessTest.php
index 971877e..de27659 100644
--- a/tests/src/Functional/RestrictIpAccessTest.php
+++ b/tests/src/Functional/RestrictIpAccessTest.php
@@ -171,6 +171,28 @@ class RestrictIpAccessTest extends RestrictIpBrowserTestBase {
$this->assertElementExists('#edit-name');
}
+ /**
+ * Tests if access denied response is returned when the 'return_403'
+ * option is enabled.
+ */
+ public function testAccessDeniedResponseWhenAccessDeniedOptionEnabled(): void {
+ $adminUser = $this->drupalCreateUser([
+ 'administer restricted ip addresses',
+ 'access administration pages',
+ 'administer modules',
+ ]);
+
+ $this->drupalLogin($adminUser);
+ $this->drupalGet('admin/config/people/restrict_ip');
+ $this->assertStatusCodeEquals(200);
+
+ $this->checkCheckbox('#edit-enable');
+ $this->checkCheckbox('#edit-return-403');
+ $this->click('#edit-submit');
+
+ $this->assertStatusCodeEquals(403);
+ }
+
/**
* Tests whitelisting paths.
*
--
GitLab
From cd3751ed0284f835cff92aa0071640d942e388e2 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 17 Jan 2025 14:31:09 +0000
Subject: [PATCH 3/7] #3498739 - Tweak the config form visibility of return_403
option to match the logic in ::checkIpRestricted(). - The return_403 option
doesn't kick in if allow_role_bypass is enabled AND the bypass_action is
redirect_login_page, the config form visibility should match this.
---
src/Form/ConfigForm.php | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php
index 2135893..c08ad67 100644
--- a/src/Form/ConfigForm.php
+++ b/src/Form/ConfigForm.php
@@ -186,8 +186,12 @@ class ConfigForm extends ConfigFormBase {
'#default_value' => $config->get('return_403'),
'#description' => $this->t('When this box is checked, a 403 access denied response will be returned instead of the access denied page or front page.'),
'#states' => [
- 'visible' => [
- '#edit-allow-role-bypass' => ['checked' => FALSE],
+ 'invisible' => [
+ [
+ '#edit-allow-role-bypass' => ['checked' => TRUE],
+ 'and',
+ 'input[name="bypass_action"]' => ['value' => 'redirect_login_page'],
+ ],
]
]
];
--
GitLab
From 7671b56de6a42b126aa7fc2f09e40c9c7b961f19 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 17 Jan 2025 14:36:17 +0000
Subject: [PATCH 4/7] #3498739 - Set the new return_403 value in config to
false.
---
restrict_ip.install | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/restrict_ip.install b/restrict_ip.install
index c9e308f..084899c 100644
--- a/restrict_ip.install
+++ b/restrict_ip.install
@@ -142,3 +142,12 @@ function restrict_ip_update_500001(): void {
function restrict_ip_update_500002(): void {
\Drupal::configFactory()->getEditable('restrict_ip.settings')->save();
}
+
+/**
+ * Ensure the new return_403 option is present in config (disabled by default).
+ */
+function restrict_ip_update_500003(): void {
+ \Drupal::configFactory()->getEditable('restrict_ip.settings')
+ ->set('return_403', FALSE)
+ ->save();
+}
--
GitLab
From 0a28409f85fe5f4e3d600b52496e640cbfc673ef Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 17 Jan 2025 14:49:27 +0000
Subject: [PATCH 5/7] #3498739 - Fix PHPCS violations with the newly added
logic.
---
src/EventSubscriber/RestrictIpEventSubscriber.php | 4 ++--
src/Form/ConfigForm.php | 4 ++--
tests/src/Functional/RestrictIpAccessTest.php | 3 +--
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/EventSubscriber/RestrictIpEventSubscriber.php b/src/EventSubscriber/RestrictIpEventSubscriber.php
index 202343f..a87d9b9 100644
--- a/src/EventSubscriber/RestrictIpEventSubscriber.php
+++ b/src/EventSubscriber/RestrictIpEventSubscriber.php
@@ -40,8 +40,8 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
* The Url Generator service.
* @param \Drupal\Core\Messenger\Messenger $messenger
* The messenger service object.
- * @param Drupal\Core\Routing\RouteMatchInterface $routeMatch
- * The route match handler.
+ * @param \Drupal\Core\Routing\RouteMatchInterface $routeMatch
+ * The route match handler.
*/
public function __construct(
protected RestrictIpServiceInterface $restrictIpService,
diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php
index c08ad67..667b8c7 100644
--- a/src/Form/ConfigForm.php
+++ b/src/Form/ConfigForm.php
@@ -192,8 +192,8 @@ class ConfigForm extends ConfigFormBase {
'and',
'input[name="bypass_action"]' => ['value' => 'redirect_login_page'],
],
- ]
- ]
+ ],
+ ],
];
$form['white_black_list'] = [
diff --git a/tests/src/Functional/RestrictIpAccessTest.php b/tests/src/Functional/RestrictIpAccessTest.php
index de27659..85625bf 100644
--- a/tests/src/Functional/RestrictIpAccessTest.php
+++ b/tests/src/Functional/RestrictIpAccessTest.php
@@ -172,8 +172,7 @@ class RestrictIpAccessTest extends RestrictIpBrowserTestBase {
}
/**
- * Tests if access denied response is returned when the 'return_403'
- * option is enabled.
+ * Tests access denied response is returned when 'return_403' option enabled.
*/
public function testAccessDeniedResponseWhenAccessDeniedOptionEnabled(): void {
$adminUser = $this->drupalCreateUser([
--
GitLab
From 4f0fdf5156f363d7fc8b575429aa2069562ec844 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Fri, 17 Jan 2025 15:18:54 +0000
Subject: [PATCH 6/7] #3498739 - Fix PHPCS violations with the newly added
logic.
---
src/Form/ConfigForm.php | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php
index 667b8c7..7b54633 100644
--- a/src/Form/ConfigForm.php
+++ b/src/Form/ConfigForm.php
@@ -189,7 +189,6 @@ class ConfigForm extends ConfigFormBase {
'invisible' => [
[
'#edit-allow-role-bypass' => ['checked' => TRUE],
- 'and',
'input[name="bypass_action"]' => ['value' => 'redirect_login_page'],
],
],
--
GitLab
From 8224dcb934e0d752ddd7b5fe1dbc6f1f4adc8fa3 Mon Sep 17 00:00:00 2001
From: Ross Bale <ross.bale@computerminds.co.uk>
Date: Tue, 21 Jan 2025 15:12:33 +0000
Subject: [PATCH 7/7] #3498739 - Ensure $_SESSION['restrict_ip'] always gets
unset even on a 403 route.
---
src/EventSubscriber/RestrictIpEventSubscriber.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/EventSubscriber/RestrictIpEventSubscriber.php b/src/EventSubscriber/RestrictIpEventSubscriber.php
index a87d9b9..f080b9c 100644
--- a/src/EventSubscriber/RestrictIpEventSubscriber.php
+++ b/src/EventSubscriber/RestrictIpEventSubscriber.php
@@ -83,13 +83,13 @@ class RestrictIpEventSubscriber implements EventSubscriberInterface, ContainerIn
* Check if response needs to be restricted.
*/
public function checkIpRestricted(RequestEvent $event): void {
+ unset($_SESSION['restrict_ip']);
+
// If we are on the system 403 route, bypass the inner logic.
if ($this->routeMatch->getRouteName() === 'system.403') {
return;
}
- unset($_SESSION['restrict_ip']);
-
$this->restrictIpService->testForBlock();
$config = $this->configFactory->get('restrict_ip.settings');
if ($this->restrictIpService->userIsBlocked()) {
--
GitLab