From d15e83607919b32910006678624529d9f5d8d906 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 17:44:05 +0200 Subject: [PATCH 01/12] extend copyright --- radicale/app/delete.py | 1 + 1 file changed, 1 insertion(+) diff --git a/radicale/app/delete.py b/radicale/app/delete.py index 53d9bfd3..52c8e060 100644 --- a/radicale/app/delete.py +++ b/radicale/app/delete.py @@ -3,6 +3,7 @@ # Copyright © 2008 Pascal Halter # Copyright © 2008-2017 Guillaume Ayoub # Copyright © 2017-2018 Unrud +# Copyright © 2024-2024 Peter Bieringer # # This library is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by From 40c8b3d03886e18d1ff9422371daaea90ab11293 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 17:44:21 +0200 Subject: [PATCH 02/12] log in case delete of collection is prevented --- radicale/app/delete.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/radicale/app/delete.py b/radicale/app/delete.py index 52c8e060..9c595da6 100644 --- a/radicale/app/delete.py +++ b/radicale/app/delete.py @@ -25,6 +25,7 @@ from typing import Optional from radicale import httputils, storage, types, xmlutils from radicale.app.base import Access, ApplicationBase from radicale.hook import HookNotificationItem, HookNotificationItemTypes +from radicale.log import logger def xml_delete(base_prefix: str, path: str, collection: storage.BaseCollection, @@ -82,6 +83,7 @@ class ApplicationPartDelete(ApplicationBase): ) xml_answer = xml_delete(base_prefix, path, item) else: + logger.info("delete of collection is prevented by config/option [rights] permit_delete_collection: %s", path) return httputils.NOT_ALLOWED else: assert item.collection is not None From 0f87897eb7745949f9b5fb9f2c36f02b764c4d8a Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 17:44:44 +0200 Subject: [PATCH 03/12] change default rights/permit_delete_collection from True to False (failsafe) --- CHANGELOG.md | 1 + config | 2 +- radicale/config.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08296c64..6d8a6dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Adjustment: option [auth] htpasswd_encryption change default from "md5" to "autodetect" * Add: option [auth] type=ldap with (group) rights management via LDAP/LDAPS +* Adjustment: option [rights] 'permit_delete_collection' change default from "True" to "False" (failsafe) ## 3.2.3 * Add: support for Python 3.13 diff --git a/config b/config index 5fb11290..56c2c61c 100644 --- a/config +++ b/config @@ -113,7 +113,7 @@ #file = /etc/radicale/rights # Permit delete of a collection (global) -#permit_delete_collection = True +#permit_delete_collection = False [storage] diff --git a/radicale/config.py b/radicale/config.py index 5bddaf91..68b3cc2a 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -242,7 +242,7 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([ "type": str_or_callable, "internal": rights.INTERNAL_TYPES}), ("permit_delete_collection", { - "value": "True", + "value": "False", "help": "permit delete of a collection", "type": bool}), ("file", { From a449d8774bfe91da706a3f48ab574ff1b9217139 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 18:10:29 +0200 Subject: [PATCH 04/12] enforce default for tests --- radicale/tests/test_base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index fc708ebc..47961785 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -41,6 +41,7 @@ class TestBaseRequests(BaseTest): def setup_method(self) -> None: BaseTest.setup_method(self) rights_file_path = os.path.join(self.colpath, "rights") + self.configure({"rights": {"permit_delete_collection": True}}) with open(rights_file_path, "w") as f: f.write("""\ [allow all] From 4ef5cad20f41f884f824418df41ea3db003bb28c Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 18:10:53 +0200 Subject: [PATCH 05/12] add test case --- radicale/tests/test_base.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index 47961785..bf93dd91 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -440,6 +440,15 @@ permissions: RrWw""") assert responses["/calendar.ics/"] == 200 self.get("/calendar.ics/", check=404) + def test_delete_collection_not_permitted(self) -> None: + """Delete a collection (try if not permitted).""" + self.configure({"rights": {"permit_delete_collection": False}}) + self.mkcalendar("/calendar.ics/") + event = get_file_content("event1.ics") + self.put("/calendar.ics/event1.ics", event) + _, responses = self.delete("/calendar.ics/", check=401) + self.get("/calendar.ics/", check=200) + def test_delete_root_collection(self) -> None: """Delete the root collection.""" self.mkcalendar("/calendar.ics/") From 0ab99d4e8fd1cbea5315645a5bab12294a60ab22 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 18:16:54 +0200 Subject: [PATCH 06/12] update doc related to changed default --- DOCUMENTATION.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 61a56ebf..d0b29822 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -909,9 +909,9 @@ File for the rights backend `from_file`. See the ##### permit_delete_collection -(New since 3.1.9) +(New since 3.1.9, default changed from True to False since 3.3.0) -Global control of permission to delete complete collection (default: True) +Global control of permission to delete complete collection (default: False) #### storage From 3e478ee6da2d7d34408a81768085ae77b0bb07a6 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Sep 2024 21:11:49 +0200 Subject: [PATCH 07/12] update doc --- DOCUMENTATION.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index d0b29822..46a9cd23 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -909,9 +909,12 @@ File for the rights backend `from_file`. See the ##### permit_delete_collection -(New since 3.1.9, default changed from True to False since 3.3.0) +(New since 3.1.9) -Global control of permission to delete complete collection (default: False) +Global control of permission to delete complete collection (default: True) + +If False it can be permitted by permissions per section with: D +If True it can be forbidden by permissions per section with: d #### storage @@ -1295,6 +1298,8 @@ The following `permissions` are recognized: (CalDAV/CardDAV is susceptible to expensive search requests) * **W:** write collections (excluding address books and calendars) * **w:** write address book and calendar collections +* **D:** permit delete of collection in case permit_delete_collection=False +* **d:** forbid delete of collection in case permit_delete_collection=True ### Storage From 72e4c4fadd2801b02123e835735877e6e412308d Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 19:54:54 +0200 Subject: [PATCH 08/12] add explicit permit/forbid test cases --- radicale/tests/test_base.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index bf93dd91..b6046c1a 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -44,6 +44,16 @@ class TestBaseRequests(BaseTest): self.configure({"rights": {"permit_delete_collection": True}}) with open(rights_file_path, "w") as f: f.write("""\ +[permit delete collection] +user: .* +collection: test-permit-delete +permissions: RrWwD + +[forbid delete collection] +user: .* +collection: test-forbid-delete +permissions: RrWwd + [allow all] user: .* collection: .* @@ -449,6 +459,24 @@ permissions: RrWw""") _, responses = self.delete("/calendar.ics/", check=401) self.get("/calendar.ics/", check=200) + def test_delete_collection_global_forbid_explicit_permit(self) -> None: + """Delete a collection with permitted path (expect permit).""" + self.configure({"rights": {"permit_delete_collection": False}}) + self.mkcalendar("/test-permit-delete/") + event = get_file_content("event1.ics") + self.put("/test-permit-delete/event1.ics", event) + _, responses = self.delete("/test-permit-delete/", check=200) + self.get("/test-permit-delete/", check=404) + + def test_delete_collection_global_permit_explicit_forbid(self) -> None: + """Delete a collection with permitted path (expect forbid).""" + self.configure({"rights": {"permit_delete_collection": True}}) + self.mkcalendar("/test-forbid-delete/") + event = get_file_content("event1.ics") + self.put("/test-forbid-delete/event1.ics", event) + _, responses = self.delete("/test-forbid-delete/", check=401) + self.get("/test-forbid-delete/", check=200) + def test_delete_root_collection(self) -> None: """Delete the root collection.""" self.mkcalendar("/calendar.ics/") From 53bc6167d357f55454858ed67f5dda5f22f117d6 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 19:52:53 +0200 Subject: [PATCH 09/12] add support for dedicated forbid/permit permission --- radicale/app/delete.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/radicale/app/delete.py b/radicale/app/delete.py index 9c595da6..ee7550ff 100644 --- a/radicale/app/delete.py +++ b/radicale/app/delete.py @@ -73,18 +73,22 @@ class ApplicationPartDelete(ApplicationBase): hook_notification_item_list = [] if isinstance(item, storage.BaseCollection): if self._permit_delete_collection: - for i in item.get_all(): - hook_notification_item_list.append( - HookNotificationItem( - HookNotificationItemTypes.DELETE, - access.path, - i.uid - ) - ) - xml_answer = xml_delete(base_prefix, path, item) + if access.check("d", item): + logger.info("delete of collection is permitted by config/option [rights] permit_delete_collection but explicit forbidden by permission 'd': %s", path) + return httputils.NOT_ALLOWED else: - logger.info("delete of collection is prevented by config/option [rights] permit_delete_collection: %s", path) - return httputils.NOT_ALLOWED + if not access.check("D", item): + logger.info("delete of collection is prevented by config/option [rights] permit_delete_collection and not explicit allowed by permission 'D': %s", path) + return httputils.NOT_ALLOWED + for i in item.get_all(): + hook_notification_item_list.append( + HookNotificationItem( + HookNotificationItemTypes.DELETE, + access.path, + i.uid + ) + ) + xml_answer = xml_delete(base_prefix, path, item) else: assert item.collection is not None assert item.href is not None From 06a9cf288639df25945b25c8f226245815c6bb35 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Sun, 29 Sep 2024 19:52:08 +0200 Subject: [PATCH 10/12] extend whitelisted permission chars --- radicale/app/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/app/base.py b/radicale/app/base.py index 5c8a9355..71fd8073 100644 --- a/radicale/app/base.py +++ b/radicale/app/base.py @@ -125,7 +125,7 @@ class Access: def check(self, permission: str, item: Optional[types.CollectionOrItem] = None) -> bool: - if permission not in "rw": + if permission not in "rwdD": raise ValueError("Invalid permission argument: %r" % permission) if not item: permissions = permission + permission.upper() From fc77cf9d66f5154253143619b9d6a64a20af8d43 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Sep 2024 21:16:36 +0200 Subject: [PATCH 11/12] revert 0f87897e --- CHANGELOG.md | 1 - config | 2 +- radicale/config.py | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d8a6dbf..08296c64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ * Adjustment: option [auth] htpasswd_encryption change default from "md5" to "autodetect" * Add: option [auth] type=ldap with (group) rights management via LDAP/LDAPS -* Adjustment: option [rights] 'permit_delete_collection' change default from "True" to "False" (failsafe) ## 3.2.3 * Add: support for Python 3.13 diff --git a/config b/config index 56c2c61c..5fb11290 100644 --- a/config +++ b/config @@ -113,7 +113,7 @@ #file = /etc/radicale/rights # Permit delete of a collection (global) -#permit_delete_collection = False +#permit_delete_collection = True [storage] diff --git a/radicale/config.py b/radicale/config.py index 68b3cc2a..5bddaf91 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -242,7 +242,7 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([ "type": str_or_callable, "internal": rights.INTERNAL_TYPES}), ("permit_delete_collection", { - "value": "False", + "value": "True", "help": "permit delete of a collection", "type": bool}), ("file", { From 77749cbbb95cf6a68cb1208ee2eab65b40fe60dc Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Mon, 30 Sep 2024 21:21:33 +0200 Subject: [PATCH 12/12] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08296c64..b1134ffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Adjustment: option [auth] htpasswd_encryption change default from "md5" to "autodetect" * Add: option [auth] type=ldap with (group) rights management via LDAP/LDAPS +* Enhancement: permit_delete_collection can be now controlled also per collection by rights 'D' or 'd' ## 3.2.3 * Add: support for Python 3.13