From 1db06bbf39b20fd3211d832610bceba6fdb25b1b Mon Sep 17 00:00:00 2001 From: Martin Zimmermann Date: Tue, 3 Dec 2013 11:35:33 +0100 Subject: [PATCH] Revert "HTTP Origin is only sent on cross-origin requests in Firefox" Revert "use Referer instead of Origin when using IE" Revert "fix unittests" Revert "check if Origin matches Host to mitigate CSRF, part of #40" This reverts commit 9376511485c70deaf908aa67bcdc8f0c9a0b003e. This reverts commit 9a03cca7939668357d739489788cca094b7df564. This reverts commit 4c16ba76cca35e528efdbcf43a205271388fb4e4. This reverts commit 32e4b70510103bb55e6aa0be15b118ea0b01342d. --- isso/views/comments.py | 33 --------------------------------- specs/test_comments.py | 25 ++++--------------------- 2 files changed, 4 insertions(+), 54 deletions(-) diff --git a/isso/views/comments.py b/isso/views/comments.py index f1e48a9..9db8c02 100644 --- a/isso/views/comments.py +++ b/isso/views/comments.py @@ -12,7 +12,6 @@ from werkzeug.http import dump_cookie from werkzeug.routing import Rule from werkzeug.wrappers import Response from werkzeug.exceptions import BadRequest, Forbidden, NotFound -from werkzeug.useragents import UserAgent from isso.compat import text_type as str @@ -32,33 +31,6 @@ class JSON(Response): return super(JSON, self).__init__(*args, content_type='application/json') -def csrf(view): - """A decorator to check if Origin matches Host (may be empty if in the same - origin, except for IE of course). When MSIE, use Referer. See - - * https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Checking_The_Origin_Header - * http://tools.ietf.org/html/draft-abarth-origin-09 - * https://wiki.mozilla.org/Security/Origin - - for details. - """ - - def dec(self, environ, request, *args, **kwargs): - - hosts = map(parse.host, self.conf.getiter("host")) - - if UserAgent(environ).browser == "msie": # yup - if parse.host(request.headers.get("Referer", "")) not in hosts: - raise Forbidden("CSRF") - elif "Origin" in request.headers: - if parse.host(request.headers.get("Origin", "")) not in hosts: - raise Forbidden("CSRF") - - return view(self, environ, request, *args, **kwargs) - - return dec - - class API(object): FIELDS = set(['id', 'parent', 'text', 'author', 'website', 'email', @@ -119,7 +91,6 @@ class API(object): return True, "" - @csrf @requires(str, 'uri') def new(self, environ, request, uri): @@ -203,7 +174,6 @@ class API(object): return Response(json.dumps(rv), 200, content_type='application/json') - @csrf def edit(self, environ, request, id): try: @@ -247,7 +217,6 @@ class API(object): resp.headers.add("X-Set-Cookie", cookie("isso-%i" % rv["id"])) return resp - @csrf def delete(self, environ, request, id, key=None): try: @@ -350,13 +319,11 @@ class API(object): return JSON(json.dumps(rv), 200) - @csrf def like(self, environ, request, id): nv = self.comments.vote(True, id, utils.anonymize(str(request.remote_addr))) return Response(json.dumps(nv), 200) - @csrf def dislike(self, environ, request, id): nv = self.comments.vote(False, id, utils.anonymize(str(request.remote_addr))) diff --git a/specs/test_comments.py b/specs/test_comments.py index 31d9e0b..1985f0e 100644 --- a/specs/test_comments.py +++ b/specs/test_comments.py @@ -269,27 +269,10 @@ class TestComments(unittest.TestCase): def testDeleteCommentRemovesThread(self): - rv = self.client.post('/new?uri=%2F', data=json.dumps({"text": "..."})) - assert '/' in self.app.db.threads - self.client.delete('/id/1') - assert '/' not in self.app.db.threads - - def testCSRF(self): - - payload = json.dumps({"text": "..."}) - - assert self.client.post('/new?uri=%2F', data=payload, - headers={"Origin": "http://localhost:8080"} - ).status_code == 201 - - assert self.client.post('/new?uri=%2F', data=payload, - headers={"Referer": "http://other.example/asdf", - "User-Agent": "msie"} - ).status_code == 403 - - assert self.client.post('/new?uri=%2F', data=payload, - headers={"Origin": "http://other.example"} - ).status_code == 403 + rv = self.client.post('/new?uri=%2F', data=json.dumps({"text": "..."})) + assert '/' in self.app.db.threads + self.client.delete('/id/1') + assert '/' not in self.app.db.threads class TestModeratedComments(unittest.TestCase):