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/tests/__init__.py b/spp_hazard/tests/__init__.py index b10eae59f..c25b529f7 100644 --- a/spp_hazard/tests/__init__.py +++ b/spp_hazard/tests/__init__.py @@ -7,3 +7,5 @@ from . import test_geofence from . import test_alert_ingestion 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 6bfba2971..1fae4f15c 100644 --- a/spp_hazard/views/hazard_incident_views.xml +++ b/spp_hazard/views/hazard_incident_views.xml @@ -84,6 +84,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" > - +

50 - + @@ -121,16 +129,20 @@ 50 - + 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/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)