diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index 421342cd625..da5a1ec19fd 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -518,15 +518,16 @@ class ir_model_access(osv.osv): """ assert access_mode in ['read','write','create','unlink'], 'Invalid access mode: %s' % access_mode cr.execute('''SELECT - g.name + c.name, g.name FROM ir_model_access a JOIN ir_model m ON (a.model_id=m.id) JOIN res_groups g ON (a.group_id=g.id) + LEFT JOIN ir_module_category c ON (c.id=g.category_id) WHERE m.model=%s AND a.perm_''' + access_mode, (model_name,)) - return [x[0] for x in cr.fetchall()] + return [('%s/%s' % x) if x[0] else x[1] for x in cr.fetchall()] @tools.ormcache() def check(self, cr, uid, model, mode='read', raise_exception=True, context=None): @@ -570,15 +571,23 @@ class ir_model_access(osv.osv): r = cr.fetchone()[0] if not r and raise_exception: - groups = ', '.join(self.group_names_with_access(cr, model_name, mode)) or '/' - msgs = { - 'read': _("You can not read this document (%s) ! Be sure your user belongs to one of these groups: %s."), - 'write': _("You can not write in this document (%s) ! Be sure your user belongs to one of these groups: %s."), - 'create': _("You can not create this document (%s) ! Be sure your user belongs to one of these groups: %s."), - 'unlink': _("You can not delete this document (%s) ! Be sure your user belongs to one of these groups: %s."), + groups = '\n\t'.join('- %s' % g for g in self.group_names_with_access(cr, model_name, mode)) + msg_heads = { + # Messages are declared in extenso so they are properly exported in translation terms + 'read': _("Sorry, you are not allowed to access this document."), + 'write': _("Sorry, you are not allowed to modify this document."), + 'create': _("Sorry, you are not allowed to create this kind of document."), + 'unlink': _("Sorry, you are not allowed to delete this document."), } - - raise except_orm(_('AccessError'), msgs[mode] % (model_name, groups) ) + if groups: + msg_tail = _("Only users with the following access level are currently allowed to do that") + ":\n%s\n\n(" + _("Document model") + ": %s)" + msg_params = (groups, model_name) + else: + msg_tail = _("Please contact your system administrator if you think this is an error.") + "\n\n(" + _("Document model") + ": %s)" + msg_params = (model_name,) + _logger.warning('Access Denied by ACLs for operation: %s, uid: %s, model: %s', mode, uid, model_name) + msg = '%s %s' % (msg_heads[mode], msg_tail) + raise except_orm(_('Access Denied'), msg % msg_params) return r or False __cache_clearing_methods = [] diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 7d6947a6255..9be2a1bed12 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -3487,10 +3487,7 @@ class BaseModel(object): for sub_ids in cr.split_for_in_conditions(ids): if rule_clause: cr.execute(query, [tuple(sub_ids)] + rule_params) - if cr.rowcount != len(sub_ids): - raise except_orm(_('AccessError'), - _('Operation prohibited by access rules, or performed on an already deleted document (Operation: read, Document type: %s).') - % (self._description,)) + self._check_record_rules_result_count(cr, user, sub_ids, 'read', context=context) else: cr.execute(query, (tuple(sub_ids),)) res.extend(cr.dictfetchall()) @@ -3669,6 +3666,26 @@ class BaseModel(object): # mention the first one only to keep the error message readable raise except_orm('ConcurrencyException', _('A document was modified since you last viewed it (%s:%d)') % (self._description, res[0])) + def _check_record_rules_result_count(self, cr, uid, ids, operation, context=None): + """Verify that number of returned rows after applying record rules matches + the length of `ids`, and raise an appropriate exception if it does not. + """ + if cr.rowcount != len(ids): + # Attempt to distinguish record rule restriction vs deleted records, + # to provide a more specific error message + cr.execute('SELECT id FROM ' + self._table + ' WHERE id IN %s', (tuple(ids),)) + if cr.rowcount != len(ids): + if operation == 'unlink': + # no need to warn about deleting an already deleted record! + return + _logger.warning('Failed operation on deleted record(s): %s, uid: %s, model: %s', operation, uid, self._name) + raise except_orm(_('Missing document(s)'), + _('One of the documents you are trying to access has been deleted, please try again after refreshing.')) + _logger.warning('Access Denied by record rules for operation: %s, uid: %s, model: %s', operation, uid, self._name) + raise except_orm(_('Access Denied'), + _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') % \ + (self._description, operation)) + def check_access_rights(self, cr, uid, operation, raise_exception=True): # no context on purpose. """Verifies that the operation given by ``operation`` is allowed for the user according to the access rights.""" @@ -3700,7 +3717,7 @@ class BaseModel(object): if self.is_transient(): # Only one single implicit access rule for transient models: owner only! # This is ok to hardcode because we assert that TransientModels always - # have log_access enabled and this the create_uid column is always there. + # have log_access enabled so that the create_uid column is always there. # And even with _inherits, these fields are always present in the local # table too, so no need for JOINs. cr.execute("""SELECT distinct create_uid @@ -3708,9 +3725,8 @@ class BaseModel(object): WHERE id IN %%s""" % self._table, (tuple(ids),)) uids = [x[0] for x in cr.fetchall()] if len(uids) != 1 or uids[0] != uid: - raise except_orm(_('AccessError'), '%s access is ' - 'restricted to your own records for transient models ' - '(except for the super-user).' % operation.capitalize()) + raise except_orm(_('Access Denied'), + _('For this kind of document, you may only access records you created yourself.\n\n(Document type: %s)') % (self._description,)) else: where_clause, where_params, tables = self.pool.get('ir.rule').domain_get(cr, uid, self._name, operation, context=context) if where_clause: @@ -3719,10 +3735,7 @@ class BaseModel(object): cr.execute('SELECT ' + self._table + '.id FROM ' + ','.join(tables) + ' WHERE ' + self._table + '.id IN %s' + where_clause, [sub_ids] + where_params) - if cr.rowcount != len(sub_ids): - raise except_orm(_('AccessError'), - _('Operation prohibited by access rules, or performed on an already deleted document (Operation: %s, Document type: %s).') - % (operation, self._description)) + self._check_record_rules_result_count(cr, uid, sub_ids, operation, context=context) def _workflow_trigger(self, cr, uid, ids, trigger, context=None): """Call given workflow trigger as a result of a CRUD operation"""