Base_getWorkflowHistoryItemList: Remove access override

This script has no reason to be an exception to user's title retrieval
permissions.
Non-ERP5 users do not have applicable access control, so handle those
cases specifically. Such users may be:
- since-deleted ERP5 users
- coming from acl_users/zodb_users
Use a generic placeholder when the user is not allowed to see a
transition's actor.

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testBase.py

@@ -1287,6 +1287,125 @@ class TestBase(ERP5TypeTestCase, ZopeTestCase.Functional):
                       person, 'invalidated', wf_id='edit_workflow')
     self.assertEqual(person.getValidationState(), 'draft')
 
+  def test_getWorkflowHistoryItemList(self):
+    """Base_getWorkflowHistoryItemList obeys user document security.
+    """
+    # Note: the tested code belongs to erp5_core, but test it as part of
+    # erp5_base in order to have the most relevant user type: one based
+    # on an ERP5 document (here, a Person).
+    test_user = getSecurityManager().getUser()
+    portal = self.portal
+    newPerson = portal.person_module.newContent
+
+    # Non-ERP5 users which will be actor in the workflow history
+    existing_non_erp5_user_id = self.username
+
+    # ERP5 users which will be actors in the workflow history
+    user_1_value = newPerson(
+      portal_type='Person',
+      first_name='Foo',
+      last_name='Foo'
+    )
+    user_1_title = user_1_value.getTitle()
+    user_1_user_id = user_1_value.getUserId()
+    user_2_value = newPerson(
+      portal_type='Person',
+      first_name='Bar',
+      last_name='Bar'
+    )
+    user_2_title = user_2_value.getTitle()
+    user_2_user_id = user_2_value.getUserId()
+    user_3_value = newPerson(
+      portal_type='Person',
+      first_name='Baz',
+      last_name='Baz'
+    )
+    user_3_user_id = user_3_value.getUserId()
+
+    # ERP5 users which will look at workflow history
+    user_4_value = newPerson(
+      portal_type='Person',
+    )
+    user_4_user_id = user_4_value.getUserId()
+    user_5_value = newPerson(
+      portal_type='Person',
+    )
+    user_5_user_id = user_5_value.getUserId()
+    user_1_value.manage_addLocalRoles(user_5_user_id, ['Assignee'])
+
+    # Document tests will be run on
+    document_value = newPerson(
+      portal_type='Person',
+    )
+    # Let all see it, otherwise the user cannot call
+    # Base_getWorkflowHistoryItemList on it to begin with (more accurately,
+    # access to the "context" binding raises).
+    document_value.manage_addLocalRoles(user_4_user_id, ['Assignee'])
+    document_value.manage_addLocalRoles(user_5_user_id, ['Assignee'])
+    self.tic()
+
+    getUserById = portal.acl_users.getUserById
+    user_1 = getUserById(user_1_user_id)
+    user_2 = getUserById(user_2_user_id)
+    user_3 = getUserById(user_3_user_id)
+    user_4 = getUserById(user_4_user_id)
+    user_5 = getUserById(user_5_user_id)
+    existing_non_erp5_user = getUserById(existing_non_erp5_user_id)
+    for user_id in (user_1_user_id, user_2_user_id, user_3_user_id, existing_non_erp5_user_id):
+      document_value.manage_addLocalRoles(user_id, ['Assignee'])
+    for user in (user_1, user_2, user_3, existing_non_erp5_user):
+      newSecurityManager(None, user)
+      document_value.edit()
+    newSecurityManager(None, test_user)
+    self.tic()
+    user_3_value.getParentValue().manage_delObjects(ids=[user_3_value.getId()])
+    del user_3_value, user_3
+    self.tic()
+
+    # Sanity checks
+    self.assertEqual(user_1.getUserValue(), user_1_value)
+    self.assertEqual(user_2.getUserValue(), user_2_value)
+    self.assertEqual(existing_non_erp5_user.getUserValue(), None)
+    self.assertEqual(getUserById(user_3_user_id), None)
+    for user, can_see_1, can_see_2, is_manager in (
+      # user_4 cannot see any of the users which acted on the test document
+      (user_4,    False, False, False),
+      # user_5 can see the first actor, but not the second
+      (user_5,    True,  False, False),
+      # test_user is a Manager, and hence can see all
+      (test_user, True,  True,  True),
+    ):
+      has_permission = user.has_permission
+      self.assertEqual(bool(has_permission('Access contents information', user_1_value)), can_see_1)
+      self.assertEqual(bool(has_permission('View', user_1_value)), can_see_1)
+      self.assertEqual(bool(has_permission('Access contents information', user_2_value)), can_see_2)
+      self.assertEqual(bool(has_permission('View', user_2_value)), can_see_2)
+      self.assertEqual(bool(user.has_role('Manager')), is_manager)
+    def assertActorHistoryEqual(expected):
+      self.assertEqual(
+        [
+          x.actor
+          for x in document_value.Base_getWorkflowHistoryItemList('edit_workflow')[1:]
+        ],
+        expected,
+      )
+
+    # The actual test starts here
+    placeholder_actor = 'Unknown'
+
+    # user_4 cannot see any actor, and only sees placeholder actor captions
+    newSecurityManager(None, user_4)
+    assertActorHistoryEqual([placeholder_actor, placeholder_actor, placeholder_actor, placeholder_actor])
+
+    # user_5 can only see user_1's caption
+    newSecurityManager(None, user_5)
+    assertActorHistoryEqual([user_1_title, placeholder_actor, placeholder_actor, placeholder_actor])
+
+    # test_user can see all actors: existing ERP5 users with their title,
+    # non-ERP5 users - including deleted ones - by their user id.
+    newSecurityManager(None, test_user)
+    assertActorHistoryEqual([user_1_title, user_2_title, user_3_user_id, existing_non_erp5_user_id])
+
 
 class TestERP5PropertyManager(unittest.TestCase):
   """Tests for ERP5PropertyManager.

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_getWorkflowHistoryItemList.py

+from AccessControl import getSecurityManager
 from Products.ERP5Type.Document import newTempBase
 from Products.ERP5Type.Utils import getTranslationStringWithContext
+from zExceptions import Unauthorized
 
-from AccessControl import getSecurityManager
-
-can_view_history = getSecurityManager().getUser().has_permission('View History', context)
+user = getSecurityManager().getUser()
+is_manager = user.has_role('Manager')
+allowed_zope_actor_set = ('System Processes', 'Anonymous User')
+placeholder_actor = 'Unknown'
+can_view_history = user.has_permission('View History', context)
 
 marker = []
 result = []
@@ -20,11 +24,25 @@ def getActorName(actor):
   try:
     return actor_name_cache[actor]
   except KeyError:
-    actor_name_cache[actor] = actor
+    # Show only actor names:
+    # - for existing ERP5 users: ...that the current security context can see
+    # - for non-ERP5 users (which includes deleted users, but this should be rare):
+    #   ...that are a few special users (which cannot be used to log in),
+    #   unless the current security context has the Manager role in which case show all these users
     person = portal_object.Base_getUserValueByUserId(actor)
-    if person is not None:
-      actor_name_cache[actor] = person.getTitle()
-    return actor_name_cache[actor]
+    if person is None:
+      actor_name = (
+        actor
+        if is_manager or actor in allowed_zope_actor_set else
+        placeholder_actor
+      )
+    else:
+      try:
+        actor_name = person.getTitle()
+      except Unauthorized:
+        actor_name = placeholder_actor
+    actor_name_cache[actor] = actor_name
+    return actor_name
 
 
 

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_getWorkflowHistoryItemList.xml

@@ -52,24 +52,6 @@
             <key> <string>_params</string> </key>
             <value> <string>workflow_id, display=1, **kw</string> </value>
         </item>
-        <item>
-            <key> <string>_proxy_roles</string> </key>
-            <value>
-              <tuple>
-                <string>Anonymous</string>
-                <string>Assignee</string>
-                <string>Assignor</string>
-                <string>Associate</string>
-                <string>Auditor</string>
-                <string>Authenticated</string>
-                <string>Author</string>
-                <string>Manager</string>
-                <string>Member</string>
-                <string>Owner</string>
-                <string>Reviewer</string>
-              </tuple>
-            </value>
-        </item>
         <item>
             <key> <string>id</string> </key>
             <value> <string>Base_getWorkflowHistoryItemList</string> </value>