From 2fc9611b53d85b1140ceeee21e37c3554577882d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 22 Dec 2021 19:06:33 +0100 Subject: [PATCH 1/6] api: Move cookie session management logic to the authenticator for more flexibility --- moulinette/actionsmap.py | 3 +- moulinette/interfaces/api.py | 87 ++++++++++++------------------------ 2 files changed, 31 insertions(+), 59 deletions(-) diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index 358575de..8f68a2f6 100644 --- a/moulinette/actionsmap.py +++ b/moulinette/actionsmap.py @@ -10,6 +10,7 @@ from typing import List, Optional from time import time from collections import OrderedDict from importlib import import_module +from functools import cache from moulinette import m18n, Moulinette from moulinette.core import ( @@ -456,6 +457,7 @@ class ActionsMap(object): self.extraparser = ExtraArgumentParser(top_parser.interface) self.parser = self._construct_parser(actionsmap, top_parser) + @cache def get_authenticator(self, auth_method): if auth_method == "default": @@ -616,7 +618,6 @@ class ActionsMap(object): _global = actionsmap.pop("_global", {}) self.namespace = _global["namespace"] - self.cookie_name = _global["cookie_name"] self.default_authentication = _global["authentication"][ interface_type ] diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index b51e79d2..3f0c881f 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -85,9 +85,15 @@ class APIQueueHandler(logging.Handler): def __init__(self): logging.Handler.__init__(self) self.queues = LogQueues() + # actionsmap is actually set during the interface's init ... + self.actionsmap = None def emit(self, record): - s_id = Session.get_infos(raise_if_no_session_exists=False)["id"] + + profile = request.params.get("profile", self.actionsmap.default_authentication) + authenticator = self.actionsmap.get_authenticator(profile) + + s_id = authenticator.get_session_cookie(raise_if_no_session_exists=False)["id"] try: queue = self.queues[s_id] except KeyError: @@ -234,47 +240,6 @@ class _HTTPArgumentParser(object): raise MoulinetteValidationError(message, raw_msg=True) -class Session: - - secret = random_ascii() - cookie_name = None # This is later set to the actionsmap name - - def set_infos(infos): - - assert isinstance(infos, dict) - - response.set_cookie( - f"session.{Session.cookie_name}", - infos, - secure=True, - secret=Session.secret, - httponly=True, - # samesite="strict", # Bottle 0.12 doesn't support samesite, to be added in next versions - ) - - def get_infos(raise_if_no_session_exists=True): - - try: - infos = request.get_cookie( - f"session.{Session.cookie_name}", secret=Session.secret, default={} - ) - except Exception: - if not raise_if_no_session_exists: - return {"id": random_ascii()} - raise MoulinetteAuthenticationError("unable_authenticate") - - if "id" not in infos: - infos["id"] = random_ascii() - - return infos - - @staticmethod - def delete_infos(): - - response.set_cookie(f"session.{Session.cookie_name}", "", max_age=-1) - response.delete_cookie(f"session.{Session.cookie_name}") - - class _ActionsMapPlugin(object): """Actions map Bottle Plugin @@ -294,7 +259,6 @@ class _ActionsMapPlugin(object): self.actionsmap = actionsmap self.log_queues = log_queues - Session.cookie_name = actionsmap.cookie_name def setup(self, app): """Setup plugin on the application @@ -398,13 +362,10 @@ class _ActionsMapPlugin(object): credentials = request.params["credentials"] profile = request.params.get("profile", self.actionsmap.default_authentication) - authenticator = self.actionsmap.get_authenticator(profile) try: - auth_info = authenticator.authenticate_credentials(credentials) - session_infos = Session.get_infos(raise_if_no_session_exists=False) - session_infos[profile] = auth_info + auth_infos = authenticator.authenticate_credentials(credentials) except MoulinetteError as e: try: self.logout() @@ -412,18 +373,14 @@ class _ActionsMapPlugin(object): pass raise HTTPResponse(e.strerror, 401) else: - Session.set_infos(session_infos) + authenticator.set_session_cookie(auth_infos) return m18n.g("logged_in") # This is called before each time a route is going to be processed def authenticate(self, authenticator): try: - session_infos = Session.get_infos()[authenticator.name] - - # Here, maybe we want to re-authenticate the session via the authenticator - # For example to check that the username authenticated is still in the admin group... - + session_infos = authenticator.get_session_cookie() except Exception: msg = m18n.g("authentication_required") raise HTTPResponse(msg, 401) @@ -431,13 +388,17 @@ class _ActionsMapPlugin(object): return session_infos def logout(self): + + profile = request.params.get("profile", self.actionsmap.default_authentication) + authenticator = self.actionsmap.get_authenticator(profile) + try: - Session.get_infos() + authenticator.get_session_cookie() except KeyError: raise HTTPResponse(m18n.g("not_logged_in"), 401) else: # Delete cookie and clean the session - Session.delete_infos() + authenticator.delete_session_cookie() return m18n.g("logged_out") def messages(self): @@ -446,7 +407,11 @@ class _ActionsMapPlugin(object): Retrieve the WebSocket stream and send to it each messages displayed by the display method. They are JSON encoded as a dict { style: message }. """ - s_id = Session.get_infos()["id"] + + profile = request.params.get("profile", self.actionsmap.default_authentication) + authenticator = self.actionsmap.get_authenticator(profile) + + s_id = authenticator.get_session_cookie()["id"] try: queue = self.log_queues[s_id] except KeyError: @@ -514,8 +479,10 @@ class _ActionsMapPlugin(object): UPLOAD_DIR = None # Close opened WebSocket by putting StopIteration in the queue + profile = request.params.get("profile", self.actionsmap.default_authentication) + authenticator = self.actionsmap.get_authenticator(profile) try: - s_id = Session.get_infos()["id"] + s_id = authenticator.get_session_cookie()["id"] queue = self.log_queues[s_id] except KeyError: pass @@ -524,7 +491,10 @@ class _ActionsMapPlugin(object): def display(self, message, style="info"): - s_id = Session.get_infos(raise_if_no_session_exists=False)["id"] + profile = request.params.get("profile", self.actionsmap.default_authentication) + authenticator = self.actionsmap.get_authenticator(profile) + s_id = authenticator.get_session_cookie(raise_if_no_session_exists=False)["id"] + try: queue = self.log_queues[s_id] except KeyError: @@ -742,6 +712,7 @@ class Interface: handler = log.getHandlersByClass(APIQueueHandler, limit=1) if handler: log_queues = handler.queues + handler.actionsmap = actionsmap # TODO: Return OK to 'OPTIONS' xhr requests (l173) app = Bottle(autojson=True) From ee1e63c7a114b7ca51692fc0d720c5a21a7fc00d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 22 Dec 2021 19:22:49 +0100 Subject: [PATCH 2/6] Propagate changes on tests --- test/actionsmap/moulitest.yml | 1 - test/src/authenticators/dummy.py | 54 +++++++++++++++++++++++++++-- test/src/authenticators/yoloswag.py | 52 ++++++++++++++++++++++++++- test/test_auth.py | 10 +++--- 4 files changed, 108 insertions(+), 9 deletions(-) diff --git a/test/actionsmap/moulitest.yml b/test/actionsmap/moulitest.yml index 88ddeae0..9802037d 100644 --- a/test/actionsmap/moulitest.yml +++ b/test/actionsmap/moulitest.yml @@ -4,7 +4,6 @@ ############################# _global: namespace: moulitest - cookie_name: moulitest authentication: api: dummy cli: dummy diff --git a/test/src/authenticators/dummy.py b/test/src/authenticators/dummy.py index 904d6ed4..7590500d 100644 --- a/test/src/authenticators/dummy.py +++ b/test/src/authenticators/dummy.py @@ -1,13 +1,16 @@ # -*- coding: utf-8 -*- import logging -from moulinette.core import MoulinetteError +from moulinette.utils.text import random_ascii +from moulinette.core import MoulinetteError, MoulinetteAuthenticationError from moulinette.authentication import BaseAuthenticator -logger = logging.getLogger("moulinette.authenticator.dummy") +logger = logging.getLogger("moulinette.authenticator.yoloswag") # Dummy authenticator implementation +session_secret = random_ascii() + class Authenticator(BaseAuthenticator): @@ -24,3 +27,50 @@ class Authenticator(BaseAuthenticator): raise MoulinetteError("invalid_password", raw_msg=True) return + + def set_session_cookie(self, infos): + + from bottle import response + + assert isinstance(infos, dict) + + # This allows to generate a new session id or keep the existing one + current_infos = self.get_session_cookie(raise_if_no_session_exists=False) + new_infos = {"id": current_infos["id"]} + new_infos.update(infos) + + response.set_cookie( + "moulitest", + new_infos, + secure=True, + secret=session_secret, + httponly=True, + # samesite="strict", # Bottle 0.12 doesn't support samesite, to be added in next versions + ) + + def get_session_cookie(self, raise_if_no_session_exists=True): + + from bottle import request + + try: + infos = request.get_cookie( + "moulitest", secret=session_secret, default={} + ) + except Exception: + if not raise_if_no_session_exists: + return {"id": random_ascii()} + raise MoulinetteAuthenticationError("unable_authenticate") + + if "id" not in infos: + infos["id"] = random_ascii() + + return infos + + @staticmethod + def delete_session_cookie(self): + + from bottle import response + + response.set_cookie("moulitest", "", max_age=-1) + response.delete_cookie("moulitest") + diff --git a/test/src/authenticators/yoloswag.py b/test/src/authenticators/yoloswag.py index d199f121..8607be40 100644 --- a/test/src/authenticators/yoloswag.py +++ b/test/src/authenticators/yoloswag.py @@ -1,13 +1,16 @@ # -*- coding: utf-8 -*- import logging -from moulinette.core import MoulinetteError +from moulinette.utils.text import random_ascii +from moulinette.core import MoulinetteError, MoulinetteAuthenticationError from moulinette.authentication import BaseAuthenticator logger = logging.getLogger("moulinette.authenticator.yoloswag") # Dummy authenticator implementation +session_secret = random_ascii() + class Authenticator(BaseAuthenticator): @@ -24,3 +27,50 @@ class Authenticator(BaseAuthenticator): raise MoulinetteError("invalid_password", raw_msg=True) return + + def set_session_cookie(self, infos): + + from bottle import response + + assert isinstance(infos, dict) + + # This allows to generate a new session id or keep the existing one + current_infos = self.get_session_cookie(raise_if_no_session_exists=False) + new_infos = {"id": current_infos["id"]} + new_infos.update(infos) + + response.set_cookie( + "moulitest", + new_infos, + secure=True, + secret=session_secret, + httponly=True, + # samesite="strict", # Bottle 0.12 doesn't support samesite, to be added in next versions + ) + + def get_session_cookie(self, raise_if_no_session_exists=True): + + from bottle import request + + try: + infos = request.get_cookie( + "moulitest", secret=session_secret, default={} + ) + except Exception: + if not raise_if_no_session_exists: + return {"id": random_ascii()} + raise MoulinetteAuthenticationError("unable_authenticate") + + if "id" not in infos: + infos["id"] = random_ascii() + + return infos + + @staticmethod + def delete_session_cookie(self): + + from bottle import response + + response.set_cookie("moulitest", "", max_age=-1) + response.delete_cookie("moulitest") + diff --git a/test/test_auth.py b/test/test_auth.py index a245cc58..5563a92e 100644 --- a/test/test_auth.py +++ b/test/test_auth.py @@ -66,7 +66,7 @@ class TestAuthAPI: def test_login(self, moulinette_webapi): assert self.login(moulinette_webapi).text == "Logged in" - assert "session.moulitest" in moulinette_webapi.cookies + assert "moulitest" in moulinette_webapi.cookies def test_login_bad_password(self, moulinette_webapi): assert ( @@ -74,7 +74,7 @@ class TestAuthAPI: == "invalid_password" ) - assert "session.moulitest" not in moulinette_webapi.cookies + assert "moulitest" not in moulinette_webapi.cookies def test_login_csrf_attempt(self, moulinette_webapi): # C.f. @@ -86,7 +86,7 @@ class TestAuthAPI: in self.login(moulinette_webapi, csrf=True, status=403).text ) assert not any( - c.name == "session.moulitest" for c in moulinette_webapi.cookiejar + c.name == "moulitest" for c in moulinette_webapi.cookiejar ) def test_login_then_legit_request_without_cookies(self, moulinette_webapi): @@ -99,7 +99,7 @@ class TestAuthAPI: def test_login_then_legit_request(self, moulinette_webapi): self.login(moulinette_webapi) - assert "session.moulitest" in moulinette_webapi.cookies + assert "moulitest" in moulinette_webapi.cookies assert ( moulinette_webapi.get("/test-auth/default", status=200).text @@ -124,7 +124,7 @@ class TestAuthAPI: def test_login_other_profile(self, moulinette_webapi): self.login(moulinette_webapi, profile="yoloswag", password="yoloswag") - assert "session.moulitest" in moulinette_webapi.cookies + assert "moulitest" in moulinette_webapi.cookies def test_login_wrong_profile(self, moulinette_webapi): self.login(moulinette_webapi) From 964483b23bec47557d65bfa97da17ba869c7a49d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Dec 2021 16:57:43 +0100 Subject: [PATCH 3/6] Fix issues in previous commits regarding authentication mecanism --- moulinette/interfaces/api.py | 7 +++++++ test/src/authenticators/dummy.py | 3 +++ test/src/authenticators/yoloswag.py | 3 +++ 3 files changed, 13 insertions(+) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 3f0c881f..eeaeaa40 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -90,6 +90,11 @@ class APIQueueHandler(logging.Handler): def emit(self, record): + # Prevent triggering this function while moulinette + # is being initialized with --debug + if not self.actionsmap or len(request.cookies) == 0: + return + profile = request.params.get("profile", self.actionsmap.default_authentication) authenticator = self.actionsmap.get_authenticator(profile) @@ -484,6 +489,8 @@ class _ActionsMapPlugin(object): try: s_id = authenticator.get_session_cookie()["id"] queue = self.log_queues[s_id] + except MoulinetteAuthenticationError: + pass except KeyError: pass else: diff --git a/test/src/authenticators/dummy.py b/test/src/authenticators/dummy.py index 7590500d..b1eec660 100644 --- a/test/src/authenticators/dummy.py +++ b/test/src/authenticators/dummy.py @@ -61,6 +61,9 @@ class Authenticator(BaseAuthenticator): return {"id": random_ascii()} raise MoulinetteAuthenticationError("unable_authenticate") + if not infos and raise_if_no_session_exists: + raise MoulinetteAuthenticationError("unable_authenticate") + if "id" not in infos: infos["id"] = random_ascii() diff --git a/test/src/authenticators/yoloswag.py b/test/src/authenticators/yoloswag.py index 8607be40..2a549526 100644 --- a/test/src/authenticators/yoloswag.py +++ b/test/src/authenticators/yoloswag.py @@ -61,6 +61,9 @@ class Authenticator(BaseAuthenticator): return {"id": random_ascii()} raise MoulinetteAuthenticationError("unable_authenticate") + if not infos and raise_if_no_session_exists: + raise MoulinetteAuthenticationError("unable_authenticate") + if "id" not in infos: infos["id"] = random_ascii() From ef08a8be5339b99fcab94fd6abd21705ef0c7e0d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 25 Dec 2021 15:41:09 +0100 Subject: [PATCH 4/6] actionsmap: add global flag to disable cache and lock --- moulinette/actionsmap.py | 6 +++++- moulinette/core.py | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index 8f68a2f6..0b1eb9d7 100644 --- a/moulinette/actionsmap.py +++ b/moulinette/actionsmap.py @@ -414,6 +414,9 @@ class ActionsMap(object): # Read actions map from yaml file actionsmap = read_yaml(actionsmap_yml) + if not actionsmap["_global"].get("cache", True): + return actionsmap + # Delete old cache files for old_cache in glob.glob(f"{actionsmap_yml_dir}/.{actionsmap_yml_file}.*.pkl"): os.remove(old_cache) @@ -536,7 +539,7 @@ class ActionsMap(object): full_action_name = "%s.%s.%s" % (namespace, category, action) # Lock the moulinette for the namespace - with MoulinetteLock(namespace, timeout): + with MoulinetteLock(namespace, timeout, self.enable_lock): start = time() try: mod = __import__( @@ -618,6 +621,7 @@ class ActionsMap(object): _global = actionsmap.pop("_global", {}) self.namespace = _global["namespace"] + self.enable_lock = _global.get("lock", True) self.default_authentication = _global["authentication"][ interface_type ] diff --git a/moulinette/core.py b/moulinette/core.py index 785cbeec..65eb396c 100644 --- a/moulinette/core.py +++ b/moulinette/core.py @@ -291,10 +291,11 @@ class MoulinetteLock(object): base_lockfile = "/var/run/moulinette_%s.lock" - def __init__(self, namespace, timeout=None, interval=0.5): + def __init__(self, namespace, timeout=None, enable_lock=True, interval=0.5): self.namespace = namespace self.timeout = timeout self.interval = interval + self.enable_lock = enable_lock self._lockfile = self.base_lockfile % namespace self._stale_checked = False @@ -421,7 +422,7 @@ class MoulinetteLock(object): return False def __enter__(self): - if not self._locked: + if self.enable_lock and not self._locked: self.acquire() return self From ce8322bbf478e5255372a3a7273b1ca741c4d4a7 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 26 Dec 2021 18:12:54 +0100 Subject: [PATCH 5/6] Fix tests --- test/src/authenticators/dummy.py | 1 - test/src/authenticators/yoloswag.py | 1 - 2 files changed, 2 deletions(-) diff --git a/test/src/authenticators/dummy.py b/test/src/authenticators/dummy.py index b1eec660..e82e6b17 100644 --- a/test/src/authenticators/dummy.py +++ b/test/src/authenticators/dummy.py @@ -69,7 +69,6 @@ class Authenticator(BaseAuthenticator): return infos - @staticmethod def delete_session_cookie(self): from bottle import response diff --git a/test/src/authenticators/yoloswag.py b/test/src/authenticators/yoloswag.py index 2a549526..9c9eba8b 100644 --- a/test/src/authenticators/yoloswag.py +++ b/test/src/authenticators/yoloswag.py @@ -69,7 +69,6 @@ class Authenticator(BaseAuthenticator): return infos - @staticmethod def delete_session_cookie(self): from bottle import response From 83a95fd8c96a541507622e4c5a5a78d3d776ae30 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 26 Dec 2021 18:13:48 +0100 Subject: [PATCH 6/6] Unused import --- moulinette/interfaces/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index eeaeaa40..0d216f4d 100644 --- a/moulinette/interfaces/api.py +++ b/moulinette/interfaces/api.py @@ -29,7 +29,6 @@ from moulinette.interfaces import ( JSONExtendedEncoder, ) from moulinette.utils import log -from moulinette.utils.text import random_ascii logger = log.getLogger("moulinette.interface.api")