Skip to content

security(roles): scope program/CR roles to Tier-3 registry read (drop registry-search menu)#353

Draft
gonzalesedwin1123 wants to merge 4 commits into
19.0from
security-program-cr-registry-viewer
Draft

security(roles): scope program/CR roles to Tier-3 registry read (drop registry-search menu)#353
gonzalesedwin1123 wants to merge 4 commits into
19.0from
security-program-cr-registry-viewer

Conversation

@gonzalesedwin1123

Copy link
Copy Markdown
Member

Problem

Role links added in the OP#951 menu audit gave several program/CR-scoped roles the Tier-2
spp_registry.group_registry_viewer group:

  • spp_programs: Program Viewer
  • spp_change_request_v2: CR Requestor, CR Local Validator, CR HQ Validator

group_registry_viewer gates the standalone Registry Search portal menu
(spp_registry_search.menu_registry_search) — a broad registrant-PII enumeration surface (name, ID
number, 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_area rule_res_partner_area_filter, keyed on
user.center_area_ids, populated from local role assignments); it is a no-op for global roles, which
see the whole registry by design. group_registry_restrict_self scopes 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_viewer to Tier-3 group_registry_read:

  • Tier-3 group_registry_read grants the same registrant read ACLs on res.partner,
    spp.registry.id, spp.phone.number (defined in spp_base_common) — so all registrant
    cross-reference reads keep working — but it does not gate the Registry app menu.
  • The CR roles also imply Tier-3 read/write via their group_cr_* chain, so read is doubly preserved.
  • spp.cycle / spp.cycle.membership / spp.program.membership read for Program Viewer comes from
    group_programs_viewer, independent of the registry group — unaffected.
  • This mirrors the existing Program Cycle Approver precedent, which already uses Tier-3 for exactly
    this reason.

Migration: user_roles.xml is noupdate="1" and the roles ship in the 2026.07 release, so a
plain upgrade would keep the old link. Added post-migration.py for each module
(spp_programs/migrations/19.0.2.2.2, spp_change_request_v2/migrations/19.0.3.0.3) that unlinks the
viewer 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 keep
group_registry_read and can functionally read a registrant's res.partner/spp.registry.id/
spp.phone.number (and Program Viewer keeps group_programs_viewer).

./spp t spp_programs0 failed of 680; ./spp t spp_change_request_v20 failed of 336.
No existing tests removed or modified.

Scope / notes

Add tests asserting Program Viewer and the CR Requestor/Local/HQ Validator
roles do NOT confer spp_registry.group_registry_viewer (which gates the
Registry Search portal menu) but DO keep registrant read (res.partner,
spp.registry.id, spp.phone.number) via Tier-3 group_registry_read.
… viewer menu

Program Viewer, CR Requestor, CR Local Validator and CR HQ Validator implied
spp_registry.group_registry_viewer (Tier-2), which gates the standalone
Registry Search portal menu and exposed a broad registrant-PII enumeration
surface to these program/CR-scoped roles. Switch them to Tier-3
group_registry_read, which grants the same registrant read ACLs (defined in
spp_base_common; also provided to CR roles via the group_cr_* chain) without
the Registry app menu.

user_roles.xml is noupdate=1 and the roles ship in the 2026.07 release, so add
post-migrations (spp_programs 19.0.2.2.2, spp_change_request_v2 19.0.3.0.3)
that unlink the viewer group, link the read group, and re-sync already-assigned
users via action_update_users.

Reduces the registry-search UI surface; underlying RPC read scope is unchanged
(governed by the area filter rule_res_partner_area_filter).
spp_programs 19.0.2.2.0 -> 19.0.2.2.2 (dodges #336's .2.1);
spp_change_request_v2 19.0.3.0.0 -> 19.0.3.0.3 (dodges #264 .0.1 / #261 .0.2).
README.rst / index.html to be regenerated from CI's pinned generator.
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.52%. Comparing base (1caf794) to head (5f743ea).
⚠️ Report is 3 commits behind head on 19.0.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             19.0     #353      +/-   ##
==========================================
- Coverage   74.28%   72.52%   -1.77%     
==========================================
  Files         372      253     -119     
  Lines       25385    16405    -8980     
==========================================
- Hits        18857    11897    -6960     
+ Misses       6528     4508    -2020     
Flag Coverage Δ
spp_analytics ?
spp_api_v2_change_request 66.53% <ø> (?)
spp_api_v2_cycles 71.03% <ø> (+0.38%) ⬆️
spp_api_v2_entitlements 70.23% <ø> (+0.27%) ⬆️
spp_api_v2_gis ?
spp_api_v2_programs 92.22% <ø> (+0.19%) ⬆️
spp_api_v2_simulation 71.19% <ø> (+0.07%) ⬆️
spp_audit_programs ?
spp_base_common 91.07% <ø> (+0.80%) ⬆️
spp_case_demo ?
spp_case_entitlements 100.00% <ø> (+2.38%) ⬆️
spp_case_programs 100.00% <ø> (+2.85%) ⬆️
spp_change_request_v2 77.91% <ø> (?)
spp_cr_type_assign_program 92.07% <ø> (+0.90%) ⬆️
spp_dci_compliance 93.01% <ø> (+0.24%) ⬆️
spp_dci_demo 94.28% <ø> (+0.88%) ⬆️
spp_dci_server_social 89.57% <ø> (+0.19%) ⬆️
spp_demo ?
spp_demo_phl_luzon ?
spp_drims ?
spp_drims_sl ?
spp_drims_sl_demo ?
spp_farmer_registry_cr 61.24% <ø> (?)
spp_farmer_registry_demo ?
spp_gis_report ?
spp_grm_demo ?
spp_indicator ?
spp_indicator_studio ?
spp_metric_service ?
spp_mis_demo_v2 ?
spp_programs 65.30% <ø> (+0.03%) ⬆️
spp_registry 86.94% <ø> (+0.10%) ⬆️
spp_security 69.56% <ø> (+2.89%) ⬆️
spp_simulation ?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 262 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…y-viewer scoping

README.rst / index.html regenerated from CI's pinned oca-gen output for
spp_programs (19.0.2.2.2) and spp_change_request_v2 (19.0.3.0.3); test files
reformatted to CI's ruff-format line length (local hook env differed).
@gonzalesedwin1123

Copy link
Copy Markdown
Member Author

Staff-review residual filed as follow-up: #355global_role_program_validator still carries Tier-2 group_registry_viewer (Registry Search menu). Left out of this PR's scope (it holds registry write, so the menu is more defensible); tracked for consistency with Cycle Approver and the roles fixed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant