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

@@ -17,14 +17,15 @@ 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
 +++++
 - (:pr:`972`) Set :py:data:`SECURITY_CSRF_COOKIE` at beginning (GET /login) of authentication
   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
 +++++++++++++++

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.

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,

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:

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

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 =