From 45d1676432aaf5f44be6ffa6eeb5630b09319151 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Mon, 17 Aug 2020 08:11:16 +0200
Subject: [PATCH] base: restrict changing a user id

while setting an initial user id should be allowed for any user which can
create a person, changing an already set user id can have security
implications, so we protect it with a more strict permission
---
 .../portal_components/document.erp5.Person.py         |  7 ++++++-
 .../portal_components/test.erp5.testPerson.py         | 11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.Person.py b/bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.Person.py
index 9c5a91769a..1c3750f757 100644
--- a/bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.Person.py
+++ b/bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.Person.py
@@ -36,6 +36,8 @@ from erp5.component.mixin.EncryptedPasswordMixin import EncryptedPasswordMixin
 from erp5.component.mixin.LoginAccountProviderMixin import LoginAccountProviderMixin
 from erp5.component.mixin.ERP5UserMixin import ERP5UserMixin
 from Products.DCWorkflow.DCWorkflow import ValidationFailed
+from Products.CMFCore.utils import _checkPermission
+from Products.CMFCore.exceptions import AccessControl_Unauthorized
 
 try:
   from Products import PluggableAuthService
@@ -216,12 +218,15 @@ class Person(Node, LoginAccountProviderMixin, EncryptedPasswordMixin, ERP5UserMi
       - we want to prevent duplicated user ids, but only when
         PAS _AND_ ERP5LoginUserManager are used
     """
-    if value != self.getUserId():
+    existing_user_id = self.getUserId()
+    if value != existing_user_id:
       if value:
         self.__checkUserIdAvailability(
           pas_plugin_class=ERP5LoginUserManager,
           user_id=value,
         )
+      if existing_user_id and not _checkPermission(Permissions.ManageUsers, self):
+        raise AccessControl_Unauthorized('setUserId')
       self._baseSetUserId(value)
 
   security.declareProtected(Permissions.ModifyPortalContent, 'initUserId')
diff --git a/bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testPerson.py b/bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testPerson.py
index 44dd9947e9..61438c9b47 100644
--- a/bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testPerson.py
+++ b/bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testPerson.py
@@ -258,6 +258,17 @@ class TestPerson(ERP5TypeTestCase):
     p.setPassword('secret')
     self.assertTrue(p.getPassword())
 
+  def testSetUserIdSecurity(self):
+    # Changing an already set user id needs "manage users" permissions,
+    # but setting an initial user id does not.
+    p = self._makeOne(user_id=None)
+    p.manage_permission(Permissions.ManageUsers, [], 0)
+    p.setUserId('initial_user_id')
+    self.tic()
+    self.assertRaises(Unauthorized, p.setUserId, 'something_else')
+    self.abort()
+    self.assertRaises(Unauthorized, p.edit, user_id='something_else')
+
   def testPasswordFormat(self):
     p = self._makeOne(id='person')
     p._setEncodedPassword('pass_A', format='A')
-- 
2.30.9