Skip to content

Commit 32a732d

Browse files
authored
Add support for logout CSRF (#1239)
- New configuration option SECURITY_LOGOUT_CSRF that enables CSRF checking - On CSRF failure, return a new logout_user_template which is basically a confirmation form (with CSRF token) - Improve CSRF documentation - Change logout menu to be a POST (and therefore a button) close #1237
2 parents 8239ba5 + 1ac0085 commit 32a732d

16 files changed

Lines changed: 242 additions & 42 deletions

File tree

CHANGES.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Features & Improvements
1616
- (:pr:`1235`) Change default LOGOUT_METHODS to be just ``"POST"``
1717
- (:issue:`1228`) Change default ``csrf`` and ``tf_validity`` cookie config to ``secure=True``
1818
- (:issue:`1228`) The ``tf_validity`` cookie name is now configurable via :py:data:`SECURITY_TWO_FACTOR_VALIDITY_COOKIE_NAME`
19+
- (:issue:`1237`) Add support for CSRF on logout
1920

2021
Fixes
2122
+++++
@@ -32,7 +33,7 @@ Backwards Compatibility Concerns
3233
- The fix for the inverted `is_locked` logic will require any application using it
3334
to invert their logic.
3435
- The change to :py:data:`SECURITY_TOKEN_MAX_AGE` default improves a long-standing
35-
'insecure-out-of-the-box' issue. Applications will have to either change the
36+
'insecure-out-of-the-box' concern. Applications will have to either change the
3637
value if they really want a long-lasting token or use the new refresh token
3738
feature.
3839
- The default for :py:data:`SECURITY_LOGOUT_METHODS` has been changed to just

docs/configuration.rst

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,28 @@ Login/Logout
876876

877877
Default: ``"security/login_user.html"``.
878878

879+
.. py:data:: SECURITY_LOGOUT_USER_TEMPLATE
880+
881+
Specifies the path to the template for the logout page.
882+
Note this is only returned on POST error.
883+
884+
Default: ``"security/logout_user.html"``.
885+
886+
.. versionadded:: 5.9.0
887+
888+
.. py:data:: SECURITY_LOGOUT_CSRF
889+
890+
Should the logout view enforce CSRF. If set to ``"True"`` and if there is a
891+
CSRF token mismatch then the ``"SECURITY_LOGOUT_USER_TEMPLATE"`` will be
892+
returned (if request was a form post). For a JSON request, a 400 response will
893+
be returned.
894+
895+
.. danger::
896+
Be sure to not configure ``"GET"`` as an accepted method since that is not
897+
CSRF protected.
898+
899+
.. versionadded:: 5.9.0
900+
879901
.. py:data:: SECURITY_VERIFY_URL
880902
881903
Specifies the reauthenticate URL. If :py:data:`SECURITY_FRESHNESS` evaluates to < 0; this
@@ -2185,6 +2207,7 @@ A list of all templates:
21852207

21862208
* :py:data:`SECURITY_FORGOT_PASSWORD_TEMPLATE`
21872209
* :py:data:`SECURITY_LOGIN_USER_TEMPLATE`
2210+
* :py:data:`SECURITY_LOGOUT_USER_TEMPLATE`
21882211
* :py:data:`SECURITY_VERIFY_TEMPLATE`
21892212
* :py:data:`SECURITY_REGISTER_USER_TEMPLATE`
21902213
* :py:data:`SECURITY_RESET_PASSWORD_TEMPLATE`

docs/customizing.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ following is a list of view templates:
1616

1717
* `security/forgot_password.html`
1818
* `security/login_user.html`
19+
* `security/logout_user.html`
1920
* `security/mf_recovery.html`
2021
* `security/mf_recovery_codes.html`
2122
* `security/recover_username.html`
@@ -76,6 +77,7 @@ The following is a list of all the available context processor decorators:
7677
* ``context_processor``: All views
7778
* ``forgot_password_context_processor``: Forgot password view
7879
* ``login_context_processor``: Login view
80+
* ``logout_context_processor``: Logout view
7981
* ``mf_recovery_codes_context_processor``: Setup recovery codes view
8082
* ``mf_recovery_context_processor``: Use recovery code view
8183
* ``register_context_processor``: Register view

docs/openapi.yaml

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,9 @@ paths:
211211
description: >
212212
If a refresh-token is supplied, it will be revoked as part
213213
of logging the user out. If SECURITY_REFRESH_TOKEN_COOKIE_NAME is configured,
214-
the refresh token will be deleted as part of logout.
214+
the refresh token (as well as the cookie) will be deleted as part of logout.
215+
If SECURITY_LOGOUT_CSRF is enabled, a valid CSRF token must be provided and
216+
an error can be returned if the token is invalid.
215217
requestBody:
216218
required: false
217219
content:
@@ -223,7 +225,7 @@ paths:
223225
$ref: "#/components/schemas/Logout"
224226
responses:
225227
200:
226-
description: Successful logout
228+
description: Logout response
227229
content:
228230
application/json:
229231
schema:
@@ -238,13 +240,24 @@ paths:
238240
type: integer
239241
example: 200
240242
description: Http status code
243+
text/html:
244+
schema:
245+
description: Unsuccessful logout.
246+
type: string
247+
example: render_template(SECURITY_LOGOUT_USER_TEMPLATE) with error values
241248
302:
242249
description: Successful logout
243250
headers:
244251
Location:
245252
description: Redirect to ``SECURITY_POST_LOGOUT_VIEW``
246253
schema:
247254
type: string
255+
400:
256+
description: Errors while validating form (CSRF).
257+
content:
258+
application/json:
259+
schema:
260+
$ref: "#/components/schemas/DefaultJsonErrorResponse"
248261
/verify:
249262
get:
250263
summary: GET verify/reauthentication form

docs/patterns.rst

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -273,37 +273,51 @@ CSRF
273273
By default, Flask-Security, via Flask-WTForms protects all form based POSTS
274274
from CSRF attacks using well vetted per-session hidden-form-field csrf-tokens.
275275

276-
Any web application that relies on session cookies for authentication must have CSRF protection.
276+
Any web application that uses a session cookie for authentication must have CSRF protection.
277277
For more details please read this `OWASP CSRF cheatsheet <https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.md>`_.
278-
A couple important take-aways - first - it isn't about forms versus JSON - it is about
279-
how the API is authenticated (session cookies versus authentication token). Second there is the
280-
concern about 'login CSRF' - is protection needed prior to authentication (yes if
281-
you have a really secure/popular site).
278+
As mentioned in the OWASP cheatsheet - pure JSON requests should be immune from CSRF exploits ASSUMING that the application
279+
has a properly set CORS policy. Flask-Security, via Werkzeug, is careful to ONLY accept JSON with an ``"application/+json"`` header.
280+
However, currently, there is no way to configure Flask-Security to accept ONLY ``"application/json"`` - meaning that it is important
281+
to properly pass CSRF tokens for ALL requests.
282282

283283
Flask-Security strives to support various options for both its endpoints (e.g. ``/login``)
284284
and the application endpoints (protected with Flask-Security decorators such as :func:`.auth_required`).
285285

286286
If your application just uses forms that are derived from ``Flask-WTF::Flaskform`` - you are done.
287-
Note that all of Flask-Security's endpoints are form based (regardless of how the request was made).
288-
289-
.. note::
290-
The logout endpoint has never been form-based and has never had CSRF protection. With the addition
291-
of the refresh token feature, logout now accepts form/json input - but still does not support
292-
CSRF protection.
287+
All of Flask-Security's endpoints are form based (regardless of how the request was made).
288+
289+
Login CSRF
290+
++++++++++++
291+
Read more about this at `OWASP <https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.md#possible-csrf-vulnerabilities-in-login-forms>`_.
292+
293+
Logout CSRF
294+
++++++++++++
295+
Prior to release 5.9 the logout endpoint never accepted any input nor enabled CSRF protection so
296+
there was never any possible errors. Furthermore, both ``"GET"`` and ``"POST"`` were accepted.
297+
Best practice is to accept only ``"POST"`` requests to avoid trivial 'auto-logout' attacks.
298+
Requiring CSRF for the logout endpoint is best practice - however there are many
299+
sources that disagree with this (and in fact, security advisories are usually NOT issued for applications that don't
300+
support that). The usability issue is that users may or may not be looking for an error when they click the logout button
301+
and NOT logging them out is worse. Flask-Security supports requiring CSRF for logout with the
302+
:py:data:`SECURITY_LOGOUT_CSRF` configuration option.
303+
304+
.. warning::
305+
Enabling CSRF protection means that the logout endpoint can return an error - be sure your application
306+
handles that. For form based applications, Flask-Security will return a 'Confirm Sign Out' template.
293307

294308
Behind-The-Scenes
295309
++++++++++++++++++
296-
Depending on configuration, there are 3 places CSRF tokens can be checked:
310+
Depending on configuration, there are 3 places CSRF tokens can be checked (in order):
311+
#) As part of an @before_request handler that Flask-WTF sets up if CSRFprotect(app) is called. On error
312+
this always returns HTTP 400 and small snippet of HTML. This can be disabled
313+
by setting app.config["WTF_CSRF_CHECK_DEFAULT"] = False.
314+
#) As part of a Flask-Security decorator (:func:`.unauth_csrf`, :func:`.auth_required`). On
315+
error, either a JSON response is returned OR CSRFError exception is raised and 400 is returned with the small snippet of HTML
316+
(the exception and default response is part of Flask-WTF).
297317
#) As part of form validation for any form derived from FlaskForm (which all Flask-Security
298318
forms are). An error here is recorded in the ``csrf_token`` field and the calling view
299319
decides whether to return 200 or 400.
300320
This is the default if no other configuration changes are made.
301-
#) As part of an @before_request handler that Flask-WTF sets up if CSRFprotect() is called. On error
302-
this always returns HTTP 400 and small snippet of HTML. This can be disabled
303-
by setting config["WTF_CSRF_CHECK_DEFAULT"] = False.
304-
#) As part of a Flask-Security decorator (:func:`.unauth_csrf`, :func:`.auth_required`). On
305-
error either a JSON response is returned OR CSRFError exception is raised and 400 is returned with the small snippet of HTML
306-
(the exception and default response is part of Flask-WTF).
307321

308322
CSRF: Single-Page-Applications and AJAX/XHR
309323
++++++++++++++++++++++++++++++++++++++++++++

flask_security/core.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@
196196
"TWO_FACTOR_POST_SETUP_VIEW": ".two_factor_setup", # endpoint or URL
197197
"TWO_FACTOR_ERROR_VIEW": ".login",
198198
"LOGOUT_METHODS": ["POST"],
199+
"LOGOUT_CSRF": False,
200+
"LOGOUT_USER_TEMPLATE": "security/logout_user.html",
199201
"POST_LOGIN_VIEW": "/",
200202
"POST_LOGOUT_VIEW": "/",
201203
"LOGIN_ERROR_VIEW": None, # spa
@@ -2031,7 +2033,6 @@ def _csrf_init(app):
20312033
)
20322034

20332035
if csrf:
2034-
csrf.exempt("flask_security.views.logout")
20352036
# Add configured header to WTF_CSRF_HEADERS
20362037
if ch := cv("CSRF_HEADER", app=app):
20372038
if ch not in app.config["WTF_CSRF_HEADERS"]:
@@ -2285,6 +2286,9 @@ def forgot_password_context_processor(
22852286
def login_context_processor(self, fn: t.Callable[[], dict[str, t.Any]]) -> None:
22862287
self._add_ctx_processor("login", fn)
22872288

2289+
def logout_context_processor(self, fn: t.Callable[[], dict[str, t.Any]]) -> None:
2290+
self._add_ctx_processor("logout", fn)
2291+
22882292
def register_context_processor(self, fn: t.Callable[[], dict[str, t.Any]]) -> None:
22892293
self._add_ctx_processor("register", fn)
22902294

flask_security/forms.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -657,9 +657,11 @@ class LogoutForm(Form):
657657
"""The logout form.
658658
No data is required to logout - however, if a refresh token
659659
is provided, it will be revoked.
660+
Also - CSRF will be checked - whether that will cause an error is up to
661+
the view.
660662
"""
661663

662-
refresh_token = StringField(
664+
refresh_token = HiddenField(
663665
get_form_field_xlate(_("Refresh Token")),
664666
validators=[Optional()],
665667
)
@@ -679,13 +681,12 @@ def __init__(self, *args: t.Any, **kwargs: t.Any):
679681
def validate(self, **kwargs: t.Any) -> bool:
680682
from .tokens import verify_refresh_token
681683

682-
if not super().validate(**kwargs): # pragma: no cover
684+
if not super().validate(**kwargs):
683685
return False
684686
if not self.refresh_token.data:
685687
return True
686688

687689
assert isinstance(self.refresh_token.errors, list)
688-
689690
self.refresh_tracker, self.refresh_errors, uid = verify_refresh_token(
690691
self.refresh_token.data
691692
)

flask_security/templates/security/_menu.html

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
security.username_recovery %}
44
<hr>
55
<h2>{{ _fsdomain('Menu') }}</h2>
6-
<ul>
6+
<menu>
77
{% if _fs_is_user_authenticated(current_user) %}
88
{# already authenticated user #}
9-
<li>
10-
<a href="{{ url_for_security('logout') }}">{{ _fsdomain("Sign out") }}</a>
9+
<li class="fs-button">
10+
<form action="{{ url_for_security('logout') }}" method="post" name="logout_form">
11+
<button type="submit">Sign Out</button>
12+
</form>
1113
</li>
1214
{% if security.changeable %}
1315
<li>
@@ -76,5 +78,5 @@ <h2>{{ _fsdomain('Menu') }}</h2>
7678
</li>
7779
{% endif %}
7880
{% endif %}
79-
</ul>
81+
</menu>
8082
{% endif %}

flask_security/templates/security/base.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
.fs-gap { margin-top: 20px; }
2323
.fs-div { margin: 4px; }
2424
.fs-error-msg { color: darkred; }
25+
.fs-button form {margin: 0}
2526
</style>
2627
{%- endblock styles %}
2728
{%- endblock head %}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{% set title = title|default(_fsdomain("Confirm Sign Out")) %}
2+
{% extends "security/base.html" %}
3+
{% from "security/_macros.html" import render_field_with_errors, render_field, render_field_errors, render_form_errors %}
4+
5+
{% block content %}
6+
{% include "security/_messages.html" %}
7+
<h1>{{ _fsdomain('Confirm Sign Out') }}</h1>
8+
<form action="{{ url_for_security('logout') }}" method="post" name="logout_form">
9+
{{ logout_form.hidden_tag() }}
10+
{{ render_form_errors(logout_form) }}
11+
{{ render_field(logout_form.refresh_token) }}
12+
{{ render_field(logout_form.csrf_token) }}
13+
{{ render_field(logout_form.submit) }}
14+
</form>
15+
{% include "security/_menu.html" %}
16+
{% endblock content %}

0 commit comments

Comments
 (0)