diff --git a/docs/CONFIGURATION.rst b/docs/CONFIGURATION.rst index dfe518d..d670388 100644 --- a/docs/CONFIGURATION.rst +++ b/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 --------- diff --git a/isso/core.py b/isso/core.py index efa59d8..061dfee 100644 --- a/isso/core.py +++ b/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 diff --git a/isso/db/__init__.py b/isso/db/__init__.py index f1030f2..538ddb5 100644 --- a/isso/db/__init__.py +++ b/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', diff --git a/isso/db/comments.py b/isso/db/comments.py index 2a549ba..3bf37c1 100644 --- a/isso/db/comments.py +++ b/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. diff --git a/isso/db/spam.py b/isso/db/spam.py index 1b840f2..b86361b 100644 --- a/isso/db/spam.py +++ b/isso/db/spam.py @@ -1,47 +1,68 @@ # -*- encoding: utf-8 -*- import time - -from isso.db import IssoDBException +import functools -class TooManyComments(IssoDBException): - pass +class Guard: + 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"): - return func(self, uri, c) + if not self.conf.getboolean("enabled"): + 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 > ?;' - ], (c["remote_addr"], time.time() + 60)).fetchall() + 'SELECT id FROM comments WHERE remote_addr = ? AND ? - created < 60;' + ], (comment["remote_addr"], time.time())).fetchall() - if len(rv) >= self.db.conf.getint("guard", "ratelimit"): - raise TooManyComments + if len(rv) >= self.conf.getint("ratelimit"): + 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 - rv = self.db.execute([ - 'SELECT id FROM comments WHERE remote_addr = ? AND parent IS NULL;' - ], (c["remote_addr"], )).fetchall() - - if len(rv) >= 3: - raise TooManyComments - - # block reply to own comment if the cookie is still available (max age) - if "parent" in c: + if comment["parent"] is None: rv = self.db.execute([ - 'SELECT id FROM comments WHERE remote_addr = ? AND id = ? AND ? - created < ?;' - ], (c["remote_addr"], c["parent"], time.time(), - self.db.conf.getint("general", "max-age"))).fetchall() + 'SELECT id FROM comments WHERE', + ' tid = (SELECT id FROM threads WHERE uri = ?)', + 'AND remote_addr = ?', + 'AND parent IS NULL;' + ], (uri, comment["remote_addr"])).fetchall() + + if len(rv) >= self.conf.getint("direct-reply"): + return False, "%i direct responses to %s" % (len(rv), uri) + + elif self.conf.getboolean("reply-to-self") == False: + rv = self.db.execute([ + 'SELECT id FROM comments WHERE' + ' remote_addr = ?', + '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, "" diff --git a/isso/views/comments.py b/isso/views/comments.py index 4af9c8e..fbe29c0 100644 --- a/isso/views/comments.py +++ b/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: - raise Forbidden + valid, reason = self.guard.validate(uri, data) + if not valid: + self.signal("comments.new:guard", reason) + raise Forbidden(reason) with self.isso.lock: rv = self.comments.add(uri, data) diff --git a/specs/fixtures.py b/specs/fixtures.py new file mode 100644 index 0000000..91f8349 --- /dev/null +++ b/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')) diff --git a/specs/test_comments.py b/specs/test_comments.py index bed2355..81d90a8 100644 --- a/specs/test_comments.py +++ b/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: - - 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) +from fixtures import curl, loads, FakeIP +http.curl = curl 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): diff --git a/specs/test_guard.py b/specs/test_guard.py new file mode 100644 index 0000000..ae76044 --- /dev/null +++ b/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 diff --git a/specs/test_vote.py b/specs/test_vote.py index 8feb97b..89e37ed 100644 --- a/specs/test_vote.py +++ b/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: - - 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) +from fixtures import curl, loads, FakeIP +http.curl = curl class TestVote(unittest.TestCase): def setUp(self): - fd, self.path = tempfile.mkstemp() - - def tearDown(self): - os.unlink(self.path) + self.path = tempfile.NamedTemporaryFile().name def makeClient(self, ip):