Merge pull request #216 from YunoHost/simplify-auth-mechanism

Simplify auth mechanism

README.md

@@ -50,7 +50,6 @@ Requirements
 
 * Python 2.7
 * python-bottle (>= 0.10)
-* python-gnupg (>= 0.3)
 * python-ldap (>= 2.4)
 * PyYAML
 

data/actionsmap/test.yml

@@ -1,72 +0,0 @@
-
-#############################
-#     Global parameters     #
-#############################
-_global:
-    configuration:
-        authenticate:
-            - api
-        authenticator:
-            default:
-                vendor: ldap
-                help: Admin Password
-                parameters:
-                    uri: ldap://localhost:389
-                    base_dn: dc=yunohost,dc=org
-                    user_rdn: cn=admin,dc=yunohost,dc=org
-            ldap-anonymous:
-                vendor: ldap
-                parameters:
-                    uri: ldap://localhost:389
-                    base_dn: dc=yunohost,dc=org
-            test-profile:
-                vendor: ldap
-                help: Admin Password (profile)
-                parameters:
-                    uri: ldap://localhost:389
-                    base_dn: dc=yunohost,dc=org
-                    user_rdn: cn=admin,dc=yunohost,dc=org
-            as-root:
-                vendor: ldap
-                parameters:
-                    # We can get this uri by (urllib.quote_plus('/var/run/slapd/ldapi')
-                    uri: ldapi://%2Fvar%2Frun%2Fslapd%2Fldapi
-                    base_dn: dc=yunohost,dc=org
-                    user_rdn: gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth
-        argument_auth: true
-        lock: false
-
-#############################
-#       Test Actions        #
-#############################
-test:
-    actions:
-        non-auth:
-            api: GET /test/non-auth
-            configuration:
-                authenticate: false
-        auth:
-            api: GET /test/auth
-            configuration:
-                authenticate: all
-        auth-profile:
-            api: GET /test/auth-profile
-            configuration:
-                authenticate: all
-                authenticator: test-profile
-        auth-cli:
-            api: GET /test/auth-cli
-            configuration:
-                authenticate:
-                    - cli
-        root-auth:
-            api: GET /test/root-auth
-            configuration:
-                authenticate: all
-                authenticator: as-root
-        anonymous:
-            api: GET /test/anon
-            configuration:
-                authenticate: all
-                authenticator: ldap-anonymous
-                argument_auth: false

data/moulinette_cli

@@ -1,33 +0,0 @@
-# yunohost(1) completion
-
-_yunohost_cli()
-{
-local argc cur prev opts
-COMPREPLY=()
-
-argc=${COMP_CWORD}
-cur="${COMP_WORDS[argc]}"
-prev="${COMP_WORDS[argc-1]}"
-opts=$(yunohost -h | sed -n "/usage/,/}/p" | awk -F"{" '{print $2}' | awk -F"}" '{print $1}' | tr ',' ' ')
-
-if [[ $argc = 1 ]];
-then
-	COMPREPLY=( $(compgen -W "$opts --help" -- $cur ) )
-fi
-
-if [[ "$prev" != "--help" ]];
-then
-	if [[ $argc = 2 ]];
-	then
-		opts2=$(yunohost $prev -h | sed -n "/usage/,/}/p" | awk -F"{" '{print $2}' | awk -F"}" '{print $1}' | tr ',' ' ')
-		COMPREPLY=( $(compgen -W "$opts2 --help" -- $cur ) )
-	elif [[ $argc = 3 ]];
-	then
-		COMPREPLY=( $(compgen -W "--help" $cur ) )
-	fi
-else
-	COMPREPLY=()
-fi
-
-}
-complete -F _yunohost_cli yunohost

debian/control

@@ -13,7 +13,6 @@ Depends: ${misc:Depends}, ${python:Depends},
     python-ldap,
     python-yaml,
     python-bottle (>= 0.12),
-    python-gnupg,
     python-gevent-websocket,
     python-argcomplete,
     python-toml,

doc/requirements.txt

@@ -1,5 +1,4 @@
 sphinx
-gnupg
 mock
 pyyaml
 toml

locales/en.json

@@ -1,6 +1,5 @@
 {
     "argument_required": "Argument '{argument}' is required",
-    "authentication_profile_required": "Authentication to profile '{profile}' required",
     "authentication_required": "Authentication required",
     "authentication_required_long": "Authentication is required to perform this action",
     "colon": "{}: ",
@@ -17,6 +16,7 @@
     "instance_already_running": "There is already a YunoHost operation running. Please wait for it to finish before running another one.",
     "invalid_argument": "Invalid argument '{argument}': {error}",
     "invalid_password": "Invalid password",
+    "invalid_token": "Invalid token - please authenticate",
     "invalid_usage": "Invalid usage, pass --help to see help",
     "ldap_attribute_already_exists": "Attribute '{attribute}' already exists with value '{value}'",
     "ldap_operation_error": "An error occurred during LDAP operation",

moulinette/actionsmap.py

@@ -7,6 +7,7 @@ import yaml
 import cPickle as pickle
 from time import time
 from collections import OrderedDict
+from importlib import import_module
 
 from moulinette import m18n, msignals
 from moulinette.cache import open_cachefile
@@ -442,25 +443,35 @@ 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
+        # Fetch the configuration for the authenticator module as defined in the actionmap
-        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)
+
+        # Load and initialize the authenticator module
+        try:
+            mod = import_module('moulinette.authenticators.%s' % auth_conf["vendor"])
+        except ImportError:
+            logger.exception("unable to load authenticator vendor '%s'", auth_conf["vendor"])
+            raise MoulinetteError('error_see_log')
         else:
-            return auth()
+            return mod.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):
         """
@@ -473,6 +484,10 @@ class ActionsMap(object):
             - **kwargs -- Additional interface arguments
 
         """
+
+        # Perform authentication if needed
+        self.check_authentication_if_required(args, **kwargs)
+
         # Parse arguments
         arguments = vars(self.parser.parse_args(args, **kwargs))
 

moulinette/authenticators/__init__.py

@@ -1,9 +1,11 @@
 # -*- coding: utf-8 -*-
 
-import gnupg
+import os
 import logging
+import hashlib
+import hmac
 
-from moulinette.cache import open_cachefile
+from moulinette.cache import open_cachefile, get_cachedir
 from moulinette.core import MoulinetteError
 
 logger = logging.getLogger('moulinette.authenticator')
@@ -31,6 +33,7 @@ class BaseAuthenticator(object):
 
     def __init__(self, name):
         self._name = name
+        self.is_authenticated = False
 
     @property
     def name(self):
@@ -43,12 +46,6 @@ class BaseAuthenticator(object):
     """The vendor name of the authenticator"""
     vendor = None
 
-    @property
-    def is_authenticated(self):
-        """Either the instance is authenticated or not"""
-        raise NotImplementedError("derived class '%s' must override this property" %
-                                  self.__class__.__name__)
-
     # Virtual methods
     # Each authenticator classes must implement these methods.
 
@@ -75,7 +72,7 @@ class BaseAuthenticator(object):
         instance is returned and the session is registered for the token
         if 'token' and 'password' are given.
         The token is composed by the session identifier and a session
-        hash - to use for encryption - as a 2-tuple.
+        hash (the "true token") - to use for encryption - as a 2-tuple.
 
         Keyword arguments:
             - password -- A clear text password
@@ -87,44 +84,57 @@ class BaseAuthenticator(object):
         """
         if self.is_authenticated:
             return self
-        store_session = True if password and token else False
 
-        if token:
+        #
+        # Authenticate using the password
+        #
+        if password:
             try:
-                # Extract id and hash from token
+                # Attempt to authenticate
-                s_id, s_hash = token
+                self.authenticate(password)
-            except TypeError as e:
+            except MoulinetteError:
-                logger.error("unable to extract token parts from '%s' because '%s'", token, e)
+                raise
-                if password is None:
-                    raise MoulinetteError('error_see_log')
-
-                logger.info("session will not be stored")
-                store_session = False
-            else:
-                if password is None:
-                    # Retrieve session
-                    password = self._retrieve_session(s_id, s_hash)
-
-        try:
-            # Attempt to authenticate
-            self.authenticate(password)
-        except MoulinetteError:
-            raise
-        except Exception as e:
-            logger.exception("authentication (name: '%s', vendor: '%s') fails because '%s'",
-                             self.name, self.vendor, e)
-            raise MoulinetteError('unable_authenticate')
-
-        # Store session
-        if store_session:
-            try:
-                self._store_session(s_id, s_hash, password)
             except Exception as e:
-                import traceback
+                logger.exception("authentication (name: '%s', vendor: '%s') fails because '%s'",
-                traceback.print_exc()
+                                 self.name, self.vendor, e)
-                logger.exception("unable to store session because %s", e)
+                raise MoulinetteError('unable_authenticate')
+
+            self.is_authenticated = True
+
+            # Store session for later using the provided (new) token if any
+            if token:
+                try:
+                    s_id, s_token = token
+                    self._store_session(s_id, s_token)
+                except Exception as e:
+                    import traceback
+                    traceback.print_exc()
+                    logger.exception("unable to store session because %s", e)
+                else:
+                    logger.debug("session has been stored")
+
+        #
+        # Authenticate using the token provided
+        #
+        elif token:
+            try:
+                s_id, s_token = token
+                # Attempt to authenticate
+                self._authenticate_session(s_id, s_token)
+            except MoulinetteError as e:
+                raise
+            except Exception as e:
+                logger.exception("authentication (name: '%s', vendor: '%s') fails because '%s'",
+                                 self.name, self.vendor, e)
+                raise MoulinetteError('unable_authenticate')
             else:
-                logger.debug("session has been stored")
+                self.is_authenticated = True
+
+        #
+        # No credentials given, can't authenticate
+        #
+        else:
+            raise MoulinetteError('unable_authenticate')
 
         return self
 
@@ -135,33 +145,62 @@ class BaseAuthenticator(object):
         return open_cachefile('%s.asc' % session_id, mode,
                               subdir='session/%s' % self.name)
 
-    def _store_session(self, session_id, session_hash, password):
+    def _store_session(self, session_id, session_token):
-        """Store a session and its associated password"""
+        """Store a session to be able to use it later to reauthenticate"""
-        gpg = gnupg.GPG()
-        gpg.encoding = 'utf-8'
-
-        # Encrypt the password using the session hash
-        s = str(gpg.encrypt(password, None, symmetric=True, passphrase=session_hash))
-        assert len(s), "For some reason GPG can't perform encryption, maybe check /root/.gnupg/gpg.conf or re-run with gpg = gnupg.GPG(verbose=True) ?"
 
+        # We store a hash of the session_id and the session_token (the token is assumed to be secret)
+        to_hash = "{id}:{token}".format(id=session_id, token=session_token)
+        hash_ = hashlib.sha256(to_hash).hexdigest()
         with self._open_sessionfile(session_id, 'w') as f:
-            f.write(s)
+            f.write(hash_)
 
-    def _retrieve_session(self, session_id, session_hash):
+    def _authenticate_session(self, session_id, session_token):
-        """Retrieve a session and return its associated password"""
+        """Checks session and token against the stored session token"""
         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...
+
             with self._open_sessionfile(session_id, 'r') as f:
-                enc_pwd = f.read()
+                stored_hash = f.read()
         except IOError as e:
             logger.debug("unable to retrieve session", exc_info=1)
             raise MoulinetteError('unable_retrieve_session', exception=e)
         else:
-            gpg = gnupg.GPG()
+            #
-            gpg.encoding = 'utf-8'
+            # session_id (or just id) : This is unique id for the current session from the user. Not too important
+            # if this info gets stolen somehow. It is stored in the client's side (browser) using regular cookies.
+            #
+            # session_token (or just token) : This is a secret info, like some sort of ephemeral password,
+            # used to authenticate the session without the user having to retype the password all the time...
+            #    - It is generated on our side during the initial auth of the user (which happens with the actual admin password)
+            #    - It is stored on the client's side (browser) using (signed) cookies.
+            #    - We also store it on our side in the form of a hash of {id}:{token} (c.f. _store_session).
+            #      We could simply store the raw token, but hashing it is an additonal low-cost security layer
+            #      in case this info gets exposed for some reason (e.g. bad file perms for reasons...)
+            #
+            # When the user comes back, we fetch the session_id and session_token from its cookies. Then we
+            # re-hash the {id}:{token} and compare it to the previously stored hash for this session_id ...
+            # It it matches, then the user is authenticated. Otherwise, the token is invalid.
+            #
+            to_hash = "{id}:{token}".format(id=session_id, token=session_token)
+            hash_ = hashlib.sha256(to_hash).hexdigest()
 
-            decrypted = gpg.decrypt(enc_pwd, passphrase=session_hash)
+            if not hmac.compare_digest(hash_, stored_hash):
-            if decrypted.ok is not True:
+                raise MoulinetteError('invalid_token')
-                error_message = "unable to decrypt password for the session: %s" % decrypted.status
+            else:
-                logger.error(error_message)
+                return
-                raise MoulinetteError('unable_retrieve_session', exception=error_message)
+
-            return decrypted.data
+    def _clean_session(self, session_id):
+        """Clean a session cache
+
+        Remove cache for the session 'session_id' and for this authenticator profile
+
+        Keyword arguments:
+            - session_id -- The session id to clean
+        """
+        sessiondir = get_cachedir('session')
+
+        try:
+            os.remove(os.path.join(sessiondir, self.name, '%s.asc' % session_id))
+        except OSError:
+            pass

moulinette/authenticators/dummy.py

@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+
+import logging
+from moulinette.core import MoulinetteError
+from moulinette.authenticators import BaseAuthenticator
+
+logger = logging.getLogger('moulinette.authenticator.dummy')
+
+# Dummy authenticator implementation
+
+
+class Authenticator(BaseAuthenticator):
+
+    """Dummy authenticator used for tests
+    """
+
+    vendor = 'dummy'
+
+    def __init__(self, name, vendor, parameters, extra):
+        logger.debug("initialize authenticator dummy")
+        super(Authenticator, self).__init__(name)
+
+    def authenticate(self, password):
+
+        if not password == "Yoloswag":
+            raise MoulinetteError("Invalid password!")
+
+        return self

moulinette/authenticators/ldap.py

@@ -33,23 +33,20 @@ class Authenticator(BaseAuthenticator):
 
     """
 
-    def __init__(self, name, uri, base_dn, user_rdn=None):
+    def __init__(self, name, vendor, parameters, extra):
+        self.uri = parameters["uri"]
+        self.basedn = parameters["base_dn"]
+        self.userdn = parameters["user_rdn"]
+        self.extra = extra
         logger.debug("initialize authenticator '%s' with: uri='%s', "
-                     "base_dn='%s', user_rdn='%s'", name, uri, base_dn, user_rdn)
+                     "base_dn='%s', user_rdn='%s'", name, self.uri, self.basedn, self.userdn)
         super(Authenticator, self).__init__(name)
 
-        self.uri = uri
+        if self.userdn:
-        self.basedn = base_dn
+            if 'cn=external,cn=auth' in self.userdn:
-        if user_rdn:
-            self.userdn = user_rdn
-            if 'cn=external,cn=auth' in user_rdn:
                 self.authenticate(None)
             else:
                 self.con = None
-        else:
-            # Initialize anonymous usage
-            self.userdn = ''
-            self.authenticate(None)
 
     def __del__(self):
         """Disconnect and free ressources"""
@@ -60,21 +57,6 @@ class Authenticator(BaseAuthenticator):
 
     vendor = 'ldap'
 
-    @property
-    def is_authenticated(self):
-        if self.con is None:
-            return False
-        try:
-            # Retrieve identity
-            who = self.con.whoami_s()
-        except Exception as e:
-            logger.warning("Error during ldap authentication process: %s", e)
-            return False
-        else:
-            if who[3:] == self.userdn:
-                return True
-        return False
-
     # Implement virtual methods
 
     def authenticate(self, password):
@@ -92,9 +74,19 @@ class Authenticator(BaseAuthenticator):
         except ldap.SERVER_DOWN:
             logger.exception('unable to reach the server to authenticate')
             raise MoulinetteError('ldap_server_down')
+
+        # Check that we are indeed logged in with the right identity
+        try:
+            who = con.whoami_s()
+        except Exception as e:
+            logger.warning("Error during ldap authentication process: %s", e)
+            raise
         else:
-            self.con = con
+            if who[3:] != self.userdn:
-            self._ensure_password_uses_strong_hash(password)
+                raise MoulinetteError("Not logged in with the expected userdn ?!")
+            else:
+                self.con = con
+                self._ensure_password_uses_strong_hash(password)
 
     def _ensure_password_uses_strong_hash(self, password):
         # XXX this has been copy pasted from YunoHost, should we put that into moulinette?

moulinette/cache.py

@@ -25,7 +25,7 @@ def get_cachedir(subdir='', make_dir=True):
     return path
 
 
-def open_cachefile(filename, mode='r', **kwargs):
+def open_cachefile(filename, mode='r', subdir=''):
     """Open a cache file and return a stream
 
     Attempt to open in 'mode' the cache file 'filename' from the
@@ -39,9 +39,6 @@ def open_cachefile(filename, mode='r', **kwargs):
         - **kwargs -- Optional arguments for get_cachedir
 
     """
-    # Set make_dir if not given
+    cache_dir = get_cachedir(subdir, make_dir=True if mode[0] == 'w' else False)
-    kwargs['make_dir'] = kwargs.get('make_dir',
-                                    True if mode[0] == 'w' else False)
-    cache_dir = get_cachedir(**kwargs)
     file_path = os.path.join(cache_dir, filename)
     return open(file_path, mode)

moulinette/core.py

@@ -9,7 +9,6 @@ from importlib import import_module
 
 import moulinette
 from moulinette.globals import init_moulinette_env
-from moulinette.cache import get_cachedir
 
 
 logger = logging.getLogger('moulinette.core')
@@ -181,7 +180,6 @@ class Moulinette18n(object):
 
         moulinette_env = init_moulinette_env()
         self.locales_dir = moulinette_env['LOCALES_DIR']
-        self.lib_dir = moulinette_env['LIB_DIR']
 
         # Init global translator
         self._global = Translator(self.locales_dir, default_locale)
@@ -202,7 +200,8 @@ class Moulinette18n(object):
         """
         if namespace not in self._namespaces:
             # Create new Translator object
-            translator = Translator('%s/%s/locales' % (self.lib_dir, namespace),
+            lib_dir = init_moulinette_env()["LIB_DIR"]
+            translator = Translator('%s/%s/locales' % (lib_dir, namespace),
                                     self.default_locale)
             translator.set_locale(self.locale)
             self._namespaces[namespace] = translator
@@ -287,7 +286,7 @@ class MoulinetteSignals(object):
     """The list of available signals"""
     signals = {'authenticate', 'prompt', 'display'}
 
-    def authenticate(self, authenticator, help):
+    def authenticate(self, authenticator):
         """Process the authentication
 
         Attempt to authenticate to the given authenticator and return
@@ -297,7 +296,6 @@ class MoulinetteSignals(object):
 
         Keyword arguments:
             - authenticator -- The authenticator object to use
-            - help -- The translation key of the authenticator's help message
 
         Returns:
             The authenticator object
@@ -305,7 +303,7 @@ class MoulinetteSignals(object):
         """
         if authenticator.is_authenticated:
             return authenticator
-        return self._authenticate(authenticator, help)
+        return self._authenticate(authenticator)
 
     def prompt(self, message, is_password=False, confirm=False, color='blue'):
         """Prompt for a value
@@ -374,8 +372,8 @@ def init_interface(name, kwargs={}, actionsmap={}):
 
     try:
         mod = import_module('moulinette.interfaces.%s' % name)
-    except ImportError:
+    except ImportError as e:
-        logger.exception("unable to load interface '%s'", name)
+        logger.exception("unable to load interface '%s' : %s", name, e)
         raise MoulinetteError('error_see_log')
     else:
         try:
@@ -398,50 +396,6 @@ def init_interface(name, kwargs={}, actionsmap={}):
     return interface(amap, **kwargs)
 
 
-def init_authenticator(vendor_and_name, kwargs={}):
-    """Return a new authenticator instance
-
-    Retrieve the given authenticator vendor and return a new instance of
-    its Authenticator class for the given profile.
-
-    Keyword arguments:
-        - vendor -- The authenticator vendor name
-        - name -- The authenticator profile name
-        - kwargs -- A dict of arguments for the authenticator profile
-
-    """
-    (vendor, name) = vendor_and_name
-    try:
-        mod = import_module('moulinette.authenticators.%s' % vendor)
-    except ImportError:
-        logger.exception("unable to load authenticator vendor '%s'", vendor)
-        raise MoulinetteError('error_see_log')
-    else:
-        return mod.Authenticator(name, **kwargs)
-
-
-def clean_session(session_id, profiles=[]):
-    """Clean a session cache
-
-    Remove cache for the session 'session_id' and for profiles in
-    'profiles' or for all of them if the list is empty.
-
-    Keyword arguments:
-        - session_id -- The session id to clean
-        - profiles -- A list of profiles to clean
-
-    """
-    sessiondir = get_cachedir('session')
-    if not profiles:
-        profiles = os.listdir(sessiondir)
-
-    for p in profiles:
-        try:
-            os.unlink(os.path.join(sessiondir, p, '%s.asc' % session_id))
-        except OSError:
-            pass
-
-
 # Moulinette core classes ----------------------------------------------
 
 class MoulinetteError(Exception):

moulinette/interfaces/__init__.py

@@ -6,8 +6,8 @@ import argparse
 import copy
 from collections import deque, OrderedDict
 
-from moulinette import msignals, msettings, m18n
+from moulinette import 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,18 +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')
-            if self.get_conf(tid, 'argument_auth') and \
-                    self.get_conf(tid, 'authenticate') == 'all':
-                namespace.auth = auth
-
         return namespace
 
     # Configuration access
@@ -172,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
 
@@ -214,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
@@ -285,72 +266,18 @@ class BaseActionsMapParser(object):
                 else:
                     auths = {}
                     for auth_name, auth_conf in auth.items():
-                        # Add authenticator profile as a 3-tuple
+                        auths[auth_name] = {'name': auth_name,
-                        # (identifier, configuration, parameters) with
+                                            'vendor': auth_conf.get('vendor'),
-                        # - identifier: the authenticator vendor and its
+                                            'parameters': auth_conf.get('parameters', {}),
-                        #     profile name as a 2-tuple
+                                            'extra': {'help': auth_conf.get('help', None)}}
-                        # - configuration: a dict of additional global
-                        #     configuration (i.e. 'help')
-                        # - parameters: a dict of arguments for the
-                        #     authenticator profile
-                        auths[auth_name] = ((auth_conf.get('vendor'), auth_name),
-                                            {'help': auth_conf.get('help', None)},
-                                            auth_conf.get('parameters', {}))
                     conf['authenticator'] = auths
             else:
                 logger.error("expecting a dict of profile(s) or a profile name "
                              "for configuration 'authenticator', got %r", auth)
                 raise MoulinetteError('error_see_log')
 
-        # -- 'argument_auth'
-        try:
-            arg_auth = configuration['argument_auth']
-        except KeyError:
-            pass
-        else:
-            if isinstance(arg_auth, bool):
-                conf['argument_auth'] = arg_auth
-            else:
-                logger.error("expecting a boolean for configuration "
-                             "'argument_auth', got %r", arg_auth)
-                raise MoulinetteError('error_see_log')
-
-        # -- 'lock'
-        try:
-            lock = configuration['lock']
-        except KeyError:
-            pass
-        else:
-            if isinstance(lock, bool):
-                conf['lock'] = lock
-            else:
-                logger.error("expecting a boolean for configuration 'lock', "
-                             "got %r", lock)
-                raise MoulinetteError('error_see_log')
-
         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

@@ -14,7 +14,7 @@ from bottle import run, request, response, Bottle, HTTPResponse
 from bottle import abort
 
 from moulinette import msignals, m18n, env
-from moulinette.core import MoulinetteError, clean_session
+from moulinette.core import MoulinetteError
 from moulinette.interfaces import (
     BaseActionsMapParser, BaseInterface, ExtendedArgumentParser,
 )
@@ -251,10 +251,7 @@ class _ActionsMapPlugin(object):
         def _logout(callback):
             def wrapper():
                 kwargs = {}
-                try:
+                kwargs['profile'] = request.POST.get('profile', "default")
-                    kwargs['profile'] = request.POST.get('profile')
-                except KeyError:
-                    pass
                 return callback(**kwargs)
             return wrapper
 
@@ -335,18 +332,18 @@ class _ActionsMapPlugin(object):
         try:
             s_secret = self.secrets[s_id]
         except KeyError:
-            s_hashes = {}
+            s_tokens = {}
         else:
-            s_hashes = request.get_cookie('session.hashes',
+            s_tokens = request.get_cookie('session.tokens',
                                           secret=s_secret) or {}
-        s_hash = random_ascii()
+        s_new_token = random_ascii()
 
         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_new_token))
         except MoulinetteError as e:
-            if len(s_hashes) > 0:
+            if len(s_tokens) > 0:
                 try:
                     self.logout(profile)
                 except:
@@ -354,15 +351,15 @@ class _ActionsMapPlugin(object):
             raise HTTPUnauthorizedResponse(e.strerror)
         else:
             # Update dicts with new values
-            s_hashes[profile] = s_hash
+            s_tokens[profile] = s_new_token
             self.secrets[s_id] = s_secret = random_ascii()
 
             response.set_cookie('session.id', s_id, secure=True)
-            response.set_cookie('session.hashes', s_hashes, secure=True,
+            response.set_cookie('session.tokens', s_tokens, secure=True,
                                 secret=s_secret)
             return m18n.g('logged_in')
 
-    def logout(self, profile=None):
+    def logout(self, profile):
         """Log out from an authenticator profile
 
         Attempt to unregister a given profile - or all by default - from
@@ -374,14 +371,21 @@ class _ActionsMapPlugin(object):
         """
         s_id = request.get_cookie('session.id')
         try:
-            del self.secrets[s_id]
+            # We check that there's a (signed) session.hash available
+            # for additional security ?
+            # (An attacker could not craft such signed hashed ? (FIXME : need to make sure of this))
+            s_secret = self.secrets[s_id]
+            request.get_cookie('session.tokens',
+                               secret=s_secret, default={})[profile]
         except KeyError:
             raise HTTPUnauthorizedResponse(m18n.g('not_logged_in'))
         else:
+            del self.secrets[s_id]
+            authenticator = self.actionsmap.get_authenticator_for_profile(profile)
+            authenticator._clean_session(s_id)
             # TODO: Clean the session for profile only
             # Delete cookie and clean the session
-            response.set_cookie('session.hashes', '', max_age=-1)
+            response.set_cookie('session.tokens', '', max_age=-1)
-            clean_session(s_id)
         return m18n.g('logged_out')
 
     def messages(self):
@@ -461,7 +465,7 @@ class _ActionsMapPlugin(object):
 
     # Signals handlers
 
-    def _do_authenticate(self, authenticator, help):
+    def _do_authenticate(self, authenticator):
         """Process the authentication
 
         Handle the core.MoulinetteSignals.authenticate signal.
@@ -470,17 +474,13 @@ class _ActionsMapPlugin(object):
         s_id = request.get_cookie('session.id')
         try:
             s_secret = self.secrets[s_id]
-            s_hash = request.get_cookie('session.hashes',
+            s_token = request.get_cookie('session.tokens',
                                         secret=s_secret, default={})[authenticator.name]
         except KeyError:
-            if authenticator.name == 'default':
+            msg = m18n.g('authentication_required')
-                msg = m18n.g('authentication_required')
-            else:
-                msg = m18n.g('authentication_profile_required',
-                             profile=authenticator.name)
             raise HTTPUnauthorizedResponse(msg)
         else:
-            return authenticator(token=(s_id, s_hash))
+            return authenticator(token=(s_id, s_token))
 
     def _do_display(self, message, style):
         """Display a message
@@ -627,6 +627,24 @@ class ActionsMapParser(BaseActionsMapParser):
         # Return the created parser
         return parser
 
+    def auth_required(self, args, route, **kwargs):
+        try:
+            # Retrieve the tid for the route
+            tid, _ = self._parsers[route]
+            if not self.get_conf(tid, 'authenticate'):
+                return False
+            else:
+                # TODO: In the future, we could make the authentication
+                # 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 with something like :
+                # return self.get_conf(tid, 'authenticator')
+                return 'default'
+        except KeyError:
+            logger.error("no argument parser found for route '%s'", route)
+            raise MoulinetteError('error_see_log')
+
     def parse_args(self, args, route, **kwargs):
         """Parse arguments
 
@@ -635,28 +653,13 @@ class ActionsMapParser(BaseActionsMapParser):
 
         """
         try:
-            # Retrieve the tid and the parser for the route
+            # Retrieve the parser for the route
-            tid, parser = self._parsers[route]
+            _, parser = self._parsers[route]
         except KeyError:
             logger.error("no argument parser found for route '%s'", route)
             raise MoulinetteError('error_see_log')
         ret = argparse.Namespace()
 
-        # Perform authentication if needed
-        if self.get_conf(tid, 'authenticate'):
-            # TODO: Clean this hard fix and find a way to set an authenticator
-            # to use for the api only
-            # auth_conf, klass = self.get_conf(tid, 'authenticator')
-            auth_conf, klass = self.get_global_conf('authenticator', 'default')
-
-            # TODO: Catch errors
-            auth = msignals.authenticate(klass(), **auth_conf)
-            if not auth.is_authenticated:
-                raise MoulinetteError('authentication_required_long')
-            if self.get_conf(tid, 'argument_auth') and \
-               self.get_conf(tid, 'authenticate') == 'all':
-                ret.auth = auth
-
         # TODO: Catch errors?
         ret = parser.parse_args(args, ret)
         parser.dequeue_callbacks(ret)

moulinette/interfaces/cli.py

@@ -355,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)
@@ -418,7 +435,7 @@ class Interface(BaseInterface):
         # Set handler for authentication
         if password:
             msignals.set_handler('authenticate',
-                                 lambda a, h: a(password=password))
+                                 lambda a: a(password=password))
 
         try:
             ret = self.actionsmap.process(args, timeout=timeout)
@@ -443,13 +460,14 @@ class Interface(BaseInterface):
 
     # Signals handlers
 
-    def _do_authenticate(self, authenticator, help):
+    def _do_authenticate(self, authenticator):
         """Process the authentication
 
         Handle the core.MoulinetteSignals.authenticate signal.
 
         """
         # TODO: Allow token authentication?
+        help = authenticator.extra.get("help")
         msg = m18n.n(help) if help else m18n.g('password')
         return authenticator(password=self._do_prompt(msg, True, False,
                                                       color='yellow'))

test/actionsmap/moulitest.yml

@@ -0,0 +1,42 @@
+
+#############################
+#     Global parameters     #
+#############################
+_global:
+    configuration:
+        authenticate:
+            - all
+        authenticator:
+            default:
+                vendor: dummy
+                help: Dummy Password
+            yoloswag:
+                vendor: dummy
+                help: Dummy Yoloswag Password
+
+#############################
+#       Test Actions        #
+#############################
+testauth:
+    actions:
+
+        none:
+            api: GET /test-auth/none
+            configuration:
+                authenticate: false
+
+        default:
+            api: GET /test-auth/default
+            configuration:
+                authenticate: all
+                authenticator: default
+
+#        only-api:
+#            api: GET /test-auth/only-api
+#            configuration:
+#                authenticate: api
+#
+        other-profile:
+            api: GET /test-auth/other-profile
+            configuration:
+                authenticator: yoloswag

test/conftest.py

@@ -2,7 +2,7 @@
 
 import json
 import os
-
+import shutil
 import pytest
 
 
@@ -38,13 +38,13 @@ def patch_translate(moulinette):
 
 def patch_logging(moulinette):
     """Configure logging to use the custom logger."""
-    handlers = set(['tty'])
+    handlers = set(['tty', 'api'])
     root_handlers = set(handlers)
 
     level = 'INFO'
-    tty_level = 'SUCCESS'
+    tty_level = 'INFO'
 
-    logging = {
+    return {
         'version': 1,
         'disable_existing_loggers': True,
         'formatters': {
@@ -61,6 +61,10 @@ def patch_logging(moulinette):
             },
         },
         'handlers': {
+            'api': {
+                'level': level,
+                'class': 'moulinette.interfaces.api.APIQueueHandler',
+            },
             'tty': {
                 'level': tty_level,
                 'class': 'moulinette.interfaces.cli.TTYHandler',
@@ -85,23 +89,57 @@ def patch_logging(moulinette):
         },
     }
 
+
+@pytest.fixture(scope='session', autouse=True)
+def moulinette(tmp_path_factory):
+    import moulinette
+
+    # Can't call the namespace just 'test' because
+    # that would lead to some "import test" not importing the right stuff
+    namespace = "moulitest"
+    tmp_cache = str(tmp_path_factory.mktemp("cache"))
+    tmp_data = str(tmp_path_factory.mktemp("data"))
+    tmp_lib = str(tmp_path_factory.mktemp("lib"))
+    os.environ['MOULINETTE_CACHE_DIR'] = tmp_cache
+    os.environ['MOULINETTE_DATA_DIR'] = tmp_data
+    os.environ['MOULINETTE_LIB_DIR'] = tmp_lib
+    shutil.copytree("./test/actionsmap", "%s/actionsmap" % tmp_data)
+    shutil.copytree("./test/src", "%s/%s" % (tmp_lib, namespace))
+    shutil.copytree("./test/locales", "%s/%s/locales" % (tmp_lib, namespace))
+
+    patch_init(moulinette)
+    patch_translate(moulinette)
+    logging = patch_logging(moulinette)
+
     moulinette.init(
         logging_config=logging,
         _from_source=False
     )
 
-
-@pytest.fixture(scope='session', autouse=True)
-def moulinette():
-    import moulinette
-
-    patch_init(moulinette)
-    patch_translate(moulinette)
-    patch_logging(moulinette)
-
     return moulinette
 
 
+@pytest.fixture
+def moulinette_webapi(moulinette):
+
+    from webtest import TestApp
+    from webtest.app import CookiePolicy
+
+    # Dirty hack needed, otherwise cookies ain't reused between request .. not
+    # sure why :|
+    def return_true(self, cookie, request):
+        return True
+    CookiePolicy.return_ok_secure = return_true
+
+    moulinette_webapi = moulinette.core.init_interface(
+        'api',
+        kwargs={'routes': {}, 'use_websocket': False},
+        actionsmap={'namespaces': ["moulitest"], 'use_cache': True}
+    )
+
+    return TestApp(moulinette_webapi._app)
+
+
 @pytest.fixture
 def test_file(tmp_path):
     test_text = 'foo\nbar\n'

test/locales/en.json

@@ -0,0 +1,3 @@
+{
+    "foo": "bar"
+}

test/src/testauth.py

@@ -0,0 +1,10 @@
+def testauth_none():
+    return "some_data_from_none"
+
+
+def testauth_default():
+    return "some_data_from_default"
+
+
+def testauth_other_profile():
+    return "some_data_from_other_profile"

test/test_actionsmap.py

@@ -83,5 +83,5 @@ def test_actions_map_unknown_authenticator(monkeypatch, tmp_path):
 
     amap = ActionsMap(BaseActionsMapParser)
     with pytest.raises(ValueError) as exception:
-        amap.get_authenticator(profile='unknown')
+        amap.get_authenticator_for_profile('unknown')
     assert 'Unknown authenticator' in str(exception)

test/test_auth.py

@@ -0,0 +1,72 @@
+import os
+
+
+def login(webapi, csrf=False, profile=None, status=200):
+
+    data = {"password": "Yoloswag"}
+    if profile:
+        data["profile"] = profile
+
+    return webapi.post("/login", data,
+                       status=status,
+                       headers=None if csrf else {"X-Requested-With": ""})
+
+
+def test_request_no_auth_needed(moulinette_webapi):
+
+    assert moulinette_webapi.get("/test-auth/none", status=200).text == '"some_data_from_none"'
+
+
+def test_request_with_auth_but_not_logged(moulinette_webapi):
+
+    assert moulinette_webapi.get("/test-auth/default", status=401).text == "Authentication required"
+
+
+def test_login(moulinette_webapi):
+
+    assert login(moulinette_webapi).text == "Logged in"
+
+    assert "session.id" in moulinette_webapi.cookies
+    assert "session.tokens" in moulinette_webapi.cookies
+
+    cache_session_default = os.environ['MOULINETTE_CACHE_DIR'] + "/session/default/"
+    assert moulinette_webapi.cookies["session.id"] + ".asc" in os.listdir(cache_session_default)
+
+
+def test_login_csrf_attempt(moulinette_webapi):
+
+    # C.f.
+    # https://security.stackexchange.com/a/58308
+    # https://stackoverflow.com/a/22533680
+
+    assert "CSRF protection" in login(moulinette_webapi, csrf=True, status=403).text
+    assert not any(c.name == "session.id" for c in moulinette_webapi.cookiejar)
+    assert not any(c.name == "session.tokens" for c in moulinette_webapi.cookiejar)
+
+
+def test_login_then_legit_request_without_cookies(moulinette_webapi):
+
+    login(moulinette_webapi)
+
+    moulinette_webapi.cookiejar.clear()
+
+    moulinette_webapi.get("/test-auth/default", status=401)
+
+
+def test_login_then_legit_request(moulinette_webapi):
+
+    login(moulinette_webapi)
+
+    assert moulinette_webapi.get("/test-auth/default", status=200).text == '"some_data_from_default"'
+
+
+def test_login_then_logout(moulinette_webapi):
+
+    login(moulinette_webapi)
+
+    moulinette_webapi.get("/logout", status=200)
+
+    cache_session_default = os.environ['MOULINETTE_CACHE_DIR'] + "/session/default/"
+    assert not moulinette_webapi.cookies["session.id"] + ".asc" in os.listdir(cache_session_default)
+
+    assert moulinette_webapi.get("/test-auth/default", status=401).text == "Authentication required"

tox.ini

@@ -15,6 +15,10 @@ deps =
   pytest-env >= 0.6.2, < 1.0
   requests >= 2.22.0, < 3.0
   requests-mock >= 1.6.0, < 2.0
+  toml >= 0.10, < 0.11
+  gevent-websocket
+  bottle >= 0.12
+  WebTest >= 2.0, < 2.1
 commands =
     pytest {posargs}