From 0b262906f14975a6e6158635ff7568e49c5258bb Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Fri, 1 Apr 2011 02:12:04 -0400 Subject: - Static views registered with ``config.add_static_view`` which also included a ``permission`` keyword argument would not work as expected, because ``add_static_view`` also registered a route factory internally. Because a route factory was registered internally, the context checked by the Pyramid permission machinery never had an ACL. ``add_static_view`` no longer registers a route with a factory, so the default root factory will be used. - ``config.add_static_view`` now passes extra keyword arguments it receives to ``config.add_route`` (calling add_static_view is mostly logically equivalent to adding a view of the type ``pyramid.static.static_view`` hooked up to a route with a subpath). This makes it possible to pass e.g., ``factory=`` to ``add_static_view`` to protect a particular static view with a custom ACL. Closes #161. --- CHANGES.txt | 14 ++++++++++++++ pyramid/config.py | 8 +++++++- pyramid/static.py | 30 ++++++++++++++++++++---------- pyramid/tests/staticpermapp/__init__.py | 25 +++++++++++++++++++++++++ pyramid/tests/test_config.py | 9 +++------ pyramid/tests/test_integration.py | 31 +++++++++++++++++++++++++++++++ pyramid/tests/test_static.py | 2 -- 7 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 pyramid/tests/staticpermapp/__init__.py diff --git a/CHANGES.txt b/CHANGES.txt index c731306e2..4fc218c0c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -48,6 +48,20 @@ Bug Fixes ``pyramid.view.append_slash_notfound_view`` (see https://github.com/Pylons/pyramid/issues#issue/149). +- Static views registered with ``config.add_static_view`` which also included + a ``permission`` keyword argument would not work as expected, because + ``add_static_view`` also registered a route factory internally. Because a + route factory was registered internally, the context checked by the Pyramid + permission machinery never had an ACL. ``add_static_view`` no longer + registers a route with a factory, so the default root factory will be used. + +- ``config.add_static_view`` now passes extra keyword arguments it receives + to ``config.add_route`` (calling add_static_view is mostly logically + equivalent to adding a view of the type ``pyramid.static.static_view`` + hooked up to a route with a subpath). This makes it possible to pass e.g., + ``factory=`` to ``add_static_view`` to protect a particular static view + with a custom ACL. + 1.0 (2011-01-30) ================ diff --git a/pyramid/config.py b/pyramid/config.py index 17bc972d1..aea58dddf 100644 --- a/pyramid/config.py +++ b/pyramid/config.py @@ -2176,7 +2176,13 @@ class Configurator(object): current application, the static view should be renderered to completely anonymous users. This default value is permissive because, in most web apps, static assets seldom need protection from - viewing. + viewing. If ``permission`` is specified, the security checking will + be performed against the default root factory ACL. + + Any other keyword arguments sent to ``add_static_view`` are passed on + to :meth:`pyramid.config.Configuration.add_route` (e.g. ``factory``, + perhaps to define a custom factory with a custom ACL for this static + view). *Usage* diff --git a/pyramid/static.py b/pyramid/static.py index eec465949..3866126ac 100644 --- a/pyramid/static.py +++ b/pyramid/static.py @@ -137,17 +137,27 @@ class StaticURLInfo(object): else: # it's a view name cache_max_age = extra.pop('cache_max_age', None) + # create a view view = static_view(spec, cache_max_age=cache_max_age) - # register a route using this view - permission = extra.pop('permission', '__no_permission_required__') - self.config.add_route( - name, - "%s*subpath" % name, # name already ends with slash - view=view, - view_for=self.__class__, - view_permission=permission, - factory=lambda *x: self, - ) + + # Mutate extra to allow factory, etc to be passed through here. + # Treat permission specially because we'd like to default to + # permissiveness (see docs of config.add_static_view). We need + # to deal with both ``view_permission`` and ``permission`` + # because ``permission`` is used in the docs for add_static_view, + # but ``add_route`` prefers ``view_permission`` + permission = extra.pop('view_permission', None) + if permission is None: + permission = extra.pop('permission', None) + if permission is None: + permission = '__no_permission_required__' + extra['view_permission'] = permission + extra['view'] = view + + # register a route using the computed view, permission, and pattern, + # plus any extras passed to us via add_static_view + pattern = "%s*subpath" % name # name already ends with slash + self.config.add_route(name, pattern, **extra) self.registrations.append((name, spec, False)) class static_view(object): diff --git a/pyramid/tests/staticpermapp/__init__.py b/pyramid/tests/staticpermapp/__init__.py new file mode 100644 index 000000000..cc690d937 --- /dev/null +++ b/pyramid/tests/staticpermapp/__init__.py @@ -0,0 +1,25 @@ +class RootFactory(object): + __acl__ = [('Allow', 'fred', 'view')] + def __init__(self, request): + pass + +class LocalRootFactory(object): + __acl__ = [('Allow', 'bob', 'view')] + def __init__(self, request): + pass + + +def includeme(config): + from pyramid.authentication import RemoteUserAuthenticationPolicy + from pyramid.authorization import ACLAuthorizationPolicy + authn_policy = RemoteUserAuthenticationPolicy() + authz_policy = ACLAuthorizationPolicy() + config._set_authentication_policy(authn_policy) + config._set_authorization_policy(authz_policy) + config.add_static_view('allowed', 'pyramid.tests:fixtures/static/') + config.add_static_view('protected', 'pyramid.tests:fixtures/static/', + permission='view') + config.add_static_view('factory_protected', + 'pyramid.tests:fixtures/static/', + permission='view', + factory=LocalRootFactory) diff --git a/pyramid/tests/test_config.py b/pyramid/tests/test_config.py index e0eb2e3be..202dce7bf 100644 --- a/pyramid/tests/test_config.py +++ b/pyramid/tests/test_config.py @@ -2252,19 +2252,16 @@ class ConfiguratorTests(unittest.TestCase): self.assertEqual(config._ctx.info, 'abc') def test_add_static_here_no_utility_registered(self): + from zope.interface import Interface from pyramid.static import PackageURLParser - from zope.interface import implementedBy - from pyramid.static import StaticURLInfo from pyramid.interfaces import IView from pyramid.interfaces import IViewClassifier config = self._makeOne(autocommit=True) config.add_static_view('static', 'fixtures/static') request_type = self._getRouteRequestIface(config, 'static/') - route = self._assertRoute(config, 'static/', 'static/*subpath') - self.assertEqual(route.factory.__class__, type(lambda x: x)) - iface = implementedBy(StaticURLInfo) + self._assertRoute(config, 'static/', 'static/*subpath') wrapped = config.registry.adapters.lookup( - (IViewClassifier, request_type, iface), IView, name='') + (IViewClassifier, request_type, Interface), IView, name='') request = self._makeRequest(config) self.assertEqual(wrapped(None, request).__class__, PackageURLParser) diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index a504db982..6d5d7d675 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -102,6 +102,37 @@ class TestFixtureApp(IntegrationBase): def test_protected(self): self.testapp.get('/protected.html', status=403) +class TestStaticPermApp(IntegrationBase): + package = 'pyramid.tests.staticpermapp' + root_factory = 'pyramid.tests.staticpermapp:RootFactory' + def test_allowed(self): + result = self.testapp.get('/allowed/index.html', status=200) + self.assertEqual( + result.body.replace('\r', ''), + open(os.path.join(here, 'fixtures/static/index.html'), 'r').read()) + + def test_denied_via_acl_global_root_factory(self): + self.testapp.extra_environ = {'REMOTE_USER':'bob'} + self.testapp.get('/protected/index.html', status=403) + + def test_allowed_via_acl_global_root_factory(self): + self.testapp.extra_environ = {'REMOTE_USER':'fred'} + result = self.testapp.get('/protected/index.html', status=200) + self.assertEqual( + result.body.replace('\r', ''), + open(os.path.join(here, 'fixtures/static/index.html'), 'r').read()) + + def test_denied_via_acl_local_root_factory(self): + self.testapp.extra_environ = {'REMOTE_USER':'fred'} + self.testapp.get('/factory_protected/index.html', status=403) + + def test_allowed_via_acl_local_root_factory(self): + self.testapp.extra_environ = {'REMOTE_USER':'bob'} + result = self.testapp.get('/factory_protected/index.html', status=200) + self.assertEqual( + result.body.replace('\r', ''), + open(os.path.join(here, 'fixtures/static/index.html'), 'r').read()) + class TestCCBug(IntegrationBase): # "unordered" as reported in IRC by author of # http://labs.creativecommons.org/2010/01/13/cc-engine-and-web-non-frameworks/ diff --git a/pyramid/tests/test_static.py b/pyramid/tests/test_static.py index 33bd51d31..acf5a754b 100644 --- a/pyramid/tests/test_static.py +++ b/pyramid/tests/test_static.py @@ -316,8 +316,6 @@ class TestStaticURLInfo(unittest.TestCase): expected = [('view/', 'anotherpackage:path/', False)] self.assertEqual(inst.registrations, expected) self.assertEqual(config.arg, ('view/', 'view/*subpath')) - self.assertEqual(config.kw['view_for'], self._getTargetClass()) - self.assertEqual(config.kw['factory'](), inst) self.assertEqual(config.kw['view_permission'], '__no_permission_required__') self.assertEqual(config.kw['view'].__class__, static_view) -- cgit v1.2.3