Ban hosts from the 'Top Visitors' page [#24135]

Comment	File	Size	Author
#11	ban_3.diff	12.73 KB	moshe weitzman

#9	ban_2.diff	10.49 KB	moshe weitzman

#7	ban_1.diff	10.5 KB	moshe weitzman

#1	ban_0.diff	5.95 KB	moshe weitzman

	ban.diff	5.94 KB	moshe weitzman

Comment #1

moshe weitzman CreditAttribution: moshe weitzman commented 2 June 2005 at 05:59

File	Size
ban_0.diff	5.95 KB

Oops. Here is the latest version.

Log in or register to post comments

Comment #2

Dries CreditAttribution: Dries commented 2 June 2005 at 10:59

Any particular reason we are not storing this in the database? Storing it in the database sounds like the better option to me.

Log in or register to post comments

Comment #3

moshe weitzman CreditAttribution: moshe weitzman commented 2 June 2005 at 11:15

well, we can't store it in the sessions table because many crawlers don't persist cookies and thus get a new session for every request. so we would have to store in a new table and I figured that I didn't need to add a new query for every page view. it would be a fast query, so perhaps a new table is OK. any more comments about variable table versus new table?

Log in or register to post comments

Comment #4

breyten CreditAttribution: breyten commented 2 June 2005 at 12:14

Having a table would allow for a simple query like:

$result = db_query("SELECT * FROM {banned_hosts} WHERE ipaddr = '%s'", $_SERVER["REMOTE_ADDR"]);

which is probably faster than a combined in_array and array_keys I suppose.

Log in or register to post comments

Comment #5

Dries CreditAttribution: Dries commented 3 June 2005 at 12:02

How about including this in the 'administer - access control - account rules' page (?q=admin/access/rules). We'd have to rephrase, and possibly rework it a little to extend the e-mail and username rules with IP/hostname rules but it sounds like a logical place to maintain IP-bans. In addition, we would be able to reuse the existing SQL table. (Let's not use variables!) Thoughts?

Log in or register to post comments

Comment #6

breyten CreditAttribution: breyten commented 3 June 2005 at 13:40

Only a +1. It sounds very sensible :)

Log in or register to post comments

Comment #7

moshe weitzman CreditAttribution: moshe weitzman commented 5 June 2005 at 22:18

Component:

statistics.module

» user.module

File	Size
ban_1.diff	10.5 KB

As suggested, this version integrates nicely into the admin/access/rules pages. You are now able to ban hosts from statistics page or the access rules page. I think there are use cases for both.

I moved renamed user_deny() tp drupal_deny() and moved it to bootstrap.inc in order to let us block hosts during the bootstrap. We check for a blocked host right after loading the database APi, which is the earliest possible moment.

This feature makes it possible now to restrict your site to a few approved IP addresses (or a range of addresses). Might be useful for high security sites.

Log in or register to post comments

Comment #8

Jaza CreditAttribution: Jaza commented 6 June 2005 at 02:30

This is a great start, but I think that this feature still needs improvement before it's ready to be used by joe shmoe administrator. The biggest danger that I foresee is administrators banning IPs that they really shouldn't be banning. "Great, so you've banned the aggressive bot from Kazakhstan, that put your throttle up to 3 when it slammed your site last night. But you've also banned GoogleBot! Now your site won't get indexed!"

There need to be mechanisms in place to stop this from happening. Possibilities:

- IP lookup, so you can see the resolved hostname before banning the host. Perhaps show the resolved hostname on the confirm dialog, e.g. "Are you sure you want to ban the IP 64.68.82.14 (crawler10.googlebot.com)?"
- Automatic rather than (or as well as) manual banning. This might create more problems than it solves, but in the long-term it's a better solution. Perhaps something similar to the filtering mechanisms used by the spam module?

But overall, +1 for this patch, as long as people know what they're doing when they use it.

Log in or register to post comments

Comment #9

moshe weitzman CreditAttribution: moshe weitzman commented 6 June 2005 at 03:38

File	Size
ban_2.diff	10.49 KB

same patch, but removing some debug comments which slipped in

Log in or register to post comments

Comment #10

Dries CreditAttribution: Dries commented 6 June 2005 at 18:13

Moshe, can you do one more round of updates? The patch can and will go in after that.

1. q=admin/access/rules/check needs to be updated. Maybe use form_group() too.

2. The ban/unban URL in statistics.module is really ugly and not very Drupal-ish. Mind to tidy up the URL scheme. (Haven't checked but do we perform a check_url() on that URL with user data? Might be taken care of at a lower level.)

3. I'd rename ip_address to either hostname (what do we use in other tables?) or ip.

Otherwise this works perfect; I just banned myself. ;)