Check in noodling for Jim to play with.

lib/python/AccessControl/ImplPython.py

@@ -17,6 +17,9 @@ import os
 import string
 
 from Acquisition import aq_base
+from Acquisition import aq_parent
+from Acquisition import aq_inner
+from Acquisition import aq_acquire
 from ExtensionClass import Base
 from zLOG import LOG, PROBLEM
 
@@ -522,16 +525,29 @@ def guarded_getattr(inst, name, default=_marker):
     Raises Unauthorized if the attribute is found but the user is
     not allowed to access the attribute.
     """
-    if name[:1] != '_':
+    if name[:1] == '_':
+        raise Unauthorized, name
+
     # Try to get the attribute normally so that unusual
     # exceptions are caught early.
-        try: v = getattr(inst, name)
+    try:
+        v = getattr(inst, name)
     except AttributeError:
         if default is not _marker:
             return default
         raise
 
-        assertion = Containers(type(inst))
+    return _verify_attribute_access(inst, name, v)
+
+def _verify_attribute_access(inst, name, v):
+
+    try:
+        container = v.im_self
+    except AttributeError:
+        container = aq_parent(aq_inner(v)) or inst
+
+    assertion = Containers(type(container))
+
     if isinstance(assertion, dict):
         # We got a table that lets us reason about individual
         # attrs
@@ -545,15 +561,21 @@ def guarded_getattr(inst, name, default=_marker):
             return v
         raise Unauthorized, name
 
-        elif assertion:
-            # So the entry in the outer table is not a dict
-            # It's allowed to be a vetoing function:
+    if assertion:
         if callable(assertion):
-                assertion(name, v)
-            # No veto, so we can return
+            factory = assertion(name, v)
+            if callable(factory):
+                return factory(inst, name)
+            assert factory == 1
+        else:
+            assert assertion == 1
         return v
 
+
+    # See if we can get the value doing a filtered acquire.
+    # aq_acquire will either return the same value as held by
+    # v or it will return an Unauthorized raised by validate.
     validate = SecurityManagement.getSecurityManager().validate
-        if validate(inst, inst, name, v):
+    aq_acquire(inst, name, aq_validate, validate)
+    
     return v
-    raise Unauthorized, name

lib/python/AccessControl/cAccessControl.c

@@ -2080,8 +2080,13 @@ guarded_getattr(PyObject *inst, PyObject *name, PyObject *default_,
   int i;
 
   /* if name[:1] != '_': */
-  if ( (PyString_Check(name) || PyUnicode_Check(name)) && 
-       PyString_AsString(name)[0] != '_')
+  if (PyString_Check(name) || PyUnicode_Check(name)) {
+    char *name_s = PyString_AsString(name);
+
+    if (name_s == NULL)
+        return NULL;
+
+    if (name_s[0] != '_')
     {
 
       /*
@@ -2107,8 +2112,15 @@ guarded_getattr(PyObject *inst, PyObject *name, PyObject *default_,
         }
 
       /*
+
         assertion = Containers(type(inst))
-            if type(assertion) is DictType:
+      */
+      t = PyDict_GetItem(ContainerAssertions, OBJECT(inst->ob_type));
+      if (t != NULL)
+        {
+
+      /*
+        if isinstance(assertion, dict):
             # We got a table that lets us reason about individual
             # attrs
             assertion = assertion.get(name)
@@ -2120,18 +2132,7 @@ guarded_getattr(PyObject *inst, PyObject *name, PyObject *default_,
                 # Nope, it's boolean
                 return v
             raise Unauthorized, name
-            
-            elif assertion:
-                # So the entry in the outer table is not a dict 
-                # It's allowed to be a vetoing function:
-                if callable(assertion):
-                    assertion(name, v)
-                # No veto, so we can return
-                return v
       */
-      t = PyDict_GetItem(ContainerAssertions, OBJECT(inst->ob_type));
-      if (t != NULL)
-        {
           if (PyDict_Check(t))
             {
               PyObject *attrv;
@@ -2156,42 +2157,58 @@ guarded_getattr(PyObject *inst, PyObject *name, PyObject *default_,
               goto unauth;
             }
 
-          i = PyObject_IsTrue(t);
-          if (i < 0) goto err;
-          if (i)
-            {
-              if (t->ob_type->tp_call)
+      /*
+        if assertion:
+            if callable(assertion):
+                factory = assertion(name, v)
+                if callable(factory):
+                    return factory(inst, name)
+                assert factory == 1
+            assert callable == 1
+            return v
+
+      */
+          if (PyCallable_Check(t))
             {
-                  PyObject *ignored;
-                  ignored = callfunction2(t, name, v);
-                  if (ignored == NULL)
+              PyObject *factory;
+
+              factory = callfunction2(t, name, v);
+              if (factory == NULL) 
+                goto err;
+
+              if (PyCallable_Check(factory))
                 {
-                      /* veto */
                   Py_DECREF(v);
-                      return NULL;
+                  v = callfunction2(factory, inst, name);
                 }
-                  Py_DECREF(ignored);
+              Py_DECREF(factory);
             }
           return v;
         }
-        }
 
       /* 
-        if validate(inst, inst, name, v):
+        # See if we can get the value doing a filtered acquire.
+        # aq_acquire will either return the same value as held by
+        # v or it will return an Unauthorized raised by validate.
+        validate = SecurityManagement.getSecurityManager().validate
+        aq_acquire(inst, name, aq_validate, validate)
+        
         return v
       */
-      validate=callfunction4(validate, inst, inst, name, v);
-      if (validate==NULL) goto err;
-      i=PyObject_IsTrue(validate);
-      Py_DECREF(validate);
-      if (i < 0) goto err;
-      if (i > 0) return v;
+
+      t = aq_Acquire(inst, name, aq_validate, validate, 1, NULL, 0);
+      if (t == NULL)
+        return NULL;
+      Py_DECREF(t);
+
+      return v;
             
       unauthErr(name, v);
     err:
       Py_DECREF(v);
       return NULL;
     }
+  }
 
  unauth:
   /* raise Unauthorized, name */

lib/python/AccessControl/tests/testAcquisition.py

+##############################################################################
+#
+# Copyright (c) 2001 Zope Corporation and Contributors. All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE
+#
+##############################################################################
+"""Tests demonstrating consequences of guarded_getattr fix from 2004/08/07
+
+   http://mail.zope.org/pipermail/zope-checkins/2004-August/028152.html
+   http://zope.org/Collectors/CMF/259
+
+"""
+
+import unittest
+
+from Testing.makerequest import makerequest
+
+import Zope2
+Zope2.startup()
+
+from OFS.SimpleItem import SimpleItem
+from Globals import InitializeClass
+from AccessControl import ClassSecurityInfo
+from AccessControl.SecurityManagement import newSecurityManager
+from AccessControl.SecurityManagement import noSecurityManager
+from AccessControl.Permissions import view, view_management_screens
+from AccessControl.ImplPython import guarded_getattr as guarded_getattr_py
+from AccessControl.ImplC import guarded_getattr as guarded_getattr_c
+from Products.SiteErrorLog.SiteErrorLog import SiteErrorLog
+
+
+class AllowedItem(SimpleItem):
+    id = 'allowed'
+    security = ClassSecurityInfo()
+    security.setDefaultAccess('allow')
+
+InitializeClass(AllowedItem)
+
+class DeniedItem(SimpleItem):
+    id = 'denied'
+    security = ClassSecurityInfo()
+    security.setDefaultAccess('deny')
+
+InitializeClass(DeniedItem)
+
+class ProtectedItem(SimpleItem):
+    id = 'protected'
+    security = ClassSecurityInfo()
+    security.declareObjectProtected(view_management_screens)
+
+InitializeClass(ProtectedItem)
+
+class ProtectedSiteErrorLog(SiteErrorLog):
+    '''This differs from the base by declaring security
+       for the object itself.
+    '''
+    id = 'error_log2'
+    security = ClassSecurityInfo()
+    security.declareObjectProtected(view)
+
+InitializeClass(ProtectedSiteErrorLog)
+
+
+class TestGetAttr(unittest.TestCase):
+
+    def setUp(self):
+        import transaction
+        self.guarded_getattr = guarded_getattr_py
+        transaction.manager.begin()
+        self.app = makerequest(Zope2.app())
+        try:
+
+            # Set up a manager user
+            self.uf = self.app.acl_users
+            self.uf._doAddUser('manager', 'secret', ['Manager'], [])
+            self.login('manager')
+
+            # Set up objects in the root that we want to aquire
+            self.app.manage_addFolder('plain_folder')
+            self.app._setObject('error_log2', ProtectedSiteErrorLog())
+
+            # We also want to be able to acquire simple attributes
+            self.app.manage_addProperty(id='simple_type', type='string', value='a string')
+
+            # Set up a subfolder and the objects we want to acquire from
+            self.app.manage_addFolder('subfolder')
+            self.folder = self.app.subfolder
+            self.folder._setObject('allowed', AllowedItem())
+            self.folder._setObject('denied', DeniedItem())
+            self.folder._setObject('protected', ProtectedItem())
+
+        except:
+            self.tearDown()
+            raise
+
+    def tearDown(self):
+        import transaction
+        noSecurityManager()
+        transaction.manager.get().abort()
+        self.app._p_jar.close()
+
+    def login(self, name):
+        user = self.uf.getUserById(name)
+        user = user.__of__(self.uf)
+        newSecurityManager(None, user)
+
+    # Acquire plain folder
+
+    def testFolderAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'plain_folder')
+        self.assertEqual(o, self.app.plain_folder)
+
+    def testFolderDenied(self):
+        o = self.guarded_getattr(self.folder.denied, 'plain_folder')
+        self.assertEqual(o, self.app.plain_folder)
+
+    def testFolderProtected(self):
+        o = self.guarded_getattr(self.folder.protected, 'plain_folder')
+        self.assertEqual(o, self.app.plain_folder)
+
+    # Acquire user folder
+
+    def testAclUsersAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'acl_users')
+        self.assertEqual(o, self.app.acl_users)
+
+    def testAclUsersDenied(self):
+        # XXX: Fails in 2.7.3
+        o = self.guarded_getattr(self.folder.denied, 'acl_users')
+        self.assertEqual(o, self.app.acl_users)
+
+    def testAclUsersProtected(self):
+        # XXX: Fails in 2.7.3 for Anonymous
+        o = self.guarded_getattr(self.folder.protected, 'acl_users')
+        self.assertEqual(o, self.app.acl_users)
+
+    # Acquire browser id manager
+
+    def testBrowserIdManagerAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'browser_id_manager')
+        self.assertEqual(o, self.app.browser_id_manager)
+
+    def testBrowserIdManagerDenied(self):
+        o = self.guarded_getattr(self.folder.denied, 'browser_id_manager')
+        self.assertEqual(o, self.app.browser_id_manager)
+
+    def testBrowserIdManagerProtected(self):
+        o = self.guarded_getattr(self.folder.protected, 'browser_id_manager')
+        self.assertEqual(o, self.app.browser_id_manager)
+
+    # Acquire error log
+
+    def testErrorLogAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'error_log')
+        self.assertEqual(o, self.app.error_log)
+
+    def testErrorLogDenied(self):
+        # XXX: Fails in 2.7.3
+        o = self.guarded_getattr(self.folder.denied, 'error_log')
+        self.assertEqual(o, self.app.error_log)
+
+    def testErrorLogProtected(self):
+        # XXX: Fails in 2.7.3 for Anonymous
+        o = self.guarded_getattr(self.folder.protected, 'error_log')
+        self.assertEqual(o, self.app.error_log)
+
+    # Now watch this: error log with object security declaration works fine!
+
+    def testProtectedErrorLogAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'error_log2')
+        self.assertEqual(o, self.app.error_log2)
+
+    def testProtectedErrorLogDenied(self):
+        o = self.guarded_getattr(self.folder.denied, 'error_log2')
+        self.assertEqual(o, self.app.error_log2)
+
+    def testProtectedErrorLogProtected(self):
+        o = self.guarded_getattr(self.folder.protected, 'error_log2')
+        self.assertEqual(o, self.app.error_log2)
+
+    # This appears to mean that any potential acquiree must make sure
+    # to declareObjectProtected(SomePermission).
+
+    # From the ZDG:
+    # We've seen how to make  assertions on methods - but in the case of
+    # someObject we are not trying to access any particular method, but
+    # rather the object itself (to pass it to some_method). Because the
+    # security machinery will try to validate access to someObject, we
+    # need a way to let the security machinery know how to handle access
+    # to the object itself in addition to protecting its methods.
+
+    # IOW, acquiring an object in restricted Python now amounts to
+    # "passing it to some_method".
+
+
+    # Also test Richard Jones' use-case of acquiring a string:
+
+    def testSimpleTypeAllowed(self):
+        o = self.guarded_getattr(self.folder.allowed, 'simple_type')
+        self.assertEqual(o, 'a string')
+
+    def testSimpleTypeDenied(self):
+        # XXX: Fails in 2.7.3
+        o = self.guarded_getattr(self.folder.denied, 'simple_type')
+        self.assertEqual(o, 'a string')
+
+    def testSimpleTypeProtected(self):
+        # XXX: Fails in 2.7.3 for Anonymous
+        o = self.guarded_getattr(self.folder.protected, 'simple_type')
+        self.assertEqual(o, 'a string')
+
+
+class TestGetAttrAnonymous(TestGetAttr):
+
+    # Run all tests again as Anonymous User
+
+    def setUp(self):
+        TestGetAttr.setUp(self)
+        # Log out
+        noSecurityManager()
+
+
+class TestGetAttr_c(TestGetAttr):
+
+    def setUp(self):
+        TestGetAttr.setUp(self)
+        self.guarded_getattr = guarded_getattr_c
+
+class TestGetAttrAnonymous_c(TestGetAttrAnonymous):
+
+    def setUp(self):
+        TestGetAttrAnonymous.setUp(self)
+        self.guarded_getattr = guarded_getattr_c
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestGetAttr))
+    suite.addTest(unittest.makeSuite(TestGetAttrAnonymous))
+    suite.addTest(unittest.makeSuite(TestGetAttr_c))
+    suite.addTest(unittest.makeSuite(TestGetAttrAnonymous_c))
+    return suite
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='test_suite')
+

lib/python/Shared/DC/Scripts/Bindings.py

@@ -15,6 +15,7 @@ __version__='$Revision$'[11:-2]
 
 import Globals
 from AccessControl import getSecurityManager
+from AccessControl.PermissionRole import _what_not_even_god_should_do
 from AccessControl.ZopeGuards import guarded_getattr
 from Persistence import Persistent
 from string import join, strip
@@ -167,15 +168,19 @@ class UnauthorizedBinding:
         self._wrapped = wrapped
 
     __allow_access_to_unprotected_subobjects__ = 1
+    __roles__ = _what_not_even_god_should_do
 
-    def __getattr__(self, name, default=None):
+    def __repr__(self):
+        return '<UnauthorizedBinding: %s>' % self._name
 
+    def __getattr__(self, name, default=None):
         # Make *extra* sure that the wrapper isn't used to access
-        # __call__, __str__, __repr__, etc.
+        # __call__, etc.
         if name.startswith('__'):
             self.__you_lose()
 
         return guarded_getattr(self._wrapped, name, default)
+        #return getattr(self._wrapped, name, default)
 
     def __you_lose(self):
         name = self.__dict__['_name']