diff --git a/openerp/addons/base/ir/ir.xml b/openerp/addons/base/ir/ir.xml index 8b39ac925b2..16630cbd40f 100644 --- a/openerp/addons/base/ir/ir.xml +++ b/openerp/addons/base/ir/ir.xml @@ -1667,24 +1667,29 @@ - + - + - - diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index 6da5846bf89..d3dfd987638 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -21,7 +21,6 @@ import logging import re import time -from operator import itemgetter from osv import fields,osv import netsvc @@ -450,6 +449,23 @@ class ir_model_access(osv.osv): # pass no groups -> no access return False + def group_names_with_access(self, cr, model_name, access_mode): + """Returns the names of visible groups which have been granted ``access_mode`` on + the model ``model_name``. + :rtype: list + """ + assert access_mode in ['read','write','create','unlink'], 'Invalid access mode: %s' % access_mode + cr.execute('''SELECT + 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) + WHERE + m.model=%s AND + a.perm_''' + access_mode, (model_name,)) + return [x[0] for x in cr.fetchall()] + def check(self, cr, uid, model, mode='read', raise_exception=True, context=None): if uid==1: # User root have all accesses @@ -493,16 +509,7 @@ class ir_model_access(osv.osv): r = cr.fetchone()[0] if not r and raise_exception: - cr.execute('''select - g.name - from - ir_model_access a - left join ir_model m on (a.model_id=m.id) - left join res_groups g on (a.group_id=g.id) - where - m.model=%s and - a.group_id is not null and perm_''' + mode, (model_name, )) - groups = ', '.join(map(lambda x: x[0], cr.fetchall())) or '/' + 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."), diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index befa47e25b0..2a36ee9ccc5 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -19,25 +19,41 @@ # ############################################################################## -from osv import fields,osv +from osv import fields, osv, expression import time from operator import itemgetter from functools import partial import tools from tools.safe_eval import safe_eval as eval +from tools.misc import unquote as unquote + +SUPERUSER_UID = 1 class ir_rule(osv.osv): _name = 'ir.rule' _order = 'name' _MODES = ['read', 'write', 'create', 'unlink'] + def _eval_context_for_combinations(self): + """Returns a dictionary to use as evaluation context for + ir.rule domains, when the goal is to obtain python lists + that are easier to parse and combine, but not to + actually execute them.""" + return {'user': unquote('user'), + 'time': unquote('time')} + + def _eval_context(self, cr, uid): + """Returns a dictionary to use as evaluation context for + ir.rule domains.""" + return {'user': self.pool.get('res.users').browse(cr, 1, uid), + 'time':time} + def _domain_force_get(self, cr, uid, ids, field_name, arg, context=None): res = {} + eval_context = self._eval_context(cr, uid) for rule in self.browse(cr, uid, ids, context): if rule.domain_force: - eval_user_data = {'user': self.pool.get('res.users').browse(cr, 1, uid), - 'time':time} - res[rule.id] = eval(rule.domain_force, eval_user_data) + res[rule.id] = expression.normalize(eval(rule.domain_force, eval_context)) else: res[rule.id] = [] return res @@ -83,25 +99,12 @@ class ir_rule(osv.osv): (_check_model_obj, 'Rules are not supported for osv_memory objects !', ['model_id']) ] - def domain_create(self, cr, uid, rule_ids): - count = 0 - dom = [] - for rule in self.browse(cr, uid, rule_ids): - if rule.domain: - dom += rule.domain - count += 1 - if count: - return ['&'] * (count-1) + dom - return [] - @tools.cache() def _compute_domain(self, cr, uid, model_name, mode="read"): if mode not in self._MODES: raise ValueError('Invalid mode: %r' % (mode,)) - group_rule = {} - global_rules = [] - if uid == 1: + if uid == SUPERUSER_UID: return None cr.execute("""SELECT r.id FROM ir_rule r @@ -111,26 +114,28 @@ class ir_rule(osv.osv): AND (r.id IN (SELECT rule_group_id FROM rule_group_rel g_rel JOIN res_groups_users_rel u_rel ON (g_rel.group_id = u_rel.gid) WHERE u_rel.uid = %s) OR r.global)""", (model_name, uid)) - ids = map(lambda x: x[0], cr.fetchall()) - if ids: - for rule in self.browse(cr, uid, ids): + rule_ids = [x[0] for x in cr.fetchall()] + if rule_ids: + # browse user as super-admin root to avoid access errors! + user = self.pool.get('res.users').browse(cr, SUPERUSER_UID, uid) + global_domains = [] # list of domains + group_domains = {} # map: group -> list of domains + for rule in self.browse(cr, SUPERUSER_UID, rule_ids): + # read 'domain' as UID to have the correct eval context for the rule. + rule_domain = self.read(cr, uid, rule.id, ['domain'])['domain'] + dom = expression.normalize(rule_domain) for group in rule.groups: - group_rule.setdefault(group.id, []).append(rule.id) + if group in user.groups_id: + group_domains.setdefault(group, []).append(dom) if not rule.groups: - global_rules.append(rule.id) - global_domain = self.domain_create(cr, uid, global_rules) - count = 0 - group_domains = [] - for value in group_rule.values(): - group_domain = self.domain_create(cr, uid, value) - if group_domain: - group_domains += group_domain - count += 1 - if count and global_domain: - return ['&'] + global_domain + ['|'] * (count-1) + group_domains - if count: - return ['|'] * (count-1) + group_domains - return global_domain + global_domains.append(dom) + # combine global domains and group domains + if group_domains: + group_domain = expression.OR(map(expression.OR, group_domains.values())) + else: + group_domain = [] + domain = expression.AND(global_domains + [group_domain]) + return domain return [] def clear_cache(self, cr, uid): diff --git a/openerp/addons/base/ir/ir_ui_view.py b/openerp/addons/base/ir/ir_ui_view.py index ebbfe7cea38..ee23a2f52f1 100644 --- a/openerp/addons/base/ir/ir_ui_view.py +++ b/openerp/addons/base/ir/ir_ui_view.py @@ -24,7 +24,6 @@ from lxml import etree from tools import graph from tools.safe_eval import safe_eval as eval import tools -import netsvc import os import logging diff --git a/openerp/addons/base/res/res_user.py b/openerp/addons/base/res/res_user.py index 8a76ba9a6f4..50d9c526957 100644 --- a/openerp/addons/base/res/res_user.py +++ b/openerp/addons/base/res/res_user.py @@ -581,9 +581,9 @@ class groups2(osv.osv): ##FIXME: Is there a reason to inherit this object ? if group_users: user_names = [user.name for user in self.pool.get('res.users').browse(cr, uid, group_users, context=context)] + user_names = list(set(user_names)) if len(user_names) >= 5: - user_names = user_names[:5] - user_names += '...' + user_names = user_names[:5] + ['...'] raise osv.except_osv(_('Warning !'), _('Group(s) cannot be deleted, because some user(s) still belong to them: %s !') % \ ', '.join(user_names)) diff --git a/openerp/addons/base/security/ir.model.access.csv b/openerp/addons/base/security/ir.model.access.csv index 158d6d98694..9ff2c6aa0be 100644 --- a/openerp/addons/base/security/ir.model.access.csv +++ b/openerp/addons/base/security/ir.model.access.csv @@ -61,7 +61,7 @@ "access_res_partner_bank_group_user","res_partner_bank group_user","model_res_partner_bank","group_user",1,0,0,0 "access_res_partner_bank_group_partner_manager","res_partner_bank group_partner_manager","model_res_partner_bank","group_partner_manager",1,1,1,1 "access_res_partner_bank_type_group_partner_manager","res_partner_bank_type group_partner_manager","model_res_partner_bank_type","group_partner_manager",1,1,1,1 -"access_res_partner_bank_type_group_user","res_partner_bank_type group_user","model_res_partner_bank_type","group_user",1,0,0,0 +"access_res_partner_bank_type_group_user","res_partner_bank_type group_user","model_res_partner_bank_type",,1,0,0,0 "access_res_partner_bank_type_field_group_partner_manager","res_partner_bank_type_field group_partner_manager","model_res_partner_bank_type_field","group_partner_manager",1,1,1,1 "access_res_partner_bank_type_field_group_user","res_partner_bank_type_field group_user","model_res_partner_bank_type_field",,1,0,0,0 "access_res_partner_canal_group_user","res_partner_canal group_user","model_res_partner_canal","group_user",1,0,0,0 diff --git a/openerp/addons/base/test/test_ir_rule.yml b/openerp/addons/base/test/test_ir_rule.yml index 9352f798508..49b368a5bb3 100644 --- a/openerp/addons/base/test/test_ir_rule.yml +++ b/openerp/addons/base/test/test_ir_rule.yml @@ -132,6 +132,48 @@ !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) assert ids, "Demo user should see some partner." +- + Modify the ir_rule for employee to have a rule that fordids + seeing any record. + We use a domain with implicit AND operator for later tests + on normalization. +- + !record {model: ir.rule, id: test_rule2}: + domain_force: "[('id','=',False),('name','=',False)]" +- + Check that demo user still sees partners, because group-rules are OR'ed. +- + !python {model: res.partner }: | + ids = self.search(cr, ref('base.user_demo'), []) + assert ids, "Demo user should see some partner." +- + Create a new group with demo user in it, and a complex rule. + Demo should still see partners. +- + !record {model: res.groups, id: test_group}: + name: Test Group + users: + - base.user_demo +- + Add the rule to the new group, with a domain containing an implicit AND operator, + which is more tricky because it will have to be normalized before combining it. +- + !record {model: ir.rule, id: test_rule3}: + model_id: base.model_res_partner + domain_force: "[('name', '!=', False),('id', '!=', False)]" + name: test_rule4 + groups: + - test_group + perm_unlink: 1 + perm_write: 1 + perm_read: 1 + perm_create: 1 +- + Read the partners again as demo user, which should give results. +- + !python {model: res.partner }: | + ids = self.search(cr, ref('base.user_demo'), []) + assert ids, "Demo user should see partners even with the combined rules." - Delete global domains (to combine only group domains). - @@ -140,7 +182,7 @@ assert ids, "Demo user should see some partner." self.unlink(cr, uid, ids) - - Read as demo user the partners (three 1=1 domains, no global domain). + Read as demo user the partners (several group domains, no global domain). - !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index c7065243174..3a5e41dacc7 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -177,4 +177,16 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Verify that normalize_domain() works. +- + !python {model: res.partner}: | + from osv import expression + norm_domain = domain = ['&',(1,'=',1),('a','=','b')] + assert norm_domain == expression.normalize(domain), "Normalized domains should be left untouched" + domain = [('x','in',['y','z']),('a.v','=','e'),'|','|',('a','=','b'),'!',('c','>','d'),('e','!=','f'),('g','=','h')] + norm_domain = ['&','&','&'] + domain + assert norm_domain == expression.normalize(domain), "Non-normalized domains should be properly normalized" + + diff --git a/openerp/import_xml.rng b/openerp/import_xml.rng index 75c4e8b05e7..d301ae47ed3 100644 --- a/openerp/import_xml.rng +++ b/openerp/import_xml.rng @@ -188,6 +188,16 @@ + + + + + + + + + + @@ -222,6 +232,7 @@ + diff --git a/openerp/netsvc.py b/openerp/netsvc.py index aca1b9c12a9..1d9e0a5fcc0 100644 --- a/openerp/netsvc.py +++ b/openerp/netsvc.py @@ -35,7 +35,6 @@ import socket import sys import threading import time -import warnings import types from pprint import pformat @@ -186,7 +185,6 @@ class ColoredFormatter(DBFormatter): return DBFormatter.format(self, record) def init_logger(): - import os from tools.translate import resetlocale resetlocale() diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index a36e741d7ff..c7fef6b5089 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -23,6 +23,72 @@ from openerp.tools import flatten, reverse_enumerate import fields +NOT_OPERATOR = '!' +OR_OPERATOR = '|' +AND_OPERATOR = '&' + +TRUE_DOMAIN = [(1,'=',1)] +FALSE_DOMAIN = [(0,'=',1)] + +def normalize(domain): + """Returns a normalized version of ``domain_expr``, where all implicit '&' operators + have been made explicit. One property of normalized domain expressions is that they + can be easily combined together as if they were single domain components. + """ + assert isinstance(domain, (list, tuple)), "Domains to normalize must have a 'domain' form: a list or tuple of domain components" + if not domain: + return TRUE_DOMAIN + result = [] + expected = 1 # expected number of expressions + op_arity = {NOT_OPERATOR: 1, AND_OPERATOR: 2, OR_OPERATOR: 2} + for token in domain: + if expected == 0: # more than expected, like in [A, B] + result[0:0] = ['&'] # put an extra '&' in front + expected = 1 + result.append(token) + if isinstance(token, (list,tuple)): # domain term + expected -= 1 + else: + expected += op_arity.get(token, 0) - 1 + assert expected == 0 + return result + +def combine(operator, unit, zero, domains): + """Returns a new domain expression where all domain components from ``domains`` + have been added together using the binary operator ``operator``. + + :param unit: the identity element of the domains "set" with regard to the operation + performed by ``operator``, i.e the domain component ``i`` which, when + combined with any domain ``x`` via ``operator``, yields ``x``. + E.g. [(1,'=',1)] is the typical unit for AND_OPERATOR: adding it + to any domain component gives the same domain. + :param zero: the absorbing element of the domains "set" with regard to the operation + performed by ``operator``, i.e the domain component ``z`` which, when + combined with any domain ``x`` via ``operator``, yields ``z``. + E.g. [(1,'=',1)] is the typical zero for OR_OPERATOR: as soon as + you see it in a domain component the resulting domain is the zero. + """ + result = [] + count = 0 + for domain in domains: + if domain == unit: + continue + if domain == zero: + return zero + if domain: + result += domain + count += 1 + result = [operator] * (count - 1) + result + return result + +def AND(domains): + """ AND([D1,D2,...]) returns a domain representing D1 and D2 and ... """ + return combine(AND_OPERATOR, TRUE_DOMAIN, FALSE_DOMAIN, domains) + +def OR(domains): + """ OR([D1,D2,...]) returns a domain representing D1 or D2 or ... """ + return combine(OR_OPERATOR, FALSE_DOMAIN, TRUE_DOMAIN, domains) + class expression(object): """ @@ -32,10 +98,12 @@ class expression(object): For more info: http://christophe-simonis-at-tiny.blogspot.com/2008/08/new-new-domain-notation.html """ - def _is_operator(self, element): - return isinstance(element, (str, unicode)) and element in ['&', '|', '!'] + @classmethod + def _is_operator(cls, element): + return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] - def _is_leaf(self, element, internal=False): + @classmethod + def _is_leaf(cls, element, internal=False): OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') INTERNAL_OPS = OPS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ @@ -92,8 +160,8 @@ class expression(object): doms = [] for o in table.browse(cr, uid, ids, context=context): if doms: - doms.insert(0, '|') - doms += ['&', ('parent_left', '<', o.parent_right), ('parent_left', '>=', o.parent_left)] + doms.insert(0, OR_OPERATOR) + doms += [AND_OPERATOR, ('parent_left', '<', o.parent_right), ('parent_left', '>=', o.parent_left)] if prefix: return [(left, 'in', table.search(cr, uid, doms, context=context))] return doms @@ -187,7 +255,7 @@ class expression(object): else: # we assume that the expression is valid # we create a dummy leaf for forcing the parsing of the resulting expression - self.__exp[i] = '&' + self.__exp[i] = AND_OPERATOR self.__exp.insert(i + 1, self.__DUMMY_LEAF) for j, se in enumerate(subexp): self.__exp.insert(i + 2 + j, se) @@ -389,7 +457,6 @@ class expression(object): ] self.__exp[i] = ('id', 'inselect', (query1, query2)) - return self def __leaf_to_sql(self, leaf, table): @@ -493,10 +560,10 @@ class expression(object): params.insert(0, p) stack.append(q) else: - if e == '!': + if e == NOT_OPERATOR: stack.append('(NOT (%s))' % (stack.pop(),)) else: - ops = {'&': ' AND ', '|': ' OR '} + ops = {AND_OPERATOR: ' AND ', OR_OPERATOR: ' OR '} q1 = stack.pop() q2 = stack.pop() stack.append('(%s %s %s)' % (q1, ops[e], q2,)) diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index f55e4b9c027..e28f7b2b02d 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -936,6 +936,9 @@ class related(function): if self._type=='many2one': ids = filter(None, res.values()) if ids: + # name_get as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. ng = dict(obj.pool.get(self._obj).name_get(cr, 1, ids, context=context)) for r in res: if res[r]: @@ -1079,7 +1082,10 @@ class property(function): value = properties.get_by_record(cr, uid, prop, context=context) res[prop.res_id.id][prop.fields_id.name] = value or False if value and (prop.type == 'many2one'): - record_exists = obj.pool.get(value._name).exists(cr, uid, value.id) + # check existence as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. + record_exists = obj.pool.get(value._name).exists(cr, 1, value.id) if record_exists: replaces.setdefault(value._name, {}) replaces[value._name][value.id] = True @@ -1087,8 +1093,11 @@ class property(function): res[prop.res_id.id][prop.fields_id.name] = False for rep in replaces: - nids = obj.pool.get(rep).search(cr, uid, [('id','in',replaces[rep].keys())], context=context) - replaces[rep] = dict(obj.pool.get(rep).name_get(cr, uid, nids, context=context)) + # search+name_get as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. + nids = obj.pool.get(rep).search(cr, 1, [('id','in',replaces[rep].keys())], context=context) + replaces[rep] = dict(obj.pool.get(rep).name_get(cr, 1, nids, context=context)) for prop in prop_name: for id in ids: diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 4141176fc5b..a817f73310d 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -972,7 +972,7 @@ class orm_template(object): if field[len(prefix)]=='id': try: data_res_id = _get_id(model_name, line[i], current_module, 'id') - except ValueError, e: + except ValueError: pass xml_id = line[i] continue @@ -2069,8 +2069,9 @@ class orm_template(object): class orm_memory(orm_template): _protected = ['read', 'write', 'create', 'default_get', 'perm_read', 'unlink', 'fields_get', 'fields_view_get', 'search', 'name_get', 'distinct_field_get', 'name_search', 'copy', 'import_data', 'search_count', 'exists'] - _max_count = config.get('osv_memory_count_limit') - _max_hours = config.get('osv_memory_age_limit') + _inherit_fields = {} + _max_count = None + _max_hours = None _check_time = 20 @classmethod @@ -2082,6 +2083,8 @@ class orm_memory(orm_template): self.datas = {} self.next_id = 0 self.check_id = 0 + self._max_count = config.get('osv_memory_count_limit') + self._max_hours = config.get('osv_memory_age_limit') cr.execute('delete from wkf_instance where res_type=%s', (self._name,)) def _check_access(self, uid, object_id, mode): diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index d0e6b503142..77abfa572b4 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -45,7 +45,6 @@ import openerp.loglevels as loglevels import openerp.pooler as pooler from config import config from translate import _ -from yaml_import import convert_yaml_import # List of etree._Element subclasses that we choose to ignore when parsing XML. from misc import SKIPPED_ELEMENT_TYPES @@ -402,7 +401,7 @@ form: module.record_id""" % (xml_id,) self._remove_ir_values(cr, string, value, model) def _tag_url(self, cr, rec, data_node=None): - url = rec.get("string",'').encode('utf8') + url = rec.get("url",'').encode('utf8') target = rec.get("target",'').encode('utf8') name = rec.get("name",'').encode('utf8') xml_id = rec.get('id','').encode('utf8') @@ -654,6 +653,12 @@ form: module.record_id""" % (xml_id,) resw = cr.fetchone() if (not values.get('name', False)) and resw: values['name'] = resw[0] + elif a_type=='url': + a_id = self.id_get(cr, a_action) + cr.execute('select name from ir_act_url where id=%s', (int(a_id),)) + resw = cr.fetchone() + if (not values.get('name')) and resw: + values['name'] = resw[0] if rec.get('sequence'): values['sequence'] = int(rec.get('sequence')) if rec.get('icon'): diff --git a/openerp/tools/yaml_import.py b/openerp/tools/yaml_import.py index 1c6a54c63b0..00bb79f0499 100644 --- a/openerp/tools/yaml_import.py +++ b/openerp/tools/yaml_import.py @@ -177,7 +177,7 @@ class YamlInterpreter(object): try: _, id = self.pool.get('ir.model.data').get_object_reference(self.cr, self.uid, module, checked_xml_id) self.id_map[xml_id] = id - except ValueError, e: + except ValueError: raise ValueError("""%s not found when processing %s. This Yaml file appears to depend on missing data. This often happens for tests that belong to a module's test suite and depend on each other.""" % (checked_xml_id, self.filename)) @@ -488,7 +488,6 @@ class YamlInterpreter(object): if self.isnoupdate(function) and self.mode != 'init': return model = self.get_model(function.model) - context = self.get_context(function, self.eval_context) if function.eval: args = self.process_eval(function.eval) else: diff --git a/openerp/tools/yaml_tag.py b/openerp/tools/yaml_tag.py index 20aabf1c54c..ba61b4825ca 100644 --- a/openerp/tools/yaml_tag.py +++ b/openerp/tools/yaml_tag.py @@ -32,7 +32,7 @@ class Record(YamlTag): return '!record {model: %s, id: %s}:' % (str(self.model,), str(self.id,)) class Python(YamlTag): - def __init__(self, model, severity=logging.WARNING, name="", **kwargs): + def __init__(self, model, severity=logging.ERROR, name="", **kwargs): self.model= model self.severity = severity self.name = name