Skip to content

[REVIEW] dms: _get_permission_domain sentinel fix vs _search override#26

Open
dnplkndll wants to merge 3 commits into
19.0-mig-dms-reorderfrom
19.0-mig-dms-search-alt
Open

[REVIEW] dms: _get_permission_domain sentinel fix vs _search override#26
dnplkndll wants to merge 3 commits into
19.0-mig-dms-reorderfrom
19.0-mig-dms-search-alt

Conversation

@dnplkndll

Copy link
Copy Markdown

Isolated review of the alternative to the new _search override (victoralmau flagged the override on OCA#475).

This PR's diff = the swap in isolation (+8 / −18, net −10): remove the _search override; fix the sentinel detection in the existing _get_permission_domain.

Why the override existed

19.0's Domain engine coerces the permission_<op> = user.id ir.rule sentinel to the Boolean field's type before the field's search= method runs (observed live: op=in value=OrderedSet([True]) instead of value=8). So value == self.env.uid no longer detects the per-user permission check, _get_permission_domain falls through to return Domain.TRUE, and the ir.rule stops filtering search() → read-isolation leak.

The alternative

During ir.rule eval the env is su=True but env.uid is still the acting user. Detect the rule context by su and uid != SUPERUSER_ID and build the domain from env.uid (which _get_access_groups_query already uses) instead of the coerced value. Enforcement stays in the existing rule→search-method path — no parallel _search filter.

Proof (dev-runner 19.0, fresh DB, --without-demo=all)

  • override removed, sentinel not fixed → 5 failed/64 (test_file_access, test_unaccessible_file, test_inaccessible_directory, test_storage_attachment_record_db_unlink, test_permission_portal_user_access_other_attachment).
  • this PR (sentinel fixed, override removed) → 0 failed/64 incl. both portal tours.
  • All 8 permission_<op> rules are positive = user.id; no other code searches these fields under sudo.

Not for upstream as-is — comparison artifact for deciding which fix lands on OCA#475.

…earch override

Alternative to the _search() override: 19.0's Domain engine coerces the
`permission_<op> = user.id` ir.rule sentinel to the Boolean field type
before the search method runs, so `value == self.env.uid` no longer
detects the per-user permission check. Detect it by su + non-root uid and
build the domain from env.uid instead. Restores the rule-based read
isolation without a parallel _search filter (net -10 lines).
@dnplkndll dnplkndll force-pushed the 19.0-mig-dms-reorder branch 3 times, most recently from b628ef8 to 171bd86 Compare June 23, 2026 10:55
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