fix p.security.ACLPermitsResult to subclass p.security.PermitsResult

The ``IAuthorizationPolicy`` is expected to return an instance of
``PermitsResult`` and the ``ACLPermitsResult`` now subclasses this to
form a consistent class hierarchy.

Similarly the ``ACLDenied`` subclasses ``Denied`` and ``ACLAllowed``
subclasses ``Allowed`` for consistency.

4 files changed, 90 insertions, 51 deletions

diff --git a/docs/api/security.rst b/docs/api/security.rst
index 88086dbbf..116459226 100644
--- a/docs/api/security.rst
+++ b/docs/api/security.rst
@@ -80,15 +80,23 @@ Return Values
     'george', 'read')`` that means deny access.  A sequence of ACEs
     makes up an ACL.  It is a string, and its actual value is "Deny".
 
+.. autoclass:: Denied
+   :members: msg
+
+   .. automethod:: __new__
+
+.. autoclass:: Allowed
+   :members: msg
+
+   .. automethod:: __new__
+
 .. autoclass:: ACLDenied
-   :members:
+   :members: msg
 
-.. autoclass:: ACLAllowed
-   :members:
+   .. automethod:: __new__
 
-.. autoclass:: Denied
-   :members:
+.. autoclass:: ACLAllowed
+   :members: msg
 
-.. autoclass:: Allowed
-   :members:
+   .. automethod:: __new__
 
diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py
index 4a069ad65..c6fbe3af8 100644
--- a/pyramid/interfaces.py
+++ b/pyramid/interfaces.py
@@ -503,8 +503,10 @@ class IAuthenticationPolicy(Interface):
 class IAuthorizationPolicy(Interface):
     """ An object representing a Pyramid authorization policy. """
     def permits(context, principals, permission):
-        """ Return ``True`` if any of the ``principals`` is allowed the
-        ``permission`` in the current ``context``, else return ``False``
+        """ Return an instance of :class:`pyramid.security.Allowed` if any
+        of the ``principals`` is allowed the ``permission`` in the current
+        ``context``, else return an instance of
+        :class:`pyramid.security.Denied`.
         """
 
     def principals_allowed_by_permission(context, permission):
diff --git a/pyramid/security.py b/pyramid/security.py
index 035f09f77..d12314684 100644
--- a/pyramid/security.py
+++ b/pyramid/security.py
@@ -245,6 +245,14 @@ def view_execution_permitted(context, request, name=''):
 
 class PermitsResult(int):
     def __new__(cls, s, *args):
+        """
+        Create a new instance.
+
+        :param fmt: A format string explaining the reason for denial.
+        :param args: Arguments are stored and used with the format string
+                      to generate the ``msg``.
+
+        """
         inst = int.__new__(cls, cls.boolval)
         inst.s = s
         inst.args = args
@@ -252,6 +260,7 @@ class PermitsResult(int):
 
     @property
     def msg(self):
+        """ A string indicating why the result was generated."""
         return self.s % self.args
 
     def __str__(self):
@@ -263,24 +272,52 @@ class PermitsResult(int):
                                                     self.msg)
 
 class Denied(PermitsResult):
-    """ An instance of ``Denied`` is returned when a security-related
+    """
+    An instance of ``Denied`` is returned when a security-related
     API or other :app:`Pyramid` code denies an action unrelated to
     an ACL check.  It evaluates equal to all boolean false types.  It
     has an attribute named ``msg`` describing the circumstances for
-    the deny."""
+    the deny.
+
+    """
     boolval = 0
 
 class Allowed(PermitsResult):
-    """ An instance of ``Allowed`` is returned when a security-related
+    """
+    An instance of ``Allowed`` is returned when a security-related
     API or other :app:`Pyramid` 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."""
+    the allow.
+
+    """
     boolval = 1
 
-class ACLPermitsResult(int):
+class ACLPermitsResult(PermitsResult):
     def __new__(cls, ace, acl, permission, principals, context):
-        inst = int.__new__(cls, cls.boolval)
+        """
+        Create a new instance.
+
+        :param ace: The :term:`ACE` that matched, triggering the result.
+        :param acl: The :term:`ACL` containing ``ace``.
+        :param permission: The required :term:`permission`.
+        :param principals: The list of :term:`principals <principal>` provided.
+        :param context: The :term:`context` providing the :term:`lineage`
+                        searched.
+
+        """
+        fmt = ('%s permission %r via ACE %r in ACL %r on context %r for '
+               'principals %r')
+        inst = PermitsResult.__new__(
+            cls,
+            fmt,
+            cls.__name__,
+            permission,
+            ace,
+            acl,
+            context,
+            principals,
+        )
         inst.permission = permission
         inst.ace = ace
         inst.acl = acl
@@ -288,44 +325,31 @@ class ACLPermitsResult(int):
         inst.context = context
         return inst
 
-    @property
-    def msg(self):
-        s = ('%s permission %r via ACE %r in ACL %r on context %r for '
-             'principals %r')
-        return s % (self.__class__.__name__,
-                    self.permission,
-                    self.ace,
-                    self.acl,
-                    self.context,
-                    self.principals)
-
-    def __str__(self):
-        return self.msg
+class ACLDenied(ACLPermitsResult, Denied):
+    """
+    An instance of ``ACLDenied`` is a specialization of
+    :class:`pyramid.security.Denied` that represents that a security check
+    made explicitly against ACL was denied.  It evaluates equal to all
+    boolean false types.  It also has the following attributes: ``acl``,
+    ``ace``, ``permission``, ``principals``, and ``context``.  These
+    attributes indicate the security values involved in the request.  Its
+    ``__str__`` method prints a summary of these attributes for debugging
+    purposes. The same summary is available as the ``msg`` attribute.
 
-    def __repr__(self):
-        return '<%s instance at %s with msg %r>' % (self.__class__.__name__,
-                                                    id(self),
-                                                    self.msg)
+    """
 
-class ACLDenied(ACLPermitsResult):
-    """ 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 the following attributes: ``acl``, ``ace``,
-    ``permission``, ``principals``, and ``context``.  These attributes
-    indicate the security values involved in the request.  Its __str__ method
-    prints a summary of these attributes for debugging purposes.  The same
-    summary is available as the ``msg`` attribute."""
-    boolval = 0
+class ACLAllowed(ACLPermitsResult, Allowed):
+    """
+    An instance of ``ACLAllowed`` is a specialization of
+    :class:`pyramid.security.Allowed` that represents that a security check
+    made explicitly against ACL was allowed.  It evaluates equal to all
+    boolean true types.  It also has the following attributes: ``acl``,
+    ``ace``, ``permission``, ``principals``, and ``context``.  These
+    attributes indicate the security values involved in the request.  Its
+    ``__str__`` method prints a summary of these attributes for debugging
+    purposes. The same summary is available as the ``msg`` attribute.
 
-class ACLAllowed(ACLPermitsResult):
-    """ An instance of ``ACLAllowed`` represents that a security check made
-    explicitly against ACL was allowed.  It evaluates equal to all boolean
-    true types.  It also has the following attributes: ``acl``, ``ace``,
-    ``permission``, ``principals``, and ``context``.  These attributes
-    indicate the security values involved in the request.  Its __str__ method
-    prints a summary of these attributes for debugging purposes.  The same
-    summary is available as the ``msg`` attribute."""
-    boolval = 1
+    """
 
 class AuthenticationAPIMixin(object):
 
@@ -395,7 +419,8 @@ class AuthorizationAPIMixin(object):
         :type permission: unicode, str
         :param context: A resource object or ``None``
         :type context: object
-        :returns: `pyramid.security.PermitsResult`
+        :returns: Either :class:`pyramid.security.Allowed` or
+                  :class:`pyramid.security.Denied`.
 
         .. versionadded:: 1.5
 
diff --git a/pyramid/tests/test_security.py b/pyramid/tests/test_security.py
index 5561a05d7..1da73ff73 100644
--- a/pyramid/tests/test_security.py
+++ b/pyramid/tests/test_security.py
@@ -92,9 +92,11 @@ class TestACLAllowed(unittest.TestCase):
         return klass(*arg, **kw)
 
     def test_it(self):
+        from pyramid.security import Allowed
         msg = ("ACLAllowed permission 'permission' via ACE 'ace' in ACL 'acl' "
                "on context 'ctx' for principals 'principals'")
         allowed = self._makeOne('ace', 'acl', 'permission', 'principals', 'ctx')
+        self.assertIsInstance(allowed, Allowed)
         self.assertTrue(msg in allowed.msg)
         self.assertEqual(allowed, True)
         self.assertTrue(allowed)
@@ -112,9 +114,11 @@ class TestACLDenied(unittest.TestCase):
         return klass(*arg, **kw)
 
     def test_it(self):
+        from pyramid.security import Denied
         msg = ("ACLDenied permission 'permission' via ACE 'ace' in ACL 'acl' "
                "on context 'ctx' for principals 'principals'")
         denied = self._makeOne('ace', 'acl', 'permission', 'principals', 'ctx')
+        self.assertIsInstance(denied, Denied)
         self.assertTrue(msg in denied.msg)
         self.assertEqual(denied, False)
         self.assertFalse(denied)