Skip to content

Commit 171bd86

Browse files
dnplkndlldnplkndll
committed
[IMP] dms: method-based record access, drop computed-permission ir.rules
1 parent 98f4088 commit 171bd86

2 files changed

Lines changed: 67 additions & 156 deletions

File tree

‎dms/models/dms_security_mixin.py‎

Lines changed: 67 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
55

66

7+
import functools
78
from logging import getLogger
89

9-
from odoo import SUPERUSER_ID, api, fields, models
10+
from odoo import api, fields, models
1011
from odoo.exceptions import AccessError
1112
from odoo.fields import Domain
1213
from odoo.tools import SQL
@@ -197,34 +198,25 @@ def _get_domain_by_access_groups(self, operation):
197198
return result
198199

199200
@api.model
200-
def _get_permission_domain(self, operator, value, operation):
201-
"""Abstract logic for searching computed permission fields."""
202-
_self = self
203-
# HACK ir.rule domains are evaluated in superuser mode while env.uid
204-
# stays the acting user, so `su` together with a non-root uid means we
205-
# are resolving the `permission_<op> = user.id` rule on that user's
206-
# behalf. The Domain engine coerces that sentinel to this Boolean
207-
# field's type before we get here, so we rely on env.uid (used by
208-
# _get_access_groups_query) rather than the value to build the domain.
209-
if self.env.su and self.env.uid != SUPERUSER_ID:
210-
_self = self.sudo(False)
211-
value = bool(value)
212-
# Tricky one, to know if you want to search
213-
# positive or negative access
214-
positive = (operator not in Domain.NEGATIVE_OPERATORS) == bool(value)
215-
if _self.env.su:
216-
# You're SUPERUSER_ID
217-
return Domain.TRUE if positive else Domain.FALSE
218-
219-
result = Domain.OR(
201+
def _get_dms_access_domain(self, operation):
202+
"""Domain matching the records the current user may access for
203+
``operation`` through DMS access groups or inheritance."""
204+
return Domain.OR(
220205
[
221-
_self._get_domain_by_access_groups(operation),
222-
_self._get_domain_by_inheritance(operation),
206+
self._get_domain_by_access_groups(operation),
207+
self._get_domain_by_inheritance(operation),
223208
]
224209
)
225-
if not positive:
226-
result = ~Domain(result)
227-
return result
210+
211+
@api.model
212+
def _get_permission_domain(self, operator, value, operation):
213+
"""Search implementation of the computed ``permission_<op>`` fields,
214+
used by field domains (e.g. ``directory_id``'s create filter)."""
215+
positive = (operator not in Domain.NEGATIVE_OPERATORS) == bool(value)
216+
if self.env.su:
217+
return Domain.TRUE if positive else Domain.FALSE
218+
result = self._get_dms_access_domain(operation)
219+
return result if positive else ~result
228220

229221
@api.model
230222
def _search_permission_create(self, operator, value):
@@ -242,49 +234,57 @@ def _search_permission_unlink(self, operator, value):
242234
def _search_permission_write(self, operator, value):
243235
return self._get_permission_domain(operator, value, "write")
244236

245-
def filtered_domain(self, domain):
246-
"""This method is needed to inhibit the behavior when called from the
247-
_check_access() method with sudo() https://github.com/odoo/odoo/blob/fc737a147b9aefbd6ae5d111835ce3f4f7b4240a/odoo/models.py#L4465.
248-
It would cause the error that multiple records are not accessed to be
249-
displayed.
250-
The _filtered_access() method is also overwritten to prevent this sudo()
251-
specific behavior and to be able to access only the appropriate records.
252-
"""
253-
if self.env.su:
254-
return self
255-
return super().filtered_domain(domain)
256-
257-
def _filtered_access_no_recursion(self, operation: str):
258-
"""This method is just the same as _filtered_access
259-
but it can not be called withoud super due to
260-
recursion error.
261-
"""
262-
if self and not self.env.su and (result := self._check_access(operation)):
263-
return self - result[0]
264-
return self
237+
def _search(
238+
self,
239+
domain,
240+
offset=0,
241+
limit=None,
242+
order=None,
243+
*,
244+
bypass_access=False,
245+
**kwargs,
246+
):
247+
"""Restrict searches to the records the current user may read through
248+
DMS access groups or inheritance (mirrors ``mail.message._search``)."""
249+
if self.env.su or bypass_access:
250+
return super()._search(
251+
domain, offset, limit, order, bypass_access=bypass_access, **kwargs
252+
)
253+
domain = Domain.AND([Domain(domain), self._get_dms_access_domain("read")])
254+
return super()._search(domain, offset, limit, order, **kwargs)
265255

266-
def _filtered_access(self, operation):
267-
# Only kept to not break inheritance; see next comment
268-
result = super()._filtered_access(operation)
269-
# HACK Always fall back to applying rules by SQL.
270-
# Upstream `_filtered_access()` doesn't use computed fields
271-
# search methods. Thus, it will take the `[('permission_{operation}',
272-
# '=', user.id)]` rule literally. Obviously that will always fail
273-
# because `self[f"permission_{operation}"]` will always be a `bool`,
274-
# while `user.id` will always be an `int`.
275-
result |= self._filtered_access_no_recursion(operation)
256+
def _check_access(self, operation):
257+
"""Add the DMS access-group / inheritance restriction to the
258+
record-level access check (mirrors ``mail.message._check_access``)."""
259+
result = super()._check_access(operation)
260+
if self.env.su or not any(self._ids):
261+
return result
262+
records = self - result[0] if result else self
263+
forbidden = records._get_forbidden_dms_access(operation)
264+
if forbidden:
265+
if result:
266+
return result[0] + forbidden, result[1]
267+
Rule = self.env["ir.rule"]
268+
return forbidden, functools.partial(
269+
Rule._make_access_error, operation, forbidden
270+
)
276271
return result
277272

278-
def _check_access_dms_record(self, operation: str) -> tuple | None:
279-
"""Specific method "similar" to _check_access() but with a different
280-
behavior: check if you do not really have access to any of the records
281-
in to avoid performing the corresponding create/write/unlink action."""
282-
if any(self._ids) and not self.env.su:
283-
Rule = self.env["ir.rule"]
284-
domain = Rule._compute_domain(self._name, operation)
285-
items = self.with_context(active_test=False).search(domain)
286-
if any(x_id not in items.ids for x_id in self.ids):
287-
raise Rule._make_access_error(operation, (self - items))
273+
def _get_forbidden_dms_access(self, operation):
274+
"""Return the subset of ``self`` the current user cannot access for
275+
``operation`` under the DMS access-group / inheritance rules. The
276+
domain carries an SQL sub-query, so it is resolved by search (not by
277+
``filtered_domain``), bypassing access to evaluate exactly this domain."""
278+
domain = Domain.AND(
279+
[
280+
Domain([("id", "in", self.ids)]),
281+
self._get_dms_access_domain(operation),
282+
]
283+
)
284+
allowed = self.browse(
285+
self.with_context(active_test=False)._search(domain, bypass_access=True)
286+
)
287+
return self - allowed
288288

289289
@api.model_create_multi
290290
def create(self, vals_list):
@@ -296,13 +296,5 @@ def create(self, vals_list):
296296
res.flush_recordset()
297297
# Go back to the original sudo state and check we really had creation permission
298298
res = res.sudo(self.env.su)
299-
res._check_access_dms_record("create")
299+
res.check_access("create")
300300
return res
301-
302-
def write(self, vals):
303-
self._check_access_dms_record("write")
304-
return super().write(vals)
305-
306-
def unlink(self):
307-
self._check_access_dms_record("unlink")
308-
return super().unlink()

‎dms/security/security.xml‎

Lines changed: 0 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -111,85 +111,4 @@
111111
<field name="perm_unlink" eval="1" />
112112
<field name="domain_force">[('is_hidden', '=', True)]</field>
113113
</record>
114-
<!-- These rules leverage computed permission management -->
115-
<record id="rule_directory_computed_create" model="ir.rule">
116-
<field name="name">Apply computed create permissions.</field>
117-
<field name="model_id" ref="model_dms_directory" />
118-
<field name="global" eval="True" />
119-
<field name="perm_read" eval="0" />
120-
<field name="perm_create" eval="1" />
121-
<field name="perm_write" eval="0" />
122-
<field name="perm_unlink" eval="0" />
123-
<field name="domain_force">[('permission_create', '=', user.id)]</field>
124-
</record>
125-
<record id="rule_directory_computed_read" model="ir.rule">
126-
<field name="name">Apply computed read permissions.</field>
127-
<field name="model_id" ref="model_dms_directory" />
128-
<field name="global" eval="True" />
129-
<field name="perm_read" eval="1" />
130-
<field name="perm_create" eval="0" />
131-
<field name="perm_write" eval="0" />
132-
<field name="perm_unlink" eval="0" />
133-
<field name="domain_force">[('permission_read', '=', user.id)]</field>
134-
</record>
135-
<record id="rule_directory_computed_unlink" model="ir.rule">
136-
<field name="name">Apply computed unlink permissions.</field>
137-
<field name="model_id" ref="model_dms_directory" />
138-
<field name="global" eval="True" />
139-
<field name="perm_read" eval="0" />
140-
<field name="perm_create" eval="0" />
141-
<field name="perm_write" eval="0" />
142-
<field name="perm_unlink" eval="1" />
143-
<field name="domain_force">[('permission_unlink', '=', user.id)]</field>
144-
</record>
145-
<record id="rule_directory_computed_write" model="ir.rule">
146-
<field name="name">Apply computed write permissions.</field>
147-
<field name="model_id" ref="model_dms_directory" />
148-
<field name="global" eval="True" />
149-
<field name="perm_read" eval="0" />
150-
<field name="perm_create" eval="0" />
151-
<field name="perm_write" eval="1" />
152-
<field name="perm_unlink" eval="0" />
153-
<field name="domain_force">[('permission_write', '=', user.id)]</field>
154-
</record>
155-
<record id="rule_file_computed_create" model="ir.rule">
156-
<field name="name">Apply computed create permissions.</field>
157-
<field name="model_id" ref="model_dms_file" />
158-
<field name="global" eval="True" />
159-
<field name="perm_read" eval="0" />
160-
<field name="perm_create" eval="1" />
161-
<field name="perm_write" eval="0" />
162-
<field name="perm_unlink" eval="0" />
163-
<field name="domain_force">[('permission_create', '=', user.id)]</field>
164-
</record>
165-
<record id="rule_file_computed_read" model="ir.rule">
166-
<field name="name">Apply computed read permissions.</field>
167-
<field name="model_id" ref="model_dms_file" />
168-
<field name="global" eval="True" />
169-
<field name="perm_read" eval="1" />
170-
<field name="perm_create" eval="0" />
171-
<field name="perm_write" eval="0" />
172-
<field name="perm_unlink" eval="0" />
173-
<field name="domain_force">[('permission_read', '=', user.id)]</field>
174-
</record>
175-
<record id="rule_file_computed_unlink" model="ir.rule">
176-
<field name="name">Apply computed unlink permissions.</field>
177-
<field name="model_id" ref="model_dms_file" />
178-
<field name="global" eval="True" />
179-
<field name="perm_read" eval="0" />
180-
<field name="perm_create" eval="0" />
181-
<field name="perm_write" eval="0" />
182-
<field name="perm_unlink" eval="1" />
183-
<field name="domain_force">[('permission_unlink', '=', user.id)]</field>
184-
</record>
185-
<record id="rule_file_computed_write" model="ir.rule">
186-
<field name="name">Apply computed write permissions.</field>
187-
<field name="model_id" ref="model_dms_file" />
188-
<field name="global" eval="True" />
189-
<field name="perm_read" eval="0" />
190-
<field name="perm_create" eval="0" />
191-
<field name="perm_write" eval="1" />
192-
<field name="perm_unlink" eval="0" />
193-
<field name="domain_force">[('permission_write', '=', user.id)]</field>
194-
</record>
195114
</odoo>

0 commit comments

Comments
 (0)