Allow "side redirects" instead of restricting to just the SERVER_NAME for ?next=URL

[mchineboy]

Environment

• Flask-Security-Too version: 5.4.3
• Flask version: 3.0.3
• Python version: 3.11
• Configuration variable SECURITY_REDIRECT_ALLOW_SUBDOMAINS is set to True

Description

There is a current behaviour in Flask-Security-Too's redirect handling during login (?next=) that permits redirects from foo.example.com to bar.foo.example.com but disallows redirects to bar.example.com. This restriction is limiting when working with subdomains, where foo.example.com, bar.example.com, and other similar subdomains are considered part of the same parent domain example.com.

It's reasonably expected that redirects between subdomains at the same domain level should be permitted, as they fall within the same administration boundary of example.com. For instance, services deployed on different subdomains may need to integrate with each other, and such a policy would hinder this integration unnecessarily.

Expected Behavior

Flask Security Too should allow for redirects from a subdomain to another subdomain where the "base" (parent) domain is the same. For example, given a parent domain example.com, redirects from foo.example.com to bar.example.com should be allowed.

Actual Behavior

Currently, Flask Security Too only allows redirects within the same subdomain hierarchy, e.g., from foo.example.com to bar.foo.example.com but does not permit redirects to bar.example.com.

Steps to Reproduce

1. Set up a Flask application with Flask Security Too enabled.
2. Attempt to login via https://foo.example.com/login?next=https://bar.example.com
3. Observe that the redirect is blocked/not allowed by Flask Security Too.

Additional Information This behaviour can potentially be managed by a configuration setting, which by default retains the current behaviour (for backward compatibility and security) but allows administrators to enable more permissive cross-subdomain redirects if their application scenario deems it safe and necessary.

I do have a patch prepared if this is an acceptable feature request and will open a PR for it. However, there are a few possible paths for accepting subdomains:

• New SECURITY_ config variable: SECURITY_ALLOW_REDIRECT_BASEDOMAIN as a Boolean. Code would remove the first portion of the hostname I.E. foo and allow the remainder of the domain name.
• New SECURITY_ config variable: SECURITY_ALLOWED_REDIRECT_BASEDOMAIN as a String. Would specify which part of the base domain is acceptable for comparison.
• Reuse the SESSION_COOKIE_DOMAIN as an acceptable configuration if SECURITY_ALLOW_REDIRECT_SUBDOMAINS is set. This one is my favourite solution.

Thank you for your consideration of this feature request. Allowing this type of redirect would significantly ease the development of web services that need to interact across different subdomains within the same parent domain.

[jwag956]

Thank you for a wonderful writeup complete with rationale and proposed solutions. Wow.

I think this is great feature to add. It does need to be an option I believe - and by default not enabled. There are cases where subdomains are actually delegated to 3rd parties and that poses a huge security risk. Of course the owner/manager of the domain knows this - but applications often do not - so from a security perspective I think default of not allowing redirects across sub domains is good.

I need to think about solutions a bit more - I don't like option 1 or 2 - too simplistic. DId you consider a configuration variable along the lines of SECURITY_ALLOW_REDIRECT_SUBDOMAINS being a string list of allowed subdomains with '*' meaning any? i.e. a list of regex's for allowable subdomain redirects?

[mchineboy]

I need to think about solutions a bit more - I don't like option 1 or 2 - too simplistic. DId you consider a configuration variable along the lines of SECURITY_ALLOW_REDIRECT_SUBDOMAINS being a string list of allowed subdomains with '*' meaning any? i.e. a list of regex's for allowable subdomain redirects?

I did, but I also didn't want to break already in place deployments by changing the API. I suppose we can make the configuration accept either form, Bool vs List, and react appropriately?

[jwag956]

Ok - I picked a bad name...

Lets leave SECURITY_REDIRECT_ALLOW_SUBDOMAINS as is as a boolean that by default doesn't allow ANY subdomains.

We can add a NEW config variable SECURITY_REDIRECT_SUBDOMAIN_MATCH which allows specifying what sort of subdomains can be redirected too. While a regex is appealing - I think that will take some thought (given that '.' (dots) will be a pain to deal with.

So we can start simpler but still improve functionality by have ...REDIRECT_SUBDOMAIN_MATCH take the following possible values:

"strict" - which would be what currently happens with ALLOW_SUBDOMAINS is True - and we could set that as the default so there are no backwards compat issues "any" - ANY subdomain is allowed (I think that's your use case [explicit_list_of_subdomains, ...]

Pretty easy - and probably will take us down the road many years :-)

Thoughts?

[mchineboy]

I like this proposal and I agree that it covers a couple more use cases as well.

[mchineboy]

So we can start simpler but still improve functionality by have ...REDIRECT_SUBDOMAIN_MATCH take the following possible values:

I've been working on the implementation of this and I think I've found the more optimal solution.

I believe that if we make REDIRECT_MATCH_SUBDOMAINS (note the name change, aesthetically matches its sister config) behave in two modes based on the set values of the configuration:

Whereas:

Strict Implementation

SECURITY_REDIRECT_MATCH_SUBDOMAINS = [ 'bar.example.com' ]

The code would implement the "must match" operators so that a developer/administrator can explicitly specify allowed hostnames only.

Partial Match Implementation

SECURITY_REDIRECT_MATCH_SUBDOMAINS = [ '.example.com' ]

The code would see that the first character of the value is a . and would use the string.endswith() function to make sure the requested hostname matches.

Default

A zero length list would indicate to the code that the user is accepting the previous implementation as is.

Thoughts?