Merge branch 'fix/spam-guard'

docs/CONFIGURATION.rst

@@ -186,14 +186,25 @@ for IPv4, ``/48`` for IPv6).
     [guard]
     enabled = true
     ratelimit = 2
+    direct-reply = 3
+    reply-to-self = false
 
 enabled
     enable guard, recommended in production. Not useful for debugging
     purposes.
 
-ratelimit: N
+ratelimit
     limit to N new comments per minute.
 
+direct-reply
+    how many comments directly to the thread (prevent a simple
+    `while true; do curl ...; done`.
+
+reply-to-self
+    allow commenters to reply to their own comments when they could still edit
+    the comment. After the editing timeframe is gone, commenters can reply to
+    their own comments anyways.
+
 
 Appendum
 ---------

isso/core.py

@@ -119,8 +119,9 @@ class Config:
         "to = ", "from = ",
         "[guard]",
         "enabled = true",
-        "ratelimit = 2"
+        "ratelimit = 2",
-        ""
+        "direct-reply = 3",
+        "reply-to-self = false"
     ]
 
     @classmethod

isso/db/__init__.py

@@ -5,11 +5,9 @@ import logging
 
 logger = logging.getLogger("isso")
 
-class IssoDBException(Exception):
-    pass
-
 from isso.db.comments import Comments
 from isso.db.threads import Threads
+from isso.db.spam import Guard
 
 
 class SQLite3:
@@ -34,6 +32,7 @@ class SQLite3:
 
         self.threads = Threads(self)
         self.comments = Comments(self)
+        self.guard = Guard(self)
 
         self.execute([
             'CREATE TRIGGER IF NOT EXISTS remove_stale_threads',

isso/db/comments.py

@@ -33,7 +33,6 @@ class Comments:
             '    text VARCHAR, author VARCHAR, email VARCHAR, website VARCHAR,',
             '    likes INTEGER DEFAULT 0, dislikes INTEGER DEFAULT 0, voters BLOB NOT NULL);'])
 
-    @spam.check
     def add(self, uri, c):
         """
         Add a new comment to the database and return public fields as dict.

isso/db/spam.py

@@ -1,47 +1,68 @@
 # -*- encoding: utf-8 -*-
 
 import time
-
+import functools
-from isso.db import IssoDBException
 
 
-class TooManyComments(IssoDBException):
+class Guard:
-    pass
 
+    def __init__(self, db):
 
-def check(func):
+        self.db = db
+        self.conf = db.conf.section("guard")
+        self.max_age = db.conf.getint("general", "max-age")
 
-    def dec(self, uri, c):
+    def validate(self, uri, comment):
 
-        if not self.db.conf.getboolean("guard", "enabled"):
+        if not self.conf.getboolean("enabled"):
-            return func(self, uri, c)
+            return True, ""
 
-        # block more than two comments per minute
+        for func in (self._limit, self._spam):
+            valid, reason = func(uri, comment)
+            if not valid:
+                return False, reason
+        return True, ""
+
+    @classmethod
+    def ids(cls, rv):
+        return [str(col[0]) for col in rv]
+
+    def _limit(self, uri, comment):
+
+        # block more than :param:`ratelimit` comments per minute
         rv = self.db.execute([
-            'SELECT id FROM comments WHERE remote_addr = ? AND created > ?;'
+            'SELECT id FROM comments WHERE remote_addr = ? AND ? - created < 60;'
-        ], (c["remote_addr"], time.time() + 60)).fetchall()
+        ], (comment["remote_addr"], time.time())).fetchall()
 
-        if len(rv) >= self.db.conf.getint("guard", "ratelimit"):
+        if len(rv) >= self.conf.getint("ratelimit"):
-            raise TooManyComments
+            return False, "{0}: ratelimit exceeded ({1})".format(
+                comment["remote_addr"], ', '.join(Guard.ids(rv)))
 
         # block more than three comments as direct response to the post
+        if comment["parent"] is None:
             rv = self.db.execute([
-            'SELECT id FROM comments WHERE remote_addr = ? AND parent IS NULL;'
+                'SELECT id FROM comments WHERE',
-        ], (c["remote_addr"], )).fetchall()
+                '    tid = (SELECT id FROM threads WHERE uri = ?)',
+                'AND remote_addr = ?',
+                'AND parent IS NULL;'
+            ], (uri, comment["remote_addr"])).fetchall()
 
-        if len(rv) >= 3:
+            if len(rv) >= self.conf.getint("direct-reply"):
-            raise TooManyComments
+                return False, "%i direct responses to %s" % (len(rv), uri)
 
-        # block reply to own comment if the cookie is still available (max age)
+        elif self.conf.getboolean("reply-to-self") == False:
-        if "parent" in c:
             rv = self.db.execute([
-                'SELECT id FROM comments WHERE remote_addr = ? AND id = ? AND ? - created < ?;'
+                'SELECT id FROM comments WHERE'
-            ], (c["remote_addr"], c["parent"], time.time(),
+                '    remote_addr = ?',
-                self.db.conf.getint("general", "max-age"))).fetchall()
+                'AND id = ?',
+                'AND ? - created < ?'
+            ], (comment["remote_addr"], comment["parent"],
+                time.time(), self.max_age)).fetchall()
 
             if len(rv) > 0:
-                raise TooManyComments
+                return False, "edit time frame is still open"
 
-        return func(self, uri, c)
+        return True, ""
 
-    return dec
+    def _spam(self, uri, comment):
+        return True, ""

isso/views/comments.py

@@ -15,7 +15,7 @@ from werkzeug.exceptions import BadRequest, Forbidden, NotFound
 
 from isso.compat import text_type as str
 
-from isso import utils, local
+from isso import utils, local, db
 from isso.utils import http, parse, markdown
 from isso.utils.crypto import pbkdf2
 from isso.views import requires
@@ -61,6 +61,7 @@ class API(object):
         self.conf = isso.conf.section("general")
         self.moderated = isso.conf.getboolean("moderation", "enabled")
 
+        self.guard = isso.db.guard
         self.threads = isso.db.threads
         self.comments = isso.db.comments
 
@@ -127,8 +128,10 @@ class API(object):
         # notify extensions that the new comment is about to save
         self.signal("comments.new:before-save", thread, data)
 
-        if data is None:
+        valid, reason = self.guard.validate(uri, data)
-            raise Forbidden
+        if not valid:
+            self.signal("comments.new:guard", reason)
+            raise Forbidden(reason)
 
         with self.isso.lock:
             rv = self.comments.add(uri, data)

specs/fixtures.py

@@ -0,0 +1,31 @@
+# -*- encoding: utf-8 -*-
+
+import json
+
+
+class FakeIP(object):
+
+    def __init__(self, app, ip):
+        self.app = app
+        self.ip = ip
+
+    def __call__(self, environ, start_response):
+        environ['REMOTE_ADDR'] = self.ip
+        return self.app(environ, start_response)
+
+
+class Dummy:
+
+    status = 200
+
+    def __enter__(self):
+        return self
+
+    def read(self):
+        return ''
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        pass
+
+curl = lambda method, host, path: Dummy()
+loads = lambda data: json.loads(data.decode('utf-8'))

specs/test_comments.py

@@ -1,6 +1,5 @@
 # -*- encoding: utf-8 -*-
 
-
 from __future__ import unicode_literals
 
 import os
@@ -20,32 +19,8 @@ from isso import Isso, core
 from isso.utils import http
 from isso.views import comments
 
-class Dummy:
+from fixtures import curl, loads, FakeIP
-
+http.curl = curl
-    status = 200
-
-    def __enter__(self):
-        return self
-
-    def read(self):
-        return ''
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        pass
-
-http.curl = lambda method, host, path: Dummy()
-
-loads = lambda data: json.loads(data.decode('utf-8'))
-
-
-class FakeIP(object):
-
-    def __init__(self, app):
-        self.app = app
-
-    def __call__(self, environ, start_response):
-        environ['REMOTE_ADDR'] = '192.168.1.1'
-        return self.app(environ, start_response)
 
 
 class TestComments(unittest.TestCase):
@@ -60,7 +35,7 @@ class TestComments(unittest.TestCase):
             pass
 
         self.app = App(conf)
-        self.app.wsgi_app = FakeIP(self.app.wsgi_app)
+        self.app.wsgi_app = FakeIP(self.app.wsgi_app, "192.168.1.1")
 
         self.client = Client(self.app, Response)
         self.get = self.client.get
@@ -307,7 +282,7 @@ class TestModeratedComments(unittest.TestCase):
             pass
 
         self.app = App(conf)
-        self.app.wsgi_app = FakeIP(self.app.wsgi_app)
+        self.app.wsgi_app = FakeIP(self.app.wsgi_app, "192.168.1.1")
         self.client = Client(self.app, Response)
 
     def tearDown(self):
@@ -338,7 +313,7 @@ class TestPurgeComments(unittest.TestCase):
             pass
 
         self.app = App(conf)
-        self.app.wsgi_app = FakeIP(self.app.wsgi_app)
+        self.app.wsgi_app = FakeIP(self.app.wsgi_app, "192.168.1.1")
         self.client = Client(self.app, Response)
 
     def testPurgeDoesNoHarm(self):

specs/test_guard.py

@@ -0,0 +1,102 @@
+# -*- encoding: utf-8 -*-
+
+import unittest
+
+import os
+import json
+import tempfile
+
+from werkzeug.test import Client
+from werkzeug.wrappers import Response
+
+from isso import Isso, core
+from isso.utils import http
+
+from fixtures import curl, FakeIP
+http.curl = curl
+
+
+class TestGuard(unittest.TestCase):
+
+    data = json.dumps({"text": "Lorem ipsum."})
+
+    def setUp(self):
+        self.path = tempfile.NamedTemporaryFile().name
+
+    def makeClient(self, ip, ratelimit=2, direct_reply=3, self_reply=False):
+
+        conf = core.Config.load(None)
+        conf.set("general", "dbpath", self.path)
+        conf.set("guard", "enabled", "true")
+        conf.set("guard", "ratelimit", str(ratelimit))
+        conf.set("guard", "direct-reply", str(direct_reply))
+        conf.set("guard", "reply-to-self", "1" if self_reply else "0")
+
+        class App(Isso, core.Mixin):
+            pass
+
+        app = App(conf)
+        app.wsgi_app = FakeIP(app.wsgi_app, ip)
+
+        return Client(app, Response)
+
+    def testRateLimit(self):
+
+        bob = self.makeClient("127.0.0.1", 2)
+
+        for i in range(2):
+            rv = bob.post('/new?uri=test', data=self.data)
+            assert rv.status_code == 201
+
+        rv = bob.post('/new?uri=test', data=self.data)
+
+        assert rv.status_code == 403
+        assert "ratelimit exceeded" in rv.get_data(as_text=True)
+
+        alice = self.makeClient("1.2.3.4", 2)
+        for i in range(2):
+            assert alice.post("/new?uri=test", data=self.data).status_code == 201
+
+        bob.application.db.execute([
+            "UPDATE comments SET",
+            "    created = created - 60",
+            "WHERE remote_addr = '127.0.0.0'"
+        ])
+
+        assert bob.post("/new?uri=test", data=self.data).status_code == 201
+
+    def testDirectReply(self):
+
+        client = self.makeClient("127.0.0.1", 15, 3)
+
+        for url in ("foo", "bar", "baz", "spam"):
+            for _ in range(3):
+                rv = client.post("/new?uri=%s" % url, data=self.data)
+                assert rv.status_code == 201
+
+        for url in ("foo", "bar", "baz", "spam"):
+            rv = client.post("/new?uri=%s" % url, data=self.data)
+
+            assert rv.status_code == 403
+            assert "direct responses to" in rv.get_data(as_text=True)
+
+    def testSelfReply(self):
+
+        payload = lambda id: json.dumps({"text": "...", "parent": id})
+
+        client = self.makeClient("127.0.0.1", self_reply=False)
+        assert client.post("/new?uri=test", data=self.data).status_code == 201
+        assert client.post("/new?uri=test", data=payload(1)).status_code == 403
+
+        client.application.db.execute([
+            "UPDATE comments SET",
+            "    created = created - ?",
+            "WHERE id = 1"
+        ], (client.application.conf.getint("general", "max-age"), ))
+
+        assert client.post("/new?uri=test", data=payload(1)).status_code == 201
+
+        client = self.makeClient("128.0.0.1", ratelimit=3, self_reply=False)
+        assert client.post("/new?uri=test", data=self.data).status_code == 201
+        assert client.post("/new?uri=test", data=payload(1)).status_code == 201
+        assert client.post("/new?uri=test", data=payload(2)).status_code == 201

specs/test_vote.py

@@ -12,42 +12,14 @@ from werkzeug.wrappers import Response
 from isso import Isso, core
 from isso.utils import http
 
-class Dummy:
+from fixtures import curl, loads, FakeIP
-
+http.curl = curl
-    status = 200
-
-    def __enter__(self):
-        return self
-
-    def read(self):
-        return ''
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        pass
-
-http.curl = lambda method, host, path: Dummy()
-
-loads = lambda data: json.loads(data.decode('utf-8'))
-
-
-class FakeIP(object):
-
-    def __init__(self, app, ip):
-        self.app = app
-        self.ip = ip
-
-    def __call__(self, environ, start_response):
-        environ['REMOTE_ADDR'] = self.ip
-        return self.app(environ, start_response)
 
 
 class TestVote(unittest.TestCase):
 
     def setUp(self):
-        fd, self.path = tempfile.mkstemp()
+        self.path = tempfile.NamedTemporaryFile().name
-
-    def tearDown(self):
-        os.unlink(self.path)
 
     def makeClient(self, ip):