Skip to content

Commit

Permalink
[FIX] base: correctly check debug mode when formatting access errors
Browse files Browse the repository at this point in the history
Intent of checking `group_no_one` was always to query the advanced
info / debug mode, however when the semantics of group_no_one got
changed in 31518bc this site was
missed, and now always displays "advanced" errors for internal
users. Which was not the intent.

Also since we're printing `display_name` and some of them annoyingly
hook onto context variables to show extended information, reset the
context to the user's default in order to avoid such
extended-formatting `display_name`.

Also fix "debug mode" in `TestIRRuleFeedback`, which has been broken
since time immemorial (likely as long as the group_no_one semantics
changed).

closes odoo#135940

X-original-commit: 9d3ffa6
Signed-off-by: Xavier Morel (xmo) <[email protected]>
  • Loading branch information
xmo-odoo committed Sep 20, 2023
1 parent 5046fd0 commit 4fec630
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 20 deletions.
3 changes: 2 additions & 1 deletion odoo/addons/base/models/ir_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def write(self, vals):

def _make_access_error(self, operation, records):
_logger.info('Access Denied by record rules for operation: %s on record ids: %r, uid: %s, model: %s', operation, records.ids[:6], self._uid, records._name)
self = self.with_context(self.env.user.context_get())

model = records._name
description = self.env['ir.model']._get(model).name or model
Expand All @@ -202,7 +203,7 @@ def _make_access_error(self, operation, records):
operation_error = msg_heads[operation]
resolution_info = _("Contact your administrator to request access if necessary.")

if not self.env.user.has_group('base.group_no_one') or not self.env.user.has_group('base.group_user'):
if not self.user_has_groups('base.group_no_one') or not self.env.user.has_group('base.group_user'):
records.invalidate_recordset()
return AccessError(f"{operation_error}\n\n{resolution_info}")

Expand Down
40 changes: 21 additions & 19 deletions odoo/addons/test_access_rights/tests/test_feedback.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# -*- coding: utf-8 -*-
from unittest.mock import Mock

import odoo
from odoo import SUPERUSER_ID, Command
from odoo.exceptions import AccessError
from odoo.tests import common, TransactionCase
from odoo.tests import TransactionCase


class Feedback(TransactionCase):
Expand Down Expand Up @@ -158,11 +161,20 @@ class TestIRRuleFeedback(Feedback):
"""
def setUp(self):
super().setUp()
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self.model = self.env['ir.model'].search([('model', '=', 'test_access_right.some_obj')])
self.record = self.env['test_access_right.some_obj'].create({
'val': 0,
}).with_user(self.user)


def debug_mode(self):
odoo.http._request_stack.push(Mock(db=self.env.cr.dbname, env=self.env, debug=True))
self.addCleanup(odoo.http._request_stack.pop)
self.env.flush_all()
self.env.invalidate_all()


def _make_rule(self, name, domain, global_=False, attr='write'):
res = self.env['ir.rule'].create({
'name': name,
Expand All @@ -179,7 +191,6 @@ def _make_rule(self, name, domain, global_=False, attr='write'):

def test_local(self):
self._make_rule('rule 0', '[("val", "=", 42)]')
self.env.ref('base.group_no_one').write({'users': [Command.unlink(self.user.id)]})
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -188,9 +199,7 @@ def test_local(self):
Contact your administrator to request access if necessary.""")

# debug mode
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand Down Expand Up @@ -223,10 +232,9 @@ def test_local(self):
)

def test_locals(self):
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self._make_rule('rule 0', '[("val", "=", 42)]')
self._make_rule('rule 1', '[("val", "=", 78)]')
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -244,10 +252,9 @@ def test_locals(self):
)

def test_globals_all(self):
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
self._make_rule('rule 1', '[("val", "=", 78)]', global_=True)
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -268,10 +275,9 @@ def test_globals_any(self):
""" Global rules are AND-eded together, so when an access fails it
might be just one of the rules, and we want an exact listing
"""
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
self._make_rule('rule 1', '[(1, "=", 1)]', global_=True)
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -288,12 +294,11 @@ def test_globals_any(self):
)

def test_combination(self):
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self._make_rule('rule 0', '[("val", "=", 42)]', global_=True)
self._make_rule('rule 1', '[(1, "=", 1)]', global_=True)
self._make_rule('rule 2', '[(0, "=", 1)]')
self._make_rule('rule 3', '[("val", "=", 55)]')
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -316,10 +321,9 @@ def test_warn_company_no_access(self):
this might be a multi-company issue, but the user doesn't access to this company
then no information about the company is showed.
"""
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]")
self._make_rule('rule 1', '[("val", "=", 0)]', global_=True)
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
self.record.write({'val': 1})
self.assertEqual(
Expand All @@ -342,8 +346,6 @@ def test_warn_company_no_company_field(self):
this might be a multi-company issue, but the record doesn't have company_id field
then no information about the company is showed.
"""
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
ChildModel = self.env['test_access_right.child'].sudo()
self.env['ir.rule'].create({
'name': 'rule 0',
Expand All @@ -355,6 +357,7 @@ def test_warn_company_no_company_field(self):
self.record.sudo().company_id = self.env['res.company'].create({'name': 'Brosse Inc.'})
self.user.sudo().company_ids = [Command.link(self.record.company_id.id)]
child_record = ChildModel.create({'parent_id': self.record.id}).with_user(self.user)
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
_ = child_record.parent_id
self.assertEqual(
Expand All @@ -376,11 +379,10 @@ def test_warn_company_access(self):
""" because of prefetching, read() goes through a different codepath
to apply rules
"""
self.env.ref('base.group_no_one').write({'users': [Command.link(self.user.id)]})
self.env.ref('base.group_user').write({'users': [Command.link(self.user.id)]})
self.record.sudo().company_id = self.env['res.company'].create({'name': 'Brosse Inc.'})
self.user.sudo().company_ids = [Command.link(self.record.company_id.id)]
self._make_rule('rule 0', "[('company_id', '=', user.company_id.id)]", attr='read')
self.debug_mode()
with self.assertRaises(AccessError) as ctx:
_ = self.record.val
self.assertEqual(
Expand Down

0 comments on commit 4fec630

Please sign in to comment.