From 8236450fde3fb8b06f87563678db0216d7096519 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:14:18 +0800 Subject: [PATCH 1/9] test(security): pin that Force Unlock requires system admin server-side TDD red: action_force_unlock on spp.cycle/spp.program has no server-side authorization, so officers, managers and cycle approvers (who hold write ACL) can clear an active operation lock via RPC. New tests assert the call raises AccessError for those roles and leaves the lock set, while system admins and trusted sudo() flows still succeed. --- spp_programs/tests/__init__.py | 1 + spp_programs/tests/test_force_unlock_authz.py | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 spp_programs/tests/test_force_unlock_authz.py diff --git a/spp_programs/tests/__init__.py b/spp_programs/tests/__init__.py index 15dc1cbe6..24988efbe 100644 --- a/spp_programs/tests/__init__.py +++ b/spp_programs/tests/__init__.py @@ -39,6 +39,7 @@ from . import test_concurrency from . import test_manager_summary_formatting from . import test_async_lock_recovery +from . import test_force_unlock_authz from . import test_membership_acl_bypass from . import test_cycle_null_entitlement_approval from . import test_approve_entitlements_program_isolation diff --git a/spp_programs/tests/test_force_unlock_authz.py b/spp_programs/tests/test_force_unlock_authz.py new file mode 100644 index 000000000..f3bc44891 --- /dev/null +++ b/spp_programs/tests/test_force_unlock_authz.py @@ -0,0 +1,113 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Server-side authorization for the Force Unlock escape hatch. + +The Force Unlock buttons on the cycle/program forms are gated to +``base.group_system`` in XML, but that only hides the button — Odoo lets +any user reach ``action_force_unlock`` through RPC/``call_kw``. Because +program officers, managers and cycle approvers hold write access on +``spp.program`` / ``spp.cycle``, without a server-side check they could +clear an active operation lock while async entitlement / payment / +eligibility jobs are still running. These tests pin that only system +administrators (and trusted ``sudo()`` flows) may force-unlock. +""" + +import uuid + +from odoo import fields +from odoo.exceptions import AccessError +from odoo.tests import TransactionCase + + +def _new_program(env): + return ( + env["spp.program"] + .with_context(create_default_managers=True) + .create({"name": f"Force Unlock Authz {uuid.uuid4().hex[:8]}"}) + ) + + +def _new_cycle(env, program): + today = fields.Date.today() + return env["spp.cycle"].create( + { + "name": f"Force Unlock Authz Cycle {uuid.uuid4().hex[:8]}", + "program_id": program.id, + "sequence": 1, + "start_date": today, + "end_date": fields.Date.add(today, days=30), + } + ) + + +class TestForceUnlockAuthorization(TransactionCase): + def setUp(self): + super().setUp() + self.program = _new_program(self.env) + self.cycle = _new_cycle(self.env, self.program) + + def _user(login, group_xmlids): + groups = [self.env.ref("base.group_user")] + groups += [self.env.ref(x) for x in group_xmlids] + return self.env["res.users"].create( + { + "name": login, + "login": login, + "group_ids": [(6, 0, [g.id for g in groups])], + } + ) + + self.officer = _user("fu_officer", ["spp_programs.group_programs_officer"]) + self.manager = _user("fu_manager", ["spp_programs.group_programs_manager"]) + self.approver = _user("fu_approver", ["spp_programs.group_programs_cycle_approver"]) + self.system = _user("fu_system", ["base.group_system"]) + + # --- cycle --------------------------------------------------------- + + def _lock_cycle(self): + self.cycle.write({"is_locked": True, "locked_reason": "Import running"}) + + def test_cycle_force_unlock_denied_for_officer(self): + self._lock_cycle() + with self.assertRaises(AccessError): + self.cycle.with_user(self.officer).action_force_unlock() + self.assertTrue(self.cycle.is_locked, "lock must remain set after a denied call") + + def test_cycle_force_unlock_denied_for_manager(self): + self._lock_cycle() + with self.assertRaises(AccessError): + self.cycle.with_user(self.manager).action_force_unlock() + self.assertTrue(self.cycle.is_locked) + + def test_cycle_force_unlock_denied_for_cycle_approver(self): + self._lock_cycle() + with self.assertRaises(AccessError): + self.cycle.with_user(self.approver).action_force_unlock() + self.assertTrue(self.cycle.is_locked) + + def test_cycle_force_unlock_allowed_for_system_admin(self): + self._lock_cycle() + before = len(self.cycle.message_ids) + self.cycle.with_user(self.system).action_force_unlock() + self.assertFalse(self.cycle.is_locked) + self.assertFalse(self.cycle.locked_reason) + self.assertGreater(len(self.cycle.message_ids), before) + + def test_cycle_force_unlock_allowed_via_sudo(self): + """Trusted server-side sudo() flows are not blocked by the guard.""" + self._lock_cycle() + self.cycle.with_user(self.officer).sudo().action_force_unlock() + self.assertFalse(self.cycle.is_locked) + + # --- program ------------------------------------------------------- + + def test_program_force_unlock_denied_for_manager(self): + self.program.write({"is_locked": True, "locked_reason": "Enrollment running"}) + with self.assertRaises(AccessError): + self.program.with_user(self.manager).action_force_unlock() + self.assertTrue(self.program.is_locked) + + def test_program_force_unlock_allowed_for_system_admin(self): + self.program.write({"is_locked": True, "locked_reason": "Enrollment running"}) + self.program.with_user(self.system).action_force_unlock() + self.assertFalse(self.program.is_locked) + self.assertFalse(self.program.locked_reason) From 98b95d68037d234b9bebb27af7d36626b9525787 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:23:15 +0800 Subject: [PATCH 2/9] security(programs): enforce system-admin authorization on Force Unlock action_force_unlock on spp.cycle and spp.program cleared an active operation lock (is_locked) with no server-side authorization. The view buttons are gated to base.group_system, but object methods are reachable via RPC/call_kw regardless of button visibility, and officers, managers and cycle approvers hold write ACL on these models - so any of them could force-clear a lock while async entitlement/payment/eligibility jobs were still running, enabling conflicting operations and integrity/availability issues. - guard both methods with base.group_system (fail fast, before the loop), mirroring the existing has_group/AccessError idiom in cycle.py; exempt self.env.su so trusted server-side sudo() flows are not blocked - correct the misleading 'manager-only' docstrings and test comment to system-administrator-only - no ACL change (write is legitimate for normal cycle/program work; the method is the exposure) and no migration (runtime check, no schema/data change) - bump to 19.0.2.2.1 with HISTORY fragment --- spp_programs/__manifest__.py | 2 +- spp_programs/models/cycle.py | 18 ++++++++++++++++- spp_programs/models/programs.py | 20 +++++++++++++++++-- spp_programs/readme/HISTORY.md | 10 ++++++++++ .../tests/test_async_lock_recovery.py | 4 ++-- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/spp_programs/__manifest__.py b/spp_programs/__manifest__.py index 8dac1cba2..48590c716 100644 --- a/spp_programs/__manifest__.py +++ b/spp_programs/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Programs", "summary": "Manage programs, cycles, beneficiary enrollment, entitlements (cash and in-kind), payments, and fund tracking for social protection.", "category": "OpenSPP/Core", - "version": "19.0.2.2.0", + "version": "19.0.2.2.1", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_programs/models/cycle.py b/spp_programs/models/cycle.py index 21b0b2df7..e72b6a8cb 100644 --- a/spp_programs/models/cycle.py +++ b/spp_programs/models/cycle.py @@ -1084,13 +1084,29 @@ def _get_related_job_domain(self): return [("id", "in", related_jobs.ids)] def action_force_unlock(self): - """Manager-only escape hatch: clear a stuck "Operation in progress" lock. + """System-administrator-only escape hatch: clear a stuck "Operation + in progress" lock. Use when an async pipeline (entitlement processing, payment prep, etc.) died without firing its on_done/on_error callback — for example after a hard server restart or before this fix was deployed. Posts an audit line to chatter so admins can see who unstuck the cycle. + + The view button is gated to base.group_system, but object methods are + reachable via RPC regardless of button visibility, so the same + restriction is enforced here: clearing an active operation lock while + jobs may still be running is an emergency control reserved for system + administrators. Trusted server-side sudo() flows are exempt. """ + if not self.env.su and not self.env.user.has_group("base.group_system"): + raise AccessError( + _( + "Force unlock is restricted to system administrators. It is an " + "emergency control for clearing a stuck operation lock; ask a " + "system administrator to confirm the async job has actually " + "stopped before the lock is cleared." + ) + ) for rec in self: if not rec.is_locked: continue diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index 5841fb332..bdde14bba 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -2,7 +2,7 @@ import logging from odoo import _, api, fields, models -from odoo.exceptions import UserError +from odoo.exceptions import AccessError, UserError from . import constants @@ -764,11 +764,27 @@ def _get_related_job_domain(self): return [("id", "in", related_jobs.ids)] def action_force_unlock(self): - """Manager-only escape hatch: clear a stuck "Operation in progress" lock. + """System-administrator-only escape hatch: clear a stuck "Operation + in progress" lock. Use when an async pipeline died without firing its on_done/on_error callback. Posts an audit line to chatter for traceability. + + The view button is gated to base.group_system, but object methods are + reachable via RPC regardless of button visibility, so the same + restriction is enforced here: clearing an active operation lock while + jobs may still be running is an emergency control reserved for system + administrators. Trusted server-side sudo() flows are exempt. """ + if not self.env.su and not self.env.user.has_group("base.group_system"): + raise AccessError( + _( + "Force unlock is restricted to system administrators. It is an " + "emergency control for clearing a stuck operation lock; ask a " + "system administrator to confirm the async job has actually " + "stopped before the lock is cleared." + ) + ) for rec in self: if not rec.is_locked: continue diff --git a/spp_programs/readme/HISTORY.md b/spp_programs/readme/HISTORY.md index 826b3233a..0d3608e3e 100644 --- a/spp_programs/readme/HISTORY.md +++ b/spp_programs/readme/HISTORY.md @@ -1,3 +1,13 @@ +### 19.0.2.2.1 + +- fix(security): enforce server-side authorization on Force Unlock. The + `action_force_unlock` methods on `spp.cycle` and `spp.program` cleared an + active operation lock with no group check, so any role holding write + access (program officers, managers, cycle approvers) could bypass the + emergency-only control via RPC and clear a lock while async jobs were + still running. The methods now require `base.group_system` (trusted + `sudo()` flows exempt), matching the view-button gating. + ### 19.0.2.1.3 - fix(security): align Program Viewer / Validator / Cycle Approver roles with the OP#951 menu audit — Program Viewer additionally gets `group_registry_viewer` + `group_approval_viewer` (read-only Registry + Approvals access); all three program roles get `group_hazard_viewer` + `group_gis_report_user` so they retain Hazard / GIS Reports visibility once those menu roots are gated. Adds `spp_hazard` and `spp_gis_report` to module dependencies. diff --git a/spp_programs/tests/test_async_lock_recovery.py b/spp_programs/tests/test_async_lock_recovery.py index da6406a05..0855f5a8b 100644 --- a/spp_programs/tests/test_async_lock_recovery.py +++ b/spp_programs/tests/test_async_lock_recovery.py @@ -11,8 +11,8 @@ - the new `mark_*_as_failed` companions clear the lock too - the existing `mark_*_as_done` paths clear the lock first (so a chatter failure can't leave the lock set) -- `action_force_unlock` is a manager-only escape hatch when no callback - fires at all (e.g. server killed mid-operation) +- `action_force_unlock` is a system-administrator-only escape hatch when + no callback fires at all (e.g. server killed mid-operation) """ import uuid From f5ff0a73a9c6b8c91ad8664810e0773e7a5474e4 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 18:32:38 +0800 Subject: [PATCH 3/9] docs(spp_programs): regenerate README from fragments (CI oca-gen output) --- spp_programs/README.rst | 12 +++++++ spp_programs/static/description/index.html | 41 ++++++++++++++-------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/spp_programs/README.rst b/spp_programs/README.rst index c05dac423..ebe45c72b 100644 --- a/spp_programs/README.rst +++ b/spp_programs/README.rst @@ -254,6 +254,18 @@ Dependencies Changelog ========= +19.0.2.2.1 +~~~~~~~~~~ + +- fix(security): enforce server-side authorization on Force Unlock. The + ``action_force_unlock`` methods on ``spp.cycle`` and ``spp.program`` + cleared an active operation lock with no group check, so any role + holding write access (program officers, managers, cycle approvers) + could bypass the emergency-only control via RPC and clear a lock while + async jobs were still running. The methods now require + ``base.group_system`` (trusted ``sudo()`` flows exempt), matching the + view-button gating. + 19.0.2.1.3 ~~~~~~~~~~ diff --git a/spp_programs/static/description/index.html b/spp_programs/static/description/index.html index 50e35cbb3..a9bd37e97 100644 --- a/spp_programs/static/description/index.html +++ b/spp_programs/static/description/index.html @@ -658,6 +658,19 @@

Changelog

+

19.0.2.2.1

+
    +
  • fix(security): enforce server-side authorization on Force Unlock. The +action_force_unlock methods on spp.cycle and spp.program +cleared an active operation lock with no group check, so any role +holding write access (program officers, managers, cycle approvers) +could bypass the emergency-only control via RPC and clear a lock while +async jobs were still running. The methods now require +base.group_system (trusted sudo() flows exempt), matching the +view-button gating.
  • +
+
+

19.0.2.1.3

  • fix(security): align Program Viewer / Validator / Cycle Approver roles @@ -676,7 +689,7 @@

    19.0.2.1.3

    cross-references — only the dedicated top-level menu disappears.
-
+

19.0.2.1.2

  • fix(security): add global ir.rule records on @@ -690,7 +703,7 @@

    19.0.2.1.2

    no-op for users with no center areas (global roles).
-
+

19.0.2.1.1

  • fix(views): apply spp_registry.x2many_no_padding widget to the @@ -699,7 +712,7 @@

    19.0.2.1.1

    19 inserts on inline list-in-form views (#943).
-
+

19.0.2.0.11

  • Fix TypeError: 'NoneType' object is not iterable when clicking @@ -710,7 +723,7 @@

    19.0.2.0.11

    omit the state filter instead of crashing on tuple(None)
-
+

19.0.2.0.10

  • Increase parallel-safe channel limits (cycle, eligibility_manager, @@ -723,7 +736,7 @@

    19.0.2.0.10

    submission on double-click
-
+

19.0.2.0.9

  • Add context flags (skip_registrant_statistics, @@ -736,7 +749,7 @@

    19.0.2.0.9

    _compute_has_members
-
+

19.0.2.0.8

  • Replace OFFSET pagination with NTILE-based ID-range batching in all @@ -747,7 +760,7 @@

    19.0.2.0.8

    program and cycle
-
+

19.0.2.0.7

  • Bulk membership creation using raw SQL INSERT ON CONFLICT DO NOTHING @@ -756,7 +769,7 @@

    19.0.2.0.7

    _add_beneficiaries with bulk SQL path
-
+

19.0.2.0.6

  • Remove unused entitlement_base_model.py (dead code, never imported)
  • @@ -765,34 +778,34 @@

    19.0.2.0.6

    payment, and fund tests (172 → 492 tests)
-
+

19.0.2.0.5

  • Batch create entitlements and payments instead of one-by-one ORM creates
-
+

19.0.2.0.4

  • Fetch fund balance once per approval batch instead of per entitlement
-
+

19.0.2.0.3

  • Replace cycle computed fields (total_amount, entitlements_count, approval flags) with SQL aggregation queries
-
+

19.0.2.0.2

  • Add composite indexes for frequent query patterns on entitlements and program memberships
-
+

19.0.2.0.1

  • Replace Python-level uniqueness checks with SQL UNIQUE constraints for @@ -801,7 +814,7 @@

    19.0.2.0.1

    constraint creation
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
  • From 9dd280033ecf2343308792b249e122b1114d2af7 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 19:02:03 +0800 Subject: [PATCH 4/9] security(programs): make the operation lock a real server-side boundary Follows the staff review of the initial method-only guard: is_locked / locked_reason were plain writable fields, so officers/managers/approvers (who hold write ACL) could bypass the action_force_unlock guard entirely with a direct write({'is_locked': False}) over RPC. - add a write() guard on spp.cycle and spp.program rejecting direct changes to is_locked/locked_reason unless self.env.su or the caller is base.group_system; this is the actual sink behind Force Unlock - route the async pipeline's lock acquire/release through new _acquire_operation_lock/_release_operation_lock helpers that sudo(), so the ~25 manager call sites (entitlement/payment/eligibility/cycle) keep working when the job runs as the initiating non-admin user - action_force_unlock keeps its base.group_system method guard (its write runs in admin context and passes the new write() guard) - tests: direct write({'is_locked': ...}) by officer/manager/approver raises AccessError and the lock stays; admin/sudo succeed; the sudo helpers still work for a non-admin operating user; non-lock field edits by a manager are unaffected - HISTORY updated to describe the field-level boundary The mark_*_as_done/_failed completion callbacks remain callable via RPC (they must run as the operating user) and are tracked separately in #337. --- spp_programs/models/cycle.py | 29 ++++++++++ .../models/managers/cycle_manager_base.py | 23 +++----- .../models/managers/eligibility_manager.py | 2 +- .../managers/entitlement_manager_base.py | 25 ++------- .../managers/entitlement_manager_cash.py | 7 +-- .../managers/entitlement_manager_inkind.py | 14 +---- .../models/managers/payment_manager.py | 18 ++---- .../models/managers/program_manager.py | 6 +- spp_programs/models/programs.py | 29 ++++++++++ spp_programs/readme/HISTORY.md | 18 +++--- spp_programs/tests/test_force_unlock_authz.py | 56 +++++++++++++++++++ 11 files changed, 150 insertions(+), 77 deletions(-) diff --git a/spp_programs/models/cycle.py b/spp_programs/models/cycle.py index e72b6a8cb..c77838b81 100644 --- a/spp_programs/models/cycle.py +++ b/spp_programs/models/cycle.py @@ -1083,6 +1083,35 @@ def _get_related_job_domain(self): related_jobs = jobs.filtered(lambda r: self in r.args[0]) return [("id", "in", related_jobs.ids)] + def write(self, vals): + # ``is_locked`` / ``locked_reason`` form an operation lock protecting + # in-flight async pipelines (entitlement, payment, eligibility). + # Clearing or setting it out of band lets conflicting operations run, + # so direct writes to these fields are restricted to system + # administrators. The pipeline manages the lock through + # ``_acquire_operation_lock`` / ``_release_operation_lock`` (which + # ``sudo()``), and Force Unlock is the admin-only manual override. + if not self.env.su and ("is_locked" in vals or "locked_reason" in vals): + if not self.env.user.has_group("base.group_system"): + raise AccessError( + _( + "Changing the operation lock is restricted to system " + "administrators. The lock is managed automatically by " + "the async pipeline; use Force Unlock only in an emergency." + ) + ) + return super().write(vals) + + def _acquire_operation_lock(self, reason): + """Set the async-operation lock. Written via ``sudo()`` because direct + writes to ``is_locked`` / ``locked_reason`` are restricted to system + administrators (see ``write``).""" + self.sudo().write({"is_locked": True, "locked_reason": reason}) + + def _release_operation_lock(self): + """Clear the async-operation lock (see ``_acquire_operation_lock``).""" + self.sudo().write({"is_locked": False, "locked_reason": False}) + def action_force_unlock(self): """System-administrator-only escape hatch: clear a stuck "Operation in progress" lock. diff --git a/spp_programs/models/managers/cycle_manager_base.py b/spp_programs/models/managers/cycle_manager_base.py index 9176181b7..a32b3bb52 100644 --- a/spp_programs/models/managers/cycle_manager_base.py +++ b/spp_programs/models/managers/cycle_manager_base.py @@ -322,7 +322,7 @@ def mark_import_as_done(self, cycle, msg): :return: """ self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -334,7 +334,7 @@ def mark_import_as_done(self, cycle, msg): def mark_import_as_failed(self, cycle, msg): """Run via on_error() when async beneficiary import fails.""" self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -351,7 +351,7 @@ def mark_prepare_entitlement_as_done(self, cycle, msg): :return: """ self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -363,7 +363,7 @@ def mark_prepare_entitlement_as_done(self, cycle, msg): def mark_prepare_entitlement_as_failed(self, cycle, msg): """Run via on_error() when async entitlement preparation fails.""" self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -379,7 +379,7 @@ def mark_check_eligibility_as_done(self, cycle): :return: """ self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=_("Eligibility check finished.")) except Exception: @@ -391,7 +391,7 @@ def mark_check_eligibility_as_done(self, cycle): def mark_check_eligibility_as_failed(self, cycle): """Run via on_error() when async eligibility check fails.""" self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=_("Eligibility check failed.")) except Exception: @@ -556,7 +556,7 @@ def _check_eligibility_async(self, cycle, beneficiaries_count): self.ensure_one() _logger.debug("Beneficiaries: %s", beneficiaries_count) cycle.message_post(body=_("Eligibility check of %s beneficiaries started.", beneficiaries_count)) - cycle.write({"is_locked": True, "locked_reason": "Eligibility check of beneficiaries"}) + cycle._acquire_operation_lock("Eligibility check of beneficiaries") states = ("draft", "enrolled", "not_eligible") id_ranges = compute_id_ranges( @@ -638,12 +638,7 @@ def prepare_entitlements(self, cycle): def _prepare_entitlements_async(self, cycle, beneficiaries_count): _logger.debug("Prepare entitlement asynchronously") cycle.message_post(body=_("Prepare entitlement for %s beneficiaries started.", beneficiaries_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Prepare entitlement for beneficiaries."), - } - ) + cycle._acquire_operation_lock(_("Prepare entitlement for beneficiaries.")) id_ranges = compute_id_ranges( self.env.cr, @@ -898,7 +893,7 @@ def add_beneficiaries(self, cycle, beneficiaries, state="draft"): def _add_beneficiaries_async(self, cycle, beneficiaries, state): _logger.debug("Adding beneficiaries asynchronously") cycle.message_post(body=f"Import of {len(beneficiaries)} beneficiaries started.") - cycle.write({"is_locked": True, "locked_reason": _("Importing beneficiaries.")}) + cycle._acquire_operation_lock(_("Importing beneficiaries.")) beneficiaries_count = len(beneficiaries) jobs = [] diff --git a/spp_programs/models/managers/eligibility_manager.py b/spp_programs/models/managers/eligibility_manager.py index c93a65e8f..1d5d51c3e 100644 --- a/spp_programs/models/managers/eligibility_manager.py +++ b/spp_programs/models/managers/eligibility_manager.py @@ -150,7 +150,7 @@ def _import_registrants_async(self, new_beneficiaries, state="draft"): self.ensure_one() program = self.program_id program.message_post(body=f"Import of {len(new_beneficiaries)} beneficiaries started.") - program.write({"is_locked": True, "locked_reason": "Importing beneficiaries"}) + program._acquire_operation_lock("Importing beneficiaries") jobs = [] for i in range(0, len(new_beneficiaries), 10000): diff --git a/spp_programs/models/managers/entitlement_manager_base.py b/spp_programs/models/managers/entitlement_manager_base.py index 32cd2bb91..80a8bd0ec 100644 --- a/spp_programs/models/managers/entitlement_manager_base.py +++ b/spp_programs/models/managers/entitlement_manager_base.py @@ -79,12 +79,7 @@ def _set_pending_validation_entitlements_async(self, cycle, entitlements): entitlements_count, ) ) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Set entitlements to pending validation for cycle."), - } - ) + cycle._acquire_operation_lock(_("Set entitlements to pending validation for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): @@ -133,12 +128,7 @@ def _validate_entitlements_async(self, cycle, entitlements, entitlements_count): """ _logger.debug("Validate entitlements asynchronously") cycle.message_post(body=_("Validate %s entitlements started.", entitlements_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Validate and approve entitlements for cycle."), - } - ) + cycle._acquire_operation_lock(_("Validate and approve entitlements for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): @@ -200,12 +190,7 @@ def _cancel_entitlements_async(self, cycle, entitlements, entitlements_count): """ _logger.debug("Cancel entitlements asynchronously") cycle.message_post(body=_("Cancel %s entitlements started.", entitlements_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Cancel entitlements for cycle."), - } - ) + cycle._acquire_operation_lock(_("Cancel entitlements for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): @@ -242,7 +227,7 @@ def mark_job_as_done(self, cycle, msg): self.ensure_one() # Clear the lock first so a chatter-side failure can't leave the # cycle stuck with "Operation in progress". - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -259,7 +244,7 @@ def mark_job_as_failed(self, cycle, msg): :param msg: A string to be posted in the chatter """ self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: diff --git a/spp_programs/models/managers/entitlement_manager_cash.py b/spp_programs/models/managers/entitlement_manager_cash.py index 2d7504077..ab377bb68 100644 --- a/spp_programs/models/managers/entitlement_manager_cash.py +++ b/spp_programs/models/managers/entitlement_manager_cash.py @@ -317,12 +317,7 @@ def _validate_entitlements_async(self, cycle, entitlements, entitlements_count): """ _logger.debug("Validate entitlements asynchronously") cycle.message_post(body=_("Validate %s entitlements started.", entitlements_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Validate and approve entitlements for cycle."), - } - ) + cycle._acquire_operation_lock(_("Validate and approve entitlements for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): diff --git a/spp_programs/models/managers/entitlement_manager_inkind.py b/spp_programs/models/managers/entitlement_manager_inkind.py index 8b44c6db7..8b72c6211 100644 --- a/spp_programs/models/managers/entitlement_manager_inkind.py +++ b/spp_programs/models/managers/entitlement_manager_inkind.py @@ -214,12 +214,7 @@ def _set_pending_validation_entitlements_async(self, cycle, entitlements_count): entitlements_count, ) ) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Set entitlements to pending validation for cycle."), - } - ) + cycle._acquire_operation_lock(_("Set entitlements to pending validation for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): @@ -319,12 +314,7 @@ def _validate_entitlements_async(self, cycle, entitlements_count): """ _logger.debug("Validate entitlements asynchronously") cycle.message_post(body=_("Validate %s entitlements started.", entitlements_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Validate and approve entitlements for cycle."), - } - ) + cycle._acquire_operation_lock(_("Validate and approve entitlements for cycle.")) jobs = [] for i in range(0, entitlements_count, self.MAX_ROW_JOB_QUEUE): diff --git a/spp_programs/models/managers/payment_manager.py b/spp_programs/models/managers/payment_manager.py index bc59ef053..980adef74 100644 --- a/spp_programs/models/managers/payment_manager.py +++ b/spp_programs/models/managers/payment_manager.py @@ -71,7 +71,7 @@ def mark_job_as_done(self, cycle, msg): :return: """ self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -80,7 +80,7 @@ def mark_job_as_done(self, cycle, msg): def mark_job_as_failed(self, cycle, msg): """Run via on_error() when the async payment pipeline fails.""" self.ensure_one() - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() try: cycle.message_post(body=msg) except Exception: @@ -331,12 +331,7 @@ def _prepare_payments(self, cycle, entitlements): def _prepare_payments_async(self, cycle, entitlements, entitlements_count): _logger.debug("Prepare Payments asynchronously") cycle.message_post(body=_("Prepare payments started for %s entitlements.", entitlements_count)) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Prepare payments for entitlements in cycle."), - } - ) + cycle._acquire_operation_lock(_("Prepare payments for entitlements in cycle.")) # Right now this is not divided into subjobs jobs = [ @@ -446,12 +441,7 @@ def _send_payments(self, batches): def _send_payments_async(self, cycle, batches): _logger.debug("Send Payments asynchronously") cycle.message_post(body=_("Send payments started for %s batches.", len(batches))) - cycle.write( - { - "is_locked": True, - "locked_reason": _("Send payments for batches in cycle."), - } - ) + cycle._acquire_operation_lock(_("Send payments for batches in cycle.")) # Right now this is not divided into subjobs jobs = [ diff --git a/spp_programs/models/managers/program_manager.py b/spp_programs/models/managers/program_manager.py index 7622e5f0b..01ba54d70 100644 --- a/spp_programs/models/managers/program_manager.py +++ b/spp_programs/models/managers/program_manager.py @@ -74,7 +74,7 @@ def mark_enroll_eligible_as_done(self): """ self.ensure_one() program = self.program_id - program.write({"is_locked": False, "locked_reason": False}) + program._release_operation_lock() try: program.message_post(body=_("Eligibility check finished.")) except Exception: @@ -88,7 +88,7 @@ def mark_enroll_eligible_as_failed(self): """Run via on_error() when async eligibility enrollment fails.""" self.ensure_one() program = self.program_id - program.write({"is_locked": False, "locked_reason": False}) + program._release_operation_lock() try: program.message_post(body=_("Eligibility check failed.")) except Exception: @@ -204,7 +204,7 @@ def _enroll_eligible_registrants_async(self, states, members_count): _logger.debug("members: %s", members_count) program = self.program_id program.message_post(body=_("Eligibility check of %s beneficiaries started.", members_count)) - program.write({"is_locked": True, "locked_reason": "Eligibility check of beneficiaries"}) + program._acquire_operation_lock("Eligibility check of beneficiaries") if isinstance(states, str): states = [states] diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index bdde14bba..39ec2ba74 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -763,6 +763,35 @@ def _get_related_job_domain(self): related_jobs = jobs.filtered(lambda r: self in r.records.program_id) return [("id", "in", related_jobs.ids)] + def write(self, vals): + # ``is_locked`` / ``locked_reason`` form an operation lock protecting + # in-flight async pipelines (enrollment, eligibility). Clearing or + # setting it out of band lets conflicting operations run, so direct + # writes to these fields are restricted to system administrators. The + # pipeline manages the lock through ``_acquire_operation_lock`` / + # ``_release_operation_lock`` (which ``sudo()``), and Force Unlock is + # the admin-only manual override. + if not self.env.su and ("is_locked" in vals or "locked_reason" in vals): + if not self.env.user.has_group("base.group_system"): + raise AccessError( + _( + "Changing the operation lock is restricted to system " + "administrators. The lock is managed automatically by " + "the async pipeline; use Force Unlock only in an emergency." + ) + ) + return super().write(vals) + + def _acquire_operation_lock(self, reason): + """Set the async-operation lock. Written via ``sudo()`` because direct + writes to ``is_locked`` / ``locked_reason`` are restricted to system + administrators (see ``write``).""" + self.sudo().write({"is_locked": True, "locked_reason": reason}) + + def _release_operation_lock(self): + """Clear the async-operation lock (see ``_acquire_operation_lock``).""" + self.sudo().write({"is_locked": False, "locked_reason": False}) + def action_force_unlock(self): """System-administrator-only escape hatch: clear a stuck "Operation in progress" lock. diff --git a/spp_programs/readme/HISTORY.md b/spp_programs/readme/HISTORY.md index 0d3608e3e..1b094a3f1 100644 --- a/spp_programs/readme/HISTORY.md +++ b/spp_programs/readme/HISTORY.md @@ -1,12 +1,16 @@ ### 19.0.2.2.1 -- fix(security): enforce server-side authorization on Force Unlock. The - `action_force_unlock` methods on `spp.cycle` and `spp.program` cleared an - active operation lock with no group check, so any role holding write - access (program officers, managers, cycle approvers) could bypass the - emergency-only control via RPC and clear a lock while async jobs were - still running. The methods now require `base.group_system` (trusted - `sudo()` flows exempt), matching the view-button gating. +- fix(security): make the async operation lock a server-side boundary. The + Force Unlock buttons were gated to `base.group_system` in the views, but + `action_force_unlock` on `spp.cycle` / `spp.program` — and direct writes to + the `is_locked` / `locked_reason` fields — had no server-side check, so any + role holding write access (program officers, managers, cycle approvers) + could clear an active operation lock via RPC while async entitlement / + payment / eligibility jobs were still running. Direct writes to the lock + fields now require `base.group_system` (via a `write()` guard), the manual + `action_force_unlock` override requires the same, and the async pipeline + manages the lock through `sudo()` helpers so legitimate acquire/release + from the initiating user keeps working. ### 19.0.2.1.3 diff --git a/spp_programs/tests/test_force_unlock_authz.py b/spp_programs/tests/test_force_unlock_authz.py index f3bc44891..99cfea109 100644 --- a/spp_programs/tests/test_force_unlock_authz.py +++ b/spp_programs/tests/test_force_unlock_authz.py @@ -111,3 +111,59 @@ def test_program_force_unlock_allowed_for_system_admin(self): self.program.with_user(self.system).action_force_unlock() self.assertFalse(self.program.is_locked) self.assertFalse(self.program.locked_reason) + + # --- direct field write (the sink behind action_force_unlock) ------ + + def test_cycle_direct_write_is_locked_denied_for_officer(self): + """Clearing the lock via a direct RPC write must be blocked too, not + just the action_force_unlock button — officers hold write on the model.""" + self._lock_cycle() + with self.assertRaises(AccessError): + self.cycle.with_user(self.officer).write({"is_locked": False, "locked_reason": False}) + self.assertTrue(self.cycle.is_locked) + + def test_cycle_direct_write_is_locked_denied_for_manager(self): + self._lock_cycle() + with self.assertRaises(AccessError): + self.cycle.with_user(self.manager).write({"is_locked": False}) + self.assertTrue(self.cycle.is_locked) + + def test_cycle_direct_write_setting_lock_denied_for_manager(self): + """Setting the lock out of band is blocked as well (availability).""" + with self.assertRaises(AccessError): + self.cycle.with_user(self.manager).write({"is_locked": True, "locked_reason": "x"}) + self.assertFalse(self.cycle.is_locked) + + def test_program_direct_write_is_locked_denied_for_manager(self): + self.program.write({"is_locked": True, "locked_reason": "Enrollment running"}) + with self.assertRaises(AccessError): + self.program.with_user(self.manager).write({"is_locked": False}) + self.assertTrue(self.program.is_locked) + + def test_cycle_direct_write_is_locked_allowed_for_system_admin(self): + self._lock_cycle() + self.cycle.with_user(self.system).write({"is_locked": False, "locked_reason": False}) + self.assertFalse(self.cycle.is_locked) + + def test_manager_editing_other_fields_still_works(self): + """The guard only covers the lock fields — normal edits by a manager + (who holds write) must not be affected.""" + self._lock_cycle() + # A non-lock field write by the manager succeeds even while locked. + self.cycle.with_user(self.manager).write({"name": "Renamed [CYCLE TEST]"}) + self.assertEqual(self.cycle.name, "Renamed [CYCLE TEST]") + + # --- pipeline helpers still work for the non-admin operating user -- + + def test_release_operation_lock_works_for_non_admin(self): + """The async pipeline releases its own lock via the sudo() helper even + though the job runs as the initiating (non-admin) user.""" + self._lock_cycle() + self.cycle.with_user(self.officer)._release_operation_lock() + self.assertFalse(self.cycle.is_locked) + self.assertFalse(self.cycle.locked_reason) + + def test_acquire_operation_lock_works_for_non_admin(self): + self.cycle.with_user(self.officer)._acquire_operation_lock("Import running") + self.assertTrue(self.cycle.is_locked) + self.assertEqual(self.cycle.locked_reason, "Import running") From 7b95c7ab7d85cf188d562e9f7db775797dd3c003 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 19:45:04 +0800 Subject: [PATCH 5/9] security(programs): document sudo in lock helpers (nosemgrep) The _acquire/_release_operation_lock helpers sudo() the write of the two lock fields because the write() guard restricts them to system admins while the async pipeline runs as the initiating non-admin user. Suppress odoo-sudo-without-context with justification (sudo is scoped to the two lock fields only). --- spp_programs/models/cycle.py | 5 ++++- spp_programs/models/programs.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/spp_programs/models/cycle.py b/spp_programs/models/cycle.py index c77838b81..c5126b8f3 100644 --- a/spp_programs/models/cycle.py +++ b/spp_programs/models/cycle.py @@ -1105,11 +1105,14 @@ def write(self, vals): def _acquire_operation_lock(self, reason): """Set the async-operation lock. Written via ``sudo()`` because direct writes to ``is_locked`` / ``locked_reason`` are restricted to system - administrators (see ``write``).""" + administrators (see ``write``); the pipeline runs as the initiating + non-admin user and must bypass that guard for the two lock fields only.""" + # nosemgrep: odoo-sudo-without-context - lock fields admin-write-only (see write()); sudo scoped self.sudo().write({"is_locked": True, "locked_reason": reason}) def _release_operation_lock(self): """Clear the async-operation lock (see ``_acquire_operation_lock``).""" + # nosemgrep: odoo-sudo-without-context - lock fields admin-write-only (see write()); sudo scoped self.sudo().write({"is_locked": False, "locked_reason": False}) def action_force_unlock(self): diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index 39ec2ba74..01e0f1d4a 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -785,11 +785,14 @@ def write(self, vals): def _acquire_operation_lock(self, reason): """Set the async-operation lock. Written via ``sudo()`` because direct writes to ``is_locked`` / ``locked_reason`` are restricted to system - administrators (see ``write``).""" + administrators (see ``write``); the pipeline runs as the initiating + non-admin user and must bypass that guard for the two lock fields only.""" + # nosemgrep: odoo-sudo-without-context - lock fields admin-write-only (see write()); sudo scoped self.sudo().write({"is_locked": True, "locked_reason": reason}) def _release_operation_lock(self): """Clear the async-operation lock (see ``_acquire_operation_lock``).""" + # nosemgrep: odoo-sudo-without-context - lock fields admin-write-only (see write()); sudo scoped self.sudo().write({"is_locked": False, "locked_reason": False}) def action_force_unlock(self): From 8093502691a9d3262d0e2825c53ae073fb768079 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 19:45:04 +0800 Subject: [PATCH 6/9] docs(spp_programs): regenerate README from fragments (CI oca-gen output) --- spp_programs/README.rst | 20 ++++++++++++-------- spp_programs/static/description/index.html | 20 ++++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/spp_programs/README.rst b/spp_programs/README.rst index ebe45c72b..5860f62a3 100644 --- a/spp_programs/README.rst +++ b/spp_programs/README.rst @@ -257,14 +257,18 @@ Changelog 19.0.2.2.1 ~~~~~~~~~~ -- fix(security): enforce server-side authorization on Force Unlock. The - ``action_force_unlock`` methods on ``spp.cycle`` and ``spp.program`` - cleared an active operation lock with no group check, so any role - holding write access (program officers, managers, cycle approvers) - could bypass the emergency-only control via RPC and clear a lock while - async jobs were still running. The methods now require - ``base.group_system`` (trusted ``sudo()`` flows exempt), matching the - view-button gating. +- fix(security): make the async operation lock a server-side boundary. + The Force Unlock buttons were gated to ``base.group_system`` in the + views, but ``action_force_unlock`` on ``spp.cycle`` / ``spp.program`` + — and direct writes to the ``is_locked`` / ``locked_reason`` fields — + had no server-side check, so any role holding write access (program + officers, managers, cycle approvers) could clear an active operation + lock via RPC while async entitlement / payment / eligibility jobs were + still running. Direct writes to the lock fields now require + ``base.group_system`` (via a ``write()`` guard), the manual + ``action_force_unlock`` override requires the same, and the async + pipeline manages the lock through ``sudo()`` helpers so legitimate + acquire/release from the initiating user keeps working. 19.0.2.1.3 ~~~~~~~~~~ diff --git a/spp_programs/static/description/index.html b/spp_programs/static/description/index.html index a9bd37e97..b18978828 100644 --- a/spp_programs/static/description/index.html +++ b/spp_programs/static/description/index.html @@ -660,14 +660,18 @@

    Changelog

    19.0.2.2.1

      -
    • fix(security): enforce server-side authorization on Force Unlock. The -action_force_unlock methods on spp.cycle and spp.program -cleared an active operation lock with no group check, so any role -holding write access (program officers, managers, cycle approvers) -could bypass the emergency-only control via RPC and clear a lock while -async jobs were still running. The methods now require -base.group_system (trusted sudo() flows exempt), matching the -view-button gating.
    • +
    • fix(security): make the async operation lock a server-side boundary. +The Force Unlock buttons were gated to base.group_system in the +views, but action_force_unlock on spp.cycle / spp.program +— and direct writes to the is_locked / locked_reason fields — +had no server-side check, so any role holding write access (program +officers, managers, cycle approvers) could clear an active operation +lock via RPC while async entitlement / payment / eligibility jobs were +still running. Direct writes to the lock fields now require +base.group_system (via a write() guard), the manual +action_force_unlock override requires the same, and the async +pipeline manages the lock through sudo() helpers so legitimate +acquire/release from the initiating user keeps working.
    From 103608677e71fe21ae490c9482a879fdab061a02 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 20:26:09 +0800 Subject: [PATCH 7/9] security(programs): route ALL lock writers through the sudo helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staff review of Option A found the pipeline conversion was incomplete: I had only converted dict-style .write({'is_locked':...}) sites in spp_programs/models/managers, missing attribute-assignment writes and dependent modules. The new spp.program/spp.cycle write() guard would raise AccessError for the non-admin operating user at those sites, leaving programs stuck-locked or breaking async imports. - spp_programs eligibility_manager.mark_import_as_done: the on_done callback cleared the lock via self.program_id.is_locked = False (attribute assignment) — now uses _release_operation_lock() - spp_program_geofence (released): its _import_registrants_async / mark_import_as_done wrote the spp.program lock directly and broke for non-admin imports under the guard; routed through the inherited helpers; version 19.0.1.0.0 -> 19.0.1.0.1 + HISTORY - spp_farmer_registry_demo (released): demo generator cleared the cycle lock directly (works only because demo runs as admin) — converted to the helper to remove the latent trap; version 19.0.2.1.1 -> 19.0.2.1.2 + HISTORY - add a regression test driving the real eligibility mark_import_as_done as a non-admin officer (would have caught the miss) Verified: exhaustive grep shows the only remaining direct lock writes on spp.cycle/spp.program are the two admin-guarded action_force_unlock methods. Suites: spp_programs 693, spp_program_geofence 41, spp_farmer_registry_demo 132 — all green. --- spp_farmer_registry_demo/__manifest__.py | 2 +- .../models/farmer_demo_generator.py | 5 +++-- spp_farmer_registry_demo/readme/HISTORY.md | 7 +++++++ spp_program_geofence/__manifest__.py | 2 +- spp_program_geofence/models/eligibility_manager.py | 5 ++--- spp_program_geofence/readme/HISTORY.md | 10 ++++++++++ spp_programs/models/managers/eligibility_manager.py | 3 +-- spp_programs/tests/test_force_unlock_authz.py | 12 ++++++++++++ 8 files changed, 37 insertions(+), 9 deletions(-) diff --git a/spp_farmer_registry_demo/__manifest__.py b/spp_farmer_registry_demo/__manifest__.py index 3a0fbab2b..f92f36d6c 100644 --- a/spp_farmer_registry_demo/__manifest__.py +++ b/spp_farmer_registry_demo/__manifest__.py @@ -3,7 +3,7 @@ "name": "OpenSPP Farmer Registry Demo", "summary": "Demo generator for Farmer Registry with fixed stories and volume generation", "category": "OpenSPP", - "version": "19.0.2.1.1", + "version": "19.0.2.1.2", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_farmer_registry_demo/models/farmer_demo_generator.py b/spp_farmer_registry_demo/models/farmer_demo_generator.py index e5b963ec1..6cb4bd71a 100644 --- a/spp_farmer_registry_demo/models/farmer_demo_generator.py +++ b/spp_farmer_registry_demo/models/farmer_demo_generator.py @@ -2297,7 +2297,7 @@ def _create_single_cycle(self, program): try: program_beneficiaries = program.get_beneficiaries("enrolled").mapped("partner_id.id") cycle_manager._add_beneficiaries(cycle, program_beneficiaries, "enrolled", do_count=True) - cycle.write({"is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() _logger.info( "Synced beneficiary import for cycle (cycle_id=%s, count=%s)", cycle.id, @@ -2343,7 +2343,8 @@ def _create_single_cycle(self, program): exc, ) if cycle.state == "draft": - cycle.write({"state": "to_approve", "is_locked": False, "locked_reason": False}) + cycle._release_operation_lock() + cycle.write({"state": "to_approve"}) # Step 4: Approve cycle (to_approve -> approved) try: diff --git a/spp_farmer_registry_demo/readme/HISTORY.md b/spp_farmer_registry_demo/readme/HISTORY.md index f397e16d8..43b602c17 100644 --- a/spp_farmer_registry_demo/readme/HISTORY.md +++ b/spp_farmer_registry_demo/readme/HISTORY.md @@ -1,3 +1,10 @@ +### 19.0.2.1.2 + +- fix(demo): release/force the cycle operation lock through the + `_release_operation_lock` helper instead of writing `is_locked` directly, + so demo generation stays compatible with the `spp_programs` 19.0.2.2.1 + guard that restricts direct writes to the lock fields to system admins. + ### 19.0.2.1.1 - fix(demo): name each farm after its head member and give every member the head's family name so a household reads as one family; farm names and registry IDs stay unique and generation remains seed-deterministic, resolving duplicate farm names and duplicate Tax/National IDs (#1114) diff --git a/spp_program_geofence/__manifest__.py b/spp_program_geofence/__manifest__.py index f678ea818..7575f18b8 100644 --- a/spp_program_geofence/__manifest__.py +++ b/spp_program_geofence/__manifest__.py @@ -4,7 +4,7 @@ "name": "OpenSPP Program Geofence", "summary": "Geofence-based geographic targeting for programs using spatial queries.", "category": "OpenSPP", - "version": "19.0.1.0.0", + "version": "19.0.1.0.1", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", "license": "LGPL-3", diff --git a/spp_program_geofence/models/eligibility_manager.py b/spp_program_geofence/models/eligibility_manager.py index aa053b311..bc5db379d 100644 --- a/spp_program_geofence/models/eligibility_manager.py +++ b/spp_program_geofence/models/eligibility_manager.py @@ -200,7 +200,7 @@ def _import_registrants_async(self, new_beneficiaries, state="draft"): self.ensure_one() program = self.program_id program.message_post(body=_("Import of %s beneficiaries started.") % len(new_beneficiaries)) - program.write({"is_locked": True, "locked_reason": _("Importing beneficiaries")}) + program._acquire_operation_lock(_("Importing beneficiaries")) jobs = [] for i in range(0, len(new_beneficiaries), self.IMPORT_CHUNK_SIZE): @@ -218,8 +218,7 @@ def mark_import_as_done(self): self.ensure_one() self.program_id._compute_eligible_beneficiary_count() self.program_id._compute_beneficiary_count() - self.program_id.is_locked = False - self.program_id.locked_reason = None + self.program_id._release_operation_lock() self.program_id.message_post(body=_("Import finished.")) def _import_registrants(self, new_beneficiaries, state="draft", do_count=False): diff --git a/spp_program_geofence/readme/HISTORY.md b/spp_program_geofence/readme/HISTORY.md index ae25b1472..92c36283f 100644 --- a/spp_program_geofence/readme/HISTORY.md +++ b/spp_program_geofence/readme/HISTORY.md @@ -1,3 +1,13 @@ +## 19.0.1.0.1 + +- fix(security): route the async import lock through the operation-lock + helpers so it keeps working under the new `spp.program` write guard. + `spp_programs` 19.0.2.2.1 restricts direct writes to `is_locked` / + `locked_reason` to system administrators; the geofence import acquired and + released the lock with plain writes as the initiating (non-admin) user, + which the guard would reject — leaving the program stuck locked. It now + uses `_acquire_operation_lock` / `_release_operation_lock` (which `sudo()`). + ## 19.0.1.0.0 - Initial release: geofence-based program targeting and eligibility management (Tier 1 coordinate intersection, Tier 2 area-intersection fallback), program configuration UI, and program creation wizard support. diff --git a/spp_programs/models/managers/eligibility_manager.py b/spp_programs/models/managers/eligibility_manager.py index 1d5d51c3e..18c04feb6 100644 --- a/spp_programs/models/managers/eligibility_manager.py +++ b/spp_programs/models/managers/eligibility_manager.py @@ -168,8 +168,7 @@ def mark_import_as_done(self): self.ensure_one() self.program_id.refresh_beneficiary_counts() - self.program_id.is_locked = False - self.program_id.locked_reason = None + self.program_id._release_operation_lock() self.program_id.message_post(body=_("Import finished.")) def _import_registrants(self, new_beneficiaries, state="draft", do_count=False): diff --git a/spp_programs/tests/test_force_unlock_authz.py b/spp_programs/tests/test_force_unlock_authz.py index 99cfea109..2dda86eb7 100644 --- a/spp_programs/tests/test_force_unlock_authz.py +++ b/spp_programs/tests/test_force_unlock_authz.py @@ -167,3 +167,15 @@ def test_acquire_operation_lock_works_for_non_admin(self): self.cycle.with_user(self.officer)._acquire_operation_lock("Import running") self.assertTrue(self.cycle.is_locked) self.assertEqual(self.cycle.locked_reason, "Import running") + + def test_eligibility_mark_import_done_releases_lock_as_non_admin(self): + """Regression: the async import on_done callback runs as the initiating + non-admin user; it must release the program lock through the sudo helper + rather than a direct field write that the guard would reject.""" + manager = self.env["spp.program.membership.manager.default"].create( + {"name": "Elig Manager", "program_id": self.program.id} + ) + self.program.write({"is_locked": True, "locked_reason": "Importing beneficiaries"}) + manager.with_user(self.officer).mark_import_as_done() + self.assertFalse(self.program.is_locked) + self.assertFalse(self.program.locked_reason) From cb14ed2968ecccf02fd1912fc07ca62c3c4df952 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 20:34:29 +0800 Subject: [PATCH 8/9] docs: regenerate README from fragments for geofence + farmer_demo (CI oca-gen output) --- spp_farmer_registry_demo/README.rst | 9 +++++++++ .../static/description/index.html | 14 ++++++++++++-- spp_program_geofence/README.rst | 12 ++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/spp_farmer_registry_demo/README.rst b/spp_farmer_registry_demo/README.rst index 281dd5018..2b57f6e6f 100644 --- a/spp_farmer_registry_demo/README.rst +++ b/spp_farmer_registry_demo/README.rst @@ -120,6 +120,15 @@ Dependencies Changelog ========= +19.0.2.1.2 +~~~~~~~~~~ + +- fix(demo): release/force the cycle operation lock through the + ``_release_operation_lock`` helper instead of writing ``is_locked`` + directly, so demo generation stays compatible with the + ``spp_programs`` 19.0.2.2.1 guard that restricts direct writes to the + lock fields to system admins. + 19.0.2.1.1 ~~~~~~~~~~ diff --git a/spp_farmer_registry_demo/static/description/index.html b/spp_farmer_registry_demo/static/description/index.html index 4e9aa8e7b..c8938bdc1 100644 --- a/spp_farmer_registry_demo/static/description/index.html +++ b/spp_farmer_registry_demo/static/description/index.html @@ -488,6 +488,16 @@

    Changelog

+

19.0.2.1.2

+
    +
  • fix(demo): release/force the cycle operation lock through the +_release_operation_lock helper instead of writing is_locked +directly, so demo generation stays compatible with the +spp_programs 19.0.2.2.1 guard that restricts direct writes to the +lock fields to system admins.
  • +
+
+

19.0.2.1.1

  • fix(demo): name each farm after its head member and give every member @@ -502,7 +512,7 @@

    19.0.2.1.1

    (#1114)
-
+

19.0.2.1.0

  • feat(demo): add GIS + irrigation scenario (FM4) with reservoir + canal @@ -521,7 +531,7 @@

    19.0.2.1.0

    tables and the CR overview
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
  • diff --git a/spp_program_geofence/README.rst b/spp_program_geofence/README.rst index 3f1140526..6459f0e36 100644 --- a/spp_program_geofence/README.rst +++ b/spp_program_geofence/README.rst @@ -72,6 +72,18 @@ Known Limitations Changelog ========= +19.0.1.0.1 +---------- + +- fix(security): route the async import lock through the operation-lock + helpers so it keeps working under the new ``spp.program`` write guard. + ``spp_programs`` 19.0.2.2.1 restricts direct writes to ``is_locked`` / + ``locked_reason`` to system administrators; the geofence import + acquired and released the lock with plain writes as the initiating + (non-admin) user, which the guard would reject — leaving the program + stuck locked. It now uses ``_acquire_operation_lock`` / + ``_release_operation_lock`` (which ``sudo()``). + 19.0.1.0.0 ---------- From 051d8ae6f45afccde4bbfbc1a5222e03b3d4c78b Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 20:52:44 +0800 Subject: [PATCH 9/9] docs(programs): add in-code #337 breadcrumb near the lock helpers Staff-review nit: the callback-vector lock-release residual (public mark_* methods can clear the lock via RPC) was disclosed only in a commit message. Add a NOTE(#337) comment by the helpers so a maintainer reading the source sees the known, tracked gap. --- spp_programs/models/cycle.py | 5 +++++ spp_programs/models/programs.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/spp_programs/models/cycle.py b/spp_programs/models/cycle.py index c5126b8f3..61f093590 100644 --- a/spp_programs/models/cycle.py +++ b/spp_programs/models/cycle.py @@ -1102,6 +1102,11 @@ def write(self, vals): ) return super().write(vals) + # NOTE(#337): these helpers sudo the lock write, so any PUBLIC method that + # calls them (e.g. the async mark_*_as_done / mark_*_as_failed completion + # callbacks) is an RPC-reachable lock-clearing path that the write() guard + # above does not cover. Closing that needs authorization on those + # callbacks and is tracked separately in issue #337. def _acquire_operation_lock(self, reason): """Set the async-operation lock. Written via ``sudo()`` because direct writes to ``is_locked`` / ``locked_reason`` are restricted to system diff --git a/spp_programs/models/programs.py b/spp_programs/models/programs.py index 01e0f1d4a..cd2d73bad 100644 --- a/spp_programs/models/programs.py +++ b/spp_programs/models/programs.py @@ -782,6 +782,11 @@ def write(self, vals): ) return super().write(vals) + # NOTE(#337): these helpers sudo the lock write, so any PUBLIC method that + # calls them (e.g. the async mark_*_as_done / mark_*_as_failed completion + # callbacks) is an RPC-reachable lock-clearing path that the write() guard + # above does not cover. Closing that needs authorization on those + # callbacks and is tracked separately in issue #337. def _acquire_operation_lock(self, reason): """Set the async-operation lock. Written via ``sudo()`` because direct writes to ``is_locked`` / ``locked_reason`` are restricted to system