Commit 0eae0158 authored by Jérome Perrin's avatar Jérome Perrin

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
parent 3ee1dfb0
...@@ -36,6 +36,8 @@ from erp5.component.mixin.EncryptedPasswordMixin import EncryptedPasswordMixin ...@@ -36,6 +36,8 @@ from erp5.component.mixin.EncryptedPasswordMixin import EncryptedPasswordMixin
from erp5.component.mixin.LoginAccountProviderMixin import LoginAccountProviderMixin from erp5.component.mixin.LoginAccountProviderMixin import LoginAccountProviderMixin
from erp5.component.mixin.ERP5UserMixin import ERP5UserMixin from erp5.component.mixin.ERP5UserMixin import ERP5UserMixin
from Products.DCWorkflow.DCWorkflow import ValidationFailed from Products.DCWorkflow.DCWorkflow import ValidationFailed
from Products.CMFCore.utils import _checkPermission
from Products.CMFCore.exceptions import AccessControl_Unauthorized
try: try:
from Products import PluggableAuthService from Products import PluggableAuthService
...@@ -216,12 +218,15 @@ class Person(Node, LoginAccountProviderMixin, EncryptedPasswordMixin, ERP5UserMi ...@@ -216,12 +218,15 @@ class Person(Node, LoginAccountProviderMixin, EncryptedPasswordMixin, ERP5UserMi
- we want to prevent duplicated user ids, but only when - we want to prevent duplicated user ids, but only when
PAS _AND_ ERP5LoginUserManager are used PAS _AND_ ERP5LoginUserManager are used
""" """
if value != self.getUserId(): existing_user_id = self.getUserId()
if value != existing_user_id:
if value: if value:
self.__checkUserIdAvailability( self.__checkUserIdAvailability(
pas_plugin_class=ERP5LoginUserManager, pas_plugin_class=ERP5LoginUserManager,
user_id=value, user_id=value,
) )
if existing_user_id and not _checkPermission(Permissions.ManageUsers, self):
raise AccessControl_Unauthorized('setUserId')
self._baseSetUserId(value) self._baseSetUserId(value)
security.declareProtected(Permissions.ModifyPortalContent, 'initUserId') security.declareProtected(Permissions.ModifyPortalContent, 'initUserId')
......
...@@ -258,6 +258,17 @@ class TestPerson(ERP5TypeTestCase): ...@@ -258,6 +258,17 @@ class TestPerson(ERP5TypeTestCase):
p.setPassword('secret') p.setPassword('secret')
self.assertTrue(p.getPassword()) 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): def testPasswordFormat(self):
p = self._makeOne(id='person') p = self._makeOne(id='person')
p._setEncodedPassword('pass_A', format='A') p._setEncodedPassword('pass_A', format='A')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment