Skip to content

security(grm): restrict GRM automation rules to GRM staff (drop portal write/create)#266

Open
gonzalesedwin1123 wants to merge 1 commit into
19.0from
security-grm-cel-portal-rule-acl
Open

security(grm): restrict GRM automation rules to GRM staff (drop portal write/create)#266
gonzalesedwin1123 wants to merge 1 commit into
19.0from
security-grm-cel-portal-rule-acl

Conversation

@gonzalesedwin1123

Copy link
Copy Markdown
Member

Problem

spp_grm_cel/security/ir.model.access.csv granted base.group_portal read + write + create
(1,1,1,0) on the global config models spp.grm.routing.rule and spp.grm.escalation.rule,
with no ir.rule record rules scoping them. A low-privileged portal user could therefore
create an always-matching routing or escalation rule via RPC:

  • Routing rules run on ticket creation and can write team_id, user_id, severity,
    priority.
  • Escalation rules run on stage changes and via the hourly cron over all open tickets
    (executed elevated), and can reassign/escalate, send configured notifications, and create
    cases.

So a single planted rule turns portal access into unauthorized, global GRM workflow
modification and potential mass disruption of grievance handling. Severity: High.

Fix

Reduce the two portal ACL rows to read-only (1,0,0,0), matching the existing
base.group_user rows:

access_spp_grm_routing_rule_portal_user,...,base.group_portal,1,0,0,0
access_spp_grm_escalation_rule_portal_user,...,base.group_portal,1,0,0,0

Rule evaluation runs as the current user (ticket.create()apply_routing()
search() on the rule model), so read access must be retained or routing silently stops
applying — exactly why base.group_user also has read-only. Portal users lose the ability to
create or modify global rules; GRM officers/managers retain full management.

Portal ticket submission via the controller is already sudo()d
(spp_grm/controllers/grm_portal.py), so that flow is unaffected either way.

Tests

New spp_grm_cel/tests/test_rule_acl.py:

  • portal user is denied create on both rule models (AccessError);
  • portal user is denied write on both rule models (AccessError);
  • portal user retains read (evaluation still works);
  • a GRM manager can still create both rule types.

Written test-first (4 denial tests failed pre-fix — confirming portal could create/write —
then passed after). ./spp test spp_grm_cel35 passed, 0 failed; ./spp lint clean.

Out of scope (noted for awareness)

  • spp_grm grants portal 1,1,1,0 on spp.grm.ticket itself (portal can create tickets via
    RPC, not only via the sudo controller) — separate question, not changed here.
  • apply_escalation writes escalation_count without sudo() (unlike routing's sudo'd
    counter) — a pre-existing inconsistency; escalation triggered by a read-only portal user now
    fails closed (caught/logged), which is the intended behavior.

The ACL granted base.group_portal read/write/create on spp.grm.routing.rule
and spp.grm.escalation.rule. These are global config models with no record
rules, so a portal user could create an always-matching routing/escalation
rule via RPC. Routing rules run on ticket creation and escalation rules run on
stage changes and via the hourly cron over all open tickets (elevated), so a
single planted rule could reassign/escalate/notify across all grievances.

Reduce the two portal rows to read-only (1,0,0,0), matching the base.group_user
rows. Rule evaluation runs as the current user and only needs read, so ticket
routing still works; portal users can no longer create or modify global rules.
GRM officers/managers retain full management.

Add regression tests: portal users are denied create/write on both rule models
but retain read, while a GRM manager can still create both.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restricts portal users to read-only access for GRM routing and escalation rules by updating the ACL configurations in ir.model.access.csv. It also introduces a comprehensive suite of integration tests in test_rule_acl.py to verify these access control constraints. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.31%. Comparing base (f260ffd) to head (e5b5cca).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             19.0     #266    +/-   ##
========================================
  Coverage   75.31%   75.31%            
========================================
  Files        1098     1104     +6     
  Lines       65317    65586   +269     
========================================
+ Hits        49191    49398   +207     
- Misses      16126    16188    +62     
Flag Coverage Δ
spp_base_common 90.26% <ø> (ø)
spp_grm_cel 76.95% <ø> (?)
spp_programs 65.27% <ø> (ø)
spp_registry 86.83% <ø> (ø)
spp_security 66.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
spp_grm_cel/__manifest__.py 0.00% <ø> (ø)

... and 5 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.

@gonzalesedwin1123 gonzalesedwin1123 marked this pull request as ready for review July 2, 2026 06:56
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