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/__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_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

+ +
+

19.0.2.1.1

-
+

19.0.2.1.0

-
+

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 ---------- 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/README.rst b/spp_programs/README.rst index c05dac423..5860f62a3 100644 --- a/spp_programs/README.rst +++ b/spp_programs/README.rst @@ -254,6 +254,22 @@ Dependencies Changelog ========= +19.0.2.2.1 +~~~~~~~~~~ + +- 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/__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..61f093590 100644 --- a/spp_programs/models/cycle.py +++ b/spp_programs/models/cycle.py @@ -1083,14 +1083,67 @@ 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) + + # 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 + 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): - """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/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..18c04feb6 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): @@ -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/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 5841fb332..cd2d73bad 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 @@ -763,12 +763,65 @@ 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) + + # 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 + 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): - """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..1b094a3f1 100644 --- a/spp_programs/readme/HISTORY.md +++ b/spp_programs/readme/HISTORY.md @@ -1,3 +1,17 @@ +### 19.0.2.2.1 + +- 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 - 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/static/description/index.html b/spp_programs/static/description/index.html index 50e35cbb3..b18978828 100644 --- a/spp_programs/static/description/index.html +++ b/spp_programs/static/description/index.html @@ -658,6 +658,23 @@

    Changelog

+

19.0.2.2.1

+ +
+

19.0.2.1.3

-
+

19.0.2.1.2

  • fix(security): add global ir.rule records on @@ -690,7 +707,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 +716,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 +727,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 +740,7 @@

    19.0.2.0.10

    submission on double-click
-
+

19.0.2.0.9

  • Add context flags (skip_registrant_statistics, @@ -736,7 +753,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 +764,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 +773,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 +782,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 +818,7 @@

    19.0.2.0.1

    constraint creation
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • 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_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 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..2dda86eb7 --- /dev/null +++ b/spp_programs/tests/test_force_unlock_authz.py @@ -0,0 +1,181 @@ +# 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) + + # --- 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") + + 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)