Skip to content

Commit

Permalink
Revert Referrer-Policy header commit. (#842)
Browse files Browse the repository at this point in the history
Although OWASP still recommends that reset password and confirmation links have the no-referrer header option set - this causes issues with HTTPS and Flask-WTF that requires a referrer header.
Also - for the past 5 years, the browser default for Referrer-Policy is 'strict-origin-when-cross-origin' which should be enough to mitigate any possible Referrer leakage.

closes #829
  • Loading branch information
jwag956 authored Sep 29, 2023
1 parent c9cf5d0 commit dc342d4
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 61 deletions.
22 changes: 19 additions & 3 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@ Flask-Security Changelog

Here you can see the full list of changes between each Flask-Security release.

Version 5.3.1
-------------

Released October XX, 2023

Please Note: py_webauthn is pinned to 1.9.0 until the issue with user_handle
is resolved.

Fixes
++++++

- (:issue:`829`) Revert change in 5.3.0 that added a Referrer-Policy header.
- (:issue:`826`) Fix error in quickstart (codycollier)
- (:pr:`835`) Update Armenian translations (amkrtchyan-tmp)
- (:pr:`831`) Update German translations. (sr-verde)

Version 5.3.0
-------------

Expand Down Expand Up @@ -51,7 +67,7 @@ Backwards Compatibility Concerns
- The SECURITY_MSG_PASSWORD_RESET_EXPIRED message no longer contains the user's identity/email.
- The default for :py:data:`SECURITY_RESET_PASSWORD_WITHIN` has been changed from `5 days` to `1 days`.
- The response to GET /reset/<token> sets the HTTP header `Referrer-Policy` to `no-referrer` as suggested
by OWASP.
by OWASP. *PLEASE NOTE: this was backed out in 5.3.1*
- Confirm email was changed to adhere to OWASP recommendations and reduce possible exploitation:

- A new email (with new token) is no longer sent upon expired token. Users must restart
Expand All @@ -63,7 +79,7 @@ Backwards Compatibility Concerns
align better with OWASP best practices. Setting it to ``True`` will restore prior behavior.
- The SECURITY_MSG_CONFIRMATION_EXPIRED message no longer contains the user's identity/email.
- The response to GET /reset/<token> sets the HTTP header `Referrer-Policy` to `no-referrer` as suggested
by OWASP.
by OWASP. *PLEASE NOTE: this was backed out in 5.3.1*

Version 5.2.0
-------------
Expand Down Expand Up @@ -200,7 +216,7 @@ Fixes
Version 5.0.0
-------------

Released August 27. 2022
Released August 27, 2022

**PLEASE READ CHANGE NOTES CAREFULLY - THERE ARE LIKELY REQUIRED CHANGES YOU WILL HAVE TO MAKE.**

Expand Down
79 changes: 29 additions & 50 deletions flask_security/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,23 +462,17 @@ def confirm_email(token):

if _security.redirect_behavior == "spa":
# No reason to expose identity info to anyone who has the link
return (
redirect(
get_url(
_security.confirm_error_view,
qparams={c: m},
)
),
{"Referrer-Policy": "no-referrer"},
return redirect(
get_url(
_security.confirm_error_view,
qparams={c: m},
)
)

do_flash(m, c)
return (
redirect(
get_url(_security.confirm_error_view)
or url_for_security("send_confirmation")
),
{"Referrer-Policy": "no-referrer"},
return redirect(
get_url(_security.confirm_error_view)
or url_for_security("send_confirmation")
)

confirm_user(user)
Expand All @@ -500,30 +494,22 @@ def confirm_email(token):
)
if response:
do_flash(m, c)
return response, {"Referrer-Policy": "no-referrer"}
return response
login_user(user, authn_via=["confirm"])

if _security.redirect_behavior == "spa":
return (
redirect(
get_url(
_security.post_confirm_view,
qparams=user.get_redirect_qparams({c: m}),
)
),
{"Referrer-Policy": "no-referrer"},
return redirect(
get_url(
_security.post_confirm_view,
qparams=user.get_redirect_qparams({c: m}),
)
)
do_flash(m, c)
return (
redirect(
get_url(_security.post_confirm_view)
or get_url(
_security.post_login_view
if cv("AUTO_LOGIN_AFTER_CONFIRM")
else ".login"
)
),
{"Referrer-Policy": "no-referrer"},
return redirect(
get_url(_security.post_confirm_view)
or get_url(
_security.post_login_view if cv("AUTO_LOGIN_AFTER_CONFIRM") else ".login"
)
)


Expand Down Expand Up @@ -616,26 +602,19 @@ def reset_password(token):
# Still - don't include PII such as identity and email if someone
# intercepts link they still won't necessarily know the login identity
# (even though they can change the password!).
# OWASP recommends setting no-referrer for requests with tokens.
if _security.redirect_behavior == "spa":
return (
redirect(
get_url(
_security.reset_view,
qparams={"token": token},
)
),
{"Referrer-Policy": "no-referrer"},
return redirect(
get_url(
_security.reset_view,
qparams={"token": token},
)
)
# for forms - render the reset password form
return (
_security.render_template(
cv("RESET_PASSWORD_TEMPLATE"),
reset_password_form=form,
reset_password_token=token,
**_ctx("reset_password"),
),
{"Referrer-Policy": "no-referrer"},
return _security.render_template(
cv("RESET_PASSWORD_TEMPLATE"),
reset_password_form=form,
reset_password_token=token,
**_ctx("reset_password"),
)

# This is the POST case.
Expand Down
4 changes: 0 additions & 4 deletions tests/test_confirmable.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def on_instructions_sent(app, **kwargs):
token = registrations[0]["confirm_token"]
response = clients.get("/confirm/" + token, follow_redirects=False)
assert len(recorded_confirms) == 1
assert response.headers.get("Referrer-Policy", None) == "no-referrer"
response = clients.get(response.location)
assert get_message("EMAIL_CONFIRMED") in response.data
# make sure not logged in
Expand Down Expand Up @@ -361,7 +360,6 @@ def test_confirm_redirect_to_post_confirm(client, get_message):

response = client.get("/confirm/" + token, follow_redirects=False)
assert "/post_confirm" in response.location
assert response.headers.get("Referrer-Policy", None) == "no-referrer"


@pytest.mark.registerable()
Expand Down Expand Up @@ -393,7 +391,6 @@ def test_spa_get(app, client, get_message):
assert "/confirm-redirect" == split.path
qparams = dict(parse_qsl(split.query))
assert qparams["email"] == "dude@lp.com"
assert response.headers.get("Referrer-Policy", None) == "no-referrer"

response = client.get("/confirm/" + token)
split = urlsplit(response.headers["Location"])
Expand All @@ -402,7 +399,6 @@ def test_spa_get(app, client, get_message):
assert "/confirm-error" in response.location
assert "email" not in qparams
assert get_message("ALREADY_CONFIRMED") in qparams["info"].encode("utf-8")
assert response.headers.get("Referrer-Policy", None) == "no-referrer"

# Arguably for json we shouldn't have any - this is buried in register_user
# but really shouldn't be.
Expand Down
4 changes: 0 additions & 4 deletions tests/test_recoverable.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ def on_instructions_sent(app, **kwargs):
# Test view for reset token
response = clients.get("/reset/" + token)
assert b"<h1>Reset password</h1>" in response.data
# OWASP recommends setting this
assert response.headers.get("Referrer-Policy", None) == "no-referrer"

# Test submitting a new password but leave out confirm
response = clients.post(
Expand Down Expand Up @@ -460,8 +458,6 @@ def test_spa_get(app, client):

response = client.get("/reset/" + token)
assert response.status_code == 302
# OWASP recommends setting this
assert response.headers.get("Referrer-Policy", None) == "no-referrer"
split = urlsplit(response.headers["Location"])
assert "localhost:8081" == split.netloc
assert "/reset-redirect" == split.path
Expand Down

0 comments on commit dc342d4

Please sign in to comment.