Support external Flood (Redis, etc.)

Rerolled flood_control-support_external_flood-2928007-52.patch for 2.3.0

Oops! Previous patch had a misnamed file. Fixed version attached.

New patch with a new branch from fork: flood_control-2928007

The patch is well applied with this link: https://git.drupalcode.org/project/flood_control/-/merge_requests/36.diff

i implement fetchIdentifiers function for database and redis class.

I completed this issue to have a more readable user name.

I have not tested the patch with the database project.

We have produced a new patch that works for us with Redis on Flood control 2.3.2.
See attached flood_control_2.3.2.patch.

Fixed some errors.

Added misleading "blocked status" fix for custom events.

With Drupal 10.0.9, and Flood control 2.3.2
with this patch (62) flood_control-support_external_flood-2928007-62.patch
I'm getting this following error:
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "flood_control.flood_unblock_manager". in Drupal\Component\DependencyInjection\Container->get() (line 157 of core/lib/Drupal/Component/DependencyInjection/Container.php).
Drupal\flood_control\Form\FloodUnblockAdminForm::create(Object) (Line: 28)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\Drupal\flood_control\Form\FloodUnblockAdminForm') (Line: 48)
Drupal\Core\Controller\HtmlFormController->getFormObject(Object, '\Drupal\flood_control\Form\FloodUnblockAdminForm') (Line: 58)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 686)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

To add a bit to my previous comment, I've been getting the same error using
flood_control_2.3.2.patch (see #59) but the patch worked on Drupal 9.
Something is going on with dependency injection but I haven't been able to figure
out what the problem is. I hope you can figure it out for
patch (62) flood_control-support_external_flood-2928007-62.patch so it can work
on Drupal 10.

I have the same issue on D10
In the FloodControlServiceProvider::alter method ... instead of setting a new service definition i'm updating the existing one... which seems to work:

$container->getDefinition('flood_control.flood_unblock_manager')
  ->setClass(FloodUnblockManagerRedis::class)
  ->setArguments( [
        new Reference('redis.factory'),
        new Reference('flood'),
        new Reference('config.factory'),
        new Reference('entity_type.manager'),
        new Reference('messenger'),
      ]);

not sure why

sonneworks - I'm eager to try your new fix but so far have not been able to get
it to work. Could you either:
post your entire new FloodControlServiceProvider::alter method
or
if you have produced a new patch based on this fix could you post that? thanks will_frank

@sonneworks - is it possible that you might have changed other parts of the code
besides the FloodControlServiceProvider::alter method to get it to work?

One idea is that if you don't have a full patch file ready, perhaps you could
just zip up your entire flood_control module (all the files) and email it to me
at fwilliams7070@gmail.com and I'll try it - and if it works I could produce a patch.

@sonneworks
We have succeeded in creating a new patch based on your idea. Many thanks.
This works on Drupal 10. Likely this patch only will work with sites that are using
redis.

I took the patch #69 and aligned the method signature of the FloodUnblockManager::fetchIdentifiers with the interface method.

@will_frank sorry, was on holiday, didn't have access to codebase :)

Hi @will_frank , flood_control_2.3.2_d10.patch not working for flood_control 2.3.3.

version: 2.3.3

@KalaiyarasiThangavel yes you're right. I've been working trying to produce
a new patch for flood_control 2.3.3 but so far without success. If anyone has
a solution I would love to see it.

I have produced a new patch that works for us on Drupal core 10.1.4
and Flood control 2.3.3.
see
flood_control_2.3.3_d10.patch (30.44 KB)

Thank You @will_frank . The patch works fine.

Hi @will_frank I am getting the same error even after applying the patch you have provided.

The website encountered an unexpected error. Please try again later.

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "flood_control.flood_unblock_manager". in Drupal\Component\DependencyInjection\Container->get() (line 157 of core/lib/Drupal/Component/DependencyInjection/Container.php).
Drupal\flood_control\Form\FloodUnblockAdminForm::create(Object) (Line: 28)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\Drupal\flood_control\Form\FloodUnblockAdminForm') (Line: 48)
Drupal\Core\Controller\HtmlFormController->getFormObject(Object, '\Drupal\flood_control\Form\FloodUnblockAdminForm') (Line: 58)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 53)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Drupal version: 10.5.1
Flood_control: 2.3.3

I really appreciate your support

The issue forks for this are pretty wildly broken. I tried to see if I could rescue at least one of them, but in reality it looks like they should all be closed off and deleted.

Here's a re-roll of the patch from the current HEAD which also includes an update - the patch in #75 remove it?

Returning to Needs Work as there are a number of changes included recently in HEAD which have not been included in the change.

Work in progress.

Should work for redis OK - just doesn't have creation/expire times showing at present. No ability to filter.

Database back end is completely untested. Interface needs to be updated to pass in filtering options so no filtering atm.

Hey @WilliBautista, Your issue seems to be deeper than just the patch as the server which is present with or without this patch is what your site is complaining about. Flush caches? Patch from HEAD?

I ran into the same problem as comments #64 & #77, finally settling on the solution in #66 with some additional guards since I had an out of order xServiceProvider occurrence where the definition did not exist before trying alter it. The issue only manifest itself after restarting my cli session and prevented the site from working until I disabled the alter completely, flushed redis completely, restarted apache and php-fpm, and then flushed caches. Discovered the FloodUnblockManagerUnknown class needed some more method stubs.

No other significant changes.

I have updated the patch for last dev version.

Hello Sergei,
Thanks for your work!
#83 created an error in `FloodUnblockManagerDatabase` re "not in GROUP".
Please check the patch including the fixes attached.

Patch from #85 reports as "corrupted".

Confirming that patch #86 works as expected on a D10.1.7 and D10.2.3.

Hi, thanks everybody! I received some error and warning when testing patch #86 on D10.1.6 native database backend:
InvalidArgumentException: The timestamp must be numeric. in Drupal\Component\Datetime\DateTimePlus::createFromTimestamp() (line 201 of core/lib/Drupal/Component/Datetime/DateTimePlus.php).

Warning: Undefined array key "value" in Drupal\flood_control\Form\FloodUnblockAdminForm->buildForm() (line 189 of modules/contrib/flood_control/src/Form/FloodUnblockAdminForm.php).
Drupal\flood_control\Form\FloodUnblockAdminForm->buildForm(Array, Object)

Used $results[$key]["value"] instead of $results[$identifier]["value"] in FloodUnblockManagerDatabase::fetchIdentifiers.

A a maintainer, I have been monitoring this issue for a while.

I think that there are still major problems with this patch:
- Code has been copied in the past from FloodUnblockManager to new classes (FloodUnblockManagerBase & FloodUnblockManagerDatabase). After that moment FloodUnblockManagerBase was changed and improved, but those changes were not transferred to the code in FloodUnblockManagerBase & FloodUnblockManagerDatabase, resulting in the code in those files to miss certain features and improvements. The same happened with code that was copied from FloodUnblockAdminForm.php
- Having a default manager that is 'unknown' is confusing to me. We better assume that we have a database connection and make the original Manager the
default again. In that case we don't need FloodUnblockManagerUnknown.php

I propose to introduce the External Flood Support in two steps, using 2 different issues:
- 1. Add the generic FloodUnblockManager. This will be achieved by adding the generic FloodUnblockManagerBase and the database specific FloodUnblockManagerDatabase. This should be done while ensuring that everything keeps functioning as it does in the latest release.
✨ Make FloodUnblockManager generic Active
- 2. Add the Redis FloodUnblockManager. This will be achieve by adding the FloodUnblockManagerRedis
✨ Add Redis support Active

Oh man, I just ran into this and was searching for an hour, why IPs are blocked, but the list at /admin/people/flood-unblock and the flood database is empty -.-

The reason is: The project uses redis as caching backend ( #3192286: Flood unblock page is always empty with redis module → )

Just wanted to point out here, that this is really tricky currently and not easy to find, if you didn't set up the project and are not aware of this special case.
I totally agree with @batigolix in #89 but perhaps we should first make this more visible, until it is solved?
We could also treat this as bug, while I understand that Redis etc. are seen as bonus.

I hope what happened to me doesn't happen to others. For now I'll exclude flood from Redis. -.-

Needs work seems like a better status than needs more info here - the problem is known, the question is how best to implement the fix for it.

Updated the last patch to support the new filter "Only blocked"
I did this the same way it is done in "dev" if I'm right.
(This means it's display filter only, not a request filter)

The code in the patches in this issue is missing a lot of the improvements that were made in the last years (among them the new filter "Only blocked").

I explained this in #89

I created a new patch that includes all improvements (including the new filter "Only blocked") plus the code that makes the flood unblock generic. See ✨ Make FloodUnblockManager generic Active

There is an open issue for adding Redis support ✨ Add Redis support Active

Please do no update the patches in because they are outdated.

I put this issue on hold until ✨ Make FloodUnblockManager generic Active is fixed.

blocker is fixed

Sorry, I was not clear. ✨ Add Redis support Active also needs to be fixed.