security(roles): scope program/CR roles to Tier-3 registry read (drop registry-search menu) - #353
Merged
gonzalesedwin1123 merged 2 commits intoAug 14, 2026
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0-staging-sec-batch2 #353 +/- ##
===========================================================
+ Coverage 72.49% 72.99% +0.50%
===========================================================
Files 450 469 +19
Lines 31136 31983 +847
===========================================================
+ Hits 22571 23345 +774
- Misses 8565 8638 +73
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Member
Author
|
Staff-review residual filed as follow-up: #355 — |
gonzalesedwin1123
added a commit
that referenced
this pull request
Aug 10, 2026
The changed-field derivation reads apply_mapping_ids, which only the field_mapping apply strategy populates. For a type with dynamic approval and a custom strategy the mapping set is empty, so the derivation returned an empty set rather than None — which makes the field-scoped conflict filter match nothing and drives duplicate similarity to 0, disabling both checks. That was strictly more permissive than the selected_field_name path it replaces, which is set regardless of strategy. No shipped CR type enables dynamic approval today, so this was latent rather than live, but it is reachable by configuration. Now returns None (use the full configured field set) unless the type is field_mapping with mappings present, with a regression test covering the custom-strategy case. Renumbered to 19.0.3.1.5: 19.0 carries 19.0.3.1.1, so the previous 19.0.3.0.3 would have been a downgrade (and collided with #353).
gonzalesedwin1123
force-pushed
the
security-program-cr-registry-viewer
branch
from
August 10, 2026 06:13
5f743ea to
2845af9
Compare
This was referenced Aug 10, 2026
gonzalesedwin1123
changed the base branch from
19.0
to
19.0-staging-sec-batch2
August 14, 2026 09:58
… registry-search menu (#353) Squashed net change of the #353 branch for the batch-2 staging rebase. Program Viewer and the CR requestor/validator/validator-HQ roles were granted spp_registry.group_registry_viewer (Tier-2), which exposes the Registry Search portal and app root. Narrow them to group_registry_read (Tier-3): registrant read is preserved, the registry-search menu subtree is pruned. Migrations on both spp_change_request_v2 (19.0.3.1.4) and spp_programs (19.0.2.2.2) revoke the viewer group from already-assigned users on upgrade via action_update_users().
gonzalesedwin1123
force-pushed
the
security-program-cr-registry-viewer
branch
from
August 14, 2026 13:32
f228b96 to
e1ac2d4
Compare
Applied verbatim from CI's pinned oca-gen output (run 31805166557).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Role links added in the OP#951 menu audit gave several program/CR-scoped roles the Tier-2
spp_registry.group_registry_viewergroup:spp_programs: Program Viewerspp_change_request_v2: CR Requestor, CR Local Validator, CR HQ Validatorgroup_registry_viewergates the standalone Registry Search portal menu(
spp_registry_search.menu_registry_search) — a broad registrant-PII enumeration surface (name, IDnumber, phone, email) — on top of the registrant read ACLs. So a read-only "Program Viewer" (and the
CR roles) got the full registry browse/search UI, not just the registrant data needed for the
program/CR records they work with.
Important nuance (report framing corrected): OpenSPP does not scope the registry per-program.
Row-level need-to-know is area-based (
spp_arearule_res_partner_area_filter, keyed onuser.center_area_ids, populated from local role assignments); it is a no-op for global roles, whichsee the whole registry by design.
group_registry_restrict_selfscopes to the user's own partner(self-service), not per-program. So this change does not (and cannot, without a new feature) make these
roles see "only their program's beneficiaries" — it removes the Registry Search UI surface while
leaving the underlying RPC read governed by the area filter. Severity: medium (over-broad PII
enumeration UI for program/CR roles).
Fix
Switch the four roles from Tier-2
group_registry_viewerto Tier-3group_registry_read:group_registry_readgrants the same registrant read ACLs onres.partner,spp.registry.id,spp.phone.number(defined inspp_base_common) — so all registrantcross-reference reads keep working — but it does not gate the Registry app menu.
group_cr_*chain, so read is doubly preserved.spp.cycle/spp.cycle.membership/spp.program.membershipread for Program Viewer comes fromgroup_programs_viewer, independent of the registry group — unaffected.this reason.
Migration:
user_roles.xmlisnoupdate="1"and the roles ship in the2026.07release, so aplain upgrade would keep the old link. Added
post-migration.pyfor each module(
spp_programs/migrations/19.0.2.2.2,spp_change_request_v2/migrations/19.0.3.0.3) that unlinks theviewer group, links the read group, and re-materializes already-assigned users' group membership via
res.users.role.action_update_users().Tests
Test-first (red → green). New tests in each module assert that each role, once assigned to a fresh user
and synced, does not carry
group_registry_viewer(would fail before the fix) but does keepgroup_registry_readand can functionally read a registrant'sres.partner/spp.registry.id/spp.phone.number(and Program Viewer keepsgroup_programs_viewer)../spp t spp_programs→ 0 failed of 680;./spp t spp_change_request_v2→ 0 failed of 336.No existing tests removed or modified.
Scope / notes
app), Cycle Approver (already Tier-3).
new cross-cutting feature, not a security patch — noted as a separate design question, not included.
spp_programs.2.2dodges security(programs): enforce system-admin authorization on Force Unlock #336's.2.1;spp_change_request_v2.0.3dodges security(cr): route and apply the same single field for dynamic approvals #264.0.1/ security(cr): add record rules to CR detail models (ownership + area) #261.0.2. Whoever merges among {security(cr): add record rules to CR detail models (ownership + area) #261, security(cr): route and apply the same single field for dynamic approvals #264,security(programs): enforce system-admin authorization on Force Unlock #336, this} later re-conflicts on manifest/HISTORY/README (trivial re-merge). No logic conflict.
README.rst/static/description/index.htmlfor both modules to be regenerated from CI's pinned generator.