From 47b7df9667141ab682455500e47a01d77c28333b Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Wed, 1 Jul 2026 11:00:04 +0800 Subject: [PATCH 1/2] security(cr): add record rules to detail models to enforce CR ownership Detail models (spp.cr.detail.*) are separate tables that do not inherit the parent spp.change.request record rules, so a low-privileged group_cr_user could read/write detail rows of change requests they do not own via RPC, tampering with pending/approved requests (e.g. re-pointing assign_program's program_id). Add per-model ir.rules mirroring the parent CR scope (users limited to own/related CRs; validators and managers unrestricted) for all 11 core detail models and the assign_program detail. Includes a completeness test asserting every concrete detail model carries a group_cr_user rule. --- spp_change_request_v2/security/rules.xml | 538 ++++++++++++++++++ spp_change_request_v2/tests/__init__.py | 2 + .../tests/test_detail_record_rules.py | 112 ++++ spp_cr_type_assign_program/__manifest__.py | 1 + spp_cr_type_assign_program/security/rules.xml | 77 +++ spp_cr_type_assign_program/tests/__init__.py | 2 + .../tests/test_detail_security.py | 99 ++++ 7 files changed, 831 insertions(+) create mode 100644 spp_change_request_v2/tests/test_detail_record_rules.py create mode 100644 spp_cr_type_assign_program/security/rules.xml create mode 100644 spp_cr_type_assign_program/tests/test_detail_security.py diff --git a/spp_change_request_v2/security/rules.xml b/spp_change_request_v2/security/rules.xml index 8d1675802..dbde95914 100644 --- a/spp_change_request_v2/security/rules.xml +++ b/spp_change_request_v2/security/rules.xml @@ -53,4 +53,542 @@ + + + + + CR Detail (add_member): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (add_member): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (add_member): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (add_member): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_individual): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (edit_individual): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_individual): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_individual): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_group): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (edit_group): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_group): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (edit_group): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (remove_member): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (remove_member): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (remove_member): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (remove_member): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (change_hoh): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (change_hoh): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (change_hoh): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (change_hoh): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (exit_registrant): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (exit_registrant): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (exit_registrant): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (exit_registrant): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (transfer_member): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (transfer_member): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (transfer_member): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (transfer_member): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (update_id): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (update_id): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (update_id): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (update_id): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (create_group): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (create_group): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (create_group): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (create_group): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (merge_registrants): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (merge_registrants): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (merge_registrants): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (merge_registrants): Manager Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (split_household): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + CR Detail (split_household): Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (split_household): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + CR Detail (split_household): Manager Access + + [(1, '=', 1)] + + + + + + diff --git a/spp_change_request_v2/tests/__init__.py b/spp_change_request_v2/tests/__init__.py index bc75e9cc0..215875233 100644 --- a/spp_change_request_v2/tests/__init__.py +++ b/spp_change_request_v2/tests/__init__.py @@ -25,3 +25,5 @@ from . import test_conflict_dynamic_approval from . import test_html_escaping from . import test_wizard_html_escaping + +from . import test_detail_record_rules diff --git a/spp_change_request_v2/tests/test_detail_record_rules.py b/spp_change_request_v2/tests/test_detail_record_rules.py new file mode 100644 index 000000000..6c179978b --- /dev/null +++ b/spp_change_request_v2/tests/test_detail_record_rules.py @@ -0,0 +1,112 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Security: CR detail models must enforce parent-CR ownership via ir.rule. + +Regression tests for the missing-record-rule vulnerability: a separate detail +model does not inherit the parent ``spp.change.request`` record rules, so a +low-privileged ``group_cr_user`` could read/write detail rows of change +requests they do not own (directly via RPC, bypassing the UI). Each concrete +detail model must ship its own ir.rule mirroring the parent's ownership scope. +""" + +from odoo.exceptions import AccessError +from odoo.tests import tagged + +from .common import CRTestCase, get_or_create_cr_type + + +@tagged("post_install", "-at_install") +class TestDetailRecordRules(CRTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.internal_group = cls.env.ref("base.group_user") + cls.user_group = cls.env.ref("spp_change_request_v2.group_cr_user") + cls.validator_group = cls.env.ref("spp_change_request_v2.group_cr_validator") + Users = cls.env["res.users"].with_context(no_reset_password=True) + cls.user_a = Users.create( + { + "name": "CR User A", + "login": "cr_detail_user_a", + "email": "cr_detail_user_a@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.user_group.id)], + } + ) + cls.user_b = Users.create( + { + "name": "CR User B", + "login": "cr_detail_user_b", + "email": "cr_detail_user_b@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.user_group.id)], + } + ) + cls.validator = Users.create( + { + "name": "CR Validator", + "login": "cr_detail_validator", + "email": "cr_detail_validator@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.validator_group.id)], + } + ) + cls.edit_type = get_or_create_cr_type(cls.env, "edit_individual") + + def _make_detail_owned_by(self, user): + """Create a CR (owned by ``user``) and return its detail record.""" + cr = self.CR.with_user(user).create( + { + "request_type_id": self.edit_type.id, + "registrant_id": self.test_individual.id, + } + ) + detail = cr.with_user(user).get_detail() + return cr, detail + + # ------------------------------------------------------------------ + # Completeness: every concrete detail model must be scoped + # ------------------------------------------------------------------ + + def test_every_concrete_detail_model_has_user_rule(self): + """Guard against a detail model shipping without an ownership rule.""" + models = self.env["ir.model"].search([("model", "=like", "spp.cr.detail.%")]) + self.assertTrue(models, "expected at least one spp.cr.detail.* model") + Rule = self.env["ir.rule"] + unscoped = [] + for model in models: + if self.env[model.model]._abstract: + continue + rules = Rule.search([("model_id", "=", model.id)]) + user_rules = rules.filtered(lambda r: self.user_group in r.groups and r.perm_read) + if not user_rules: + unscoped.append(model.model) + self.assertFalse( + unscoped, + "detail models missing a group_cr_user ir.rule (ownership bypass): %s" % ", ".join(unscoped), + ) + + # ------------------------------------------------------------------ + # Functional: cross-user isolation (edit_individual as a representative) + # ------------------------------------------------------------------ + + def test_cr_user_cannot_read_others_detail(self): + _cr, detail = self._make_detail_owned_by(self.user_a) + # A different cr_user cannot even see it via search. + found = self.env["spp.cr.detail.edit_individual"].with_user(self.user_b).search([("id", "=", detail.id)]) + self.assertFalse(found, "user B must not see user A's detail row") + # Direct read of the known id is denied. + with self.assertRaises(AccessError): + detail.with_user(self.user_b).read(["change_request_id"]) + + def test_cr_user_cannot_write_others_detail(self): + _cr, detail = self._make_detail_owned_by(self.user_a) + # Writing even a same-value field triggers the record-rule check. + with self.assertRaises(AccessError): + detail.with_user(self.user_b).write({"change_request_id": detail.change_request_id.id}) + + def test_cr_user_can_access_own_detail(self): + _cr, detail = self._make_detail_owned_by(self.user_a) + # The owner reads their own detail without error. + self.assertTrue(detail.with_user(self.user_a).read(["change_request_id"])) + + def test_validator_can_access_any_detail(self): + _cr, detail = self._make_detail_owned_by(self.user_a) + # Validators (implying cr_user) retain full visibility, matching the parent CR rule. + self.assertTrue(detail.with_user(self.validator).read(["change_request_id"])) diff --git a/spp_cr_type_assign_program/__manifest__.py b/spp_cr_type_assign_program/__manifest__.py index f81be529e..1e8b81ee6 100644 --- a/spp_cr_type_assign_program/__manifest__.py +++ b/spp_cr_type_assign_program/__manifest__.py @@ -14,6 +14,7 @@ ], "data": [ "security/ir.model.access.csv", + "security/rules.xml", "views/detail_assign_program_views.xml", "data/cr_types.xml", ], diff --git a/spp_cr_type_assign_program/security/rules.xml b/spp_cr_type_assign_program/security/rules.xml new file mode 100644 index 000000000..0b73d517d --- /dev/null +++ b/spp_cr_type_assign_program/security/rules.xml @@ -0,0 +1,77 @@ + + + + + + + CR Detail (assign_program): User Access + + [ + '|', + ('change_request_id.create_uid', '=', user.id), + ('change_request_id.registrant_id', 'in', user.partner_id.ids) + ] + + + + + + + + + + CR Detail (assign_program): Validator Access + + [(1, '=', 1)] + + + + + + + + + + CR Detail (assign_program): HQ Validator Access + + [(1, '=', 1)] + + + + + + + + + + CR Detail (assign_program): Manager Access + + [(1, '=', 1)] + + + + + + + diff --git a/spp_cr_type_assign_program/tests/__init__.py b/spp_cr_type_assign_program/tests/__init__.py index 270981145..32d199be1 100644 --- a/spp_cr_type_assign_program/tests/__init__.py +++ b/spp_cr_type_assign_program/tests/__init__.py @@ -1 +1,3 @@ from . import test_assign_program + +from . import test_detail_security diff --git a/spp_cr_type_assign_program/tests/test_detail_security.py b/spp_cr_type_assign_program/tests/test_detail_security.py new file mode 100644 index 000000000..6ccc69469 --- /dev/null +++ b/spp_cr_type_assign_program/tests/test_detail_security.py @@ -0,0 +1,99 @@ +"""Security: the assign_program detail must enforce parent-CR ownership. + +Regression test for the reported "Assign-program detail ACL bypasses CR +ownership" issue: without an ir.rule, a low-privileged ``group_cr_user`` could +re-point ``program_id`` on a change request they do not own, enrolling a +beneficiary into an unauthorized program. +""" + +from odoo.exceptions import AccessError +from odoo.tests import tagged + +from odoo.addons.spp_change_request_v2.tests.common import CRTestCase + +ASSIGN_PROGRAM_CR_TYPE_DEFS = { + "name": "Assign to Program", + "target_type": "both", + "detail_model": "spp.cr.detail.assign_program", + "apply_strategy": "custom", + "apply_model": "spp.cr.apply.assign_program", +} + + +@tagged("post_install", "-at_install") +class TestAssignProgramDetailSecurity(CRTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.cr_type = cls.CRType.search([("code", "=", "assign_program")], limit=1) + if not cls.cr_type: + cls.cr_type = cls.CRType.create({"code": "assign_program", **ASSIGN_PROGRAM_CR_TYPE_DEFS}) + + Program = cls.env["spp.program"] + cls.program_a = Program.create({"name": "Program A", "target_type": "individual"}) + cls.program_b = Program.create({"name": "Program B", "target_type": "individual"}) + + cls.internal_group = cls.env.ref("base.group_user") + cls.user_group = cls.env.ref("spp_change_request_v2.group_cr_user") + cls.validator_group = cls.env.ref("spp_change_request_v2.group_cr_validator") + Users = cls.env["res.users"].with_context(no_reset_password=True) + cls.user_a = Users.create( + { + "name": "Assign User A", + "login": "assign_user_a", + "email": "assign_user_a@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.user_group.id)], + } + ) + cls.user_b = Users.create( + { + "name": "Assign User B", + "login": "assign_user_b", + "email": "assign_user_b@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.user_group.id)], + } + ) + cls.validator = Users.create( + { + "name": "Assign Validator", + "login": "assign_validator", + "email": "assign_validator@test.com", + "group_ids": [(4, cls.internal_group.id), (4, cls.validator_group.id)], + } + ) + + def _make_cr_owned_by(self, user, program=None): + cr = self.CR.with_user(user).create( + { + "request_type_id": self.cr_type.id, + "registrant_id": self.test_individual.id, + } + ) + detail = cr.with_user(user).get_detail() + if program is not None: + detail.with_user(user).program_id = program.id + return cr, detail + + def test_cr_user_cannot_read_others_detail(self): + _cr, detail = self._make_cr_owned_by(self.user_a, self.program_a) + found = self.env["spp.cr.detail.assign_program"].with_user(self.user_b).search([("id", "=", detail.id)]) + self.assertFalse(found, "user B must not see user A's assign-program detail") + with self.assertRaises(AccessError): + detail.with_user(self.user_b).read(["program_id"]) + + def test_cr_user_cannot_repoint_program_on_others_detail(self): + """The exact reported attack: tamper with another user's program assignment.""" + _cr, detail = self._make_cr_owned_by(self.user_a, self.program_a) + with self.assertRaises(AccessError): + detail.with_user(self.user_b).write({"program_id": self.program_b.id}) + # The value is unchanged. + self.assertEqual(detail.program_id, self.program_a) + + def test_owner_can_set_program(self): + _cr, detail = self._make_cr_owned_by(self.user_a) + detail.with_user(self.user_a).write({"program_id": self.program_a.id}) + self.assertEqual(detail.program_id, self.program_a) + + def test_validator_can_read_any_detail(self): + _cr, detail = self._make_cr_owned_by(self.user_a, self.program_a) + self.assertTrue(detail.with_user(self.validator).read(["program_id"])) From d9a894702ca1cb12931410b7c26ac4fe238b0430 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Wed, 1 Jul 2026 12:41:11 +0800 Subject: [PATCH 2/2] security(cr): mirror parent area filter to detail models + harden test Post-review follow-ups: - Add a global area-filter ir.rule to every CR detail model (11 core + assign_program), mirroring the parent CR's area rule. Detail models do not inherit the parent's global rule, so an area-scoped officer could otherwise reach (via RPC) detail rows of change requests whose registrant is outside their center area. Global rule ANDs with the ownership rules; users without center areas are unaffected. - Strengthen the completeness test to require, per concrete detail model, that group_cr_user is scoped on read/write/create (not just read), that each higher role retains a permissive rule, and that a global area rule exists. - Add a functional test proving the area filter scopes a detail by its registrant's area, isolating the area dimension from ownership. --- .../security/area_filter_rules.xml | 174 ++++++++++++++++++ .../tests/test_detail_record_rules.py | 83 ++++++++- spp_cr_type_assign_program/security/rules.xml | 21 +++ 3 files changed, 269 insertions(+), 9 deletions(-) diff --git a/spp_change_request_v2/security/area_filter_rules.xml b/spp_change_request_v2/security/area_filter_rules.xml index 1a352d5c6..51425052c 100644 --- a/spp_change_request_v2/security/area_filter_rules.xml +++ b/spp_change_request_v2/security/area_filter_rules.xml @@ -31,4 +31,178 @@ can be referenced without defensive guards. + + + + + CR Detail (add_member): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (edit_individual): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (edit_group): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (remove_member): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (change_hoh): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (exit_registrant): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (transfer_member): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (update_id): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (create_group): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (merge_registrants): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + + + + CR Detail (split_household): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + + diff --git a/spp_change_request_v2/tests/test_detail_record_rules.py b/spp_change_request_v2/tests/test_detail_record_rules.py index 6c179978b..f742a0cf0 100644 --- a/spp_change_request_v2/tests/test_detail_record_rules.py +++ b/spp_change_request_v2/tests/test_detail_record_rules.py @@ -64,23 +64,88 @@ def _make_detail_owned_by(self, user): # Completeness: every concrete detail model must be scoped # ------------------------------------------------------------------ - def test_every_concrete_detail_model_has_user_rule(self): - """Guard against a detail model shipping without an ownership rule.""" + def test_every_concrete_detail_model_is_fully_scoped(self): + """Guard against a detail model shipping without complete ownership rules. + + Asserts, for every concrete ``spp.cr.detail.*`` model, that ``group_cr_user`` + is scoped on EVERY operation the ACL grants it (read/write/create) — a rule + missing only ``perm_write`` would still leave a tamper path — and that the + higher roles each retain a permissive rule (else the group hierarchy would + cage them behind the restrictive user rule). + """ models = self.env["ir.model"].search([("model", "=like", "spp.cr.detail.%")]) self.assertTrue(models, "expected at least one spp.cr.detail.* model") Rule = self.env["ir.rule"] - unscoped = [] + higher_roles = [ + ("validator", self.validator_group), + ("validator_hq", self.env.ref("spp_change_request_v2.group_cr_validator_hq")), + ("manager", self.env.ref("spp_change_request_v2.group_cr_manager")), + ] + problems = [] for model in models: if self.env[model.model]._abstract: continue rules = Rule.search([("model_id", "=", model.id)]) - user_rules = rules.filtered(lambda r: self.user_group in r.groups and r.perm_read) - if not user_rules: - unscoped.append(model.model) - self.assertFalse( - unscoped, - "detail models missing a group_cr_user ir.rule (ownership bypass): %s" % ", ".join(unscoped), + + def grants(group, perm, _rules=rules): + return any(group in r.groups and getattr(r, perm) for r in _rules) + + for perm in ("perm_read", "perm_write", "perm_create"): + if not grants(self.user_group, perm): + problems.append(f"{model.model}: group_cr_user missing {perm} rule (bypass)") + for label, group in higher_roles: + if not grants(group, "perm_read"): + problems.append(f"{model.model}: {label} missing read rule (would be caged)") + # A global (no-group) read rule mirrors the parent CR area filter. + if not any(not r.groups and r.perm_read for r in rules): + problems.append(f"{model.model}: missing global area-filter rule") + self.assertFalse(problems, "detail model rule gaps:\n " + "\n ".join(problems)) + + # ------------------------------------------------------------------ + # Functional: area scoping (mirrors the parent CR area filter) + # ------------------------------------------------------------------ + + def test_area_filter_scopes_detail_by_registrant_area(self): + """An area-scoped user cannot reach details of out-of-area CRs they own. + + Ownership is held constant (the area user creates both CRs while + unrestricted), so this isolates the area dimension: once the user is + restricted to area_1, only the in-area detail remains readable. + """ + Area = self.env["spp.area"] + area_1 = Area.create({"draft_name": "CR Detail Area 1"}) + area_2 = Area.create({"draft_name": "CR Detail Area 2"}) + reg_in = self.Partner.create( + {"name": "Reg In Area", "is_registrant": True, "is_group": False, "area_id": area_1.id} ) + reg_out = self.Partner.create( + {"name": "Reg Out Area", "is_registrant": True, "is_group": False, "area_id": area_2.id} + ) + # user_a has no center areas yet -> unrestricted create; owns both CRs. + cr_in = self.CR.with_user(self.user_a).create( + {"request_type_id": self.edit_type.id, "registrant_id": reg_in.id} + ) + cr_out = self.CR.with_user(self.user_a).create( + {"request_type_id": self.edit_type.id, "registrant_id": reg_out.id} + ) + detail_in = cr_in.with_user(self.user_a).get_detail() + detail_out = cr_out.with_user(self.user_a).get_detail() + + # Unrestricted (no center areas): both readable — global roles unaffected. + self.assertTrue(detail_out.with_user(self.user_a).read(["change_request_id"])) + + # Restrict user_a to area_1 (center_area_ids is a stored computed field; + # write it directly, after creation, to isolate the area dimension). + self.user_a.sudo().center_area_ids = [(6, 0, [area_1.id])] + self.assertEqual(self.user_a.center_area_ids, area_1) + # ir.rule evaluates and caches its domain per (model, mode); the earlier + # unrestricted read cached an empty domain, so drop the cache to pick up + # the new center-area scope (a real role change invalidates this too). + self.env.registry.clear_cache() + + self.assertTrue(detail_in.with_user(self.user_a).read(["change_request_id"])) + with self.assertRaises(AccessError): + detail_out.with_user(self.user_a).read(["change_request_id"]) # ------------------------------------------------------------------ # Functional: cross-user isolation (edit_individual as a representative) diff --git a/spp_cr_type_assign_program/security/rules.xml b/spp_cr_type_assign_program/security/rules.xml index 0b73d517d..d6bbe4a5f 100644 --- a/spp_cr_type_assign_program/security/rules.xml +++ b/spp_cr_type_assign_program/security/rules.xml @@ -74,4 +74,25 @@ + + + + CR Detail (assign_program): visible only within user's center areas + + [('change_request_id.registrant_id.area_id', 'child_of', user.center_area_ids.ids)] if user.center_area_ids else [] + + + + + +