diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index 7c7d237ba1..56d5d5d5ff 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Enforce readonly on field when checking access * Add the icon parameter to register_authentication_service * Add routes for sao custom in base router * Replace the administration group by a flag on the user diff --git a/trytond/doc/ref/models.rst b/trytond/doc/ref/models.rst index 95fc7210e9..a5c4d263dd 100644 --- a/trytond/doc/ref/models.rst +++ b/trytond/doc/ref/models.rst @@ -428,8 +428,9 @@ Class methods: .. classmethod:: ModelStorage.check_modification(mode, records[, values[, external]]) - Method called after ``records`` are created and before ``records`` are - modified with ``values`` or deleted. + Method called after ``records`` are created with ``values`` as a list of + created values and before ``records`` are modified with the modified + ``values`` or deleted without ``values``. ``external`` specifies whether remote access check must be enforced. .. note:: diff --git a/trytond/trytond/ir/message.xml b/trytond/trytond/ir/message.xml index d1ea423a59..64a89c6b25 100644 --- a/trytond/trytond/ir/message.xml +++ b/trytond/trytond/ir/message.xml @@ -244,6 +244,12 @@ this repository contains the full copyright notices and license terms. --> You are not allowed to delete records "%(ids)s" of "%(model)s" because of at lease one of those rules: %(rules)s + + You are not allowed to modify the field "%(field)s" of "%(model)s". + + + You are not allowed to modify the field "%(field)s" in record "%(record)s" of "%(model)s". + At date/time: %(datetime)s diff --git a/trytond/trytond/model/modelsql.py b/trytond/trytond/model/modelsql.py index 29153bd1b6..2127316cb9 100644 --- a/trytond/trytond/model/modelsql.py +++ b/trytond/trytond/model/modelsql.py @@ -1008,6 +1008,7 @@ class ModelSQL(ModelStorage): pool = Pool() Translation = pool.get('ir.translation') + vlist_orig = vlist vlist = cls._before_create(vlist) table = cls.__table__() @@ -1179,7 +1180,7 @@ class ModelSQL(ModelStorage): cls._insert_history(new_ids) cls.__check_domain_rule(new_ids, 'create') - return cls.browse(cls._after_create(new_ids)) + return cls.browse(cls._after_create(new_ids, vlist_orig)) @classmethod def read(cls, ids, fields_names): diff --git a/trytond/trytond/model/modelstorage.py b/trytond/trytond/model/modelstorage.py index 30c184d0ae..48b01aea06 100644 --- a/trytond/trytond/model/modelstorage.py +++ b/trytond/trytond/model/modelstorage.py @@ -324,7 +324,7 @@ class ModelStorage(Model): raise NotImplementedError @classmethod - def _after_create(cls, ids): + def _after_create(cls, ids, vlist): Trigger = Pool().get('ir.trigger') transaction = Transaction() check_access = transaction.user and transaction.check_access @@ -336,7 +336,7 @@ class ModelStorage(Model): records = cls.browse(sub_ids) cls._validate(records) cls.check_modification( - 'create', records, external=check_access) + 'create', records, values=vlist, external=check_access) cls._compute_fields(records) cls.on_modification('create', records) if triggers: @@ -562,6 +562,52 @@ class ModelStorage(Model): def check_modification(cls, mode, records, values=None, external=False): assert mode in {'create', 'write', 'delete'} + def test_readonly(records, field_name, override): + field = cls._fields[field_name] + if isinstance(field, (fields.One2Many, fields.Many2Many)): + # Must be enforced on the target + return + if field.readonly: + raise AccessError( + gettext('ir.msg_modification_readonly_field_error', + **cls.__names__(field=field_name))) + if 'readonly' not in field.states: + return + if is_pyson(field.states['readonly']): + pyson_readonly = PYSONEncoder().encode( + field.states['readonly']) + for record in sub_records: + readonly = _record_eval_pyson( + record, pyson_readonly, encoded=True, + override=override) + if readonly: + raise AccessError( + gettext( + 'ir.' + 'msg_modification_readonly_field_record_error', + **cls.__names__( + field=field_name, + record=record))) + elif field.states['readonly']: + for record in sub_records: + raise AccessError( + gettext( + 'ir.msg_modification_readonly_field_record_error', + **cls.__names__( + field=field_name, + record=record))) + + if external and values and mode in {'create', 'write'}: + if mode == 'create': + iterator = (([r], v) for r, v in zip(records, values)) + override = {'id': -1} + else: + iterator = ((records, values),) + override = {} + for sub_records, sub_values in iterator: + for field_name in sub_values: + test_readonly(sub_records, field_name, override) + @classmethod def on_modification(cls, mode, records, field_names=None): assert mode in {'create', 'write', 'delete'} @@ -1449,25 +1495,6 @@ class ModelStorage(Model): def _validate(cls, records, field_names=None): pool = Pool() - def is_pyson(test): - if isinstance(test, PYSON): - return True - if isinstance(test, (list, tuple)): - for i in test: - if isinstance(i, PYSON): - return True - if isinstance(i, (list, tuple)): - if is_pyson(i): - return True - if isinstance(test, dict): - for key, value in list(test.items()): - if isinstance(value, PYSON): - return True - if isinstance(value, (list, tuple, dict)): - if is_pyson(value): - return True - return False - def validate_domain(field): if not field.domain: return @@ -2462,15 +2489,38 @@ class BrowseList(list): self._ids.extend(map(int, list.__iter__(self))) -class EvalEnvironment(dict): - __slots__ = ('_record', '_model') +def is_pyson(test): + if isinstance(test, PYSON): + return True + if isinstance(test, (list, tuple)): + for i in test: + if isinstance(i, PYSON): + return True + if isinstance(i, (list, tuple)): + if is_pyson(i): + return True + if isinstance(test, dict): + for key, value in list(test.items()): + if isinstance(value, PYSON): + return True + if isinstance(value, (list, tuple, dict)): + if is_pyson(value): + return True + return False - def __init__(self, record, Model): + +class EvalEnvironment(dict): + __slots__ = ('_record', '_model', '_override') + + def __init__(self, record, Model, override=None): super().__init__() self._record = record self._model = Model + self._override = dict(override) if override is not None else {} def __getitem__(self, item): + if item in self._override: + return self._override[item] if item.startswith('_parent_'): field = item[8:] model_name = self._model._fields[field].model_name @@ -2506,13 +2556,13 @@ class EvalEnvironment(dict): return bool(self._record) -def _record_eval_pyson(record, source, encoded=False): +def _record_eval_pyson(record, source, encoded=False, override=None): transaction = Transaction() if not encoded: pyson = _pyson_encoder.encode(source) else: pyson = source - env = EvalEnvironment(record, record.__class__) + env = EvalEnvironment(record, record.__class__, override=override) env['context'] = transaction.context env['active_model'] = record.__class__.__name__ env['active_id'] = record.id diff --git a/trytond/trytond/tests/access.py b/trytond/trytond/tests/access.py index 247917d5ec..2ee305b1c6 100644 --- a/trytond/trytond/tests/access.py +++ b/trytond/trytond/tests/access.py @@ -2,6 +2,7 @@ # this repository contains the full copyright notices and license terms. from trytond.model import ModelSQL, fields from trytond.pool import Pool +from trytond.pyson import Eval class TestAccess(ModelSQL): @@ -14,6 +15,17 @@ class TestAccess(ModelSQL): ('test.access.relate', "Reference"), ]) dict_ = fields.Dict(None, "Dict") + field_readonly = fields.Char("Field Readonly", readonly=True) + field_readonly_state = fields.Char( + "Field Readonly State", + states={ + 'readonly': Eval('field1') == 'readonly', + }) + field_readonly_id = fields.Char( + "Field Readonly ID", + states={ + 'readonly': Eval('id', -1) >= 0, + }) class TestAccessRelate(ModelSQL): diff --git a/trytond/trytond/tests/test_access.py b/trytond/trytond/tests/test_access.py index ebab6d9640..f14b03d038 100644 --- a/trytond/trytond/tests/test_access.py +++ b/trytond/trytond/tests/test_access.py @@ -12,6 +12,80 @@ from trytond.transaction import Transaction _context = {'_check_access': True} +class ReadonlyTestCase(DBTestCase): + module = 'tests' + + @with_transaction(context=_context) + def test_create_readonly(self): + "Test create readonly field" + pool = Pool() + Model = pool.get('test.access') + + with self.assertRaises(AccessError): + Model.create([{'field_readonly': "test"}]) + + @with_transaction(context=_context) + def test_write_readonly(self): + pool = Pool() + Model = pool.get('test.access') + + record, = Model.create([{}]) + + with self.assertRaises(AccessError): + Model.write([record], {'field_readonly': "test"}) + + @with_transaction(context=_context) + def test_create_readonly_state(self): + "Test create field with readonly states" + pool = Pool() + Model = pool.get('test.access') + + Model.create([{ + 'field1': 'not readonly', + 'field_readonly_state': 'test', + }]) + Model.create([{ + 'field1': 'readonly', + }]) + with self.assertRaises(AccessError): + Model.create([{ + 'field1': 'readonly', + 'field_readonly_state': 'test', + }]) + + @with_transaction(context=_context) + def test_write_readonly_state(self): + "Test write field with readonly states" + pool = Pool() + Model = pool.get('test.access') + + record, = Model.create([{}]) + + Model.write([record], { + 'field1': 'not readonly', + 'field_readonly_state': 'foo', + }) + Model.write([record], { + 'field1': 'readonly', + }) + with self.assertRaises(AccessError): + Model.write([record], { + 'field_readonly_state': 'bar', + }) + + @with_transaction(context=_context) + def test_readonly_id(self): + "Test field with readonly based on id" + pool = Pool() + Model = pool.get('test.access') + + record, = Model.create([{'field_readonly_id': 'foo'}]) + with self.assertRaises(AccessError): + Model.write([record], { + 'field_readonly_id': 'bar', + }) + + class _ModelAccessTestCase(DBTestCase): module = 'tests' _perm = None @@ -261,7 +335,9 @@ class _ModelAccessTestCase(DBTestCase): ModelAccess = pool.get('ir.model.access') TestAccess = pool.get(self.model_name) - inactive_group, = Group.create([{'name': 'Test', 'active': False}]) + inactive_group, = Group.create([{'name': 'Test'}]) + inactive_group.active = False + inactive_group.save() record, = TestAccess.create([{}]) ModelAccess.create([{ 'model': self.model_name, @@ -287,9 +363,10 @@ class _ModelAccessTestCase(DBTestCase): inactive_group, = Group.create([{ 'name': 'Test', - 'active': False, 'users': [('add', [Transaction().user])], }]) + inactive_group.active = False + inactive_group.save() record, = TestAccess.create([{}]) ModelAccess.create([{ 'model': self.model_name, @@ -810,7 +887,9 @@ class _ModelFieldAccessTestCase(DBTestCase): FieldAccess = pool.get('ir.model.field.access') TestAccess = pool.get('test.access') - inactive_group, = Group.create([{'name': 'Test', 'active': False}]) + inactive_group, = Group.create([{'name': 'Test'}]) + inactive_group.active = False + inactive_group.save() record, = TestAccess.create([{}]) FieldAccess.create([{ 'model': 'test.access', @@ -839,9 +918,10 @@ class _ModelFieldAccessTestCase(DBTestCase): inactive_group, = Group.create([{ 'name': 'Test', - 'active': False, 'users': [('add', [Transaction().user])], }]) + inactive_group.active = False + inactive_group.save() record, = TestAccess.create([{}]) FieldAccess.create([{ 'model': 'test.access', diff --git a/trytond/trytond/tests/test_report.py b/trytond/trytond/tests/test_report.py index 60807fcd4a..d1cdc6e7be 100644 --- a/trytond/trytond/tests/test_report.py +++ b/trytond/trytond/tests/test_report.py @@ -52,7 +52,7 @@ class ReportTestCase(DBTestCase): Report.execute([], {}), ('txt', 'Administrator\n', False, 'Test Report')) - @with_transaction(context={'_check_access': True}) + @with_transaction() def test_execute_without_access(self): "Execute report without model access" with file_open('report.xml', subdir='tests') as xml: @@ -63,6 +63,7 @@ class ReportTestCase(DBTestCase): User = pool.get('res.user') Group = pool.get('res.group') Report = pool.get('test.test_report', type='report') + transaction = Transaction() user = User(login='foo') user.save() @@ -75,12 +76,13 @@ class ReportTestCase(DBTestCase): action_report.save() with self.assertRaises(AccessError): - with Transaction().set_user(user.id): + with (transaction.set_user(user.id), + transaction.set_context(_check_access=True)): Report.execute( [], {'model': 'test.access', 'action_id': action_report.id}) - @with_transaction(context={'_check_access': True}) + @with_transaction() def test_execute_without_model_access(self): "Execute report without model access" with file_open('report.xml', subdir='tests') as xml: @@ -90,6 +92,7 @@ class ReportTestCase(DBTestCase): Report = pool.get('test.test_report', type='report') ModelAccess = pool.get('ir.model.access') User = pool.get('res.user') + transaction = Transaction() user = User(login='foo') user.save() @@ -99,10 +102,11 @@ class ReportTestCase(DBTestCase): }]) with self.assertRaises(AccessError): - with Transaction().set_user(user.id): + with (transaction.set_user(user.id), + transaction.set_context(_check_access=True)): Report.execute([], {'model': 'test.access'}) - @with_transaction(context={'_check_access': True}) + @with_transaction() def test_execute_without_read_access(self): "Execute report without read access" with file_open('report.xml', subdir='tests') as xml: @@ -113,6 +117,7 @@ class ReportTestCase(DBTestCase): Report = pool.get('test.test_report', type='report') Model = pool.get('test.access') RuleGroup = pool.get('ir.rule.group') + transaction = Transaction() record, = Model.create([{'field1': 'foo'}]) rule_group, = RuleGroup.create([{ @@ -130,7 +135,8 @@ class ReportTestCase(DBTestCase): user.save() with self.assertRaises(AccessError): - with Transaction().set_user(user.id): + with (transaction.set_user(user.id), + transaction.set_context(_check_access=True)): Report.execute([record.id], {'model': 'test.access'}) @unittest.skipUnless(mrml, "required mrml") diff --git a/trytond/trytond/tests/test_resource.py b/trytond/trytond/tests/test_resource.py index 6ee045dc54..f4d13b0fec 100644 --- a/trytond/trytond/tests/test_resource.py +++ b/trytond/trytond/tests/test_resource.py @@ -104,7 +104,7 @@ class ResourceTestCase(DBTestCase): 'resource': record, 'message': "Foo", }]) - Note.write([note], {'message': "Bar"}) + Note.write([note], {'unread': False}) Note.read([note.id], ['message']) Note.delete([note]) @@ -178,7 +178,7 @@ class ResourceTestCase(DBTestCase): 'resource': record, 'message': "Foo", }]) - Note.write([note], {'message': "Bar"}) + Note.write([note], {'unread': False}) Note.read([note.id], ['message']) Note.delete([note])