Problem

  • Create a view of users, add VBO to it, and select all actions matching for users.
  • List users. Choose some troll on your site and hit "Ban IP address..."

Goal

  • At minimum, the code should clarify what it does.

Solution

  • Attached patch implements that in the most trivial way.
CommentFileSizeAuthor
#4 drupal8.ban-ip-action-remove.4.patch1.79 KBsun
#3 ban_IP.png55.64 KBmbyrnes
drupal8.block-ip-action.0.patch2.64 KBsun

Comments

Tor Arne Thune’s picture

Tor Arne Thune commented
Status: Needs review » Needs work 
+++ b/core/modules/system/system.moduleundefined
@@ -3341,7 +3341,54 @@ function system_goto_action($entity, $context) {
+ *    sSSs   .S       S.    .S_sSSs     .S_sSSs      sSSs   .S_sSSs   sdSS_SSSSSSbs
+ *   d%%SP  .SS       SS.  .SS~YS%%b   .SS~YS%%b    d%%SP  .SS~YS%%b  YSSS~S%SSSSSP

Two lines surpass 80 characters.

chx’s picture

chx commented

Let's consider then eaton's masterpiece http://drupal.org/files/issues/settings.php_1.patch too :P

mbyrnes’s picture

mbyrnes commented
StatusFileSize
new ban_IP.png55.64 KB

Thanks to Sun for drawing attention to this issue. The only thing that I like better than ASCII art is the Drupal community of developers listening to their users. Having seen this happen to Drupal site managers three times and discussing it at several Drupal camps with various folks, I'm really excited to see that it is being raised in the issue queue for D8.

Here are some links to past mentions of the issues:
http://drupal.org/node/148410
http://drupal.org/node/556570

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Assigned: Unassigned » sun
Status: Needs work » Needs review
StatusFileSize
new drupal8.ban-ip-action-remove.4.patch1.79 KB

Despite the original patch, this issue is actually not a joke.

Given the current state of the entity system and action system, I doubt that anyone of us will have time to work on a completely revamped, properly implemented ban IP action.

Therefore:

To prevent further havoc in D8 for innocent users, I strongly recommend to remove this flawed action entirely.

It can be re-implemented in contrib, in case anyone actually has a use-case for it. (Dare I say I doubt that.)

This removal gets more important, since there is a good chance that we'll actually have #1823574: [Meta] Improve the Views Bulk Operations (VBOs) that are in core for D8, in which case it would be Drupal core and no longer a contributed module that is exposing this option as a possible action. We definitely do not want that.

Thus, good bye, ban_ip_action().

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: system_block_ip_action() blocks the current user » Remove ban_ip_action() from core. It blocks the currently logged-in user. (and potentially, your entire team)
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: -not in source

#4: drupal8.ban-ip-action-remove.4.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +not in source

#4: drupal8.ban-ip-action-remove.4.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Feedback, anyone?

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Needs review » Reviewed & tested by the community

Patch is simple and sane. Looks good to me.

catch’s picture

catch
he/him
Primary language English
commented
Title: Remove ban_ip_action() from core. It blocks the currently logged-in user. (and potentially, your entire team) » Change notice: Remove ban_ip_action() from core. It blocks the currently logged-in user. (and potentially, your entire team)
Category: bug » task
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active

i really wanted to commit the original patch.

But instead i committed #4. Allowing people to block themselves and only unblock via raw database access seems a bit wrong. Thanks!

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Change notice: Remove ban_ip_action() from core. It blocks the currently logged-in user. (and potentially, your entire team) » Remove ban_ip_action() from core. It blocks the currently logged-in user. (and potentially, your entire team)
Category: task » bug
Priority: Critical » Normal
Status: Active » Fixed

Change notice: http://drupal.org/node/1844972

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +API clean-up

Aaaand... let me directly complement this removal with this newborn feature request:

#1844992: Allow to ban an entity author's IP address through an action (or entity operation)

That's the pragmatic way. And dare I say, I really love it for this special case of issues: First, make sure to prevent further havoc, and only afterwards, try to re-introduce lost functionality in a proper way.

Thanks all! :)

Automatically closed -- issue fixed for 2 weeks with no activity.