From ccb0f6db61c9b435856dab01cde5cc366f183dde Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Tue, 12 Jan 2016 14:40:33 +0100
Subject: [PATCH] [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
---
 openerp/addons/base/res/res_users.py          | 16 +++++-
 openerp/addons/base/tests/__init__.py         |  1 +
 .../addons/base/tests/test_user_has_group.py  | 49 +++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 openerp/addons/base/tests/test_user_has_group.py

diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py
index 43e5a34c02b..0a2d8339ac5 100644
--- a/openerp/addons/base/res/res_users.py
+++ b/openerp/addons/base/res/res_users.py
@@ -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
diff --git a/openerp/addons/base/tests/__init__.py b/openerp/addons/base/tests/__init__.py
index c597542d052..e6441d42aae 100644
--- a/openerp/addons/base/tests/__init__.py
+++ b/openerp/addons/base/tests/__init__.py
@@ -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
diff --git a/openerp/addons/base/tests/test_user_has_group.py b/openerp/addons/base/tests/test_user_has_group.py
new file mode 100644
index 00000000000..e13d04f5f6f
--- /dev/null
+++ b/openerp/addons/base/tests/test_user_has_group.py
@@ -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"
+        )