From 26e6325e680c3d2098e1a8938807e64d7e359308 Mon Sep 17 00:00:00 2001 From: Chris Wagner Date: Sun, 21 Jul 2024 11:43:06 -0700 Subject: [PATCH] Allow more flexibility in allowed redirect targets. (#1011) * Chores before 5.5 - add flask_principal to our 'test main branch' tox * Allow more flexibility in allowed redirect targets. Add a new config SECURITY_REDIRECT_BASE_DOMAIN which specifies a domain against which to check SECURITY_REDIRECT_ALLOWED_SUBDOMAINS. This enables applications where Flask's SERVER_NAME isn't actually at the base so can't be used to validate redirects. close #983 --- CHANGES.rst | 3 ++- docs/configuration.rst | 30 +++++++++++++++++++++++++++- flask_security/core.py | 6 ++++-- flask_security/utils.py | 21 ++++++++++++++++---- tests/test_basic.py | 44 +++++++++++++++++++++++++++++++++++++++-- tox.ini | 1 + 6 files changed, 95 insertions(+), 10 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 517aec12..069a07e2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -17,6 +17,7 @@ Features & Improvements - (:issue:`994`) Add support for Flask-SQLAlchemy-Lite - including new all-inclusive models that conform to sqlalchemy latest best-practice (type-annotated). - (:pr:`1007`) Convert other sqlalchemy-based datastores from legacy 'model.query' to best-practice 'select' +- (:issue:`983`) Allow applications more flexibility defining redirects. Fixes +++++ @@ -24,7 +25,7 @@ Fixes ritual - just as we return the CSRF token. (thanks @e-goto) - (:issue:`973`) login and unified sign in should handle GET for authenticated user consistently. - (:pr:`995`) Don't show sms options if not defined in US_ENABLED_METHODS. (fredipevcin) -- (:pr:`xxx`) Change :py:data:`SECURITY_DEPRECATED_HASHING_SCHEMES` to ``["auto"]``. +- (:pr:`1009`) Change :py:data:`SECURITY_DEPRECATED_HASHING_SCHEMES` to ``["auto"]``. Docs and Chores +++++++++++++++ diff --git a/docs/configuration.rst b/docs/configuration.rst index a178effc..3355574f 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -182,13 +182,41 @@ These configuration keys are used globally across all features. If ``True`` then subdomains (and the root domain) of the top-level host set by Flask's ``SERVER_NAME`` configuration will be allowed as post-view redirect targets. - This is beneficial if you wish to place your authentiation on one subdomain and + This is beneficial if you wish to place your authentication on one subdomain and authenticated content on another, for example ``auth.domain.tld`` and ``app.domain.tld``. Default: ``False``. .. versionadded:: 4.0.0 +.. py:data:: SECURITY_REDIRECT_BASE_DOMAIN + + Set the base domain for checking allowable redirects. The intent here is to + allow an application to be server on e.g. "flaskapp.my.org" and redirect + to "myservice.my.org" (which maybe isn't a Flask app). Flask's SERVER_NAME + can't be used to verify redirects in this case. Note that in most cases + the application will want to set Flask's SESSION_COOKIE_DOMAIN to be this base domain - + otherwise authorization information won't be sent. + + Default: ``None`` + + .. versionadded:: 5.5.0 + +.. py:data:: SECURITY_REDIRECT_ALLOWED_SUBDOMAINS + + A list of subdomains. Each will be prepended to + ``SECURITY_REDIRECT_BASE_DOMAIN`` and checked against the requested redirect. + + Default: ``[]`` + + .. versionadded:: 5.5.0 + + +.. note:: + The above 4 config options apply BOTH to the handling of ``next`` parameter + as well as all the ``XXX_VIEW`` URL configuration options + for those views that perform a redirect after processing. + .. py:data:: SECURITY_CSRF_PROTECT_MECHANISMS Authentication mechanisms that require CSRF protection. diff --git a/flask_security/core.py b/flask_security/core.py index 47c9ddd1..6f3c37ff 100644 --- a/flask_security/core.py +++ b/flask_security/core.py @@ -203,6 +203,8 @@ "REDIRECT_HOST": None, "REDIRECT_BEHAVIOR": None, "REDIRECT_ALLOW_SUBDOMAINS": False, + "REDIRECT_BASE_DOMAIN": None, + "REDIRECT_ALLOWED_SUBDOMAINS": [], "FORGOT_PASSWORD_TEMPLATE": "security/forgot_password.html", "LOGIN_USER_TEMPLATE": "security/login_user.html", "REGISTER_USER_TEMPLATE": "security/register_user.html", @@ -231,8 +233,8 @@ "CHANGE_EMAIL_WITHIN": "2 hours", "CHANGE_EMAIL_URL": "/change-email", "CHANGE_EMAIL_CONFIRM_URL": "/change-email-confirm", - "CHANGE_EMAIL_ERROR_VIEW": None, - "POST_CHANGE_EMAIL_VIEW": None, + "CHANGE_EMAIL_ERROR_VIEW": None, # spa + "POST_CHANGE_EMAIL_VIEW": None, # spa "CHANGE_EMAIL_SALT": "change-email-salt", "CHANGE_EMAIL_SUBJECT": _("Confirm your new email address"), "TWO_FACTOR_AUTHENTICATOR_VALIDITY": 120, diff --git a/flask_security/utils.py b/flask_security/utils.py index 11554f08..c0486c87 100644 --- a/flask_security/utils.py +++ b/flask_security/utils.py @@ -582,7 +582,14 @@ def url_for_security(endpoint: str, **values: t.Any) -> str: def validate_redirect_url(url: str) -> bool: - """Validate that the URL for redirect is relative.""" + """Validate redirect URL + In the default configuration only redirects to the same domain (and scheme) + are allowed. + The REDIRECT_ALLOW_SUBDOMAINS allows ANY subdomain of SERVER_NAME + to be a redirect target. + The REDIRECT_BASE_DOMAIN and REDIRECT_ALLOWED_SUBDOMAINS allow specifying 'side' + redirects. + """ if url is None or url.strip() == "": return False @@ -599,8 +606,14 @@ def validate_redirect_url(url: str) -> bool: ) ): return True - else: - return False + base_domain = config_value("REDIRECT_BASE_DOMAIN") + if base_domain: + allowable = [ + f"{sub}.{base_domain}" + for sub in config_value("REDIRECT_ALLOWED_SUBDOMAINS") + ] + return url_next.netloc in allowable + return False return True @@ -1038,7 +1051,7 @@ def csrf_cookie_handler(response: Response) -> Response: elif current_app.config["WTF_CSRF_TIME_LIMIT"]: current_cookie = request.cookies.get(csrf_cookie_name, None) if current_cookie: - # Lets make sure it isn't expired if app doesn't set TIME_LIMIT to None. + # Let's make sure it isn't expired if app doesn't set TIME_LIMIT to None. try: csrf.validate_csrf(current_cookie) except ValidationError: diff --git a/tests/test_basic.py b/tests/test_basic.py index 27118d2c..d52be429 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -152,15 +152,17 @@ def test_authenticate_with_subdomain_next(app, client, get_message): data = dict(email="matt@lp.com", password="password") response = client.post("/login?next=http://sub.lp.com", data=data) assert response.status_code == 302 + assert response.location == "http://sub.lp.com" +@pytest.mark.settings(subdomain="auth") def test_authenticate_with_root_domain_next(app, client, get_message): app.config["SERVER_NAME"] = "lp.com" - app.config["SECURITY_SUBDOMAIN"] = "auth" app.config["SECURITY_REDIRECT_ALLOW_SUBDOMAINS"] = True data = dict(email="matt@lp.com", password="password") - response = client.post("/login?next=http://lp.com", data=data) + response = client.post("http://auth.lp.com/login?next=http://lp.com", data=data) assert response.status_code == 302 + assert response.location == "http://lp.com" def test_authenticate_with_invalid_subdomain_next(app, client, get_message): @@ -178,6 +180,44 @@ def test_authenticate_with_subdomain_next_default_config(app, client, get_messag assert get_message("INVALID_REDIRECT") in response.data +@pytest.mark.settings( + redirect_base_domain="bigidea.org", redirect_allowed_subdomains=["my.photo", "blog"] +) +def test_allow_subdomains(app, client, get_message): + app.config["SERVER_NAME"] = "app.bigidea.org" + data = dict(email="matt@lp.com", password="password") + # not in subdomain allowed list + response = client.post("/login?next=http://blog2.bigidea.org", data=data) + assert get_message("INVALID_REDIRECT") in response.data + + response = client.post("/login?next=http://my.photo.bigidea.org/image", data=data) + assert response.location == "http://my.photo.bigidea.org/image" + + +@pytest.mark.settings( + redirect_base_domain="bigidea.org", redirect_allowed_subdomains=[] +) +def test_redirect_allow_subdomains(app, client, get_message): + app.config["SERVER_NAME"] = "bigidea.org" + data = dict(email="matt@lp.com", password="password") + response = client.post("/login?next=http://blog2.bigidea.org", data=data) + assert get_message("INVALID_REDIRECT") in response.data + response = client.post("/login?next=http://bigidea.org/imin", data=data) + assert response.location == "http://bigidea.org/imin" + + +@pytest.mark.settings( + post_login_view="http://blog.bigidea.org/post_login", + redirect_base_domain="bigidea.org", + redirect_allowed_subdomains=["my.photo", "blog"], +) +def test_view_redirect(app, client, get_message): + app.config["SERVER_NAME"] = "bigidea.org" + data = dict(email="matt@lp.com", password="password") + response = client.post("/login", data=data) + assert response.location == "http://blog.bigidea.org/post_login" + + def test_authenticate_case_insensitive_email(app, client): response = authenticate(client, "MATT@lp.com", follow_redirects=True) assert b"Welcome matt@lp.com" in response.data diff --git a/tox.ini b/tox.ini index a7872f5c..7ed22528 100644 --- a/tox.ini +++ b/tox.ini @@ -46,6 +46,7 @@ deps = git+https://github.com/pallets/flask@main#egg=flask git+https://github.com/pallets/flask-sqlalchemy@main#egg=flask-sqlalchemy git+https://github.com/pallets/jinja@main#egg=jinja2 + git+https://github.com/pallets-eco/flask-principal@main#egg=flask-principal git+https://github.com/wtforms/wtforms@master#egg=wtforms git+https://github.com/maxcountryman/flask-login@main#egg=flask-login commands =