[Zope3-checkins] SVN: Zope3/trunk/src/zope/security/ Make changes as discussed in http://www.zope.org/Collectors/Zope3-dev/506: canWrite now does not raise Forbidden if canAccess does not, effectively.

[Gary Poster]

Log message for revision 40886:
  Make changes as discussed in http://www.zope.org/Collectors/Zope3-dev/506: canWrite now does not raise Forbidden if canAccess does not, effectively.
  

Changed:
  U   Zope3/trunk/src/zope/security/checker.py
  U   Zope3/trunk/src/zope/security/tests/test_checker.py

-=-
Modified: Zope3/trunk/src/zope/security/checker.py
===================================================================
--- Zope3/trunk/src/zope/security/checker.py	2005-12-19 13:14:52 UTC (rev 40885)
+++ Zope3/trunk/src/zope/security/checker.py	2005-12-19 17:47:35 UTC (rev 40886)
@@ -97,8 +97,26 @@
         checker.check_setattr(obj, name)
     except Unauthorized:
         return False
-    # if it is Forbidden (or anything else), let it be raised: it probably
-    # indicates a programming or configuration error
+    except ForbiddenAttribute:
+        # we are going to be a bit DWIM-y here: see
+        # http://www.zope.org/Collectors/Zope3-dev/506
+        
+        # generally, if the check is ForbiddenAttribute we want it to be
+        # raised: it probably indicates a programming or configuration error.
+        # However, we special case a write ForbiddenAttribute when one can
+        # actually read the attribute: this represents a reasonable
+        # configuration of a readonly attribute, and returning False (meaning
+        # "no, you can't write it") is arguably more useful than raising the
+        # exception.
+        try:
+            checker.check_getattr(obj, name)
+            # we'll let *this* ForbiddenAttribute fall through, if any.  It
+            # means that both read and write are forbidden.
+        except Unauthorized:
+            pass
+        return False
+    # all other exceptions, other than Unauthorized and ForbiddenAttribute,
+    # should be passed through uncaught, as they indicate programmer error
     return True
 
 def canAccess(obj, name):

Modified: Zope3/trunk/src/zope/security/tests/test_checker.py
===================================================================
--- Zope3/trunk/src/zope/security/tests/test_checker.py	2005-12-19 13:14:52 UTC (rev 40885)
+++ Zope3/trunk/src/zope/security/tests/test_checker.py	2005-12-19 17:47:35 UTC (rev 40886)
@@ -435,11 +435,12 @@
         # write is not merely unauthorized but forbidden--including write access
         # for baz.
         checker = Checker(
-            {'foo':'test_allowed',
+            {'foo':'test_allowed', # these are the read settings
              'bar':'test_allowed',
              'baz':'you_will_not_have_this_permission'},
-            {'foo':'test_allowed',
-             'bar':'you_will_not_have_this_permission'})
+            {'foo':'test_allowed', # these are the write settings
+             'bar':'you_will_not_have_this_permission',
+             'bing':'you_will_not_have_this_permission'})
         defineChecker(SomeClass, checker)
 
         # so, our hapless interaction may write and access foo...
@@ -453,13 +454,28 @@
         # ...and may access baz.
         self.assert_(not canAccess(obj, 'baz'))
 
-        # there are no security assertions for writing baz or accessing
-        # anything else, so these actually raise Forbidden.  The rationale
-        # behind exposing the Forbidden exception is primarily that it is
-        # usually indicative of programming or configuration errors.
-        self.assertRaises(Forbidden, canWrite, obj, 'baz')
+        # there are no security assertions for writing or reading shazam, so
+        # checking these actually raises Forbidden.  The rationale behind
+        # exposing the Forbidden exception is primarily that it is usually
+        # indicative of programming or configuration errors.
         self.assertRaises(Forbidden, canAccess, obj, 'shazam')
+        self.assertRaises(Forbidden, canWrite, obj, 'shazam')
 
+        # However, we special-case canWrite when an attribute has a Read
+        # setting but no Write setting.  Consider the 'baz' attribute from the
+        # checker above: it is readonly.  All users are forbidden to write
+        # it.  This is a very reasonable configuration.  Therefore, canWrite
+        # will hide the Forbidden exception if and only if there is a
+        # setting for accessing the attribute.
+        self.assert_(not canWrite(obj, 'baz'))
+
+        # The reverse is not true at the moment: an unusal case like the
+        # write-only 'bing' attribute will return a boolean for canWrite,
+        # but canRead will simply raise a Forbidden exception, without checking
+        # write settings.
+        self.assert_(not canWrite(obj, 'bing'))
+        self.assertRaises(Forbidden, canAccess, obj, 'bing')
+
 class TestCheckerPublic(TestCase):
 
     def test_that_pickling_CheckerPublic_retains_identity(self):