Merge pull request #1665 from alphagov/change-csrf-token-status

Change the CSRF error from 400 to 403

app/__init__.py

@@ -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(400, 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('There’s something wrong with the link you’ve 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

tests/app/main/test_errorhandlers.py

@@ -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 == "There’s something wrong with the link you’ve 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)