Problem/Motivation
The CrowdSec module integrates with the CrowdSec platform, which the Drupal Association partnered up with recently.
The CrowdSec module now provides a plugin manager that allows it for other modules like this to leverage this by e.g. sending a signal every time somebody fails in solving a captcha. This could then lead to a scenario where e.g. after 10 failed attempts within a configurable number of minutes, that IP would be banned on the Drupal site for a period of time, and the IP would also be signalled upstream, so that other Drupal sites could benefit from the fact, that an IP is probably brute-forcing Drupal sites.
Proposed resolution
Implement the integration.
Issue fork captcha-3552563
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
anybody@lrwebks can do this like in login_security :)
Comment #3
lrwebks commentedComment #4
anybodyComment #6
lrwebks commentedCan confirm that it works fine via monkey testing. Log messages show that CrowdSec is receiving correct signals.
Comment #7
anybodyNice @lrwebks LGTM. I left a final comment. @jurgenhaas would you do the final RTBC? Then we can merge it and tag a new release. Quite a big thing with captcha.
Comment #8
anybodyI'll set this RTBC for @jurgenhaas to sign it off.
Comment #9
jurgenhaasRTBC+1
You may want to add the crowdsec module to require-dev in composer.json so that tests know the interfaces and methods.
Comment #10
anybodyThanks @jurgenhaas! Good point!
@lrwebks could you do that please? Afterwards I'll RTBC and merge it.
Comment #11
lrwebks commentedComment #12
anybodyLooks like the
require-deventry in composer.json causes tests to fail...Comment #13
anybodyOk I think we should for now opt out from
OPT_IN_TEST_NEXT_MAJOR, which also causes issues in other projects and ist not clearly something useful.Comment #14
anybodyMhm unsure, if we shouldn't better remove the
which are breaking the tests.
Removing
OPT_IN_TEST_NEXT_MAJOR(Drupal 12) would be fine, but removingOPT_IN_TEST_PREVIOUS_MAJOR(Drupal 10) doesn't make much sense to me and makes things worse. I don't think it's worth it..Comment #15
anybody@jurgenhaas any opinions or experience with this?
Are you maybe planning to introduce
flags for Crowdsec Module GitLab CI? Or do you think it's an antipattern?
Otherwise, I'm tending to remove the dev dependency. And I'm still not sure...
I think best might be to have these as warnings and indicator, e.g. for the next major version, not as errors maybe?
Comment #17
jurgenhaasYes, I'm just working on #3578265: Get ready for Drupal 12 and should be ready for you shortly.
Comment #18
anybodyWhao thanks @jurgenhaas!!
Comment #19
jurgenhaas@anybody this has been completed, 1.2.x-dev is ready for Drupal 12. A new release will follow as soon as we have completed the 1 remaining open issue.
Comment #20
anybodyComment #21
anybodyMaybe we could improve .gitlab-ci.yml as follows:
That's similar to what I suggested above. Found it here: #3578759: Fix GitLab pipelines (tests, phpstan, phpcs, cspell) and reduce waste
Comment #22
anybodyNice! CI is finally green! @jurgenhaas could you do the final RTBC signoff? Then I'd like to merge and release this important one :)
Comment #23
jurgenhaasLGTM
Comment #24
anybodyThank you @jurgenhaas! Merging!