Change the semantics of IForbiddenAppFactory.

5 files changed, 62 insertions, 16 deletions

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):