From 6d8688bb124df311648f65f6d43608ee22d5f63b Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Mon, 13 Jan 2020 15:18:36 +0100 Subject: [PATCH] [IMP] base: use ir.model.access on transient models Before this commit models with the `_transient` flag were ignored in ir.model.access verifications. Only an implicit ir.rule with the domain (create_uid=user.id) was applied to avoid most side effects. The problem is that, often, the security does not lie in side-effects of abusing of somebody else's wizard record but in the fact that the wizard methods blindly trust only the right users are creating these records. Too often, too many sudo were used and creating wizard with chosen values could lead to an abuse scenario. Instead, explicitly require the developer to declare security rules the same way as on any other model. --- odoo/addons/base/models/ir_model.py | 4 +--- odoo/addons/base/models/ir_rule.py | 5 ----- odoo/models.py | 14 +------------- odoo/modules/loading.py | 9 +-------- 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 9c69d8057df..ae4ee63f335 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -1455,7 +1455,7 @@ class IrModelAccess(models.Model): name = fields.Char(required=True, index=True) active = fields.Boolean(default=True, help='If you uncheck the active field, it will disable the ACL without deleting it (if you delete a native ACL, it will be re-created when you reload the module).') - model_id = fields.Many2one('ir.model', string='Object', required=True, domain=[('transient', '=', False)], index=True, ondelete='cascade') + model_id = fields.Many2one('ir.model', string='Object', required=True, index=True, ondelete='cascade') group_id = fields.Many2one('res.groups', string='Group', ondelete='cascade', index=True) perm_read = fields.Boolean(string='Read Access') perm_write = fields.Boolean(string='Write Access') @@ -1533,8 +1533,6 @@ class IrModelAccess(models.Model): # TransientModel records have no access rights, only an implicit access rule if model not in self.env: _logger.error('Missing model %s', model) - elif self.env[model].is_transient(): - return True # We check if a specific rule exists self._cr.execute("""SELECT MAX(CASE WHEN perm_{mode} THEN 1 ELSE 0 END) diff --git a/odoo/addons/base/models/ir_rule.py b/odoo/addons/base/models/ir_rule.py index 7566a867384..e2703b36bd9 100644 --- a/odoo/addons/base/models/ir_rule.py +++ b/odoo/addons/base/models/ir_rule.py @@ -62,11 +62,6 @@ class IrRule(models.Model): for rule in self: rule['global'] = not rule.groups - @api.constrains('model_id') - def _check_model_transience(self): - if any(self.env[rule.model_id.model].is_transient() for rule in self): - raise ValidationError(_('Rules can not be applied on Transient models.')) - @api.constrains('model_id') def _check_model_name(self): # Don't allow rules on rules records (this model). diff --git a/odoo/models.py b/odoo/models.py index 77ba7c8eaa3..db725cdff98 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -557,7 +557,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})): if ModelClass._transient: assert ModelClass._log_access, \ "TransientModels must have log_access turned on, " \ - "in order to implement their access rights policy" + "in order to implement their vacuum policy" # link the class to the registry, and update the registry ModelClass.pool = pool @@ -3241,9 +3241,6 @@ Record ids: %(records)s forbidden = invalid.exists() if forbidden: # the invalid records are (partially) hidden by access rules - if self.is_transient(): - raise AccessError(_('For this kind of document, you may only access records you created yourself.\n\n(Document type: %s)') % (self._description,)) - raise self.env['ir.rule']._make_access_error(operation, forbidden) # If we get here, the invalid records are not in the database. @@ -4166,15 +4163,6 @@ Record ids: %(records)s if table not in query.tables: query.tables.append(table) - if self._transient: - # One single implicit access rule for transient models: owner only! - # This is ok because we assert that TransientModels always have - # log_access enabled, so that 'create_uid' is always there. - domain = [('create_uid', '=', self._uid)] - tquery = self._where_calc(domain, active_test=False) - apply_rule(tquery.where_clause, tquery.where_clause_params, tquery.tables) - return - # apply main rules on the object Rule = self.env['ir.rule'] where_clause, where_params, tables = Rule.domain_get(self._name, mode) diff --git a/odoo/modules/loading.py b/odoo/modules/loading.py index b5cde25115c..9b1efca7cc4 100644 --- a/odoo/modules/loading.py +++ b/odoo/modules/loading.py @@ -443,17 +443,10 @@ def load_modules(db, force_demo=False, status=None, update_module=False): env = api.Environment(cr, SUPERUSER_ID, {}) cr.execute("""select model,name from ir_model where id NOT IN (select distinct model_id from ir_model_access)""") for (model, name) in cr.fetchall(): - if model in registry and not registry[model]._abstract and not registry[model]._transient: + if model in registry and not registry[model]._abstract: _logger.warning('The model %s has no access rules, consider adding one. E.g. access_%s,access_%s,model_%s,base.group_user,1,0,0,0', model, model.replace('.', '_'), model.replace('.', '_'), model.replace('.', '_')) - # Temporary warning while we remove access rights on osv_memory objects, as they have - # been replaced by owner-only access rights - cr.execute("""select distinct mod.model, mod.name from ir_model_access acc, ir_model mod where acc.model_id = mod.id""") - for (model, name) in cr.fetchall(): - if model in registry and registry[model]._transient: - _logger.warning('The transient model %s (%s) should not have explicit access rules!', model, name) - cr.execute("SELECT model from ir_model") for (model,) in cr.fetchall(): if model in registry: