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

GETs no longer return the auth token.

closes: #421
  • Loading branch information
jwag956 committed Jan 5, 2021
1 parent 1f13d36 commit 6d50ee9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 10 deletions.
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
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 6d50ee9

Please sign in to comment.