tryton-do/tryton
2
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Display ids and rule names on access error

Browse Source 
issue8093
review60551002
This commit is contained in:
Cédric Krier
2019-03-03 23:11:17 +01:00
parent c4ddbb064a
commit c255cfe785
7 changed files with 211 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG
+1
View File
@@ -1,3 +1,4 @@
* Display ids and rule names on access error
* Move field definition from Model to Field
* Use number of verbose flag as log level
* Add coroutine concurrency option
trytond/ir/message.xml
+18 -2
View File
@@ -84,11 +84,27 @@ this repository contains the full copyright notices and license terms. -->
<record model="ir.message" id="msg_access_rule_field_error">
<field name="text">You are not allowed to access "%(model)s.%(field)s".</field>
</record>
<record model="ir.message" id="msg_create_rule_error">
<field name="text">You are not allowed to create records of "%(model)s" because they fail on at least one of these rules:
%(rules)s</field>
</record>
<record model="ir.message" id="msg_read_rule_error">
<field name="text">You are not allowed to read records "%(ids)s" of "%(model)s" because of at least one of these rules:
%(rules)s</field>
</record>
<record model="ir.message" id="msg_read_error">
<field name="text">You are trying to read records of "%(model)s" that don't exist anymore.</field>
<field name="text">You are trying to read records "%(ids)s" of "%(model)s" that don't exist anymore.</field>
</record>
<record model="ir.message" id="msg_write_rule_error">
<field name="text">You are not allowed to write to records "%(ids)s" of "%(model)s" because of at least one of these rules:
%(rules)s</field>
</record>
<record model="ir.message" id="msg_write_error">
<field name="text">You are trying to write to records of "%(model)s" that don't exist anymore.</field>
<field name="text">You are trying to write to records "%(ids)s" of "%(model)s" that don't exist anymore.</field>
</record>
<record model="ir.message" id="msg_delete_rule_error">
<field name="text">You are not allowed to delete records "%(ids)s" of "%(model)s" because of at lease one of those rules:
%(rules)s</field>
</record>
<record model="ir.message" id="msg_dict_schema_invalid_domain">
<field name="text">Invalid domain in schema "%(schema)s".</field>
trytond/ir/rule.py
+62 -44
View File
@@ -1,5 +1,7 @@
# This file is part of Tryton. The COPYRIGHT file at the top level of
# this repository contains the full copyright notices and license terms.
from collections import defaultdict
from trytond.i18n import gettext
from trytond.model.exceptions import ValidationError
from ..model import ModelView, ModelSQL, fields, EvalEnvironment, Check
@@ -20,7 +22,9 @@ class DomainError(ValidationError):
class RuleGroup(ModelSQL, ModelView):
"Rule group"
__name__ = 'ir.rule.group'
name = fields.Char('Name', select=True)
name = fields.Char(
"Name", select=True, translate=True, required=True,
help="Displayed to users when access error is raised for this rule.")
model = fields.Many2One('ir.model', 'Model', select=True,
required=True, ondelete='CASCADE')
global_p = fields.Boolean('Global', select=True,
@@ -104,6 +108,8 @@ class Rule(ModelSQL, ModelView):
'- "user" as the current user')
_domain_get_cache = Cache('ir_rule.domain_get', context=False)
modes = {'read', 'write', 'create', 'delete'}
@classmethod
def validate(cls, rules):
super(Rule, cls).validate(rules)
@@ -144,35 +150,29 @@ class Rule(ModelSQL, ModelView):
return (Transaction().user, Transaction().context.get('_datetime'))
@classmethod
def domain_get(cls, model_name, mode='read'):
assert mode in ['read', 'write', 'create', 'delete'], \
'Invalid domain mode for security'
# root user above constraint
if Transaction().user == 0:
if not Transaction().context.get('user'):
return
with Transaction().set_user(Transaction().context['user']):
return cls.domain_get(model_name, mode=mode)
key = (model_name, mode) + cls._get_cache_key()
domain = cls._domain_get_cache.get(key, False)
if domain is not False:
return domain
def get(cls, model_name, mode='read'):
"Return dictionary of non-global and global rules"
pool = Pool()
RuleGroup = pool.get('ir.rule.group')
Model = pool.get('ir.model')
RuleGroup_Group = pool.get('ir.rule.group-res.group')
User_Group = pool.get('res.user-res.group')
cursor = Transaction().connection.cursor()
rule_table = cls.__table__()
rule_group = RuleGroup.__table__()
rule_group_group = RuleGroup_Group.__table__()
user_group = User_Group.__table__()
model = Model.__table__()
user_id = Transaction().user
transaction = Transaction()
assert mode in cls.modes
cursor = transaction.connection.cursor()
user_id = transaction.user
# root user above constraint
if user_id == 0:
user_id = transaction.context.get('user')
if not user_id:
return {}, {}
cursor.execute(*rule_table.join(rule_group,
condition=rule_group.id == rule_table.rule_group
).join(model,
@@ -191,26 +191,7 @@ class Rule(ModelSQL, ModelView):
| (rule_group.default_p == True)
| (rule_group.global_p == True)
)))
ids = [x[0] for x in cursor.fetchall()]
if not ids:
cls._domain_get_cache.set(key, None)
return
clause = {}
clause_global = {}
ctx = cls._get_context()
# Use root user without context to prevent recursion
with Transaction().set_user(0), \
Transaction().set_context(user=0):
for rule in cls.browse(ids):
assert rule.domain, ('Rule domain empty,'
'check if migration was done')
dom = PYSONDecoder(ctx).decode(rule.domain)
if rule.rule_group.global_p:
clause_global.setdefault(rule.rule_group.id, ['OR'])
clause_global[rule.rule_group.id].append(dom)
else:
clause.setdefault(rule.rule_group.id, ['OR'])
clause[rule.rule_group.id].append(dom)
ids = [x for x, in cursor]
# Test if there is no rule_group that have no rule
cursor.execute(*rule_group.join(model,
@@ -222,10 +203,47 @@ class Rule(ModelSQL, ModelView):
condition=rule_group_group.group == user_group.group
).select(rule_group_group.rule_group,
where=user_group.user == user_id))))
fetchone = cursor.fetchone()
if fetchone:
group_id = fetchone[0]
clause[group_id] = []
no_rules = cursor.fetchone()
clause = defaultdict(lambda: ['OR'])
clause_global = defaultdict(lambda: ['OR'])
decoder = PYSONDecoder(cls._get_context())
# Use root user without context to prevent recursion
with transaction.set_user(0), transaction.set_context(user=0):
for rule in cls.browse(ids):
assert rule.domain, ('Rule domain empty,'
'check if migration was done')
dom = decoder.decode(rule.domain)
if rule.rule_group.global_p:
clause_global[rule.rule_group].append(dom)
else:
clause[rule.rule_group].append(dom)
if no_rules:
group_id = no_rules[0]
clause[cls(group_id)] = []
return clause, clause_global
@classmethod
def domain_get(cls, model_name, mode='read'):
transaction = Transaction()
# root user above constraint
if transaction.user == 0:
if not transaction.context.get('user'):
return
with transaction.set_user(Transaction().context['user']):
return cls.domain_get(model_name, mode=mode)
assert mode in cls.modes
key = (model_name, mode) + cls._get_cache_key()
domain = cls._domain_get_cache.get(key, False)
if domain is not False:
return domain
clause, clause_global = cls.get(model_name, mode=mode)
clause = list(clause.values())
if clause:
clause.insert(0, 'OR')
trytond/model/modelsql.py
+70 -30
View File
@@ -748,23 +748,10 @@ class ModelSQL(ModelStorage):
order_by=history_order, limit=history_limit))
fetchall = list(cursor_dict(cursor))
if not len(fetchall) == len({}.fromkeys(sub_ids)):
if domain:
where = red_sql
if history_clause:
where &= history_clause
where &= dom_exp
cursor.execute(*from_.select(table.id, where=where,
order_by=history_order, limit=history_limit))
rowcount = cursor.rowcount
if rowcount == -1 or rowcount is None:
rowcount = len(cursor.fetchall())
if rowcount == len({}.fromkeys(sub_ids)):
raise AccessError(
gettext('ir.msg_access_rule_error',
model=cls.__name__))
raise AccessError(
gettext('ir.msg_read_error',
model=cls.__name__))
cls.__check_domain_rule(
ids, 'read', nodomain='ir.msg_read_error')
cls.__check_domain_rule(ids, 'read')
raise RuntimeError("Undetected access error")
result.extend(fetchall)
else:
result = [{'id': x} for x in ids]
@@ -1154,32 +1141,85 @@ class ModelSQL(ModelStorage):
def __check_domain_rule(cls, ids, mode, nodomain=None):
pool = Pool()
Rule = pool.get('ir.rule')
Model = pool.get('ir.model')
table = cls.__table__()
cursor = Transaction().connection.cursor()
transaction = Transaction()
in_max = transaction.database.IN_MAX
history_clause = None
limit = None
if (mode == 'read'
and cls._history
and transaction.context.get('_datetime')
and not callable(cls.table_query)):
in_max = 1
table = cls.__table_history__()
column = Coalesce(table.write_date, table.create_date)
history_clause = (column <= Transaction().context['_datetime'])
limit = 1
cursor = transaction.connection.cursor()
assert mode in Rule.modes
domain = Rule.domain_get(cls.__name__, mode=mode)
tables = {None: (table, None)}
if domain or nodomain:
def test_domain(ids, domain):
result = []
tables = {None: (table, None)}
if domain:
tables, dom_exp = cls.search_domain(
domain, active_test=False, tables=tables)
from_ = convert_from(None, tables)
for sub_ids in grouped_slice(ids):
sub_ids = list(set(sub_ids))
for sub_ids in grouped_slice(ids, in_max):
sub_ids = set(sub_ids)
where = reduce_ids(table.id, sub_ids)
if history_clause:
where &= history_clause
if domain:
where &= dom_exp
cursor.execute(*from_.select(table.id, where=where))
cursor.execute(
*from_.select(table.id, where=where, limit=limit))
rowcount = cursor.rowcount
if rowcount == -1 or rowcount is None:
rowcount = len(cursor.fetchall())
if rowcount != len(sub_ids):
if domain:
msg = 'ir.msg_access_rule_error'
else:
msg = nodomain
raise AccessError(
gettext(msg, model=cls.__name__))
cursor.execute(
*from_.select(table.id, where=where, limit=limit))
result.extend(
sub_ids.difference([x for x, in cursor]))
return result
domain = Rule.domain_get(cls.__name__, mode=mode)
if not domain and not nodomain:
return
wrong_ids = test_domain(ids, domain)
if wrong_ids:
model = cls.__name__
if Model:
models = Model.search([
('model', '=', cls.__name__),
], limit=1)
if models:
model, = models
model = model.name
ids = ', '.join(map(str, ids[:5]))
if len(wrong_ids) > 5:
ids += '...'
if domain:
rules = []
clause, clause_global = Rule.get(cls.__name__, mode=mode)
if clause:
dom = list(clause.values())
dom.insert(0, 'OR')
if test_domain(wrong_ids, dom):
rules.extend(clause.keys())
for rule, dom in clause_global.items():
if test_domain(wrong_ids, dom):
rules.append(rule)
msg = gettext(
'ir.msg_%s_rule_error' % mode, ids=ids, model=model,
rules='\n'.join(r.name for r in rules))
else:
msg = gettext(nodomain, ids=ids, model=model)
raise AccessError(msg)
@classmethod
def search(cls, domain, offset=0, limit=None, order=None, count=False,
trytond/res/ir.xml
+14
View File
@@ -447,6 +447,7 @@ this repository contains the full copyright notices and license terms. -->
<field name="perm_delete" eval="True"/>
</record>
<record model="ir.rule.group" id="rule_group_menu">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.ui.menu')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -461,6 +462,7 @@ this repository contains the full copyright notices and license terms. -->
<field name="rule_group" ref="rule_group_menu"/>
</record>
<record model="ir.rule.group" id="rule_group_action">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -475,6 +477,7 @@ this repository contains the full copyright notices and license terms. -->
<field name="rule_group" ref="rule_group_action"/>
</record>
<record model="ir.rule.group" id="rule_group_action_keyword">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action.keyword')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -490,6 +493,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_action_report">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action.report')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -505,6 +509,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_action_act_window">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action.act_window')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -520,6 +525,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_action_wizard">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action.wizard')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -535,6 +541,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_action_url">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.action.url')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -768,6 +775,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_sequence">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.sequence')]"/>
<field name="global_p" eval="True"/>
<field name="perm_read" eval="False"/>
@@ -780,6 +788,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_sequence_strict">
<field name="name">User in groups</field>
<field name="model" search="[('model', '=', 'ir.sequence.strict')]"/>
<field name="global_p" eval="True"/>
<field name="perm_read" eval="False"/>
@@ -831,6 +840,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_view_search">
<field name="name">Own view search</field>
<field name="model"
search="[('model', '=', 'ir.ui.view_search')]"/>
<field name="global_p" eval="False"/>
@@ -848,6 +858,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_view_search_admin">
<field name="name">Any view search</field>
<field name="model"
search="[('model', '=', 'ir.ui.view_search')]"/>
<field name="global_p" eval="False"/>
@@ -864,6 +875,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_session">
<field name="name">Own session</field>
<field name="model"
search="[('model', '=', 'ir.session')]"/>
<field name="global_p" eval="True"/>
@@ -876,6 +888,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_session_wizard">
<field name="name">Own session</field>
<field name="model"
search="[('model', '=', 'ir.session.wizard')]"/>
<field name="global_p" eval="True"/>
@@ -888,6 +901,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_ui_menu_favorite">
<field name="name">Own favorite</field>
<field name="model"
search="[('model', '=', 'ir.ui.menu.favorite')]"/>
<field name="global_p" eval="True"/>
trytond/res/user.xml
+3
View File
@@ -106,6 +106,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_user_warning">
<field name="name">Own warning</field>
<field name="model" search="[('model', '=', 'res.user.warning')]"/>
<field name="global_p" eval="True"/>
</record>
@@ -128,6 +129,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_user_application">
<field name="name">Own user application</field>
<field name="model"
search="[('model', '=', 'res.user.application')]"/>
<field name="global_p" eval="False"/>
@@ -141,6 +143,7 @@ this repository contains the full copyright notices and license terms. -->
</record>
<record model="ir.rule.group" id="rule_group_user_application_admin">
<field name="name">Any user application</field>
<field name="model"
search="[('model', '=', 'res.user.application')]"/>
<field name="global_p" eval="False"/>
trytond/tests/test_rule.py
+43 -5
View File
@@ -33,6 +33,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -57,6 +58,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -69,7 +71,34 @@ class ModelRuleTestCase(unittest.TestCase):
}])],
}])
with self.assertRaises(AccessError):
with self.assertRaisesRegex(AccessError, "Field different from foo"):
test, = TestRule.create([{'field': 'foo'}])
@with_transaction()
def test_perm_create_with_default_rule_fail(self):
"Test create with default rule fail"
pool = Pool()
TestRule = pool.get('test.rule')
RuleGroup = pool.get('ir.rule.group')
Model = pool.get('ir.model')
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': False,
'default_p': True,
'perm_read': False,
'perm_create': True,
'perm_write': False,
'perm_delete': False,
'rules': [('create', [{
'domain': json.dumps(
[('field', '!=', 'foo')]),
}])],
}])
with self.assertRaisesRegex(AccessError, "Field different from foo"):
test, = TestRule.create([{'field': 'foo'}])
@with_transaction()
@@ -92,6 +121,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -117,6 +147,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -130,7 +161,7 @@ class ModelRuleTestCase(unittest.TestCase):
}])
test, = TestRule.create([{'field': 'foo'}])
with self.assertRaises(AccessError):
with self.assertRaisesRegex(AccessError, "Field different from foo"):
TestRule.write([test], {'field': 'bar'})
@with_transaction()
@@ -143,6 +174,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -156,7 +188,7 @@ class ModelRuleTestCase(unittest.TestCase):
}])
test, = TestRule.create([{'field': 'bar'}])
with self.assertRaises(AccessError):
with self.assertRaisesRegex(AccessError, "Field different from foo"):
TestRule.write([test], {'field': 'foo'})
@with_transaction()
@@ -179,6 +211,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -204,6 +237,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': False,
@@ -217,7 +251,7 @@ class ModelRuleTestCase(unittest.TestCase):
}])
test, = TestRule.create([{'field': 'foo'}])
with self.assertRaises(AccessError):
with self.assertRaisesRegex(AccessError, "Field different from foo"):
TestRule.delete([test])
@with_transaction()
@@ -240,6 +274,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': True,
@@ -265,6 +300,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': True,
@@ -278,7 +314,7 @@ class ModelRuleTestCase(unittest.TestCase):
}])
test, = TestRule.create([{'field': 'foo'}])
with self.assertRaises(AccessError):
with self.assertRaisesRegex(AccessError, "Field different from foo"):
TestRule.read([test.id], ['field'])
@with_transaction()
@@ -301,6 +337,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': True,
@@ -326,6 +363,7 @@ class ModelRuleTestCase(unittest.TestCase):
model, = Model.search([('model', '=', 'test.rule')])
rule_group, = RuleGroup.create([{
'name': "Field different from foo",
'model': model.id,
'global_p': True,
'perm_read': True,