From d48bacc8e3414ffba7c3c8c21987f2ada8f0ab48 Mon Sep 17 00:00:00 2001 From: Unrud Date: Sat, 26 Sep 2020 22:08:23 +0200 Subject: [PATCH] Improve log messages Log failed login attempts with remote host as warning (closes #1104) Add component UID to log message about invalid recurrence rules (reference #602) Use "forwarded for" instead of "forwarded by" for remote host --- radicale/app/__init__.py | 7 ++++--- radicale/app/mkcalendar.py | 2 +- radicale/app/mkcol.py | 2 +- radicale/app/propfind.py | 2 +- radicale/app/proppatch.py | 2 +- radicale/app/put.py | 2 +- radicale/app/report.py | 4 ++-- radicale/config.py | 2 +- radicale/item/__init__.py | 8 ++++---- 9 files changed, 16 insertions(+), 15 deletions(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index b0c53a9..564a45b 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -167,8 +167,8 @@ class Application( elif environ.get("REMOTE_ADDR"): remote_host = environ["REMOTE_ADDR"] if environ.get("HTTP_X_FORWARDED_FOR"): - remote_host = "%r (forwarded by %s)" % ( - environ["HTTP_X_FORWARDED_FOR"], remote_host) + remote_host = "%s (forwarded for %r)" % ( + remote_host, environ["HTTP_X_FORWARDED_FOR"]) remote_useragent = "" if environ.get("HTTP_USER_AGENT"): remote_useragent = " using %r" % environ["HTTP_USER_AGENT"] @@ -230,7 +230,8 @@ class Application( elif user: logger.info("Successful login: %r -> %r", login, user) elif login: - logger.info("Failed login attempt: %r", login) + logger.warning("Failed login attempt from %s: %r", + remote_host, login) # Random delay to avoid timing oracles and bruteforce attacks delay = self.configuration.get("auth", "delay") if delay > 0: diff --git a/radicale/app/mkcalendar.py b/radicale/app/mkcalendar.py index 5b2c76f..ae14181 100644 --- a/radicale/app/mkcalendar.py +++ b/radicale/app/mkcalendar.py @@ -39,7 +39,7 @@ class ApplicationMkcalendarMixin: "Bad MKCALENDAR request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT # Prepare before locking props = xmlutils.props_from_request(xml_content) diff --git a/radicale/app/mkcol.py b/radicale/app/mkcol.py index 2cef253..4118141 100644 --- a/radicale/app/mkcol.py +++ b/radicale/app/mkcol.py @@ -40,7 +40,7 @@ class ApplicationMkcolMixin: "Bad MKCOL request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT # Prepare before locking props = xmlutils.props_from_request(xml_content) diff --git a/radicale/app/propfind.py b/radicale/app/propfind.py index 59162a6..cacbed9 100644 --- a/radicale/app/propfind.py +++ b/radicale/app/propfind.py @@ -357,7 +357,7 @@ class ApplicationPropfindMixin: "Bad PROPFIND request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT with self._storage.acquire_lock("r", user): items = self._storage.discover( diff --git a/radicale/app/proppatch.py b/radicale/app/proppatch.py index fc2ef3d..a77228f 100644 --- a/radicale/app/proppatch.py +++ b/radicale/app/proppatch.py @@ -76,7 +76,7 @@ class ApplicationProppatchMixin: "Bad PROPPATCH request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT with self._storage.acquire_lock("w", user): item = next(self._storage.discover(path), None) diff --git a/radicale/app/put.py b/radicale/app/put.py index efaf905..2951974 100644 --- a/radicale/app/put.py +++ b/radicale/app/put.py @@ -123,7 +123,7 @@ class ApplicationPutMixin: logger.warning("Bad PUT request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT # Prepare before locking content_type = environ.get("CONTENT_TYPE", "").split(";")[0] diff --git a/radicale/app/report.py b/radicale/app/report.py index 528b17f..99cd01a 100644 --- a/radicale/app/report.py +++ b/radicale/app/report.py @@ -181,7 +181,7 @@ def xml_report(base_prefix, path, xml_request, collection, encoding, radicale_filter.prop_match(item.vobject_item, f, "CR") for f in filter_) raise ValueError("Unsupported filter test: %r" % test) - raise ValueError("unsupported filter %r for %r" % (filter_.tag, tag)) + raise ValueError("Unsupported filter %r for %r" % (filter_.tag, tag)) while retrieved_items: # ``item.vobject_item`` might be accessed during filtering. @@ -268,7 +268,7 @@ class ApplicationReportMixin: "Bad REPORT request on %r: %s", path, e, exc_info=True) return httputils.BAD_REQUEST except socket.timeout: - logger.debug("client timed out", exc_info=True) + logger.debug("Client timed out", exc_info=True) return httputils.REQUEST_TIMEOUT with contextlib.ExitStack() as lock_stack: lock_stack.enter_context(self._storage.acquire_lock("r", user)) diff --git a/radicale/config.py b/radicale/config.py index d30e677..10dd59f 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -94,7 +94,7 @@ def unspecified_type(value): def _convert_to_bool(value): if value.lower() not in RawConfigParser.BOOLEAN_STATES: - raise ValueError("Not a boolean: %r" % value) + raise ValueError("not a boolean: %r" % value) return RawConfigParser.BOOLEAN_STATES[value.lower()] diff --git a/radicale/item/__init__.py b/radicale/item/__init__.py index 53a2113..3bbc55e 100644 --- a/radicale/item/__init__.py +++ b/radicale/item/__init__.py @@ -134,8 +134,8 @@ def check_and_sanitize_items(vobject_items, is_collection=False, tag=None): try: component.rruleset except Exception as e: - raise ValueError("invalid recurrence rules in %s" % - component.name) from e + raise ValueError("Invalid recurrence rules in %s in object %r" + % (component.name, component_uid)) from e elif tag == "VADDRESSBOOK": # https://tools.ietf.org/html/rfc6352#section-5.1 object_uids = set() @@ -311,10 +311,10 @@ class Item: """ if text is None and vobject_item is None: raise ValueError( - "at least one of 'text' or 'vobject_item' must be set") + "At least one of 'text' or 'vobject_item' must be set") if collection_path is None: if collection is None: - raise ValueError("at least one of 'collection_path' or " + raise ValueError("At least one of 'collection_path' or " "'collection' must be set") collection_path = collection.path assert collection_path == pathutils.strip_path(