[Zope-Coders] Changes to ObjectManager, Role, and User in Zope2

[Zachery Bir]

In trying to justify the changes I made (and then reverted, pending 
tests), I'd like to ask those here.

In OFS.ObjectManager._setObject:

	[... snip ...]
         if set_owner:
             object.manage_fixupOwnershipAfterAdd()

             # Try to give user the local role "Owner", but only if
             # no local roles have been set on the object yet.
             if hasattr(object, '__ac_local_roles__'):
                 if object.__ac_local_roles__ is None:
                     user=getSecurityManager().getUser()
                     if user is not None:
                         name=user.getUserName()
                         if name != 'Anonymous User':
                             object.manage_setLocalRoles(name, ['Owner'])

This goes against AccessControl.Role.manage_setLocalRoles' signature 
expectation of a userId:

     def manage_setLocalRoles(self, userid, roles, REQUEST=None):
         """Set local roles for a user."""
         if not roles:
             raise ValueError, 'One or more roles must be given!'
         dict=self.__ac_local_roles__ or {}
         dict[userid]=roles
         self.__ac_local_roles__=dict
         if REQUEST is not None:
             stat='Your changes have been saved.'
             return self.manage_listLocalRoles(self, REQUEST, stat=stat)

Especially, as that is how 
AccessControl.User.SimpleUser.getRolesInContext is intended to 
ascertain them:

     def getRolesInContext(self, object):
         """Return the list of roles assigned to the user,
            including local roles assigned in context of
            the passed in object."""
         userid=self.getId()
         roles=self.getRoles()
         local={}
         object=getattr(object, 'aq_inner', object)
         while 1:
             local_roles = getattr(object, '__ac_local_roles__', None)
             if local_roles:
                 if callable(local_roles):
                     local_roles=local_roles()
                 dict=local_roles or {}
                 for r in dict.get(userid, []):
                     local[r]=1
             inner = getattr(object, 'aq_inner', object)
             parent = getattr(inner, 'aq_parent', None)
             if parent is not None:
                 object = parent
                 continue
             if hasattr(object, 'im_self'):
                 object=object.im_self
                 object=getattr(object, 'aq_inner', object)
                 continue
             break
         roles=list(roles) + local.keys()
         return roles

So, as it is, while we except the Anonymous User from playing in 
setting the local role of Owner, we don't except the Emergency User 
from doing so, but in doing so, the Emergency User will never get back 
those roles assigned to it.

I propose the following changes to OFS.ObjectManager._setObject:

         if set_owner:
             object.manage_fixupOwnershipAfterAdd()

             # Try to give user the local role "Owner", but only if
             # no local roles have been set on the object yet.
             if hasattr(object, '__ac_local_roles__'):
                 if object.__ac_local_roles__ is None:
                     user=getSecurityManager().getUser()
                     if user is not None:
                         id=user.getId()
                         name=user.getUserName()
                         if name != 'Anonymous User':
                             object.manage_setLocalRoles(id, ['Owner'])

Again, preventing the Anonymous User from playing. This will then, 
however, put Anonymous User and Emergency User on the same level WRT 
AccessControl.User.SimpleUser.getRolesInContext (both would have access 
to the roles keyed to None).

So, an additional change could be implemented as such:

AccessControl.User:

class SpecialUser(SimpleUser):
     """Class for special users, like emergency user and nobody"""
     def getId(self): return self._id # instead of return 
self.getUserName()

[...]

info = readUserAccessFile('access')
if info:
     _remote_user_mode = info[3]
     emergency_user = UnrestrictedUser(
         info[0], info[1], ('manage',), info[2])
else:
     emergency_user = NullUnrestrictedUser()
emergency_user._id = '_emergency_user'

super = emergency_user  # Note: use of the 'super' name is deprecated.
del info


nobody=SpecialUser('Anonymous User','',('Anonymous',), [])
nobody._id = '_anonymous_user'
system=UnrestrictedUser('System Processes','',('manage',), [])
system._id = '_system_user'

These changes would ensure that the local_roles machinery WRT setting 
the Owner would work properly, and we'd have the foundation for 
disambiguation between usernames and userids

The only thing this might obscure is an intention to provide for 
emergency users and anonymous users to always share the same local 
roles (since both would be mapped to and accessed with None as the key)

Thoughts?

Zac