Skip to content

Commit

Permalink
Changes to default_unauthz_handler to remove use of referrer header.
Browse files Browse the repository at this point in the history
Also - if a configured callable returns None or empty - return a 403, just as the default behavior does.

Document precisely what happens.

closes #904
  • Loading branch information
jwag956 committed Feb 16, 2024
1 parent bfe1deb commit 0447c4d
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 53 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ repos:
- id: pyupgrade
args: [--py38-plus]
- repo: https://github.com/psf/black
rev: 24.1.1
rev: 24.2.0
hooks:
- id: black
- repo: https://github.com/pycqa/flake8
Expand Down
45 changes: 29 additions & 16 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,42 @@ Released xxx
Among other changes, this continues the process of dis-entangling Flask-Security
from Flask-Login and may require some application changes due to backwards incompatible changes.

Features
++++++++
Features & Improvements
+++++++++++++++++++++++
- (:issue:`879`) Work with Flask[async]. view decorators and signals support async handlers.
- (:pr:`900`) CI support for python 3.12
- (:issue:`912`) Improve oauth debugging support. Handle next propagation in a more general way.
- (:pr:`877`) Make AnonymousUser optional and deprecated.
- (:pr:`906`) Remove undocumented and untested looking in session for possible 'next'
redirect location.
- (:pr:`901`) Work with py_webauthn 2.0
- (:pr:`881`) No longer rely on Flask-Login.unauthorized callback. See below for implications.
- (:pr:`899`) Improve (and simplify) Two-Factor setup. See below for backwards compatability issues and new functionality.
- (:issue:`904`) Changes to default unauthorized handler - remove use of referrer header (see below).


Docs and Chores
+++++++++++++++
- (:pr:`889`) Improve method translations for unified signin and two factor. Remove support for Flask-Babelex.
- (:pr:`911`) Chore - stop setting all config as attributes. init_app(\*\*kwargs) can only
set forms, flags, and utility classes.
- (:pr:`873`) Update Spanish and Italian translations. (gissimo)
- (:pr:`855`) Improve translations for two-factor method selection. (gissimo)
- (:pr:`866`) Improve German translations. (sr-verde)
- (:pr:`911`) Remove deprecation of AUTO_LOGIN_AFTER_CONFIRM - it has a reasonable use case.

Fixes
+++++

- (:issue:`845`) us-signin magic link should use fs_uniquifier. (not email)
- (:issue:`893`) Improve open-redirect vulnerability mitigation. (see below)
- (:pr:`873`) Update Spanish and Italian translations. (gissimo)
- (:pr:`877`) Make AnonymousUser optional and deprecated.
- (:issue:`875`) user_datastore.create_user has side effects on mutable inputs. (NoRePercussions)
- (:pr:`878`) The long deprecated _unauthorized_callback/handler has been removed.
- (:pr:`881`) No longer rely on Flask-Login.unauthorized callback. See below for implications.
- (:pr:`855`) Improve translations for two-factor method selection. (gissimo)
- (:pr:`866`) Improve German translations. (sr-verde)
- (:pr:`889`) Improve method translations for unified signin and two factor. Remove support for Flask-Babelex.
- (:issue:`884`) Oauth re-used POST_LOGIN_VIEW which caused confusion. See below for the new configuration and implications.
- (:pr:`899`) Improve (and simplify) Two-Factor setup. See below for backwards compatability issues and new functionality.
- (:pr:`901`) Work with py_webauthn 2.0
- (:pr:`906`) Remove undocumented and untested looking in session for possible 'next'
redirect location.
- (:pr:`908`) Improve CSRF documentation and testing. Fix bug where a CSRF failure could
return an HTML page even if the request was JSON.
- (:pr:`911`) Chore - stop setting all config as attributes. init_app(\*\*kwargs) can only
set forms, flags, and utility classes.
- (:pr:`911`) Remove deprecation of AUTO_LOGIN_AFTER_CONFIRM - it has a reasonable use case.
- (:issue:`912`) Improve oauth debugging support. Handle next propagation in a more general way.
- (:pr:`914`) It was possible that if :data:`SECURITY_EMAIL_VALIDATOR_ARGS` were set that
deliverability would be checked even for login.

Notes
++++++
Expand Down Expand Up @@ -114,6 +121,12 @@ Backwards Compatibility Concerns
the default settings for JSON serialization in Flask attempt to sort the keys - which fails
with the `None` key. An issue has been filed with WTForms - and maybe it will be changed.
Flask-Security now changes any `None` key to `""`.
- The default unauthorized handler behavior has changed slightly and is now documented. The default
(:data:`SECURITY_UNAUTHORIZED_VIEW` == ``None``) has not changed (a default HTTP 403 response).
The precise behavior when :data:`SECURITY_UNAUTHORIZED_VIEW` was set was never documented.
The important change is that Flask-Security no longer ever looks at the request.referrer header and
will never redirect to it. If an application needs that, it can provide a callable that can return
that or any other header.

Version 5.3.3
-------------
Expand Down
4 changes: 2 additions & 2 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,8 @@ Login/Logout
.. py:data:: SECURITY_UNAUTHORIZED_VIEW
Specifies the view to redirect to if a user attempts to access a URL/endpoint that they do
not have permission to access. If this value is ``None``, the user is presented with a default
HTTP 403 response.
not have permission to access. This can be a callable (which returns a URL or ``None``) or an endpoint or a URL.
If this value is ``None`` or the configured callable returns ``None`` or empty, the user is presented with a default HTTP 403 response.

Default: ``None``.

Expand Down
20 changes: 5 additions & 15 deletions flask_security/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from flask_principal import Identity, Permission, RoleNeed, identity_changed
from flask_wtf.csrf import CSRFError
from werkzeug.local import LocalProxy
from werkzeug.routing import BuildError

from .proxies import _security, DecoratedView
from .signals import user_unauthenticated
Expand Down Expand Up @@ -135,23 +134,14 @@ def default_unauthz_handler(func_name, params):
if _security._want_json(request):
payload = json_error_response(errors=unauthz_message)
return _security._render_json(payload, 403, None, None)
view = cv("UNAUTHORIZED_VIEW")
if view:
if view := cv("UNAUTHORIZED_VIEW"):
if callable(view):
view = view()
if not (redirect_to := view()):
abort(403)
else:
try:
view = get_url(view)
except BuildError:
view = None
redirect_to = get_url(view)
do_flash(unauthz_message, unauthz_message_type)
redirect_to = "/"
if request.referrer and not request.referrer.split("?")[0].endswith(
request.path
):
redirect_to = request.referrer

return redirect(view or redirect_to)
return redirect(redirect_to)
abort(403)


Expand Down
2 changes: 1 addition & 1 deletion flask_security/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ def do_flash(message: str, category: str) -> None:
flash(message, category)


def get_url(endpoint_or_url: str, qparams: dict[str, str] | None = None) -> str:
def get_url(endpoint_or_url: str, qparams: dict[str, str] | None = None) -> str | None:
"""Returns a URL if a valid endpoint is found. Otherwise, returns the
provided value.
Expand Down
68 changes: 50 additions & 18 deletions tests/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
get_form_input,
get_num_queries,
hash_password,
init_app_with_options,
is_authenticated,
json_authenticate,
logout,
populate_data,
Expand Down Expand Up @@ -463,34 +465,64 @@ def test_unauthorized_access(client, get_message):
assert response.status_code == 403


@pytest.mark.settings(unauthorized_view=lambda: None)
def test_unauthorized_access_with_referrer(client, get_message):
authenticate(client, "[email protected]")
response = client.get("/admin", headers={"referer": "/admin"})
assert response.location != "/admin"
client.get(response.location)
def test_unauthorized_callable_view(app, sqlalchemy_datastore, get_message):
# Test various options using custom unauthorized view
def unauthz_view():
from flask import request

if request.path == "/admin":
return None
elif request.path == "/admin_perm":
return ""
elif request.path == "/admin_and_editor":
return "/profile"
elif request.path == "/simple":
# N.B. security issue - app should verify this is local
return request.referrer
else:
return "not_implemented"

app.config["SECURITY_UNAUTHORIZED_VIEW"] = unauthz_view
init_app_with_options(app, sqlalchemy_datastore)
client = app.test_client()
# activate tiya
with app.test_request_context("/"):
user = app.security.datastore.find_user(email="[email protected]")
app.security.datastore.activate_user(user)
app.security.datastore.commit()
authenticate(client, "[email protected]")
assert is_authenticated(client, get_message)

response = client.get(
"/admin?a=b", headers={"referer": "http://localhost/admin?x=y"}
)
assert "/" in response.location
client.get(response.headers["Location"])
response = client.get("/admin")
assert response.status_code == 403
response = client.get("/admin_perm")
assert response.status_code == 403

response = client.get("/admin_and_editor", follow_redirects=False)
assert check_location(app, response.location, "/profile")
response = client.get(response.location)
assert response.data.count(get_message("UNAUTHORIZED")) == 1

response = client.get(
"/admin", headers={"referer": "/admin"}, follow_redirects=True
"/simple", headers={"referer": "/myhome"}, follow_redirects=False
)
assert response.data.count(get_message("UNAUTHORIZED")) == 1
assert check_location(app, response.location, "/myhome")


# When referrer is from another path and unauthorized,
# we expect a temp redirect (302) to the referer
response = client.get("/admin?w=s", headers={"referer": "/profile"})
def test_unauthorized_url_view(app, sqlalchemy_datastore):
# Test unknown endpoint basically results in redirect to the given string.
app.config["SECURITY_UNAUTHORIZED_VIEW"] = ".myendpoint"
init_app_with_options(app, sqlalchemy_datastore)
client = app.test_client()
authenticate(client, "[email protected]")
response = client.get("/admin")
assert response.status_code == 302
assert "/profile" in response.location
check_location(app, response.location, ".myendpoint")


@pytest.mark.settings(unauthorized_view="/unauthz")
def test_roles_accepted(clients):
# This specificaly tests that we can pass a URL for unauthorized_view.
# This specifically tests that we can pass a URL for unauthorized_view.
for user in ("[email protected]", "[email protected]"):
authenticate(clients, user)
response = clients.get("/admin_or_editor")
Expand Down

0 comments on commit 0447c4d

Please sign in to comment.