Project page
https://drupal.org/sandbox/programadoresweb/2062343
Installation
git clone --branch 6.x-1.x http://git.drupal.org/sandbox/programadoresweb/2062343.git antispammer_bot
Issue summary
My main motivation to write this module was having thousands of comments and nodes published after a week off. What it is worst, a single spammer can affect your server, even force it down because of publishing some millions of comments/nodes in your site.
Said that, this is Not Just Another Spam Module which detects spam, but a bot itself which detects if the user is a possible spammer and take some actions, like:
- block the user account
- un-publish all the contents of this user
- notify the site administrator
There are nodes which control the number of comments or nodes a user can publish, like:
- https://drupal.org/project/node_limit
- https://drupal.org/project/node_limitnumber
- https://drupal.org/project/user_quota
- Domain Rights Management Module https://drupal.org/node/1837270
- https://drupal.org/node/2081543#comment-7832947
- https://drupal.org/node/1908284#comment-7028838
- https://drupal.org/node/2065035#comment-7859035
- https://drupal.org/node/2087205#comment-7861709
- https://drupal.org/node/2088341#comment-7858117
- https://drupal.org/node/2070535#comment-7861759
- https://drupal.org/node/2087025#comment-7862959
but this is a bot module, not not a quota module. It is a way of preventing spam flood attacks and reacts depending on the attack. For example, if a user or bot tries to publish 2 nodes in a threshold bellow a number of seconds (defined by the admin), the module will block the user and unpublish all his comments. Forget at last of having thousands of spam comments in your approval queue, Antispammer Bot will block this user/bot before this happens again.
Some things can be configured to adapt to the kind of traffic or user in every site. You can configure things like the time frame allowed between two posts, maximum posts per non-safe user before the module should take an action, etc... Of course you add a "secure" role which will be able to publish without the previous limitations.
New users can also have a threshold beyond which the system will automatically unpublish their posts and block the user himself. That way, for example, any new user publishing his first post will be blocked, posts unpublished and moved into moderation.
Contributions with other projects
https://drupal.org/node/1959338 (Drupal 8 Core)
https://drupal.org/node/1912834 (Code per node)
https://drupal.org/node/1978436 (Path auto)
Manual reviews of other projects
Pledge#: I would be updating this module for Drupal#8 and Drupal#7 also
Comments
Comment #1
sergei_brill CreditAttribution: sergei_brill commented1. antispam_bot.module, line 86. Use empty array as a default value for variable. After installation I got warning: Invalid argument supplied for foreach() in /home/sergei/web/drupal_6_test/includes/form.inc on line 1217. on bot settings page.
2. antispam_bot.module, line 53-55. You add submit callback to the form, there is no function with name 'antispammer_bot'
3. function antispammer_bot_settings_form is not used anywhere.
4. warning: Missing argument 2 for variable_get(), called in .../antispammer_bot/antispammer_bot.module on line 166. Also there is few other case where 2nd argument is missing for variable_get()
5. I limited content per non safe user by 1. I created 1 node. When I tried to edit the node, antispammer bot didn't allow me to do that. I think, it is wrong behavior. Try to use $op === 'validate' instead of $op === 'prepare'
6. I changed limit to 30 and added 2nd node. After I was able to add nodes, edit 1st node, but was not able to edit 2nd node. It showed 'Your account must be reviewed before being able to edit or create new contents. Contact the admin in case of doubt'
As I see, the module require much work and tests. Please, finish work on base features of module. Reviewing of modules with many bugs take a lot time.
Comment #2
alexmoreno CreditAttribution: alexmoreno commentedHi sergei,
thanks a lot for your time in the review.
1. I have never seen the 1st issue, but basically because I have always worked in an environment with roles. Probably that was the problem, I have added a couple of lines to check what could it happen if there ara no roles in the site:
2. Deleted these lines, you are right, you do not need a submit button here.
3. Deleted too, I was thinking a couple of steps forward in the evolution of the module.
4. Added the argument in all variable_get functions.
5. It is a deliberate behaviour. The bots consumes a lot of time in your site, and resources. Removing them completely the access to your forms limits the access to sensible sections of your site, and processing time in your server (of thousand of bots trying to submit forms). They simply have no access there.
I can be wrong with the approach of course, I'll be very happy to discuss it.
6. What it has happen here is that the time threshold is too short, 1 minute I think I remember. So, you have probably submitted two nodes in the same minute, so the bot has flagged you as possible spammer. I will double check this anyway.
What I am going to do is add the possibility to select the seconds between posts in which we can flag a user as spammer bot, so the admin site will be able to change this.
I am sorry you felt angry Sergei. I have tested the module in different installations and I did not get some of your problems, but I will try to be more careful in my next submit.
Thanks a lot for your time, I really appreciate it Sergei.
Comment #3
sergei_brill CreditAttribution: sergei_brill commentedNo, problem is in other. you need use
'#default_value' => variable_get('antispammer_bot_safe_roles', array()),
instead of
'#default_value' => variable_get('antispammer_bot_safe_roles', 0),
Comment #4
alexmoreno CreditAttribution: alexmoreno commentedthanks a lot for your work sergei. I will try to help reviewing your module too.
problem in #3 corrected. I have also added a "block user" functionality when the time limit is passed.
Comment #5
alexmoreno CreditAttribution: alexmoreno commentedMore reviews done:
http://drupal.org/node/1908284
https://drupal.org/node/2083347
https://drupal.org/node/2083109
Comment #5.0
alexmoreno CreditAttribution: alexmoreno commentedCorrected urls
Comment #6
pgautam CreditAttribution: pgautam commentedHi,
1) Please specify file responsibility in @file comment in antispammer_bot.module.
2) Function antispammer_bot_settings() form titles should be in t() function for module antispammer_bot.module.
3) hook_permission missing form module file for ( 'access arguments' => array("administer Antispammer bot") in line no 33).
Thanks,
Paritosh Gautam
Comment #7
alexmoreno CreditAttribution: alexmoreno commentedHi pgautam,
thanks a lot for your help. This are my changes:
1)
2) All added
3) Sorry, what do you mean? The hook is there:
The form also have the permissions and in fact they seems to work fine:
'access arguments' => array("administer Antispammer bot"),
Thanks again.
Comment #8
joshi.rohit100Please fix below :-
(1) You must remove all the variables defined in system_settings_form() in hook_uninstall.
(2) Your hook_menu implementation should use a file to define the form as :-
$items['admin/settings/antispammer_bot'] = array(
'title' => 'Antispammer bot settings',
'description' => 'Antispammer bot settings.',
'page callback' => 'drupal_get_form',
'page arguments' => array('antispammer_bot_settings'),
'file' => 'antispammer_bot.admin.inc' //file to define form
'access arguments' => array("administer Antispammer bot"),
);
As it will not include the function on each page load.
Suggesstions :-
All the functions you have created in module, can be defined in separate file. Module should contain only drupal hooks or some related functionality.
Cheers
Comment #9
joshi.rohit100Sorry forgot to change the status.
Comment #10
joshi.rohit100Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #11
alexmoreno CreditAttribution: alexmoreno commentedHi Joshi, and thanks a lot for your help.
1) done as suggested,
2) added includes/antispammer_bot.admin.inc
Regarding the suggestions, which one is the best way to load this separate file? I have seen a lot of modules and I use to use the module_load_include like that:
module_load_include('inc', 'module', 'includes/file_to_load');
But, where this function should be loaded? At the beginning of the file or (as i guess) just when a function is going to use it?
Thanks again.
Comment #12
alexmoreno CreditAttribution: alexmoreno commentedok, everything moved to .inc (non hook of course), uninstall in .install file and everything tested and "cleaned" :-).
Thanks for your suggestions and help joshi.
Comment #13
alexmoreno CreditAttribution: alexmoreno commentedI already added 3 new reviews, but anyway, I have been collaborating lastly, I have enough reviews to share :-):
Thanks.
Comment #14
bappa.sarkar CreditAttribution: bappa.sarkar commentedManual Review
Rather than passing entire user object pass only $user->roles when you are calling _anmmer_bot_is_saftispae_role($user) in below function
Then you can modify the function function _antispammer_bot_is_safe_role($user) as below
Comment #15
alexmoreno CreditAttribution: alexmoreno commentedHi bappa,
thanks for your review.
Added the a user paramter in _antispammer_bot_violates_limits, so we can reuse the same user object:
I need to pass all the user so we also use uid, not just roles. So the _antispammer_bot_user_permissions function also changes:
and the last one, as suggested:
thanks again.
Comment #16
alexmoreno CreditAttribution: alexmoreno commentedI am finally not using this:
as it was throwing a warning because a non used variable. The final code is:
and global user is loaded inside the antispammer_bot_is_active function.
Comment #17
bappa.sarkar CreditAttribution: bappa.sarkar commentedPlease change below code in function _antispammer_bot_is_safe_role($roles)
Please replace with below code as the foreach is time consuming and once after $is_safe = TRUE no need to loop through till end.
Comment #18
alexmoreno CreditAttribution: alexmoreno commentedbappa, your ammend throws this error:
PHP Fatal error: Can't use function return value in write context in /var/www/html/bounty/sites/all/modules/antispammer_bot/includes/antispammer_bot.inc on line 113
thank you.
Comment #19
alexmoreno CreditAttribution: alexmoreno commentedsolution:
Comment #20
bappa.sarkar CreditAttribution: bappa.sarkar commenteduse below
Comment #21
bappa.sarkar CreditAttribution: bappa.sarkar commentedYou dont need the second if
Comment #22
alexmoreno CreditAttribution: alexmoreno commentedyep, the review tool throw the issue of non used variable. I forgot to remove it.
Thanks bappa, quite interesting changes, very performance focused :-).
Comment #23
alexmoreno CreditAttribution: alexmoreno commenteddoes it really worth to use intersect bappa? I mean, this array will never be bigger than 10 or 20 elements, and array_intersect is good with big arrays. Maybe a while function would be better?
Comment #24
bappa.sarkar CreditAttribution: bappa.sarkar commentedWell I think avoiding loop/ nested loop is always better if you have an option to use php primitive function.
Thanks for changing :)
Comment #25
alexmoreno CreditAttribution: alexmoreno commentedneeds review
Comment #26
joshi.rohit100Hi @urwen
Reviewd points :-
(1) In your _antispammer_bot_latest_post_date($uid) method, if you only required created field, then you should not use '*' in your db_query.
(2) In system settings form, you are setting default value for field 'antispammer_bot_peruser_nodes_threshold' is 2 but in method '_antispammer_bot_violates_amount_limit' , you are seting default 4 which is quite ambiguous.
look above issues.
cheers
Comment #27
alexmoreno CreditAttribution: alexmoreno commentedHi joshi.rohit100,
did as recommended and tested, thanks for your suggestions.
Comment #28
joshi.rohit100Hello @urwen.
In antispammer_bot.admin.inc file, you referred about INSTALL.txt file for admin settings but there is no INSTALL.txt file.
Also in antispammer_bot.inc, method _antispammer_bot_latest_post_date() is returning the last created node's timestamp but comment says that it will return the node.
Otherwise your module now looks fine to me but waiting for few more responses.
Comment #29
alexmoreno CreditAttribution: alexmoreno commentedvery good eye joshi.
Changes done, thank you very much for your help.
Comment #29.0
alexmoreno CreditAttribution: alexmoreno commentedPledge added
Comment #29.1
klausiremoved automated reviews
Comment #30
klausiRemoving review bonus tag, you have not done all manual reviews, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.
I also removed the automated reviews from the issue summary.
Comment #31
alexmoreno CreditAttribution: alexmoreno commentedHi Klausi,
I have done a lot of reviews, not just the published. Some of my lasts for exmaple are those:
https://drupal.org/node/2082901#comment-7849395
https://drupal.org/node/2085833#comment-7849461
http://drupal.org/node/1886184#comment-6987222
http://drupal.org/node/1885736#comment-6987374
http://drupal.org/node/1837270#comment-6987422
http://drupal.org/node/1721378#comment-6987532
Thank you.
Comment #32
alexmoreno CreditAttribution: alexmoreno commentedplease, note that some reviews start with a simple code review, but after that there are several tests in different posts, manual code reviews, recommendations, etc.
For example, yesterday:
https://drupal.org/node/2065035#comment-7859035
or that one: https://drupal.org/node/2088341#comment-7858117
Thanks for your time Klausi, and sorry for the confusion.
Comment #33
klausiSorry about that, please add all your valid review comment links to the issue summary, so that we can track them.
manual review:
But otherwise looks good to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Assigning to dman as he might have time to take a final look at this.
Comment #34
alexmoreno CreditAttribution: alexmoreno commentedThanks a lot klausi. I'll do the reviews and rest of changes this same evening. Anyway, I've learnt a lot reviewing code in some sprints and at home. I'll continue collaborating, if I can help you somehow just let me know.
Best regards.
Comment #34.0
alexmoreno CreditAttribution: alexmoreno commentedfixed item
Comment #34.1
alexmoreno CreditAttribution: alexmoreno commentednew reviews added
Comment #34.2
alexmoreno CreditAttribution: alexmoreno commentedadded one new review
Comment #35
alexmoreno CreditAttribution: alexmoreno commented1. done,
2. I love the change, it looks more simple and it uses the Drupal user permissions interface, thanks a lot klausi. Changes done, tested and commited.
I've just added a text message to explain how to bypass the limitations:
Maybe it is not 100% necessary.
New reviews added too, so bonus review tag is back with your permission :-).
Thanks a lot for your time, much appreciated.
Comment #35.0
alexmoreno CreditAttribution: alexmoreno commentedadded similar modules and detailed explanation of differences with the Antispammer bot module.
Comment #36
alexmoreno CreditAttribution: alexmoreno commentednew "fresh" review added in the issue summary: https://drupal.org/node/2087025#comment-7862959
Thanks
Comment #37
dman CreditAttribution: dman commentedVery good collaborative work in the reviews here! Great to see.
Visual review only :
* I read the whole project page through twice, but I'm still unclear on where this differs from some of the other methods. Surely a number of existing solutions also allow you to block user accounts based on bad behavior? But still, the work here seems extensive and sounds useful to you.
* It's a bit disappointing reviewing a 'new' module for Drupal6 this month. But there is no rule against it.
* Code stucture looks fine. Internal documentation is good.
* I'm curious that you needed to do all that date parsing in _antispammer_bot_violates_time_limit(). Surely just comparing timestamps directly would have saved you 12 lines of calculation?
* I won't complain about going direct to the database like you do here in D6, but that would need improvement in D7.
* I don't see any security concerns here, and I'll expect that any further user issues will be able to be found and tested in the issue queue.
Generally, I'm content with this.
BASED ON THE REVIEWS BY OTHERS ABOVE, and the RTBC status :
-----
Thanks for your contribution, urwen!
I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.
Here are some recommended readings to help with excellent maintainership:
You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!
Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
Thanks to the dedicated reviewer(s) as well.
Comment #38
dman CreditAttribution: dman commentedremoving bonus tag
Comment #39
alexmoreno CreditAttribution: alexmoreno commentedHi dman,
thanks a lot for your time and manual review. I will make the changes as you suggest, and I absolutely agree about D6. Some of my current projects are still under D6, but my plans once I finish next month in my current company is to migrate it to D7 and specially D8, which changes I reaaaaly love (twig, symfony, OO, ...).
Thank you everyone who has contributed with his time and effort: pgautam, sergei, joshi, bappa and of course, Klausi and dman. You all guys are the reason why Drupal is so great :-).
Warm regards.
Comment #40.0
(not verified) CreditAttribution: commentedadded a new review