Moodle
  1. Moodle
  2. MDL-29067

Expand the spam cleaner search locations

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.1.1, 2.2, 2.3
    • Fix Version/s: 2.1.4, 2.2
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide
      1. Add "World of Warcraft imposes casino ban" in
        • As Admin, user description (Site administration->Users -> accounts -> Browse list of users, Select user and click on Settings -> edit user)
        • As user, Comments
        • As user, Blog post
        • As user, send message to different users
      2. Login as admin
      3. Go to Spam page (Site administration -> Reports -> Spam cleaner)
      4. Auto detect common spam patterns
      5. check if entry with casino is visible (Only one entry will be visible for one user)
      6. Try with different keywords and make sure it's visible in spam cleaner.

      NOTE:
      Make sure to check it on 21, as the patch was not cherry-picked.

      Show
      Add "World of Warcraft imposes casino ban" in As Admin, user description (Site administration->Users -> accounts -> Browse list of users, Select user and click on Settings -> edit user) As user, Comments As user, Blog post As user, send message to different users Login as admin Go to Spam page (Site administration -> Reports -> Spam cleaner) Auto detect common spam patterns check if entry with casino is visible (Only one entry will be visible for one user) Try with different keywords and make sure it's visible in spam cleaner. NOTE: Make sure to check it on 21, as the patch was not cherry-picked.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      wip-mdl-29067
    • Rank:
      18590

      Description

      Currently the spam cleaner tool only searches in user profiles. It would be great if it also searched messages (both unread and read), comments, blog entries (including content from external blogs), tags, forum posts, glossary entries, database activity entries etc.

        Issue Links

          Activity

          Hide
          Helen Foster added a comment -

          Having spent over 2 hours this morning cleaning up a load of spam in the forums on moodle.org, laboriously deleting threads one by one, I'm hoping this improvement can be implemented one day.

          Show
          Helen Foster added a comment - Having spent over 2 hours this morning cleaning up a load of spam in the forums on moodle.org, laboriously deleting threads one by one, I'm hoping this improvement can be implemented one day.
          Hide
          Helen Foster added a comment -

          Another note about spam cleanup on moodle.org being very time-consuming without access to phpmyadmin or an improved spam cleaner tool.

          One spammer added multiple spam comments to almost every entry in http://moodle.org/books. Unfortunately the comments report was no help at all in deleting them.

          Show
          Helen Foster added a comment - Another note about spam cleanup on moodle.org being very time-consuming without access to phpmyadmin or an improved spam cleaner tool. One spammer added multiple spam comments to almost every entry in http://moodle.org/books . Unfortunately the comments report was no help at all in deleting them.
          Hide
          Martin Dougiamas added a comment -

          Raj are you able to have a look at this soon? It's becoming quite an issue on moodle.org.

          We can chat about it anytime.

          Show
          Martin Dougiamas added a comment - Raj are you able to have a look at this soon? It's becoming quite an issue on moodle.org. We can chat about it anytime.
          Hide
          Rajesh Taneja added a comment -

          Sure Martin,
          I will look into this

          Show
          Rajesh Taneja added a comment - Sure Martin, I will look into this
          Hide
          Rajesh Taneja added a comment - - edited

          Looking at Spam Cleaner, it uses simple sql query on user-description and post-summary. I can add more search sql, but my only concern is server load by adding all these sql.
          IMO:

          1. We might want to improve this by parsing all post etc. and report spams, rather then searching for it.
          2. Blacklisted words should be modifiable rather then static array.
          Show
          Rajesh Taneja added a comment - - edited Looking at Spam Cleaner, it uses simple sql query on user-description and post-summary. I can add more search sql, but my only concern is server load by adding all these sql. IMO: We might want to improve this by parsing all post etc. and report spams, rather then searching for it. Blacklisted words should be modifiable rather then static array.
          Hide
          Martin Dougiamas added a comment -

          Totally agree that parsing at post time is a better idea, but that's another tool.

          This tool is a sledgehammer - the approach here is

          1) Identify definite spammers, and
          2) Zap everything we can think of for them

          Helen said that it would good if we searched for (at least) comments, messages and forum posts to identify spammers.

          If we could just print a number for each user with the number of matches that would be good.

          However, Helen also said that she usually knows the spammers, and just needs a tool to delete all their stuff.

          So it's probably a good idea to first make a new block for moodle.org that we can put on the user profile page, with a nuclear bomb button that deletes all the content this user ever produced and also rewrites their profile, changes their auth to nologin and so on. Easier than modifying something in core.

          Helen can you spec this on a separate issue?

          Show
          Martin Dougiamas added a comment - Totally agree that parsing at post time is a better idea, but that's another tool. This tool is a sledgehammer - the approach here is 1) Identify definite spammers, and 2) Zap everything we can think of for them Helen said that it would good if we searched for (at least) comments, messages and forum posts to identify spammers. If we could just print a number for each user with the number of matches that would be good. However, Helen also said that she usually knows the spammers, and just needs a tool to delete all their stuff. So it's probably a good idea to first make a new block for moodle.org that we can put on the user profile page, with a nuclear bomb button that deletes all the content this user ever produced and also rewrites their profile, changes their auth to nologin and so on. Easier than modifying something in core. Helen can you spec this on a separate issue?
          Hide
          Helen Foster added a comment -

          Thanks Martin, I've created MDLSITE-1619 and MDLSITE-1620.

          Show
          Helen Foster added a comment - Thanks Martin, I've created MDLSITE-1619 and MDLSITE-1620 .
          Hide
          Rajesh Taneja added a comment -

          There are two commits:

          1. Added sql for comments, messages and forum posts.
          2. Added user_id and number of matches for each section with link. Not sure if this is back-portable.
            Also, spam cleaner can be further cleaned and sql can be optimized. Probably in another bug/improvement.
          Show
          Rajesh Taneja added a comment - There are two commits: Added sql for comments, messages and forum posts. Added user_id and number of matches for each section with link. Not sure if this is back-portable. Also, spam cleaner can be further cleaned and sql can be optimized. Probably in another bug/improvement.
          Hide
          Ankit Agarwal added a comment -

          Hi Raj,
          This is a great improvement!
          Here are a few suggestions:-

          • post.subject (blog subject) should be searched as well not just post.summary IMO
          • the way user->description is used in filter_user function seems very hacky to me, might be a better to use a separate variable instead
            title
          • since we are doing this in fill_user_entry() when user->id is already set:-
            $SESSION->users_result[$user->id]->count[$spamin] += 1;
            

            $SESSION->users_result[$user->id]->description will have the value of first spam encountered always.

          • filter_user function should also have an if condition for the case when the spam came is from user's profile.
          • Does filter_user function really need $count variable passed?
          • +1 to built a tool that can handle this during posting. There are a few proposed structure for the tool open for discussion in forums.
            Thanks
          Show
          Ankit Agarwal added a comment - Hi Raj, This is a great improvement! Here are a few suggestions:- post.subject (blog subject) should be searched as well not just post.summary IMO the way user->description is used in filter_user function seems very hacky to me, might be a better to use a separate variable instead title since we are doing this in fill_user_entry() when user->id is already set:- $SESSION->users_result[$user->id]->count[$spamin] += 1; $SESSION->users_result [$user->id] ->description will have the value of first spam encountered always. filter_user function should also have an if condition for the case when the spam came is from user's profile. Does filter_user function really need $count variable passed? +1 to built a tool that can handle this during posting. There are a few proposed structure for the tool open for discussion in forums. Thanks
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback Ankit,

          I know user->description usage is a bit hacky, I tried not to deviate from current implementation. Also, as per count variable, it was kept for BC. Why do you think, if statement is needed in filter_user?

          I will add post.subject in search. For rest of the point, can you please confirm, if I should consider changing the code. Only reason, I kept that is to make minimum changes and make it BC. Although second commit is quite a change in functionality and not sure, if it should be done with minimum changes (As I have done it currently.)

          Show
          Rajesh Taneja added a comment - Thanks for the feedback Ankit, I know user->description usage is a bit hacky, I tried not to deviate from current implementation. Also, as per count variable, it was kept for BC. Why do you think, if statement is needed in filter_user? I will add post.subject in search. For rest of the point, can you please confirm, if I should consider changing the code. Only reason, I kept that is to make minimum changes and make it BC. Although second commit is quite a change in functionality and not sure, if it should be done with minimum changes (As I have done it currently.)
          Hide
          Ankit Agarwal added a comment - - edited

          Hi Raj,

          • As you can see in the attached pic we have "From blog post:" etc to specify where the keyword was found. But when its found in the description, it just displays the text without any header explaining where it was found, that is the reason I was asking another "if clause" to be included for this case.
          • IMO, we should make the changes required, the $user->direction things definitely needs to be changed. If not in Stables, than at least in the master. But again that's just me
            Thanks
            Edit: the attached image somehow ended being way too small in size, open in a separate window to see the enlarged version
          Show
          Ankit Agarwal added a comment - - edited Hi Raj, As you can see in the attached pic we have "From blog post:" etc to specify where the keyword was found. But when its found in the description, it just displays the text without any header explaining where it was found, that is the reason I was asking another "if clause" to be included for this case. IMO, we should make the changes required, the $user->direction things definitely needs to be changed. If not in Stables, than at least in the master. But again that's just me Thanks Edit: the attached image somehow ended being way too small in size, open in a separate window to see the enlarged version
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback Ankit,

          I have removed my second commit, as count and showing all spams, can't be done without a design change. Will open another bug to handle it nicely. Can you please review the new patch.

          Show
          Rajesh Taneja added a comment - Thanks for the feedback Ankit, I have removed my second commit, as count and showing all spams, can't be done without a design change. Will open another bug to handle it nicely. Can you please review the new patch.
          Hide
          Ankit Agarwal added a comment -

          Looks good to me!
          Thanks

          Show
          Ankit Agarwal added a comment - Looks good to me! Thanks
          Hide
          Rajesh Taneja added a comment -

          Thanks Ankit,
          Pushing it for integration review.

          Show
          Rajesh Taneja added a comment - Thanks Ankit, Pushing it for integration review.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated... thanks!

          BUT, this is clearly a dead-end. We cannot keep adding queries there like crazy in that way IMO. They all are full table scans + OR operations, really not sustainable IMO.

          We should prospect other alternatives asap IMO.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated... thanks! BUT, this is clearly a dead-end. We cannot keep adding queries there like crazy in that way IMO. They all are full table scans + OR operations, really not sustainable IMO. We should prospect other alternatives asap IMO. Ciao
          Hide
          Ankit Agarwal added a comment -

          works as expected
          Thanks

          Show
          Ankit Agarwal added a comment - works as expected Thanks
          Hide
          Helen Foster added a comment -

          Sorry I've not had time to test this cool improvement, but just wondering whether anyone can confirm that if you search for the word "ugg" that posts, comments etc. containing the word "suggestion" are NOT listed in the search results.

          Show
          Helen Foster added a comment - Sorry I've not had time to test this cool improvement, but just wondering whether anyone can confirm that if you search for the word "ugg" that posts, comments etc. containing the word "suggestion" are NOT listed in the search results.
          Hide
          Ankit Agarwal added a comment -

          Hi Helen,
          Looking at the code, we use simple wild card match %keyword%, which means 'suggestion' will be included if you are searching for 'ugg' as far as I can predict.
          Spam cleaner definitely needs a lot more major improvements.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Helen, Looking at the code, we use simple wild card match %keyword%, which means 'suggestion' will be included if you are searching for 'ugg' as far as I can predict. Spam cleaner definitely needs a lot more major improvements. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

          Now... disconnect, relax and enjoy the next days, yay!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: