From 51930c1e783b8276a02f5b3f9046f4f869767fff Mon Sep 17 00:00:00 2001 From: Luca Matei Pintilie Date: Fri, 28 Nov 2025 20:50:55 +0100 Subject: [PATCH] add default_persistence_allowed In addition set all config options in every test, as catch2 can run tests out of order and as demonstrated in e1ad60af4fd267ac1df57748afc910b2ea26a8b4 this can be an issue --- CHANGELOG.rst | 5 +-- doc/admin.rst | 6 ++++ src/utils/is_requester_allowed_to_persist.hpp | 4 +-- tests/is_requester_allowed_to_persist.cpp | 36 +++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 981e000..c6101fc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,8 +19,9 @@ For admins ---------- - Command line option --test-config (or -t) has been added. When used, biboumi will just exit without any error if the configuration is correct -- Options persist_user_denylist and persist_user_allowlist have been added to - granualy control who can set the persist option +- Options default_persistence_allowed, persist_user_denylist, and + persist_user_allowlist have been added to granualy control who can set the + persist option For packagers ------------- diff --git a/doc/admin.rst b/doc/admin.rst index a04a59a..effe6ac 100644 --- a/doc/admin.rst +++ b/doc/admin.rst @@ -260,6 +260,12 @@ persist_user_allowlist A list of XMPP XMPP users or domains who are allowed to change the `persist` ad-hoc option. +default_persistence_allowed +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If a given XMPP user or domain is not in either denylist or allowlist this +option decides if the user is allowed to persist or not. + TLS configuration ----------------- diff --git a/src/utils/is_requester_allowed_to_persist.hpp b/src/utils/is_requester_allowed_to_persist.hpp index 5dc5956..b81f3d3 100644 --- a/src/utils/is_requester_allowed_to_persist.hpp +++ b/src/utils/is_requester_allowed_to_persist.hpp @@ -19,7 +19,7 @@ namespace utils * 2. `persist_user_allowlist` contains the requester's jid * 3. `persist_user_denylist` contains the requester's domain * 4. `persist_user_allowlist` contains the contains the requester's domain - * 5. allow + * 5. `default_persistence_allowed` is true (default true) */ inline bool is_requester_allowed_to_persist(const Jid &requester) { // requester.bare checks @@ -50,6 +50,6 @@ inline bool is_requester_allowed_to_persist(const Jid &requester) { // default allow // 5. - return true; + return Config::get_bool("default_persistence_allowed", true); } } diff --git a/tests/is_requester_allowed_to_persist.cpp b/tests/is_requester_allowed_to_persist.cpp index 69cf5f4..372e4b3 100644 --- a/tests/is_requester_allowed_to_persist.cpp +++ b/tests/is_requester_allowed_to_persist.cpp @@ -9,6 +9,31 @@ TEST_CASE("default") Config::clear(); Config::set("persist_user_denylist", ""); Config::set("persist_user_allowlist", ""); + Config::set("default_persistence_allowed", ""); + bool result = utils::is_requester_allowed_to_persist(jid); + CHECK(result == true); + Config::clear(); +} + +TEST_CASE("default - default_persistence_allowed - false") +{ + Jid jid("foo@example.com"); + Config::clear(); + Config::set("persist_user_denylist", ""); + Config::set("persist_user_allowlist", ""); + Config::set("default_persistence_allowed", "false"); + bool result = utils::is_requester_allowed_to_persist(jid); + CHECK(result == false); + Config::clear(); +} + +TEST_CASE("default - default_persistence_allowed - true") +{ + Jid jid("foo@example.com"); + Config::clear(); + Config::set("persist_user_denylist", ""); + Config::set("persist_user_allowlist", ""); + Config::set("default_persistence_allowed", "true"); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == true); Config::clear(); @@ -19,6 +44,8 @@ TEST_CASE("denylist - jid") Jid jid("foo@example.com"); Config::clear(); Config::set("persist_user_denylist", "foo@example.com"); + Config::set("persist_user_allowlist", ""); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == false); Config::clear(); @@ -29,6 +56,8 @@ TEST_CASE("denylist - domain") Jid jid("foo@example.com"); Config::clear(); Config::set("persist_user_denylist", "example.com"); + Config::set("persist_user_allowlist", ""); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == false); Config::clear(); @@ -39,6 +68,8 @@ TEST_CASE("allowlist - jid") Jid jid("foo@example.com"); Config::clear(); Config::set("persist_user_allowlist", "foo@example.com"); + Config::set("persist_user_denylist", ""); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == true); Config::clear(); @@ -49,6 +80,8 @@ TEST_CASE("allowlist - domain") Jid jid("foo@example.com"); Config::clear(); Config::set("persist_user_allowlist", "example.com"); + Config::set("persist_user_denylist", ""); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == true); Config::clear(); @@ -60,6 +93,7 @@ TEST_CASE("denylist + allowlist - deny domain, allow jid") Config::clear(); Config::set("persist_user_denylist", "example.com"); Config::set("persist_user_allowlist", "foo@example.com"); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == true); Config::clear(); @@ -71,6 +105,7 @@ TEST_CASE("denylist + allowlist - deny jid, allow domain") Config::clear(); Config::set("persist_user_denylist", "foo@example.com"); Config::set("persist_user_allowlist", "example.com"); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == false); Config::clear(); @@ -82,6 +117,7 @@ TEST_CASE("denylist + allowlist - deny jid, allow jid") Config::clear(); Config::set("persist_user_denylist", "foo@example.com"); Config::set("persist_user_allowlist", "foo@example.com"); + Config::set("default_persistence_allowed", ""); bool result = utils::is_requester_allowed_to_persist(jid); CHECK(result == false); Config::clear();