Get rid of the _format_conf madness. Instead, have clear functions to define if authentication is required, trigger the auth process if so

moulinette/actionsmap.py

@@ -9,6 +9,7 @@ from time import time
 from collections import OrderedDict
 
 from moulinette import m18n, msignals
+from moulinette.core import init_authenticator
 from moulinette.cache import open_cachefile
 from moulinette.globals import init_moulinette_env
 from moulinette.core import (MoulinetteError, MoulinetteLock)
@@ -442,25 +443,27 @@ class ActionsMap(object):
         """Return the instance of the interface's actions map parser"""
         return self._parser
 
-    def get_authenticator(self, profile='default'):
+    def get_authenticator_for_profile(self, auth_profile):
-        """Get an authenticator instance
 
-        Retrieve the authenticator for the given profile and return a
-        new instance.
-
-        Keyword arguments:
-            - profile -- An authenticator profile name
-
-        Returns:
-            A new _BaseAuthenticator derived instance
-
-        """
         try:
-            auth = self.parser.get_global_conf('authenticator', profile)[1]
+            auth_conf = self.parser.global_conf['authenticator'][auth_profile]
         except KeyError:
-            raise ValueError("Unknown authenticator profile '%s'" % profile)
+            raise ValueError("Unknown authenticator profile '%s'" % auth_profile)
-        else:
+
-            return auth()
+        return init_authenticator(auth_conf)
+
+    def check_authentication_if_required(self, args, **kwargs):
+
+        auth_profile = self.parser.auth_required(args, **kwargs)
+
+        if not auth_profile:
+            return
+
+        authenticator = self.get_authenticator_for_profile(auth_profile)
+        auth = msignals.authenticate(authenticator)
+
+        if not auth.is_authenticated:
+            raise MoulinetteError('authentication_required_long')
 
     def process(self, args, timeout=None, **kwargs):
         """
@@ -475,14 +478,7 @@ class ActionsMap(object):
         """
 
         # Perform authentication if needed
-        auth_required = self.parser.auth_required(args, **kwargs)
+        self.check_authentication_if_required(args, **kwargs)
-        if auth_required:
-            auth_conf, klass = auth_required
-
-            # TODO: Catch errors
-            auth = msignals.authenticate(klass(), **auth_conf)
-            if not auth.is_authenticated:
-                raise MoulinetteError('authentication_required_long')
 
         # Parse arguments
         arguments = vars(self.parser.parse_args(args, **kwargs))

moulinette/interfaces/__init__.py

@@ -7,7 +7,7 @@ import copy
 from collections import deque, OrderedDict
 
 from moulinette import msignals, msettings, m18n
-from moulinette.core import (init_authenticator, MoulinetteError)
+from moulinette.core import MoulinetteError
 
 logger = logging.getLogger('moulinette.interface')
 
@@ -119,6 +119,19 @@ class BaseActionsMapParser(object):
         raise NotImplementedError("derived class '%s' must override this method" %
                                   self.__class__.__name__)
 
+    def auth_required(self, args, **kwargs):
+        """Check if authentication is required to run the requested action
+
+        Keyword arguments:
+            - args -- Arguments string or dict (TODO)
+
+        Returns:
+            False, or the authentication profile required
+
+        """
+        raise NotImplementedError("derived class '%s' must override this method" %
+                                  self.__class__.__name__)
+
     def parse_args(self, args, **kwargs):
         """Parse arguments
 
@@ -151,15 +164,6 @@ class BaseActionsMapParser(object):
             namespace = argparse.Namespace()
         namespace._tid = tid
 
-        # Perform authentication if needed
-        if self.get_conf(tid, 'authenticate'):
-            auth_conf, cls = self.get_conf(tid, 'authenticator')
-
-            # TODO: Catch errors
-            auth = msignals.authenticate(cls(), **auth_conf)
-            if not auth.is_authenticated:
-                raise MoulinetteError('authentication_required_long')
-
         return namespace
 
     # Configuration access
@@ -169,24 +173,6 @@ class BaseActionsMapParser(object):
         """Return the global configuration of the parser"""
         return self._o._global_conf
 
-    def get_global_conf(self, name, profile='default'):
-        """Get the global value of a configuration
-
-        Return the formated global value of the configuration 'name' for
-        the given profile. If the configuration doesn't provide profile,
-        the formated default value is returned.
-
-        Keyword arguments:
-            - name -- The configuration name
-            - profile -- The profile of the configuration
-
-        """
-        if name == 'authenticator':
-            value = self.global_conf[name][profile]
-        else:
-            value = self.global_conf[name]
-        return self._format_conf(name, value)
-
     def set_global_conf(self, configuration):
         """Set global configuration
 
@@ -211,11 +197,9 @@ class BaseActionsMapParser(object):
 
         """
         try:
-            value = self._o._conf[action][name]
+            return self._o._conf[action][name]
         except KeyError:
-            return self.get_global_conf(name)
+            return self.global_conf[name]
-        else:
-            return self._format_conf(name, value)
 
     def set_conf(self, action, configuration):
         """Set configuration for an action
@@ -301,27 +285,6 @@ class BaseActionsMapParser(object):
 
         return conf
 
-    def _format_conf(self, name, value):
-        """Format a configuration value
-
-        Return the formated value of the configuration 'name' from its
-        given value.
-
-        Keyword arguments:
-            - name -- The name of the configuration
-            - value -- The value to format
-
-        """
-        if name == 'authenticator' and value:
-            (identifier, configuration, parameters) = value
-
-            # Return global configuration and an authenticator
-            # instanciator as a 2-tuple
-            return (configuration,
-                    lambda: init_authenticator(identifier, parameters))
-
-        return value
-
 
 class BaseInterface(object):
 

moulinette/interfaces/api.py

@@ -343,8 +343,8 @@ class _ActionsMapPlugin(object):
 
         try:
             # Attempt to authenticate
-            auth = self.actionsmap.get_authenticator(profile)
+            authenticator = self.actionsmap.get_authenticator_for_profile(profile)
-            auth(password, token=(s_id, s_hash))
+            authenticator(password, token=(s_id, s_hash))
         except MoulinetteError as e:
             if len(s_hashes) > 0:
                 try:
@@ -634,9 +634,9 @@ class ActionsMapParser(BaseActionsMapParser):
                 # dependent of the route being hit ...
                 # e.g. in the context of friend2friend stuff that could
                 # auth with some custom auth system to access some
-                # data
+                # data with something like :
                 # return self.get_conf(tid, 'authenticator')
-                return self.get_global_conf('authenticator', 'default')
+                return 'default'
         except KeyError:
             logger.error("no argument parser found for route '%s'", route)
             raise MoulinetteError('error_see_log')

moulinette/interfaces/cli.py

@@ -347,12 +347,6 @@ class ActionsMapParser(BaseActionsMapParser):
                                            deprecated=deprecated,
                                            deprecated_alias=deprecated_alias)
 
-    def auth_required(self, args, **kwargs):
-        # No auth is required for CLI,
-        # e.g. in the context of Yunohost we only run as root
-        # but we could someday change this code to check for
-        return False
-
     def add_global_arguments(self, arguments):
         for argument_name, argument_options in arguments.items():
             # will adapt arguments name for cli or api context
@@ -361,6 +355,23 @@ class ActionsMapParser(BaseActionsMapParser):
 
             self.global_parser.add_argument(*names, **argument_options)
 
+    def auth_required(self, args, **kwargs):
+        # FIXME? idk .. this try/except is duplicated from parse_args below
+        # Just to be able to obtain the tid
+        try:
+            ret = self._parser.parse_args(args)
+        except SystemExit:
+            raise
+        except:
+            logger.exception("unable to parse arguments '%s'", ' '.join(args))
+            raise MoulinetteError('error_see_log')
+
+        tid = getattr(ret, '_tid', None)
+        if self.get_conf(tid, 'authenticate'):
+            return self.get_conf(tid, 'authenticator')
+        else:
+            return False
+
     def parse_args(self, args, **kwargs):
         try:
             ret = self._parser.parse_args(args)