Features

  - The ``BFG_DEBUG_AUTHORIZATION`` envvar and the
    ``debug_authorization`` config file value now only imply debugging
    of view-invoked security checks.  Previously, information was
    printed for every call to ``has_permission`` as well, which made
    output confusing.  To debug ``has_permission`` checks and other
    manual permission checks, use the debugger and print statements in
    your own code.

  - Authorization debugging info is now only present in the HTTP
    response body oif ``debug_authorization`` is true.

  - The format of authorization debug messages was improved.

  - A new ``BFG_DEBUG_NOTFOUND`` envvar was added and a symmetric
    ``debug_notfound`` config file value was added.  When either is
    true, and a NotFound response is returned by the BFG router
    (because a view could not be found), debugging information is
    printed to stderr.  When this value is set true, the body of
    HTTPNotFound responses will also contain the same debugging
    information.

  - ``Allowed`` and ``Denied`` responses from the security machinery
    are now specialized into two types: ACL types, and non-ACL types.
    The ACL-related responses are instances of
    ``repoze.bfg.security.ACLAllowed`` and
    ``repoze.bfg.security.ACLDenied``.  The non-ACL-related responses
    are ``repoze.bfg.security.Allowed`` and
    ``repoze.bfg.security.Denied``.  The allowed-type responses
    continue to evaluate equal to things that themselves evaluate
    equal to the ``True`` boolean, while the denied-type responses
    continue to evaluate equal to things that themselves evaluate
    equal to the ``False`` boolean.  The only difference between the
    two types is the information attached to them for debugging
    purposes.

  - Added a new ``BFG_DEBUG_ALL`` envvar and a symmetric ``debug_all``
    config file value.  When either is true, all other debug-related
    flags are set true unconditionally (e.g. ``debug_notfound`` and
    ``debug_authorization``).

  Documentation

  - Added info about debug flag changes.

  - Added a section to the security chapter named "Debugging
    Imperative Authorization Failures" (for e.g. ``has_permssion``).

1 files changed, 75 insertions, 44 deletions

diff --git a/repoze/bfg/security.py b/repoze/bfg/security.py
index f1149cc7a..21b7f98d3 100644
--- a/repoze/bfg/security.py
+++ b/repoze/bfg/security.py
@@ -7,7 +7,6 @@ from repoze.bfg.interfaces import ISecurityPolicy
 from repoze.bfg.interfaces import IViewPermission
 from repoze.bfg.interfaces import IViewPermissionFactory
 from repoze.bfg.interfaces import NoAuthorizationInformation
-from repoze.bfg.interfaces import ILogger
 
 Everyone = 'system.Everyone'
 Authenticated = 'system.Authenticated'
@@ -65,14 +64,14 @@ def principals_allowed_by_permission(context, permission):
 
 class ACLAuthorizer(object):
 
-    def __init__(self, context, logger):
+    def __init__(self, context):
         self.context = context
-        self.logger = logger
 
     def permits(self, permission, *principals):
         acl = getattr(self.context, '__acl__', None)
         if acl is None:
-            raise NoAuthorizationInformation('%s item has no __acl__' % acl)
+            raise NoAuthorizationInformation('%s item has no __acl__' %
+                                             self.context)
 
         for ace in acl:
             ace_action, ace_principal, ace_permissions = ace
@@ -80,18 +79,14 @@ class ACLAuthorizer(object):
                 if ace_principal == principal:
                     permissions = flatten(ace_permissions)
                     if permission in permissions:
-                        action = ace_action
-                        if action == Allow:
-                            result = Allowed(ace, acl, permission, principals,
+                        if ace_action == Allow:
+                            return ACLAllowed(ace, acl, permission, principals,
+                                              self.context)
+                        else:
+                            return ACLDenied(ace, acl, permission, principals,
                                              self.context)
-                            self.logger and self.logger.debug(str(result))
-                            return result
-                        result = Denied(ace, acl, permission, principals,
-                                        self.context)
-                        self.logger and self.logger.debug(str(result))
-                        return result
-        result = Denied(None, acl, permission, principals, self.context)
-        self.logger and self.logger.debug(str(result))
+        # default deny
+        result = ACLDenied(None, acl, permission, principals, self.context)
         return result
 
 class ACLSecurityPolicy(object):
@@ -102,18 +97,18 @@ class ACLSecurityPolicy(object):
         self.get_principals = get_principals
 
     def permits(self, context, request, permission):
-        """ Return ``Allowed`` if the policy permits access,
-        ``Denied`` if not."""
-        logger = queryUtility(ILogger, name='repoze.bfg.debug')
+        """ Return ``ACLAllowed`` if the policy permits access,
+        ``ACLDenied`` if not. """
         principals = self.effective_principals(request)
         for location in lineage(context):
-            authorizer = self.authorizer_factory(location, logger)
+            authorizer = self.authorizer_factory(location)
             try:
                 return authorizer.permits(permission, *principals)
+
             except NoAuthorizationInformation:
                 continue
 
-        return False
+        return Denied(None, None, permission, principals, self.context)
 
     def authenticated_userid(self, request):
         principals = self.get_principals(request)
@@ -202,26 +197,22 @@ def RepozeWhoIdentityACLSecurityPolicy():
     return ACLSecurityPolicy(get_who_principals)
 
 class PermitsResult:
-    def __init__(self, ace, acl, permission, principals, context):
-        self.acl = acl
-        self.ace = ace
-        self.permission = permission
-        self.principals = principals
-        self.context_repr = repr(context)
-
     def __str__(self):
-        msg = '%s: %r via ace %r in acl %r or principals %r in context %s'
-        msg = msg % (self.__class__.__name__,
-                     self.permission, self.ace, self.acl, self.principals,
-                     self.context_repr)
-        return msg
+        return self.msg
+
+    def __repr__(self):
+        return '<%s instance at %s with msg %r>' % (self.__class__.__name__,
+                                                    id(self),
+                                                    self.msg)
 
 class Denied(PermitsResult):
-    """ An instance of ``Denied`` is returned by an ACL denial.  It
-    evaluates equal to all boolean false types.  It also has
-    attributes which indicate which acl, ace, permission, principals,
-    and context were involved in the request.  Its __str__ method
-    prints a summary of these attributes for debugging purposes."""
+    """ An instance of ``Denied`` is returned when a security policy
+    or other ``repoze.bfg`` code denies an action unlrelated to an ACL
+    check.  It evaluates equal to all boolean false types.  It has an
+    attribute named ``msg`` describing the circumstances for the deny."""
+    def __init__(self, msg):
+        self.msg = msg
+
     def __nonzero__(self):
         return False
 
@@ -229,17 +220,55 @@ class Denied(PermitsResult):
         return bool(other) is False
 
 class Allowed(PermitsResult):
-    """ An instance of ``Allowed`` is returned by an ACL allow.  It
-    evaluates equal to all boolean true types.  It also has attributes
-    which indicate which acl, ace, permission, principals, and context
-    were involved in the request.  Its __str__ method prints a summary
-    of these attributes for debugging purposes."""
+    """ An instance of ``Allowed`` is returned when a security policy
+    or other ``repoze.bfg`` code allows an action unrelated to an ACL
+    check.  It evaluates equal to all boolean true types.  It has an
+    attribute named ``msg`` describing the circumstances for the
+    allow."""
+    def __init__(self, msg):
+        self.msg = msg
+
     def __nonzero__(self):
         return True
 
     def __eq__(self, other):
         return bool(other) is True
 
+class ACLPermitsResult:
+    def __init__(self, ace, acl, permission, principals, context):
+        self.permission = permission
+        self.ace = ace
+        self.acl = acl
+        self.principals = principals
+        self.context = context
+        msg = ('%s permission %r via ACE %r in ACL %r on context %r for '
+               'principals %r')
+        msg = msg % (self.__class__.__name__,
+                     self.permission,
+                     self.ace,
+                     self.acl,
+                     self.context,
+                     self.principals)
+        self.msg = msg
+
+class ACLDenied(ACLPermitsResult, Denied):
+    """ An instance of ``ACLDenied`` represents that a security check
+    made explicitly against ACL was denied.  It evaluates equal to all
+    boolean false types.  It also has attributes which indicate which
+    acl, ace, permission, principals, and context were involved in the
+    request.  Its __str__ method prints a summary of these attributes
+    for debugging purposes.  The same summary is available as he
+    ``msg`` attribute."""
+
+class ACLAllowed(ACLPermitsResult, Allowed):
+    """ An instance of ``ACLDenied`` represents that a security check
+    made explicitly against ACL was allowed.  It evaluates equal to
+    all boolean true types.  It also has attributes which indicate
+    which acl, ace, permission, principals, and context were involved
+    in the request.  Its __str__ method prints a summary of these
+    attributes for debugging purposes.  The same summary is available
+    as he ``msg`` attribute."""
+
 def flatten(x):
     """flatten(sequence) -> list
 
@@ -269,8 +298,9 @@ class ViewPermission(object):
         self.request = request
         self.permission_name = permission_name
     
-    def __call__(self, security_policy):
-        return security_policy.permits(self.context, self.request,
+    def __call__(self, security_policy, debug_info=None):
+        return security_policy.permits(self.context,
+                                       self.request,
                                        self.permission_name)
 
     def __repr__(self):
@@ -289,5 +319,6 @@ class ViewPermissionFactory(object):
 class Unauthorized(Exception):
     pass
 
+