Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Fix CSRF annoyance #4193

Merged
merged 4 commits into from
Nov 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gratipay/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

Expand Down
9 changes: 7 additions & 2 deletions gratipay/security/__init__.py
Original file line number Diff line number Diff line change
@@ -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)


Expand Down
8 changes: 2 additions & 6 deletions gratipay/security/csrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from aspen import Response

from . import _requesting_asset
from .crypto import constant_time_compare, get_random_string


Expand All @@ -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()
Expand All @@ -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":
Expand Down
3 changes: 3 additions & 0 deletions tests/py/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down