Fix API v2 exception on incorrect API key. Fixes #5425 (#6703)

* Fix API v2 exception on incorrect API key.

Fixes #5425

* Update changelog

* Fix authentication endpoint

* Test bad auth conditions

* Fix RESTful and non-RESTful error handling

* Fix API pagination

This reverts commit b291939.

* Fix Tornado request logging

- Fix requests getting logged twice.
- Fix API v2 HTTPErrors getting logged as warnings.

* Fix `write_error` for `raise HTTPError(code, message)`

CHANGELOG.md

@@ -18,6 +18,7 @@
 - Fixed "send to trash" option not doing anything (Python 3.6 and higher) ([#6625](https://github.com/pymedusa/Medusa/pull/6625))
 - Fixed setting episodes to archived in backlog overview ([#6636](https://github.com/pymedusa/Medusa/pull/6636))
 - Fixed exception in Elte-Tracker provider when no result is found ([#6680](https://github.com/pymedusa/Medusa/pull/6680))
+- Fixed exception in API v2 when an incorrect API key was provided, or none was provided ([#6703](https://github.com/pymedusa/Medusa/pull/6703))
 
 ## 0.3.1 (2019-03-20)
 

medusa/server/api/v2/auth.py

@@ -30,9 +30,9 @@ class AuthHandler(BaseRequestHandler):
     #: allowed HTTP methods
     allowed_methods = ('POST', )
 
-    def prepare(self):
-        """Prepare."""
-        pass
+    def _check_authentication(self):
+        """Override authentication check for the authentication endpoint."""
+        return None
 
     def http_post(self, *args, **kwargs):
         """Request JWT."""

medusa/server/api/v2/base.py

@@ -25,10 +25,9 @@
 from six import iteritems, string_types, text_type, viewitems
 
 from tornado.gen import coroutine
-from tornado.httpclient import HTTPError
 from tornado.httputil import url_concat
 from tornado.ioloop import IOLoop
-from tornado.web import RequestHandler
+from tornado.web import HTTPError, RequestHandler
 
 log = BraceAdapter(logging.getLogger(__name__))
 log.logger.addHandler(logging.NullHandler())
@@ -56,11 +55,8 @@ class BaseRequestHandler(RequestHandler):
     #: parent resource handler
     parent_handler = None
 
-    def prepare(self):
+    def _check_authentication(self):
         """Check if JWT or API key is provided and valid."""
-        if self.request.method == 'OPTIONS':
-            return
-
         api_key = self.get_argument('api_key', default=None) or self.request.headers.get('X-Api-Key')
         if api_key and api_key == app.API_KEY:
             return
@@ -90,13 +86,11 @@ def async_call(self, name, *args, **kwargs):
         try:
             method = getattr(self, 'http_' + name)
         except AttributeError:
-            raise HTTPError(405, '{name} method is not allowed'.format(name=name.upper()))
+            raise HTTPError(405)
 
         def blocking_call():
-            try:
-                return method(*args, **kwargs)
-            except Exception as error:
-                self._handle_request_exception(error)
+            result = self._check_authentication()
+            return method(*args, **kwargs) if result is None else result
 
         return IOLoop.current().run_in_executor(executor, blocking_call)
 
@@ -154,17 +148,41 @@ def put(self, *args, **kwargs):
         if not self._finished:
             self.finish(content)
 
-    def write_error(self, *args, **kwargs):
+    def write_error(self, status_code, *args, **kwargs):
         """Only send traceback if app.DEVELOPER is true."""
-        if app.DEVELOPER and 'exc_info' in kwargs:
+        response = None
+        exc_info = kwargs.get('exc_info', None)
+
+        if exc_info and isinstance(exc_info[1], HTTPError):
+            error = exc_info[1].log_message or exc_info[1].reason
+            response = self.api_response(status=status_code, error=error)
+        elif app.DEVELOPER and exc_info:
             self.set_header('content-type', 'text/plain')
             self.set_status(500)
-            for line in traceback.format_exception(*kwargs['exc_info']):
+            for line in traceback.format_exception(*exc_info):
                 self.write(line)
-            self.finish()
         else:
             response = self._internal_server_error()
-            self.finish(response)
+
+        self.finish(response)
+
+    def log_exception(self, typ, value, tb):
+        """
+        Customize logging of uncaught exceptions.
+
+        Only logs unhandled exceptions, as ``HTTPErrors`` are common for a RESTful API handler.
+        """
+        if not app.WEB_LOG:
+            return
+
+        if isinstance(value, HTTPError):
+            return
+
+        log.error('Uncaught exception: {summary}\n{req!r}', {
+            'summary': self._request_summary(),
+            'req': self.request,
+            'exc_info': (typ, value, tb),
+        })
 
     def options(self, *args, **kwargs):
         """OPTIONS HTTP method."""
@@ -248,13 +266,6 @@ def create_app_handler(cls, base):
 
         return cls.create_url(base, cls.name, *(cls.identifier, cls.path_param)), cls
 
-    def _handle_request_exception(self, error):
-        if isinstance(error, HTTPError):
-            response = self.api_response(error.code, error.message)
-            self.finish(response)
-        else:
-            super(BaseRequestHandler, self)._handle_request_exception(error)
-
     def _ok(self, data=None, headers=None, stream=None, content_type=None):
         return self.api_response(200, data=data, headers=headers, stream=stream, content_type=content_type)
 
@@ -335,11 +346,8 @@ def _get_limit(self, default=20, maximum=1000):
             self._raise_bad_request_error('Invalid limit parameter')
 
     def _paginate(self, data=None, data_generator=None, sort=None):
-        try:
-            arg_page = self._get_page()
-            arg_limit = self._get_limit()
-        except HTTPError as error:
-            return self._bad_request(error.message)
+        arg_page = self._get_page()
+        arg_limit = self._get_limit()
 
         headers = {
             'X-Pagination-Page': arg_page,

tests/apiv2/test_auth.py

@@ -0,0 +1,37 @@
+# coding=utf-8
+"""Test /authentication route and authentication checks."""
+from __future__ import unicode_literals
+
+import json
+
+import pytest
+
+
+@pytest.mark.gen_test
+def test_no_api_key(http_client, create_url):
+    # given
+    expected = {'error': 'No authorization token.'}
+
+    url = create_url('/log', api_key=None)
+
+    # when
+    response = yield http_client.fetch(url, raise_error=False)
+
+    # then
+    assert response.code == 401
+    assert expected == json.loads(response.body)
+
+
+@pytest.mark.gen_test
+def test_bad_api_key(http_client, create_url):
+    # given
+    expected = {'error': 'No authorization token.'}
+
+    url = create_url('/log', api_key='123')
+
+    # when
+    response = yield http_client.fetch(url, raise_error=False)
+
+    # then
+    assert response.code == 401
+    assert expected == json.loads(response.body)