Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISS 983 Allow for side redirects with new configuration #984

Closed
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Features & Improvements
+++++++++++++++++++++++
- (:issue:`956`) Add support for changing registered user's email (:py:data:`SECURITY_CHANGE_EMAIL`).
- (:pr:`xxx`) Change default password hash to argon2 (was bcrypt). See below for details.
- (:issue:`983`) Add new configuration for (:py:data:`SECURITY_REDIRECT_MATCH_SUBDOMAINS`) to allow for
more flexible redirect matching.

Fixes
+++++
Expand Down
23 changes: 23 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,29 @@ These configuration keys are used globally across all features.

.. versionadded:: 4.0.0

.. py:data:: SECURITY_REDIRECT_MATCH_SUBDOMAINS

Define which subdomains are allowed to be redirected to. This is a list of strings
that are matched against the subdomain of the redirect target. If the subdomain
matches, the redirect is allowed. If the subdomain is not in the list, the redirect
is not allowed. This is useful when you have multiple subdomains and you want to
restrict the redirect to a specific set of subdomains.

For security reasons, if this setting is configured then the default behavior of
mchineboy marked this conversation as resolved.
Show resolved Hide resolved
allowing all subdomains of SERVER_NAME is disabled. This setting assumes that you
wish to have **explicit** control over your allowed subdomains. If you do not wish this
behavior, then also include an entry that matches your SERVER_NAME variable. I.E.
if SERVER_NAME is 'example.com' then include '.example.com' in the list.

This setting requires that :py:data:`SECURITY_REDIRECT_ALLOW_SUBDOMAINS` is set to ``True``.

Examples: ``['.example.com']`` will allow all subdomains of example.com to be redirected to.
``['auth.example.com']`` will only allow the auth.example.com subdomain to be redirected to.
mchineboy marked this conversation as resolved.
Show resolved Hide resolved

Default: ``[]``.

.. versionadded:: 5.4.X

.. py:data:: SECURITY_CSRF_PROTECT_MECHANISMS

Authentication mechanisms that require CSRF protection.
Expand Down
1 change: 1 addition & 0 deletions flask_security/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@
"REDIRECT_HOST": None,
"REDIRECT_BEHAVIOR": None,
"REDIRECT_ALLOW_SUBDOMAINS": False,
"REDIRECT_MATCH_SUBDOMAINS": [],
"FORGOT_PASSWORD_TEMPLATE": "security/forgot_password.html",
"LOGIN_USER_TEMPLATE": "security/login_user.html",
"REGISTER_USER_TEMPLATE": "security/register_user.html",
Expand Down
64 changes: 53 additions & 11 deletions flask_security/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,17 +616,59 @@ def validate_redirect_url(url: str) -> bool:
url_base = urlsplit(request.host_url)
if (url_next.netloc or url_next.scheme) and url_next.netloc != url_base.netloc:
base_domain = current_app.config.get("SERVER_NAME")
if (
config_value("REDIRECT_ALLOW_SUBDOMAINS")
and base_domain
and (
url_next.netloc == base_domain
or url_next.netloc.endswith(f".{base_domain}")
)
):
return True
else:
return False

# Are we allowed to redirect to other hosts outside of ourself?

if (config_value("REDIRECT_ALLOW_SUBDOMAINS")):
if (config_value("REDIRECT_MATCH_SUBDOMAINS")):

# This is a list of allowed sub domains, there are two
# possible ways to match the subdomain:
# 1. The subdomain is in the list of allowed subdomains (strict)
# 2. The subdomain starts with a . and the netloc ends with this (loose)

allowed_subdomains = config_value("REDIRECT_ALLOWED_SUBDOMAINS")

# Safety check - do we have a list of allowed subdomains?

if (len(allowed_subdomains) > 0):
mchineboy marked this conversation as resolved.
Show resolved Hide resolved

# First, let's check if we have a direct match to the list.

if (url_next.netloc in allowed_subdomains):
return True

# Now, let's check for subdomains that start with a dot.

else:
for subdomain in allowed_subdomains:

mchineboy marked this conversation as resolved.
Show resolved Hide resolved
# We found a subdomain that starts with a dot.
# Let's check if the netloc ends with this subdomain.

if (subdomain.startswith(".") and
url_next.netloc.endswith(subdomain)):

# Gotcha! We found a match.

return True

# Default to False if we didn't find a match.

return False

# Fall through to the original check if we don't have a list of allowed subdomains.

if (
mchineboy marked this conversation as resolved.
Show resolved Hide resolved
base_domain
and (
url_next.netloc == base_domain
or url_next.netloc.endswith(f".{base_domain}")
)
):
return True
else:
return False
return True


Expand Down
36 changes: 36 additions & 0 deletions tests/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,42 @@ def test_authenticate_with_invalid_subdomain_next(app, client, get_message):
assert get_message("INVALID_REDIRECT") in response.data


def test_authenticate_with_subdomain_fuzzy_match_next(app, client, get_message):
app.config["SERVER_NAME"] = "lp.com"
app.config["SECURITY_REDIRECT_ALLOW_SUBDOMAINS"] = True
app.config["SECURITY_REDIRECT_MATCH_SUBDOMAINS"] = ['.lp.com']
data = dict(email="[email protected]", password="password")
response = client.post("/login?next=http://sub.lp.com", data=data)
assert response.status_code == 302


def test_authenticate_with_subdomain_fuzzy_match_next_invalid(app, client, get_message):
app.config["SERVER_NAME"] = "lp.com"
app.config["SECURITY_REDIRECT_ALLOW_SUBDOMAINS"] = True
app.config["SECURITY_REDIRECT_MATCH_SUBDOMAINS"] = ['foo.lp.com']
data = dict(email="[email protected]", password="password")
response = client.post("/login?next=http://sub.lp.net", data=data)
assert get_message("INVALID_REDIRECT") in response.data


def test_authenticate_with_subdomain_fuzzy_match_next_strict(app, client, get_message):
app.config["SERVER_NAME"] = "lp.com"
app.config["SECURITY_REDIRECT_ALLOW_SUBDOMAINS"] = True
app.config["SECURITY_REDIRECT_MATCH_SUBDOMAINS"] = ['sub.lp.com']
data = dict(email="[email protected]", password="password")
response = client.post("/login?next=http://sub.lp.com", data=data)
assert response.status_code == 302


def test_authenticate_with_subdomain_fuzzy_match_next_strict_invalid(app, client, get_message):
app.config["SERVER_NAME"] = "lp.com"
app.config["SECURITY_REDIRECT_ALLOW_SUBDOMAINS"] = True
app.config["SECURITY_REDIRECT_MATCH_SUBDOMAINS"] = ['foo.lp.com']
data = dict(email="[email protected]", password="password")
response = client.post("/login?next=http://sub.lp.com", data=data)
assert get_message("INVALID_REDIRECT") in response.data


def test_authenticate_with_subdomain_next_default_config(app, client, get_message):
app.config["SERVER_NAME"] = "lp.com"
data = dict(email="[email protected]", password="password")
Expand Down
Loading