You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2013/05/24 19:04:55 UTC
[19/50] git commit: [#5571] ticket:302 Spam moderation: comments
[#5571] ticket:302 Spam moderation: comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/9badd061
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/9badd061
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/9badd061
Branch: refs/heads/db/6255
Commit: 9badd061e9fa07a9ee3312249679cc464d780b64
Parents: 62e3582
Author: Yuriy Arhipov <yu...@yandex.ru>
Authored: Mon Apr 1 12:08:26 2013 +0400
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Wed May 22 20:54:26 2013 +0000
----------------------------------------------------------------------
Allura/allura/controllers/discuss.py | 4 +--
Allura/allura/lib/spam/__init__.py | 3 ++
Allura/allura/lib/spam/akismetfilter.py | 27 +++++++++++++--
Allura/allura/lib/spam/mollomfilter.py | 3 ++
Allura/allura/model/discuss.py | 18 +++++++++-
Allura/allura/tests/functional/test_discuss.py | 8 +++-
Allura/allura/tests/model/test_discussion.py | 22 ++++++++++++
Allura/allura/tests/unit/spam/test_akismet.py | 10 +++++
Allura/allura/tests/unit/spam/test_mollom.py | 26 ++++++++++++--
Allura/allura/tests/unit/spam/test_spam_filter.py | 3 ++
.../forgediscussion/controllers/root.py | 1 -
.../forgediscussion/tests/functional/test_forum.py | 13 +++----
.../forgetracker/tests/functional/test_root.py | 4 ++-
13 files changed, 119 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/controllers/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/discuss.py b/Allura/allura/controllers/discuss.py
index 96f8979..e3284fc 100644
--- a/Allura/allura/controllers/discuss.py
+++ b/Allura/allura/controllers/discuss.py
@@ -200,7 +200,6 @@ class ThreadController(BaseController, FeedController):
file_info = kw.get('file_info', None)
p = self.thread.add_post(**kw)
- is_spam = g.spam_checker.check(kw['text'], artifact=p, user=c.user)
p.add_attachment(file_info)
if self.thread.artifact:
self.thread.artifact.mod_date = datetime.utcnow()
@@ -317,7 +316,6 @@ class PostController(BaseController):
require_access(self.thread, 'post')
kw = self.W.edit_post.to_python(kw, None)
p = self.thread.add_post(parent_id=self.post._id, **kw)
- is_spam = g.spam_checker.check(kw['text'], artifact=p, user=c.user)
p.add_attachment(file_info)
redirect(request.referer)
@@ -331,6 +329,7 @@ class PostController(BaseController):
self.post.delete()
elif kw.pop('spam', None):
self.post.status = 'spam'
+ g.spam_checker.submit_spam(self.post.text, artifact=self.post, user=c.user)
elif kw.pop('approve', None):
self.post.status = 'ok'
self.thread.update_stats()
@@ -458,7 +457,6 @@ class PostRestController(PostController):
require_access(self.thread, 'post')
kw = self.W.edit_post.to_python(kw, None)
post = self.thread.post(parent_id=self.post._id, **kw)
- is_spam = g.spam_checker.check(kw['text'], artifact=post, user=c.user)
self.thread.num_replies += 1
redirect(post.slug.split('/')[-1] + '/')
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/lib/spam/__init__.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/spam/__init__.py b/Allura/allura/lib/spam/__init__.py
index 869e6fd..d8b52aa 100644
--- a/Allura/allura/lib/spam/__init__.py
+++ b/Allura/allura/lib/spam/__init__.py
@@ -32,6 +32,9 @@ class SpamFilter(object):
log.info("No spam checking enabled")
return False
+ def submit_spam(self, text, artifact=None, user=None, content_type='comment', **kw):
+ log.info("No spam checking enabled")
+
@classmethod
def get(cls, config, entry_points):
"""Return an instance of the SpamFilter impl specified in ``config``.
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/lib/spam/akismetfilter.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/spam/akismetfilter.py b/Allura/allura/lib/spam/akismetfilter.py
index bd2e89e..13e0032 100644
--- a/Allura/allura/lib/spam/akismetfilter.py
+++ b/Allura/allura/lib/spam/akismetfilter.py
@@ -47,8 +47,7 @@ class AkismetSpamFilter(SpamFilter):
self.service = akismet.Akismet(config.get('spam.key'), config.get('base_url'))
self.service.verify_key()
- def check(self, text, artifact=None, user=None, content_type='comment', **kw):
- log_msg = text
+ def get_data(self, text, artifact=None, user=None, content_type='comment', **kw):
kw['comment_content'] = text
kw['comment_type'] = content_type
if artifact:
@@ -65,6 +64,28 @@ class AkismetSpamFilter(SpamFilter):
# kw will be urlencoded, need to utf8-encode
for k, v in kw.items():
kw[k] = h.really_unicode(v).encode('utf8')
- res = self.service.comment_check(text, data=kw, build_data=False)
+ return kw
+
+ def check(self, text, artifact=None, user=None, content_type='comment', **kw):
+ log_msg = text
+ res = self.service.comment_check(text,
+ data=self.get_data(text=text,
+ artifact=artifact,
+ user=user,
+ content_type=content_type),
+ build_data=False)
log.info("spam=%s (akismet): %s" % (str(res), log_msg))
return res
+
+ def submit_spam(self, text, artifact=None, user=None, content_type='comment'):
+ self.service.submit_spam(text,
+ data=self.get_data(text=text,
+ artifact=artifact,
+ user=user,
+ content_type=content_type),
+ build_data=False)
+
+
+
+
+
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/lib/spam/mollomfilter.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/spam/mollomfilter.py b/Allura/allura/lib/spam/mollomfilter.py
index 7852fda..a9d586b 100644
--- a/Allura/allura/lib/spam/mollomfilter.py
+++ b/Allura/allura/lib/spam/mollomfilter.py
@@ -73,6 +73,9 @@ class MollomSpamFilter(SpamFilter):
kw[k] = h.really_unicode(v).encode('utf8')
cc = self.service.checkContent(**kw)
res = cc['spam'] == 2
+ artifact.spam_check_id = cc.get('session_id','')
log.info("spam=%s (mollom): %s" % (str(res), log_msg))
return res
+ def submit_spam(self, text, artifact=None, user=None, content_type='comment', **kw):
+ self.service.sendFeedback(artifact.spam_check_id, 'spam')
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/model/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py
index c947c82..3938a43 100644
--- a/Allura/allura/model/discuss.py
+++ b/Allura/allura/model/discuss.py
@@ -33,6 +33,7 @@ from allura.lib import security
from allura.lib.security import require_access, has_access
from allura.lib import utils
from allura.model.notification import Notification, Mailbox
+from allura.model.auth import ProjectRole
from .artifact import Artifact, ArtifactReference, VersionedArtifact, Snapshot, Message, Feed
from .attachments import BaseAttachment
from .auth import User
@@ -235,6 +236,18 @@ class Thread(Artifact, ActivityObject):
Feed.post(self.primary(), title=p.subject, description=p.text, link=link)
return p
+ def check_spam(self, post):
+ result = True
+ for role in c.user.project_role(c.project).roles:
+ if ((ProjectRole.query.get(_id=role).name == 'Admin') or
+ (ProjectRole.query.get(_id=role).name == 'Developer')):
+ return True
+
+ if g.spam_checker.check('%s\n%s' % (post.subject, post.text), artifact=post, user=c.user):
+ result = False
+ return result
+
+
def post(self, text, message_id=None, parent_id=None,
timestamp=None, ignore_security=False, **kw):
if not ignore_security:
@@ -258,7 +271,8 @@ class Thread(Artifact, ActivityObject):
if message_id is not None:
kwargs['_id'] = message_id
post = self.post_class()(**kwargs)
- if ignore_security or has_access(self, 'unmoderated_post')():
+
+ if ignore_security or self.check_spam(post):
log.info('Auto-approving message from %s', c.user.username)
file_info = kw.get('file_info', None)
post.approve(file_info, notify=kw.get('notify', True))
@@ -439,6 +453,7 @@ class Post(Message, VersionedArtifact, ActivityObject):
last_edit_date = FieldProperty(datetime, if_missing=None)
last_edit_by_id = ForeignIdProperty(User)
edit_count = FieldProperty(int, if_missing=0)
+ spam_check_id = FieldProperty(str, if_missing='')
thread = RelationProperty(Thread)
discussion = RelationProperty(Discussion)
@@ -648,6 +663,7 @@ class Post(Message, VersionedArtifact, ActivityObject):
def spam(self):
self.status = 'spam'
self.thread.num_replies = max(0, self.thread.num_replies - 1)
+ g.spam_checker.submit_spam(self.text, artifact=self, user=c.user)
class DiscussionAttachment(BaseAttachment):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/tests/functional/test_discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_discuss.py b/Allura/allura/tests/functional/test_discuss.py
index 633f078..8a51ac4 100644
--- a/Allura/allura/tests/functional/test_discuss.py
+++ b/Allura/allura/tests/functional/test_discuss.py
@@ -70,7 +70,8 @@ class TestDiscuss(TestController):
r = r.follow()
return r
- def test_post(self):
+ @patch('allura.controllers.discuss.g.spam_checker.submit_spam')
+ def test_post(self, submit_spam):
home = self.app.get('/wiki/_discuss/')
thread_link = [ a for a in home.html.findAll('a')
if 'thread' in a['href'] ][0]['href']
@@ -109,6 +110,7 @@ class TestDiscuss(TestController):
self.app.post(permalinks[1]+'flag')
self.app.post(permalinks[1]+'moderate', params=dict(delete='delete'))
self.app.post(permalinks[0]+'moderate', params=dict(spam='spam'))
+ assert submit_spam.call_args[0] ==('This is a new post',), submit_spam.call_args[0]
def test_permissions(self):
home = self.app.get('/wiki/_discuss/')
@@ -136,12 +138,14 @@ class TestDiscuss(TestController):
self.app.get(thread_url, status=403, # forbidden
extra_environ=dict(username=non_admin))
- def test_moderate(self):
+ @patch('allura.controllers.discuss.g.spam_checker.submit_spam')
+ def test_moderate(self, submit_spam):
r = self._make_post('Test post')
post_link = str(r.html.find('div', {'class': 'edit_post_form reply'}).find('form')['action'])
post = M.Post.query.find().first()
post.status = 'pending'
self.app.post(post_link + 'moderate', params=dict(spam='spam'))
+ assert submit_spam.call_args[0] ==('Test post',), submit_spam.call_args[0]
post = M.Post.query.find().first()
assert post.status == 'spam'
self.app.post(post_link + 'moderate', params=dict(approve='approve'))
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/tests/model/test_discussion.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_discussion.py b/Allura/allura/tests/model/test_discussion.py
index 35c5594..68611b3 100644
--- a/Allura/allura/tests/model/test_discussion.py
+++ b/Allura/allura/tests/model/test_discussion.py
@@ -356,3 +356,25 @@ def test_post_notify():
pass # method not called as expected
else:
assert False, 'send_simple must not be called'
+
+@with_setup(setUp, tearDown)
+def test_check_spam_for_admin():
+ d = M.Discussion(shortname='test', name='test')
+ t = M.Thread(discussion_id=d._id, subject='Test Thread')
+ t.post('This is a post')
+ post = M.Post.query.get(text='This is a post')
+ assert t.check_spam(post), t.check_spam(post)
+
+@with_setup(setUp, tearDown)
+@patch('allura.model.discuss.c.user.project_role')
+def test_check_spam(role):
+ d = M.Discussion(shortname='test', name='test')
+ t = M.Thread(discussion_id=d._id, subject='Test Thread')
+ role.roles.return_value = []
+ with mock.patch('allura.controllers.discuss.g.spam_checker') as spam_checker:
+ spam_checker.check.return_value = True
+ post = mock.Mock()
+ assert not t.check_spam(post)
+ assert spam_checker.check.call_count == 1, spam_checker.call_count
+
+
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/tests/unit/spam/test_akismet.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/test_akismet.py b/Allura/allura/tests/unit/spam/test_akismet.py
index a03b148..bb9a90e 100644
--- a/Allura/allura/tests/unit/spam/test_akismet.py
+++ b/Allura/allura/tests/unit/spam/test_akismet.py
@@ -59,6 +59,7 @@ class TestAkismet(unittest.TestCase):
def test_check(self, request, c):
request.headers = self.fake_headers
c.user = None
+ self.akismet.service.comment_check.side_effect({'side_effect':''})
self.akismet.check(self.content)
self.akismet.service.comment_check.assert_called_once_with(self.content,
data=self.expected_data, build_data=False)
@@ -119,3 +120,12 @@ class TestAkismet(unittest.TestCase):
self.akismet.check(self.content)
self.akismet.service.comment_check.assert_called_once_with(self.content,
data=self.expected_data, build_data=False)
+
+ @mock.patch('allura.lib.spam.akismetfilter.c')
+ @mock.patch('allura.lib.spam.akismetfilter.request')
+ def test_submit_spam(self, request, c):
+ request.headers = self.fake_headers
+ c.user = None
+ self.akismet.submit_spam(self.content)
+ self.akismet.service.submit_spam.assert_called_once_with(self.content, data=self.expected_data, build_data=False)
+
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/tests/unit/spam/test_mollom.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/test_mollom.py b/Allura/allura/tests/unit/spam/test_mollom.py
index 63b0b6d..7828908 100644
--- a/Allura/allura/tests/unit/spam/test_mollom.py
+++ b/Allura/allura/tests/unit/spam/test_mollom.py
@@ -58,15 +58,19 @@ class TestMollom(unittest.TestCase):
def test_check(self, request, c):
request.headers = self.fake_headers
c.user = None
- self.mollom.check(self.content)
+ artifact = mock.Mock()
+ artifact.spam_check_id = 'test_id'
+ self.mollom.check(self.content, artifact = artifact)
self.mollom.service.checkContent.assert_called_once_with(**self.expected_data)
@mock.patch('allura.lib.spam.mollomfilter.c')
@mock.patch('allura.lib.spam.mollomfilter.request')
def test_check_with_user(self, request, c):
+ artifact = mock.Mock()
+ artifact.spam_check_id = 'test_id'
request.headers = self.fake_headers
c.user = None
- self.mollom.check(self.content, user=self.fake_user)
+ self.mollom.check(self.content, user=self.fake_user, artifact=artifact)
expected_data = self.expected_data
expected_data.update(authorName=u'Søme User'.encode('utf8'),
authorMail='user@domain')
@@ -75,9 +79,11 @@ class TestMollom(unittest.TestCase):
@mock.patch('allura.lib.spam.mollomfilter.c')
@mock.patch('allura.lib.spam.mollomfilter.request')
def test_check_with_implicit_user(self, request, c):
+ artifact = mock.Mock()
+ artifact.spam_check_id = 'test_id'
request.headers = self.fake_headers
c.user = self.fake_user
- self.mollom.check(self.content)
+ self.mollom.check(self.content, artifact=artifact)
expected_data = self.expected_data
expected_data.update(authorName=u'Søme User'.encode('utf8'),
authorMail='user@domain')
@@ -86,10 +92,22 @@ class TestMollom(unittest.TestCase):
@mock.patch('allura.lib.spam.mollomfilter.c')
@mock.patch('allura.lib.spam.mollomfilter.request')
def test_check_with_fallback_ip(self, request, c):
+ artifact = mock.Mock()
+ artifact.spam_check_id = 'test_id'
self.expected_data['authorIP'] = 'fallback ip'
self.fake_headers.pop('X_FORWARDED_FOR')
request.headers = self.fake_headers
request.remote_addr = self.fake_headers['REMOTE_ADDR']
c.user = None
- self.mollom.check(self.content)
+ self.mollom.check(self.content, artifact=artifact)
self.mollom.service.checkContent.assert_called_once_with(**self.expected_data)
+
+ @mock.patch('allura.lib.spam.mollomfilter.c')
+ @mock.patch('allura.lib.spam.mollomfilter.request')
+ def test_submit_spam(self, request, c):
+ request.headers = self.fake_headers
+ c.user = None
+ artifact = mock.Mock()
+ artifact.spam_check_id = 'test_id'
+ self.mollom.submit_spam('test', artifact=artifact)
+ assert 'test_id' in self.mollom.service.sendFeedback.call_args[0]
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/Allura/allura/tests/unit/spam/test_spam_filter.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/test_spam_filter.py b/Allura/allura/tests/unit/spam/test_spam_filter.py
index 3f29247..a871aaa 100644
--- a/Allura/allura/tests/unit/spam/test_spam_filter.py
+++ b/Allura/allura/tests/unit/spam/test_spam_filter.py
@@ -34,6 +34,9 @@ class TestSpamFilter(unittest.TestCase):
# default no-op impl always returns False
self.assertFalse(SpamFilter({}).check('foo'))
+ def test_submit_spam(self):
+ SpamFilter({}).submit_spam('foo')
+
def test_get_default(self):
config = {}
entry_points = None
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/ForgeDiscussion/forgediscussion/controllers/root.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/controllers/root.py b/ForgeDiscussion/forgediscussion/controllers/root.py
index e358e18..8d18847 100644
--- a/ForgeDiscussion/forgediscussion/controllers/root.py
+++ b/ForgeDiscussion/forgediscussion/controllers/root.py
@@ -126,7 +126,6 @@ class RootController(BaseController, DispatchIndex, FeedController):
thd = discussion.get_discussion_thread(dict(
headers=dict(Subject=subject)))[0]
post = thd.post(subject, text)
- is_spam = g.spam_checker.check('%s\n%s' % (subject, text), artifact=post, user=c.user)
flash('Message posted')
redirect(thd.url())
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
index 4fa8921..4acf35c 100644
--- a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
+++ b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
@@ -385,8 +385,7 @@ class TestForum(TestController):
'delete': 'Delete Marked'})
_check()
- @mock.patch('forgediscussion.controllers.root.g.spam_checker')
- def test_posting(self, spam_checker):
+ def test_posting(self):
r = self.app.get('/discussion/create_topic/')
f = r.html.find('form',{'action':'/p/test/discussion/save_new_topic'})
params = dict()
@@ -398,8 +397,6 @@ class TestForum(TestController):
params[f.find('select')['name']] = 'testforum'
params[f.find('input',{'style':'width: 90%'})['name']] = 'Test Thread'
r = self.app.post('/discussion/save_new_topic', params=params)
- assert spam_checker.check.call_args[0] == ('Test Thread\nThis is a *test thread*',), \
- spam_checker.check.call_args[0]
r = self.app.get('/admin/discussion/forums')
assert 'Message posted' in r
r = self.app.get('/discussion/testforum/moderate/')
@@ -410,7 +407,9 @@ class TestForum(TestController):
assert 'noreply' not in n.reply_to_address, n
assert 'testforum@discussion.test.p' in n.reply_to_address, n
- def test_anonymous_post(self):
+ @mock.patch('allura.model.discuss.g.spam_checker')
+ def test_anonymous_post(self, spam_checker):
+ spam_checker.check.return_value = True
r = self.app.get('/admin/discussion/permissions')
select = r.html.find('select', {'name': 'card-3.new'})
opt_anon = select.find(text='*anonymous').parent
@@ -456,8 +455,7 @@ class TestForum(TestController):
link = '<a href="%s">[%s]</a>' % (post.thread.url() + '?limit=25#' + post.slug, post.shorthand_id())
assert link in r, link
- @mock.patch('forgediscussion.controllers.root.g.spam_checker')
- def test_thread(self, spam_checker):
+ def test_thread(self):
r = self.app.get('/discussion/create_topic/')
f = r.html.find('form',{'action':'/p/test/discussion/save_new_topic'})
params = dict()
@@ -481,7 +479,6 @@ class TestForum(TestController):
params[field['name']] = field.has_key('value') and field['value'] or ''
params[f.find('textarea')['name']] = 'bbb'
thread = self.app.post(str(rep_url), params=params)
- assert spam_checker.check.call_args[0] == ('bbb',), spam_checker.check.call_args[0]
thread = self.app.get(url)
# beautiful soup is getting some unicode error here - test without it
assert thread.html.findAll('div',{'class':'display_post'})[0].find('p').string == 'aaa'
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9badd061/ForgeTracker/forgetracker/tests/functional/test_root.py
----------------------------------------------------------------------
diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py
index 1dc3922..814edb8 100644
--- a/ForgeTracker/forgetracker/tests/functional/test_root.py
+++ b/ForgeTracker/forgetracker/tests/functional/test_root.py
@@ -1972,7 +1972,9 @@ class TestEmailMonitoring(TrackerTestController):
@td.with_tool('test', 'Tickets', 'doc-bugs', post_install_hook=post_install_hook)
@patch('forgetracker.model.ticket.Notification.send_direct')
- def test_notifications_moderators(self, send_direct):
+ @patch('allura.model.discuss.Thread.check_spam')
+ def test_notifications_moderators(self, check_spam, send_direct):
+ check_spam.return_value = False
self.new_ticket(summary='test moderation', mount_point='/doc-bugs/')
self.app.post('/doc-bugs/1/update_ticket',{
'summary':'test moderation',