diff --git a/doc/api/field_level_acl.rst b/doc/api/field_level_acl.rst new file mode 100644 index 00000000000..53a304a1faf --- /dev/null +++ b/doc/api/field_level_acl.rst @@ -0,0 +1,45 @@ +Field-level access control +========================== + +.. versionadded:: 7.0 + +OpenERP now supports real access control at the field level, not just on the view side. +Previously it was already possible to set a ``groups`` attribute on a ```` element +(or in fact most view elements), but with cosmetics effects only: the element was made +invisible on the client side, while still perfectly available for read/write access at +the RPC level. + +As of OpenERP 7.0 the existing behavior is preserved on the view level, but a new ``groups`` +attribute is available on all model fields, introducing a model-level access control on +each field. The syntax is the same as for the view-level attribute:: + + _columns = { + 'secret_key': fields.char('Secret Key', groups="base.group_erp_manager,base.group_system") + } + +There is a major difference with the view-level ``groups`` attribute: restricting +the access at the model level really means that the field will be completely unavailable +for users who do not belong to the authorized groups: + +* Restricted fields will be **completely removed** from all related views, not just + hidden. This is important to keep in mind because it means the field value will not be + available at all on the client side, and thus unavailable e.g. for ``on_change`` calls. +* Restricted fields will not be returned as part of a call to + :meth:`~openerp.osv.orm.fields_get` or :meth:`~openerp.osv.orm.fields_view_get` + This is in order to avoid them appearing in the list of fields available for + advanced search filters, for example. This does not prevent getting the list of + a model's fields by querying ``ir.model.fields`` directly, which is fine. +* Any attempt to read or write directly the value of the restricted fields will result + in an ``AccessError`` exception. +* As a consequence of the previous item, restricted fields will not be available for + use within search filters (domains) or anything that would require read or write access. +* It is quite possible to set ``groups`` attributes for the same field both at the model + and view level, even with different values. Both will carry their effect, with the + model-level restriction taking precedence and removing the field completely in case of + restriction. + +.. note:: The tests related to this feature are in ``openerp/tests/test_acl.py``. + +.. warning:: At the time of writing the implementation of this feature is partial + and does not yet restrict read/write RPC access to the field. + The corresponding test is written already but currently disabled. \ No newline at end of file diff --git a/doc/api/startup.rst b/doc/api/startup.rst index 98e8f6ac3c3..4e0c51a6900 100644 --- a/doc/api/startup.rst +++ b/doc/api/startup.rst @@ -2,6 +2,8 @@ Start-up script --------------- +.. versionadded:: 6.1 + To run the OpenERP server, the conventional approach is to use the `openerp-server` script. It loads the :ref:`openerp library`, sets a few configuration variables corresponding to command-line arguments, and starts to @@ -11,10 +13,8 @@ Depending on your deployment needs, you can write such a start-up script very easily. We also recommend you take a look at an alternative tool called `openerp-command` that can, among other things, launch the server. -.. versionadded:: 6.1 - Yet another alternative is to use a WSGI-compatible HTTP server and let it call into one of the WSGI entry points of the server. -.. versionadded:: 6.1 + diff --git a/doc/api/user_img_specs.rst b/doc/api/user_img_specs.rst index 4ee97999aca..a915529accd 100644 --- a/doc/api/user_img_specs.rst +++ b/doc/api/user_img_specs.rst @@ -1,6 +1,8 @@ User avatar =========== +.. versionadded:: 7.0 + This revision adds an avatar for users. This replaces the use of gravatar to emulate avatars, used in views like the tasks kanban view. Two fields have been added to the res.users model: - avatar_big, a binary field holding the image. It is base-64 encoded, and PIL-supported. Images stored are resized to 540x450 px, to limitate the binary field size. - avatar, a function binary field holding an automatically resized version of the avatar_big field. It is also base-64 encoded, and PIL-supported. Dimensions of the resized avatar are 180x150. This field is used as an inteface to get and set the user avatar. diff --git a/doc/index.rst.inc b/doc/index.rst.inc index 98c01c8354e..6182f6251e9 100644 --- a/doc/index.rst.inc +++ b/doc/index.rst.inc @@ -7,8 +7,8 @@ OpenERP Server test-framework -New feature merges -++++++++++++++++++ +Changed in 7.0 +++++++++++++++ .. toctree:: :maxdepth: 1 @@ -16,3 +16,4 @@ New feature merges api/user_img_specs api/need_action_specs api/font_style + api/field_level_acl \ No newline at end of file diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index 3101d446707..f4a46e3eca9 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -540,6 +540,22 @@ class users(osv.osv): return self.write(cr, uid, uid, {'password': new_passwd}) raise osv.except_osv(_('Warning!'), _("Setting empty passwords is not allowed for security reasons!")) + def has_group(self, cr, uid, group_ext_id): + """Checks whether user belongs to given group. + + :param str group_ext_id: external ID (XML ID) of the group. + Must be provided in fully-qualified form (``module.ext_id``), as there + is no implicit module to use.. + :return: True if the current user is a member of the group with the + given external ID (XML ID), else False. + """ + assert group_ext_id and '.' in group_ext_id, "External ID must be fully qualified" + module, ext_id = group_ext_id.split('.') + cr.execute("""SELECT 1 FROM res_groups_users_rel WHERE uid=%s AND gid IN + (SELECT res_id FROM ir_model_data WHERE module=%s AND name=%s)""", + (uid, module, ext_id)) + return bool(cr.fetchone()) + users() diff --git a/openerp/addons/base/tests/test_ir_values.py b/openerp/addons/base/tests/test_ir_values.py index 8e2707cc4ad..b7ea0107776 100644 --- a/openerp/addons/base/tests/test_ir_values.py +++ b/openerp/addons/base/tests/test_ir_values.py @@ -93,3 +93,7 @@ class test_ir_values(common.TransactionCase): assert len(actions[0]) == 3, "Malformed action definition" assert actions[0][1] == 'Related Stuff', 'Bound action does not match definition' assert isinstance(actions[0][2], dict) and actions[0][2]['id'] == 14, 'Bound action does not match definition' + + +if __name__ == '__main__': + unittest2.main() \ No newline at end of file diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index 4364c0f985e..2a6d0d6e3eb 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -108,10 +108,11 @@ class _column(object): self.manual = manual self.selectable = True self.group_operator = args.get('group_operator', False) + self.groups = False # CSV list of ext IDs of groups that can access this field for a in args: if args[a]: setattr(self, a, args[a]) - + def restart(self): pass @@ -1502,7 +1503,7 @@ def field_to_dict(model, cr, user, field, context=None): res['related_columns'] = [col1, col2] res['third_table'] = table for arg in ('string', 'readonly', 'states', 'size', 'required', 'group_operator', - 'change_default', 'translate', 'help', 'select', 'selectable'): + 'change_default', 'translate', 'help', 'select', 'selectable', 'groups'): if getattr(field, arg): res[arg] = getattr(field, arg) for arg in ('digits', 'invisible', 'filters'): diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 6d5877e9bd5..e99296a15e1 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -1578,8 +1578,21 @@ class BaseModel(object): def view_header_get(self, cr, user, view_id=None, view_type='form', context=None): return False + def user_has_groups(self, cr, uid, groups, context=None): + """Return true if the user is at least member of one of the groups + in groups_str. Typically used to resolve ``groups`` attribute + in view and model definitions. + + :param str groups: comma-separated list of fully-qualified group + external IDs, e.g.: ``base.group_user,base.group_system`` + :return: True if the current user is a member of one of the + given groups + """ + return any([self.pool.get('res.users').has_group(cr, uid, group_ext_id) + for group_ext_id in groups.split(',')]) + def __view_look_dom(self, cr, user, node, view_id, in_tree_view, model_fields, context=None): - """ Return the description of the fields in the node. + """Return the description of the fields in the node. In a normal call to this method, node is a complete view architecture but it is actually possible to give some sub-node (this is used so @@ -1604,17 +1617,35 @@ class BaseModel(object): return s def check_group(node): - """ Set invisible to true if the user is not in the specified groups. """ + """Apply group restrictions, may be set at view level or model level:: + * at view level this means the element should be made invisible to + people who are not members + * at model level (exclusively for fields, obviously), this means + the field should be completely removed from the view, as it is + completely unavailable for non-members + + :return: True if field should be included in the result of fields_view_get + """ + if node.tag == 'field' and node.get('name') in self._all_columns: + column = self._all_columns[node.get('name')].column + if column.groups and not self.user_has_groups(cr, user, + groups=column.groups, + context=context): + node.getparent().remove(node) + fields.pop(node.get('name'), None) + # no point processing view-level ``groups`` anymore, return + return False if node.get('groups'): - groups = node.get('groups').split(',') - ir_model_access = self.pool.get('ir.model.access') - can_see = any(ir_model_access.check_groups(cr, user, group) for group in groups) + can_see = self.user_has_groups(cr, user, + groups=node.get('groups'), + context=context) if not can_see: node.set('invisible', '1') modifiers['invisible'] = True if 'attrs' in node.attrib: del(node.attrib['attrs']) #avoid making field visible later del(node.attrib['groups']) + return True if node.tag in ('field', 'node', 'arrow'): if node.get('object'): @@ -1699,7 +1730,9 @@ class BaseModel(object): if node.get(additional_field): fields[node.get(additional_field)] = {} - check_group(node) + if not check_group(node): + # node must be removed, no need to proceed further with its children + return fields # The view architeture overrides the python model. # Get the attrs before they are (possibly) deleted by check_group below @@ -3344,7 +3377,8 @@ class BaseModel(object): res.update(self.pool.get(parent).fields_get(cr, user, allfields, context)) for f, field in self._columns.iteritems(): - if allfields and f not in allfields: + if (allfields and f not in allfields) or \ + (field.groups and not self.user_has_groups(cr, user, groups=field.groups, context=context)): continue res[f] = fields.field_to_dict(self, cr, user, field, context=context) diff --git a/openerp/tests/test_acl.py b/openerp/tests/test_acl.py new file mode 100644 index 00000000000..a10a705092c --- /dev/null +++ b/openerp/tests/test_acl.py @@ -0,0 +1,89 @@ +import unittest2 +from lxml import etree + +import common + +# test group that demo user should not have +GROUP_TECHNICAL_FEATURES = 'base.group_no_one' + +class TestACL(common.TransactionCase): + + def setUp(self): + super(TestACL, self).setUp() + self.res_currency = self.registry('res.currency') + self.res_partner = self.registry('res.partner') + self.res_users = self.registry('res.users') + self.demo_uid = 3 + self.tech_group = self.registry('ir.model.data').get_object(self.cr, self.uid, + *(GROUP_TECHNICAL_FEATURES.split('.'))) + + 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""" + # 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') + view_arch = etree.fromstring(form_view.get('arch')) + 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 before the test") + self.assertTrue('rate' in original_fields, "'rate' field must be properly visible before the test") + self.assertNotEquals(view_arch.xpath("//field[@name='rate']"), [], + "Field 'rate' must be found in view definition before the test") + + # Restrict access to the field and check it's gone + self.res_currency._columns['rate'].groups = GROUP_TECHNICAL_FEATURES + 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') + view_arch = etree.fromstring(form_view.get('arch')) + self.assertFalse('rate' in fields, "'rate' field should be gone") + self.assertEquals(view_arch.xpath("//field[@name='rate']"), [], + "Field 'rate' 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)]}) + has_tech_feat = self.res_users.has_group(self.cr, self.demo_uid, GROUP_TECHNICAL_FEATURES) + 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') + view_arch = etree.fromstring(form_view.get('arch')) + #import pprint; pprint.pprint(fields); pprint.pprint(form_view) + self.assertTrue(has_tech_feat, "`demo` user should now belong to the restricted group") + self.assertTrue('rate' in fields, "'rate' field must be properly visible again") + self.assertNotEquals(view_arch.xpath("//field[@name='rate']"), [], + "Field 'rate' must be found in view definition again") + + #cleanup + self.tech_group.write({'users': [(3, self.demo_uid)]}) + self.res_currency._columns['rate'].groups = False + + def test_field_crud_restriction(self): + "Read/Write RPC access to restricted field should be forbidden" + # Verify the test environment first + 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': []})) + + # Now restrict access to the field and check it's forbidden + self.res_partner._columns['bank_ids'].groups = GROUP_TECHNICAL_FEATURES + # FIXME TODO: enable next tests when access rights checks per field are implemented + # from openerp.osv.orm import except_orm + # with self.assertRaises(except_orm): + # self.res_partner.read(self.cr, self.demo_uid, [1], ['bank_ids']) + # with self.assertRaises(except_orm): + # self.res_partner.write(self.cr, self.demo_uid, [1], {'bank_ids': []}) + + # Add the restricted group, and check that it works again + self.tech_group.write({'users': [(4, self.demo_uid)]}) + 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': []})) + + #cleanup + self.tech_group.write({'users': [(3, self.demo_uid)]}) + self.res_partner._columns['bank_ids'].groups = False + +if __name__ == '__main__': + unittest2.main() + +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: \ No newline at end of file