Merge pull request #2853 from minrk/api-403

Update error handling on APIHandlers

notebook/base/handlers.py

@@ -10,6 +10,8 @@ import os
 import re
 import sys
 import traceback
+import types
+import warnings
 try:
     # py3
     from http.client import responses
@@ -450,6 +452,34 @@ class APIHandler(IPythonHandler):
             raise web.HTTPError(404)
         return super(APIHandler, self).prepare()
 
+    def write_error(self, status_code, **kwargs):
+        """APIHandler errors are JSON, not human pages"""
+        self.set_header('Content-Type', 'application/json')
+        message = responses.get(status_code, 'Unknown HTTP Error')
+        reply = {
+            'message': message,
+        }
+        exc_info = kwargs.get('exc_info')
+        if exc_info:
+            e = exc_info[1]
+            if isinstance(e, HTTPError):
+                reply['message'] = e.log_message or message
+            else:
+                reply['message'] = 'Unhandled error'
+                reply['traceback'] = ''.join(traceback.format_exception(*exc_info))
+        self.log.warning(reply['message'])
+        self.finish(json.dumps(reply))
+
+    def get_current_user(self):
+        """Raise 403 on API handlers instead of redirecting to human login page"""
+        # preserve _user_cache so we don't raise more than once
+        if hasattr(self, '_user_cache'):
+            return self._user_cache
+        self._user_cache = user = super(APIHandler, self).get_current_user()
+        if user is None:
+            raise web.HTTPError(403)
+        return user
+
     @property
     def content_security_policy(self):
         csp = '; '.join([
@@ -547,32 +577,14 @@ def json_errors(method):
     2. Create and return a JSON body with a message field describing
        the error in a human readable form.
     """
+    warnings.warn('@json_errors is deprecated in notebook 5.2.0. Subclass APIHandler instead.',
+        DeprecationWarning,
+        stacklevel=2,
+    )
     @functools.wraps(method)
-    @gen.coroutine
     def wrapper(self, *args, **kwargs):
-        try:
-            result = yield gen.maybe_future(method(self, *args, **kwargs))
-        except web.HTTPError as e:
-            self.set_header('Content-Type', 'application/json')
-            status = e.status_code
-            message = e.log_message
-            self.log.warning(message)
-            self.set_status(e.status_code)
-            reply = dict(message=message, reason=e.reason)
-            self.finish(json.dumps(reply))
-        except Exception:
-            self.set_header('Content-Type', 'application/json')
-            self.log.error("Unhandled error in API request", exc_info=True)
-            status = 500
-            message = "Unknown server error"
-            t, value, tb = sys.exc_info()
-            self.set_status(status)
-            tb_text = ''.join(traceback.format_exception(t, value, tb))
-            reply = dict(message=message, reason=None, traceback=tb_text)
-            self.finish(json.dumps(reply))
-        else:
-            # FIXME: can use regular return in generators in py3
-            raise gen.Return(result)
+        self.write_error = types.MethodType(APIHandler.write_error, self)
+        return method(self, *args, **kwargs)
     return wrapper
 
 
@@ -643,7 +655,6 @@ class FileFindHandler(IPythonHandler, web.StaticFileHandler):
 
 class APIVersionHandler(APIHandler):
 
-    @json_errors
     def get(self):
         # not authenticated, so give as few info as possible
         self.finish(json.dumps({"version":notebook.__version__}))

notebook/services/api/handlers.py

@@ -8,7 +8,7 @@ import json
 
 from tornado import gen, web
 
-from ...base.handlers import IPythonHandler, APIHandler, json_errors
+from ...base.handlers import IPythonHandler, APIHandler
 from notebook._tz import utcfromtimestamp, isoformat
 
 import os
@@ -28,7 +28,6 @@ class APIStatusHandler(APIHandler):
 
     _track_activity = False
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self):

notebook/services/config/handlers.py

@@ -9,24 +9,21 @@ import errno
 from tornado import web
 
 from ipython_genutils.py3compat import PY3
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 
 class ConfigHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     def get(self, section_name):
         self.set_header("Content-Type", 'application/json')
         self.finish(json.dumps(self.config_manager.get(section_name)))
 
-    @json_errors
     @web.authenticated
     def put(self, section_name):
         data = self.get_json_body()  # Will raise 400 if content is not valid JSON
         self.config_manager.set(section_name, data)
         self.set_status(204)
 
-    @json_errors
     @web.authenticated
     def patch(self, section_name):
         new_data = self.get_json_body()

notebook/services/contents/handlers.py

@@ -14,7 +14,7 @@ from notebook.utils import url_path_join, url_escape
 from jupyter_client.jsonutil import date_default
 
 from notebook.base.handlers import (
-    IPythonHandler, APIHandler, json_errors, path_regex,
+    IPythonHandler, APIHandler, path_regex,
 )
 
 
@@ -87,7 +87,6 @@ class ContentsHandler(APIHandler):
         self.set_header('Content-Type', 'application/json')
         self.finish(json.dumps(model, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self, path=''):
@@ -115,7 +114,6 @@ class ContentsHandler(APIHandler):
         validate_model(model, expect_content=content)
         self._finish_model(model, location=False)
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def patch(self, path=''):
@@ -168,7 +166,6 @@ class ContentsHandler(APIHandler):
         validate_model(model, expect_content=False)
         self._finish_model(model)
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self, path=''):
@@ -204,7 +201,6 @@ class ContentsHandler(APIHandler):
         else:
             yield self._new_untitled(path)
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def put(self, path=''):
@@ -230,7 +226,6 @@ class ContentsHandler(APIHandler):
         else:
             yield gen.maybe_future(self._new_untitled(path))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def delete(self, path=''):
@@ -244,7 +239,6 @@ class ContentsHandler(APIHandler):
 
 class CheckpointsHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self, path=''):
@@ -254,7 +248,6 @@ class CheckpointsHandler(APIHandler):
         data = json.dumps(checkpoints, default=date_default)
         self.finish(data)
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self, path=''):
@@ -271,7 +264,6 @@ class CheckpointsHandler(APIHandler):
 
 class ModifyCheckpointsHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self, path, checkpoint_id):
@@ -281,7 +273,6 @@ class ModifyCheckpointsHandler(APIHandler):
         self.set_status(204)
         self.finish()
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def delete(self, path, checkpoint_id):
@@ -310,7 +301,6 @@ class NotebooksRedirectHandler(IPythonHandler):
 class TrustNotebooksHandler(IPythonHandler):
     """ Handles trust/signing of notebooks """
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self,path=''):

notebook/services/kernels/handlers.py

@@ -18,14 +18,13 @@ from jupyter_client.jsonutil import date_default
 from ipython_genutils.py3compat import cast_unicode
 from notebook.utils import url_path_join, url_escape
 
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 from ...base.zmqhandlers import AuthenticatedZMQStreamHandler, deserialize_binary_message
 
 from jupyter_client import protocol_version as client_protocol_version
 
 class MainKernelHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self):
@@ -33,7 +32,6 @@ class MainKernelHandler(APIHandler):
         kernels = yield gen.maybe_future(km.list_kernels())
         self.finish(json.dumps(kernels, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self):
@@ -56,7 +54,6 @@ class MainKernelHandler(APIHandler):
 
 class KernelHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     def get(self, kernel_id):
         km = self.kernel_manager
@@ -64,7 +61,6 @@ class KernelHandler(APIHandler):
         model = km.kernel_model(kernel_id)
         self.finish(json.dumps(model, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def delete(self, kernel_id):
@@ -76,7 +72,6 @@ class KernelHandler(APIHandler):
 
 class KernelActionHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self, kernel_id, action):

notebook/services/kernelspecs/handlers.py

@@ -13,7 +13,7 @@ pjoin = os.path.join
 
 from tornado import web
 
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 from ...utils import url_path_join, url_unescape
 
 def kernelspec_model(handler, name):
@@ -45,7 +45,6 @@ def kernelspec_model(handler, name):
 
 class MainKernelSpecHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     def get(self):
         ksm = self.kernel_spec_manager
@@ -66,7 +65,6 @@ class MainKernelSpecHandler(APIHandler):
 
 class KernelSpecHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     def get(self, kernel_name):
         try:

notebook/services/nbconvert/handlers.py

@@ -2,11 +2,10 @@ import json
 
 from tornado import web
 
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 
 class NbconvertRootHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     def get(self):
         try:

notebook/services/security/handlers.py

@@ -5,7 +5,7 @@
 
 from tornado import web
 
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 from . import csp_report_uri
 
 class CSPReportHandler(APIHandler):
@@ -21,7 +21,6 @@ class CSPReportHandler(APIHandler):
         # don't check XSRF for CSP reports
         return
 
-    @json_errors
     @web.authenticated
     def post(self):
         '''Log a content security policy violation report'''

notebook/services/sessions/handlers.py

@@ -11,7 +11,7 @@ import os
 
 from tornado import gen, web
 
-from ...base.handlers import APIHandler, json_errors
+from ...base.handlers import APIHandler
 from jupyter_client.jsonutil import date_default
 from notebook.utils import url_path_join
 from jupyter_client.kernelspec import NoSuchKernel
@@ -19,7 +19,6 @@ from jupyter_client.kernelspec import NoSuchKernel
 
 class SessionRootHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self):
@@ -28,7 +27,6 @@ class SessionRootHandler(APIHandler):
         sessions = yield gen.maybe_future(sm.list_sessions())
         self.finish(json.dumps(sessions, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def post(self):
@@ -90,7 +88,6 @@ class SessionRootHandler(APIHandler):
 
 class SessionHandler(APIHandler):
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def get(self, session_id):
@@ -99,7 +96,6 @@ class SessionHandler(APIHandler):
         model = yield gen.maybe_future(sm.get_session(session_id=session_id))
         self.finish(json.dumps(model, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def patch(self, session_id):
@@ -153,7 +149,6 @@ class SessionHandler(APIHandler):
             )
         self.finish(json.dumps(model, default=date_default))
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def delete(self, session_id):

notebook/terminal/api_handlers.py

@@ -1,17 +1,15 @@
 import json
 from tornado import web, gen
-from ..base.handlers import APIHandler, json_errors
+from ..base.handlers import APIHandler
 from ..utils import url_path_join
 
 class TerminalRootHandler(APIHandler):
-    @json_errors
     @web.authenticated
     def get(self):
         tm = self.terminal_manager
         terms = [{'name': name} for name in tm.terminals]
         self.finish(json.dumps(terms))
 
-    @json_errors
     @web.authenticated
     def post(self):
         """POST /terminals creates a new terminal and redirects to it"""
@@ -22,7 +20,6 @@ class TerminalRootHandler(APIHandler):
 class TerminalHandler(APIHandler):
     SUPPORTED_METHODS = ('GET', 'DELETE')
 
-    @json_errors
     @web.authenticated
     def get(self, name):
         tm = self.terminal_manager
@@ -31,7 +28,6 @@ class TerminalHandler(APIHandler):
         else:
             raise web.HTTPError(404, "Terminal not found: %r" % name)
 
-    @json_errors
     @web.authenticated
     @gen.coroutine
     def delete(self, name):