security(hazard): gate stored registrant impact indicators with field-level groups (fix-up for #262)

spp_hazard/README.rst

@@ -1196,6 +1196,14 @@ Changelog
   Gate the impact UI on the registrant and incident forms (stat buttons,
   Emergency Response / Impacts pages, list columns, search filters) to
   users with impact read.
+- fix(security): guard the stored registrant indicators
+  ``res.partner.hazard_impact_count`` / ``has_active_impact`` with the
+  same field-level ``groups=`` as the impact ACL. Gating only the
+  registrant list/search views left both columns readable over RPC
+  (``search_read``, ``read_group``, export, search domains) by any
+  internal user — a per-registrant victim list. Stored computes run as
+  superuser, so partner creation by users without impact read is
+  unaffected (pinned by test).
 - fix(security): guard ``spp.hazard.incident.affected_registrant_count``
   with field-level ``groups=``. ``spp.hazard.incident`` stays broadly
   readable (sibling modules read incidents), but this aggregate is

spp_hazard/models/registrant.py

@@ -17,15 +17,26 @@ class ResPartner(models.Model):
         "spp.hazard.impact",
         "registrant_id",
         string="Hazard Impacts",
+        # Reading the O2M searches spp.hazard.impact in the user's env; gate it
+        # like the impact ACL so a bare read() of a partner by a user without
+        # impact read does not fail on this field.
+        groups="spp_hazard.group_hazard_read,spp_registry.group_registry_viewer,spp_security.group_spp_admin",
     )
+    # Both indicators are derived from the sensitive impact table and stored on
+    # the partner, so they are readable through the ORM (search_read, read_group,
+    # export, search domains) independently of the view-level gating. Field-level
+    # groups= mirrors the impact model's read ACL so a plain internal user cannot
+    # enumerate which registrants are disaster victims over RPC.
     hazard_impact_count = fields.Integer(
         compute="_compute_hazard_impact_count",
         string="Impact Count",
         store=True,
+        groups="spp_hazard.group_hazard_read,spp_registry.group_registry_viewer,spp_security.group_spp_admin",
     )
     has_active_impact = fields.Boolean(
         compute="_compute_has_active_impact",
         store=True,
+        groups="spp_hazard.group_hazard_read,spp_registry.group_registry_viewer,spp_security.group_spp_admin",
         help="Whether the registrant has an impact from an active incident",
     )
 

spp_hazard/readme/HISTORY.md

@@ -1,6 +1,7 @@
 ### 19.0.2.1.1
 
 - fix(security): remove the `base.group_user` read grant on `spp.hazard.impact` so registrant-linked impact records (name, damage level, verification, notes) are readable only by hazard roles, `registry_viewer`, and admins — not every internal user via RPC. Gate the impact UI on the registrant and incident forms (stat buttons, Emergency Response / Impacts pages, list columns, search filters) to users with impact read.
+- fix(security): guard the stored registrant indicators `res.partner.hazard_impact_count` / `has_active_impact` with the same field-level `groups=` as the impact ACL. Gating only the registrant list/search views left both columns readable over RPC (`search_read`, `read_group`, export, search domains) by any internal user — a per-registrant victim list. Stored computes run as superuser, so partner creation by users without impact read is unaffected (pinned by test).
 - fix(security): guard `spp.hazard.incident.affected_registrant_count` with field-level `groups=`. `spp.hazard.incident` stays broadly readable (sibling modules read incidents), but this aggregate is derived from the sensitive impact table via raw ACL-bypassing SQL, so a plain internal user could read the affected-registrant count over RPC even without impact read. The field is now restricted to hazard read / `registry_viewer` / admin, which also strips it from the incident list column for other users.
 
 ### 19.0.2.1.0

spp_hazard/static/description/index.html

@@ -2463,6 +2463,14 @@ <h1>19.0.2.1.1</h1>
 Gate the impact UI on the registrant and incident forms (stat buttons,
 Emergency Response / Impacts pages, list columns, search filters) to
 users with impact read.</li>
+<li>fix(security): guard the stored registrant indicators
+<tt class="docutils literal">res.partner.hazard_impact_count</tt> / <tt class="docutils literal">has_active_impact</tt> with the
+same field-level <tt class="docutils literal">groups=</tt> as the impact ACL. Gating only the
+registrant list/search views left both columns readable over RPC
+(<tt class="docutils literal">search_read</tt>, <tt class="docutils literal">read_group</tt>, export, search domains) by any
+internal user — a per-registrant victim list. Stored computes run as
+superuser, so partner creation by users without impact read is
+unaffected (pinned by test).</li>
 <li>fix(security): guard <tt class="docutils literal">spp.hazard.incident.affected_registrant_count</tt>
 with field-level <tt class="docutils literal">groups=</tt>. <tt class="docutils literal">spp.hazard.incident</tt> stays broadly
 readable (sibling modules read incidents), but this aggregate is

spp_hazard/tests/test_acl_group_user.py

@@ -165,3 +165,77 @@ def test_incident_form_shows_impacts_to_hazard_user(self):
         """A hazard user still gets the Impacts O2M on the incident form."""
         arch = self.env["spp.hazard.incident"].with_user(self.hazard_viewer).get_view(view_type="form")["arch"]
         self.assertIn("impact_ids", arch)
+
+    def test_plain_internal_user_cannot_read_registrant_impact_fields(self):
+        """``res.partner.hazard_impact_count`` / ``has_active_impact`` are stored
+        columns derived from the sensitive impact table. Gating only the views is
+        not enough: a plain internal user could still ``search_read`` them over RPC
+        and enumerate which registrants are disaster victims. The fields must carry
+        field-level ``groups=`` so the ORM refuses the read."""
+        partner_as_plain = self.env["res.partner"].with_user(self.plain_user)
+        with self.assertRaises(AccessError):
+            partner_as_plain.search_read(
+                [("id", "=", self.registrant.id)],
+                ["name", "hazard_impact_count", "has_active_impact"],
+            )
+        # The headline attack is the domain, not the field list: filtering or
+        # ordering on the gated columns must be refused too (presence oracle).
+        with self.assertRaises(AccessError):
+            partner_as_plain.search([("has_active_impact", "=", True)])
+        with self.assertRaises(AccessError):
+            partner_as_plain.search([("hazard_impact_count", ">", 0)])
+        with self.assertRaises(AccessError):
+            partner_as_plain.search([("id", "=", self.registrant.id)], order="hazard_impact_count desc")
+        with self.assertRaises(AccessError):
+            partner_as_plain.read_group([], ["hazard_impact_count:sum"], ["has_active_impact"])
+        # The O2M itself must not be reachable either, and all three must be
+        # hidden from fields_get(): that is what makes a bare read() with no
+        # field list (generic RPC clients) skip them instead of failing on them.
+        with self.assertRaises(AccessError):
+            partner_as_plain.search_read([("id", "=", self.registrant.id)], ["hazard_impact_ids"])
+        visible = partner_as_plain.fields_get(["hazard_impact_ids", "hazard_impact_count", "has_active_impact"])
+        self.assertEqual(visible, {})
+        # And the gated columns/filters are stripped from the list/search arch.
+        arch = partner_as_plain.get_view(view_type="list")["arch"]
+        self.assertNotIn("hazard_impact_count", arch)
+        self.assertNotIn("has_active_impact", arch)
+
+    def test_hazard_viewer_can_read_registrant_impact_fields(self):
+        """A hazard-group user keeps read on the registrant impact indicator fields."""
+        rows = (
+            self.env["res.partner"]
+            .with_user(self.hazard_viewer)
+            .search_read([("id", "=", self.registrant.id)], ["hazard_impact_count", "has_active_impact"])
+        )
+        self.assertEqual(len(rows), 1)
+
+    def test_contact_creator_without_impact_read_can_create_partner(self):
+        """Guard: the two stored impact indicators are computed on every
+        ``res.partner`` create by querying ``spp.hazard.impact``. Stored computes
+        run as superuser (``compute_sudo`` defaults to True for stored fields), so
+        removing ``base.group_user`` read on impacts must NOT break partner
+        creation for an internal user who may create contacts but holds no
+        hazard/registry role (e.g. Contact Creation only). Pins that behaviour,
+        including the flush that actually runs the compute."""
+        creator = self.env["res.users"].create(
+            {
+                "name": "Contact Creator (no hazard/registry group)",
+                "login": "contact_creator_hazard_test",
+                "group_ids": [
+                    Command.link(self.env.ref("base.group_user").id),
+                    Command.link(self.env.ref("base.group_partner_manager").id),
+                ],
+            }
+        )
+        self.assertFalse(creator.has_group("spp_hazard.group_hazard_read"))
+        self.assertFalse(creator.has_group("spp_registry.group_registry_viewer"))
+        with self.assertRaises(AccessError):
+            self.env[SENSITIVE_MODEL].with_user(creator).check_access("read")
+
+        partner = self.env["res.partner"].with_user(creator).create({"name": "Created by contact creator"})
+        # Stored computes are deferred to flush time; flush in the CREATOR's env
+        # (as a real request does at its end) so the compute runs as that user.
+        partner.env.flush_all()
+        self.assertTrue(partner.exists())
+        self.assertEqual(partner.sudo().hazard_impact_count, 0)
+        self.assertFalse(partner.sudo().has_active_impact)

spp_hazard_programs/README.rst

@@ -87,7 +87,7 @@ access from:
 Extension Points
 ~~~~~~~~~~~~~~~~
 
-- Override ``get_emergency_eligible_registrants()`` to customize
+- Override ``_get_emergency_eligible_registrants()`` to customize
   eligibility logic beyond damage levels
 - Override ``_get_damage_level_domain()`` to add custom damage filtering
   rules
@@ -331,8 +331,14 @@ Changelog
   emergency-eligibility computes (``affected_registrant_count``,
   ``get_emergency_eligible_registrants``), so they keep working for
   non-hazard program users after impact read access was restricted to
-  hazard/registry roles. Only aggregate counts / eligible registrants
-  are surfaced, not impact rows.
+  hazard/registry roles. Only aggregate counts are surfaced to program
+  users without impact read; the list of eligible (impacted) registrants
+  is the identity linkage the impact ACL protects, so
+  ``action_view_affected_registrants`` now checks impact read access
+  server-side, its stat button is gated in the form, and
+  ``get_emergency_eligible_registrants()`` is renamed
+  ``_get_emergency_eligible_registrants()`` so it is no longer callable
+  over RPC (Python callers and overrides are unaffected).
 
 19.0.2.0.0
 ~~~~~~~~~~

spp_hazard_programs/models/program.py

@@ -113,10 +113,16 @@ def _get_damage_level_domain(self):
             return [("damage_level", "in", ("critical", "totally_damaged"))]
         return []
 
-    def get_emergency_eligible_registrants(self):
+    def _get_emergency_eligible_registrants(self):
         """
         Get registrants eligible for this emergency program based on hazard impacts.
 
+        Private on purpose: the result is the list of registrants affected by a
+        hazard, i.e. the identity linkage the impact ACL protects. It is meant
+        for Python callers (eligibility logic, overrides), not for RPC. The UI
+        entry point is ``action_view_affected_registrants``, which checks impact
+        read access before exposing the list.
+
         Returns registrants who:
         - Have verified impact from one of the target incidents
         - Meet the qualifying damage level threshold
@@ -159,9 +165,15 @@ def action_view_target_incidents(self):
         }
 
     def action_view_affected_registrants(self):
-        """Open a list view of potentially affected registrants."""
+        """Open a list view of potentially affected registrants.
+
+        The aggregate count stays visible to every program user, but the list
+        names the impacted registrants, so it requires impact read access. The
+        stat button is gated in the view; this check covers RPC callers.
+        """
         self.ensure_one()
-        registrants = self.get_emergency_eligible_registrants()
+        self.env["spp.hazard.impact"].check_access("read")
+        registrants = self._get_emergency_eligible_registrants()
         return {
             "name": _("Affected Registrants - %s", self.name),
             "type": "ir.actions.act_window",

spp_hazard_programs/readme/DESCRIPTION.md

@@ -39,7 +39,7 @@ No new models or ACL entries. Fields added to existing models inherit access fro
 
 ### Extension Points
 
-- Override `get_emergency_eligible_registrants()` to customize eligibility logic beyond damage levels
+- Override `_get_emergency_eligible_registrants()` to customize eligibility logic beyond damage levels
 - Override `_get_damage_level_domain()` to add custom damage filtering rules
 - Inherit `spp.program` to add fields used in emergency calculations
 - Use `is_emergency_program` and `is_emergency_mode` flags in downstream program logic

spp_hazard_programs/readme/HISTORY.md

@@ -1,6 +1,6 @@
 ### 19.0.2.0.1
 
-- fix(security): read `spp.hazard.impact` via `sudo` in the emergency-eligibility computes (`affected_registrant_count`, `get_emergency_eligible_registrants`), so they keep working for non-hazard program users after impact read access was restricted to hazard/registry roles. Only aggregate counts / eligible registrants are surfaced, not impact rows.
+- fix(security): read `spp.hazard.impact` via `sudo` in the emergency-eligibility computes (`affected_registrant_count`, `get_emergency_eligible_registrants`), so they keep working for non-hazard program users after impact read access was restricted to hazard/registry roles. Only aggregate counts are surfaced to program users without impact read; the list of eligible (impacted) registrants is the identity linkage the impact ACL protects, so `action_view_affected_registrants` now checks impact read access server-side, its stat button is gated in the form, and `get_emergency_eligible_registrants()` is renamed `_get_emergency_eligible_registrants()` so it is no longer callable over RPC (Python callers and overrides are unaffected).
 
 ### 19.0.2.0.0
 

spp_hazard_programs/static/description/index.html

@@ -447,7 +447,7 @@ <h1>Security</h1>
 <div class="section" id="extension-points">
 <h1>Extension Points</h1>
 <ul class="simple">
-<li>Override <tt class="docutils literal">get_emergency_eligible_registrants()</tt> to customize
+<li>Override <tt class="docutils literal">_get_emergency_eligible_registrants()</tt> to customize
 eligibility logic beyond damage levels</li>
 <li>Override <tt class="docutils literal">_get_damage_level_domain()</tt> to add custom damage filtering
 rules</li>
@@ -704,8 +704,14 @@ <h1>19.0.2.0.1</h1>
 emergency-eligibility computes (<tt class="docutils literal">affected_registrant_count</tt>,
 <tt class="docutils literal">get_emergency_eligible_registrants</tt>), so they keep working for
 non-hazard program users after impact read access was restricted to
-hazard/registry roles. Only aggregate counts / eligible registrants
-are surfaced, not impact rows.</li>
+hazard/registry roles. Only aggregate counts are surfaced to program
+users without impact read; the list of eligible (impacted) registrants
+is the identity linkage the impact ACL protects, so
+<tt class="docutils literal">action_view_affected_registrants</tt> now checks impact read access
+server-side, its stat button is gated in the form, and
+<tt class="docutils literal">get_emergency_eligible_registrants()</tt> is renamed
+<tt class="docutils literal">_get_emergency_eligible_registrants()</tt> so it is no longer callable
+over RPC (Python callers and overrides are unaffected).</li>
 </ul>
 </div>
 <div class="section" id="section-2">

spp_hazard_programs/tests/test_hazard_programs.py

@@ -137,7 +137,7 @@ def test_get_emergency_eligible_registrants(self):
                 "qualifying_damage_levels": "any",
             }
         )
-        registrants = self.program.get_emergency_eligible_registrants()
+        registrants = self.program._get_emergency_eligible_registrants()
         self.assertEqual(len(registrants), 2)
         self.assertIn(self.registrant_1, registrants)
         self.assertIn(self.registrant_2, registrants)
@@ -146,7 +146,7 @@ def test_get_emergency_eligible_registrants(self):
 
     def test_get_emergency_eligible_registrants_no_incidents(self):
         """Test eligible registrants returns empty when no incidents linked."""
-        registrants = self.program.get_emergency_eligible_registrants()
+        registrants = self.program._get_emergency_eligible_registrants()
         self.assertEqual(len(registrants), 0)
 
     def test_get_emergency_eligible_registrants_with_filter(self):
@@ -157,7 +157,7 @@ def test_get_emergency_eligible_registrants_with_filter(self):
                 "qualifying_damage_levels": "critical_only",
             }
         )
-        registrants = self.program.get_emergency_eligible_registrants()
+        registrants = self.program._get_emergency_eligible_registrants()
         self.assertEqual(len(registrants), 1)
         self.assertIn(self.registrant_1, registrants)
 

spp_hazard_programs/tests/test_program_user_access.py

@@ -43,7 +43,47 @@ def test_program_user_without_hazard_group_can_compute_eligibility(self):
         program = self.program.with_user(self.program_user)
         # Non-stored compute -> runs live as this user; reads impact via sudo.
         self.assertEqual(program.affected_registrant_count, 2)
-        # Method -> runs live as this user; reads impact via sudo.
-        eligible = program.get_emergency_eligible_registrants()
+        # Method -> runs live as this user; reads impact via sudo. Private so it
+        # is reachable from Python (eligibility, overrides) but not over RPC.
+        eligible = program._get_emergency_eligible_registrants()
         self.assertIn(self.registrant_1, eligible)
         self.assertIn(self.registrant_2, eligible)
+
+    def test_eligible_registrants_method_is_not_rpc_callable(self):
+        """The eligible-registrant list is the identity linkage the impact ACL
+        protects. It must not be exposed as a public (call_kw-reachable) method."""
+        self.assertFalse(hasattr(type(self.program), "get_emergency_eligible_registrants"))
+        self.assertTrue(hasattr(type(self.program), "_get_emergency_eligible_registrants"))
+
+    def test_program_user_without_impact_read_cannot_open_affected_registrants(self):
+        """The 'Affected' stat button opens the list of impacted registrants. A
+        program user without impact read keeps the aggregate count but must be
+        refused the list, server-side (buttons are RPC-callable) and in the arch."""
+        from odoo.exceptions import AccessError
+
+        program = self.program.with_user(self.program_user)
+        self.assertEqual(program.affected_registrant_count, 2)
+        with self.assertRaises(AccessError):
+            program.action_view_affected_registrants()
+        arch = self.env["spp.program"].with_user(self.program_user).get_view(view_type="form")["arch"]
+        self.assertNotIn("action_view_affected_registrants", arch)
+
+    def test_hazard_user_can_open_affected_registrants(self):
+        """A user with impact read (hazard viewer + program manager) keeps the list."""
+        hazard_program_user = self.env["res.users"].create(
+            {
+                "name": "Program Manager with hazard read",
+                "login": "program_mgr_hazard_read_test",
+                "group_ids": [
+                    Command.link(self.env.ref("base.group_user").id),
+                    Command.link(self.env.ref("spp_programs.group_programs_manager").id),
+                    Command.link(self.env.ref("spp_hazard.group_hazard_viewer").id),
+                ],
+            }
+        )
+        program = self.program.with_user(hazard_program_user)
+        action = program.action_view_affected_registrants()
+        self.assertEqual(action["res_model"], "res.partner")
+        self.assertEqual(set(action["domain"][0][2]), {self.registrant_1.id, self.registrant_2.id})
+        arch = self.env["spp.program"].with_user(hazard_program_user).get_view(view_type="form")["arch"]
+        self.assertIn("action_view_affected_registrants", arch)