Change the semantics of IForbiddenAppFactory.

7 files changed, 90 insertions, 32 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index fa9f44fb6..090c0f412 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,17 +4,30 @@ Next release
 Features
 --------
 
+- It is now possible to register a custom
+  ``repoze.bfg.interfaces.IForbiddenAppFactory`` for a given
+  application.  This feature replaces the
+  ``repoze.bfg.interfaces.IUnauthorizedAppFactory`` feature previously
+  described in the Hooks chapter.  The IForbiddenAppFactory will be
+  called when the framework detects an authorization failure; it
+  should accept a context object and a request object; it should
+  return a WSGI application.  Read the below point for more info and
+  see the Hooks narrative chapter of the BFG docs for more info.
+
 - It is now possible to register a security policy that returns a
   customized ``Forbidden`` WSGI application when BFG cannot authorize
   an invocation of a view.  To this end, ISecurityPolicy objects must
-  now have a ``forbidden`` method.  This method should return a WSGI
-  application.  The returned WSGI application should generate a
-  response which is appropriate when access to a view resource was
-  forbidden by the security policy (e.g. perhaps a login page).
-  ``repoze.bfg`` is willing to operate with a custom security policy
-  that does not have a ``forbidden`` method, but it will issue a
-  warning; eventually security policies without a ``forbidden`` method
-  will cease to work under ``repoze.bfg``.
+  now have a ``forbidden`` method that accepts two arguments:
+  ``context`` and ``request``.  The ``context`` will be the context
+  found by the router, the ``request`` will be the current request.
+  This method should return a WSGI application.  The returned WSGI
+  application should generate a response which is appropriate when
+  access to a view resource was forbidden by the security policy
+  (e.g. perhaps a login page).  ``repoze.bfg`` is willing to operate
+  with a custom security policy that does not have a ``forbidden``
+  method, but it will issue a warning; eventually security policies
+  without a ``forbidden`` method will cease to work under
+  ``repoze.bfg``.
 
   Note that the ``forbidden`` WSGI application returned by the
   security policy is not used if a developer has registered an
@@ -43,7 +56,8 @@ Deprecations
 ------------
 
 - The ``repoze.bfg.interfaces.IUnauthorizedAppFactory`` interface has
-  been renamed to ``repoze.bfg.interfaces.IForbiddenAppFactory``.
+  been deprecated in favor of using the new
+  ``repoze.bfg.interfaces.IForbiddenAppFactory`` mechanism.
 
 
 0.8.1 (2009-05-21)
diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst
index 5eab5e41e..aefa95046 100644
--- a/docs/narr/hooks.rst
+++ b/docs/narr/hooks.rst
@@ -145,13 +145,10 @@ factory:
 
 .. code-block:: python
 
-   from webob.exc import HTTPForbidden
+   from repoze.bfg.chameleon_zpt import render_template_to_response
 
-   class MyForbidden(HTTPForbidden):
-       pass
-
-   def forbidden_app_factory():
-       return MyForbidden
+   def forbidden_app_factory(context, request):
+       return render_template_to_response('templates/login_form.pt')
 
 .. note:: When an Forbidden application factory is invoked, it is
    passed the WSGI environ and the WSGI ``start_response`` handler by
@@ -159,7 +156,8 @@ factory:
    ``repoze.bfg.message`` that has a value explaining why the action
    was forbidden.  This error will be different when the
    ``debug_authorization`` environment setting is true than it is when
-   it is false.
+   it is false.  A WebOb ``Response`` object is a valid WSGI
+   application, by the way.
 
 .. warning:: the default forbidden application factory sends a
    response with a ``401 Unauthorized`` status code for backwards
diff --git a/repoze/bfg/interfaces.py b/repoze/bfg/interfaces.py
index 034e0ac1c..78db34ced 100644
--- a/repoze/bfg/interfaces.py
+++ b/repoze/bfg/interfaces.py
@@ -131,7 +131,7 @@ class ISecurityPolicy(Interface):
         implementation, in which case, it should raise a
         ``NotImplementedError`` exception."""
 
-    def forbidden():
+    def forbidden(context, request):
         """ This method should return a WSGI application (a callable
         accepting ``environ`` and ``start_response``).  This WSGI
         application will be called by ``repoze.bfg`` when view
@@ -140,7 +140,10 @@ class ISecurityPolicy(Interface):
         to a view resource was forbidden by the security policy.  Note
         that the ``repoze.bfg.message`` key in the environ passed to
         the WSGI app will contain the 'raw' reason that view
-        invocation was denied by repoze.bfg."""
+        invocation was denied by repoze.bfg.  The ``context`` object
+        passed in will be the context found by ``repoze.bfg`` when the
+        denial was found and the ``request`` will be the request which
+        caused the denial."""
 
 class IViewPermission(Interface):
     def __call__(security_policy):
@@ -209,20 +212,23 @@ class INotFoundAppFactory(Interface):
         pertaining to the reason for the notfound."""
 
 class IForbiddenAppFactory(Interface):
-    """ A utility which returns an Unauthorized WSGI application
+    """ A utility which returns an Forbidden WSGI application
     factory"""
-    def __call__():
+    def __call__(context, request):
         """ Return a callable which returns an unauthorized WSGI
         application.  When the WSGI application is invoked, a
         ``message`` key in the WSGI environ provides information
-        pertaining to the reason for the unauthorized."""
-
-IUnauthorizedAppFactory = IForbiddenAppFactory
-deprecated('IUnauthorizedAppFactory',
-           '(repoze.bfg.interfaces.IUnauthorizedAppFactory should now be '
-           'imported as repoze.bfg.interfaces.IForbiddenAppFactory)',
-           )
+        pertaining to the reason for the unauthorized.  The
+        ``context`` passed to the forbidden app factory will be the
+        context found by the repoze.bfg router during traversal or url
+        dispatch.  The ``request`` will be the request object which
+        caused the deny. """
 
+class IUnauthorizedAppFactory(Interface):
+    """ A utility which returns an Unauthorized WSGI application
+    factory (deprecated in repoze.bfg 0.8.2) in favor of
+    IForbiddenAppFactory """
+    
 class IContextURL(Interface):
     """ An adapter which deals with URLs related to a context.
     """
diff --git a/repoze/bfg/router.py b/repoze/bfg/router.py
index 885a08184..2af5df4c7 100644
--- a/repoze/bfg/router.py
+++ b/repoze/bfg/router.py
@@ -18,6 +18,7 @@ from repoze.bfg.interfaces import IRoutesMapper
 from repoze.bfg.interfaces import ISecurityPolicy
 from repoze.bfg.interfaces import ISettings
 from repoze.bfg.interfaces import IForbiddenAppFactory
+from repoze.bfg.interfaces import IUnauthorizedAppFactory
 from repoze.bfg.interfaces import IView
 from repoze.bfg.interfaces import IViewPermission
 
@@ -55,7 +56,28 @@ class Router(object):
         self.request_factory = registry.queryUtility(IRequestFactory)
         security_policy = registry.queryUtility(ISecurityPolicy)
 
-        self.forbidden_app_factory = registry.queryUtility(IForbiddenAppFactory)
+        unauthorized_app_factory = registry.queryUtility(
+            IUnauthorizedAppFactory)
+
+        forbidden = None
+
+        if unauthorized_app_factory is not None:
+            warning = (
+                'Instead of registering a utility against the '
+                'repoze.bfg.interfaces.IUnauthorizedAppFactory interface '
+                'to return a custom forbidden response, you should now '
+                'register a "repoze.interfaces.IForbiddenAppFactory".  '
+                'The IUnauthorizedAppFactory interface was deprecated in '
+                'repoze.bfg 0.8.2 and will be removed in a subsequent version '
+                'of repoze.bfg.  See the "Hooks" chapter of the repoze.bfg '
+                'documentation for more information about '
+                'IForbiddenAppFactory.')
+            self.logger and self.logger.warn(warning)
+            def forbidden(context, request):
+                return unauthorized_app_factory()
+
+        self.forbidden_app_factory = registry.queryUtility(IForbiddenAppFactory,
+                                                           default=forbidden)
 
         if security_policy is not None:
             if hasattr(security_policy, 'forbidden'):
diff --git a/repoze/bfg/security.py b/repoze/bfg/security.py
index bd1edaf6d..ba89a80e3 100644
--- a/repoze/bfg/security.py
+++ b/repoze/bfg/security.py
@@ -147,7 +147,8 @@ class ACLSecurityPolicy(object):
 
         return []
 
-    forbidden = UnauthorizedApp
+    def forbidden(self, context, request):
+        return UnauthorizedApp()
 
 class InheritingACLSecurityPolicy(object):
     """ A security policy which uses ACLs in the following ways:
@@ -272,7 +273,8 @@ class InheritingACLSecurityPolicy(object):
 
         return allowed
 
-    forbidden = UnauthorizedApp
+    def forbidden(self, context, request):
+        return UnauthorizedApp()
 
 def get_remoteuser(request):
     user_id = request.environ.get('REMOTE_USER')
diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py
index d7c55ae78..99316f056 100644
--- a/repoze/bfg/tests/test_router.py
+++ b/repoze/bfg/tests/test_router.py
@@ -137,6 +137,22 @@ class RouterTests(unittest.TestCase):
         self.failUnless('which does not have a "forbidden" method'
                         in logger.messages[0])
 
+    def test_secpol_with_iunauthorized_appfactory(self):
+        from repoze.bfg.interfaces import IUnauthorizedAppFactory
+        environ = self._makeEnviron()
+        context = DummyContext()
+        self._registerTraverserFactory(context)
+        rootfactory = self._registerRootFactory(None)
+        logger = self._registerLogger()
+        secpol = self._registerSecurityPolicy()
+        def factory():
+            return 'yo'
+        self.registry.registerUtility(factory, IUnauthorizedAppFactory)
+        router = self._makeOne()
+        self.assertEqual(len(logger.messages), 1)
+        self.failUnless('IForbiddenAppFactory' in logger.messages[0])
+        self.assertEqual(router.forbidden_app_factory(None, None), 'yo')
+
     def test_inotfound_appfactory_override(self):
         from repoze.bfg.interfaces import INotFoundAppFactory
         def app():
diff --git a/repoze/bfg/tests/test_security.py b/repoze/bfg/tests/test_security.py
index b596a1547..ffac19e0d 100644
--- a/repoze/bfg/tests/test_security.py
+++ b/repoze/bfg/tests/test_security.py
@@ -245,7 +245,7 @@ class TestACLSecurityPolicy(unittest.TestCase):
 
     def test_forbidden(self):
         policy = self._makeOne(lambda *arg: None)
-        forbidden_app = policy.forbidden()
+        forbidden_app = policy.forbidden(None, None)
         environ = {}
         result = []
         def start_response(status, headers):
@@ -444,7 +444,7 @@ class TestInheritingACLSecurityPolicy(unittest.TestCase):
 
     def test_forbidden(self):
         policy = self._makeOne(lambda *arg: None)
-        forbidden_app = policy.forbidden()
+        forbidden_app = policy.forbidden(None, None)
         environ = {}
         result = []
         def start_response(status, headers):