From 6310ef5b6eba5fdf915c5cf39b77249000a4dca7 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 19 Jun 2021 17:16:19 +0200 Subject: [PATCH] [wip] Try to simplify authentication code more, properly differentiate credentials authentication vs. session authentication, trash the 'msignals' mess --- moulinette/__init__.py | 7 +-- moulinette/actionsmap.py | 10 +-- moulinette/authentication.py | 8 +-- moulinette/core.py | 95 ----------------------------- moulinette/interfaces/__init__.py | 22 ------- moulinette/interfaces/api.py | 86 ++++++++------------------ moulinette/interfaces/cli.py | 43 +++++-------- test/src/authenticators/dummy.py | 2 +- test/src/authenticators/yoloswag.py | 2 +- test/test_actionsmap.py | 15 +++-- 10 files changed, 62 insertions(+), 228 deletions(-) diff --git a/moulinette/__init__.py b/moulinette/__init__.py index 625217f8..16e62392 100755 --- a/moulinette/__init__.py +++ b/moulinette/__init__.py @@ -2,7 +2,6 @@ from moulinette.core import ( MoulinetteError, - MoulinetteSignals, Moulinette18n, ) from moulinette.globals import init_moulinette_env @@ -31,14 +30,12 @@ __all__ = [ "api", "cli", "m18n", - "msignals", "env", "init_interface", "MoulinetteError", ] -msignals = MoulinetteSignals() msettings = dict() m18n = Moulinette18n() @@ -116,9 +113,7 @@ def cli(args, top_parser, output_as=None, timeout=None): try: load_only_category = args[0] if args and not args[0].startswith("-") else None - Cli(top_parser=top_parser, load_only_category=load_only_category).run( - args, output_as=output_as, timeout=timeout - ) + Cli(top_parser=top_parser, load_only_category=load_only_category).run(args, output_as=output_as, timeout=timeout) except MoulinetteError as e: import logging diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index 437fa62a..d5952dc0 100644 --- a/moulinette/actionsmap.py +++ b/moulinette/actionsmap.py @@ -11,7 +11,7 @@ from time import time from collections import OrderedDict from importlib import import_module -from moulinette import m18n, msignals +from moulinette import m18n, msettings from moulinette.cache import open_cachefile from moulinette.globals import init_moulinette_env from moulinette.core import ( @@ -98,7 +98,7 @@ class CommentParameter(_ExtraParameter): def __call__(self, message, arg_name, arg_value): if arg_value is None: return - return msignals.display(m18n.n(message)) + return self.iface.display(m18n.n(message)) @classmethod def validate(klass, value, arg_name): @@ -135,7 +135,7 @@ class AskParameter(_ExtraParameter): try: # Ask for the argument value - return msignals.prompt(m18n.n(message)) + return self.iface.prompt(m18n.n(message)) except NotImplementedError: return arg_value @@ -173,7 +173,7 @@ class PasswordParameter(AskParameter): try: # Ask for the password - return msignals.prompt(m18n.n(message), True, True) + return self.iface.prompt(m18n.n(message), True, True) except NotImplementedError: return arg_value @@ -500,7 +500,7 @@ class ActionsMap(object): return authenticator = self.get_authenticator(auth_method) - if not msignals.authenticate(authenticator): + if not msettings['interface'].authenticate(authenticator): raise MoulinetteAuthenticationError("authentication_required_long") def process(self, args, timeout=None, **kwargs): diff --git a/moulinette/authentication.py b/moulinette/authentication.py index 2b3fd9c1..a14a0b8a 100644 --- a/moulinette/authentication.py +++ b/moulinette/authentication.py @@ -5,6 +5,7 @@ import logging import hashlib import hmac +from moulinette.utils.text import random_ascii from moulinette.cache import open_cachefile, get_cachedir, cachefile_exists from moulinette.core import MoulinetteError, MoulinetteAuthenticationError @@ -32,11 +33,11 @@ class BaseAuthenticator(object): # Virtual methods # Each authenticator classes must implement these methods. - def authenticate_credentials(self, credentials=None, store_session=False): + def authenticate_credentials(self, credentials, store_session=False): try: # Attempt to authenticate - self.authenticate(credentials) + self._authenticate_credentials(credentials) except MoulinetteError: raise except Exception as e: @@ -57,7 +58,6 @@ class BaseAuthenticator(object): else: logger.debug("session has been stored") - def _authenticate_credentials(self, credentials=None): """Attempt to authenticate @@ -91,7 +91,7 @@ class BaseAuthenticator(object): with self._open_sessionfile(session_id, "w") as f: f.write(hash_) - def authenticate_session(s_id, s_token): + def authenticate_session(self, s_id, s_token): try: # Attempt to authenticate self._authenticate_session(s_id, s_token) diff --git a/moulinette/core.py b/moulinette/core.py index d82f38b2..f97a83e7 100644 --- a/moulinette/core.py +++ b/moulinette/core.py @@ -270,101 +270,6 @@ class Moulinette18n(object): return self._namespaces[self._current_namespace].key_exists(key) -class MoulinetteSignals(object): - - """Signals connector for the moulinette - - Allow to easily connect signals from the moulinette to handlers. A - signal is emitted by calling the relevant method which call the - handler. - For the moment, a return value can be requested by a signal to its - connected handler - make them not real-signals. - - Keyword arguments: - - kwargs -- A dict of {signal: handler} to connect - - """ - - def __init__(self, **kwargs): - # Initialize handlers - for s in self.signals: - self.clear_handler(s) - - # Iterate over signals to connect - for s, h in kwargs.items(): - self.set_handler(s, h) - - def set_handler(self, signal, handler): - """Set the handler for a signal""" - if signal not in self.signals: - logger.error("unknown signal '%s'", signal) - return - setattr(self, "_%s" % signal, handler) - - def clear_handler(self, signal): - """Clear the handler of a signal""" - if signal not in self.signals: - logger.error("unknown signal '%s'", signal) - return - setattr(self, "_%s" % signal, self._notimplemented) - - # Signals definitions - - """The list of available signals""" - signals = {"authenticate", "prompt", "display"} - - def authenticate(self, authenticator): - if hasattr(authenticator, "is_authenticated"): - return authenticator.is_authenticated - # self._authenticate corresponds to the stuff defined with - # msignals.set_handler("authenticate", ...) per interface... - return self._authenticate(authenticator) - - def prompt(self, message, is_password=False, confirm=False, color="blue"): - """Prompt for a value - - Prompt the interface for a parameter value which is a password - if 'is_password' and must be confirmed if 'confirm'. - Is is called when a parameter value is needed and when the - current interface should allow user interaction (e.g. to parse - extra parameter 'ask' in the cli). - - Keyword arguments: - - message -- The message to display - - is_password -- True if the parameter is a password - - confirm -- True if the value must be confirmed - - color -- Color to use for the prompt ... - - Returns: - The collected value - - """ - return self._prompt(message, is_password, confirm, color=color) - - def display(self, message, style="info"): - """Display a message - - Display a message with a given style to the user. - It is called when a message should be printed to the user if the - current interface allows user interaction (e.g. print a success - message to the user). - - Keyword arguments: - - message -- The message to display - - style -- The type of the message. Possible values are: - info, success, warning - - """ - try: - self._display(message, style) - except NotImplementedError: - pass - - @staticmethod - def _notimplemented(*args, **kwargs): - raise NotImplementedError("this signal is not handled") - - # Moulinette core classes ---------------------------------------------- diff --git a/moulinette/interfaces/__init__.py b/moulinette/interfaces/__init__.py index e53c5f43..b5385ded 100644 --- a/moulinette/interfaces/__init__.py +++ b/moulinette/interfaces/__init__.py @@ -37,7 +37,6 @@ class BaseActionsMapParser(object): def __init__(self, parent=None, **kwargs): if not parent: logger.debug("initializing base actions map parser for %s", self.interface) - msettings["interface"] = self.interface # Virtual properties # Each parser classes must implement these properties. @@ -167,27 +166,6 @@ class BaseActionsMapParser(object): return namespace -class BaseInterface(object): - - """Moulinette's base Interface - - Each interfaces must implement an Interface class derived from this - class which must overrides virtual properties and methods. - It is used to provide a user interface for an actions map. - - Keyword arguments: - - actionsmap -- The ActionsMap instance to connect to - - """ - - # TODO: Add common interface methods and try to standardize default ones - - def __init__(self, actionsmap): - raise NotImplementedError( - "derived class '%s' must override this method" % self.__class__.__name__ - ) - - # Argument parser ------------------------------------------------------ diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 544a9f30..818cdb2a 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -13,12 +13,11 @@ from geventwebsocket import WebSocketError from bottle import request, response, Bottle, HTTPResponse from bottle import abort -from moulinette import msignals, m18n, env +from moulinette import m18n, env, msettings from moulinette.actionsmap import ActionsMap from moulinette.core import MoulinetteError, MoulinetteValidationError from moulinette.interfaces import ( BaseActionsMapParser, - BaseInterface, ExtendedArgumentParser, ) from moulinette.utils import log @@ -226,9 +225,6 @@ class _ActionsMapPlugin(object): api = 2 def __init__(self, actionsmap, log_queues={}): - # Connect signals to handlers - msignals.set_handler("authenticate", self._do_authenticate) - msignals.set_handler("display", self._do_display) self.actionsmap = actionsmap self.log_queues = log_queues @@ -253,6 +249,10 @@ class _ActionsMapPlugin(object): except KeyError: raise HTTPResponse("Missing credentials parameter", 400) + # Apparently even if the key doesn't exists, request.POST.foobar just returns empty string... + if not kwargs["credentials"]: + raise HTTPResponse("Missing credentials parameter", 400) + kwargs["profile"] = request.POST.get( "profile", self.actionsmap.default_authentication ) @@ -361,12 +361,6 @@ class _ActionsMapPlugin(object): """ authenticator = self.actionsmap.get_authenticator(profile) - ################################################################## - # Case 1 : credentials were provided # - # We want to validate that the credentials are right # - # Then save the session id/token, and return then in the cookies # - ################################################################## - try: s_id, s_token = authenticator.authenticate_credentials(credentials, store_session=True) except MoulinetteError as e: @@ -398,14 +392,8 @@ class _ActionsMapPlugin(object): ) return m18n.g("logged_in") - # This is called before each time a route is going to be processed - def _do_authenticate(self, authenticator): - """Process the authentication - - Handle the core.MoulinetteSignals.authenticate signal. - - """ + def authenticate(self, authenticator): s_id = request.get_cookie("session.id") try: @@ -419,7 +407,6 @@ class _ActionsMapPlugin(object): else: authenticator.authenticate_session(s_id, s_token) - def logout(self, profile): """Log out from an authenticator profile @@ -465,9 +452,7 @@ class _ActionsMapPlugin(object): """Listen to the messages WebSocket stream Retrieve the WebSocket stream and send to it each messages displayed by - the core.MoulinetteSignals.display signal. They are JSON encoded as a - dict { style: message }. - + the display method. They are JSON encoded as a dict { style: message }. """ s_id = request.get_cookie("session.id") try: @@ -536,14 +521,8 @@ class _ActionsMapPlugin(object): else: queue.put(StopIteration) - # Signals handlers + def display(self, message, style): - def _do_display(self, message, style): - """Display a message - - Handle the core.MoulinetteSignals.display signal. - - """ s_id = request.get_cookie("session.id") try: queue = self.log_queues[s_id] @@ -557,6 +536,8 @@ class _ActionsMapPlugin(object): # populate the new message in the queue sleep(0) + def prompt(self, *args, **kwargs): + raise NotImplementedError("Prompt is not implemented for this interface") # HTTP Responses ------------------------------------------------------- @@ -734,7 +715,7 @@ class ActionsMapParser(BaseActionsMapParser): return key -class Interface(BaseInterface): +class Interface: """Application Programming Interface for the moulinette @@ -749,15 +730,16 @@ class Interface(BaseInterface): """ - def __init__(self, routes={}, log_queues=None): + type = "api" + + def __init__(self, routes={}): actionsmap = ActionsMap(ActionsMapParser()) # Attempt to retrieve log queues from an APIQueueHandler - if log_queues is None: - handler = log.getHandlersByClass(APIQueueHandler, limit=1) - if handler: - log_queues = handler.queues + handler = log.getHandlersByClass(APIQueueHandler, limit=1) + if handler: + log_queues = handler.queues # TODO: Return OK to 'OPTIONS' xhr requests (l173) app = Bottle(autojson=True) @@ -786,11 +768,12 @@ class Interface(BaseInterface): app.install(filter_csrf) app.install(apiheader) app.install(api18n) - app.install(_ActionsMapPlugin(actionsmap, log_queues)) + actionsmapplugin = _ActionsMapPlugin(actionsmap, log_queues) + app.install(actionsmapplugin) - # Append default routes - # app.route(['/api', '/api/'], method='GET', - # callback=self.doc, skip=['actionsmap']) + self.authenticate = actionsmapplugin.authenticate + self.display = actionsmapplugin.display + self.prompt = actionsmapplugin.prompt # Append additional routes # TODO: Add optional authentication to those routes? @@ -799,6 +782,8 @@ class Interface(BaseInterface): self._app = app + msettings["interface"] = self + def run(self, host="localhost", port=80): """Run the moulinette @@ -810,6 +795,7 @@ class Interface(BaseInterface): - port -- Server port to bind to """ + logger.debug( "starting the server instance in %s:%d", host, @@ -832,25 +818,3 @@ class Interface(BaseInterface): if e.args[0] == errno.EADDRINUSE: raise MoulinetteError("server_already_running") raise MoulinetteError(error_message) - - # Routes handlers - - def doc(self, category=None): - """ - Get API documentation for a category (all by default) - - Keyword argument: - category -- Name of the category - - """ - DATA_DIR = env()["DATA_DIR"] - - if category is None: - with open("%s/../doc/resources.json" % DATA_DIR) as f: - return f.read() - - try: - with open("%s/../doc/%s.json" % (DATA_DIR, category)) as f: - return f.read() - except IOError: - return None diff --git a/moulinette/interfaces/cli.py b/moulinette/interfaces/cli.py index ad2a77b8..07c3d661 100644 --- a/moulinette/interfaces/cli.py +++ b/moulinette/interfaces/cli.py @@ -11,12 +11,11 @@ from datetime import date, datetime import argcomplete -from moulinette import msignals, m18n +from moulinette import m18n, msettings from moulinette.actionsmap import ActionsMap from moulinette.core import MoulinetteError, MoulinetteValidationError from moulinette.interfaces import ( BaseActionsMapParser, - BaseInterface, ExtendedArgumentParser, ) from moulinette.utils import log @@ -450,7 +449,7 @@ class ActionsMapParser(BaseActionsMapParser): return ret -class Interface(BaseInterface): +class Interface: """Command-line Interface for the moulinette @@ -462,22 +461,20 @@ class Interface(BaseInterface): """ + type = "cli" + def __init__(self, top_parser=None, load_only_category=None): # Set user locale m18n.set_locale(get_locale()) - # Connect signals to handlers - msignals.set_handler("display", self._do_display) - if os.isatty(1): - msignals.set_handler("authenticate", self._do_authenticate) - msignals.set_handler("prompt", self._do_prompt) - self.actionsmap = ActionsMap( ActionsMapParser(top_parser=top_parser), load_only_category=load_only_category, ) + msettings["interface"] = self + def run(self, args, output_as=None, timeout=None): """Run the moulinette @@ -493,15 +490,13 @@ class Interface(BaseInterface): - timeout -- Number of seconds before this command will timeout because it can't acquire the lock (meaning that another command is currently running), by default there is no timeout and the command will wait until it can get the lock """ + if output_as and output_as not in ["json", "plain", "none"]: raise MoulinetteValidationError("invalid_usage") # auto-complete argcomplete.autocomplete(self.actionsmap.parser._parser) - # Set handler for authentication - msignals.set_handler("authenticate", self._do_authenticate) - try: ret = self.actionsmap.process(args, timeout=timeout) except (KeyboardInterrupt, EOFError): @@ -524,32 +519,26 @@ class Interface(BaseInterface): else: print(ret) - # Signals handlers - - def _do_authenticate(self, authenticator): - """Process the authentication - - Handle the core.MoulinetteSignals.authenticate signal. - - """ + def authenticate(self, authenticator): # Hmpf we have no-use case in yunohost anymore where we need to auth # because everything is run as root ... # I guess we could imagine some yunohost-independant use-case where # moulinette is used to create a CLI for non-root user that needs to # auth somehow but hmpf -.- msg = m18n.g("password") - credentials = self._do_prompt(msg, True, False, color="yellow") + credentials = self.prompt(msg, True, False, color="yellow") return authenticator.authenticate_credentials(credentials=credentials) - def _do_prompt(self, message, is_password, confirm, color="blue"): + def prompt(self, message, is_password, confirm, color="blue"): """Prompt for a value - Handle the core.MoulinetteSignals.prompt signal. - Keyword arguments: - color -- The color to use for prompting message - """ + + if not os.isatty(1): + raise MoulinetteError("No a tty, can't do interactive prompts", raw_msg=True) + if is_password: prompt = lambda m: getpass.getpass(colorize(m18n.g("colon", m), color)) else: @@ -563,11 +552,9 @@ class Interface(BaseInterface): return value - def _do_display(self, message, style): + def display(self, message, style): """Display a message - Handle the core.MoulinetteSignals.display signal. - """ if style == "success": print("{} {}".format(colorize(m18n.g("success"), "green"), message)) diff --git a/test/src/authenticators/dummy.py b/test/src/authenticators/dummy.py index 8de685be..e0a23401 100644 --- a/test/src/authenticators/dummy.py +++ b/test/src/authenticators/dummy.py @@ -18,7 +18,7 @@ class Authenticator(BaseAuthenticator): def __init__(self, *args, **kwargs): pass - def authenticate(self, credentials=None): + def _authenticate_credentials(self, credentials=None): if not credentials == self.name: raise MoulinetteError("invalid_password") diff --git a/test/src/authenticators/yoloswag.py b/test/src/authenticators/yoloswag.py index a632d1b8..a746f599 100644 --- a/test/src/authenticators/yoloswag.py +++ b/test/src/authenticators/yoloswag.py @@ -18,7 +18,7 @@ class Authenticator(BaseAuthenticator): def __init__(self, *args, **kwargs): pass - def authenticate(self, credentials=None): + def _authenticate_credentials(self, credentials=None): if not credentials == self.name: raise MoulinetteError("invalid_password") diff --git a/test/test_actionsmap.py b/test/test_actionsmap.py index 6768f066..57348fc5 100644 --- a/test/test_actionsmap.py +++ b/test/test_actionsmap.py @@ -17,7 +17,12 @@ from moulinette import m18n @pytest.fixture def iface(): - return "iface" + class DummyInterface: + + def prompt(): + pass + + return DummyInterface() def test_comment_parameter_bad_bool_value(iface, caplog): @@ -67,10 +72,10 @@ def test_ask_parameter(iface, mocker): arg = ask("foobar", "a", "a") assert arg == "a" - from moulinette.core import Moulinette18n, MoulinetteSignals + from moulinette.core import Moulinette18n mocker.patch.object(Moulinette18n, "n", return_value="awesome_test") - mocker.patch.object(MoulinetteSignals, "prompt", return_value="awesome_test") + mocker.patch.object(iface, "prompt", return_value="awesome_test") arg = ask("foobar", "a", None) assert arg == "awesome_test" @@ -80,10 +85,10 @@ def test_password_parameter(iface, mocker): arg = ask("foobar", "a", "a") assert arg == "a" - from moulinette.core import Moulinette18n, MoulinetteSignals + from moulinette.core import Moulinette18n mocker.patch.object(Moulinette18n, "n", return_value="awesome_test") - mocker.patch.object(MoulinetteSignals, "prompt", return_value="awesome_test") + mocker.patch.object(iface, "prompt", return_value="awesome_test") arg = ask("foobar", "a", None) assert arg == "awesome_test"