security(registry): enforce configured search limits server-side in search_registrants

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
 ~~~~~~~~~~
 

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",

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
@@ -119,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"):

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)

spp_registry_search/static/description/index.html

@@ -497,6 +497,19 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.1.2</h1>
+<ul class="simple">
+<li>fix(security): enforce the configured Registry Search controls
+server-side in the <tt class="docutils literal">search_registrants</tt> 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.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.2.1.1</h1>
 <ul class="simple">
 <li>fix(security): gate the Registry Search “New Individual/Group” buttons
@@ -505,7 +518,7 @@ <h1>19.0.2.1.1</h1>
 rights (e.g. validators) can no longer initiate creation (#1124)</li>
 </ul>
 </div>
-<div class="section" id="section-2">
+<div class="section" id="section-3">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

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

spp_registry_search/tests/test_search_registrants.py

@@ -0,0 +1,149 @@
+# 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_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")
+        self.assertEqual(results, [])
+        results = self._search("Alicia", search_type="individuals")
+        self.assertIn(self.alice.id, [r["id"] for r in results])