[Zope-dev] ENHANCED PATCH: Expanded "access" file

Phillip J. Eby pje@telecommunity.com
Sun, 23 Jul 2000 13:46:27 -0500


--=====================_964395987==_
Content-Type: text/plain; charset="us-ascii"

I've further enhanced yesterday's patch with the following additions:

* "Short-circuit evaluation" of local roles in User.getRolesInContext().
This speeds up security evaluation for complex DTML by stopping the local
role search as soon as one of the desired roles is found.  The change is
fully backward compatible with any other usage of getRolesInContext()
(although there *arent'* any other usages in the Zope core).  (Note: this
change also prevents infinite recursion of local roles lookup when the
local roles are provided by a ZPatterns attribute provider which is owned
by a user who has the necessary roles to compute local roles.)

* Encapsulation fix.  The current version of Zope directly accesses
__ac_local_roles__ when checking access rights, which negates any ability
for Zope objects to provide rule-based local roles without computing all
possible roles for all possible users.

* Added "get_local_roles_for_user()" method to
AccessControl.Role.RoleManager that works with a user object, rather than a
user name.  This is to allow objects to supply computed local roles using
attributes of the user as part of their decision process.  An object need
only implement get_local_roles_for_user(user,roles) to carry this out.

These additions, in conjunction with the ability to add more users to the
"access" file, will allow ZPatterns and LoginManager to do without
'unownedness' and other convoluted alterations of the Zope security model.
They will also make it possible to add local role plugins to ZPatterns.

This is somewhat more tested than yesterday's variant of the patch,
although I have not found any bugs in what I posted yesterday.  The post
itself was flawed, however; this time I'm attaching it as a file in the
hopes of preventing that happening again.

If there are no comments/questions/feedback on it, I'd like to go ahead and
submit it to the Collector for inclusion in Zope CVS; as it will make
continued development of ZPatterns and LoginManager much cleaner and in
full compliance with the 2.2 security model.

As I move further into the development of local role plugin support for
ZPatterns, I may have additional patches to suggest, as there are some
other encapsulation/interface issues with the "get_local_roles()" method as
currently used/implemented in Zope.  Most likely, there needs to be a
"get_users_with_local_role()" method for those uses, leaving
"get_local_roles()" to mean "get *editable* local roles".  Also,
ObjectManager still inspects __ac_local_roles__ rather than going through
an interface to set the initial owner role of an object.  Personally, I
think this should be done by one of the many other after-add type calls
such as manage_afterAdd or manage_fixupOwnershipAfterAdd, etc., but
backward compatibility for that would be tricky.

Patch follows.

--=====================_964395987==_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="ZopeUsers.patch"

Index: AccessControl/Role.py
===================================================================
RCS file: /cvs-repository/Zope2/lib/python/AccessControl/Role.py,v
retrieving revision 1.39
diff -u -r1.39 Role.py
--- Role.py	2000/06/20 01:59:40	1.39
+++ Role.py	2000/07/23 18:46:03
@@ -365,9 +365,12 @@
         keys.sort()
         return keys
 
-    def get_local_roles_for_userid(self, userid):
+    def get_local_roles_for_userid(self, userid, roles=()):
         dict=self.__ac_local_roles__ or {}
         return dict.get(userid, [])
+
+    def get_local_roles_for_user(self, user, roles=()):
+        return self.get_local_roles_for_userid(user.getUserName(),roles)
 
     def manage_addLocalRoles(self, userid, roles, REQUEST=None):
         """Set local roles for a user."""
Index: AccessControl/User.py
===================================================================
RCS file: /cvs-repository/Zope2/lib/python/AccessControl/User.py,v
retrieving revision 1.112
diff -u -r1.112 User.py
--- User.py	2000/07/11 18:42:48	1.112
+++ User.py	2000/07/23 18:46:04
@@ -137,22 +137,27 @@
         """Return the list of roles assigned to a user."""
         raise NotImplemented
 
-    def getRolesInContext(self, object):
+    def getRolesInContext(self, object, findRoles=()):
         """Return the list of roles assigned to the user,
            including local roles assigned in context of
-           the passed in object."""
-        name=self.getUserName()
+           the passed in object.  If asked to find specific
+           roles, return true if any of the specified roles
+           is found, false otherwise.
+        """
+
         roles=self.getRoles()
+        for r in findRoles:
+            if r in roles: return roles
+
         local={}
         object=getattr(object, 'aq_inner', object)
+
         while 1:
-            if hasattr(object, '__ac_local_roles__'):
-                local_roles=object.__ac_local_roles__
-                if callable(local_roles):
-                    local_roles=local_roles()
-                dict=local_roles or {}
-                for r in dict.get(name, []):
+            if hasattr(object, 'get_local_roles_for_user'):
+                for r in object.get_local_roles_for_user(self):
                     local[r]=1
+                    if r in findRoles: return list(roles)+local.keys()
+
             if hasattr(object, 'aq_parent'):
                 object=object.aq_parent
                 continue
@@ -161,6 +166,9 @@
                 object=getattr(object, 'aq_inner', object)
                 continue
             break
+
+        if findRoles: return ()
+
         roles=list(roles) + local.keys()
         return roles
 
@@ -204,26 +212,26 @@
     def allowed(self, parent, roles=None):
         """Check whether the user has access to parent, assuming that
            parent.__roles__ is the given roles."""
+
         if roles is None or 'Anonymous' in roles:
             return 1
-        usr_roles=self.getRolesInContext(parent)
-        for role in roles:
-            if role in usr_roles:
-                if (hasattr(self,'aq_parent') and
-                    hasattr(self.aq_parent,'aq_parent')):
-                    if parent is None: return 1
-                    if (not hasattr(parent, 'aq_inContextOf') and
-                        hasattr(parent, 'im_self')):
-                        # This is a method, grab it's self.
-                        parent=parent.im_self
-                    if not parent.aq_inContextOf(self.aq_parent.aq_parent,1):
-                        if 'Shared' in roles:
-                            # Damn, old role setting. Waaa
-                            roles=self._shared_roles(parent)
-                            if 'Anonymous' in roles: return 1
-                        return None
-                return 1
 
+        if self.getRolesInContext(parent,roles):
+            if (hasattr(self,'aq_parent') and
+                hasattr(self.aq_parent,'aq_parent')):
+                if parent is None: return 1
+                if (not hasattr(parent, 'aq_inContextOf') and
+                    hasattr(parent, 'im_self')):
+                    # This is a method, grab it's self.
+                    parent=parent.im_self
+                if not parent.aq_inContextOf(self.aq_parent.aq_parent,1):
+                    if 'Shared' in roles:
+                        # Damn, old role setting. Waaa
+                        roles=self._shared_roles(parent)
+                        if 'Anonymous' in roles: return 1
+                    return None
+            return 1
+
         if 'Shared' in roles:
             # Damn, old role setting. Waaa
             roles=self._shared_roles(parent)
@@ -314,13 +322,30 @@
         'No access file found at %s - see INSTALL.txt' % INSTANCE_HOME
         )
 try:
-    data=split(strip(f.readline()),':')
-    f.close()
-    _remote_user_mode=not data[1]
-    try:    ds=split(data[2], ' ')
-    except: ds=[]
-    super=Super(data[0],data[1],('manage',), ds)
-    del data
+    d=f.readlines(); f.close(); del f
+    rootUsers = SpecialUsers.rootUsers = {}
+
+    for data in map(strip,d):
+        if not data or data[0]=='#': continue
+        data=split(data+':::',':')     # allow for missing fields
+
+        n = data[0]
+        ds = split(strip(data[2])) # space-delimited domains
+        pw = data[1]
+        r  = split(strip(data[3])) # space-delimited roles
+
+        if rootUsers or r:
+            # If not first user in file, or if roles are specified,
+            # user is a "normal" user object
+            rootUsers[n] = SimpleUser(n,data[1],tuple(split(data[3])),data[2])
+        else:
+            super = rootUsers[n] = Super(n,pw,('manage',), ds)
+            _remote_user_mode=not pw
+
+        del data,n,ds,pw,r
+
+    del d
+
 except:
     raise 'InstallError', 'Invalid format for access file - see INSTALL.txt'
 
@@ -383,6 +408,11 @@
         """
         try: return self.getUser(id)
         except:
+
+           # Don't return the superuser, so that super can't own things
+           if rootUsers.has_key(id) and id!=super.getUserName():
+               return rootUsers[id].__of__(self)
+
            if default is _marker: raise
            return default
 
@@ -405,7 +435,6 @@
 
 
     _remote_user_mode=_remote_user_mode
-    _super=super
     _nobody=nobody
             
     def validate(self,request,auth='',roles=None):
@@ -440,11 +469,11 @@
             return None
         name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1))
 
-        # Check for superuser
-        super=self._super
-        if self._isTop() and (name==super.getUserName()) and \
-        super.authenticate(password, request):
-            return super
+        # Check for superuser/root users
+        if self._isTop() and rootUsers.has_key(name):
+            user = rootUsers[name].__of__(self)
+            if user.authenticate(password, request):
+                return user
 
         # Try to get user
         user=self.getUser(name)
@@ -508,10 +537,9 @@
                     return ob
                 return None
 
-            # Check for superuser
-            super=self._super
-            if self._isTop() and (name==super.getUserName()):
-                return super
+            # Check for superuser/root users
+            if self._isTop() and rootUsers.has_key(name):
+                return rootUsers[name].__of__(self)
 
             # Try to get user
             user=self.getUser(name)
@@ -560,7 +588,7 @@
                    title  ='Illegal value', 
                    message='Password and confirmation must be specified',
                    action ='manage_main')
-        if self.getUser(name) or (name==self._super.getUserName()):
+        if self.getUser(name) or (rootUsers.has_key(name)):
             return MessageDialog(
                    title  ='Illegal value', 
                    message='A user with the specified name already exists',

--=====================_964395987==_--