From 815fc8f84aa0e03b7d55551061a0708fcddb639d Mon Sep 17 00:00:00 2001 From: Christophe Simonis Date: Mon, 19 Aug 2013 18:46:22 +0200 Subject: [PATCH 1/2] [FIX] browse records do not try to prefetch fields user can't read bzr revid: chs@openerp.com-20130819164622-7cre7yqpvlyzsslj --- openerp/osv/orm.py | 10 ++++++++-- openerp/tests/test_acl.py | 28 +++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 499a21f8555..5e19ebc6aed 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -383,6 +383,7 @@ class browse_record(object): raise KeyError(error_msg) # if the field is a classic one or a many2one, we'll fetch all classic and many2one fields + fields_to_fetch = [] if col._prefetch: # gen the list of "local" (ie not inherited) fields which are classic or many2one fields_to_fetch = filter(lambda x: x[1]._classic_write and x[1]._prefetch, self._table._columns.items()) @@ -390,9 +391,14 @@ class browse_record(object): inherits = map(lambda x: (x[0], x[1][2]), self._table._inherit_fields.items()) # complete the field list with the inherited fields which are classic or many2one fields_to_fetch += filter(lambda x: x[1]._classic_write and x[1]._prefetch, inherits) - # otherwise we fetch only that field - else: + + # filter out non accessible fields + accessible_fields = self._table.check_field_access_rights(self._cr, self._uid, 'read', fields=None, context=self._context) + fields_to_fetch = [f for f in fields_to_fetch if f[0] in accessible_fields] + + if not fields_to_fetch: fields_to_fetch = [(name, col)] + ids = filter(lambda id: name not in self._data[id], self._data.keys()) # read the results field_names = map(lambda x: x[0], fields_to_fetch) diff --git a/openerp/tests/test_acl.py b/openerp/tests/test_acl.py index 9f9019e5db9..b3a7f123888 100644 --- a/openerp/tests/test_acl.py +++ b/openerp/tests/test_acl.py @@ -9,6 +9,7 @@ import common # test group that demo user should not have GROUP_TECHNICAL_FEATURES = 'base.group_no_one' + class TestACL(common.TransactionCase): def setUp(self): @@ -22,7 +23,7 @@ class TestACL(common.TransactionCase): def test_field_visibility_restriction(self): """Check that model-level ``groups`` parameter effectively restricts access to that - field for users who do not belong to one of the explicitly allowed groups""" + field for users who do not belong to one of the explicitly allowed groups""" # Verify the test environment first original_fields = self.res_currency.fields_get(self.cr, self.demo_uid, []) form_view = self.res_currency.fields_view_get(self.cr, self.demo_uid, False, 'form') @@ -40,7 +41,7 @@ class TestACL(common.TransactionCase): view_arch = etree.fromstring(form_view.get('arch')) self.assertFalse('accuracy' in fields, "'accuracy' field should be gone") self.assertEquals(view_arch.xpath("//field[@name='accuracy']"), [], - "Field 'accuracy' must not be found in view definition") + "Field 'accuracy' must not be found in view definition") # Make demo user a member of the restricted group and check that the field is back self.tech_group.write({'users': [(4, self.demo_uid)]}) @@ -65,7 +66,7 @@ class TestACL(common.TransactionCase): has_tech_feat = self.res_users.has_group(self.cr, self.demo_uid, GROUP_TECHNICAL_FEATURES) self.assertFalse(has_tech_feat, "`demo` user should not belong to the restricted group") self.assert_(self.res_partner.read(self.cr, self.demo_uid, [1], ['bank_ids'])) - self.assert_(self.res_partner.write(self.cr, self.demo_uid, [1], {'bank_ids': []})) + self.assert_(self.res_partner.write(self.cr, self.demo_uid, [1], {'bank_ids': []})) # Now restrict access to the field and check it's forbidden self.res_partner._columns['bank_ids'].groups = GROUP_TECHNICAL_FEATURES @@ -79,12 +80,29 @@ class TestACL(common.TransactionCase): has_tech_feat = self.res_users.has_group(self.cr, self.demo_uid, GROUP_TECHNICAL_FEATURES) self.assertTrue(has_tech_feat, "`demo` user should now belong to the restricted group") self.assert_(self.res_partner.read(self.cr, self.demo_uid, [1], ['bank_ids'])) - self.assert_(self.res_partner.write(self.cr, self.demo_uid, [1], {'bank_ids': []})) - + self.assert_(self.res_partner.write(self.cr, self.demo_uid, [1], {'bank_ids': []})) + #cleanup self.tech_group.write({'users': [(3, self.demo_uid)]}) self.res_partner._columns['bank_ids'].groups = False + def test_fields_browse_restriction(self): + """Test access to records having restricted fields""" + self.res_partner._columns['email'].groups = GROUP_TECHNICAL_FEATURES + try: + P = self.res_partner + pid = P.search(self.cr, self.demo_uid, [], limit=1)[0] + part = P.browse(self.cr, self.demo_uid, pid) + # accessing fields must no raise exceptions... + part.name + # ... except they are restricted + with self.assertRaises(AttributeError): + part.email + + finally: + self.res_partner._columns['email'].groups = False + + if __name__ == '__main__': unittest2.main() From e78a83ac0306a3401e4ce08d0ccecc00aabbeca2 Mon Sep 17 00:00:00 2001 From: Christophe Simonis Date: Mon, 19 Aug 2013 19:49:30 +0200 Subject: [PATCH 2/2] [FIX] browse records do not prefetch fields with groups bzr revid: chs@openerp.com-20130819174930-xjzmrhuuuuwnbdg0 --- openerp/osv/orm.py | 16 ++++++---------- openerp/tests/test_acl.py | 3 ++- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 5e19ebc6aed..9ee5d5416d5 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -383,20 +383,16 @@ class browse_record(object): raise KeyError(error_msg) # if the field is a classic one or a many2one, we'll fetch all classic and many2one fields - fields_to_fetch = [] - if col._prefetch: + if col._prefetch and not col.groups: # gen the list of "local" (ie not inherited) fields which are classic or many2one - fields_to_fetch = filter(lambda x: x[1]._classic_write and x[1]._prefetch, self._table._columns.items()) + field_filter = lambda x: x[1]._classic_write and x[1]._prefetch and not x[1].groups + fields_to_fetch = filter(field_filter, self._table._columns.items()) # gen the list of inherited fields inherits = map(lambda x: (x[0], x[1][2]), self._table._inherit_fields.items()) # complete the field list with the inherited fields which are classic or many2one - fields_to_fetch += filter(lambda x: x[1]._classic_write and x[1]._prefetch, inherits) - - # filter out non accessible fields - accessible_fields = self._table.check_field_access_rights(self._cr, self._uid, 'read', fields=None, context=self._context) - fields_to_fetch = [f for f in fields_to_fetch if f[0] in accessible_fields] - - if not fields_to_fetch: + fields_to_fetch += filter(field_filter, inherits) + # otherwise we fetch only that field + else: fields_to_fetch = [(name, col)] ids = filter(lambda id: name not in self._data[id], self._data.keys()) diff --git a/openerp/tests/test_acl.py b/openerp/tests/test_acl.py index b3a7f123888..e80390cd5a2 100644 --- a/openerp/tests/test_acl.py +++ b/openerp/tests/test_acl.py @@ -96,9 +96,10 @@ class TestACL(common.TransactionCase): # accessing fields must no raise exceptions... part.name # ... except they are restricted - with self.assertRaises(AttributeError): + with self.assertRaises(openerp.osv.orm.except_orm) as cm: part.email + self.assertEqual(cm.exception.args[0], 'Access Denied') finally: self.res_partner._columns['email'].groups = False