Skip to content
Permalink
Browse files Browse the repository at this point in the history
I421backport (#425)
* Fix security vuln - GET on /login or /change could reveal authentication token with no CSRF checks. (#422)

GETs no longer return the auth token.

closes: #421

* Backport CSRF /login vulnerability.

This will go out at 3.4.5

aargg - issues with black, and other packages w.r.t  py2.7
  • Loading branch information
jwag956 committed Jan 8, 2021
1 parent 1f13d36 commit 61d3131
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 17 deletions.
31 changes: 31 additions & 0 deletions CHANGES.rst
Expand Up @@ -14,6 +14,37 @@ Release Target 2020

.. _here: https://github.com/Flask-Middleware/flask-security/issues/85

Version 3.4.5
--------------

Release January x, 2021

Security Vulnerability Fix.

Two CSRF vulnerabilities were reported: `qrcode`_ and `login`_. This release
fixes the more severe of the 2 - the /login vulnerability. The QRcode issue
has a much smaller risk profile since a) it is only for two-factor authentication
using an authenticator app b) the qrcode is only available during the time
the user is first setting up their authentication app and c) there isn't
an obvious backward compatible fix.
This issue has been fixed in 4.0.

.. _qrcode: https://github.com/Flask-Middleware/flask-security/issues/418
.. _login: https://github.com/Flask-Middleware/flask-security/issues/421

Fixed
+++++

- (:issue:`421`) GET on /login and /change could return the callers authentication_token. This is a security
concern since GETs don't have CSRF protection. This bug was introduced in 3.3.0.

Backwards Compatibility Concerns
++++++++++++++++++++++++++++++++

- (:issue:`421`) Fix CSRF vulnerability on /login and /change that could return the callers authentication token.
Now, callers can only get the authentication token on successful POST calls.


Version 3.4.4
--------------

Expand Down
4 changes: 1 addition & 3 deletions docs/openapi.yaml
Expand Up @@ -10,7 +10,7 @@ info:
By default, all POST requests require a CSRF token. This is handled automatically
if you render the form from your Flask application. If you send JSON, then you must include a request header (configured via __SECURITY_CSRF_HEADER__).
Please read the online documentation to find out details on how CSRF can be confifgured.
Please read the online documentation to find out details on how CSRF can be configured.
_Be aware that the current renderer is great! but has some limitations._
In particular
Expand All @@ -29,8 +29,6 @@ paths:
/login:
get:
summary: Retrieve login form and/or user information
parameters:
- $ref: "#/components/parameters/include_auth_token"
responses:
200:
description: >
Expand Down
23 changes: 13 additions & 10 deletions flask_security/views.py
Expand Up @@ -181,13 +181,14 @@ def login():
login_user(form.user, remember=remember_me, authn_via=["password"])
after_this_request(_commit)

if not _security._want_json(request):
return redirect(get_post_login_redirect())
if _security._want_json(request):
return base_render_json(form, include_auth_token=True)
return redirect(get_post_login_redirect())

if _security._want_json(request):
if current_user.is_authenticated:
form.user = current_user
return base_render_json(form, include_auth_token=True)
return base_render_json(form)

if current_user.is_authenticated:
return redirect(get_url(_security.post_login_view))
Expand Down Expand Up @@ -622,16 +623,18 @@ def change_password():
if form.validate_on_submit():
after_this_request(_commit)
change_user_password(current_user._get_current_object(), form.new_password.data)
if not _security._want_json(request):
do_flash(*get_message("PASSWORD_CHANGE"))
return redirect(
get_url(_security.post_change_view)
or get_url(_security.post_login_view)
)
if _security._want_json(request):
form.user = current_user
return base_render_json(form, include_auth_token=True)

do_flash(*get_message("PASSWORD_CHANGE"))
return redirect(
get_url(_security.post_change_view) or get_url(_security.post_login_view)
)

if _security._want_json(request):
form.user = current_user
return base_render_json(form, include_auth_token=True)
return base_render_json(form)

return _security.render_template(
config_value("CHANGE_PASSWORD_TEMPLATE"),
Expand Down
2 changes: 1 addition & 1 deletion pytest.ini
@@ -1,5 +1,5 @@
[pytest]
addopts = -rs --cov flask_security --cov-report term-missing --black --flake8 --cache-clear
addopts = -rs --cov flask_security --cov-report term-missing --flake8 --cache-clear
flake8-max-line-length = 88
flake8-ignore =
tests/view_scaffold.py E402
Expand Down
6 changes: 3 additions & 3 deletions setup.py
Expand Up @@ -13,7 +13,7 @@
version = re.search(r'__version__ = "(.*?)"', f.read()).group(1)

tests_require = [
"Flask-Mongoengine>=0.9.5",
"Flask-Mongoengine~=0.9.5",
"peewee>=3.11.2",
"Flask-SQLAlchemy>=2.3",
"argon2_cffi>=19.1.0",
Expand All @@ -24,8 +24,8 @@
"cryptography>=2.3.1",
"isort>=4.2.2",
"mock>=1.3.0",
"mongoengine>=0.15.3",
"mongomock>=3.14.0",
"mongoengine~=0.19.1",
"mongomock~=3.19.0",
"msgcheck>=2.9",
"pony>=0.7.11",
"phonenumberslite>=8.11.1",
Expand Down
23 changes: 23 additions & 0 deletions tests/test_common.py
Expand Up @@ -749,3 +749,26 @@ def test_session_query(in_app_context):
assert response.status_code == 200
end_nqueries = get_num_queries(app.security.datastore)
assert current_nqueries is None or end_nqueries == (current_nqueries + 2)


@pytest.mark.changeable()
def test_no_get_auth_token(app, client):
# Test that GETs don't return an auth token. This is a security issue since
# GETs aren't protected with CSRF
authenticate(client)
response = client.get(
"/login?include_auth_token", headers={"Content-Type": "application/json"}
)
assert "authentication_token" not in response.json["response"]["user"]

data = dict(
password="password",
new_password="new strong password",
new_password_confirm="new strong password",
)
response = client.get(
"/change?include_auth_token",
json=data,
headers={"Content-Type": "application/json"},
)
assert "authentication_token" not in response.json["response"]["user"]

0 comments on commit 61d3131

Please sign in to comment.