Backport PR #1938: don't check origin on token-authenticated requests

adds LoginHandler.should_check_origin classmethod API

While testing, I noticed that we were checking origin and authentication on CSP violation reports, but browsers are sending CSP reports with no Origin, meaning that no CSP report will ever be accepted. That doesn't make sense to me, so I disabled both - a CSP report can now be made from *anywhere* (no auth, no check).  rgbkrk does that sound right to you? Or should it be authenticated but no origin check?

Signed-off-by: Min RK <[email protected]>

Author: rgbkrk

notebook/auth/login.py

@@ -114,6 +114,18 @@ def get_user_token(cls, handler):
                 user_token = m.group(1)
         return user_token
 
+    @classmethod
+    def should_check_origin(cls, handler):
+        """Should the Handler check for CORS origin validation?
+        
+        Origin check should be skipped for token-authenticated requests.
+        """
+        if getattr(handler, '_user_id', None) is None:
+            # ensure get_user has been called, so we know if we're token-authenticated
+            handler.get_current_user()
+        token_authenticated = getattr(handler, '_token_authenticated', False)
+        return not token_authenticated
+
     @classmethod
     def get_user(cls, handler):
         """Called by handlers.get_current_user for identifying the current user.
@@ -137,17 +149,23 @@ def get_user(cls, handler):
                 # check login token from URL argument or Authorization header
                 user_token = cls.get_user_token(handler)
                 one_time_token = handler.one_time_token
+                authenticated = False
                 if user_token == token:
                     # token-authenticated, set the login cookie
                     handler.log.info("Accepting token-authenticated connection from %s", handler.request.remote_ip)
-                    user_id = uuid.uuid4().hex
-                    cls.set_login_cookie(handler, user_id)
-                if one_time_token and user_token == one_time_token:
-                    # one-time token-authenticated, only allow this token once
+                    authenticated = True
+                elif one_time_token and user_token == one_time_token:
+                    # one-time-token-authenticated, only allow this token once
                     handler.settings.pop('one_time_token', None)
                     handler.log.info("Accepting one-time-token-authenticated connection from %s", handler.request.remote_ip)
+                    authenticated = True
+                if authenticated:
                     user_id = uuid.uuid4().hex
                     cls.set_login_cookie(handler, user_id)
+                    # Record that we've been authenticated with a token.
+                    # Used in should_check_origin above.
+                    handler._token_authenticated = True
+
         # cache value for future retrievals on the same request
         handler._user_id = user_id
         return user_id

notebook/base/handlers.py

@@ -79,6 +79,16 @@ def get_current_user(self):
             return 'anonymous'
         return self.login_handler.get_user(self)
 
+    def skip_check_origin(self):
+        """Ask my login_handler if I should skip the origin_check
+        
+        For example: in the default LoginHandler, if a request is token-authenticated,
+        origin checking should be skipped.
+        """
+        if self.login_handler is None or not hasattr(self.login_handler, 'should_check_origin'):
+            return False
+        return not self.login_handler.should_check_origin(self)
+
     @property
     def cookie_name(self):
         default_cookie_name = non_alphanum.sub('-', 'username-{}'.format(
@@ -264,8 +274,9 @@ def check_origin(self, origin_to_satisfy_tornado=""):
         Copied from WebSocket with changes:
 
         - allow unspecified host/origin (e.g. scripts)
+        - allow token-authenticated requests
         """
-        if self.allow_origin == '*':
+        if self.allow_origin == '*' or self.skip_check_origin():
             return True
 
         host = self.request.headers.get("Host")
@@ -292,8 +303,8 @@ def check_origin(self, origin_to_satisfy_tornado=""):
             # No CORS headers deny the request
             allow = False
         if not allow:
-            self.log.warn("Blocking Cross Origin API request.  Origin: %s, Host: %s",
-                origin, host,
+            self.log.warning("Blocking Cross Origin API request for %s.  Origin: %s, Host: %s",
+                self.request.path, origin, host,
             )
         return allow
 

notebook/base/zmqhandlers.py

@@ -126,7 +126,9 @@ def check_origin(self, origin=None):
         
         Tornado >= 4 calls this method automatically, raising 403 if it returns False.
         """
-        if self.allow_origin == '*':
+
+        if self.allow_origin == '*' or (
+            hasattr(self, 'skip_check_origin') and self.skip_check_origin()):
             return True
 
         host = self.request.headers.get("Host")

notebook/services/security/handlers.py

@@ -3,19 +3,23 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 
-from tornado import gen, web
+from tornado import web
 
 from ...base.handlers import APIHandler, json_errors
 from . import csp_report_uri
 
 class CSPReportHandler(APIHandler):
     '''Accepts a content security policy violation report'''
+
+    def skip_origin_check(self):
+        """Don't check origin when reporting origin-check violations!"""
+        return True
+
     @json_errors
     @web.authenticated
     def post(self):
         '''Log a content security policy violation report'''
-        csp_report = self.get_json_body()
-        self.log.warn("Content security violation: %s",
+        self.log.warning("Content security violation: %s",
                       self.request.body.decode('utf8', 'replace'))
 
 default_handlers = [