diff --git a/spp_change_request_v2/README.rst b/spp_change_request_v2/README.rst index cec371c0..99b63c96 100644 --- a/spp_change_request_v2/README.rst +++ b/spp_change_request_v2/README.rst @@ -853,6 +853,20 @@ Before declaring a new CR type complete: Changelog ========= +19.0.3.1.12 +~~~~~~~~~~~ + +- fix(change_request): auto-apply-on-approve runs through the public + ``action_apply`` again. Requiring change-request manager rights to + apply meant auto-apply was routed to the internal mechanism instead, + so the approver could be a validator — but ``action_apply`` is the + extension point modules override to hang post-apply work off an apply, + and bypassing it left those overrides silently not running on + approval: no error, just missing side effects. Auto-apply now calls + ``action_apply`` under ``sudo()``, which the manager gate already + exempts. ``sudo()`` sets superuser mode without changing the user, so + the applying user is still recorded as the approver. + 19.0.3.1.11 ~~~~~~~~~~~ diff --git a/spp_change_request_v2/__manifest__.py b/spp_change_request_v2/__manifest__.py index f34e3557..d3d91b88 100644 --- a/spp_change_request_v2/__manifest__.py +++ b/spp_change_request_v2/__manifest__.py @@ -1,6 +1,6 @@ { "name": "OpenSPP Change Request V2", - "version": "19.0.3.1.11", + "version": "19.0.3.1.12", "sequence": 50, "category": "OpenSPP", "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention", diff --git a/spp_change_request_v2/models/change_request.py b/spp_change_request_v2/models/change_request.py index 6a703e54..81d9e481 100644 --- a/spp_change_request_v2/models/change_request.py +++ b/spp_change_request_v2/models/change_request.py @@ -1093,10 +1093,16 @@ def _on_approve(self): self._create_log("approved") if self.request_type_id.auto_apply_on_approve: # Auto-apply is authorized by the approval workflow itself, so it - # goes through the internal mechanism rather than the manager-gated - # public action_apply (the approver may be a validator, not a - # manager). - self._apply_change_request() + # runs with sudo: ``action_apply``'s manager gate exempts + # ``env.su``, which lets the approver be a validator rather than a + # manager. Going through the public entry point rather than the + # internal mechanism keeps ``action_apply`` the single extension + # point for apply -- downstream modules override it to hang + # post-apply work off the apply, and routing around it left those + # overrides silently not running on approval. ``sudo()`` sets + # ``su`` without changing ``uid``, so ``applied_by_id`` still + # records the real approver. + self.sudo().action_apply() # nosemgrep: odoo-sudo-without-context def _on_reject(self, reason): super()._on_reject(reason) @@ -1448,9 +1454,16 @@ def action_apply(self): roles cannot (e.g. ``spp.group.membership``), so it must be gated server-side to managers: the XML button ``groups=`` is NOT an authorization boundary because Odoo object methods are callable over - RPC. Superuser (sudo) callers and the auto-apply-on-approve path (which - invokes ``_apply_change_request`` directly, already authorized by the - approval workflow) are unaffected. + RPC. Superuser (``env.su``) callers are exempt, and + auto-apply-on-approve is one of them: ``_on_approve`` reaches this + method through ``sudo()``, already authorized by the approval workflow + itself, so the approver may be a validator rather than a manager. + + This is also the extension point for apply, so an override runs on both + paths -- under ``su`` when auto-applied on approval, and as the manager + who clicked Apply on the manual path. ``sudo()`` sets ``su`` without + changing ``uid``, so ``self.env.user`` is the approver either way and + ``applied_by_id`` records them, not the superuser. """ if not (self.env.su or self.env.user.has_group("spp_change_request_v2.group_cr_manager")): raise AccessError(_("Only Change Request managers can apply change requests.")) diff --git a/spp_change_request_v2/readme/HISTORY.md b/spp_change_request_v2/readme/HISTORY.md index 54cd843e..5e160703 100644 --- a/spp_change_request_v2/readme/HISTORY.md +++ b/spp_change_request_v2/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.3.1.12 + +- fix(change_request): auto-apply-on-approve runs through the public `action_apply` again. Requiring change-request manager rights to apply meant auto-apply was routed to the internal mechanism instead, so the approver could be a validator — but `action_apply` is the extension point modules override to hang post-apply work off an apply, and bypassing it left those overrides silently not running on approval: no error, just missing side effects. Auto-apply now calls `action_apply` under `sudo()`, which the manager gate already exempts. `sudo()` sets superuser mode without changing the user, so the applying user is still recorded as the approver. + ### 19.0.3.1.11 - fix(change_request): field-mapping transform expressions are evaluated again. `_eval_expression` passed `nocopy=True` to `safe_eval`, which takes no such argument in Odoo 19, so every expression raised `TypeError`; the blanket fallback swallowed it and the **untransformed** value was written to the registrant. A configured transform was therefore ignored, reported only as a warning in the log. **Behaviour change:** request types that already have an Expression transform configured will start transforming values on upgrade, having silently passed the raw value through until now. diff --git a/spp_change_request_v2/static/description/index.html b/spp_change_request_v2/static/description/index.html index 705cc88b..c8d57bc4 100644 --- a/spp_change_request_v2/static/description/index.html +++ b/spp_change_request_v2/static/description/index.html @@ -1339,6 +1339,21 @@

Changelog

+

19.0.3.1.12

+ +
+

19.0.3.1.11

-
+

19.0.3.1.10

-
+

19.0.3.1.9

  • fix(security): duplicate detection now scores the fields both change @@ -1434,7 +1449,7 @@

    19.0.3.1.9

    requester-writable selected_field_name / field_to_modify.
-
+

19.0.3.1.8

  • fix(security): scope the Create-Group member wizards to the parent @@ -1452,7 +1467,7 @@

    19.0.3.1.8

    access-control entry grants.
-
+

19.0.3.1.7

  • fix(security): require change-request manager rights to apply a change @@ -1467,7 +1482,7 @@

    19.0.3.1.7

    endpoint.
-
+

19.0.3.1.6

  • fix(security): derive conflict and duplicate detection from the change @@ -1481,7 +1496,7 @@

    19.0.3.1.6

    an empty one, so detection cannot silently disable itself.
-
+

19.0.3.1.5

  • fix(security): scope the CR Requestor, Local Validator and HQ @@ -1493,7 +1508,7 @@

    19.0.3.1.5

    are noupdate.
-
+

19.0.3.1.4

  • fix(security): add ownership and area record rules to every concrete @@ -1510,7 +1525,7 @@

    19.0.3.1.4

    unrestricted delete their access-control entries grant.
-
+

19.0.3.1.3

  • fix(security): route and apply the same single field for @@ -1523,7 +1538,7 @@

    19.0.3.1.3

    the routing selector.
-
+

19.0.3.1.2

  • fix(change_request_v2): adding an ID now looks for a live one of that @@ -1532,7 +1547,7 @@

    19.0.3.1.2

    (#1136)
-
+

19.0.3.1.1

  • fix(change_request): enforce the (cr_type_id, reason) uniqueness @@ -1546,7 +1561,7 @@

    19.0.3.1.1

    applied) so the constraint applies cleanly on upgrade.
-
+

19.0.3.1.0

  • revert(change_request): restore the create-a-new-individual Add @@ -1564,7 +1579,7 @@

    19.0.3.1.0

    not restored here; reinstate separately if needed.
-
+

19.0.3.0.0

  • feat(change_request): redesign the group/membership CR flows (#242) — @@ -1586,7 +1601,7 @@

    19.0.3.0.0

    must adapt (see #1133).
-
+

19.0.2.0.8

  • fix(views): disable inline creation of CR document types on the Change @@ -1597,7 +1612,7 @@

    19.0.2.0.8

    Documents” modal (missing Name field) that blocked saving (#1125)
-
+

19.0.2.0.7

  • fix(security): align CR Requestor / CR Local Validator / CR HQ @@ -1609,7 +1624,7 @@

    19.0.2.0.7

    dependencies.
-
+

19.0.2.0.6

  • fix(views): route post-submit CRs (pending / approved / applied / @@ -1624,7 +1639,7 @@

    19.0.2.0.6

    list so row-click goes through the stage router.
-
+

19.0.2.0.5

  • fix(security): add a global ir.rule on spp.change.request that @@ -1637,27 +1652,27 @@

    19.0.2.0.5

    roles).
-
+

19.0.2.0.3

  • fix: add HTML escaping to all computed Html fields with sanitize=False to prevent stored XSS (#50)
-
+

19.0.2.0.2

  • fix: fix batch approval wizard line deletion (#130)
-
+

19.0.2.0.1

  • fix: skip field types before getattr and isolate detail prefetch (#129)
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
    • diff --git a/spp_change_request_v2/tests/test_apply_authorization.py b/spp_change_request_v2/tests/test_apply_authorization.py index 33f16dcd..c4a700a0 100644 --- a/spp_change_request_v2/tests/test_apply_authorization.py +++ b/spp_change_request_v2/tests/test_apply_authorization.py @@ -99,10 +99,46 @@ def test_manager_can_apply(self): self.assertTrue(cr.is_applied) self.assertTrue(self.membership.ended_date) + def test_auto_apply_runs_the_public_apply_entry_point(self): + """``action_apply`` is the extension point downstream modules override. + + Auto-apply used to bypass it, so any override hung off apply silently + stopped running on approval -- no error, just missing side effects. + """ + from unittest.mock import patch + + cr = self._make_approved_cr() + cr.request_type_id.auto_apply_on_approve = True + + seen = [] + original = type(cr).action_apply + + def spy(records): + seen.append(tuple(records.ids)) + return original(records) + + with patch.object(type(cr), "action_apply", spy): + cr.with_user(self.cr_validator)._on_approve() + + self.assertTrue(seen, "auto-apply must go through the public action_apply") + self.assertTrue(cr.is_applied) + + def test_auto_apply_records_the_real_approver(self): + """sudo() sets su without changing uid, so attribution is preserved.""" + cr = self._make_approved_cr() + cr.request_type_id.auto_apply_on_approve = True + cr.with_user(self.cr_validator)._on_approve() + self.assertEqual( + cr.applied_by_id, + self.cr_validator, + "applying under sudo must still record the approver, not the superuser", + ) + def test_auto_apply_on_approve_runs_for_non_manager_approver(self): """Auto-apply-on-approve must still work when the approver is a - validator (not a manager): _on_approve routes through the internal - apply mechanism, which is not gated.""" + validator (not a manager): ``_on_approve`` reaches the public + ``action_apply`` through ``sudo()``, and the manager gate exempts + superuser callers.""" cr = self._make_approved_cr() cr.request_type_id.auto_apply_on_approve = True cr.with_user(self.cr_validator)._on_approve()