Skip to content

[IMP] dms: enforce record access in code instead of computed-permission ir.rules - #31

Open
dnplkndll wants to merge 1 commit into
19.0-mig-dmsfrom
19.0-mig-dms-access-tighten
Open

[IMP] dms: enforce record access in code instead of computed-permission ir.rules#31
dnplkndll wants to merge 1 commit into
19.0-mig-dmsfrom
19.0-mig-dms-access-tighten

Conversation

@dnplkndll

@dnplkndll dnplkndll commented Jul 26, 2026

Copy link
Copy Markdown

Parked here for review after OCA#475 merges. This is the access modernization that was previously the last commit on the OCA#475 branch; it was dropped from the migration so the MIG stays scope-clean, and rebuilt here as one commit on top of that branch.

What it does

Replaces the eight permission_<op> ir.rules — and the overrides that existed to compensate for them (filtered_domain, _filtered_access, _filtered_access_no_recursion, _check_access_dms_record, manual write/unlink) — with the pattern core uses for dynamic per-user visibility: _search ANDs the DMS access domain into SQL, _check_access subtracts forbidden records, and create/write/unlink are enforced by core via check_access. Same shape as mail.message (which has no read ir.rule at all) and ir.attachment.

Access rules themselves are unchanged: _get_domain_by_access_groups() and _get_domain_by_inheritance() untouched, permission_<op> fields keep their search= for field domains.

Why it is NOT in the migration

It is a refactor, not a fix. Verified against the [MIG] commit alone on a 19.0 DB with demo data, using a user with dms.group_dms_user and no DMS access groups:

[MIG] only with this commit
records visible 2 directories / 5 files 2 directories / 5 files
sudo() bypass 41/41 files, 16/16 dirs 41/41, 16/16
read()/write() on forbidden record AccessError AccessError
field-domain search on permission_create works works
dms tests, demo on 65 passed, 0 failed 65 passed, 0 failed

The [MIG] commit already adapts the 19.0 sentinel coercion (it keys on env.su + non-root env.uid rather than reading the coerced value), so nothing here is required by 19.0.

The case for it, on its own merits

  • The env.su/env.uid heuristic infers "we are inside ir.rule evaluation" from core internals rather than from a contract — it breaks silently if core changes how rule domains are evaluated.
  • filtered_domain() returning self under su suppresses filtering for all domains on every DMS model, not just the permission ones.
  • −102 lines, and what remains matches core.

Assisted-by: Claude Fable 5

…on ir.rules

Replace the eight `permission_<op>` ir.rules and the overrides that
compensated for them (`filtered_domain`, `_filtered_access`,
`_filtered_access_no_recursion`, `_check_access_dms_record` and the manual
`write`/`unlink` checks) with the pattern core uses for dynamic per-user
visibility: `_search` ANDs the DMS access domain into SQL and `_check_access`
subtracts the forbidden records. `create`, `write` and `unlink` are then
enforced by core through `check_access`.

The access rules themselves are unchanged: `_get_domain_by_access_groups()`
and `_get_domain_by_inheritance()` are untouched, and the `permission_<op>`
fields keep their `search=` methods for use in field domains.

Behaviour verified equivalent to the previous implementation on a 19.0
database with demo data: same records visible to a restricted user, same
`sudo()` bypass, same AccessError on forbidden read/write, and the same
65 passing dms tests.

Assisted-by: Claude Fable 5
@dnplkndll
dnplkndll force-pushed the 19.0-mig-dms-access-tighten branch from e76da03 to 99a908e Compare July 26, 2026 21:21
@dnplkndll dnplkndll changed the title [IMP] dms: tighten method-based access implementation [IMP] dms: enforce record access in code instead of computed-permission ir.rules Jul 26, 2026
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