security: batch 2 — CR + programs chain (#264, #261, #338, #336, #353, #343, #365)

spp_change_request_v2/README.rst

@@ -853,6 +853,144 @@ Before declaring a new CR type complete:
 Changelog
 =========
 
+19.0.3.1.10
+~~~~~~~~~~~
+
+- fix(security): conflict and duplicate detection now decide whether a
+  mapped field changed using the same comparison the apply strategy
+  uses. Detection compared through a helper that lowercases and strips
+  strings while apply compares raw, so a case- or whitespace-only edit
+  was invisible to detection yet still written to the registrant —
+  enough to sidestep a field-scoped conflict rule with a cosmetic edit.
+  Detection also ignored transform expressions, which apply evaluates
+  before comparing. Similarity scoring is unchanged and stays
+  case-insensitive, since that is the point of a fuzzy match.
+- fix: applying a change request that has no mapping to write is now
+  rejected instead of reported as successful. Because
+  ``_effective_mappings`` fails closed, a request whose routed field
+  lost its mapping — or whose type has none configured — wrote nothing
+  yet was still stamped applied, with an applied date, an audit event
+  and a log line, so operators saw a green request whose change had been
+  silently dropped. A genuine no-op, where the registrant already holds
+  the proposed values, still applies cleanly.
+- fix: a submitted change request with no detail row can be repaired
+  again. ``detail_res_id`` is frozen after submission so a substituted
+  detail cannot be attached post-approval, but the guard did not
+  distinguish binding from re-pointing, so ``_ensure_detail()`` could
+  not create the missing row and the request could not be opened from
+  any context. Binding is now accepted only for a row that already
+  points back at the request.
+- fix: an empty string now reads as unset in the post-submit freeze.
+  Odoo stores an unset field as ``False`` while a JSON-RPC client or
+  integration re-saving a record sends ``""``, so an idempotent re-save
+  was rejected as though it had altered the approved content. Clearing a
+  populated frozen field with ``""`` is still rejected. The
+  normalisation existed verbatim on both the change request and the
+  detail base; it now lives once, so the two guards cannot disagree.
+- perf: the caller's proposed-change set is derived once per
+  duplicate-detection run rather than recomputed for every candidate,
+  each derivation having re-browsed the detail and re-read every
+  configured mapping.
+
+19.0.3.1.9
+~~~~~~~~~~
+
+- fix(security): duplicate detection now scores the fields both change
+  requests actually propose to change, instead of demanding the two
+  derived change sets be identical. Because a dynamic-approval type
+  applies only the routed field, a requester could add a throwaway edit
+  to another mapped field, make the two sets unequal and drop similarity
+  to zero, while apply discarded that edit — so the evasion cost
+  nothing. Similarity is now computed over the shared changed fields,
+  proportionally, on the same scale the static path uses, which also
+  stops a mostly identical multi-field request collapsing to zero as
+  soon as one shared field differs. The change set is still derived from
+  the detail-versus-registrant diff and never from the
+  requester-writable ``selected_field_name`` / ``field_to_modify``.
+
+19.0.3.1.8
+~~~~~~~~~~
+
+- fix(security): scope the Create-Group member wizards to the parent
+  change request. ``spp.cr.detail.create_group.member.wizard`` and its
+  ``.phone`` / ``.bank`` children are transient models whose
+  access-control entries grant change-request users read, write, create
+  **and** delete, and no record rule covered them. Transient models get
+  no implicit creator-only scoping from the ORM — ``ir.rule`` applies to
+  them as it does to persistent models, and with no rule the domain
+  resolves to true — so any change-request user could enumerate, read,
+  alter or delete another user's proposed-member data, including names,
+  birthdates, phone numbers and bank account numbers. Each wizard model
+  now carries the same parent-change-request ownership rules as the
+  persistent Create-Group detail rows, scoped on every operation its
+  access-control entry grants.
+
+19.0.3.1.7
+~~~~~~~~~~
+
+- fix(security): require change-request manager rights to apply a change
+  request. ``action_apply()`` runs the apply strategy with elevated
+  rights and is callable over RPC, but the manager restriction existed
+  only on the review button — so a change-request user could apply their
+  own approved request and drive privileged writes such as membership
+  changes. The public entry point is now gated and the mechanism moved
+  to an internal method, so approval-driven auto-apply is unaffected.
+  **Deployments using the API v2 change-request endpoints must grant the
+  API user the change-request manager role to keep using the apply
+  endpoint.**
+
+19.0.3.1.6
+~~~~~~~~~~
+
+- fix(security): derive conflict and duplicate detection from the change
+  actually proposed rather than a user-writable label.
+  ``selected_field_name`` and the detail's ``field_to_modify`` are both
+  writable by the requester, so either could be re-pointed at an
+  unchanged field to clear a field-scoped conflict or drop duplicate
+  similarity to zero. Detection now compares the detail against the
+  registrant. Types whose apply strategy writes outside the configured
+  field mappings fall back to the full configured field set instead of
+  an empty one, so detection cannot silently disable itself.
+
+19.0.3.1.5
+~~~~~~~~~~
+
+- fix(security): scope the CR Requestor, Local Validator and HQ
+  Validator roles to Tier-3 registry read instead of Tier-2 registry
+  viewer. The viewer tier gates the Registry Search portal, a broad
+  registrant-PII enumeration surface these change-request roles do not
+  need; registrant read access is unchanged. A migration re-points the
+  roles and resynchronises existing users, since the role definitions
+  are ``noupdate``.
+
+19.0.3.1.4
+~~~~~~~~~~
+
+- fix(security): add ownership and area record rules to every concrete
+  change-request detail model. Detail rows were reachable by any
+  ``group_cr_user`` regardless of who owned the parent change request,
+  so a requester could read or tamper with another user's detail data
+  over RPC. Each detail model now carries
+  user/validator/validator-HQ/manager rules scoped through its parent
+  change request, plus a global rule mirroring the parent's area filter.
+  ``spp.cr.detail.split_household.member`` is additionally scoped on
+  delete, the one detail model whose access-control entry grants
+  ``unlink`` to change-request users: requesters may delete member rows
+  only on their own requests, while validators and managers keep the
+  unrestricted delete their access-control entries grant.
+
+19.0.3.1.3
+~~~~~~~~~~
+
+- fix(security): route and apply the same single field for
+  dynamic-approval change requests, and freeze the proposed change once
+  the request leaves draft. The selected field, its old/new values and
+  the detail pointer were writable after submission, so a requester
+  could re-route an approval or alter the value that had already been
+  approved. Note the mapped-source-field freeze applies to
+  ``field_mapping`` request types; ``custom``-strategy types freeze only
+  the routing selector.
+
 19.0.3.1.2
 ~~~~~~~~~~
 

spp_change_request_v2/__manifest__.py

@@ -1,6 +1,6 @@
 {
     "name": "OpenSPP Change Request V2",
-    "version": "19.0.3.1.2",
+    "version": "19.0.3.1.10",
     "sequence": 50,
     "category": "OpenSPP",
     "summary": "Configuration-driven change request system with UX improvements, conflict detection and duplicate prevention",

spp_change_request_v2/data/user_roles.xml

@@ -5,6 +5,16 @@ Part of OpenSPP. See LICENSE file for full copyright and licensing details.
 User roles for Change Request module.
 -->
 <odoo noupdate="1">
+    <!--
+        Registry: these CR roles use Tier-3 `group_registry_read` (ACL-only),
+        not Tier-2 `group_registry_viewer`. Both grant the same registrant read
+        ACLs (defined in spp_base_common), but Tier-2 also gates the standalone
+        Registry Search portal menu — an over-broad registrant PII enumeration
+        surface. CR work needs to read the registrant a request is about, not
+        to browse the whole registry via the search portal. (Read is also
+        provided transitively through the group_cr_* chain.)
+    -->
+
     <!-- CR Requestor Role -->
     <!-- For users who can create and submit change requests -->
     <record id="global_role_cr_requestor" model="res.users.role">
@@ -18,7 +28,7 @@ User roles for Change Request module.
             eval="[
                 Command.link(ref('base.group_user')),
                 Command.link(ref('group_cr_manager')),
-                Command.link(ref('spp_registry.group_registry_viewer')),
+                Command.link(ref('spp_registry.group_registry_read')),
                 Command.link(ref('spp_hazard.group_hazard_viewer')),
             ]"
         />
@@ -35,7 +45,7 @@ User roles for Change Request module.
             eval="[
                 Command.link(ref('base.group_user')),
                 Command.link(ref('group_cr_validator')),
-                Command.link(ref('spp_registry.group_registry_viewer')),
+                Command.link(ref('spp_registry.group_registry_read')),
                 Command.link(ref('spp_hazard.group_hazard_viewer')),
             ]"
         />
@@ -52,7 +62,7 @@ User roles for Change Request module.
             eval="[
                 Command.link(ref('base.group_user')),
                 Command.link(ref('group_cr_validator_hq')),
-                Command.link(ref('spp_registry.group_registry_viewer')),
+                Command.link(ref('spp_registry.group_registry_read')),
                 Command.link(ref('spp_hazard.group_hazard_viewer')),
             ]"
         />

spp_change_request_v2/migrations/19.0.3.1.5/post-migration.py

@@ -0,0 +1,50 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Swap the CR roles from Tier-2 ``group_registry_viewer`` to Tier-3
+``group_registry_read``.
+
+The roles' ``implied_ids`` are seeded from ``data/user_roles.xml`` with
+``noupdate="1"``, so a released database (2026.07) keeps the old
+``group_registry_viewer`` link on upgrade and would retain the Registry Search
+portal menu. This migration unlinks the Tier-2 viewer group, links the Tier-3
+read group (same registrant read ACLs, no menu; read is also provided through
+the group_cr_* chain), and re-materializes the group membership of users
+already assigned each role.
+"""
+
+import logging
+
+from odoo import SUPERUSER_ID, Command, api
+
+_logger = logging.getLogger(__name__)
+
+_ROLE_XMLIDS = [
+    "spp_change_request_v2.global_role_cr_requestor",
+    "spp_change_request_v2.local_role_cr_validator",
+    "spp_change_request_v2.global_role_cr_validator_hq",
+]
+
+
+def migrate(cr, version):
+    if not version:
+        return
+    env = api.Environment(cr, SUPERUSER_ID, {})
+    viewer = env.ref("spp_registry.group_registry_viewer", raise_if_not_found=False)
+    read = env.ref("spp_registry.group_registry_read", raise_if_not_found=False)
+    if not viewer or not read:
+        return
+    for xmlid in _ROLE_XMLIDS:
+        role = env.ref(xmlid, raise_if_not_found=False)
+        if not role:
+            continue
+        commands = []
+        if viewer in role.implied_ids:
+            commands.append(Command.unlink(viewer.id))
+        if read not in role.implied_ids:
+            commands.append(Command.link(read.id))
+        if commands:
+            role.implied_ids = commands
+            role.action_update_users()
+            _logger.info(
+                "Migrated role %s: registry viewer -> registry read (re-synced users)",
+                xmlid,
+            )