[FIX] base: has_group behaviour under the new API
While the signature makes sense in an old API context (sort-of) it's very easy to misuse in a new API context as one'd expect `has_group` to use the subject (recordset), not the user in the environment, compounded by ``env.user`` being browsed as administrator, resulting in the "obvious" ``env.user.has_group(foo)`` checking if the administrator (rather than the current user) has the specified group. Adds a divergent v8 version of has_group which first checks if it's called on a non-empty recordset, and in that case use the recordset id, otherwise fallback on the context user (to handle new API conversions which used the "correct" call convention rather than the obvious one). fixes #9649
This commit is contained in:
parent
3ac46ee3d4
commit
ccb0f6db61
|
@ -31,6 +31,7 @@ import openerp
|
|||
from openerp import SUPERUSER_ID, models
|
||||
from openerp import tools
|
||||
import openerp.exceptions
|
||||
from openerp import api
|
||||
from openerp.osv import fields, osv, expression
|
||||
from openerp.service.security import check_super
|
||||
from openerp.tools.translate import _
|
||||
|
@ -548,8 +549,19 @@ class res_users(osv.osv):
|
|||
'target': 'new',
|
||||
}
|
||||
|
||||
@tools.ormcache(skiparg=2)
|
||||
@api.v7
|
||||
def has_group(self, cr, uid, group_ext_id):
|
||||
return self._has_group(cr, uid, group_ext_id)
|
||||
@api.v8
|
||||
def has_group(self, group_ext_id):
|
||||
# use singleton's id if called on a non-empty recordset, otherwise
|
||||
# context uid
|
||||
uid = self.id if self else self.env.uid
|
||||
return self._has_group(self.env.cr, uid, group_ext_id)
|
||||
|
||||
@api.noguess
|
||||
@tools.ormcache(skiparg=2)
|
||||
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.
|
||||
|
@ -564,6 +576,8 @@ class res_users(osv.osv):
|
|||
(SELECT res_id FROM ir_model_data WHERE module=%s AND name=%s)""",
|
||||
(uid, module, ext_id))
|
||||
return bool(cr.fetchone())
|
||||
# for a few places explicitly clearing the has_group cache
|
||||
has_group.clear_cache = _has_group.clear_cache
|
||||
|
||||
#----------------------------------------------------------
|
||||
# Implied groups
|
||||
|
|
|
@ -20,6 +20,7 @@ import test_res_lang
|
|||
import test_search
|
||||
import test_translate
|
||||
#import test_uninstall
|
||||
import test_user_has_group
|
||||
import test_view_validation
|
||||
import test_views
|
||||
import test_xmlrpc
|
||||
|
|
|
@ -0,0 +1,49 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
import openerp.tests
|
||||
|
||||
class TestHasGroup(openerp.tests.TransactionCase):
|
||||
def setUp(self):
|
||||
super(TestHasGroup, self).setUp()
|
||||
|
||||
group0 = self.env['ir.model.data']._update(
|
||||
'res.groups', 'test_user_has_group',
|
||||
{'name': 'group0'},
|
||||
xml_id='group0'
|
||||
)
|
||||
self.group0 = 'test_user_has_group.group0'
|
||||
self.env['ir.model.data']._update(
|
||||
'res.groups', 'test_user_has_group',
|
||||
{'name': 'group1'},
|
||||
xml_id='group1'
|
||||
)
|
||||
self.group1 = 'test_user_has_group.group1'
|
||||
|
||||
self.test_user = self.env['res.users'].create({
|
||||
'login': 'testuser',
|
||||
'partner_id': self.env['res.partner'].create({
|
||||
'name': "Strawman Test User"
|
||||
}).id,
|
||||
'groups_id': [(4, group0, 0)]
|
||||
})
|
||||
|
||||
def test_old_api(self):
|
||||
Users = self.registry['res.users']
|
||||
|
||||
self.assertTrue(
|
||||
Users.has_group(self.cr, self.test_user.id, self.group0),
|
||||
"the test user should belong to group0"
|
||||
)
|
||||
self.assertFalse(
|
||||
Users.has_group(self.cr, self.test_user.id, self.group1),
|
||||
"the test user should *not* belong to group1"
|
||||
)
|
||||
|
||||
def test_new_api(self):
|
||||
self.assertTrue(
|
||||
self.test_user.has_group(self.group0),
|
||||
"the test user should belong to group0",
|
||||
)
|
||||
self.assertFalse(
|
||||
self.test_user.has_group(self.group1),
|
||||
"the test user shoudl not belong to group1"
|
||||
)
|
Loading…
Reference in New Issue