Enforce readonly on field when checking access

Closes #4207

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

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::

trytond/trytond/ir/message.xml

@@ -244,6 +244,12 @@ this repository contains the full copyright notices and license terms. -->
             <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_modification_readonly_field_error">
+            <field name="text">You are not allowed to modify the field "%(field)s" of "%(model)s".</field>
+        </record>
+        <record model="ir.message" id="msg_modification_readonly_field_record_error">
+            <field name="text">You are not allowed to modify the field "%(field)s" in record "%(record)s" of "%(model)s".</field>
+        </record>
         <record model="ir.message" id="msg_context_datetime">
             <field name="text">At date/time: %(datetime)s</field>
         </record>

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):

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

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):

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',

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")

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])