From 7d1da854e77b56ab6e50f0b8a3e0e61d8ebfb7a7 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Mon, 25 May 2009 01:37:25 +0000 Subject: IForbiddenAppFactory -> IForbiddenResponseFactory. --- CHANGES.txt | 63 ++++++++++++++++++++------------------- docs/narr/hooks.rst | 42 ++++++++++++++------------ repoze/bfg/interfaces.py | 49 +++++++++++++++++------------- repoze/bfg/router.py | 25 +++++++++------- repoze/bfg/security.py | 63 ++++++++++++++++++++++++++++++++++++--- repoze/bfg/tests/test_router.py | 52 ++++++++++++++++++-------------- repoze/bfg/tests/test_security.py | 28 ++++++++--------- 7 files changed, 199 insertions(+), 123 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 090c0f412..6df0c8171 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -5,42 +5,44 @@ Features -------- - It is now possible to register a custom - ``repoze.bfg.interfaces.IForbiddenAppFactory`` for a given + ``repoze.bfg.interfaces.IForbiddenResponseFactory`` 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 + described in the Hooks chapter. The IForbiddenResponseFactory 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. + return an IResponse object (a webob response, basically). Read the + below point for more info and see the Hooks narrative chapter of the + BFG docs for more info; this registration trumps the security policy + ``forbidden`` feature when it is registered. - 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 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 - IForbiddenAppFactory (see the "Hooks" narrative chapter); the - explicitly registered IForbiddenAppFactory will be preferred over - the (more general) security policy forbidden app factory. + customized ``Forbidden`` response when BFG cannot authorize an + invocation of a view. To this end, ISecurityPolicy objects must 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 an IResponse (a webob response). The returned + response application should be appropriate when access to a view + resource was forbidden by the security policy (e.g. perhaps a login + page or a general "forbidden" 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`` method of a security policy is not used + if a developer has registered an IForbiddenResponseFactory (see the + "Hooks" narrative chapter); the explicitly registered + IForbiddenResponseFactory will be preferred over the (more general) + security policy forbidden method. - All default security policies now have a ``forbidden`` callable - attached to them. This particular callable returns a WSGI - application which generates a ``401 Unauthorized`` response for - backwards compatibility (had backwards compatibility not been an - issue, this callable would have returned a WSGI app that generated a - ``403 Forbidden`` response). + attached to them. This particular callable returns an IResponse (a + webob response) with a ``401 Unauthorized`` status for backwards + compatibility (had backwards compatibility not been an issue, this + callable would have returned a WSGI app that generated a ``403 + Forbidden`` response). Backwards Incompatibilities --------------------------- @@ -57,8 +59,7 @@ Deprecations - The ``repoze.bfg.interfaces.IUnauthorizedAppFactory`` interface has been deprecated in favor of using the new - ``repoze.bfg.interfaces.IForbiddenAppFactory`` mechanism. - + ``repoze.bfg.interfaces.IForbiddenResponseFactory`` mechanism. 0.8.1 (2009-05-21) ================== diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst index aefa95046..6428408e8 100644 --- a/docs/narr/hooks.rst +++ b/docs/narr/hooks.rst @@ -124,40 +124,44 @@ sample code that implements a minimal NotFound application factory: ``debug_notfound`` environment setting is true than it is when it is false. -Changing the Forbidden Application ----------------------------------- +Changing the Forbidden Response +------------------------------- When :mod:`repoze.bfg` can't authorize execution of a view based on -the security policy in use, it creates and invokes a Forbidden WSGI -application. The application it invokes can be customized by placing -something like the following ZCML in your ``configure.zcml`` file. +the security policy in use, it invokes a "forbidden response factory". +Usually this forbidden response factory is serviced by the currently +active :term:`security policy`, but it can be overridden as necessary +by placing something like the following ZCML in your +``configure.zcml`` file. .. code-block:: xml :linenos: - + Replace ``helloworld.factories.forbidden_app_factory`` with the Python -dotted name to the WSGI application factory you want to use. Here's -some sample code that implements a minimal Unauthorized application -factory: +dotted name to the forbidden response factory you want to use. The +response factory must accept two parameters: ``context`` and +``request``. The ``context`` is the context found by the router when +the view invocation was denied. The ``request`` is the current +:term:`request` representing the denied action. Here's some sample +code that implements a minimal forbidden response factory: .. code-block:: python from repoze.bfg.chameleon_zpt import render_template_to_response - def forbidden_app_factory(context, request): + def forbidden_response_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 - :mod:`repoze.bfg`. Within the WSGI environ will be a key named - ``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. A WebOb ``Response`` object is a valid WSGI - application, by the way. +.. note:: When an forbidden response factory is invoked, it is passed + the request as the second argument. An attribute of the request is + ``environ``, which is the WSGI environment. Within the WSGI + environ will be a key named ``repoze.bfg.message`` that has a value + explaining why the current view invocation was forbidden. This + error will be different when the ``debug_authorization`` + environment setting is true than it is when it is false. .. 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 78db34ced..78311962f 100644 --- a/repoze/bfg/interfaces.py +++ b/repoze/bfg/interfaces.py @@ -132,14 +132,21 @@ class ISecurityPolicy(Interface): ``NotImplementedError`` exception.""" 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 - invocation is denied due to a security policy deny. The WSGI - application should return a response appropriate when access - 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 + """ This method should return an IResponse object (an object + with the attributes ``status``, ``headerlist``, and + ``app_iter``) as a result of a view invocation denial. The + ``forbidden`` method of a security policy will be called by + ``repoze.bfg`` when view invocation is denied (usually as a + result of the ``permit`` method of the same security policy + returning False to the Router). + + The ``forbidden`` method of a security will not be called when + an ``IForbiddenResponseFactory`` utility is registered; + instead the ``IForbiddenResponseFactory`` utility will serve + the forbidden response. + + 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. 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 @@ -211,23 +218,25 @@ class INotFoundAppFactory(Interface): a``message`` key in the WSGI environ provides information pertaining to the reason for the notfound.""" -class IForbiddenAppFactory(Interface): - """ A utility which returns an Forbidden WSGI application - factory""" +class IForbiddenResponseFactory(Interface): + """ A utility which returns an IResponse as the result of the + denial of a view invocation by a security policy.""" 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. 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. """ + """ Return an object implementing IResponse (an object with + the status, headerlist, and app_iter attributes) as a result + of a view invocation denial by a security policy. + + Note that the ``message`` key in the WSGI environ + (request.environ) provides information pertaining to the + reason for the view invocation denial. 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 """ + IForbiddenResponseFactory """ 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 2af5df4c7..fdd5495fe 100644 --- a/repoze/bfg/router.py +++ b/repoze/bfg/router.py @@ -17,7 +17,7 @@ from repoze.bfg.interfaces import IRouter 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 IForbiddenResponseFactory from repoze.bfg.interfaces import IUnauthorizedAppFactory from repoze.bfg.interfaces import IView from repoze.bfg.interfaces import IViewPermission @@ -66,18 +66,21 @@ class Router(object): '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". ' + 'register a "repoze.interfaces.IForbiddenResponseFactory". ' '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.') + 'IForbiddenResponseFactory.') self.logger and self.logger.warn(warning) def forbidden(context, request): - return unauthorized_app_factory() + app = unauthorized_app_factory() + response = request.get_response(app) + return response - self.forbidden_app_factory = registry.queryUtility(IForbiddenAppFactory, - default=forbidden) + self.forbidden_resp_factory = registry.queryUtility( + IForbiddenResponseFactory, + default=forbidden) if security_policy is not None: if hasattr(security_policy, 'forbidden'): @@ -93,10 +96,10 @@ class Router(object): 'security policy without a "forbidden" method.' % security_policy) self.logger and self.logger.warn(warning) - # allow a specifically-registered IForbiddenAppFactory to + # allow a specifically-registered IForbiddenResponseFactory to # override the security policy's forbidden - self.forbidden_app_factory = (self.forbidden_app_factory or - security_policy_forbidden) + self.forbidden_resp_factory = (self.forbidden_resp_factory or + security_policy_forbidden) self.security_policy = security_policy self.notfound_app_factory = registry.queryUtility(INotFoundAppFactory, @@ -193,7 +196,9 @@ class Router(object): environ['repoze.bfg.message'] = msg - return self.forbidden_app_factory()(environ, start_response) + response = self.forbidden_resp_factory(context, request) + start_response(response.status, response.headerlist) + return response.app_iter response = registry.queryMultiAdapter( (context, request), IView, name=view_name) diff --git a/repoze/bfg/security.py b/repoze/bfg/security.py index ba89a80e3..81ecf88ee 100644 --- a/repoze/bfg/security.py +++ b/repoze/bfg/security.py @@ -1,3 +1,6 @@ +from webob import Response +from cgi import escape + from zope.component import queryUtility from zope.deprecation import deprecated from zope.interface import implements @@ -7,6 +10,7 @@ from repoze.bfg.location import lineage from repoze.bfg.interfaces import ISecurityPolicy from repoze.bfg.interfaces import IViewPermission from repoze.bfg.interfaces import IViewPermissionFactory +from repoze.bfg.interfaces import IResponseFactory from repoze.bfg.wsgi import Unauthorized as UnauthorizedApp @@ -78,7 +82,9 @@ def principals_allowed_by_permission(context, permission): class ACLSecurityPolicy(object): implements(ISecurityPolicy) - + + forbidden_status = '401 Unauthorized' # b/c, should be 403 + def __init__(self, get_principals): self.get_principals = get_principals @@ -148,7 +154,11 @@ class ACLSecurityPolicy(object): return [] def forbidden(self, context, request): - return UnauthorizedApp() + body, headerlist = _forbidden_html(request, self.forbidden_status) + response_factory = queryUtility(IResponseFactory, default=Response) + return response_factory(status = self.forbidden_status, + headerlist = headerlist, + app_iter = body) class InheritingACLSecurityPolicy(object): """ A security policy which uses ACLs in the following ways: @@ -194,7 +204,9 @@ class InheritingACLSecurityPolicy(object): ``authenticated_userid``). """ implements(ISecurityPolicy) - + + forbidden_status = '401 Unauthorized' # b/c, should be 403 + def __init__(self, get_principals): self.get_principals = get_principals @@ -274,7 +286,11 @@ class InheritingACLSecurityPolicy(object): return allowed def forbidden(self, context, request): - return UnauthorizedApp() + body, headerlist = _forbidden_html(request, self.forbidden_status) + response_factory = queryUtility(IResponseFactory, default=Response) + return response_factory(status = self.forbidden_status, + headerlist = headerlist, + app_iter = body) def get_remoteuser(request): user_id = request.environ.get('REMOTE_USER') @@ -491,6 +507,27 @@ def WhoInheritingACLSecurityPolicy(): """ return InheritingACLSecurityPolicy(get_who_principals) +## class StandaloneInheritingACLSecurityPolicy(InheritingACLSecurityPolicy): +## def __init__(self, get_principals, login_view_name='login_view', +## forbidden_view_name='forbidden_view'): +## self.get_principals = get_principals +## self.login_view_name = login_view_name +## self.forbidden_view_name = forbidden_view_name + +## def forbidden(self, context, request): +## from repoze.bfg.view import render_view_to_response +## from webob import Response + +## userid = self.authenticated_userid(request) + +## if userid is None: +## view_name = self.login_view_name +## else: +## view_name = self.forbidden_view_name + +## return render_view_to_response(context, request, name=view_name, +## secure=False) + class PermitsResult(int): def __new__(cls, s, *args): inst = int.__new__(cls, cls.boolval) @@ -603,3 +640,21 @@ class ViewPermissionFactory(object): class Unauthorized(Exception): pass +def _forbidden_html(request, status): + try: + msg = escape(request.environ['repoze.bfg.message']) + except KeyError: + msg = '' + html = """ + + %s + +

%s

+ %s + + + """ % (status, status, msg) + headers = [('Content-Length', str(len(html))), + ('Content-Type', 'text/html')] + return [html], headers + diff --git a/repoze/bfg/tests/test_router.py b/repoze/bfg/tests/test_router.py index 99316f056..1b76c0b7e 100644 --- a/repoze/bfg/tests/test_router.py +++ b/repoze/bfg/tests/test_router.py @@ -78,8 +78,9 @@ class RouterTests(unittest.TestCase): from repoze.bfg.interfaces import IViewPermission self.registry.registerAdapter(permission, for_, IViewPermission, name) - def _registerSecurityPolicy(self): - secpol = DummySecurityPolicy() + def _registerSecurityPolicy(self, secpol=None): + if secpol is None: + secpol = DummySecurityPolicy() from repoze.bfg.interfaces import ISecurityPolicy self.registry.registerUtility(secpol, ISecurityPolicy) return secpol @@ -130,8 +131,9 @@ class RouterTests(unittest.TestCase): self._registerTraverserFactory(context) rootfactory = self._registerRootFactory(None) logger = self._registerLogger() - secpol = self._registerSecurityPolicy() - del secpol.forbidden + class Dummy: + pass + self._registerSecurityPolicy(Dummy()) router = self._makeOne() self.assertEqual(len(logger.messages), 1) self.failUnless('which does not have a "forbidden" method' @@ -150,8 +152,12 @@ class RouterTests(unittest.TestCase): 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') + self.failUnless('IForbiddenResponseFactory' in logger.messages[0]) + class DummyRequest: + def get_response(self, app): + return app + req = DummyRequest() + self.assertEqual(router.forbidden_resp_factory(None, req), 'yo') def test_inotfound_appfactory_override(self): from repoze.bfg.interfaces import INotFoundAppFactory @@ -162,31 +168,31 @@ class RouterTests(unittest.TestCase): router = self._makeOne() self.assertEqual(router.notfound_app_factory, app) - def test_iforbidden_appfactory_override_withsecpol(self): - from repoze.bfg.interfaces import IForbiddenAppFactory + def test_iforbidden_respfactory_override_withsecpol(self): + from repoze.bfg.interfaces import IForbiddenResponseFactory def app(): """ """ - self.registry.registerUtility(app, IForbiddenAppFactory) + self.registry.registerUtility(app, IForbiddenResponseFactory) self._registerSecurityPolicy() self._registerRootFactory(None) router = self._makeOne() - self.assertEqual(router.forbidden_app_factory, app) + self.assertEqual(router.forbidden_resp_factory, app) - def test_iforbidden_appfactory_override_nosecpol(self): - from repoze.bfg.interfaces import IForbiddenAppFactory + def test_iforbidden_responsefactory_override_nosecpol(self): + from repoze.bfg.interfaces import IForbiddenResponseFactory def app(): """ """ - self.registry.registerUtility(app, IForbiddenAppFactory) + self.registry.registerUtility(app, IForbiddenResponseFactory) self._registerRootFactory(None) router = self._makeOne() - self.assertEqual(router.forbidden_app_factory, app) + self.assertEqual(router.forbidden_resp_factory, app) - def test_iforbidden_appfactory_nooverride(self): + def test_iforbidden_responsefactory_nooverride(self): secpol = self._registerSecurityPolicy() context = DummyContext() self._registerRootFactory(None) router = self._makeOne() - self.assertEqual(router.forbidden_app_factory, secpol.forbidden) + self.assertEqual(router.forbidden_resp_factory, secpol.forbidden) def test_call_no_view_registered_no_isettings(self): environ = self._makeEnviron() @@ -782,11 +788,11 @@ class DummyResponse: app_iter = () class DummySecurityPolicy: - def __init__(self): - def wsgiapp(environ, start_response): - self.environ = environ - self.start_response = start_response - start_response('401 Unauthorized', []) - return 'Unauthorized' - self.forbidden = lambda *x: wsgiapp + def forbidden(self, context, request): + self.request = request + ob = DummyResponse() + ob.status = '401 Unauthorized' + ob.app_iter = ['Unauthorized'] + ob.headerlist = () + return ob diff --git a/repoze/bfg/tests/test_security.py b/repoze/bfg/tests/test_security.py index ffac19e0d..b9f9624d4 100644 --- a/repoze/bfg/tests/test_security.py +++ b/repoze/bfg/tests/test_security.py @@ -245,14 +245,12 @@ class TestACLSecurityPolicy(unittest.TestCase): def test_forbidden(self): policy = self._makeOne(lambda *arg: None) - forbidden_app = policy.forbidden(None, None) - environ = {} - result = [] - def start_response(status, headers): - result.append((status, headers)) - response = forbidden_app(environ, start_response) - self.assertEqual(result[0][0], '401 Unauthorized') - self.failUnless(len(result[0][1]), 2) # headers + context = DummyContext() + request = DummyRequest({}) + response = policy.forbidden(context, request) + self.failUnless('401 Unauthorized' in response.app_iter[0]) + self.assertEqual(response.status, '401 Unauthorized') + self.assertEqual(len(response.headerlist), 2) class TestInheritingACLSecurityPolicy(unittest.TestCase): @@ -444,14 +442,12 @@ class TestInheritingACLSecurityPolicy(unittest.TestCase): def test_forbidden(self): policy = self._makeOne(lambda *arg: None) - forbidden_app = policy.forbidden(None, None) - environ = {} - result = [] - def start_response(status, headers): - result.append((status, headers)) - response = forbidden_app(environ, start_response) - self.assertEqual(result[0][0], '401 Unauthorized') - self.failUnless(len(result[0][1]), 2) # headers + context = DummyContext() + request = DummyRequest({}) + response = policy.forbidden(context, request) + self.failUnless('401 Unauthorized' in response.app_iter[0]) + self.assertEqual(response.status, '401 Unauthorized') + self.assertEqual(len(response.headerlist), 2) class TestAllPermissionsList(unittest.TestCase): def setUp(self): -- cgit v1.2.3