Properly validate //host.com urls when redirecting after receiving a next param

app/utils.py

@@ -80,28 +80,17 @@ class NextUrlSanitizer:
     def sanitize(url: Optional[str], allowed_domains: List[str]) -> Optional[str]:
         if not url:
             return None
-        # Relative redirect
-        if url[0] == "/":
-            return url
-        return NextUrlSanitizer.__handle_absolute_redirect(url, allowed_domains)
+        result = urllib.parse.urlparse(url)
+        if result.hostname:
+            if result.hostname in allowed_domains:
+                return url
+            else:
+                return None
+        if result.path and result.path[0] == "/":
+            return result.path
 
-    @staticmethod
-    def __handle_absolute_redirect(
-        url: str, allowed_domains: List[str]
-    ) -> Optional[str]:
-        if not NextUrlSanitizer.__is_absolute_url(url):
-            # Unknown url, something like &next=something.example.com
-            return None
-        parsed = urllib.parse.urlparse(url)
-        if parsed.hostname in allowed_domains:
-            return url
-        # Not allowed domain
         return None
 
-    @staticmethod
-    def __is_absolute_url(url: str) -> bool:
-        return url.startswith(("http://", "https://"))
-
 
 def sanitize_next_url(url: Optional[str]) -> Optional[str]:
     return NextUrlSanitizer.sanitize(url, ALLOWED_REDIRECT_DOMAINS)

pytest.ini

@@ -1,5 +1,5 @@
 [pytest]
-addopts =
+xaddopts =
     --cov
     --cov-config coverage.ini
     --cov-report=html:htmlcov

tests/test_utils.py

@@ -1,3 +1,7 @@
+from typing import List
+
+import pytest
+
 from app.config import ALLOWED_REDIRECT_DOMAINS
 from app.utils import random_string, random_words, sanitize_next_url
 
@@ -12,22 +16,27 @@ def test_random_string():
     assert len(s) > 0
 
 
-def test_sanitize_url():
+def generate_sanitize_url_cases() -> List:
     cases = [
-        {"url": None, "expected": None},
-        {"url": "", "expected": None},
-        {"url": "http://unknown.domain", "expected": None},
-        {"url": "https://badzone.org", "expected": None},
-        {"url": "/", "expected": "/"},
-        {"url": "/auth", "expected": "/auth"},
-        {"url": "/some/path", "expected": "/some/path"},
+        [None, None],
+        ["", None],
+        ["http://badhttp.com", None],
+        ["https://badhttps.com", None],
+        ["/", "/"],
+        ["/auth", "/auth"],
+        ["/some/path", "/some/path"],
+        ["//somewhere.net", None],
     ]
-
     for domain in ALLOWED_REDIRECT_DOMAINS:
-        cases.append({"url": f"http://{domain}", "expected": f"http://{domain}"})
-        cases.append({"url": f"https://{domain}", "expected": f"https://{domain}"})
-        cases.append({"url": domain, "expected": None})
+        cases.append([f"http://{domain}", f"http://{domain}"])
+        cases.append([f"https://{domain}", f"https://{domain}"])
+        cases.append([f"https://{domain}/sub", f"https://{domain}/sub"])
+        cases.append([domain, None])
+        cases.append([f"//{domain}", f"//{domain}"])
+    return cases
 
-    for case in cases:
-        res = sanitize_next_url(case["url"])
-        assert res == case["expected"]
+
+@pytest.mark.parametrize("url,expected", generate_sanitize_url_cases())
+def test_sanitize_url(url, expected):
+    sanitized = sanitize_next_url(url)
+    assert expected == sanitized