error handlers should not raise. Not even abort(400)s.

Refactor csrf handler into the normal error handler area, and then add
some tests to make sure it does the right thing. Also, change it back
to a 400, because the 403 err page talks about being in the wrong
place, but this is about sending the wrong data through, even though
it's technically a 403. Will need to think about wording but this is a
fine first pass
This commit is contained in:
Leo Hemsted
2017-11-28 12:11:29 +00:00
parent c38d10d773
commit 18b50ddfde
2 changed files with 46 additions and 23 deletions

View File

@@ -92,7 +92,7 @@ def create_app(application):
init_app(application)
statsd_client.init_app(application)
logging.init_app(application, statsd_client)
init_csrf(application)
csrf.init_app(application)
request_helper.init_app(application)
service_api_client.init_app(application)
@@ -133,27 +133,6 @@ def create_app(application):
setup_event_handlers()
def init_csrf(application):
csrf.init_app(application)
@application.errorhandler(CSRFError)
def csrf_handler(reason):
application.logger.warning('csrf.error_message: {}'.format(reason))
if 'user_id' not in session:
application.logger.warning(
u'csrf.session_expired: Redirecting user to log in page'
)
return application.login_manager.unauthorized()
application.logger.warning(
u'csrf.invalid_token: Aborting request, user_id: {user_id}',
extra={'user_id': session['user_id']})
abort(403, reason)
def init_app(application):
application.after_request(useful_headers_after_request)
application.after_request(save_service_after_request)
@@ -492,6 +471,27 @@ def register_errorhandlers(application): # noqa (C901 too complex)
flash('Theres something wrong with the link youve used.')
return _error_response(404)
@application.errorhandler(CSRFError)
def handle_csrf(reason):
application.logger.warning('csrf.error_message: {}'.format(reason))
if 'user_id' not in session:
application.logger.warning(
u'csrf.session_expired: Redirecting user to log in page'
)
return application.login_manager.unauthorized()
application.logger.warning(
u'csrf.invalid_token: Aborting request, user_id: {user_id}',
extra={'user_id': session['user_id']})
resp = make_response(render_template(
"error/400.html",
message=['Something went wrong, please go back and try again.']
), 400)
return useful_headers_after_request(resp)
def setup_event_handlers():
from flask_login import user_logged_in

View File

@@ -1,4 +1,5 @@
from flask import Response
from flask import Response, url_for
from flask_wtf.csrf import CSRFError
import pytest
from bs4 import BeautifulSoup
from notifications_python_client.errors import HTTPError
@@ -38,3 +39,25 @@ def test_malformed_token_returns_page_not_found(logged_in_client, url):
assert page.h1.string.strip() == 'Page could not be found'
flash_banner = page.find('div', class_='banner-dangerous').string.strip()
assert flash_banner == "Theres something wrong with the link youve used."
def test_csrf_returns_400(logged_in_client, mocker):
# we turn off CSRF handling for tests, so fake a CSRF response here.
csrf_err = CSRFError('400 Bad Request: The CSRF tokens do not match.')
mocker.patch('app.main.views.index.render_template', side_effect=csrf_err)
response = logged_in_client.get('/cookies')
assert response.status_code == 400
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'Something went wrong, please go back and try again.'
def test_csrf_redirects_to_sign_in_page_if_not_signed_in(client, mocker):
csrf_err = CSRFError('400 Bad Request: The CSRF tokens do not match.')
mocker.patch('app.main.views.index.render_template', side_effect=csrf_err)
response = client.get('/cookies')
assert response.status_code == 302
assert response.location == url_for('main.sign_in', next='/cookies', _external=True)