From 4b3187c019a394b31c76bd1831f18d06b57074f9 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:11:39 +0800 Subject: [PATCH 1/6] test(spp_registry_search): reproduce server-side search governance bypasses The Registry Search controls (min_chars, result_limit, targeted mode) were enforced only in the JS client. Add regression tests (currently failing) pinning server-side enforcement in the search_registrants RPC: short and wildcard-only terms, caller limit above the configured maximum, targeted mode falling back to unified when search_field is missing/invalid, and TypeError crashes on malformed limit/search_term. --- spp_registry_search/tests/__init__.py | 1 + .../tests/test_search_registrants.py | 135 ++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 spp_registry_search/tests/test_search_registrants.py diff --git a/spp_registry_search/tests/__init__.py b/spp_registry_search/tests/__init__.py index 049ae4fb9..72460818a 100644 --- a/spp_registry_search/tests/__init__.py +++ b/spp_registry_search/tests/__init__.py @@ -1,4 +1,5 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. from . import test_registrant_create_permission +from . import test_search_registrants from . import test_registry_view_history from . import test_registry_view_history_security diff --git a/spp_registry_search/tests/test_search_registrants.py b/spp_registry_search/tests/test_search_registrants.py new file mode 100644 index 000000000..7827c0f4b --- /dev/null +++ b/spp_registry_search/tests/test_search_registrants.py @@ -0,0 +1,135 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the ``search_registrants`` RPC method. + +The administrator-configurable Registry Search controls (``min_chars``, +``result_limit``, targeted search mode) were previously enforced only in the +JavaScript client. These tests pin the server-side enforcement: a caller +invoking the RPC directly must be subject to the same governance as the +portal UI, because the method searches sensitive registrant PII fields +(name, ID number, phone, email). +""" + +from odoo.tests import TransactionCase, tagged + + +@tagged("post_install", "-at_install") +class TestSearchRegistrants(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.Partner = cls.env["res.partner"] + cls.ICP = cls.env["ir.config_parameter"].sudo() + + cls.alice = cls.Partner.create( + { + "name": "Alicia Registrant", + "is_registrant": True, + "is_group": False, + } + ) + cls.alice_phone = cls.env["spp.phone.number"].create( + { + "partner_id": cls.alice.id, + "phone_no": "09171234567", + } + ) + # A pool larger than the admin limit used in the limit tests. + cls.bulk = cls.Partner.create( + [ + { + "name": f"Bulklimit Person {i:02d}", + "is_registrant": True, + "is_group": False, + } + for i in range(1, 13) + ] + ) + + def setUp(self): + super().setUp() + # Explicit baseline so tests never depend on deployment defaults. + self.ICP.set_param("spp_registry_search.search_mode", "unified") + self.ICP.set_param("spp_registry_search.target_field", "name") + self.ICP.set_param("spp_registry_search.result_limit", "50") + self.ICP.set_param("spp_registry_search.min_chars", "3") + + def _search(self, term, **kwargs): + return self.Partner.search_registrants(term, **kwargs) + + # --- min_chars enforcement -------------------------------------------------- + + def test_min_chars_enforced_server_side(self): + """A term shorter than the configured minimum must return nothing, + even when the RPC is called directly (bypassing the JS check).""" + self.assertEqual(self._search("Al"), []) + + def test_wildcard_only_term_rejected(self): + """SQL LIKE wildcards must not count toward min_chars: '%%%' is three + characters but zero effective characters and would otherwise match + every registrant.""" + self.assertEqual(self._search("%%%"), []) + + def test_wildcard_padding_does_not_satisfy_min_chars(self): + """Wildcards mixed with too few literal characters are rejected.""" + self.assertEqual(self._search("Al%"), []) + + def test_min_chars_met_returns_results(self): + """Regression: a compliant term still finds the registrant.""" + results = self._search("Alicia") + self.assertIn(self.alice.id, [r["id"] for r in results]) + + # --- result limit enforcement ---------------------------------------------- + + def test_limit_capped_by_admin_config(self): + """A caller-supplied limit must never exceed the configured maximum.""" + self.ICP.set_param("spp_registry_search.result_limit", "10") + results = self._search("Bulklimit", limit=200) + self.assertLessEqual(len(results), 10) + + def test_lower_caller_limit_honored(self): + """A caller may request fewer results than the configured maximum.""" + results = self._search("Bulklimit", limit=5) + self.assertLessEqual(len(results), 5) + + def test_non_numeric_limit_does_not_crash(self): + """A malformed limit must fall back to the configured limit, not + raise TypeError (which would surface as a generic 500).""" + results = self._search("Alicia", limit="not-a-number") + self.assertIn(self.alice.id, [r["id"] for r in results]) + + # --- targeted mode enforcement ---------------------------------------------- + + def test_targeted_mode_missing_field_stays_targeted(self): + """In targeted mode, omitting search_field must NOT fall back to + unified search over all PII fields; it must use the configured + default field instead.""" + self.ICP.set_param("spp_registry_search.search_mode", "targeted") + # Configured field is 'name'; the term only matches via phone. + self.assertEqual(self._search("09171234567", search_field=None), []) + + def test_targeted_mode_invalid_field_uses_configured_default(self): + """An unknown search_field falls back to the configured default + field rather than widening or silently returning nothing.""" + self.ICP.set_param("spp_registry_search.search_mode", "targeted") + results = self._search("Alicia", search_field="bogus") + self.assertIn(self.alice.id, [r["id"] for r in results]) + + def test_targeted_mode_caller_field_honored(self): + """Regression: the portal lets users pick a valid field in targeted + mode; a valid caller-supplied field keeps working.""" + self.ICP.set_param("spp_registry_search.search_mode", "targeted") + results = self._search("0917123", search_field="phone") + self.assertIn(self.alice.id, [r["id"] for r in results]) + + # --- misc RPC-surface hardening ---------------------------------------------- + + def test_non_string_search_term_returns_empty(self): + """A non-string term must be rejected, not flow into ilike.""" + self.assertEqual(self._search({"weird": "input"}), []) + + def test_search_type_filter_regression(self): + """Regression: search_type filtering still works.""" + results = self._search("Alicia", search_type="groups") + self.assertEqual(results, []) + results = self._search("Alicia", search_type="individuals") + self.assertIn(self.alice.id, [r["id"] for r in results]) From 6480b637fbf6d60ef2e509852dc272f91a6861a1 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:11:39 +0800 Subject: [PATCH 2/6] security(registry): enforce configured search limits server-side in search_registrants The admin-configurable Registry Search controls were applied only in the JavaScript portal; a direct RPC call could search registrant PII (name, ID number, phone, email) with a one-character or wildcard-only term despite spp_registry_search.min_chars, request up to 200 results regardless of the configured maximum, and downgrade targeted mode to unified search by omitting search_field. Enforce everything at the RPC entry point: - min_chars counted on effective (non-wildcard) characters, so '%%%' no longer matches every registrant; matching semantics are unchanged - caller limit capped at the configured result_limit (callers may request fewer results, never more), mirroring get_search_config's clamps - targeted mode never falls back to unified: a missing or unknown search_field uses the configured target_field (valid caller-supplied fields stay honored, matching the portal UX) - malformed RPC inputs (non-string term, non-numeric limit) are rejected or coerced instead of raising TypeError as a generic 500 The JS client already complies with all of these, so no client change. --- spp_registry_search/models/res_partner.py | 32 ++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/spp_registry_search/models/res_partner.py b/spp_registry_search/models/res_partner.py index a11f4303a..d8f76824a 100644 --- a/spp_registry_search/models/res_partner.py +++ b/spp_registry_search/models/res_partner.py @@ -91,18 +91,38 @@ def search_registrants(self, search_term, search_type="all", search_field=None, Returns: list: List of dicts with partner data """ - if not search_term: + # Validate RPC inputs: this method is callable directly (bypassing the + # JS portal), so malformed values must not crash into a generic 500. + if not isinstance(search_term, str) or not search_term: return [] + try: + limit = int(limit) if limit else 0 + except (TypeError, ValueError): + limit = 0 - # Read config with fallback defaults + # Read config with fallback defaults, mirroring the clamps that + # get_search_config() applies for the JS client. get_param = self.env["ir.config_parameter"].sudo().get_param # nosemgrep: odoo-sudo-without-context - config_limit = int(get_param("spp_registry_search.result_limit", "50")) - limit = max(10, min(200, limit or config_limit)) + config_limit = max(10, min(200, int(get_param("spp_registry_search.result_limit", "50")))) + min_chars = max(1, min(10, int(get_param("spp_registry_search.min_chars", "3")))) + + # Enforce the configured minimum on *effective* characters: SQL LIKE + # wildcards (%, _) are stripped before counting, otherwise a term like + # '%%%' passes a plain length check yet matches every registrant. + if len(search_term.replace("%", "").replace("_", "").strip()) < min_chars: + return [] + + # Callers may request fewer results than the configured maximum, + # never more. + limit = min(limit, config_limit) if limit > 0 else config_limit search_mode = get_param("spp_registry_search.search_mode", "unified") - # If targeted mode and a field is specified, only search that field - if search_mode == "targeted" and search_field: + if search_mode == "targeted": + # Targeted mode never widens to unified search: a missing or + # unknown field falls back to the configured default field. + if search_field not in ("name", "id_number", "phone", "email"): + search_field = get_param("spp_registry_search.target_field", "name") partner_ids = self._search_by_field(search_term, search_field, limit) else: # Unified mode: run separate queries per field and merge From 8b2be8d645d4d1ab4b0b4d0abb8e25ea50fb79bd Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:11:39 +0800 Subject: [PATCH 3/6] docs(spp_registry_search): bump version + HISTORY for server-side search limits Patch bump 19.0.2.1.1 -> 19.0.2.1.2. README.rst and static/description/index.html to be regenerated from CI's pinned generator. --- spp_registry_search/__manifest__.py | 2 +- spp_registry_search/readme/HISTORY.md | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/spp_registry_search/__manifest__.py b/spp_registry_search/__manifest__.py index a3d026b85..8925d95b1 100644 --- a/spp_registry_search/__manifest__.py +++ b/spp_registry_search/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP Registry Search Portal", "category": "OpenSPP/Registry", - "version": "19.0.2.1.1", + "version": "19.0.2.1.2", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_registry_search/readme/HISTORY.md b/spp_registry_search/readme/HISTORY.md index 1de70f102..d4fb6e3d9 100644 --- a/spp_registry_search/readme/HISTORY.md +++ b/spp_registry_search/readme/HISTORY.md @@ -1,3 +1,12 @@ +### 19.0.2.1.2 + +- fix(security): enforce the configured Registry Search controls server-side in the + `search_registrants` RPC method. The minimum-character requirement (counting effective, + non-wildcard characters), the administrator's maximum result limit, and the targeted + search mode (no fallback to unified search when the field is missing or invalid) are now + applied on the server instead of only in the JavaScript client, and malformed RPC inputs + are rejected instead of raising errors. + ### 19.0.2.1.1 - fix(security): gate the Registry Search "New Individual/Group" buttons on the registry create-permission roles instead of generic `res.partner` create access, so roles without registrant-create rights (e.g. validators) can no longer initiate creation (#1124) From 386129e575ac8b93e0f276cbb0172565ccac6277 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:46:22 +0800 Subject: [PATCH 4/6] docs(spp_registry_search): regenerate README + index.html for 19.0.2.1.2 Generated files applied verbatim from CI's pinned oca-gen-addon-readme output, matching the HISTORY.md fragment. --- spp_registry_search/README.rst | 12 ++++++++++++ spp_registry_search/static/description/index.html | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/spp_registry_search/README.rst b/spp_registry_search/README.rst index 0a19b8f1e..421f00e2c 100644 --- a/spp_registry_search/README.rst +++ b/spp_registry_search/README.rst @@ -124,6 +124,18 @@ Dependencies Changelog ========= +19.0.2.1.2 +~~~~~~~~~~ + +- fix(security): enforce the configured Registry Search controls + server-side in the ``search_registrants`` RPC method. The + minimum-character requirement (counting effective, non-wildcard + characters), the administrator's maximum result limit, and the + targeted search mode (no fallback to unified search when the field is + missing or invalid) are now applied on the server instead of only in + the JavaScript client, and malformed RPC inputs are rejected instead + of raising errors. + 19.0.2.1.1 ~~~~~~~~~~ diff --git a/spp_registry_search/static/description/index.html b/spp_registry_search/static/description/index.html index 25e1fcec2..8758867b0 100644 --- a/spp_registry_search/static/description/index.html +++ b/spp_registry_search/static/description/index.html @@ -497,6 +497,19 @@

Changelog

+

19.0.2.1.2

+
    +
  • fix(security): enforce the configured Registry Search controls +server-side in the search_registrants RPC method. The +minimum-character requirement (counting effective, non-wildcard +characters), the administrator’s maximum result limit, and the +targeted search mode (no fallback to unified search when the field is +missing or invalid) are now applied on the server instead of only in +the JavaScript client, and malformed RPC inputs are rejected instead +of raising errors.
    • +
+
+

19.0.2.1.1

  • fix(security): gate the Registry Search “New Individual/Group” buttons @@ -505,7 +518,7 @@

    19.0.2.1.1

    rights (e.g. validators) can no longer initiate creation (#1124)
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • From 8dea61bab86ebf04381745264a01df334f22c00f Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 19:42:13 +0800 Subject: [PATCH 5/6] test(spp_registry_search): pin advanced_filters crash and invalid admin target_field From staff review: a non-dict advanced_filters value raised AttributeError -> generic 500 (currently failing test), and the fail-closed behavior of targeted mode with an invalid admin-configured target_field was verified but untested. --- .../tests/test_search_registrants.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spp_registry_search/tests/test_search_registrants.py b/spp_registry_search/tests/test_search_registrants.py index 7827c0f4b..9779f19a1 100644 --- a/spp_registry_search/tests/test_search_registrants.py +++ b/spp_registry_search/tests/test_search_registrants.py @@ -127,6 +127,20 @@ def test_non_string_search_term_returns_empty(self): """A non-string term must be rejected, not flow into ilike.""" self.assertEqual(self._search({"weird": "input"}), []) + def test_non_dict_advanced_filters_does_not_crash(self): + """A malformed advanced_filters value must be ignored, not raise + AttributeError (which would surface as a generic 500).""" + results = self._search("Alicia", advanced_filters="not-a-dict") + self.assertIn(self.alice.id, [r["id"] for r in results]) + + def test_targeted_mode_invalid_admin_target_field_fails_closed(self): + """If the admin-configured target_field is itself invalid, targeted + mode must fail closed (no results) rather than widen to unified.""" + self.ICP.set_param("spp_registry_search.search_mode", "targeted") + self.ICP.set_param("spp_registry_search.target_field", "bogus") + # Term matches via phone; a fallback to unified would find it. + self.assertEqual(self._search("09171234567", search_field=None), []) + def test_search_type_filter_regression(self): """Regression: search_type filtering still works.""" results = self._search("Alicia", search_type="groups") From 178c39be31b2568a63ac8b9932fc4acb79596cc1 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 19:42:13 +0800 Subject: [PATCH 6/6] security(registry): ignore non-dict advanced_filters instead of crashing A malformed advanced_filters RPC value (string, list, int) reached .get() and raised AttributeError -> unhandled 500, the same malformed-input class this fix closes for search_term and limit. Guard with isinstance so the filter block is skipped for non-dict values. --- spp_registry_search/models/res_partner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spp_registry_search/models/res_partner.py b/spp_registry_search/models/res_partner.py index d8f76824a..70ad184bd 100644 --- a/spp_registry_search/models/res_partner.py +++ b/spp_registry_search/models/res_partner.py @@ -139,7 +139,7 @@ def search_registrants(self, search_term, search_type="all", search_field=None, elif search_type == "groups": domain.append(("is_group", "=", True)) - if advanced_filters: + if advanced_filters and isinstance(advanced_filters, dict): if advanced_filters.get("registrationDateFrom"): domain.append(("registration_date", ">=", advanced_filters["registrationDateFrom"])) if advanced_filters.get("registrationDateTo"):