Merge 709585be83 into 8de98670e9

.github/workflows/tox.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.9]
+        python-version: [3.11]
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
@@ -26,13 +26,13 @@ jobs:
         python -m pip install --upgrade pip
         pip install tox tox-gh-actions
     - name: Test with tox
-      run: tox -e py39-pytest
+      run: tox -e py311-pytest
 
   invalidcode:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.9]
+        python-version: [3.11]
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
@@ -44,6 +44,6 @@ jobs:
         python -m pip install --upgrade pip
         pip install tox tox-gh-actions
     - name: Linter
-      run: tox -e py39-invalidcode
+      run: tox -e py311-invalidcode
     - name: Mypy
-      run: tox -e py39-mypy
+      run: tox -e py311-mypy

debian/changelog

@@ -1,3 +1,20 @@
+moulinette (12.0.2) testing; urgency=low
+
+  - portal-api: Bypass CSRF protection for login route ([#340](http://github.com/YunoHost/moulinette/pull/340))
+  - portal-api: login/logout redirect to referer when param referer_redirect is set ([#339](http://github.com/YunoHost/moulinette/pull/339))
+
+  Thanks to all contributors <3 ! (selfhoster1312)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Sat, 31 Aug 2024 20:12:43 +0200
+
+moulinette (12.0.1) testing; urgency=low
+
+  - Tweaks and fixes for new portal API / ssowat refactoring ([#341](https://github.com/YunoHost/moulinette/pull/341))
+  - Simplify logging : unecessary messages + obscure concept of "action id" ([#337](https://github.com/YunoHost/moulinette/pull/337))
+  - Misc tweaks to adapt code and tests to Python 3.11
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Thu, 04 May 2023 20:30:19 +0200
+
 moulinette (11.2.1) stable; urgency=low
 
   - repo chores: various black enhancements

moulinette/__init__.py

@@ -53,7 +53,7 @@ class Moulinette:
 
 
 # Easy access to interfaces
-def api(host="localhost", port=80, routes={}, actionsmap=None, locales_dir=None):
+def api(host="localhost", port=80, routes={}, actionsmap=None, locales_dir=None, allowed_cors_origins=[]):
     """Web server (API) interface
 
     Run a HTTP server with the moulinette for an API usage.
@@ -73,6 +73,7 @@ def api(host="localhost", port=80, routes={}, actionsmap=None, locales_dir=None)
         Api(
             routes=routes,
             actionsmap=actionsmap,
+            allowed_cors_origins=allowed_cors_origins,
         ).run(host, port)
     except MoulinetteError as e:
         import logging

moulinette/actionsmap.py

@@ -19,7 +19,6 @@ from moulinette.core import (
     MoulinetteValidationError,
 )
 from moulinette.interfaces import BaseActionsMapParser
-from moulinette.utils.log import start_action_logging
 from moulinette.utils.filesystem import read_yaml
 
 logger = logging.getLogger("moulinette.actionsmap")
@@ -392,8 +391,6 @@ class ActionsMap:
 
         self.from_cache = False
 
-        logger.debug("loading actions map")
-
         actionsmap_yml_dir = os.path.dirname(actionsmap_yml)
         actionsmap_yml_file = os.path.basename(actionsmap_yml)
         actionsmap_yml_stat = os.stat(actionsmap_yml)
@@ -461,7 +458,7 @@ class ActionsMap:
 
         # Load and initialize the authenticator module
         auth_module = f"{self.namespace}.authenticators.{auth_method}"
-        logger.debug(f"Loading auth module {auth_module}")
+        #logger.debug(f"Loading auth module {auth_module}")
         try:
             mod = import_module(auth_module)
         except ImportError as e:
@@ -556,17 +553,7 @@ class ActionsMap:
                 logger.exception(error_message)
                 raise MoulinetteError(error_message, raw_msg=True)
             else:
-                log_id = start_action_logging()
+                logger.debug("processing action '%s'", full_action_name)
-                if logger.isEnabledFor(logging.DEBUG):
-                    # Log arguments in debug mode only for safety reasons
-                    logger.debug(
-                        "processing action [%s]: %s with args=%s",
-                        log_id,
-                        full_action_name,
-                        arguments,
-                    )
-                else:
-                    logger.debug("processing action [%s]: %s", log_id, full_action_name)
 
                 # Load translation and process the action
                 start = time()
@@ -574,7 +561,7 @@ class ActionsMap:
                     return func(**arguments)
                 finally:
                     stop = time()
-                    logger.debug("action [%s] executed in %.3fs", log_id, stop - start)
+                    logger.debug("action executed in %.3fs", stop - start)
 
     # Private methods
 
@@ -592,9 +579,6 @@ class ActionsMap:
 
         """
 
-        logger.debug("building parser...")
-        start = time()
-
         interface_type = top_parser.interface
 
         # If loading from cache, extra were already checked when cache was
@@ -711,5 +695,4 @@ class ActionsMap:
                     else:
                         action_parser.want_to_take_lock = True
 
-        logger.debug("building parser took %.3fs", time() - start)
         return top_parser

moulinette/interfaces/__init__.py

@@ -32,8 +32,7 @@ class BaseActionsMapParser:
     """
 
     def __init__(self, parent=None, **kwargs):
-        if not parent:
+        pass
-            logger.debug("initializing base actions map parser for %s", self.interface)
 
     # Virtual properties
     # Each parser classes must implement these properties.

moulinette/interfaces/api.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-
+import sys
 import re
 import errno
 import logging
@@ -13,7 +13,7 @@ from gevent import sleep
 from gevent.queue import Queue
 from geventwebsocket import WebSocketError
 
-from bottle import request, response, Bottle, HTTPResponse, FileUpload
+from bottle import redirect, request, response, Bottle, HTTPResponse, FileUpload
 from bottle import abort
 
 from moulinette import m18n, Moulinette
@@ -30,7 +30,7 @@ from moulinette.interfaces import (
 )
 from moulinette.utils import log
 
-logger = log.getLogger("moulinette.interface.api")
+logger = logging.getLogger("moulinette.interface.api")
 
 
 # API helpers ----------------------------------------------------------
@@ -269,13 +269,14 @@ class _ActionsMapPlugin:
             name="login",
             method="POST",
             callback=self.login,
-            skip=["actionsmap"],
+            skip=[filter_csrf, "actionsmap"],
         )
         app.route(
             "/logout",
             name="logout",
             method="GET",
             callback=self.logout,
+            # No need to bypass CSRF here because filter allows GET requests
             skip=["actionsmap"],
         )
 
@@ -309,6 +310,9 @@ class _ActionsMapPlugin:
             return value
 
         def wrapper(*args, **kwargs):
+            if request.get_header("Content-Type") == "application/json":
+                return callback((request.method, context.rule), request.json)
+
             params = kwargs
             # Format boolean params
             for a in args:
@@ -350,11 +354,21 @@ class _ActionsMapPlugin:
 
         """
 
-        if "credentials" not in request.params:
+        if request.get_header("Content-Type") == "application/json":
-            raise HTTPResponse("Missing credentials parameter", 400)
+            if "credentials" not in request.json:
-        credentials = request.params["credentials"]
+                raise HTTPResponse("Missing credentials parameter", 400)
+            credentials = request.json["credentials"]
+            profile = request.json.get("profile", self.actionsmap.default_authentication)
+        else:
+            if "credentials" in request.params:
+                credentials = request.params["credentials"]
+            elif "username" in request.params and "password" in request.params:
+                credentials = request.params["username"] + ":" + request.params["password"]
+            else:
+                raise HTTPResponse("Missing credentials parameter", 400)
+
+            profile = request.params.get("profile", self.actionsmap.default_authentication)
 
-        profile = request.params.get("profile", self.actionsmap.default_authentication)
         authenticator = self.actionsmap.get_authenticator(profile)
 
         try:
@@ -367,13 +381,18 @@ class _ActionsMapPlugin:
             raise HTTPResponse(e.strerror, 401)
         else:
             authenticator.set_session_cookie(auth_infos)
-            return m18n.g("logged_in")
+            referer = request.get_header("Referer")
+            if "referer_redirect" in request.params and referer:
+                redirect(referer)
+            else:
+                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 = authenticator.get_session_cookie()
         except Exception:
+            authenticator.delete_session_cookie()
             msg = m18n.g("authentication_required")
             raise HTTPResponse(msg, 401)
 
@@ -390,7 +409,11 @@ class _ActionsMapPlugin:
         else:
             # Delete cookie and clean the session
             authenticator.delete_session_cookie()
-            return m18n.g("logged_out")
+            referer = request.get_header("Referer")
+            if "referer_redirect" in request.params and referer:
+                redirect(referer)
+            else:
+                return m18n.g("logged_in")
 
     def messages(self):
         """Listen to the messages WebSocket stream
@@ -457,6 +480,7 @@ class _ActionsMapPlugin:
 
             tb = traceback.format_exc()
             logs = {"route": _route, "arguments": arguments, "traceback": tb}
+            print(tb, file=sys.stderr)
             return HTTPResponse(json_encode(logs), 500)
         else:
             return format_for_response(ret)
@@ -702,9 +726,11 @@ class Interface:
 
     type = "api"
 
-    def __init__(self, routes={}, actionsmap=None):
+    def __init__(self, routes={}, actionsmap=None, allowed_cors_origins=[]):
         actionsmap = ActionsMap(actionsmap, ActionsMapParser())
 
+        self.allowed_cors_origins = allowed_cors_origins
+
         # Attempt to retrieve log queues from an APIQueueHandler
         handler = log.getHandlersByClass(APIQueueHandler, limit=1)
         if handler:
@@ -714,11 +740,22 @@ class Interface:
         # TODO: Return OK to 'OPTIONS' xhr requests (l173)
         app = Bottle(autojson=True)
 
-        # Wrapper which sets proper header
+        def cors(callback):
-        def apiheader(callback):
             def wrapper(*args, **kwargs):
-                response.set_header("Access-Control-Allow-Origin", "*")
+                try:
-                return callback(*args, **kwargs)
+                    r = callback(*args, **kwargs)
+                except HTTPResponse as e:
+                    r = e
+
+                origin = request.headers.environ.get("HTTP_ORIGIN", "")
+                if origin and origin in self.allowed_cors_origins:
+                    resp = r if isinstance(r, HTTPResponse) else response
+                    resp.headers['Access-Control-Allow-Origin'] = origin
+                    resp.headers['Access-Control-Allow-Methods'] = 'GET, HEAD, POST, PUT, OPTIONS, DELETE'
+                    resp.headers['Access-Control-Allow-Headers'] = 'Origin, Accept, Content-Type, X-Requested-With, X-CSRF-Token'
+                    resp.headers['Access-Control-Allow-Credentials'] = 'true'
+
+                return r
 
             return wrapper
 
@@ -727,7 +764,7 @@ class Interface:
             def wrapper(*args, **kwargs):
                 try:
                     locale = request.params.pop("locale")
-                except KeyError:
+                except (KeyError, ValueError):
                     locale = m18n.default_locale
                 m18n.set_locale(locale)
                 return callback(*args, **kwargs)
@@ -736,7 +773,7 @@ class Interface:
 
         # Install plugins
         app.install(filter_csrf)
-        app.install(apiheader)
+        app.install(cors)
         app.install(api18n)
         actionsmapplugin = _ActionsMapPlugin(actionsmap, log_queues)
         app.install(actionsmapplugin)
@@ -745,6 +782,12 @@ class Interface:
         self.display = actionsmapplugin.display
         self.prompt = actionsmapplugin.prompt
 
+        def handle_options():
+            return HTTPResponse("", 204)
+
+        app.route('/<:re:.*>', method="OPTIONS",
+                  callback=handle_options, skip=["actionsmap"])
+
         # Append additional routes
         # TODO: Add optional authentication to those routes?
         for (m, p), c in routes.items():

moulinette/interfaces/cli.py

@@ -50,7 +50,7 @@ def monkey_get_action_name(argument):
 
 argparse._get_action_name = monkey_get_action_name
 
-logger = log.getLogger("moulinette.cli")
+logger = logging.getLogger("moulinette.cli")
 
 
 # CLI helpers ----------------------------------------------------------
@@ -234,28 +234,26 @@ class TTYHandler(logging.StreamHandler):
         log.CRITICAL: "red",
     }
 
-    def __init__(self, message_key="fmessage"):
+    def __init__(self, message_key="message_with_color"):
         logging.StreamHandler.__init__(self)
         self.message_key = message_key
 
     def format(self, record):
         """Enhance message with level and colors if supported."""
         msg = record.getMessage()
+        level = record.levelname
+        level_with_color = level
         if self.supports_color():
-            level = ""
+            if self.level > log.DEBUG and record.levelname in ["SUCCESS", "WARNING", "ERROR", "INFO"]:
-            if self.level <= log.DEBUG:
+                level = m18n.g(record.levelname.lower())
-                # add level name before message
-                level = "%s " % record.levelname
-            elif record.levelname in ["SUCCESS", "WARNING", "ERROR", "INFO"]:
-                # add translated level name before message
-                level = "%s " % m18n.g(record.levelname.lower())
             color = self.LEVELS_COLOR.get(record.levelno, "white")
-            msg = "{}{}{}{}".format(colors_codes[color], level, END_CLI_COLOR, msg)
+            level_with_color = f"{colors_codes[color]}{level}{END_CLI_COLOR}"
+            if self.level == log.DEBUG:
+                level_with_color = level_with_color + " " * max(0, 7 - len(level))
         if self.formatter:
-            # use user-defined formatter
+            record.__dict__["level_with_color"] = level_with_color
-            record.__dict__[self.message_key] = msg
             return self.formatter.format(record)
-        return msg
+        return level_with_color + " " + msg
 
     def emit(self, record):
         # set proper stream first

moulinette/utils/log.py

@@ -6,7 +6,6 @@ from logging import (
     addLevelName,
     setLoggerClass,
     Logger,
-    getLogger,
     NOTSET,  # noqa
     DEBUG,
     INFO,
@@ -109,88 +108,14 @@ class MoulinetteLogger(Logger):
         f = currentframe()
         if f is not None:
             f = f.f_back
-        rv = "(unknown file)", 0, "(unknown function)"
+        rv = "(unknown file)", 0, "(unknown function)", None
         while hasattr(f, "f_code"):
             co = f.f_code
             filename = os.path.normcase(co.co_filename)
             if filename == _srcfile or filename == __file__:
                 f = f.f_back
                 continue
-            rv = (co.co_filename, f.f_lineno, co.co_name)
+            rv = (co.co_filename, f.f_lineno, co.co_name, None)
             break
+
         return rv
-
-    def _log(self, *args, **kwargs):
-        """Append action_id if available to the extra."""
-        if self.action_id is not None:
-            extra = kwargs.get("extra", {})
-            if "action_id" not in extra:
-                # FIXME: Get real action_id instead of logger/current one
-                extra["action_id"] = _get_action_id()
-                kwargs["extra"] = extra
-        return super()._log(*args, **kwargs)
-
-
-# Action logging -------------------------------------------------------
-
-pid = os.getpid()
-action_id = 0
-
-
-def _get_action_id():
-    return "%d.%d" % (pid, action_id)
-
-
-def start_action_logging():
-    """Configure logging for a new action
-
-    Returns:
-        The new action id
-
-    """
-    global action_id
-    action_id += 1
-
-    return _get_action_id()
-
-
-def getActionLogger(name=None, logger=None, action_id=None):
-    """Get the logger adapter for an action
-
-    Return a logger for the specified name - or use given logger - and
-    optionally for a given action id, retrieving it if necessary.
-
-    Either a name or a logger must be specified.
-
-    """
-    if not name and not logger:
-        raise ValueError("Either a name or a logger must be specified")
-
-    logger = logger or getLogger(name)
-    logger.action_id = action_id if action_id else _get_action_id()
-    return logger
-
-
-class ActionFilter:
-    """Extend log record for an optionnal action
-
-    Filter a given record and look for an `action_id` key. If it is not found
-    and `strict` is True, the record will not be logged. Otherwise, the key
-    specified by `message_key` will be added to the record, containing the
-    message formatted for the action or just the original one.
-
-    """
-
-    def __init__(self, message_key="fmessage", strict=False):
-        self.message_key = message_key
-        self.strict = strict
-
-    def filter(self, record):
-        msg = record.getMessage()
-        action_id = record.__dict__.get("action_id", None)
-        if action_id is not None:
-            msg = "[{:s}] {:s}".format(action_id, msg)
-        elif self.strict:
-            return False
-        record.__dict__[self.message_key] = msg
-        return True

moulinette/utils/process.py

@@ -82,7 +82,7 @@ def call_async_output(args, callback, **kwargs):
         while p.poll() is None:
             while True:
                 try:
-                    callback, message = log_queue.get(True, 1)
+                    callback, message = log_queue.get(True, 0.1)
                 except queue.Empty:
                     break
 

setup.py

@@ -60,7 +60,7 @@ setup(
     license="AGPL",
     packages=find_packages(exclude=["test"]),
     data_files=[("/usr/share/moulinette/locales", locale_files)],
-    python_requires=">=3.7.0,<3.10",
+    python_requires=">=3.11.0,<3.12",
     install_requires=install_deps,
     tests_require=test_deps,
     extras_require=extras,

test/conftest.py

@@ -46,7 +46,7 @@ def logging_configuration(moulinette):
                 "format": "%(asctime)-15s %(levelname)-8s %(name)s %(funcName)s - %(fmessage)s"  # noqa
             },
         },
-        "filters": {"action": {"()": "moulinette.utils.log.ActionFilter"}},
+        "filters": {},
         "handlers": {
             "api": {
                 "level": level,

test/test_auth.py

@@ -180,7 +180,7 @@ class TestAuthAPI:
 
     def test_request_arg_without_action(self, moulinette_webapi, caplog, mocker):
         self.login(moulinette_webapi)
-        moulinette_webapi.get("/test-auth", status=404)
+        moulinette_webapi.get("/test-auth", status=405)
 
 
 class TestAuthCLI:

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 envlist =
-  py{37,39}-{pytest,lint,invalidcode,mypy}
+  py311-{pytest,lint,invalidcode,mypy}
   format
   format-check
   docs
@@ -11,20 +11,19 @@ usedevelop = True
 passenv = *
 extras = tests
 deps =
-  py{37,39}-pytest: .[tests]
+  py311-pytest: .[tests]
-  py{37,39}-lint: flake8
+  py311-lint: flake8
-  py{37,39}-invalidcode: flake8
+  py311-invalidcode: flake8
-  py{37,39}-mypy: mypy >= 0.761
+  py311-mypy: mypy >= 0.761
 commands =
-  py{37,39}-pytest: pytest {posargs} -c pytest.ini
+  py311-pytest: pytest {posargs} -c pytest.ini
-  py{37,39}-lint: flake8 moulinette test
+  py311-lint: flake8 moulinette test
-  py{37,39}-invalidcode: flake8 moulinette test --select F
+  py311-invalidcode: flake8 moulinette test --select F
-  py{37,39}-mypy: mypy  --ignore-missing-imports --install-types --non-interactive moulinette/
+  py311-mypy: mypy  --ignore-missing-imports --install-types --non-interactive moulinette/
 
 [gh-actions]
 python =
-  3.7: py37
+  3.11: py311
-  3.9: py39
 
 [testenv:format]
 basepython = python3