Skip to content

Commit 5edfb2b

Browse files
committed
Add open redirect protection to RedirectResponse
RedirectResponse now rejects external URLs by default, raising ValueError for URLs with schemes (http://, ftp://), protocol-relative (//), or backslash variants. Callers must opt in with allow_external=True. All legitimate external redirect sites (OAuth, CDN, HTTPS upgrade, admin Referer, pages/redirection config, redirect_to_login) updated with allow_external=True. Includes tests for bypass vectors (whitespace, backslash normalization, mixed-case schemes).
1 parent b7c430d commit 5edfb2b

File tree

12 files changed

+91
-12
lines changed

12 files changed

+91
-12
lines changed

plain-admin/plain/admin/builtin_views.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def post(self) -> Response:
8484

8585
# Redirect back to current page (or referer)
8686
referer = self.request.headers.get("Referer", "/admin/")
87-
return RedirectResponse(referer)
87+
return RedirectResponse(referer, allow_external=True)
8888

8989

9090
class UnpinNavView(AdminView):
@@ -105,7 +105,7 @@ def post(self) -> Response:
105105

106106
# Redirect back to current page (or referer)
107107
referer = self.request.headers.get("Referer", "/admin/")
108-
return RedirectResponse(referer)
108+
return RedirectResponse(referer, allow_external=True)
109109

110110

111111
class ReorderPinnedView(AdminView):

plain-auth/plain/auth/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,4 @@ def redirect_to_login(
144144
querystring[redirect_field_name] = next
145145
login_url_parts[4] = querystring.urlencode(safe="/")
146146

147-
return RedirectResponse(str(urlunparse(login_url_parts)))
147+
return RedirectResponse(str(urlunparse(login_url_parts)), allow_external=True)

plain-loginlink/plain/loginlink/views.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ class LoginLinkSentView(AuthView, TemplateView):
5050
def get(self) -> Response:
5151
# Redirect if the user is already logged in
5252
if self.user:
53-
next_url = self.request.query_params.get("next", "/")
54-
return RedirectResponse(next_url)
53+
return RedirectResponse(self.request.query_params.get("next", "/"))
5554

5655
return super().get()
5756

plain-oauth/plain/oauth/providers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ def get_redirect_response(self, redirect_url: str) -> Response:
213213
Returns a redirect response to the given URL.
214214
This is a utility method to ensure consistent redirect handling.
215215
"""
216-
response = RedirectResponse(redirect_url)
216+
response = RedirectResponse(redirect_url, allow_external=True)
217217
add_never_cache_headers(response)
218218
return response
219219

plain-pages/plain/pages/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ def get(self) -> RedirectResponse:
106106
raise RedirectPageError("Redirect page is missing a url")
107107

108108
status_code = self.page.vars.get("status_code", 302)
109-
return RedirectResponse(url, status_code=status_code)
109+
return RedirectResponse(url, status_code=status_code, allow_external=True)
110110

111111

112112
class PageAssetView(PageViewMixin, AssetView):

plain-redirection/plain/redirection/middleware.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ def after_response(self, request: Request, response: Response) -> Response:
1717
redirect_log = RedirectLog.from_redirect(redirect, request)
1818
# Then redirect
1919
return RedirectResponse(
20-
str(redirect_log.to_url), status_code=redirect.http_status
20+
str(redirect_log.to_url),
21+
status_code=redirect.http_status,
22+
allow_external=True,
2123
)
2224

2325
# Nothing matched, just log the 404

plain-support/plain/support/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,4 @@ def get_response(self) -> ResponseBase:
6161

6262
class SupportFormJSView(View):
6363
def get(self) -> RedirectResponse:
64-
return RedirectResponse(get_asset_url("support/embed.js"))
64+
return RedirectResponse(get_asset_url("support/embed.js"), allow_external=True)

plain/plain/assets/views.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ def get_cdn_redirect_response(self, path: str) -> RedirectResponse | None:
270270
return RedirectResponse(
271271
redirect_to=cdn_url,
272272
status_code=status_code,
273+
allow_external=True,
273274
headers={"Cache-Control": cache_control},
274275
)
275276

plain/plain/http/response.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,12 +540,37 @@ def set_headers(self, filelike: IO[bytes]) -> None:
540540
self.headers["Content-Disposition"] = content_disposition
541541

542542

543+
def _is_external_url(url: str) -> bool:
544+
"""Check if a URL would redirect to an external host."""
545+
if not url:
546+
return False
547+
# Browsers strip leading whitespace from Location headers
548+
url = url.strip()
549+
# Browsers normalize backslashes to forward slashes in URLs,
550+
# so \\ and /\ are equivalent to //
551+
if url[:2].replace("\\", "/") == "//":
552+
return True
553+
colon_pos = url.find("://")
554+
if colon_pos > 0 and url[:colon_pos].isalpha():
555+
return True
556+
return False
557+
558+
543559
class RedirectResponse(Response):
544560
"""HTTP redirect response"""
545561

546562
status_code = 302
547563

548-
def __init__(self, redirect_to: str, **kwargs: Any):
564+
def __init__(
565+
self, redirect_to: str, *, allow_external: bool = False, **kwargs: Any
566+
):
567+
if not allow_external and _is_external_url(redirect_to):
568+
raise ValueError(
569+
f"Unsafe redirect URL: {redirect_to!r}. "
570+
"RedirectResponse does not allow external URLs by default. "
571+
"Use allow_external=True if you intentionally want to redirect "
572+
"to an external URL."
573+
)
549574
super().__init__(**kwargs)
550575
self.headers["Location"] = iri_to_uri(redirect_to) or ""
551576

plain/plain/internal/middleware/https.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def before_request(self, request: Request) -> Response | None:
2727
def maybe_https_redirect(self, request: Request) -> Response | None:
2828
if self.https_redirect_enabled and not request.is_https():
2929
return RedirectResponse(
30-
f"https://{request.host}{request.get_full_path()}", status_code=301
30+
f"https://{request.host}{request.get_full_path()}",
31+
status_code=301,
32+
allow_external=True,
3133
)
3234
return None

0 commit comments

Comments
 (0)