Merge pull request #271 from YunoHost/improve-error-semantic

Improve error semantic

moulinette/actionsmap.py

@@ -14,7 +14,12 @@ from importlib import import_module
 from moulinette import m18n, msignals
 from moulinette.cache import open_cachefile
 from moulinette.globals import init_moulinette_env
-from moulinette.core import MoulinetteError, MoulinetteLock
+from moulinette.core import (
+    MoulinetteError,
+    MoulinetteLock,
+    MoulinetteAuthenticationError,
+    MoulinetteValidationError,
+)
 from moulinette.interfaces import BaseActionsMapParser, GLOBAL_SECTION, TO_RETURN_PROP
 from moulinette.utils.log import start_action_logging
 
@@ -207,7 +212,9 @@ class PatternParameter(_ExtraParameter):
             if msg == message:
                 msg = m18n.g(message)
 
-            raise MoulinetteError("invalid_argument", argument=arg_name, error=msg)
+            raise MoulinetteValidationError(
+                "invalid_argument", argument=arg_name, error=msg
+            )
         return arg_value
 
     @staticmethod
@@ -238,7 +245,7 @@ class RequiredParameter(_ExtraParameter):
     def __call__(self, required, arg_name, arg_value):
         if required and (arg_value is None or arg_value == ""):
             logger.warning("argument '%s' is required", arg_name)
-            raise MoulinetteError("argument_required", argument=arg_name)
+            raise MoulinetteValidationError("argument_required", argument=arg_name)
         return arg_value
 
     @staticmethod
@@ -497,7 +504,7 @@ class ActionsMap(object):
         auth = msignals.authenticate(authenticator)
 
         if not auth.is_authenticated:
-            raise MoulinetteError("authentication_required_long")
+            raise MoulinetteAuthenticationError("authentication_required_long")
 
     def process(self, args, timeout=None, **kwargs):
         """

moulinette/authenticators/__init__.py

@@ -6,7 +6,7 @@ import hashlib
 import hmac
 
 from moulinette.cache import open_cachefile, get_cachedir, cachefile_exists
-from moulinette.core import MoulinetteError
+from moulinette.core import MoulinetteError, MoulinetteAuthenticationError
 
 logger = logging.getLogger("moulinette.authenticator")
 
@@ -105,7 +105,7 @@ class BaseAuthenticator(object):
                     self.vendor,
                     e,
                 )
-                raise MoulinetteError("unable_authenticate")
+                raise MoulinetteAuthenticationError("unable_authenticate")
 
             self.is_authenticated = True
 
@@ -139,7 +139,7 @@ class BaseAuthenticator(object):
                     self.vendor,
                     e,
                 )
-                raise MoulinetteError("unable_authenticate")
+                raise MoulinetteAuthenticationError("unable_authenticate")
             else:
                 self.is_authenticated = True
 
@@ -147,7 +147,7 @@ class BaseAuthenticator(object):
         # No credentials given, can't authenticate
         #
         else:
-            raise MoulinetteError("unable_authenticate")
+            raise MoulinetteAuthenticationError("unable_authenticate")
 
         return self
 
@@ -175,7 +175,7 @@ class BaseAuthenticator(object):
     def _authenticate_session(self, session_id, session_token):
         """Checks session and token against the stored session token"""
         if not self._session_exists(session_id):
-            raise MoulinetteError("session_expired")
+            raise MoulinetteAuthenticationError("session_expired")
         try:
             # FIXME : shouldn't we also add a check that this session file
             # is not too old ? e.g. not older than 24 hours ? idk...
@@ -184,7 +184,7 @@ class BaseAuthenticator(object):
                 stored_hash = f.read()
         except IOError as e:
             logger.debug("unable to retrieve session", exc_info=1)
-            raise MoulinetteError("unable_retrieve_session", exception=e)
+            raise MoulinetteAuthenticationError("unable_retrieve_session", exception=e)
         else:
             #
             # session_id (or just id) : This is unique id for the current session from the user. Not too important
@@ -206,7 +206,7 @@ class BaseAuthenticator(object):
             hash_ = hashlib.sha256(to_hash).hexdigest()
 
             if not hmac.compare_digest(hash_, stored_hash):
-                raise MoulinetteError("invalid_token")
+                raise MoulinetteAuthenticationError("invalid_token")
             else:
                 return
 

moulinette/authenticators/ldap.py

@@ -10,7 +10,11 @@ import time
 import ldap.modlist as modlist
 
 from moulinette import m18n
-from moulinette.core import MoulinetteError, MoulinetteLdapIsDownError
+from moulinette.core import (
+    MoulinetteError,
+    MoulinetteAuthenticationError,
+    MoulinetteLdapIsDownError,
+)
 from moulinette.authenticators import BaseAuthenticator
 
 logger = logging.getLogger("moulinette.authenticator.ldap")
@@ -86,7 +90,7 @@ class Authenticator(BaseAuthenticator):
         try:
             con = _reconnect()
         except ldap.INVALID_CREDENTIALS:
-            raise MoulinetteError("invalid_password")
+            raise MoulinetteAuthenticationError("invalid_password")
         except ldap.SERVER_DOWN:
             # ldap is down, attempt to restart it before really failing
             logger.warning(m18n.g("ldap_server_is_down_restart_it"))

moulinette/core.py

@@ -382,6 +382,8 @@ class MoulinetteSignals(object):
 
 class MoulinetteError(Exception):
 
+    http_code = 500
+
     """Moulinette base exception"""
 
     def __init__(self, key, raw_msg=False, *args, **kwargs):
@@ -396,6 +398,16 @@ class MoulinetteError(Exception):
         return self.strerror
 
 
+class MoulinetteValidationError(MoulinetteError):
+
+    http_code = 400
+
+
+class MoulinetteAuthenticationError(MoulinetteError):
+
+    http_code = 401
+
+
 class MoulinetteLdapIsDownError(MoulinetteError):
     """Used when ldap is down"""
 

moulinette/interfaces/api.py

@@ -15,7 +15,7 @@ from bottle import abort
 
 from moulinette import msignals, m18n, env
 from moulinette.actionsmap import ActionsMap
-from moulinette.core import MoulinetteError
+from moulinette.core import MoulinetteError, MoulinetteValidationError
 from moulinette.interfaces import (
     BaseActionsMapParser,
     BaseInterface,
@@ -207,8 +207,7 @@ class _HTTPArgumentParser(object):
         return self._parser.dequeue_callbacks(*args, **kwargs)
 
     def _error(self, message):
-        # TODO: Raise a proper exception
+        raise MoulinetteValidationError(message, raw_msg=True)
-        raise MoulinetteError(message, raw_msg=True)
 
 
 class _ActionsMapPlugin(object):
@@ -252,7 +251,7 @@ class _ActionsMapPlugin(object):
                 try:
                     kwargs["password"] = request.POST["password"]
                 except KeyError:
-                    raise HTTPBadRequestResponse("Missing password parameter")
+                    raise HTTPResponse("Missing password parameter", 400)
 
                 kwargs["profile"] = request.POST.get("profile", "default")
                 return callback(**kwargs)
@@ -387,7 +386,7 @@ class _ActionsMapPlugin(object):
                     self.logout(profile)
                 except:
                     pass
-            raise HTTPUnauthorizedResponse(e.strerror)
+            raise HTTPResponse(e.strerror, 401)
         else:
             # Update dicts with new values
             s_tokens[profile] = s_new_token
@@ -420,7 +419,7 @@ class _ActionsMapPlugin(object):
         if profile not in request.get_cookie(
             "session.tokens", secret=s_secret, default={}
         ):
-            raise HTTPUnauthorizedResponse(m18n.g("not_logged_in"))
+            raise HTTPResponse(m18n.g("not_logged_in"), 401)
         else:
             del self.secrets[s_id]
             authenticator = self.actionsmap.get_authenticator_for_profile(profile)
@@ -448,7 +447,7 @@ class _ActionsMapPlugin(object):
 
         wsock = request.environ.get("wsgi.websocket")
         if not wsock:
-            raise HTTPErrorResponse(m18n.g("websocket_request_expected"))
+            raise HTTPResponse(m18n.g("websocket_request_expected"), 500)
 
         while True:
             item = queue.get()
@@ -484,7 +483,7 @@ class _ActionsMapPlugin(object):
         try:
             ret = self.actionsmap.process(arguments, timeout=30, route=_route)
         except MoulinetteError as e:
-            raise HTTPBadRequestResponse(e)
+            raise moulinette_error_to_http_response(e)
         except Exception as e:
             if isinstance(e, HTTPResponse):
                 raise e
@@ -492,7 +491,7 @@ class _ActionsMapPlugin(object):
 
             tb = traceback.format_exc()
             logs = {"route": _route, "arguments": arguments, "traceback": tb}
-            return HTTPErrorResponse(json_encode(logs))
+            return HTTPResponse(json_encode(logs), 500)
         else:
             return format_for_response(ret)
         finally:
@@ -520,7 +519,7 @@ class _ActionsMapPlugin(object):
             ]
         except KeyError:
             msg = m18n.g("authentication_required")
-            raise HTTPUnauthorizedResponse(msg)
+            raise HTTPResponse(msg, 401)
         else:
             return authenticator(token=(s_id, s_token))
 
@@ -547,36 +546,17 @@ class _ActionsMapPlugin(object):
 # HTTP Responses -------------------------------------------------------
 
 
-class HTTPOKResponse(HTTPResponse):
+def moulinette_error_to_http_response(error):
-    def __init__(self, output=""):
-        super(HTTPOKResponse, self).__init__(output, 200)
 
-
+    content = error.content()
-class HTTPBadRequestResponse(HTTPResponse):
+    if isinstance(content, dict):
-    def __init__(self, error=""):
+        return HTTPResponse(
-
+            json_encode(content),
-        if isinstance(error, MoulinetteError):
+            error.http_code,
-            content = error.content()
+            headers={"Content-type": "application/json"},
-            if isinstance(content, dict):
+        )
-                super(HTTPBadRequestResponse, self).__init__(
+    else:
-                    json_encode(content),
+        return HTTPResponse(content, error.http_code)
-                    400,
-                    headers={"Content-type": "application/json"},
-                )
-            else:
-                super(HTTPBadRequestResponse, self).__init__(content, 400)
-        else:
-            super(HTTPBadRequestResponse, self).__init__(error, 400)
-
-
-class HTTPUnauthorizedResponse(HTTPResponse):
-    def __init__(self, output=""):
-        super(HTTPUnauthorizedResponse, self).__init__(output, 401)
-
-
-class HTTPErrorResponse(HTTPResponse):
-    def __init__(self, output=""):
-        super(HTTPErrorResponse, self).__init__(output, 500)
 
 
 def format_for_response(content):
@@ -693,7 +673,7 @@ class ActionsMapParser(BaseActionsMapParser):
                 e,
             )
             logger.error(error_message)
-            raise MoulinetteError(error_message, raw_msg=True)
+            raise MoulinetteValidationError(error_message, raw_msg=True)
 
         if self.get_conf(tid, "authenticate"):
             authenticator = self.get_conf(tid, "authenticator")
@@ -722,7 +702,7 @@ class ActionsMapParser(BaseActionsMapParser):
         except KeyError as e:
             error_message = "no argument parser found for route '%s': %s" % (route, e)
             logger.error(error_message)
-            raise MoulinetteError(error_message, raw_msg=True)
+            raise MoulinetteValidationError(error_message, raw_msg=True)
         ret = argparse.Namespace()
 
         # TODO: Catch errors?

moulinette/interfaces/cli.py

@@ -13,7 +13,7 @@ import argcomplete
 
 from moulinette import msignals, m18n
 from moulinette.actionsmap import ActionsMap
-from moulinette.core import MoulinetteError
+from moulinette.core import MoulinetteError, MoulinetteValidationError
 from moulinette.interfaces import (
     BaseActionsMapParser,
     BaseInterface,
@@ -411,7 +411,7 @@ class ActionsMapParser(BaseActionsMapParser):
                 e,
             )
             logger.exception(error_message)
-            raise MoulinetteError(error_message, raw_msg=True)
+            raise MoulinetteValidationError(error_message, raw_msg=True)
 
         tid = getattr(ret, "_tid", None)
         if self.get_conf(tid, "authenticate"):
@@ -439,7 +439,7 @@ class ActionsMapParser(BaseActionsMapParser):
                 e,
             )
             logger.exception(error_message)
-            raise MoulinetteError(error_message, raw_msg=True)
+            raise MoulinetteValidationError(error_message, raw_msg=True)
         else:
             self.prepare_action_namespace(getattr(ret, "_tid", None), ret)
             self._parser.dequeue_callbacks(ret)
@@ -490,7 +490,7 @@ class Interface(BaseInterface):
 
         """
         if output_as and output_as not in ["json", "plain", "none"]:
-            raise MoulinetteError("invalid_usage")
+            raise MoulinetteValidationError("invalid_usage")
 
         # auto-complete
         argcomplete.autocomplete(self.actionsmap.parser._parser)
@@ -555,7 +555,7 @@ class Interface(BaseInterface):
         if confirm:
             m = message[0].lower() + message[1:]
             if prompt(m18n.g("confirm", prompt=m)) != value:
-                raise MoulinetteError("values_mismatch")
+                raise MoulinetteValidationError("values_mismatch")
 
         return value