diff --git a/README.md b/README.md index 01ee55cc..b6f8b03d 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,6 @@ Requirements * Python 2.7 * python-bottle (>= 0.10) -* python-gnupg (>= 0.3) * python-ldap (>= 2.4) * PyYAML diff --git a/data/actionsmap/test.yml b/data/actionsmap/test.yml deleted file mode 100644 index 36d1a270..00000000 --- a/data/actionsmap/test.yml +++ /dev/null @@ -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 diff --git a/data/moulinette_cli b/data/moulinette_cli deleted file mode 100644 index 15ac21f2..00000000 --- a/data/moulinette_cli +++ /dev/null @@ -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 diff --git a/debian/control b/debian/control index 55d63073..b98a13e5 100644 --- a/debian/control +++ b/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, diff --git a/doc/requirements.txt b/doc/requirements.txt index 142c12e7..c41bd5e9 100644 --- a/doc/requirements.txt +++ b/doc/requirements.txt @@ -1,5 +1,4 @@ sphinx -gnupg mock pyyaml toml diff --git a/locales/en.json b/locales/en.json index 36d93a8f..0b55cfc5 100644 --- a/locales/en.json +++ b/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", diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index fd88ec72..b902e6d1 100644 --- a/moulinette/actionsmap.py +++ b/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'): - """Get an authenticator instance + def get_authenticator_for_profile(self, auth_profile): - 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 - - """ + # Fetch the configuration for the authenticator module as defined in the actionmap 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)) diff --git a/moulinette/authenticators/__init__.py b/moulinette/authenticators/__init__.py index afbdc87e..6622baf0 100644 --- a/moulinette/authenticators/__init__.py +++ b/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 - s_id, s_hash = token - except TypeError as e: - logger.error("unable to extract token parts from '%s' because '%s'", token, e) - 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) + # Attempt to authenticate + self.authenticate(password) + except MoulinetteError: + raise except Exception as e: - import traceback - traceback.print_exc() - logger.exception("unable to store session because %s", e) + logger.exception("authentication (name: '%s', vendor: '%s') fails because '%s'", + self.name, self.vendor, 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): - """Store a session and its associated password""" - 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) ?" + def _store_session(self, session_id, session_token): + """Store a session to be able to use it later to reauthenticate""" + # 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): - """Retrieve a session and return its associated password""" + def _authenticate_session(self, session_id, session_token): + """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 decrypted.ok is not True: - error_message = "unable to decrypt password for the session: %s" % decrypted.status - logger.error(error_message) - raise MoulinetteError('unable_retrieve_session', exception=error_message) - return decrypted.data + if not hmac.compare_digest(hash_, stored_hash): + raise MoulinetteError('invalid_token') + else: + return + + 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 diff --git a/moulinette/authenticators/dummy.py b/moulinette/authenticators/dummy.py new file mode 100644 index 00000000..0fa91958 --- /dev/null +++ b/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 diff --git a/moulinette/authenticators/ldap.py b/moulinette/authenticators/ldap.py index f3625efc..07593483 100644 --- a/moulinette/authenticators/ldap.py +++ b/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 - self.basedn = base_dn - if user_rdn: - self.userdn = user_rdn - if 'cn=external,cn=auth' in user_rdn: + if self.userdn: + if 'cn=external,cn=auth' in self.userdn: 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 - self._ensure_password_uses_strong_hash(password) + if who[3:] != self.userdn: + 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? diff --git a/moulinette/cache.py b/moulinette/cache.py index 4da4ece8..dad7b93b 100644 --- a/moulinette/cache.py +++ b/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 - kwargs['make_dir'] = kwargs.get('make_dir', - True if mode[0] == 'w' else False) - cache_dir = get_cachedir(**kwargs) + cache_dir = get_cachedir(subdir, make_dir=True if mode[0] == 'w' else False) file_path = os.path.join(cache_dir, filename) return open(file_path, mode) diff --git a/moulinette/core.py b/moulinette/core.py index e1c9edeb..a5ac788f 100644 --- a/moulinette/core.py +++ b/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: - logger.exception("unable to load interface '%s'", name) + except ImportError as e: + 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): diff --git a/moulinette/interfaces/__init__.py b/moulinette/interfaces/__init__.py index a42683b3..54ad4e50 100644 --- a/moulinette/interfaces/__init__.py +++ b/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.core import (init_authenticator, MoulinetteError) +from moulinette import msettings, m18n +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) - else: - return self._format_conf(name, value) + return self.global_conf[name] 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 - # (identifier, configuration, parameters) with - # - identifier: the authenticator vendor and its - # profile name as a 2-tuple - # - 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', {})) + auths[auth_name] = {'name': auth_name, + 'vendor': auth_conf.get('vendor'), + 'parameters': auth_conf.get('parameters', {}), + 'extra': {'help': auth_conf.get('help', None)}} 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): diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 79a31f4a..fba1725d 100644 --- a/moulinette/interfaces/api.py +++ b/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') - except KeyError: - pass + kwargs['profile'] = request.POST.get('profile', "default") 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) - auth(password, token=(s_id, s_hash)) + authenticator = self.actionsmap.get_authenticator_for_profile(profile) + 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) - clean_session(s_id) + response.set_cookie('session.tokens', '', max_age=-1) 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') - else: - msg = m18n.g('authentication_profile_required', - profile=authenticator.name) + msg = m18n.g('authentication_required') 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 - tid, parser = self._parsers[route] + # Retrieve the parser for the 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) diff --git a/moulinette/interfaces/cli.py b/moulinette/interfaces/cli.py index 70e2833a..80eb69d2 100644 --- a/moulinette/interfaces/cli.py +++ b/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')) diff --git a/test/actionsmap/moulitest.yml b/test/actionsmap/moulitest.yml new file mode 100644 index 00000000..a14bd67a --- /dev/null +++ b/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 diff --git a/test/conftest.py b/test/conftest.py index b5669aac..a450e638 100644 --- a/test/conftest.py +++ b/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' diff --git a/test/locales/en.json b/test/locales/en.json new file mode 100644 index 00000000..e63d37b6 --- /dev/null +++ b/test/locales/en.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/test/src/__init__.py b/test/src/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/src/testauth.py b/test/src/testauth.py new file mode 100644 index 00000000..0a28f9c4 --- /dev/null +++ b/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" diff --git a/test/test_actionsmap.py b/test/test_actionsmap.py index 82f153a8..ec44a926 100644 --- a/test/test_actionsmap.py +++ b/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) diff --git a/test/test_auth.py b/test/test_auth.py new file mode 100644 index 00000000..df0f749f --- /dev/null +++ b/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" diff --git a/tox.ini b/tox.ini index 895a8f06..feeb4faa 100644 --- a/tox.ini +++ b/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}