diff --git a/gratipay/main.py b/gratipay/main.py index eaebc89818..811200b545 100644 --- a/gratipay/main.py +++ b/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'], diff --git a/gratipay/security/__init__.py b/gratipay/security/__init__.py index 7e55e87436..1f41550385 100644 --- a/gratipay/security/__init__.py +++ b/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) diff --git a/gratipay/security/csrf.py b/gratipay/security/csrf.py index 247285b1cf..87f4571c3b 100644 --- a/gratipay/security/csrf.py +++ b/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": diff --git a/tests/py/test_security.py b/tests/py/test_security.py index fdfb5024ae..eae24c24dd 100644 --- a/tests/py/test_security.py +++ b/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