even more careful with login redirect checks

handle empty netloc being interpreted as first path part being the netloc by unhelpful browsers

Author: minrk

notebook/auth/login.py

@@ -7,9 +7,9 @@
 import os
 
 try:
-    from urllib.parse import urlparse # Py 3
+    from urllib.parse import urlparse, urlunparse  # Py 3
 except ImportError:
-    from urlparse import urlparse # Py 2
+    from urlparse import urlparse, urlunparse  # Py 2
 import uuid
 
 from tornado.escape import url_escape
@@ -44,15 +44,18 @@ def _redirect_safe(self, url, default=None):
         # instead of %5C, causing `\\` to behave as `//`
         url = url.replace("\\", "%5C")
         parsed = urlparse(url)
-        if parsed.netloc or not (parsed.path + '/').startswith(self.base_url):
+        path_only = urlunparse(parsed._replace(netloc='', scheme=''))
+        if url != path_only 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 parsed.netloc:
+            if url != path_only:
                 # if full URL, run our cross-origin check:
                 origin = '%s://%s' % (parsed.scheme, parsed.netloc)
                 origin = origin.lower()
-                if self.allow_origin:
+                if origin == '%s://%s' % (self.request.protocol, self.request.host):
+                    allow = True
+                elif self.allow_origin:
                     allow = self.allow_origin == origin
                 elif self.allow_origin_pat:
                     allow = bool(self.allow_origin_pat.match(origin))

notebook/auth/tests/test_login.py

@@ -31,6 +31,8 @@ def test_next_bad(self):
             "//host" + self.url_prefix + "tree",
             "https://google.com",
             "/absolute/not/base_url",
+            "///jupyter.org",
+            "/\\some-host",
         ):
             url = self.login(next=bad_next)
             self.assertEqual(url, self.url_prefix)
@@ -39,10 +41,14 @@ def test_next_bad(self):
     def test_next_ok(self):
         for next_path in (
             "tree/",
-            "//" + self.url_prefix + "tree",
+            self.base_url() + "has/host",
             "notebooks/notebook.ipynb",
             "tree//something",
         ):
-            expected = self.url_prefix + next_path
+            if "://" in next_path:
+                expected = next_path
+            else:
+                expected = self.url_prefix + next_path
+
             actual = self.login(next=expected)
             self.assertEqual(actual, expected)