Merge lp:~widelands-dev/widelands-website/anti_spam_3 into lp:widelands-website

Proposed by kaputtnik
Status: Merged
Merged at revision: 429
Proposed branch: lp:~widelands-dev/widelands-website/anti_spam_3
Merge into: lp:widelands-website
Diff against target: 229 lines (+83/-34)
5 files modified
mainpage/views.py (+0/-1)
pybb/admin.py (+22/-4)
pybb/forms.py (+42/-7)
pybb/models.py (+14/-0)
pybb/views.py (+5/-22)
To merge this branch: bzr merge lp:~widelands-dev/widelands-website/anti_spam_3
Reviewer Review Type Date Requested Status
SirVer Approve
Review via email: mp+309167@code.launchpad.net

Description of the change

A bunch of changes:

- Moved the spam check from pybb.views.py to pybb.forms.py
- Prevent e-mails and notifications to users if spam is detected
- Instead inform settings.ADMINS per E-Mail of potential spam. The E-mail contains the topic name and the whole post with a link to the admin page pybb/post
- Importand: Deleting a post over the admin page pybb/post uses now the method of the model instead of Django's default action. This prevents index errors if a topic has no post.
- Added a method to pybb.Post to unhide posts. This is currently callable only over the admin site but having this in pybb.Post makes it available over a button in the posts view (just like 'Stick topic' or similar)
- Changed the views in the admin pages as follows:
   - Added hidden property to the list of pybb/post
   - Added post_count to list in pybb/topic
   - Added the above admin actions to pybb/post
- If an ADMIN decides of no spam to a hidden post and unhide it over the admin action 'Unhide post and inform subscribers', the usual notices and E-mails are send when the post got unhided.
- Added a regex for International phone numbers and used it to search for in topic_name and post_body. The regex was made by janus and tested against the gathered spam-posts. Thanks janus :-)

To post a comment you must log in.
Revision history for this message
SirVer (sirver) wrote :

Instead of sending an email immediately and for each post, how about adding a django command that sends an email if there are hidden posts every night. That way even us admins would not be drowned by the SPAM and we could delete accordingly.

one suggestion in the code, otherwise code lgtm.

review: Approve
Revision history for this message
kaputtnik (franku) wrote :

> Instead of sending an email immediately and for each post, how about adding a
> django command that sends an email if there are hidden posts every night. That
> way even us admins would not be drowned by the SPAM and we could delete
> accordingly.

Yes that would be good if all hidden posts are really spam. In case of no Spam the post should be unhided as soon as possible imho. If one is posting something in the morning which is marked as spam accidentally, he has to wait until the next morning to unhide it with your suggestion. Maybe this is a rare use case, but we should not forget the possibility of accidentally marked spam.

As an alternative we could use another group of email recipients. E.g. there is a group 'forum_admin' which may could be used.

>
> one suggestion in the code, otherwise code lgtm.

Thanks. The code is similar to the 'legal_notice' email so it would make sense to have a global method. The method should be e.g. in wl_utils.py and accept various parameters. But i guess this is not what you meant :-)

I add a comment to the code and merge it tomorrow if no one complains.

Revision history for this message
GunChleoc (gunchleoc) wrote :

How about setting the e-mail notification interval to 1 hour instead of 1 day, with sending the first notification immediately? This way, if we get 200 spam a day, there will be only 24 e-mails, and legitimate users still won't have to wait for too long.

436. By kaputtnik

added some comments; changed the admin link in email

Revision history for this message
kaputtnik (franku) wrote :

Sounds good to me. I guess doing this would be better if we move the spam check to an own directory/app. So the spam check could also be done like

is_spam = check_spam(text_to_check)
if is_spam:
   do something

This is then independent from pybb and could be used also for the wiki

But not in this branch. I merge and deploy this now to have the regex for intern phone numbers in. The spam of today had been hidden if we had this check formerly :-S

Btw: Thanks for cleaning up those spams

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'mainpage/views.py'
2--- mainpage/views.py 2016-08-07 18:35:30 +0000
3+++ mainpage/views.py 2016-10-26 15:28:36 +0000
4@@ -37,7 +37,6 @@
5 recipients = []
6 for recipient in INQUIRY_RECIPIENTS:
7 recipients.append(recipient[1])
8- print('recipeients: ', recipients)
9
10 send_mail(subject, message, sender,
11 recipients, fail_silently=False)
12
13=== modified file 'pybb/admin.py'
14--- pybb/admin.py 2016-10-13 15:50:48 +0000
15+++ pybb/admin.py 2016-10-26 15:28:36 +0000
16@@ -3,6 +3,23 @@
17 from django.contrib import admin
18 from pybb.models import Category, Forum, Topic, Post, Read
19
20+def delete_selected(modeladmin, request, queryset):
21+ """ Overwritten Django's default action to delete a post.
22+
23+ This action uses the delete() method of the post model.
24+ This ensures also deleting a topic if neccesary, preventing
25+ index-errors if a topic has only one post.
26+ """
27+ for obj in queryset:
28+ obj.delete()
29+delete_selected.short_description = 'Delete selected posts'
30+
31+def unhide_post(modeladmin, request, queryset):
32+ "Unhide post(s) and inform subscribers"
33+ for obj in queryset:
34+ obj.unhide_post()
35+unhide_post.short_description = 'Unhide post and inform subscribers'
36+
37 class CategoryAdmin(admin.ModelAdmin):
38 list_display = ['name', 'position', 'forum_count']
39 list_per_page = 20
40@@ -27,7 +44,7 @@
41 )
42
43 class TopicAdmin(admin.ModelAdmin):
44- list_display = ['name', 'forum', 'created', 'head']
45+ list_display = ['name', 'forum', 'created', 'head', 'is_hidden']
46 list_per_page = 20
47 ordering = ['-created']
48 date_hierarchy = 'created'
49@@ -45,19 +62,20 @@
50 )
51
52 class PostAdmin(admin.ModelAdmin):
53- list_display = ['topic', 'user', 'created', 'updated', 'summary']
54+ list_display = ['summary', 'topic', 'user', 'created', 'hidden']
55 list_per_page = 20
56 ordering = ['-created']
57 date_hierarchy = 'created'
58 search_fields = ['body']
59+ actions = [delete_selected, unhide_post]
60 fieldsets = (
61 (None, {
62- 'fields': ('topic', 'user', 'markup')
63+ 'fields': ('topic', 'user', 'markup', 'hidden')
64 }
65 ),
66 (_('Additional options'), {
67 'classes': ('collapse',),
68- 'fields' : (('created', 'updated'), 'user_ip', 'hidden')
69+ 'fields' : (('created', 'updated'), 'user_ip' )
70 }
71 ),
72 (_('Message'), {
73
74=== modified file 'pybb/forms.py'
75--- pybb/forms.py 2016-10-03 14:10:48 +0000
76+++ pybb/forms.py 2016-10-26 15:28:36 +0000
77@@ -9,8 +9,12 @@
78
79 from pybb.models import Topic, Post, PrivateMessage, Attachment
80 from pybb import settings as pybb_settings
81-
82+from django.conf import settings
83 from notification.models import send
84+from django.core.mail import send_mail
85+from django.contrib.sites.models import Site
86+
87+INTERN_PHONE_NR = re.compile('([0]{2}|[\+\@\ ]{1})\d{2}[\ \-\=]{,1}\d{8,11}')
88
89 class AddPostForm(forms.ModelForm):
90 name = forms.CharField(label=_('Subject'))
91@@ -63,20 +67,51 @@
92 topic_is_new = False
93 topic = self.topic
94
95+ # Check for spam
96+ # TODO: This is currently a simple keyword search. Maybe add akismet check here
97+ # and move it to an own directory/app
98+ hidden = False
99+ text = self.cleaned_data['body']
100+ if any(x in text.lower() for x in settings.ANTI_SPAM_BODY):
101+ hidden = True
102+
103+ if re.search(INTERN_PHONE_NR, text):
104+ hidden = True
105+
106+ if topic_is_new:
107+ text = self.cleaned_data['name']
108+ if any(x in text.lower() for x in settings.ANTI_SPAM_TOPIC):
109+ hidden = True
110+ if re.search(INTERN_PHONE_NR, text):
111+ hidden = True
112+
113 post = Post(topic=topic, user=self.user, user_ip=self.ip,
114 markup=self.cleaned_data['markup'],
115- body=self.cleaned_data['body'])
116+ body=self.cleaned_data['body'], hidden=hidden)
117 post.save(*args, **kwargs)
118
119 if pybb_settings.ATTACHMENT_ENABLE:
120 self.save_attachment(post, self.cleaned_data['attachment'])
121
122- if topic_is_new:
123- send(User.objects.all(), "forum_new_topic",
124- {'topic': topic, 'post':post, 'user':topic.user})
125+ if not hidden:
126+ if topic_is_new:
127+ send(User.objects.all(), "forum_new_topic",
128+ {'topic': topic, 'post':post, 'user':topic.user})
129+ else:
130+ send(self.topic.subscribers.all(), "forum_new_post",
131+ {'post':post, 'topic':topic, 'user':post.user})
132 else:
133- send(self.topic.subscribers.all(), "forum_new_post",
134- {'post':post, 'topic':topic, 'user':post.user})
135+ # Inform admins of a hidden post
136+ # Moving this to an own method makes the code clearer
137+ # There is also similar code in mainpage.views.py
138+ recipients = [addr[1] for addr in settings.ADMINS]
139+ message = '\n'.join(['Hidden post:',
140+ 'Topic name: ' + topic.name,
141+ 'Post body: ' + post.body,
142+ 'Admin page: http://'+ Site.objects.get_current().domain + '/admin/login/?next=/admin/pybb/post/'])
143+ send_mail('A post was hidden by spam check', message, 'pybb@widelands.org',
144+ recipients, fail_silently=False)
145+
146 return post
147
148
149
150=== modified file 'pybb/models.py'
151--- pybb/models.py 2016-10-08 09:48:25 +0000
152+++ pybb/models.py 2016-10-26 15:28:36 +0000
153@@ -15,6 +15,8 @@
154 from pybb import settings as pybb_settings
155
156 from django.conf import settings
157+from notification.models import send
158+from django.contrib.auth.models import User
159 if settings.USE_SPHINX:
160 from djangosphinx.models import SphinxSearch
161
162@@ -260,6 +262,18 @@
163 def get_absolute_url(self):
164 return reverse('pybb_post', args=[self.id])
165
166+ def unhide_post(self):
167+ "Unhide post(s) and inform subscribers"
168+ self.hidden = False
169+ self.save()
170+ if self.topic.post_count == 1:
171+ # The topic is new
172+ send(User.objects.all(), "forum_new_topic",
173+ {'topic': self.topic, 'post':self, 'user':self.topic.user})
174+ else:
175+ # Inform topic subscribers
176+ send(self.topic.subscribers.all(), "forum_new_post",
177+ {'post':self, 'topic':self.topic, 'user':self.user})
178
179 def delete(self, *args, **kwargs):
180 self_id = self.id
181
182=== modified file 'pybb/views.py'
183--- pybb/views.py 2016-10-12 20:42:21 +0000
184+++ pybb/views.py 2016-10-26 15:28:36 +0000
185@@ -18,7 +18,7 @@
186 from pybb.forms import AddPostForm, EditPostForm, UserSearchForm
187 from pybb import settings as pybb_settings
188 from pybb.orm import load_related
189-from django.conf import settings
190+
191 from wl_utils import get_real_ip
192
193 try:
194@@ -161,31 +161,14 @@
195 initial={'markup': "markdown", 'body': quote})
196
197 if form.is_valid():
198- # TODO: Add akismet check here
199- spam = False
200-
201- # Check in post text.
202- text = form.cleaned_data['body']
203- if any(x in text.lower() for x in settings.ANTI_SPAM_BODY):
204- spam = True
205-
206- # Check in topic subject ('name' is empty if this a post to an existing topic)
207- text = form.cleaned_data['name']
208- if text != '':
209- # This is a new topic
210- if any(x in text.lower() for x in settings.ANTI_SPAM_TOPIC):
211- spam = True
212-
213 post = form.save()
214- if spam:
215- # Hide the post against normal users
216- post.hidden = True
217- post.save()
218+ if not topic:
219+ post.topic.subscribers.add(request.user)
220+
221+ if post.hidden:
222 # Redirect to an info page to inform the user
223 return HttpResponseRedirect('pybb_moderate_info')
224
225- if not topic:
226- post.topic.subscribers.add(request.user)
227 return HttpResponseRedirect(post.get_absolute_url())
228
229 if topic:

Subscribers

People subscribed via source and target branches