[Zope3-checkins] CVS: Zope3/src/zope/security - checker.py:1.31 interfaces.py:1.7

Marius Gedminas mgedmin@codeworks.lt
Thu, 5 Jun 2003 07:45:34 -0400


Update of /cvs-repository/Zope3/src/zope/security
In directory cvs.zope.org:/tmp/cvs-serv11729/src/zope/security

Modified Files:
	checker.py interfaces.py 
Log Message:
Documented the behavior of CombinedChecker explicitly.  Fixed a bug where
CombinedChecker could raise a ForbiddenAttribute instead of Unauthorized.

Documented Checker.permission_id and setattr_permission_id in an interface
(INameBasedChecker).

Remove the dependency on permission_id/setattr_permission_id from
CheckerLoggingMixin.  I do not think anyone cares about the distinction of
'Public' and 'Granted' when debugging checkers.

Make CombinedChecker log things when ZOPE_WATCH_CHECKERS is set.



=== Zope3/src/zope/security/checker.py 1.30 => 1.31 ===
--- Zope3/src/zope/security/checker.py:1.30	Thu Jun  5 05:47:37 2003
+++ Zope3/src/zope/security/checker.py	Thu Jun  5 07:45:03 2003
@@ -27,7 +27,7 @@
 from zope.interface.declarations import ImplementsOnlySpecification
 from zope.interface.declarations import ImplementsSpecification
 from zope.interface.declarations import InterfaceSpecification
-from zope.security.interfaces import IChecker
+from zope.security.interfaces import IChecker, INameBasedChecker
 from zope.security.interfaces import ISecurityProxyFactory
 from zope.security.management import getSecurityManager
 from zope.security._proxy import _Proxy as Proxy, getChecker
@@ -71,11 +71,13 @@
 
 directlyProvides(ProxyFactory, ISecurityProxyFactory)
 
+
 class TrustedCheckerBase:
     """Marker type used by zope.security.proxy.trustedRemoveSecurityProxy"""
 
+
 class Checker(TrustedCheckerBase):
-    implements(IChecker)
+    implements(INameBasedChecker)
 
     def __init__(self, permission_func,
                  setattr_permission_func=lambda name: None
@@ -108,13 +110,11 @@
         return self._setattr_permission_func
 
     def permission_id(self, name):
-        """Return the result of calling the permission func
-        """
+        'See INameBasedChecker'
         return self._permission_func(name)
 
     def setattr_permission_id(self, name):
-        """Return the result of calling the permission func
-        """
+        'See INameBasedChecker'
         return self._setattr_permission_func(name)
 
     def check_getattr(self, object, name):
@@ -165,47 +165,62 @@
 
         return Proxy(value, checker)
 
+
 class CombinedChecker(TrustedCheckerBase):
-    """A checker that combines two other checkers."""
+    """A checker that combines two other checkers in a logical-or fashion.
+
+    The following table describes the result of a combined checker in detail.
+
+    checker1           checker2           CombinedChecker(checker1, checker2)
+    ------------------ ------------------ -----------------------------------
+    ok                 anything           ok (checker2 is never called)
+    Unauthorized       ok                 ok
+    Unauthorized       Unauthorized       Unauthorized
+    Unauthorized       ForbiddenAttribute Unauthorized
+    ForbiddenAttribute ok                 ok
+    ForbiddenAttribute Unauthorized       Unauthorized
+    ForbiddenAttribute ForbiddenAttribute ForbiddenAttribute
+    ------------------ ------------------ -----------------------------------
+    """
     implements(IChecker)
 
     def __init__(self, checker1, checker2):
-        """Create a combined checker
-
-        checker1 takes precedence over checker2.
-        """
+        """Create a combined checker."""
         self._checker1 = checker1
         self._checker2 = checker2
 
-    def permission_id(self, name):
-        permission = self._permission_func(name)
-        if permission is None:
-            permission = self._original_checker.permission_id(name)
-        return permission
-
-    def setattr_permission_id(self, name):
-        permission = self._setattr_permission_func(name)
-        if permission is None:
-            permission = self._original_checker.setattr_permission_id(name)
-        return permission
-
     def check(self, object, name):
+        'See IChecker'
         try:
             self._checker1.check(object, name)
-        except (Unauthorized, ForbiddenAttribute):
+        except ForbiddenAttribute:
             self._checker2.check(object, name)
+        except Unauthorized, unauthorized_exception:
+            try: self._checker2.check(object, name)
+            except ForbiddenAttribute:
+                raise unauthorized_exception
 
     def check_getattr(self, object, name):
+        'See IChecker'
         try:
             self._checker1.check_getattr(object, name)
-        except (Unauthorized, ForbiddenAttribute):
+        except ForbiddenAttribute:
             self._checker2.check_getattr(object, name)
+        except Unauthorized, unauthorized_exception:
+            try: self._checker2.check_getattr(object, name)
+            except ForbiddenAttribute:
+                raise unauthorized_exception
 
     def check_setattr(self, object, name):
+        'See IChecker'
         try:
             self._checker1.check_setattr(object, name)
-        except (Unauthorized, ForbiddenAttribute):
+        except ForbiddenAttribute:
             self._checker2.check_setattr(object, name)
+        except Unauthorized, unauthorized_exception:
+            try: self._checker2.check_setattr(object, name)
+            except ForbiddenAttribute:
+                raise unauthorized_exception
 
     def proxy(self, value):
         'See IChecker'
@@ -248,19 +263,25 @@
             setattr_permission_func = setattr_permission_func.get
         self._setattr_permission_func = setattr_permission_func
 
+        if INameBasedChecker.isImplementedBy(original_checker):
+            directlyProvides(self, INameBasedChecker)
+
     def permission_id(self, name):
+        'See INameBasedChecker'
         permission = self._permission_func(name)
         if permission is None:
             permission = self._original_checker.permission_id(name)
         return permission
 
     def setattr_permission_id(self, name):
+        'See INameBasedChecker'
         permission = self._setattr_permission_func(name)
         if permission is None:
             permission = self._original_checker.setattr_permission_id(name)
         return permission
 
     def check(self, object, name):
+        'See IChecker'
         permission = self._permission_func(name)
         if permission is not None:
             if permission is CheckerPublic:
@@ -277,6 +298,7 @@
             return
 
     def check_getattr(self, object, name):
+        'See IChecker'
         permission = self._permission_func(name)
         if permission is not None:
             if permission is CheckerPublic:
@@ -293,6 +315,7 @@
             return
 
     def check_setattr(self, object, name):
+        'See IChecker'
         permission = self._setattr_permission_func(name)
         if permission is not None:
             if permission is CheckerPublic:
@@ -320,13 +343,10 @@
 
 
 class CheckerLoggingMixin:
-    """Debugging mixin for Checker.
+    """Debugging mixin for checkers.
 
     Prints verbose debugging information about every performed check to
     sys.stderr.
-
-    This class relies on the class it's mixed into having permission_id
-    and setattr_permission_id methods.
     """
 
     def check(self, object, name):
@@ -334,8 +354,6 @@
             super(CheckerLoggingMixin, self).check(object, name)
             if name in _always_available:
                 print >> sys.stderr, '[CHK] + Always available: %s on %r' % (name, object)
-            elif self.permission_id(name) is CheckerPublic:
-                print >> sys.stderr, '[CHK] + Public: %s on %r' % (name, object)
             else:
                 print >> sys.stderr, '[CHK] + Granted: %s on %r' % (name, object)
         except Unauthorized:
@@ -350,8 +368,6 @@
             super(CheckerLoggingMixin, self).check(object, name)
             if name in _always_available:
                 print >> sys.stderr, '[CHK] + Always available getattr: %s on %r' % (name, object)
-            elif self.permission_id(name) is CheckerPublic:
-                print >> sys.stderr, '[CHK] + Public getattr: %s on %r' % (name, object)
             else:
                 print >> sys.stderr, '[CHK] + Granted getattr: %s on %r' % (name, object)
         except Unauthorized:
@@ -364,10 +380,7 @@
     def check_setattr(self, object, name):
         try:
             super(CheckerLoggingMixin, self).check_setattr(object, name)
-            if self.setattr_permission_id(name) is CheckerPublic:
-                print >> sys.stderr, '[CHK] + Public setattr: %s on %r' % (name, object)
-            else:
-                print >> sys.stderr, '[CHK] + Granted setattr: %s on %r' % (name, object)
+            print >> sys.stderr, '[CHK] + Granted setattr: %s on %r' % (name, object)
         except Unauthorized:
             print >> sys.stderr, '[CHK] - Unauthorized setattr: %s on %r' % (name, object)
             raise
@@ -375,11 +388,15 @@
             print >> sys.stderr, '[CHK] - Forbidden setattr: %s on %r' % (name, object)
             raise
 
+
 if WATCH_CHECKERS:
     class Checker(CheckerLoggingMixin, Checker):
         pass
+    class CombinedChecker(CheckerLoggingMixin, CombinedChecker):
+        pass
     class DecoratedChecker(CheckerLoggingMixin, DecoratedChecker):
         pass
+
 
 # Helper class for __traceback_supplement__
 class TracebackSupplement:


=== Zope3/src/zope/security/interfaces.py 1.6 => 1.7 ===
--- Zope3/src/zope/security/interfaces.py:1.6	Thu Mar 13 11:28:14 2003
+++ Zope3/src/zope/security/interfaces.py	Thu Jun  5 07:45:03 2003
@@ -150,6 +150,22 @@
         """Return a security proxy for the value."""
 
 
+class INameBasedChecker(IChecker):
+    """Security checker that uses permissions to check attribute access."""
+
+    def permission_id(name):
+        """Return the permission used to check attribute access on name.
+
+        This permission is used by both check and check_getattr.
+        """
+
+    def setattr_permission_id(name):
+        """Return the permission used to check attribute assignment on name.
+
+        This permission is used by check_setattr.
+        """
+
+
 class ISecurityPolicy(Interface):
 
     def checkPermission(permission, object, context):