diff --git a/spp_hazard/README.rst b/spp_hazard/README.rst index 4634a07c0..ff3b65dcc 100644 --- a/spp_hazard/README.rst +++ b/spp_hazard/README.rst @@ -1186,6 +1186,17 @@ encounter unexpected behavior, please report it as a new issue. Changelog ========= +19.0.2.0.3 +~~~~~~~~~~ + +- 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. + 19.0.2.0.2 ~~~~~~~~~~ diff --git a/spp_hazard/__manifest__.py b/spp_hazard/__manifest__.py index 336923eb1..97f454468 100644 --- a/spp_hazard/__manifest__.py +++ b/spp_hazard/__manifest__.py @@ -8,7 +8,7 @@ "for emergency response. Links registrants to disaster events with geographic scope " "and severity tracking to enable targeted humanitarian assistance.", "category": "OpenSPP/Targeting", - "version": "19.0.2.0.2", + "version": "19.0.2.0.3", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_hazard/readme/HISTORY.md b/spp_hazard/readme/HISTORY.md index c02593c5b..24041ef76 100644 --- a/spp_hazard/readme/HISTORY.md +++ b/spp_hazard/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.2.0.3 + +- 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. + ### 19.0.2.0.2 - fix(security): grant `group_hazard_viewer` to spp_user_roles roles (Registry Viewer, Program Manager, Global/Local Registrar) that the OP#951 menu audit identifies as needing read-only Hazard menu access. Other affected roles defined outside this module (program/CR/farm roles) are wired in their own modules. diff --git a/spp_hazard/security/ir.model.access.csv b/spp_hazard/security/ir.model.access.csv index 36c1b5f6f..826732791 100644 --- a/spp_hazard/security/ir.model.access.csv +++ b/spp_hazard/security/ir.model.access.csv @@ -3,7 +3,6 @@ access_spp_hazard_category_user,spp.hazard.category user,model_spp_hazard_catego access_spp_hazard_incident_user,spp.hazard.incident user,model_spp_hazard_incident,base.group_user,1,0,0,0 access_spp_hazard_incident_area_user,spp.hazard.incident.area user,model_spp_hazard_incident_area,base.group_user,1,0,0,0 access_spp_hazard_impact_type_user,spp.hazard.impact.type user,model_spp_hazard_impact_type,base.group_user,1,0,0,0 -access_spp_hazard_impact_user,spp.hazard.impact user,model_spp_hazard_impact,base.group_user,1,0,0,0 access_spp_hazard_category_sysadmin,Hazard Category System Admin,model_spp_hazard_category,base.group_system,1,1,1,1 access_spp_hazard_incident_sysadmin,Hazard Incident System Admin,model_spp_hazard_incident,base.group_system,1,1,1,1 access_spp_hazard_incident_area_sysadmin,Hazard Incident Area System Admin,model_spp_hazard_incident_area,base.group_system,1,1,1,1 diff --git a/spp_hazard/static/description/index.html b/spp_hazard/static/description/index.html index 9df1ed8e5..3110f6ed5 100644 --- a/spp_hazard/static/description/index.html +++ b/spp_hazard/static/description/index.html @@ -2454,6 +2454,18 @@

Changelog

+

19.0.2.0.3

+ +
+

19.0.2.0.2

-
+

19.0.2.0.1

-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_hazard/tests/__init__.py b/spp_hazard/tests/__init__.py index 9956de3b9..700f0a64e 100644 --- a/spp_hazard/tests/__init__.py +++ b/spp_hazard/tests/__init__.py @@ -6,3 +6,5 @@ from . import test_hazard_impact_type from . import test_geofence from . import test_registrant + +from . import test_acl_group_user diff --git a/spp_hazard/tests/test_acl_group_user.py b/spp_hazard/tests/test_acl_group_user.py new file mode 100644 index 000000000..4f63b8cfe --- /dev/null +++ b/spp_hazard/tests/test_acl_group_user.py @@ -0,0 +1,107 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Security: hazard models must not be readable by every internal user. + +Regression test for "Broad internal read access exposes hazard impact records": +the ACL granted ``base.group_user`` read on the hazard models, so any internal +user could read hazard data (including registrant-linked impact records) via RPC, +even without a hazard role. Access must require a dedicated hazard group (or +``registry_viewer``/admin), not merely being an internal user. +""" + +from odoo import Command +from odoo.exceptions import AccessError +from odoo.tests import tagged + +from .common import HazardTestCase + +# The registrant-linked impact model is sensitive and must NOT be readable by +# every internal user. The other hazard models are non-PII reference/operational +# data that sibling modules (e.g. spp_drims) legitimately read broadly. +SENSITIVE_MODEL = "spp.hazard.impact" +NON_SENSITIVE_MODELS = [ + "spp.hazard.category", + "spp.hazard.incident", + "spp.hazard.incident.area", + "spp.hazard.impact.type", +] +ALL_HAZARD_MODELS = [SENSITIVE_MODEL, *NON_SENSITIVE_MODELS] + + +@tagged("post_install", "-at_install") +class TestHazardBaseUserNoAccess(HazardTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.plain_user = cls.env["res.users"].create( + { + "name": "Plain Internal User", + "login": "plain_internal_hazard_test", + "group_ids": [Command.link(cls.env.ref("base.group_user").id)], + } + ) + + def test_plain_internal_user_cannot_read_impact(self): + """base.group_user (any internal user) must NOT read the sensitive impact model.""" + with self.assertRaises(AccessError): + self.env[SENSITIVE_MODEL].with_user(self.plain_user).check_access("read") + + def test_plain_internal_user_can_read_non_sensitive_models(self): + """Non-PII hazard reference/operational models remain internally readable + (sibling modules such as spp_drims depend on reading incidents).""" + for model in NON_SENSITIVE_MODELS: + # Raises AccessError only if broad read was wrongly removed here. + self.env[model].with_user(self.plain_user).check_access("read") + + def test_hazard_viewer_retains_read(self): + """A hazard-group user must keep read access to all hazard models.""" + for model in ALL_HAZARD_MODELS: + self.env[model].with_user(self.hazard_viewer).check_access("read") + + def test_registry_user_can_still_read_registrant_hazard_fields(self): + """Regression: the registrant form's hazard indicator fields read + spp.hazard.impact in their compute. A registry user (Officer implies + Registry Viewer, which retains hazard read) must still be able to load + them after the ACL tightening — i.e. the fix must not break the form.""" + officer = self.env["res.users"].create( + { + "name": "Registry Officer (no hazard group)", + "login": "registry_officer_hazard_test", + "group_ids": [Command.link(self.env.ref("spp_registry.group_registry_officer").id)], + } + ) + # Sanity: this user is NOT in any hazard group. + self.assertFalse(officer.has_group("spp_hazard.group_hazard_read")) + + incident = self.env["spp.hazard.incident"].create( + { + "name": "Registry Officer Incident", + "code": "ROI-HAZ-001", + "category_id": self.category_typhoon.id, + "start_date": "2024-01-01", + } + ) + self.env["spp.hazard.impact"].create( + { + "incident_id": incident.id, + "registrant_id": self.registrant.id, + "impact_type_id": self.impact_type_displacement.id, + "damage_level": "moderate", + "impact_date": "2024-01-02", + } + ) + registrant_as_officer = self.registrant.with_user(officer) + # Force a live read through the impact O2M (not just the stored count), + # which must not raise AccessError for a registry user. + self.assertEqual(registrant_as_officer.hazard_impact_ids.mapped("damage_level"), ["moderate"]) + + def test_incident_form_hides_impacts_from_non_hazard_user(self): + """The incident form's Impacts O2M reads spp.hazard.impact; it must be + stripped from the arch for a user without impact read (e.g. a DRIMS-only + user), so opening an incident does not raise AccessError.""" + arch = self.env["spp.hazard.incident"].with_user(self.plain_user).get_view(view_type="form")["arch"] + self.assertNotIn("impact_ids", arch) + + 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) diff --git a/spp_hazard/views/hazard_incident_views.xml b/spp_hazard/views/hazard_incident_views.xml index 50a7f6c28..a346e2569 100644 --- a/spp_hazard/views/hazard_incident_views.xml +++ b/spp_hazard/views/hazard_incident_views.xml @@ -85,6 +85,7 @@ type="object" class="oe_stat_button" icon="fa-users" + groups="spp_hazard.group_hazard_read,spp_registry.group_registry_viewer,spp_security.group_spp_admin" > - + diff --git a/spp_hazard/views/registrant_views.xml b/spp_hazard/views/registrant_views.xml index c7a0f7c7b..574fe5148 100644 --- a/spp_hazard/views/registrant_views.xml +++ b/spp_hazard/views/registrant_views.xml @@ -16,6 +16,7 @@ class="oe_stat_button" icon="fa-bolt" invisible="hazard_impact_count == 0" + groups="spp_hazard.group_hazard_read,spp_registry.group_registry_viewer,spp_security.group_spp_admin" >
    50 - + @@ -121,16 +129,20 @@ 50 - + diff --git a/spp_hazard_programs/README.rst b/spp_hazard_programs/README.rst index 0b6fdee90..2d9b09aac 100644 --- a/spp_hazard_programs/README.rst +++ b/spp_hazard_programs/README.rst @@ -324,6 +324,16 @@ Test Scenario 9: Incident List View Column Changelog ========= +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. + 19.0.2.0.0 ~~~~~~~~~~ diff --git a/spp_hazard_programs/__manifest__.py b/spp_hazard_programs/__manifest__.py index 1871df01a..dfb115f96 100644 --- a/spp_hazard_programs/__manifest__.py +++ b/spp_hazard_programs/__manifest__.py @@ -7,7 +7,7 @@ "summary": "Links hazard impacts to program eligibility and entitlements. " "Enables emergency programs to use hazard data for targeting and benefit calculation.", "category": "OpenSPP/Targeting", - "version": "19.0.2.0.0", + "version": "19.0.2.0.1", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_hazard_programs/models/program.py b/spp_hazard_programs/models/program.py index 4d48edc0e..0778b030d 100644 --- a/spp_hazard_programs/models/program.py +++ b/spp_hazard_programs/models/program.py @@ -84,8 +84,12 @@ def _compute_affected_registrant_count(self): # Build domain for qualifying damage levels damage_domain = rec._get_damage_level_domain() - # Count unique registrants with qualifying impacts - impacts = self.env["spp.hazard.impact"].search( + # Count unique registrants with qualifying impacts. + # sudo: emergency-program eligibility must consider all qualifying + # impacts regardless of the viewing user's hazard access; impact rows + # are not exposed, only the aggregate count. + impact_sudo = self.env["spp.hazard.impact"].sudo() # nosemgrep: odoo-sudo-without-context + impacts = impact_sudo.search( [ ("incident_id", "in", rec.target_incident_ids.ids), ("verification_status", "=", "verified"), @@ -123,8 +127,11 @@ def get_emergency_eligible_registrants(self): damage_domain = self._get_damage_level_domain() - # Find qualifying impacts - impacts = self.env["spp.hazard.impact"].search( + # Find qualifying impacts. + # sudo: eligibility must consider all qualifying impacts regardless of the + # viewing user's hazard access; only the resulting registrants are returned. + impact_sudo = self.env["spp.hazard.impact"].sudo() # nosemgrep: odoo-sudo-without-context + impacts = impact_sudo.search( [ ("incident_id", "in", self.target_incident_ids.ids), ("verification_status", "=", "verified"), diff --git a/spp_hazard_programs/readme/HISTORY.md b/spp_hazard_programs/readme/HISTORY.md index 4aaf9afef..329ff0b7d 100644 --- a/spp_hazard_programs/readme/HISTORY.md +++ b/spp_hazard_programs/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 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. + ### 19.0.2.0.0 - Initial migration to OpenSPP2 diff --git a/spp_hazard_programs/static/description/index.html b/spp_hazard_programs/static/description/index.html index 8a1fc0b68..68329ad5b 100644 --- a/spp_hazard_programs/static/description/index.html +++ b/spp_hazard_programs/static/description/index.html @@ -698,6 +698,17 @@

    Changelog

+

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.
    • +
+
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_hazard_programs/tests/__init__.py b/spp_hazard_programs/tests/__init__.py index 66fbca0e0..669012f1b 100644 --- a/spp_hazard_programs/tests/__init__.py +++ b/spp_hazard_programs/tests/__init__.py @@ -1,3 +1,5 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. from . import test_hazard_programs + +from . import test_program_user_access diff --git a/spp_hazard_programs/tests/test_program_user_access.py b/spp_hazard_programs/tests/test_program_user_access.py new file mode 100644 index 000000000..bdfe7bed1 --- /dev/null +++ b/spp_hazard_programs/tests/test_program_user_access.py @@ -0,0 +1,49 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Regression: emergency-eligibility logic must work for non-hazard program users. + +After tightening the hazard-impact ACL (removing the broad ``base.group_user`` +read grant), the program eligibility computes read ``spp.hazard.impact`` via +``sudo`` so a program user without any hazard group can still use them. Without +that sudo, ``affected_registrant_count`` / ``get_emergency_eligible_registrants`` +would raise ``AccessError`` for such users. +""" + +from odoo import Command +from odoo.tests import tagged + +from .common import HazardProgramsTestCase + + +@tagged("post_install", "-at_install") +class TestProgramUserHazardAccess(HazardProgramsTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.program_user = cls.env["res.users"].create( + { + "name": "Program Manager (no hazard group)", + "login": "program_mgr_no_hazard_test", + "group_ids": [ + Command.link(cls.env.ref("base.group_user").id), + Command.link(cls.env.ref("spp_programs.group_programs_manager").id), + ], + } + ) + cls.program.write( + { + "target_incident_ids": [Command.link(cls.incident_active.id)], + "qualifying_damage_levels": "any", + } + ) + + def test_program_user_without_hazard_group_can_compute_eligibility(self): + """A program user with no hazard group must still compute emergency + eligibility (the impact reads are sudo'd).""" + self.assertFalse(self.program_user.has_group("spp_hazard.group_hazard_read")) + 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() + self.assertIn(self.registrant_1, eligible) + self.assertIn(self.registrant_2, eligible)