fix(starter_sp_mis): make registry access control persist and enforce - #411
Open
emjay0921 wants to merge 3 commits into
Open
fix(starter_sp_mis): make registry access control persist and enforce#411emjay0921 wants to merge 3 commits into
emjay0921 wants to merge 3 commits into
Conversation
The "Restrict Registry Edits to Admin Only" setting could not be turned
off, and did not restrict anything when on.
Odoo stores a False config_parameter by deleting the row, and default_get
falls back to the field default when the row is missing, so default=True
made "off" unrepresentable: the toggle sprang back on at every reload.
The enforcement side read a missing row as False, so unticking the box in
fact disabled the restriction while the form went on claiming the registry
was locked. Persist the value explicitly as a string and take the install
default from the data file, which is now noupdate so an upgrade stops
re-locking a registry an administrator deliberately opened.
Enforcement was client-side only: JavaScript hid buttons with display:none
and nothing on the server refused the write, so RPC and import went
straight through. The list "New" button was never hidden either, because
the patch assigned a canCreate property that Odoo 19's ListController does
not read - its template gates on activeActions.create.
Replace all of it with a _check_access override on res.partner. That is
the single chokepoint behind check_access, has_access and the ORM's own
create/write/unlink guards, so one override refuses the change on every
path and removes New/Edit/Delete from registry views for free:
ir.ui.view._postprocess_access_rights stamps create="false" onto an arch
whenever has_access('create') is False. Scoped to registrants so the
setting cannot lock the Contacts app.
The demo access-control tests assert the plain role model and were only
passing because the restriction did nothing, so they now pin the switch
off explicitly.
emjay0921
marked this pull request as ready for review
August 12, 2026 02:32
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #411 +/- ##
==========================================
+ Coverage 71.49% 71.54% +0.04%
==========================================
Files 243 242 -1
Lines 20785 20814 +29
==========================================
+ Hits 14860 14891 +31
+ Misses 5925 5923 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
CI's semgrep flags odoo-sudo-without-context on all three sudo() calls. Each is deliberate, so record why next to it: - writing the config parameter is a Settings-manager operation, and the settings form is already gated on that group; - reading it is how the guard decides whether to withhold access, so every user has to be able to read it; - filtering on is_registrant as the acting user would recurse straight back into the access check being evaluated. The pragma has to sit on the line immediately above the match, not at the head of the comment block, or semgrep does not associate the two. Also applies ruff's preferred spacing on a docstring that opens with a quoted word.
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
2 participants
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.
Why is this change needed?
OP#1142 — SPMIS settings unable to toggle off. Test pass.
The "Restrict Registry Edits to Admin Only" setting could not be turned off, and did not restrict anything when it was on.
Odoo stores a False
config_parameterby deleting the row, anddefault_getfalls back to the field default when the row is missing — sodefault=Truemade "off" unrepresentable and the toggle sprang back on at every reload. Worse, the enforcement side read a missing row as False, so unticking the box in fact disabled the restriction while the form went on claiming the registry was locked.Enforcement was client-side only: JavaScript hid buttons with
display: noneand nothing on the server refused the write, so RPC and import went straight through. The list "New" button was never hidden either, because the patch assigned acanCreateproperty that Odoo 19's ListController does not read — its template gates onactiveActions.create.How was the change implemented?
set_valueswrites the value explicitly as a string, so "off" is a stored fact rather than an absence. The install default moved todata/config_parameters.xml, nownoupdateso an upgrade stops silently re-locking a registry an administrator deliberately opened._check_accessoverride onres.partner. That is the single chokepoint behindcheck_access,has_accessand the ORM's own create/write/unlink guards, so one override refuses the change on every path and removes New/Edit/Delete from registry views for free:ir.ui.view._postprocess_access_rightsstampscreate="false"onto an arch wheneverhas_access('create')is False.registry_restriction.js(260 lines) and the controller it called are deleted — the DOM hiding is no longer needed and never worked on lists anyway.New unit tests
14, in
spp_starter_sp_mis/tests/test_registry_restriction.py, covering the toggle round-trip in both directions, that the form and the enforcement can never disagree, that non-admins are refused create/write/unlink on registrants, that admins are exempt, that plain contacts are unaffected, and that the registry arch dropscreatefor a restricted user.Unit tests executed by the author
Re-run after merging
19.0up:spp_mis_demo_v2is the only dependent module. Four of its access-control tests asserted the plain role model and were only passing because the restriction did nothing; they now pin the switch off explicitly, since they measure ACLs rather than this setting../spp lintclean.How to test manually
Related links
https://openspp.openproject.com/work_packages/1142
Reviewer notes
This is a real behaviour change for the SP-MIS bundle. The setting ships enabled and now actually works, so registrars lose registrant create/edit/delete by default where the restriction was previously decorative.
One residual on upgrade: databases where an administrator had already unticked the setting have no parameter row, because the old code deleted it. The
noupdaterecord is recreated as True once, then stays where it is put.