Merge pull request #4193 from gratipay/fix-csrf-annoyance

Fix CSRF annoyance

gratipay/main.py

@@ -104,9 +104,9 @@
     i18n.set_up_i18n,
     authentication.start_user_as_anon,
     authentication.authenticate_user_if_possible,
+    security.only_allow_certain_methods,
     csrf.extract_token_from_cookie,
     csrf.reject_forgeries,
-    security.only_allow_certain_methods,
 
     algorithm['dispatch_request_to_filesystem'],
 

gratipay/security/__init__.py

@@ -1,9 +1,14 @@
 from aspen import Response
 
 
+_requesting_asset = lambda r: r.path.raw.startswith('/assets/')
+
+
 def only_allow_certain_methods(request):
-    whitelisted = ['GET', 'HEAD', 'POST']
-    if request.method.upper() not in whitelisted:
+    method = request.method.upper()
+    whitelist = ('GET', 'HEAD') if _requesting_asset(request) else ('GET', 'HEAD', 'POST')
+    # POSTing to /assets/ interferes with the csrf.* functions if we're not careful
+    if method not in whitelist:
         raise Response(405)
 
 

gratipay/security/csrf.py

@@ -14,6 +14,7 @@
 
 from aspen import Response
 
+from . import _requesting_asset
 from .crypto import constant_time_compare, get_random_string
 
 
@@ -36,9 +37,8 @@ def extract_token_from_cookie(request):
         token = _sanitize_token(token)
 
     # Don't set a CSRF cookie on assets, to avoid busting the cache.
-    # Don't set it on callbacks, because we don't need it there.
 
-    if request.path.raw.startswith('/assets/') or request.path.raw.startswith('/callbacks/'):
+    if _requesting_asset(request):
         token = None
     else:
         token = token or _get_new_token()
@@ -50,10 +50,6 @@ def reject_forgeries(request, csrf_token):
     # Assume that anything not defined as 'safe' by RC2616 needs protection.
     if request.line.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'):
 
-        # But for webhooks we depend on IP filtering for security.
-        if request.line.uri.startswith('/callbacks/'):
-            return
-
         # Check non-cookie token for match.
         second_token = ""
         if request.line.method == "POST":

tests/py/test_security.py

@@ -35,6 +35,9 @@ def test_oacm_disallows_a_bunch_of_other_stuff(self):
     def test_oacm_doesnt_choke_error_handling(self):
         assert self.client.hit("OPTIONS", "/", raise_immediately=False).code == 405
 
+    def test_oacm_prevents_csrf_from_choking(self):
+        assert self.client.PxST('/assets/gratipay.css').code == 405
+
 
     # ahtr - add_headers_to_response