From 51d2355ee709ead70120448365e1a19c03fbf26f Mon Sep 17 00:00:00 2001 From: Chris Wagner Date: Thu, 2 May 2024 13:30:02 -0700 Subject: [PATCH] Improve CSRF and SPA (CSRF_COOKIE). (#972) We used to set the CSRF_COOKIE (if configured) at the end of a successful authentication. For 2-factor that meant that /tf-validate needed to have the CSRF-HEADER set manually (as well as /login). There seems no reason not to set the CSRF-COOKIE on GET /login - just as we return the csrf_token - so that all endpoints can use the cookie if wanted (which is what many js frameworks do). There appeared to be no CSRF tests for logging in with unified sign in - now there is. closes #965 --- docs/patterns.rst | 18 ++++--- docs/spa.rst | 2 +- flask_security/unified_signin.py | 10 +++- flask_security/views.py | 6 +++ requirements/dev.txt | 6 +-- requirements/docs.txt | 6 +-- requirements/tests.txt | 2 +- tests/conftest.py | 5 ++ tests/test_two_factor.py | 89 ++++++++++++++++++++++++++++++++ tests/test_unified_signin.py | 69 +++++++++++++++++++++++++ 10 files changed, 195 insertions(+), 18 deletions(-) diff --git a/docs/patterns.rst b/docs/patterns.rst index 011901bd..36cb2f62 100644 --- a/docs/patterns.rst +++ b/docs/patterns.rst @@ -234,12 +234,12 @@ Behind-The-Scenes ++++++++++++++++++ Depending on configuration, there are 3 places CSRF tokens can be checked: #) As part of form validation for any form derived from FlaskForm (which all Flask-Security - forms are. An error here is recorded in the ``csrf_token`` field and the calling view - decided whether to return 200 or 400 (all Flask-Security views return 200 with form field errors). + forms are). An error here is recorded in the ``csrf_token`` field and the calling view + decides whether to return 200 or 400. This is the default if no other configuration changes are made. #) As part of an @before_request handler that Flask-WTF sets up if CSRFprotect() is called. On error this always returns HTTP 400 and small snippet of HTML. This can be disabled - by setting config["WTF_CSRF_CHECK_DEFAULT"] = True. + by setting config["WTF_CSRF_CHECK_DEFAULT"] = False. #) As part of a Flask-Security decorator (:func:`.unauth_csrf`, :func:`.auth_required`). On error either a JSON response is returned OR CSRFError exception is raised and 400 is returned with the small snippet of HTML (the exception and default response is part of Flask-WTF). @@ -325,11 +325,16 @@ Be aware that if you enable this it will ONLY work if you send the session cooki is to set `WTF_CSRF_CHECK_DEFAULT` to `False` - which will disable the @before_request and let Flask-Security handle CSRF protection including properly returning a JSON response if the caller asks for it. +.. danger:: + If you set `WTF_CSRF_CHECK_DEFAULT` to `False` this will turn off CSRF checking for all your endpoints unless they are protected with + @auth_required() or have explicit CSRF checking. + Using a Cookie -------------- -You can instruct Flask-Security to send a cookie that contains the csrf token. This can be very -convenient since various javascript AJAX packages are pre-configured to extract the contents of a cookie +You can instruct Flask-Security to send a cookie that contains the csrf token. +The cookie will be set on a call to ``GET /login`` or ``GET /us-signin`` - as well as after a successful authentication. +This can be very convenient since various javascript AJAX packages are pre-configured to extract the contents of a cookie and send it on every mutating request as an HTTP header. `axios`_ for example has a default configuration that it will look for a cookie named ``XSRF-TOKEN`` and will send the contents of that back in an HTTP header called ``X-XSRF-Token``. This means that if you use that package you don't need to make @@ -341,9 +346,6 @@ any changes to your UI and just need the following configuration:: # Don't have csrf tokens expire (they are invalid after logout) app.config["WTF_CSRF_TIME_LIMIT"] = None - # You can't get the cookie until you are logged in. - app.config["SECURITY_CSRF_IGNORE_UNAUTH_ENDPOINTS"] = True - # Enable CSRF protection flask_wtf.CSRFProtect(app) diff --git a/docs/spa.rst b/docs/spa.rst index 6dbf7d53..dcace667 100644 --- a/docs/spa.rst +++ b/docs/spa.rst @@ -191,7 +191,7 @@ Some background material: .. _Single Page Applications (spa): https://en.wikipedia.org/wiki/Single-page_application .. _Nginx: https://www.nginx.com/ .. _S3: https://www.savjee.be/2018/05/Content-security-policy-and-aws-s3-cloudfront/ -.. _Flask-Talisman: https://github.com/GoogleCloudPlatform/flask-talisman +.. _Flask-Talisman: https://pypi.org/project/flask-talisman/ .. _CORS: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS .. _Flask-CORS: https://github.com/corydolphin/flask-cors .. _Zappa: https://github.com/Miserlou/Zappa diff --git a/flask_security/unified_signin.py b/flask_security/unified_signin.py index 0bd19f22..abe953e4 100644 --- a/flask_security/unified_signin.py +++ b/flask_security/unified_signin.py @@ -590,14 +590,20 @@ def us_signin() -> ResponseValue: return base_render_json(form, include_auth_token=True) return redirect(get_post_login_redirect()) - elif request.method == "POST" and cv("RETURN_GENERIC_RESPONSES"): + + # Here on GET or failed POST validate + if request.method == "POST" and cv("RETURN_GENERIC_RESPONSES"): rinfo = dict( identity=dict(replace_msg="GENERIC_AUTHN_FAILED"), passcode=dict(replace_msg="GENERIC_AUTHN_FAILED"), ) form_errors_munge(form, rinfo) - # Here on GET or failed POST validate + if request.method == "GET": + # set CSRF COOKIE if configured. This is the equivalent of forms and + # base_render_json always sending the csrf_token + session["fs_cc"] = "set" + code_methods = _compute_code_methods() if _security._want_json(request): payload = { diff --git a/flask_security/views.py b/flask_security/views.py index 5748450f..f6ea6164 100644 --- a/flask_security/views.py +++ b/flask_security/views.py @@ -216,6 +216,11 @@ def login() -> "ResponseValue": fields_to_squash["username"] = dict(replace_msg="GENERIC_AUTHN_FAILED") form_errors_munge(form, fields_to_squash) + if request.method == "GET": + # set CSRF COOKIE if configured. This is the equivalent of forms and + # base_render_json always sending the csrf_token + session["fs_cc"] = "set" + if _security._want_json(request): payload = { "identity_attributes": get_identity_attributes(), @@ -227,6 +232,7 @@ def login() -> "ResponseValue": and cv("REQUIRES_CONFIRMATION_ERROR_VIEW") and not cv("RETURN_GENERIC_RESPONSES") ): + # Validation failed BECAUSE user needs to confirm assert form.user_authenticated do_flash(*get_message("CONFIRMATION_REQUIRED")) return redirect( diff --git a/requirements/dev.txt b/requirements/dev.txt index d7042fa4..3a7a68a8 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -12,12 +12,12 @@ types-requests # for dev - might not install Flask-Security - list those dependencies here flask -flask_wtf -flask_login +flask-wtf +flask-login wtforms markupsafe passlib blinker -email-validator +email_validator itsdangerous importlib_resources>=5.10.0 diff --git a/requirements/docs.txt b/requirements/docs.txt index c491d265..b3f0d791 100644 --- a/requirements/docs.txt +++ b/requirements/docs.txt @@ -1,6 +1,6 @@ -Pallets-Sphinx-Themes~=2.1 -Sphinx~=7.2 -sphinx-issues==3.0.1 +Pallets-Sphinx-Themes +Sphinx +sphinx-issues packaging Flask-Login Flask-SQLAlchemy diff --git a/requirements/tests.txt b/requirements/tests.txt index 603e175c..9501c4dc 100644 --- a/requirements/tests.txt +++ b/requirements/tests.txt @@ -5,7 +5,7 @@ Flask-Mailman Flask-Principal peewee; python_version < '3.12' Flask-SQLAlchemy -argon2_cffi +argon2-cffi authlib bcrypt bleach diff --git a/tests/conftest.py b/tests/conftest.py index db51f4b7..ddeb55f7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -301,6 +301,11 @@ def page_1(): def echo_json(): return jsonify(flask_request.get_json()) + @app.route("/json_auth", methods=["POST"]) + @auth_required() + def echo_jsonauth(): + return jsonify(flask_request.get_json()) + @app.route("/unauthz", methods=["GET", "POST"]) def unauthz(): return render_template("index.html", content="Unauthorized") diff --git a/tests/test_two_factor.py b/tests/test_two_factor.py index 7525b2a0..2a5cf669 100644 --- a/tests/test_two_factor.py +++ b/tests/test_two_factor.py @@ -1505,3 +1505,92 @@ def test_setup_csrf_header(app, client): "tf-setup", json=dict(setup="disable"), headers={"X-CSRF-Token": csrf_token} ) assert response.status_code == 200 + + +@pytest.mark.csrf(csrfprotect=True) +@pytest.mark.settings(CSRF_COOKIE_NAME="XSRF-Token") +def test_csrf_2fa_login_cookie(app, client): + # Use XSRF-Token cookie for entire login sequence + sms_sender = SmsSenderFactory.createSender("test") + response = client.get( + "/login", data={}, headers={"Content-Type": "application/json"} + ) + assert client.get_cookie("XSRF-Token") + csrf_token = response.json["response"]["csrf_token"] + assert csrf_token == client.get_cookie("XSRF-Token").value + + response = client.post( + "/login", + json=dict(email="gal@lp.com", password="password"), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert b'"code": 200' in response.data + session = get_session(response) + assert session["tf_state"] == "ready" + + assert sms_sender.get_count() == 1 + code = sms_sender.messages[0].split()[-1] + + response = client.post( + "/tf-validate", + json=dict(code=code), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert response.status_code == 200 + + # verify original session csrf_token still works. + response = client.post( + "/json_auth", + json=dict(label="label"), + headers={"Content-Type": "application/json", "X-CSRF-Token": csrf_token}, + ) + assert response.status_code == 200 + + # use XSRF_Cookie to send in csrf_token + response = client.post( + "/json_auth", + json=dict(label="label"), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert response.status_code == 200 + assert response.json["label"] == "label" + + +@pytest.mark.csrf(ignore_unauth=True, csrfprotect=True) +@pytest.mark.settings(CSRF_COOKIE_NAME="XSRF-Token") +def test_csrf_2fa_nounauth_cookie(app, client): + # use CSRF cookie when ignoring unauth endpoints + sms_sender = SmsSenderFactory.createSender("test") + response = client.post( + "/login", + json=dict(email="gal@lp.com", password="password"), + headers={"Content-Type": "application/json"}, + ) + + code = sms_sender.messages[0].split()[-1] + response = client.post( + "/tf-validate", + json=dict(code=code), + headers={"Content-Type": "application/json"}, + ) + assert response.status_code == 200 + + response = client.post( + "/json_auth", + json=dict(label="label"), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert response.status_code == 200 + assert response.json["label"] == "label" diff --git a/tests/test_unified_signin.py b/tests/test_unified_signin.py index b9435179..ce283876 100644 --- a/tests/test_unified_signin.py +++ b/tests/test_unified_signin.py @@ -29,6 +29,7 @@ check_location, check_xlation, get_form_action, + get_session, is_authenticated, logout, reset_fresh, @@ -2214,3 +2215,71 @@ def test_empty_password_xlate(app, client, get_message): ).encode() in response.data ) + + +@pytest.mark.two_factor() +@pytest.mark.csrf(csrfprotect=True) +@pytest.mark.settings(CSRF_COOKIE_NAME="XSRF-Token") +def test_csrf_2fa_us_cookie(app, client): + # Use XSRF-Token cookie for entire login sequence + sms_sender = SmsSenderFactory.createSender("test") + response = client.get( + "/us-signin", data={}, headers={"Content-Type": "application/json"} + ) + assert client.get_cookie("XSRF-Token") + csrf_token = response.json["response"]["csrf_token"] + assert csrf_token == client.get_cookie("XSRF-Token").value + + # verify requires CSRF + response = client.post( + "/us-signin", + json=dict(identity="gal@lp.com", passcode="password"), + headers={"Content-Type": "application/json"}, + ) + assert response.status_code == 400 + assert response.json["response"]["errors"][0] == "The CSRF token is missing." + + response = client.post( + "/us-signin", + json=dict(identity="gal@lp.com", passcode="password"), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert b'"code": 200' in response.data + session = get_session(response) + assert session["tf_state"] == "ready" + + assert sms_sender.get_count() == 1 + code = sms_sender.messages[0].split()[-1] + + response = client.post( + "/tf-validate", + json=dict(code=code), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert response.status_code == 200 + + # verify original session csrf_token still works. + response = client.post( + "/json_auth", + json=dict(label="label"), + headers={"Content-Type": "application/json", "X-CSRF-Token": csrf_token}, + ) + assert response.status_code == 200 + + # use XSRF_Cookie to send in csrf_token + response = client.post( + "/json_auth", + json=dict(label="label"), + headers={ + "Content-Type": "application/json", + "X-CSRF-Token": client.get_cookie("XSRF-Token").value, + }, + ) + assert response.status_code == 200 + assert response.json["label"] == "label"