fix: only redirect to same domain (#26304)
This limits post login redirects to same domain to avoid social engineering attempts.
This commit is contained in:
parent
4bfcccfd11
commit
65b3c42635
2 changed files with 39 additions and 1 deletions
|
|
@ -7,7 +7,7 @@ from random import choice
|
|||
from threading import Thread
|
||||
from time import time
|
||||
from unittest.mock import patch
|
||||
from urllib.parse import urljoin
|
||||
from urllib.parse import urlencode, urljoin
|
||||
|
||||
import requests
|
||||
from filetype import guess_mime
|
||||
|
|
@ -454,6 +454,19 @@ class TestResponse(FrappeAPITestCase):
|
|||
self.assertEqual(self.get(file.unique_url, {"sid": self.sid}).text, test_content)
|
||||
self.assertEqual(self.get(file.file_url, {"sid": self.sid}).text, test_content)
|
||||
|
||||
def test_login_redirects(self):
|
||||
expected_redirects = {
|
||||
"/app/user": "/app/user",
|
||||
"/app/user?enabled=1": "/app/user?enabled=1",
|
||||
"http://example.com": "/app", # No external redirect
|
||||
"https://google.com": "/app",
|
||||
"http://localhost:8000": "/app",
|
||||
"http://localhost/app": "http://localhost/app",
|
||||
}
|
||||
for redirect, expected_redirect in expected_redirects.items():
|
||||
response = self.get(f"/login?{urlencode({'redirect-to':redirect})}", {"sid": self.sid})
|
||||
self.assertEqual(response.location, expected_redirect)
|
||||
|
||||
|
||||
def generate_admin_keys():
|
||||
from frappe.core.doctype.user.user import generate_keys
|
||||
|
|
|
|||
|
|
@ -1,6 +1,9 @@
|
|||
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# License: MIT. See LICENSE
|
||||
|
||||
|
||||
from urllib.parse import urlparse
|
||||
|
||||
import frappe
|
||||
import frappe.utils
|
||||
from frappe import _
|
||||
|
|
@ -19,6 +22,7 @@ no_cache = True
|
|||
|
||||
def get_context(context):
|
||||
redirect_to = frappe.local.request.args.get("redirect-to")
|
||||
redirect_to = sanitize_redirect(redirect_to)
|
||||
|
||||
if frappe.session.user != "Guest":
|
||||
if not redirect_to:
|
||||
|
|
@ -179,3 +183,24 @@ def login_via_key(key: str):
|
|||
http_status_code=403,
|
||||
indicator_color="red",
|
||||
)
|
||||
|
||||
|
||||
def sanitize_redirect(redirect: str | None) -> str | None:
|
||||
"""Only allow redirect on same domain.
|
||||
|
||||
Allowed redirects:
|
||||
- Same host e.g. https://frappe.localhost/path
|
||||
- Just path e.g. /app
|
||||
"""
|
||||
if not redirect:
|
||||
return redirect
|
||||
|
||||
parsed_redirect = urlparse(redirect)
|
||||
if not parsed_redirect.netloc:
|
||||
return redirect
|
||||
|
||||
parsed_request_host = urlparse(frappe.local.request.url)
|
||||
if parsed_request_host.netloc == parsed_redirect.netloc:
|
||||
return redirect
|
||||
|
||||
return None
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue