parse urls when validating redirect targets

simplifies check for redirects to external hosts

Author: minrk

notebook/auth/login.py

@@ -39,14 +39,15 @@ def _redirect_safe(self, url, default=None):
         """
         if default is None:
             default = self.base_url
-        if not url.startswith(self.base_url):
+        parsed = urlparse(url)
+        if parsed.netloc or not (parsed.path + '/').startswith(self.base_url):
             # require that next_url be absolute path within our path
             allow = False
             # OR pass our cross-origin check
-            if '://' in url:
+            if parsed.netloc:
                 # if full URL, run our cross-origin check:
-                parsed = urlparse(url.lower())
                 origin = '%s://%s' % (parsed.scheme, parsed.netloc)
+                origin = origin.lower()
                 if self.allow_origin:
                     allow = self.allow_origin == origin
                 elif self.allow_origin_pat:

notebook/auth/tests/test_login.py

@@ -0,0 +1,48 @@
+"""Tests for login redirects"""
+
+import requests
+from tornado.httputil import url_concat
+
+from notebook.tests.launchnotebook import NotebookTestBase
+
+
+class LoginTest(NotebookTestBase):
+    def login(self, next):
+        first = requests.get(self.base_url() + "login")
+        first.raise_for_status()
+        resp = requests.post(
+            url_concat(
+                self.base_url() + "login",
+                {'next': next},
+            ),
+            allow_redirects=False,
+            data={
+                "password": self.token,
+                "_xsrf": first.cookies.get("_xsrf", ""),
+            },
+            cookies=first.cookies,
+        )
+        resp.raise_for_status()
+        return resp.headers['Location']
+
+    def test_next_bad(self):
+        for bad_next in (
+            "//some-host",
+            "//host" + self.url_prefix + "tree",
+            "https://google.com",
+            "/absolute/not/base_url",
+        ):
+            url = self.login(next=bad_next)
+            self.assertEqual(url, self.url_prefix)
+        assert url
+
+    def test_next_ok(self):
+        for next_path in (
+            "tree/",
+            "//" + self.url_prefix + "tree",
+            "notebooks/notebook.ipynb",
+            "tree//something",
+        ):
+            expected = self.url_prefix + next_path
+            actual = self.login(next=expected)
+            self.assertEqual(actual, expected)