Contrib.social

Use flood control API in flag

Open on Drupal.org →
Created on 12 February 2020, over 4 years ago
Updated 15 November 2023, 7 months ago

Problem/Motivation

Either maliciously* or by accident, an automated script can create a large number of flaggings by crawling a site and clicking flag links. Especially when these flaggings trigger some behaviour, such as activities in a timeline, this can be quite disruptive.

*: @eelkeblok reported this as a security issue to be on the safe side, even though our personal assessment said strictly speaking it is not. Also, the module is still in beta and thus not covered by the security policy. The security team agreed.

Proposed resolution

Use Flood API to limit the number of flaggings that can be made per time window. Since the nature of flags can be very different, this needs to be configurable per flag. For example:

  1. a "report an issue" flag would typically not be used very often. Reporting more than 10 issues in 10 minutes might be indicative of abuse
  2. a person might run through some kind of activity stream quite quickly and like a fairly large number of items in a short amount of time. However, if they create a solid 120 likings within an hour, there might be something up
  3. a "bookmark" flag only impacts the user themselves. It does not need flood control

Of course, these are just examples. You might disagree with the parameters, the point is that different flags demand different approaches, so this needs to be configurable per flag.

Remaining tasks

  2. Review
  3. Add tests
  4. Review some more
  5. Commit

User interface changes

Every flag will have a field group that allows an administrator to control flag parameters.

API changes

None (outside facing).

Data model changes

The module will make use of the Flood API and thereby cause entries to be written to the flood table.

Original report by dutchyoda

To prevent mindless clicking on different flags (f.e. by bots or crawlers) I created a patch that adds the use of the flood control API to flag.
This does the following:

  • Enable flood control per flag (default is turned off)
  • Sets number of flaggings allowed per flag
  • Sets the time window per flag
  • Revokes access if the number is surpassed in that time window
✨ Feature request
Status

Needs work

Version

4.0

Component

Flag core

Created by

🇳🇱Netherlands dutchyoda Lelystad

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇳🇱Netherlands dutchyoda Lelystad

    I updated the patch to work with version 8.x-4.0-beta4.
    Hadn't had time to fix the incompatibility from #21 yet.

  • Comment over 1 year ago →
    🇳🇱Netherlands dutchyoda Lelystad

    Oops. Wrongly named file. my bad.

  • Comment over 1 year ago →
    🇳🇱Netherlands dutchyoda Lelystad

    Sorry about the mess.
    I finally fixed the patch for 8.x-4.0-beta4.

  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇳🇱Netherlands eelkeblok Netherlands 🇳🇱

    The last patch is empty... The interdiff does suggest changes. I'll see if I can figure it out.

  • Comment about 1 year ago →
    🇳🇱Netherlands eelkeblok Netherlands 🇳🇱

    We created an issue fork. It currently contains the result of #20, merged with beta4 (which had a small merge conflict). I'll see if I can address some of the code style violations before creating a merge request.

    @bernardopaulino I think it would need to be addressed in whatever issue is merged last... If you need both patches, the best course of action is try to get a hybrid patch (of course you are welcome to post that in either issue, this clearly mark that it is a combination of two issues).

  • @dutchyoda opened merge request.
  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇳🇱Netherlands eelkeblok Netherlands 🇳🇱

    I don't think there are any new code style violations, but not sure what the best way to check is. The number of violations in the base branch makes this a little hard.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update about 1 year ago
    46 pass
  • Comment about 1 year ago →
    🇧🇪Belgium Ludo.R Brussels

    Patch from the merge request on #27 didn't work for with version 4.0.0-beta4.

    I adapted the patch.

  • Comment 11 months ago →
    erdm

    There should be a customized warning message to show before or after this ban as to why it happened.

  • Status changed to Needs work 7 months ago
  • Comment 7 months ago →
    🇺🇸United States TR Cascadia

    From #18:

    flag_update_8402() should probably be changed to do everything in a batch - loading every single flag entity and looping through them is going to be very expensive - this won't work as written on a large site.

    Also, this new feature still doesn't have test cases to demonstrate it works and to prevent it from being broken by future changes to this module. All new features should have test cases. All old features should too, but the very least we could do it to make sure that when we add features we also add tests so as not to make the problem of untested code worse.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024