From 14be695bd7d187e162145a28ac07fe341dae3208 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 28 Mar 2017 01:29:33 -0500 Subject: rewrite low-level pyramid config functions to use plaster --- docs/api/paster.rst | 6 +- pyramid/paster.py | 59 +++++------- pyramid/scripts/common.py | 27 +----- pyramid/tests/test_paster.py | 180 +++++++++++++----------------------- pyramid/tests/test_scripts/dummy.py | 41 ++++++++ setup.py | 2 + 6 files changed, 137 insertions(+), 178 deletions(-) diff --git a/docs/api/paster.rst b/docs/api/paster.rst index 27bc81a1f..f0784d0f8 100644 --- a/docs/api/paster.rst +++ b/docs/api/paster.rst @@ -7,8 +7,8 @@ .. autofunction:: bootstrap - .. autofunction:: get_app(config_uri, name=None, options=None) + .. autofunction:: get_app - .. autofunction:: get_appsettings(config_uri, name=None, options=None) + .. autofunction:: get_appsettings - .. autofunction:: setup_logging(config_uri, global_conf=None) + .. autofunction:: setup_logging diff --git a/pyramid/paster.py b/pyramid/paster.py index 5429a7860..f7544f0c5 100644 --- a/pyramid/paster.py +++ b/pyramid/paster.py @@ -1,14 +1,17 @@ -import os +from pyramid.scripting import prepare +from pyramid.scripts.common import get_config_loader -from paste.deploy import ( - loadapp, - appconfig, - ) +def setup_logging(config_uri, global_conf=None): + """ + Set up Python logging with the filename specified via ``config_uri`` + (a string in the form ``filename#sectionname``). -from pyramid.scripting import prepare -from pyramid.scripts.common import setup_logging # noqa, api + Extra defaults can optionally be specified as a dict in ``global_conf``. + """ + loader = get_config_loader(config_uri) + loader.setup_logging(global_conf) -def get_app(config_uri, name=None, options=None, loadapp=loadapp): +def get_app(config_uri, name=None, options=None): """ Return the WSGI application named ``name`` in the PasteDeploy config file specified by ``config_uri``. @@ -18,20 +21,13 @@ def get_app(config_uri, name=None, options=None, loadapp=loadapp): If the ``name`` is None, this will attempt to parse the name from the ``config_uri`` string expecting the format ``inifile#name``. - If no name is found, the name will default to "main".""" - path, section = _getpathsec(config_uri, name) - config_name = 'config:%s' % path - here_dir = os.getcwd() + If no name is found, the name will default to "main". - app = loadapp( - config_name, - name=section, - relative_to=here_dir, - global_conf=options) - - return app + """ + loader = get_config_loader(config_uri) + return loader.get_wsgi_app(name, options) -def get_appsettings(config_uri, name=None, options=None, appconfig=appconfig): +def get_appsettings(config_uri, name=None, options=None): """ Return a dictionary representing the key/value pairs in an ``app`` section within the file represented by ``config_uri``. @@ -41,24 +37,11 @@ def get_appsettings(config_uri, name=None, options=None, appconfig=appconfig): If the ``name`` is None, this will attempt to parse the name from the ``config_uri`` string expecting the format ``inifile#name``. - If no name is found, the name will default to "main".""" - path, section = _getpathsec(config_uri, name) - config_name = 'config:%s' % path - here_dir = os.getcwd() - return appconfig( - config_name, - name=section, - relative_to=here_dir, - global_conf=options) - -def _getpathsec(config_uri, name): - if '#' in config_uri: - path, section = config_uri.split('#', 1) - else: - path, section = config_uri, 'main' - if name: - section = name - return path, section + If no name is found, the name will default to "main". + + """ + loader = get_config_loader(config_uri) + return loader.get_wsgi_app_settings(name, options) def bootstrap(config_uri, request=None, options=None): """ Load a WSGI application from the PasteDeploy config file specified diff --git a/pyramid/scripts/common.py b/pyramid/scripts/common.py index fc141f6e2..f4b8027db 100644 --- a/pyramid/scripts/common.py +++ b/pyramid/scripts/common.py @@ -1,6 +1,4 @@ -import os -from pyramid.compat import configparser -from logging.config import fileConfig +import plaster def parse_vars(args): """ @@ -17,26 +15,9 @@ def parse_vars(args): result[name] = value return result -def setup_logging(config_uri, global_conf=None, - fileConfig=fileConfig, - configparser=configparser): +def get_config_loader(config_uri): """ - Set up logging via :func:`logging.config.fileConfig` with the filename - specified via ``config_uri`` (a string in the form - ``filename#sectionname``). + Find a ``plaster.ILoader`` object supporting the "wsgi" protocol. - ConfigParser defaults are specified for the special ``__file__`` - and ``here`` variables, similar to PasteDeploy config loading. - Extra defaults can optionally be specified as a dict in ``global_conf``. """ - path = config_uri.split('#', 1)[0] - parser = configparser.ConfigParser() - parser.read([path]) - if parser.has_section('loggers'): - config_file = os.path.abspath(path) - full_global_conf = dict( - __file__=config_file, - here=os.path.dirname(config_file)) - if global_conf: - full_global_conf.update(global_conf) - return fileConfig(config_file, full_global_conf) + return plaster.get_loader(config_uri, protocols=['wsgi']) diff --git a/pyramid/tests/test_paster.py b/pyramid/tests/test_paster.py index 22a5cde3d..fc0cf20a5 100644 --- a/pyramid/tests/test_paster.py +++ b/pyramid/tests/test_paster.py @@ -1,58 +1,32 @@ import os import unittest +from pyramid.tests.test_scripts.dummy import DummyLoader here = os.path.dirname(__file__) class Test_get_app(unittest.TestCase): - def _callFUT(self, config_file, section_name, **kw): - from pyramid.paster import get_app - return get_app(config_file, section_name, **kw) + def _callFUT(self, config_file, section_name, options=None, _loader=None): + import pyramid.paster + old_loader = pyramid.paster.get_config_loader + try: + if _loader is not None: + pyramid.paster.get_config_loader = _loader + return pyramid.paster.get_app(config_file, section_name, + options=options) + finally: + pyramid.paster.get_config_loader = old_loader def test_it(self): app = DummyApp() - loadapp = DummyLoadWSGI(app) - result = self._callFUT('/foo/bar/myapp.ini', 'myapp', loadapp=loadapp) - self.assertEqual(loadapp.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(loadapp.section_name, 'myapp') - self.assertEqual(loadapp.relative_to, os.getcwd()) - self.assertEqual(result, app) - - def test_it_with_hash(self): - app = DummyApp() - loadapp = DummyLoadWSGI(app) - result = self._callFUT( - '/foo/bar/myapp.ini#myapp', None, loadapp=loadapp - ) - self.assertEqual(loadapp.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(loadapp.section_name, 'myapp') - self.assertEqual(loadapp.relative_to, os.getcwd()) - self.assertEqual(result, app) - - def test_it_with_hash_and_name_override(self): - app = DummyApp() - loadapp = DummyLoadWSGI(app) - result = self._callFUT( - '/foo/bar/myapp.ini#myapp', 'yourapp', loadapp=loadapp - ) - self.assertEqual(loadapp.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(loadapp.section_name, 'yourapp') - self.assertEqual(loadapp.relative_to, os.getcwd()) - self.assertEqual(result, app) - - def test_it_with_options(self): - app = DummyApp() - loadapp = DummyLoadWSGI(app) - options = {'a':1} + loader = DummyLoader(app=app) result = self._callFUT( - '/foo/bar/myapp.ini#myapp', - 'yourapp', - loadapp=loadapp, - options=options, - ) - self.assertEqual(loadapp.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(loadapp.section_name, 'yourapp') - self.assertEqual(loadapp.relative_to, os.getcwd()) - self.assertEqual(loadapp.kw, {'global_conf':options}) + '/foo/bar/myapp.ini', 'myapp', options={'a': 'b'}, + _loader=loader) + self.assertEqual(loader.uri, '/foo/bar/myapp.ini') + self.assertEqual(len(loader.calls), 1) + self.assertEqual(loader.calls[0]['op'], 'app') + self.assertEqual(loader.calls[0]['name'], 'myapp') + self.assertEqual(loader.calls[0]['defaults'], {'a': 'b'}) self.assertEqual(result, app) def test_it_with_dummyapp_requiring_options(self): @@ -63,38 +37,28 @@ class Test_get_app(unittest.TestCase): self.assertEqual(app.settings['foo'], 'baz') class Test_get_appsettings(unittest.TestCase): - def _callFUT(self, config_file, section_name, **kw): - from pyramid.paster import get_appsettings - return get_appsettings(config_file, section_name, **kw) + def _callFUT(self, config_file, section_name, options=None, _loader=None): + import pyramid.paster + old_loader = pyramid.paster.get_config_loader + try: + if _loader is not None: + pyramid.paster.get_config_loader = _loader + return pyramid.paster.get_appsettings(config_file, section_name, + options=options) + finally: + pyramid.paster.get_config_loader = old_loader def test_it(self): - values = {'a':1} - appconfig = DummyLoadWSGI(values) - result = self._callFUT('/foo/bar/myapp.ini', 'myapp', - appconfig=appconfig) - self.assertEqual(appconfig.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(appconfig.section_name, 'myapp') - self.assertEqual(appconfig.relative_to, os.getcwd()) - self.assertEqual(result, values) - - def test_it_with_hash(self): - values = {'a':1} - appconfig = DummyLoadWSGI(values) - result = self._callFUT('/foo/bar/myapp.ini#myapp', None, - appconfig=appconfig) - self.assertEqual(appconfig.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(appconfig.section_name, 'myapp') - self.assertEqual(appconfig.relative_to, os.getcwd()) - self.assertEqual(result, values) - - def test_it_with_hash_and_name_override(self): - values = {'a':1} - appconfig = DummyLoadWSGI(values) - result = self._callFUT('/foo/bar/myapp.ini#myapp', 'yourapp', - appconfig=appconfig) - self.assertEqual(appconfig.config_name, 'config:/foo/bar/myapp.ini') - self.assertEqual(appconfig.section_name, 'yourapp') - self.assertEqual(appconfig.relative_to, os.getcwd()) + values = {'a': 1} + loader = DummyLoader(app_settings=values) + result = self._callFUT( + '/foo/bar/myapp.ini', 'myapp', options={'a': 'b'}, + _loader=loader) + self.assertEqual(loader.uri, '/foo/bar/myapp.ini') + self.assertEqual(len(loader.calls), 1) + self.assertEqual(loader.calls[0]['op'], 'app_settings') + self.assertEqual(loader.calls[0]['name'], 'myapp') + self.assertEqual(loader.calls[0]['defaults'], {'a': 'b'}) self.assertEqual(result, values) def test_it_with_dummyapp_requiring_options(self): @@ -105,40 +69,39 @@ class Test_get_appsettings(unittest.TestCase): self.assertEqual(result['foo'], 'baz') class Test_setup_logging(unittest.TestCase): - def _callFUT(self, config_file, global_conf=None): - from pyramid.paster import setup_logging - dummy_cp = DummyConfigParserModule - return setup_logging( - config_uri=config_file, - global_conf=global_conf, - fileConfig=self.fileConfig, - configparser=dummy_cp, - ) + def _callFUT(self, config_file, global_conf=None, _loader=None): + import pyramid.paster + old_loader = pyramid.paster.get_config_loader + try: + if _loader is not None: + pyramid.paster.get_config_loader = _loader + return pyramid.paster.setup_logging(config_file, global_conf) + finally: + pyramid.paster.get_config_loader = old_loader def test_it_no_global_conf(self): - config_file, dict = self._callFUT('/abc') - # os.path.abspath is a sop to Windows - self.assertEqual(config_file, os.path.abspath('/abc')) - self.assertEqual(dict['__file__'], os.path.abspath('/abc')) - self.assertEqual(dict['here'], os.path.abspath('/')) + loader = DummyLoader() + self._callFUT('/abc', _loader=loader) + self.assertEqual(loader.uri, '/abc') + self.assertEqual(len(loader.calls), 1) + self.assertEqual(loader.calls[0]['op'], 'logging') + self.assertEqual(loader.calls[0]['defaults'], None) def test_it_global_conf_empty(self): - config_file, dict = self._callFUT('/abc', global_conf={}) - # os.path.abspath is a sop to Windows - self.assertEqual(config_file, os.path.abspath('/abc')) - self.assertEqual(dict['__file__'], os.path.abspath('/abc')) - self.assertEqual(dict['here'], os.path.abspath('/')) + loader = DummyLoader() + self._callFUT('/abc', global_conf={}, _loader=loader) + self.assertEqual(loader.uri, '/abc') + self.assertEqual(len(loader.calls), 1) + self.assertEqual(loader.calls[0]['op'], 'logging') + self.assertEqual(loader.calls[0]['defaults'], {}) def test_it_global_conf_not_empty(self): - config_file, dict = self._callFUT('/abc', global_conf={'key': 'val'}) - # os.path.abspath is a sop to Windows - self.assertEqual(config_file, os.path.abspath('/abc')) - self.assertEqual(dict['__file__'], os.path.abspath('/abc')) - self.assertEqual(dict['here'], os.path.abspath('/')) - self.assertEqual(dict['key'], 'val') - - def fileConfig(self, config_file, dict): - return config_file, dict + loader = DummyLoader() + self._callFUT('/abc', global_conf={'key': 'val'}, _loader=loader) + self.assertEqual(loader.uri, '/abc') + self.assertEqual(len(loader.calls), 1) + self.assertEqual(loader.calls[0]['op'], 'logging') + self.assertEqual(loader.calls[0]['defaults'], {'key': 'val'}) class Test_bootstrap(unittest.TestCase): def _callFUT(self, config_uri, request=None): @@ -187,17 +150,6 @@ class DummyRegistry(object): dummy_registry = DummyRegistry() -class DummyLoadWSGI: - def __init__(self, result): - self.result = result - - def __call__(self, config_name, name=None, relative_to=None, **kw): - self.config_name = config_name - self.section_name = name - self.relative_to = relative_to - self.kw = kw - return self.result - class DummyApp: def __init__(self): self.registry = dummy_registry diff --git a/pyramid/tests/test_scripts/dummy.py b/pyramid/tests/test_scripts/dummy.py index ced09d0b0..b904adfbd 100644 --- a/pyramid/tests/test_scripts/dummy.py +++ b/pyramid/tests/test_scripts/dummy.py @@ -162,3 +162,44 @@ class DummyPkgResources(object): def iter_entry_points(self, name): return self.entry_points + + +class dummy_setup_logging(object): + def __call__(self, config_uri, global_conf): + self.config_uri = config_uri + self.global_conf = global_conf + + +class DummyLoader(object): + def __init__(self, settings=None, app_settings=None, app=None): + if not settings: + settings = {} + if not app_settings: + app_settings = {} + self.settings = settings + self.app_settings = app_settings + self.app = app + self.calls = [] + + def __call__(self, uri): + self.uri = uri + return self + + def add_call(self, op, name, defaults): + self.calls.append({'op': op, 'name': name, 'defaults': defaults}) + + def get_settings(self, name=None, defaults=None): + self.add_call('settings', name, defaults) + return self.result + + def get_wsgi_app(self, name=None, defaults=None): + self.add_call('app', name, defaults) + return self.app + + def get_wsgi_app_settings(self, name=None, defaults=None): + self.add_call('app_settings', name, defaults) + return self.app_settings + + def setup_logging(self, defaults): + self.add_call('logging', None, defaults) + self.defaults = defaults diff --git a/setup.py b/setup.py index ff3e38874..046341f13 100644 --- a/setup.py +++ b/setup.py @@ -34,10 +34,12 @@ install_requires = [ 'venusian >= 1.0a3', # ``ignore`` 'translationstring >= 0.4', # py3 compat 'PasteDeploy >= 1.5.0', # py3 compat + 'plaster', 'hupper', ] tests_require = [ + 'plaster_pastedeploy', 'WebTest >= 1.3.1', # py3 compat 'zope.component >= 4.0', # py3 compat ] -- cgit v1.2.3 From 6787903d36d2f011f921f7e61b502b1db94e833a Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 29 Mar 2017 00:04:53 -0500 Subject: update prequest, proutes, ptweens and pviews Also ensure that each script is invoking setup_logging. --- pyramid/scripts/prequest.py | 19 ++++---- pyramid/scripts/proutes.py | 35 +++++++------- pyramid/scripts/ptweens.py | 8 +++- pyramid/scripts/pviews.py | 10 ++-- pyramid/tests/test_scripts/dummy.py | 4 +- pyramid/tests/test_scripts/test_prequest.py | 62 +++++------------------- pyramid/tests/test_scripts/test_proutes.py | 74 ++++++++++++++--------------- pyramid/tests/test_scripts/test_ptweens.py | 3 +- pyramid/tests/test_scripts/test_pviews.py | 3 +- 9 files changed, 93 insertions(+), 125 deletions(-) diff --git a/pyramid/scripts/prequest.py b/pyramid/scripts/prequest.py index 66feff624..f0681afd7 100644 --- a/pyramid/scripts/prequest.py +++ b/pyramid/scripts/prequest.py @@ -5,9 +5,8 @@ import textwrap from pyramid.compat import url_unquote from pyramid.request import Request -from pyramid.paster import get_app +from pyramid.scripts.common import get_config_loader from pyramid.scripts.common import parse_vars -from pyramid.scripts.common import setup_logging def main(argv=sys.argv, quiet=False): command = PRequestCommand(argv, quiet) @@ -110,7 +109,7 @@ class PRequestCommand(object): "passed here.", ) - get_app = staticmethod(get_app) + _get_config_loader = staticmethod(get_config_loader) stdin = sys.stdin def __init__(self, argv, quiet=False): @@ -121,17 +120,18 @@ class PRequestCommand(object): if not self.quiet: print(msg) - def configure_logging(self, app_spec): - setup_logging(app_spec) - def run(self): if not self.args.config_uri or not self.args.path_info: self.out('You must provide at least two arguments') return 2 - app_spec = self.args.config_uri + config_uri = self.args.config_uri + config_vars = parse_vars(self.args.config_vars) path = self.args.path_info - self.configure_logging(app_spec) + loader = self._get_config_loader(config_uri) + loader.setup_logging(config_vars) + + app = loader.get_wsgi_app(self.args.app_name, config_vars) if not path.startswith('/'): path = '/' + path @@ -158,9 +158,6 @@ class PRequestCommand(object): name, value = item.split(':', 1) headers[name] = value.strip() - app = self.get_app(app_spec, self.args.app_name, - options=parse_vars(self.args.config_vars)) - request_method = (self.args.method or 'GET').upper() environ = { diff --git a/pyramid/scripts/proutes.py b/pyramid/scripts/proutes.py index 80c8238a2..69d61ae8f 100644 --- a/pyramid/scripts/proutes.py +++ b/pyramid/scripts/proutes.py @@ -7,10 +7,11 @@ import re from zope.interface import Interface from pyramid.paster import bootstrap -from pyramid.compat import (string_types, configparser) +from pyramid.compat import string_types from pyramid.interfaces import IRouteRequest from pyramid.config import not_ +from pyramid.scripts.common import get_config_loader from pyramid.scripts.common import parse_vars from pyramid.static import static_view from pyramid.view import _find_views @@ -175,7 +176,6 @@ def get_route_data(route, registry): (route.name, route_intr['external_url'], UNKNOWN_KEY, ANY_KEY) ] - route_request_methods = route_intr['request_methods'] view_intr = registry.introspector.related(route_intr) @@ -245,9 +245,9 @@ class PRoutesCommand(object): will be assumed. Example: 'proutes myapp.ini'. """ - bootstrap = (bootstrap,) + bootstrap = staticmethod(bootstrap) # testing + get_config_loader = staticmethod(get_config_loader) # testing stdout = sys.stdout - ConfigParser = configparser.ConfigParser # testing parser = argparse.ArgumentParser( description=textwrap.dedent(description), formatter_class=argparse.RawDescriptionHelpFormatter, @@ -308,18 +308,12 @@ class PRoutesCommand(object): return True - def proutes_file_config(self, filename): - config = self.ConfigParser() - config.read(filename) - try: - items = config.items('proutes') - for k, v in items: - if 'format' == k: - cols = re.split(r'[,|\s\n]+', v) - self.column_format = [x.strip() for x in cols] - - except configparser.NoSectionError: - return + def proutes_file_config(self, loader, global_conf=None): + settings = loader.get_settings('proutes', global_conf) + format = settings.get('format') + if format: + cols = re.split(r'[,|\s\n]+', format) + self.column_format = [x.strip() for x in cols] def out(self, msg): # pragma: no cover if not self.quiet: @@ -336,12 +330,15 @@ class PRoutesCommand(object): return 2 config_uri = self.args.config_uri - env = self.bootstrap[0](config_uri, options=parse_vars(self.args.config_vars)) + config_vars = parse_vars(self.args.config_vars) + loader = self.get_config_loader(config_uri) + loader.setup_logging(config_vars) + self.proutes_file_config(loader, config_vars) + + env = self.bootstrap(config_uri, options=config_vars) registry = env['registry'] mapper = self._get_mapper(registry) - self.proutes_file_config(config_uri) - if self.args.format: columns = self.args.format.split(',') self.column_format = [x.strip() for x in columns] diff --git a/pyramid/scripts/ptweens.py b/pyramid/scripts/ptweens.py index 5ca77e52a..d5cbebe12 100644 --- a/pyramid/scripts/ptweens.py +++ b/pyramid/scripts/ptweens.py @@ -7,6 +7,7 @@ from pyramid.interfaces import ITweens from pyramid.tweens import MAIN from pyramid.tweens import INGRESS from pyramid.paster import bootstrap +from pyramid.paster import setup_logging from pyramid.scripts.common import parse_vars def main(argv=sys.argv, quiet=False): @@ -47,7 +48,8 @@ class PTweensCommand(object): ) stdout = sys.stdout - bootstrap = (bootstrap,) # testing + bootstrap = staticmethod(bootstrap) # testing + setup_logging = staticmethod(setup_logging) # testing def __init__(self, argv, quiet=False): self.quiet = quiet @@ -76,7 +78,9 @@ class PTweensCommand(object): self.out('Requires a config file argument') return 2 config_uri = self.args.config_uri - env = self.bootstrap[0](config_uri, options=parse_vars(self.args.config_vars)) + config_vars = parse_vars(self.args.config_vars) + self.setup_logging(config_uri, global_conf=config_vars) + env = self.bootstrap(config_uri, options=config_vars) registry = env['registry'] tweens = self._get_tweens(registry) if tweens is not None: diff --git a/pyramid/scripts/pviews.py b/pyramid/scripts/pviews.py index 4d3312917..c0df2f078 100644 --- a/pyramid/scripts/pviews.py +++ b/pyramid/scripts/pviews.py @@ -4,6 +4,7 @@ import textwrap from pyramid.interfaces import IMultiView from pyramid.paster import bootstrap +from pyramid.paster import setup_logging from pyramid.request import Request from pyramid.scripts.common import parse_vars from pyramid.view import _find_views @@ -51,7 +52,8 @@ class PViewsCommand(object): ) - bootstrap = (bootstrap,) # testing + bootstrap = staticmethod(bootstrap) # testing + setup_logging = staticmethod(setup_logging) # testing def __init__(self, argv, quiet=False): self.quiet = quiet @@ -252,13 +254,15 @@ class PViewsCommand(object): self.out('Command requires a config file arg and a url arg') return 2 config_uri = self.args.config_uri + config_vars = parse_vars(self.args.config_vars) url = self.args.url + self.setup_logging(config_uri, global_conf=config_vars) + if not url.startswith('/'): url = '/%s' % url request = Request.blank(url) - env = self.bootstrap[0](config_uri, options=parse_vars(self.args.config_vars), - request=request) + env = self.bootstrap(config_uri, options=config_vars, request=request) view = self._find_view(request) self.out('') self.out("URL = %s" % url) diff --git a/pyramid/tests/test_scripts/dummy.py b/pyramid/tests/test_scripts/dummy.py index b904adfbd..18810f8f3 100644 --- a/pyramid/tests/test_scripts/dummy.py +++ b/pyramid/tests/test_scripts/dummy.py @@ -167,7 +167,7 @@ class DummyPkgResources(object): class dummy_setup_logging(object): def __call__(self, config_uri, global_conf): self.config_uri = config_uri - self.global_conf = global_conf + self.defaults = global_conf class DummyLoader(object): @@ -190,7 +190,7 @@ class DummyLoader(object): def get_settings(self, name=None, defaults=None): self.add_call('settings', name, defaults) - return self.result + return self.settings.get(name, {}) def get_wsgi_app(self, name=None, defaults=None): self.add_call('app', name, defaults) diff --git a/pyramid/tests/test_scripts/test_prequest.py b/pyramid/tests/test_scripts/test_prequest.py index 45db0dbaf..ec5119270 100644 --- a/pyramid/tests/test_scripts/test_prequest.py +++ b/pyramid/tests/test_scripts/test_prequest.py @@ -1,4 +1,5 @@ import unittest +from pyramid.tests.test_scripts import dummy class TestPRequestCommand(unittest.TestCase): def _getTargetClass(self): @@ -7,23 +8,17 @@ class TestPRequestCommand(unittest.TestCase): def _makeOne(self, argv, headers=None): cmd = self._getTargetClass()(argv) - cmd.get_app = self.get_app - self._headers = headers or [] - self._out = [] - cmd.out = self.out - return cmd - - def get_app(self, spec, app_name=None, options=None): - self._spec = spec - self._app_name = app_name - self._options = options or {} def helloworld(environ, start_request): self._environ = environ self._path_info = environ['PATH_INFO'] - start_request('200 OK', self._headers) + start_request('200 OK', headers or []) return [b'abc'] - return helloworld + self.loader = dummy.DummyLoader(app=helloworld) + self._out = [] + cmd._get_config_loader = self.loader + cmd.out = self.out + return cmd def out(self, msg): self._out.append(msg) @@ -38,8 +33,10 @@ class TestPRequestCommand(unittest.TestCase): [('Content-Type', 'text/html; charset=UTF-8')]) command.run() self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) + self.assertEqual(self.loader.uri, 'development.ini') + self.assertEqual(self.loader.calls[0]['op'], 'logging') + self.assertEqual(self.loader.calls[1]['op'], 'app') + self.assertEqual(self.loader.calls[1]['name'], None) self.assertEqual(self._out, ['abc']) def test_command_path_doesnt_start_with_slash(self): @@ -47,8 +44,7 @@ class TestPRequestCommand(unittest.TestCase): [('Content-Type', 'text/html; charset=UTF-8')]) command.run() self.assertEqual(self._path_info, '/abc') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) + self.assertEqual(self.loader.uri, 'development.ini') self.assertEqual(self._out, ['abc']) def test_command_has_bad_config_header(self): @@ -67,8 +63,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['HTTP_NAME'], 'value') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_w_basic_auth(self): @@ -81,8 +75,6 @@ class TestPRequestCommand(unittest.TestCase): self.assertEqual(self._environ['HTTP_AUTHORIZATION'], 'Basic dXNlcjpwYXNzd29yZA==') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_has_content_type_header_var(self): @@ -92,8 +84,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['CONTENT_TYPE'], 'app/foo') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_has_multiple_header_vars(self): @@ -109,8 +99,6 @@ class TestPRequestCommand(unittest.TestCase): self.assertEqual(self._environ['HTTP_NAME'], 'value') self.assertEqual(self._environ['HTTP_NAME2'], 'value2') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_get(self): @@ -119,8 +107,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['REQUEST_METHOD'], 'GET') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_post(self): @@ -134,8 +120,6 @@ class TestPRequestCommand(unittest.TestCase): self.assertEqual(self._environ['CONTENT_LENGTH'], '-1') self.assertEqual(self._environ['wsgi.input'], stdin) self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_put(self): @@ -149,8 +133,6 @@ class TestPRequestCommand(unittest.TestCase): self.assertEqual(self._environ['CONTENT_LENGTH'], '-1') self.assertEqual(self._environ['wsgi.input'], stdin) self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_patch(self): @@ -164,8 +146,6 @@ class TestPRequestCommand(unittest.TestCase): self.assertEqual(self._environ['CONTENT_LENGTH'], '-1') self.assertEqual(self._environ['wsgi.input'], stdin) self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_propfind(self): @@ -178,8 +158,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['REQUEST_METHOD'], 'PROPFIND') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_method_options(self): @@ -192,8 +170,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['REQUEST_METHOD'], 'OPTIONS') self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_with_query_string(self): @@ -202,8 +178,6 @@ class TestPRequestCommand(unittest.TestCase): command.run() self.assertEqual(self._environ['QUERY_STRING'], 'a=1&b=2&c') self.assertEqual(self._path_info, '/abc') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, ['abc']) def test_command_display_headers(self): @@ -212,8 +186,6 @@ class TestPRequestCommand(unittest.TestCase): [('Content-Type', 'text/html; charset=UTF-8')]) command.run() self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual( self._out, ['200 OK', 'Content-Type: text/html; charset=UTF-8', 'abc']) @@ -223,21 +195,13 @@ class TestPRequestCommand(unittest.TestCase): headers=[('Content-Type', 'image/jpeg')]) command.run() self.assertEqual(self._path_info, '/') - self.assertEqual(self._spec, 'development.ini') - self.assertEqual(self._app_name, None) self.assertEqual(self._out, [b'abc']) def test_command_method_configures_logging(self): command = self._makeOne(['', 'development.ini', '/']) - called_args = [] - - def configure_logging(app_spec): - called_args.append(app_spec) - - command.configure_logging = configure_logging command.run() - self.assertEqual(called_args, ['development.ini']) + self.assertEqual(self.loader.calls[0]['op'], 'logging') class Test_main(unittest.TestCase): diff --git a/pyramid/tests/test_scripts/test_proutes.py b/pyramid/tests/test_scripts/test_proutes.py index 74293a112..fab5e163e 100644 --- a/pyramid/tests/test_scripts/test_proutes.py +++ b/pyramid/tests/test_scripts/test_proutes.py @@ -19,7 +19,8 @@ class TestPRoutesCommand(unittest.TestCase): def _makeOne(self): cmd = self._getTargetClass()([]) - cmd.bootstrap = (dummy.DummyBootstrap(),) + cmd.bootstrap = dummy.DummyBootstrap() + cmd.get_config_loader = dummy.DummyLoader() cmd.args.config_uri = '/foo/bar/myapp.ini#myapp' return cmd @@ -37,14 +38,15 @@ class TestPRoutesCommand(unittest.TestCase): def test_good_args(self): cmd = self._getTargetClass()([]) - cmd.bootstrap = (dummy.DummyBootstrap(),) + cmd.bootstrap = dummy.DummyBootstrap() + cmd.get_config_loader = dummy.DummyLoader() cmd.args.config_uri = '/foo/bar/myapp.ini#myapp' cmd.args.config_args = ('a=1',) route = dummy.DummyRoute('a', '/a') mapper = dummy.DummyMapper(route) cmd._get_mapper = lambda *arg: mapper registry = self._makeRegistry() - cmd.bootstrap = (dummy.DummyBootstrap(registry=registry),) + cmd.bootstrap = dummy.DummyBootstrap(registry=registry) L = [] cmd.out = lambda msg: L.append(msg) cmd.run() @@ -52,7 +54,8 @@ class TestPRoutesCommand(unittest.TestCase): def test_bad_args(self): cmd = self._getTargetClass()([]) - cmd.bootstrap = (dummy.DummyBootstrap(),) + cmd.bootstrap = dummy.DummyBootstrap() + cmd.get_config_loader = dummy.DummyLoader() cmd.args.config_uri = '/foo/bar/myapp.ini#myapp' cmd.args.config_vars = ('a',) route = dummy.DummyRoute('a', '/a') @@ -86,7 +89,7 @@ class TestPRoutesCommand(unittest.TestCase): mapper = dummy.DummyMapper(route) command._get_mapper = lambda *arg: mapper registry = self._makeRegistry() - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) L = [] command.out = L.append @@ -103,7 +106,7 @@ class TestPRoutesCommand(unittest.TestCase): L = [] command.out = L.append registry = self._makeRegistry() - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -124,7 +127,7 @@ class TestPRoutesCommand(unittest.TestCase): command._get_mapper = lambda *arg: mapper L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -150,7 +153,7 @@ class TestPRoutesCommand(unittest.TestCase): command._get_mapper = lambda *arg: mapper L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -190,7 +193,7 @@ class TestPRoutesCommand(unittest.TestCase): command._get_mapper = lambda *arg: mapper L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -218,7 +221,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -252,7 +255,7 @@ class TestPRoutesCommand(unittest.TestCase): command._get_mapper = lambda *arg: mapper L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -288,7 +291,7 @@ class TestPRoutesCommand(unittest.TestCase): command._get_mapper = lambda *arg: mapper L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=registry),) + command.bootstrap = dummy.DummyBootstrap(registry=registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -327,7 +330,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -354,7 +357,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -382,7 +385,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -410,7 +413,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -436,7 +439,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 5) @@ -461,7 +464,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -491,7 +494,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config2.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config2.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -521,7 +524,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -551,7 +554,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -592,7 +595,7 @@ class TestPRoutesCommand(unittest.TestCase): L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -624,7 +627,7 @@ class TestPRoutesCommand(unittest.TestCase): command.args.format = 'method,name' L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) @@ -654,7 +657,7 @@ class TestPRoutesCommand(unittest.TestCase): command.args.format = 'predicates,name,pattern' L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) expected = ( "You provided invalid formats ['predicates'], " "Available formats are ['name', 'pattern', 'view', 'method']" @@ -682,10 +685,9 @@ class TestPRoutesCommand(unittest.TestCase): L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) - config_factory = dummy.DummyConfigParserFactory() - command.ConfigParser = config_factory - config_factory.items = [('format', 'method\nname')] + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) + command.get_config_loader = dummy.DummyLoader( + {'proutes': {'format': 'method\nname'}}) result = command.run() self.assertEqual(result, 0) @@ -715,10 +717,9 @@ class TestPRoutesCommand(unittest.TestCase): L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) - config_factory = dummy.DummyConfigParserFactory() - command.ConfigParser = config_factory - config_factory.items = [('format', 'method name')] + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) + command.get_config_loader = dummy.DummyLoader( + {'proutes': {'format': 'method name'}}) result = command.run() self.assertEqual(result, 0) @@ -748,10 +749,9 @@ class TestPRoutesCommand(unittest.TestCase): L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) - config_factory = dummy.DummyConfigParserFactory() - command.ConfigParser = config_factory - config_factory.items = [('format', 'method,name')] + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) + command.get_config_loader = dummy.DummyLoader( + {'proutes': {'format': 'method,name'}}) result = command.run() self.assertEqual(result, 0) @@ -771,7 +771,7 @@ class TestPRoutesCommand(unittest.TestCase): command = self._makeOne() L = [] command.out = L.append - command.bootstrap = (dummy.DummyBootstrap(registry=config.registry),) + command.bootstrap = dummy.DummyBootstrap(registry=config.registry) result = command.run() self.assertEqual(result, 0) self.assertEqual(len(L), 3) diff --git a/pyramid/tests/test_scripts/test_ptweens.py b/pyramid/tests/test_scripts/test_ptweens.py index f63069fed..6907b858d 100644 --- a/pyramid/tests/test_scripts/test_ptweens.py +++ b/pyramid/tests/test_scripts/test_ptweens.py @@ -8,7 +8,8 @@ class TestPTweensCommand(unittest.TestCase): def _makeOne(self): cmd = self._getTargetClass()([]) - cmd.bootstrap = (dummy.DummyBootstrap(),) + cmd.bootstrap = dummy.DummyBootstrap() + cmd.setup_logging = dummy.dummy_setup_logging() cmd.args.config_uri = '/foo/bar/myapp.ini#myapp' return cmd diff --git a/pyramid/tests/test_scripts/test_pviews.py b/pyramid/tests/test_scripts/test_pviews.py index 7bdab5804..6ec9defbd 100644 --- a/pyramid/tests/test_scripts/test_pviews.py +++ b/pyramid/tests/test_scripts/test_pviews.py @@ -8,7 +8,8 @@ class TestPViewsCommand(unittest.TestCase): def _makeOne(self, registry=None): cmd = self._getTargetClass()([]) - cmd.bootstrap = (dummy.DummyBootstrap(registry=registry),) + cmd.bootstrap = dummy.DummyBootstrap(registry=registry) + cmd.setup_logging = dummy.dummy_setup_logging() cmd.args.config_uri = '/foo/bar/myapp.ini#myapp' return cmd -- cgit v1.2.3 From e1d1af88e314fe59d9197182f8c2b56ecdcbd115 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 29 Mar 2017 00:37:09 -0500 Subject: update pshell --- pyramid/scripts/pshell.py | 31 +++++++---------- pyramid/tests/test_scripts/test_pshell.py | 55 ++++++++----------------------- 2 files changed, 25 insertions(+), 61 deletions(-) diff --git a/pyramid/scripts/pshell.py b/pyramid/scripts/pshell.py index 83e640c32..bb201dbc2 100644 --- a/pyramid/scripts/pshell.py +++ b/pyramid/scripts/pshell.py @@ -5,15 +5,14 @@ import sys import textwrap import pkg_resources -from pyramid.compat import configparser from pyramid.compat import exec_ from pyramid.util import DottedNameResolver from pyramid.paster import bootstrap from pyramid.settings import aslist +from pyramid.scripts.common import get_config_loader from pyramid.scripts.common import parse_vars -from pyramid.scripts.common import setup_logging def main(argv=sys.argv, quiet=False): command = PShellCommand(argv, quiet) @@ -41,7 +40,8 @@ class PShellCommand(object): than one Pyramid application within it, the loader will use the last one. """ - bootstrap = (bootstrap,) # for testing + bootstrap = staticmethod(bootstrap) # for testing + get_config_loader = staticmethod(get_config_loader) # for testing pkg_resources = pkg_resources # for testing parser = argparse.ArgumentParser( @@ -78,7 +78,6 @@ class PShellCommand(object): "passed here.", ) - ConfigParser = configparser.ConfigParser # testing default_runner = python_shell_runner # testing loaded_objects = {} @@ -91,20 +90,13 @@ class PShellCommand(object): self.quiet = quiet self.args = self.parser.parse_args(argv[1:]) - def pshell_file_config(self, filename): - config = self.ConfigParser() - config.optionxform = str - config.read(filename) - try: - items = config.items('pshell') - except configparser.NoSectionError: - return - + def pshell_file_config(self, loader, defaults): + settings = loader.get_settings('pshell', defaults) resolver = DottedNameResolver(None) self.loaded_objects = {} self.object_help = {} self.setup = None - for k, v in items: + for k, v in settings.items(): if k == 'setup': self.setup = v elif k == 'default_shell': @@ -124,13 +116,12 @@ class PShellCommand(object): self.out('Requires a config file argument') return 2 config_uri = self.args.config_uri - config_file = config_uri.split('#', 1)[0] - setup_logging(config_file) - self.pshell_file_config(config_file) + config_vars = parse_vars(self.args.config_vars) + loader = self.get_config_loader(config_uri) + loader.setup_logging(config_vars) + self.pshell_file_config(loader, config_vars) - # bootstrap the environ - env = self.bootstrap[0](config_uri, - options=parse_vars(self.args.config_vars)) + env = self.bootstrap(config_uri, options=config_vars) # remove the closer from the env self.closer = env.pop('closer') diff --git a/pyramid/tests/test_scripts/test_pshell.py b/pyramid/tests/test_scripts/test_pshell.py index 303964b2b..ca9eb7af2 100644 --- a/pyramid/tests/test_scripts/test_pshell.py +++ b/pyramid/tests/test_scripts/test_pshell.py @@ -8,16 +8,16 @@ class TestPShellCommand(unittest.TestCase): from pyramid.scripts.pshell import PShellCommand return PShellCommand - def _makeOne(self, patch_bootstrap=True, patch_config=True, + def _makeOne(self, patch_bootstrap=True, patch_loader=True, patch_args=True, patch_options=True): cmd = self._getTargetClass()([]) if patch_bootstrap: self.bootstrap = dummy.DummyBootstrap() - cmd.bootstrap = (self.bootstrap,) - if patch_config: - self.config_factory = dummy.DummyConfigParserFactory() - cmd.ConfigParser = self.config_factory + cmd.bootstrap = self.bootstrap + if patch_loader: + self.loader = dummy.DummyLoader() + cmd.get_config_loader = self.loader if patch_args: class Args(object): pass self.args = Args() @@ -46,9 +46,6 @@ class TestPShellCommand(unittest.TestCase): command.default_runner = shell command.run() - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':self.bootstrap.root, @@ -79,9 +76,6 @@ class TestPShellCommand(unittest.TestCase): self.assertEqual( out_calls, ['could not find a shell named "unknown_python_shell"'] ) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertTrue(self.bootstrap.closer.called) @@ -100,9 +94,6 @@ class TestPShellCommand(unittest.TestCase): command.args.python_shell = 'ipython' command.run() - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':self.bootstrap.root, @@ -199,12 +190,9 @@ class TestPShellCommand(unittest.TestCase): command = self._makeOne() model = dummy.Dummy() user = dummy.Dummy() - self.config_factory.items = [('m', model), ('User', user)] + self.loader.settings = {'pshell': {'m': model, 'User': user}} shell = dummy.DummyShell() command.run(shell) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':self.bootstrap.root, @@ -223,12 +211,9 @@ class TestPShellCommand(unittest.TestCase): env['a'] = 1 env['root'] = 'root override' env['none'] = None - self.config_factory.items = [('setup', setup)] + self.loader.settings = {'pshell': {'setup': setup}} shell = dummy.DummyShell() command.run(shell) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':'root override', @@ -252,12 +237,9 @@ class TestPShellCommand(unittest.TestCase): 'python': dshell, } ) - self.config_factory.items = [ - ('default_shell', 'bpython python\nipython')] + self.loader.settings = {'pshell': { + 'default_shell': 'bpython python\nipython'}} command.run() - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertTrue(dshell.called) @@ -268,12 +250,9 @@ class TestPShellCommand(unittest.TestCase): env['a'] = 1 env['m'] = 'model override' env['root'] = 'root override' - self.config_factory.items = [('setup', setup), ('m', model)] + self.loader.settings = {'pshell': {'setup': setup, 'm': model}} shell = dummy.DummyShell() command.run(shell) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':'root override', @@ -291,14 +270,10 @@ class TestPShellCommand(unittest.TestCase): env['a'] = 1 env['root'] = 'root override' model = dummy.Dummy() - self.config_factory.items = [('setup', 'abc'), - ('m', model)] + self.loader.settings = {'pshell': {'setup': 'abc', 'm': model}} command.args.setup = setup shell = dummy.DummyShell() command.run(shell) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':self.bootstrap.app, 'root':'root override', @@ -313,13 +288,11 @@ class TestPShellCommand(unittest.TestCase): def test_command_custom_section_override(self): command = self._makeOne() dummy_ = dummy.Dummy() - self.config_factory.items = [('app', dummy_), ('root', dummy_), - ('registry', dummy_), ('request', dummy_)] + self.loader.settings = {'pshell': { + 'app': dummy_, 'root': dummy_, 'registry': dummy_, + 'request': dummy_}} shell = dummy.DummyShell() command.run(shell) - self.assertTrue(self.config_factory.parser) - self.assertEqual(self.config_factory.parser.filename, - '/foo/bar/myapp.ini') self.assertEqual(self.bootstrap.a[0], '/foo/bar/myapp.ini#myapp') self.assertEqual(shell.env, { 'app':dummy_, 'root':dummy_, 'registry':dummy_, 'request':dummy_, -- cgit v1.2.3 From 3e489b740b1836c81af240cba579245ab18177da Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 29 Mar 2017 22:46:02 -0500 Subject: update pserve --- pyramid/scripts/pserve.py | 103 ++++++++++------------------ pyramid/tests/test_paster.py | 26 +++---- pyramid/tests/test_scripts/dummy.py | 33 +++------ pyramid/tests/test_scripts/test_prequest.py | 4 +- pyramid/tests/test_scripts/test_pserve.py | 69 ++++++++----------- 5 files changed, 80 insertions(+), 155 deletions(-) diff --git a/pyramid/scripts/pserve.py b/pyramid/scripts/pserve.py index c469dde04..f7d094980 100644 --- a/pyramid/scripts/pserve.py +++ b/pyramid/scripts/pserve.py @@ -18,16 +18,11 @@ import time import webbrowser import hupper -from paste.deploy import ( - loadapp, - loadserver, -) from pyramid.compat import PY2 -from pyramid.compat import configparser +from pyramid.scripts.common import get_config_loader from pyramid.scripts.common import parse_vars -from pyramid.scripts.common import setup_logging from pyramid.path import AssetResolver from pyramid.settings import aslist @@ -113,9 +108,7 @@ class PServeCommand(object): "passed here.", ) - ConfigParser = configparser.ConfigParser # testing - loadapp = staticmethod(loadapp) # testing - loadserver = staticmethod(loadserver) # testing + _get_config_loader = staticmethod(get_config_loader) # for testing open_url = None @@ -133,26 +126,14 @@ class PServeCommand(object): if self.args.verbose > 0: print(msg) - def get_config_vars(self): - restvars = self.args.config_vars - return parse_vars(restvars) + def get_config_path(self, loader): + return os.path.abspath(loader.uri.path) - def pserve_file_config(self, filename, global_conf=None): - here = os.path.abspath(os.path.dirname(filename)) - defaults = {} - if global_conf: - defaults.update(global_conf) - defaults['here'] = here - - config = self.ConfigParser(defaults=defaults) - config.optionxform = str - config.read(filename) - try: - items = dict(config.items('pserve')) - except configparser.NoSectionError: - return - - watch_files = aslist(items.get('watch_files', ''), flatten=False) + def pserve_file_config(self, loader, global_conf=None): + settings = loader.get_settings('pserve', global_conf) + config_path = self.get_config_path(loader) + here = os.path.dirname(config_path) + watch_files = aslist(settings.get('watch_files', ''), flatten=False) # track file paths relative to the ini file resolver = AssetResolver(package=None) @@ -164,45 +145,30 @@ class PServeCommand(object): self.watch_files.add(os.path.abspath(file)) # attempt to determine the url of the server - open_url = items.get('open_url') + open_url = settings.get('open_url') if open_url: self.open_url = open_url - def _guess_server_url(self, filename, server_name, - global_conf=None): # pragma: no cover + def guess_server_url(self, loader, server_name, global_conf=None): server_name = server_name or 'main' - here = os.path.abspath(os.path.dirname(filename)) - defaults = {} - if global_conf: - defaults.update(global_conf) - defaults['here'] = here - - config = self.ConfigParser(defaults=defaults) - config.optionxform = str - config.read(filename) - try: - items = dict(config.items('server:' + server_name)) - except configparser.NoSectionError: - return - - if 'port' in items: - return 'http://127.0.0.1:{port}'.format(**items) + settings = loader.get_settings('server:' + server_name, global_conf) + if 'port' in settings: + return 'http://127.0.0.1:{port}'.format(**settings) def run(self): # pragma: no cover if not self.args.config_uri: self.out('You must give a config file') return 2 + config_uri = self.args.config_uri + config_vars = parse_vars(self.args.config_vars) app_spec = self.args.config_uri - - vars = self.get_config_vars() app_name = self.args.app_name - base = os.getcwd() - if not self._scheme_re.search(app_spec): - config_path = os.path.join(base, app_spec) - app_spec = 'config:' + app_spec - else: - config_path = None + loader = self._get_config_loader(config_uri) + loader.setup_logging(config_vars) + + self.pserve_file_config(loader, global_conf=config_vars) + server_name = self.args.server_name if self.args.server: server_spec = 'egg:pyramid' @@ -211,15 +177,17 @@ class PServeCommand(object): else: server_spec = app_spec + server_loader = loader + if server_spec != app_spec: + server_loader = self.get_config_loader(server_spec) + # do not open the browser on each reload so check hupper first if self.args.browser and not hupper.is_active(): - self.pserve_file_config(config_path, global_conf=vars) url = self.open_url - # do not guess the url if the server is sourced from a different - # location than the config_path - if not url and server_spec == app_spec: - url = self._guess_server_url(config_path, server_name, vars) + if not url: + url = self.guess_server_url( + server_loader, server_name, config_vars) if not url: self.out('WARNING: could not determine the server\'s url to ' @@ -246,20 +214,19 @@ class PServeCommand(object): ) return 0 - if config_path: - setup_logging(config_path, global_conf=vars) - self.pserve_file_config(config_path, global_conf=vars) - self.watch_files.add(config_path) + config_path = self.get_config_path(loader) + self.watch_files.add(config_path) + + server_path = self.get_config_path(server_loader) + self.watch_files.add(server_path) if hupper.is_active(): reloader = hupper.get_reloader() reloader.watch_files(list(self.watch_files)) - server = self.loadserver( - server_spec, name=server_name, relative_to=base, global_conf=vars) + server = server_loader.get_wsgi_server(server_name, config_vars) - app = self.loadapp( - app_spec, name=app_name, relative_to=base, global_conf=vars) + app = loader.get_wsgi_app(app_name, config_vars) if self.args.verbose > 0: if hasattr(os, 'getpid'): diff --git a/pyramid/tests/test_paster.py b/pyramid/tests/test_paster.py index fc0cf20a5..784458647 100644 --- a/pyramid/tests/test_paster.py +++ b/pyramid/tests/test_paster.py @@ -22,7 +22,7 @@ class Test_get_app(unittest.TestCase): result = self._callFUT( '/foo/bar/myapp.ini', 'myapp', options={'a': 'b'}, _loader=loader) - self.assertEqual(loader.uri, '/foo/bar/myapp.ini') + self.assertEqual(loader.uri.path, '/foo/bar/myapp.ini') self.assertEqual(len(loader.calls), 1) self.assertEqual(loader.calls[0]['op'], 'app') self.assertEqual(loader.calls[0]['name'], 'myapp') @@ -54,7 +54,7 @@ class Test_get_appsettings(unittest.TestCase): result = self._callFUT( '/foo/bar/myapp.ini', 'myapp', options={'a': 'b'}, _loader=loader) - self.assertEqual(loader.uri, '/foo/bar/myapp.ini') + self.assertEqual(loader.uri.path, '/foo/bar/myapp.ini') self.assertEqual(len(loader.calls), 1) self.assertEqual(loader.calls[0]['op'], 'app_settings') self.assertEqual(loader.calls[0]['name'], 'myapp') @@ -81,24 +81,24 @@ class Test_setup_logging(unittest.TestCase): def test_it_no_global_conf(self): loader = DummyLoader() - self._callFUT('/abc', _loader=loader) - self.assertEqual(loader.uri, '/abc') + self._callFUT('/abc.ini', _loader=loader) + self.assertEqual(loader.uri.path, '/abc.ini') self.assertEqual(len(loader.calls), 1) self.assertEqual(loader.calls[0]['op'], 'logging') self.assertEqual(loader.calls[0]['defaults'], None) def test_it_global_conf_empty(self): loader = DummyLoader() - self._callFUT('/abc', global_conf={}, _loader=loader) - self.assertEqual(loader.uri, '/abc') + self._callFUT('/abc.ini', global_conf={}, _loader=loader) + self.assertEqual(loader.uri.path, '/abc.ini') self.assertEqual(len(loader.calls), 1) self.assertEqual(loader.calls[0]['op'], 'logging') self.assertEqual(loader.calls[0]['defaults'], {}) def test_it_global_conf_not_empty(self): loader = DummyLoader() - self._callFUT('/abc', global_conf={'key': 'val'}, _loader=loader) - self.assertEqual(loader.uri, '/abc') + self._callFUT('/abc.ini', global_conf={'key': 'val'}, _loader=loader) + self.assertEqual(loader.uri.path, '/abc.ini') self.assertEqual(len(loader.calls), 1) self.assertEqual(loader.calls[0]['op'], 'logging') self.assertEqual(loader.calls[0]['defaults'], {'key': 'val'}) @@ -166,13 +166,3 @@ class DummyRequest: def __init__(self, environ): self.environ = environ self.matchdict = {} - -class DummyConfigParser(object): - def read(self, x): - pass - - def has_section(self, name): - return True - -class DummyConfigParserModule(object): - ConfigParser = DummyConfigParser diff --git a/pyramid/tests/test_scripts/dummy.py b/pyramid/tests/test_scripts/dummy.py index 18810f8f3..2d2b0549f 100644 --- a/pyramid/tests/test_scripts/dummy.py +++ b/pyramid/tests/test_scripts/dummy.py @@ -81,29 +81,6 @@ class DummyMultiView(object): self.views = [(None, view, None) for view in views] self.__request_attrs__ = attrs -class DummyConfigParser(object): - def __init__(self, result, defaults=None): - self.result = result - self.defaults = defaults - - def read(self, filename): - self.filename = filename - - def items(self, section): - self.section = section - if self.result is None: - from pyramid.compat import configparser - raise configparser.NoSectionError(section) - return self.result - -class DummyConfigParserFactory(object): - items = None - - def __call__(self, defaults=None): - self.defaults = defaults - self.parser = DummyConfigParser(self.items, defaults) - return self.parser - class DummyCloser(object): def __call__(self): self.called = True @@ -171,7 +148,7 @@ class dummy_setup_logging(object): class DummyLoader(object): - def __init__(self, settings=None, app_settings=None, app=None): + def __init__(self, settings=None, app_settings=None, app=None, server=None): if not settings: settings = {} if not app_settings: @@ -179,10 +156,12 @@ class DummyLoader(object): self.settings = settings self.app_settings = app_settings self.app = app + self.server = server self.calls = [] def __call__(self, uri): - self.uri = uri + import plaster + self.uri = plaster.parse_uri(uri) return self def add_call(self, op, name, defaults): @@ -200,6 +179,10 @@ class DummyLoader(object): self.add_call('app_settings', name, defaults) return self.app_settings + def get_wsgi_server(self, name=None, defaults=None): + self.add_call('server', name, defaults) + return self.server + def setup_logging(self, defaults): self.add_call('logging', None, defaults) self.defaults = defaults diff --git a/pyramid/tests/test_scripts/test_prequest.py b/pyramid/tests/test_scripts/test_prequest.py index ec5119270..75d5cc198 100644 --- a/pyramid/tests/test_scripts/test_prequest.py +++ b/pyramid/tests/test_scripts/test_prequest.py @@ -33,7 +33,7 @@ class TestPRequestCommand(unittest.TestCase): [('Content-Type', 'text/html; charset=UTF-8')]) command.run() self.assertEqual(self._path_info, '/') - self.assertEqual(self.loader.uri, 'development.ini') + self.assertEqual(self.loader.uri.path, 'development.ini') self.assertEqual(self.loader.calls[0]['op'], 'logging') self.assertEqual(self.loader.calls[1]['op'], 'app') self.assertEqual(self.loader.calls[1]['name'], None) @@ -44,7 +44,7 @@ class TestPRequestCommand(unittest.TestCase): [('Content-Type', 'text/html; charset=UTF-8')]) command.run() self.assertEqual(self._path_info, '/abc') - self.assertEqual(self.loader.uri, 'development.ini') + self.assertEqual(self.loader.uri.path, 'development.ini') self.assertEqual(self._out, ['abc']) def test_command_has_bad_config_header(self): diff --git a/pyramid/tests/test_scripts/test_pserve.py b/pyramid/tests/test_scripts/test_pserve.py index d5578b3ea..485cf38cb 100644 --- a/pyramid/tests/test_scripts/test_pserve.py +++ b/pyramid/tests/test_scripts/test_pserve.py @@ -10,16 +10,10 @@ class TestPServeCommand(unittest.TestCase): def setUp(self): from pyramid.compat import NativeIO self.out_ = NativeIO() - self.config_factory = dummy.DummyConfigParserFactory() def out(self, msg): self.out_.write(msg) - def _get_server(*args, **kwargs): - def server(app): - return '' - return server - def _getTargetClass(self): from pyramid.scripts.pserve import PServeCommand return PServeCommand @@ -29,7 +23,8 @@ class TestPServeCommand(unittest.TestCase): effargs.extend(args) cmd = self._getTargetClass()(effargs) cmd.out = self.out - cmd.ConfigParser = self.config_factory + self.loader = dummy.DummyLoader() + cmd._get_config_loader = self.loader return cmd def test_run_no_args(self): @@ -38,41 +33,33 @@ class TestPServeCommand(unittest.TestCase): self.assertEqual(result, 2) self.assertEqual(self.out_.getvalue(), 'You must give a config file') - def test_config_vars_no_command(self): - inst = self._makeOne() - inst.args.config_uri = 'foo' - inst.args.config_vars = ['a=1', 'b=2'] - result = inst.get_config_vars() - self.assertEqual(result, {'a': '1', 'b': '2'}) - def test_parse_vars_good(self): inst = self._makeOne('development.ini', 'a=1', 'b=2') - inst.loadserver = self._get_server - app = dummy.DummyApp() - def get_app(*args, **kwargs): - app.global_conf = kwargs.get('global_conf', None) + def get_app(name, global_conf): + app.name = name + app.global_conf = global_conf + return app + self.loader.get_wsgi_app = get_app + self.loader.server = lambda x: x - inst.loadapp = get_app inst.run() self.assertEqual(app.global_conf, {'a': '1', 'b': '2'}) def test_parse_vars_bad(self): inst = self._makeOne('development.ini', 'a') - inst.loadserver = self._get_server self.assertRaises(ValueError, inst.run) def test_config_file_finds_watch_files(self): inst = self._makeOne('development.ini') - self.config_factory.items = [( - 'watch_files', - 'foo\n/baz\npyramid.tests.test_scripts:*.py', - )] - inst.pserve_file_config('/base/path.ini', global_conf={'a': '1'}) - self.assertEqual(self.config_factory.defaults, { + loader = self.loader('/base/path.ini') + loader.settings = {'pserve': { + 'watch_files': 'foo\n/baz\npyramid.tests.test_scripts:*.py', + }} + inst.pserve_file_config(loader, global_conf={'a': '1'}) + self.assertEqual(loader.calls[0]['defaults'], { 'a': '1', - 'here': os.path.abspath('/base'), }) self.assertEqual(inst.watch_files, set([ os.path.abspath('/base/foo'), @@ -82,28 +69,26 @@ class TestPServeCommand(unittest.TestCase): def test_config_file_finds_open_url(self): inst = self._makeOne('development.ini') - self.config_factory.items = [( - 'open_url', 'http://127.0.0.1:8080/', - )] - inst.pserve_file_config('/base/path.ini', global_conf={'a': '1'}) - self.assertEqual(self.config_factory.defaults, { + loader = self.loader('/base/path.ini') + loader.settings = {'pserve': { + 'open_url': 'http://127.0.0.1:8080/', + }} + inst.pserve_file_config(loader, global_conf={'a': '1'}) + self.assertEqual(loader.calls[0]['defaults'], { 'a': '1', - 'here': os.path.abspath('/base'), }) self.assertEqual(inst.open_url, 'http://127.0.0.1:8080/') - def test__guess_server_url(self): + def test_guess_server_url(self): inst = self._makeOne('development.ini') - self.config_factory.items = [( - 'port', '8080', - )] - url = inst._guess_server_url( - '/base/path.ini', 'main', global_conf={'a': '1'}) - self.assertEqual(self.config_factory.defaults, { + loader = self.loader('/base/path.ini') + loader.settings = {'server:foo': { + 'port': '8080', + }} + url = inst.guess_server_url(loader, 'foo', global_conf={'a': '1'}) + self.assertEqual(loader.calls[0]['defaults'], { 'a': '1', - 'here': os.path.abspath('/base'), }) - self.assertEqual(self.config_factory.parser.section, 'server:main') self.assertEqual(url, 'http://127.0.0.1:8080') def test_reload_call_hupper_with_correct_args(self): -- cgit v1.2.3 From db8aeae4c79b0bec3b8c28f5b060a75e453e1f98 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 30 Mar 2017 01:53:03 -0500 Subject: depend on plaster_pastedeploy --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 046341f13..3048428aa 100644 --- a/setup.py +++ b/setup.py @@ -35,11 +35,11 @@ install_requires = [ 'translationstring >= 0.4', # py3 compat 'PasteDeploy >= 1.5.0', # py3 compat 'plaster', + 'plaster_pastedeploy', 'hupper', ] tests_require = [ - 'plaster_pastedeploy', 'WebTest >= 1.3.1', # py3 compat 'zope.component >= 4.0', # py3 compat ] -- cgit v1.2.3 From fa8a9dcb11a7e2ec81fa041fd0292d7ce8e2138a Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 31 Mar 2017 00:09:51 -0500 Subject: add changelog for #2985 --- CHANGES.txt | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7676a69f9..c617adf95 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,14 +1,32 @@ unreleased ========== -Features --------- +Major Features +-------------- + +- The file format used by all ``p*`` command line scripts such as ``pserve`` + and ``pshell``, as well as the ``pyramid.paster.bootstrap`` function + is now replaceable thanks to a new dependency on + `plaster `. + + For now, Pyramid is still shipping with integrated support for the + PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding. + + See https://github.com/Pylons/pyramid/pull/2985 - Added an execution policy hook to the request pipeline. An execution policy has the ability to control creation and execution of the request - objects before they enter rest of the pipeline. This means for a given - request that the policy may create more than one request for retry - purposes. See https://github.com/Pylons/pyramid/pull/2964 + objects before they enter rest of the pipeline. This means for a single + request environ the policy may create more than one request object. + + The first library to use this feature is + `pyramid_retry + `. + + See https://github.com/Pylons/pyramid/pull/2964 + +Features +-------- - Support an ``open_url`` config setting in the ``pserve`` section of the config file. This url is used to open a web browser when ``pserve --browser`` -- cgit v1.2.3 From f454b80b0f6e6442fa27e48b7e1e38c5a7cbef03 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 31 Mar 2017 01:49:42 -0500 Subject: add some simple notes about plaster in the narrative docs --- CHANGES.txt | 6 +++--- docs/glossary.rst | 8 ++++++++ docs/narr/paste.rst | 12 ++++++------ docs/narr/startup.rst | 9 ++++++++- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c617adf95..59196626d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,7 +7,7 @@ Major Features - The file format used by all ``p*`` command line scripts such as ``pserve`` and ``pshell``, as well as the ``pyramid.paster.bootstrap`` function is now replaceable thanks to a new dependency on - `plaster `. + `plaster `_. For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding. @@ -16,12 +16,12 @@ Major Features - Added an execution policy hook to the request pipeline. An execution policy has the ability to control creation and execution of the request - objects before they enter rest of the pipeline. This means for a single + objects before they enter the rest of the pipeline. This means for a single request environ the policy may create more than one request object. The first library to use this feature is `pyramid_retry - `. + `_. See https://github.com/Pylons/pyramid/pull/2964 diff --git a/docs/glossary.rst b/docs/glossary.rst index 0a46fac3b..8f7ea70a1 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -366,6 +366,14 @@ Glossary :term:`WSGI` components together declaratively within an ``.ini`` file. It was developed by Ian Bicking. + plaster + `plaster `_ is + a library used by :app:`Pyramid` which acts as an abstraction between + command-line scripts and the file format used to load the :term:`WSGI` + components and application settings. By default :app:`Pyramid` ships + with the ``plaster_pastedeploy`` library installed which provides + integrated support for loading a :term:`PasteDeploy` INI file. + Chameleon `chameleon `_ is an attribute language template compiler which supports the :term:`ZPT` diff --git a/docs/narr/paste.rst b/docs/narr/paste.rst index 2d4e76e24..26cb1bfa5 100644 --- a/docs/narr/paste.rst +++ b/docs/narr/paste.rst @@ -26,12 +26,7 @@ documentation, see http://pythonpaste.org/deploy/. PasteDeploy ----------- -:term:`PasteDeploy` is the system that Pyramid uses to allow :term:`deployment -settings` to be specified using an ``.ini`` configuration file format. It also -allows the ``pserve`` command to work. Its configuration format provides a -convenient place to define application :term:`deployment settings` and WSGI -server settings, and its server runner allows you to stop and start a Pyramid -application easily. +:term:`plaster` is the system that Pyramid uses to load settings from configuration files. The most common format for these files is an ``.ini`` format structured in a way defined by :term:`PasteDeploy`. The format supports mechanisms to define WSGI app :term:`deployment settings`, WSGI server settings and logging. This allows the ``pserve`` command to work, allowing you to stop and start a Pyramid application easily. .. _pastedeploy_entry_points: @@ -96,3 +91,8 @@ applications, servers, and :term:`middleware` defined within the configuration file. The values in a ``[DEFAULT]`` section will be passed to your application's ``main`` function as ``global_config`` (see the reference to the ``main`` function in :ref:`init_py`). + +Alternative Configuration File Formats +-------------------------------------- + +It is possible to use different file formats with :app:`Pyramid` if you do not like :term:`PasteDeploy`. Under the hood all command-line scripts such as ``pserve`` and ``pshell`` pass the ``config_uri`` (e.g. ``development.ini`` or ``production.ini``) to the :term:`plaster` library which performs a lookup for an appropriate parser. For ``.ini`` files it uses PasteDeploy but you can register your own configuration formats that plaster will find instead. diff --git a/docs/narr/startup.rst b/docs/narr/startup.rst index cf4612602..29a75cba2 100644 --- a/docs/narr/startup.rst +++ b/docs/narr/startup.rst @@ -38,7 +38,14 @@ Here's a high-level time-ordered overview of what happens when you press begin to run and serve an application using the information contained within the ``development.ini`` file. -#. The framework finds a section named either ``[app:main]``, +#. ``pserve`` passes the ``development.ini`` path to :term:`plaster` which + finds an available configuration loader that recognizes the ``ini`` format. + +#. :term:`plaster` finds the ``plaster_pastedeploy`` library which binds + the :term:`PasteDeploy` library and returns a parser that can understand + the format. + +#. The :term:`PasteDeploy` finds a section named either ``[app:main]``, ``[pipeline:main]``, or ``[composite:main]`` in the ``.ini`` file. This section represents the configuration of a :term:`WSGI` application that will be served. If you're using a simple application (e.g., ``[app:main]``), the -- cgit v1.2.3 From 134ef7e8d17842e286528aeb8d2936579ed81053 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Fri, 16 Dec 2016 00:48:52 -0600 Subject: turn the Configurator into a context manager fixes #2872 --- README.rst | 8 +++---- pyramid/config/__init__.py | 40 ++++++++++++++++++++++++++++++++-- pyramid/tests/test_config/test_init.py | 16 ++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index f05dd8bbf..302590fe1 100644 --- a/README.rst +++ b/README.rst @@ -31,10 +31,10 @@ and deployment more fun, more predictable, and more productive. return Response('Hello %(name)s!' % request.matchdict) if __name__ == '__main__': - config = Configurator() - config.add_route('hello', '/hello/{name}') - config.add_view(hello_world, route_name='hello') - app = config.make_wsgi_app() + with Configurator() as config: + config.add_route('hello', '/hello/{name}') + config.add_view(hello_world, route_name='hello') + app = config.make_wsgi_app() server = make_server('0.0.0.0', 8080, app) server.serve_forever() diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py index 6c661aa59..bcd4b3904 100644 --- a/pyramid/config/__init__.py +++ b/pyramid/config/__init__.py @@ -110,6 +110,17 @@ class Configurator( A Configurator is used to configure a :app:`Pyramid` :term:`application registry`. + The Configurator lifecycle can be managed by using a context manager to + automatically handle calling :meth:`pyramid.config.Configurator.begin` and + :meth:`pyramid.config.Configurator.end` as well as + :meth:`pyramid.config.Configurator.commit`. + + .. code-block:: python + + with Configurator(settings=settings) as config: + config.add_route('home', '/') + app = config.make_wsgi_app() + If the ``registry`` argument is not ``None``, it must be an instance of the :class:`pyramid.registry.Registry` class representing the registry to configure. If ``registry`` is ``None``, the @@ -265,6 +276,11 @@ class Configurator( .. versionadded:: 1.6 The ``root_package`` argument. The ``response_factory`` argument. + + .. versionadded:: 1.9 + The ability to use the configurator as a context manager with the + ``with``-statement to make threadlocal configuration available for + further configuration with an implicit commit. """ manager = manager # for testing injection venusian = venusian # for testing injection @@ -646,12 +662,22 @@ class Configurator( _ctx = action_state # bw compat def commit(self): - """ Commit any pending configuration actions. If a configuration + """ + Commit any pending configuration actions. If a configuration conflict is detected in the pending configuration actions, this method will raise a :exc:`ConfigurationConflictError`; within the traceback of this error will be information about the source of the conflict, usually including file names and line numbers of the cause of the - configuration conflicts.""" + configuration conflicts. + + .. warning:: + You should think very carefully before manually invoking + ``commit()``. Especially not as part of any reusable configuration + methods. Normally it should only be done by an application author at + the end of configuration in order to override certain aspects of an + addon. + + """ self.begin() try: self.action_state.execute_actions(introspector=self.introspector) @@ -933,6 +959,16 @@ class Configurator( """ return self.manager.pop() + def __enter__(self): + self.begin() + return self + + def __exit__(self, exc_type, exc_value, exc_traceback): + self.end() + + if exc_value is None: + self.commit() + # this is *not* an action method (uses caller_package) def scan(self, package=None, categories=None, onerror=None, ignore=None, **kw): diff --git a/pyramid/tests/test_config/test_init.py b/pyramid/tests/test_config/test_init.py index 53c601537..ab584cc3d 100644 --- a/pyramid/tests/test_config/test_init.py +++ b/pyramid/tests/test_config/test_init.py @@ -141,6 +141,22 @@ class ConfiguratorTests(unittest.TestCase): self.assertEqual(manager.pushed, pushed) self.assertEqual(manager.popped, True) + def test_context_manager(self): + from pyramid.config import Configurator + config = Configurator() + manager = DummyThreadLocalManager() + config.manager = manager + view = lambda r: None + with config as ctx: + self.assertTrue(config is ctx) + self.assertEqual(manager.pushed, + {'registry': config.registry, 'request': None}) + self.assertFalse(manager.popped) + config.add_view(view) + self.assertTrue(manager.popped) + config.add_view(view) # did not raise a conflict because of commit + config.commit() + def test_ctor_with_package_registry(self): import sys from pyramid.config import Configurator -- cgit v1.2.3 From 1bd681193feef31d032c13e7022bc2d65d9e0a21 Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Mon, 10 Apr 2017 14:55:46 +1000 Subject: replace deprecated cgi.escape() with html.escape() As suggested by https://docs.python.org/3.6/library/cgi.html cgi.escape() Deprecated since version 3.2: This function is unsafe because quote is false by default, and therefore deprecated. Use html.escape() instead. --- docs/tutorials/wiki2/src/views/tutorial/views/default.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/wiki2/src/views/tutorial/views/default.py b/docs/tutorials/wiki2/src/views/tutorial/views/default.py index bb6300b75..0a05b33e6 100644 --- a/docs/tutorials/wiki2/src/views/tutorial/views/default.py +++ b/docs/tutorials/wiki2/src/views/tutorial/views/default.py @@ -1,4 +1,4 @@ -import cgi +import html import re from docutils.core import publish_parts @@ -31,10 +31,10 @@ def view_page(request): exists = request.dbsession.query(Page).filter_by(name=word).all() if exists: view_url = request.route_url('view_page', pagename=word) - return '%s' % (view_url, cgi.escape(word)) + return '%s' % (view_url, html.escape(word)) else: add_url = request.route_url('add_page', pagename=word) - return '%s' % (add_url, cgi.escape(word)) + return '%s' % (add_url, html.escape(word)) content = publish_parts(page.data, writer_name='html')['html_body'] content = wikiwords.sub(add_link, content) -- cgit v1.2.3 From a2c7c7a49bceeaaab2853e7e73c3671979d4c9ed Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Mon, 5 Dec 2016 12:16:26 +0100 Subject: Create a new ICSRF implementation for getting CSRF tokens, split out from the session machinery. Adds configuration of this to the csrf_options configurator commands. Make the default implementation a fallback to the old one. Documentation patches for new best practices given updates CSRF implementation. --- CHANGES.txt | 12 ++ docs/api/csrf.rst | 18 ++ docs/api/interfaces.rst | 3 + docs/api/session.rst | 4 - docs/narr/security.rst | 191 +++++++++++++++++++++ docs/narr/sessions.rst | 175 ------------------- pyramid/config/security.py | 16 ++ pyramid/config/views.py | 14 +- pyramid/csrf.py | 286 ++++++++++++++++++++++++++++++++ pyramid/interfaces.py | 29 +++- pyramid/predicates.py | 2 +- pyramid/renderers.py | 1 - pyramid/session.py | 155 +---------------- pyramid/tests/test_config/test_views.py | 2 +- pyramid/tests/test_csrf.py | 172 +++++++++++++++++++ pyramid/tests/test_session.py | 4 +- pyramid/viewderivers.py | 2 +- 17 files changed, 731 insertions(+), 355 deletions(-) create mode 100644 docs/api/csrf.rst create mode 100644 pyramid/csrf.py create mode 100644 pyramid/tests/test_csrf.py diff --git a/CHANGES.txt b/CHANGES.txt index c8a87f625..9d6264688 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,6 +24,14 @@ Features can be alleviated by invoking ``config.begin()`` and ``config.end()`` appropriately. See https://github.com/Pylons/pyramid/pull/2989 +- A new CSRF implementation, :class:`pyramid.csrf.SessionCSRF` has been added, + which deleagates all CSRF generation to the current session, following the + old API for this. A ``get_csrf_token()`` method is now available in template + global scope, to make it easy for template developers to get the current CSRF + token without adding it to Python code. + See https://github.com/Pylons/pyramid/pull/2854 + + Bug Fixes --------- @@ -50,3 +58,7 @@ Backward Incompatibilities Documentation Changes --------------------- + +- Retrieving CSRF token from the session has been deprecated, in favor of + equivalent methods in :mod:`pyramid.csrf`. + See https://github.com/Pylons/pyramid/pull/2854 diff --git a/docs/api/csrf.rst b/docs/api/csrf.rst new file mode 100644 index 000000000..3125bdac9 --- /dev/null +++ b/docs/api/csrf.rst @@ -0,0 +1,18 @@ +.. _csrf_module: + +:mod:`pyramid.csrf` +------------------- + +.. automodule:: pyramid.csrf + + .. autofunction:: get_csrf_token + + .. autofunction:: new_csrf_token + + .. autoclass:: SessionCSRF + :members: + + .. autofunction:: check_csrf_origin + + .. autofunction:: check_csrf_token + diff --git a/docs/api/interfaces.rst b/docs/api/interfaces.rst index a212ba7a9..2ca472616 100644 --- a/docs/api/interfaces.rst +++ b/docs/api/interfaces.rst @@ -44,6 +44,9 @@ Other Interfaces .. autointerface:: IRoutePregenerator :members: + .. autointerface:: ICSRF + :members: + .. autointerface:: ISession :members: diff --git a/docs/api/session.rst b/docs/api/session.rst index 56c4f52d7..53bae7c52 100644 --- a/docs/api/session.rst +++ b/docs/api/session.rst @@ -9,10 +9,6 @@ .. autofunction:: signed_deserialize - .. autofunction:: check_csrf_origin - - .. autofunction:: check_csrf_token - .. autofunction:: SignedCookieSessionFactory .. autofunction:: UnencryptedCookieSessionFactoryConfig diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 77e7fd707..b4fb3b8a8 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -765,3 +765,194 @@ which would allow the attacker to control the content of the payload. Re-using a secret across two different subsystems might drop the security of signing to zero. Keys should not be re-used across different contexts where an attacker has the possibility of providing a chosen plaintext. + +Preventing Cross-Site Request Forgery Attacks +--------------------------------------------- + +`Cross-site request forgery +`_ attacks are a +phenomenon whereby a user who is logged in to your website might inadvertantly +load a URL because it is linked from, or embedded in, an attacker's website. +If the URL is one that may modify or delete data, the consequences can be dire. + +You can avoid most of these attacks by issuing a unique token to the browser +and then requiring that it be present in all potentially unsafe requests. +:app:`Pyramid` sessions provide facilities to create and check CSRF tokens. + +To use CSRF tokens, you must first enable a :term:`session factory` as +described in :ref:`using_the_default_session_factory` or +:ref:`using_alternate_session_factories`. + +.. index:: + single: csrf.get_csrf_token + +Using the ``csrf.get_csrf_token`` Method +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +To get the current CSRF token, use the +:data:`pyramid.csrf.get_csrf_token` method. + +.. code-block:: python + + from pyramid.csrf import get_csrf_token + token = get_csrf_token(request) + +The ``get_csrf_token()`` method accepts a single argument: the request. It +returns a CSRF *token* string. If ``get_csrf_token()`` or ``new_csrf_token()`` +was invoked previously for this user, then the existing token will be returned. +If no CSRF token previously existed for this user, then a new token will be set +into the session and returned. The newly created token will be opaque and +randomized. + + +Using the ``get_csrf_token`` global in templates +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Templates have a ``get_csrf_token()`` method inserted into their globals, which +allows you to get the current token without modifying the view code. This +method takes no arguments and returns a CSRF token string. You can use the +returned token as the value of a hidden field in a form that posts to a method +that requires elevated privileges, or supply it as a request header in AJAX +requests. + +For example, include the CSRF token as a hidden field: + +.. code-block:: html + +
+ + +
+ +Or include it as a header in a jQuery AJAX request: + +.. code-block:: javascript + + var csrfToken = "${get_csrf_token()}"; + $.ajax({ + type: "POST", + url: "/myview", + headers: { 'X-CSRF-Token': csrfToken } + }).done(function() { + alert("Deleted"); + }); + +The handler for the URL that receives the request should then require that the +correct CSRF token is supplied. + +.. index:: + single: csrf.new_csrf_token + +Using the ``csrf.new_csrf_token`` Method +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +To explicitly create a new CSRF token, use the ``csrf.new_csrf_token()`` +method. This differs only from ``csrf.get_csrf_token()`` inasmuch as it +clears any existing CSRF token, creates a new CSRF token, sets the token into +the user, and returns the token. + +.. code-block:: python + + from pyramid.csrf import get_csrf_token + token = new_csrf_token() + +.. note:: + + It is not possible to force a new CSRF token from a template. If you + want to regenerate your CSRF token then do it in the view code and return + the new token as part of the context. + +Checking CSRF Tokens Manually +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In request handling code, you can check the presence and validity of a CSRF +token with :func:`pyramid.session.check_csrf_token`. If the token is valid, it +will return ``True``, otherwise it will raise ``HTTPBadRequest``. Optionally, +you can specify ``raises=False`` to have the check return ``False`` instead of +raising an exception. + +By default, it checks for a POST parameter named ``csrf_token`` or a header +named ``X-CSRF-Token``. + +.. code-block:: python + + from pyramid.session import check_csrf_token + + def myview(request): + # Require CSRF Token + check_csrf_token(request) + + # ... + +.. _auto_csrf_checking: + +Checking CSRF Tokens Automatically +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: 1.7 + +:app:`Pyramid` supports automatically checking CSRF tokens on requests with an +unsafe method as defined by RFC2616. Any other request may be checked manually. +This feature can be turned on globally for an application using the +:meth:`pyramid.config.Configurator.set_default_csrf_options` directive. +For example: + +.. code-block:: python + + from pyramid.config import Configurator + + config = Configurator() + config.set_default_csrf_options(require_csrf=True) + +CSRF checking may be explicitly enabled or disabled on a per-view basis using +the ``require_csrf`` view option. A value of ``True`` or ``False`` will +override the default set by ``set_default_csrf_options``. For example: + +.. code-block:: python + + @view_config(route_name='hello', require_csrf=False) + def myview(request): + # ... + +When CSRF checking is active, the token and header used to find the +supplied CSRF token will be ``csrf_token`` and ``X-CSRF-Token``, respectively, +unless otherwise overridden by ``set_default_csrf_options``. The token is +checked against the value in ``request.POST`` which is the submitted form body. +If this value is not present, then the header will be checked. + +In addition to token based CSRF checks, if the request is using HTTPS then the +automatic CSRF checking will also check the referrer of the request to ensure +that it matches one of the trusted origins. By default the only trusted origin +is the current host, however additional origins may be configured by setting +``pyramid.csrf_trusted_origins`` to a list of domain names (and ports if they +are non standard). If a host in the list of domains starts with a ``.`` then +that will allow all subdomains as well as the domain without the ``.``. + +If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or +:class:`pyramid.exceptions.BadCSRFOrigin` exception will be raised. This +exception may be caught and handled by an :term:`exception view` but, by +default, will result in a ``400 Bad Request`` response being sent to the +client. + +Checking CSRF Tokens with a View Predicate +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. deprecated:: 1.7 + Use the ``require_csrf`` option or read :ref:`auto_csrf_checking` instead + to have :class:`pyramid.exceptions.BadCSRFToken` exceptions raised. + +A convenient way to require a valid CSRF token for a particular view is to +include ``check_csrf=True`` as a view predicate. See +:meth:`pyramid.config.Configurator.add_view`. + +.. code-block:: python + + @view_config(request_method='POST', check_csrf=True, ...) + def myview(request): + ... + +.. note:: + A mismatch of a CSRF token is treated like any other predicate miss, and the + predicate system, when it doesn't find a view, raises ``HTTPNotFound`` + instead of ``HTTPBadRequest``, so ``check_csrf=True`` behavior is different + from calling :func:`pyramid.session.check_csrf_token`. diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 5b24201a9..90b5f4585 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -321,178 +321,3 @@ flash storage. single: preventing cross-site request forgery attacks single: cross-site request forgery attacks, prevention -Preventing Cross-Site Request Forgery Attacks ---------------------------------------------- - -`Cross-site request forgery -`_ attacks are a -phenomenon whereby a user who is logged in to your website might inadvertantly -load a URL because it is linked from, or embedded in, an attacker's website. -If the URL is one that may modify or delete data, the consequences can be dire. - -You can avoid most of these attacks by issuing a unique token to the browser -and then requiring that it be present in all potentially unsafe requests. -:app:`Pyramid` sessions provide facilities to create and check CSRF tokens. - -To use CSRF tokens, you must first enable a :term:`session factory` as -described in :ref:`using_the_default_session_factory` or -:ref:`using_alternate_session_factories`. - -.. index:: - single: session.get_csrf_token - -Using the ``session.get_csrf_token`` Method -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -To get the current CSRF token from the session, use the -``session.get_csrf_token()`` method. - -.. code-block:: python - - token = request.session.get_csrf_token() - -The ``session.get_csrf_token()`` method accepts no arguments. It returns a -CSRF *token* string. If ``session.get_csrf_token()`` or -``session.new_csrf_token()`` was invoked previously for this session, then the -existing token will be returned. If no CSRF token previously existed for this -session, then a new token will be set into the session and returned. The newly -created token will be opaque and randomized. - -You can use the returned token as the value of a hidden field in a form that -posts to a method that requires elevated privileges, or supply it as a request -header in AJAX requests. - -For example, include the CSRF token as a hidden field: - -.. code-block:: html - -
- - -
- -Or include it as a header in a jQuery AJAX request: - -.. code-block:: javascript - - var csrfToken = ${request.session.get_csrf_token()}; - $.ajax({ - type: "POST", - url: "/myview", - headers: { 'X-CSRF-Token': csrfToken } - }).done(function() { - alert("Deleted"); - }); - -The handler for the URL that receives the request should then require that the -correct CSRF token is supplied. - -.. index:: - single: session.new_csrf_token - -Using the ``session.new_csrf_token`` Method -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -To explicitly create a new CSRF token, use the ``session.new_csrf_token()`` -method. This differs only from ``session.get_csrf_token()`` inasmuch as it -clears any existing CSRF token, creates a new CSRF token, sets the token into -the session, and returns the token. - -.. code-block:: python - - token = request.session.new_csrf_token() - -Checking CSRF Tokens Manually -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -In request handling code, you can check the presence and validity of a CSRF -token with :func:`pyramid.session.check_csrf_token`. If the token is valid, it -will return ``True``, otherwise it will raise ``HTTPBadRequest``. Optionally, -you can specify ``raises=False`` to have the check return ``False`` instead of -raising an exception. - -By default, it checks for a POST parameter named ``csrf_token`` or a header -named ``X-CSRF-Token``. - -.. code-block:: python - - from pyramid.session import check_csrf_token - - def myview(request): - # Require CSRF Token - check_csrf_token(request) - - # ... - -.. _auto_csrf_checking: - -Checking CSRF Tokens Automatically -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. versionadded:: 1.7 - -:app:`Pyramid` supports automatically checking CSRF tokens on requests with an -unsafe method as defined by RFC2616. Any other request may be checked manually. -This feature can be turned on globally for an application using the -:meth:`pyramid.config.Configurator.set_default_csrf_options` directive. -For example: - -.. code-block:: python - - from pyramid.config import Configurator - - config = Configurator() - config.set_default_csrf_options(require_csrf=True) - -CSRF checking may be explicitly enabled or disabled on a per-view basis using -the ``require_csrf`` view option. A value of ``True`` or ``False`` will -override the default set by ``set_default_csrf_options``. For example: - -.. code-block:: python - - @view_config(route_name='hello', require_csrf=False) - def myview(request): - # ... - -When CSRF checking is active, the token and header used to find the -supplied CSRF token will be ``csrf_token`` and ``X-CSRF-Token``, respectively, -unless otherwise overridden by ``set_default_csrf_options``. The token is -checked against the value in ``request.POST`` which is the submitted form body. -If this value is not present, then the header will be checked. - -In addition to token based CSRF checks, if the request is using HTTPS then the -automatic CSRF checking will also check the referrer of the request to ensure -that it matches one of the trusted origins. By default the only trusted origin -is the current host, however additional origins may be configured by setting -``pyramid.csrf_trusted_origins`` to a list of domain names (and ports if they -are non standard). If a host in the list of domains starts with a ``.`` then -that will allow all subdomains as well as the domain without the ``.``. - -If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or -:class:`pyramid.exceptions.BadCSRFOrigin` exception will be raised. This -exception may be caught and handled by an :term:`exception view` but, by -default, will result in a ``400 Bad Request`` response being sent to the -client. - -Checking CSRF Tokens with a View Predicate -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -.. deprecated:: 1.7 - Use the ``require_csrf`` option or read :ref:`auto_csrf_checking` instead - to have :class:`pyramid.exceptions.BadCSRFToken` exceptions raised. - -A convenient way to require a valid CSRF token for a particular view is to -include ``check_csrf=True`` as a view predicate. See -:meth:`pyramid.config.Configurator.add_view`. - -.. code-block:: python - - @view_config(request_method='POST', check_csrf=True, ...) - def myview(request): - ... - -.. note:: - A mismatch of a CSRF token is treated like any other predicate miss, and the - predicate system, when it doesn't find a view, raises ``HTTPNotFound`` - instead of ``HTTPBadRequest``, so ``check_csrf=True`` behavior is different - from calling :func:`pyramid.session.check_csrf_token`. diff --git a/pyramid/config/security.py b/pyramid/config/security.py index 33593376b..102a61e0c 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -3,16 +3,21 @@ from zope.interface import implementer from pyramid.interfaces import ( IAuthorizationPolicy, IAuthenticationPolicy, + ICSRFPolicy, IDefaultCSRFOptions, IDefaultPermission, PHASE1_CONFIG, PHASE2_CONFIG, ) +from pyramid.csrf import csrf_token_template_global +from pyramid.csrf import SessionCSRF +from pyramid.events import BeforeRender from pyramid.exceptions import ConfigurationError from pyramid.util import action_method from pyramid.util import as_sorted_tuple + class SecurityConfiguratorMixin(object): @action_method def set_authentication_policy(self, policy): @@ -165,6 +170,7 @@ class SecurityConfiguratorMixin(object): @action_method def set_default_csrf_options( self, + implementation=None, require_csrf=True, token='csrf_token', header='X-CSRF-Token', @@ -174,6 +180,10 @@ class SecurityConfiguratorMixin(object): """ Set the default CSRF options used by subsequent view registrations. + ``implementation`` is a class that implements the + :meth:`pyramid.interfaces.ICSRFPolicy` interface that will be used for all + CSRF functionality. Default: :class:`pyramid.csrf.SessionCSRF`. + ``require_csrf`` controls whether CSRF checks will be automatically enabled on each view in the application. This value is used as the fallback when ``require_csrf`` is left at the default of ``None`` on @@ -207,7 +217,10 @@ class SecurityConfiguratorMixin(object): options = DefaultCSRFOptions( require_csrf, token, header, safe_methods, callback, ) + if implementation is None: + implementation = SessionCSRF() def register(): + self.registry.registerUtility(implementation, ICSRFPolicy) self.registry.registerUtility(options, IDefaultCSRFOptions) intr = self.introspectable('default csrf view options', None, @@ -218,9 +231,12 @@ class SecurityConfiguratorMixin(object): intr['header'] = header intr['safe_methods'] = as_sorted_tuple(safe_methods) intr['callback'] = callback + + self.add_subscriber(csrf_token_template_global, [BeforeRender]) self.action(IDefaultCSRFOptions, register, order=PHASE1_CONFIG, introspectables=(intr,)) + @implementer(IDefaultCSRFOptions) class DefaultCSRFOptions(object): def __init__(self, require_csrf, token, header, safe_methods, callback): diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 65c9da585..7a383be44 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -641,16 +641,14 @@ class ViewsConfiguratorMixin(object): 'check name'. If the value provided is ``True``, ``csrf_token`` will be used as the check name. - If CSRF checking is performed, the checked value will be the value - of ``request.params[check_name]``. This value will be compared - against the value of ``request.session.get_csrf_token()``, and the - check will pass if these two values are the same. If the check - passes, the associated view will be permitted to execute. If the + If CSRF checking is performed, the checked value will be the value of + ``request.params[check_name]``. This value will be compared against + the value of ``impl.get_csrf_token()`` (where ``impl`` is an + implementation of :meth:`pyramid.interfaces.ICSRFPolicy`), and the + check will pass if these two values are the same. If the check + passes, the associated view will be permitted to execute. If the check fails, the associated view will not be permitted to execute. - Note that using this feature requires a :term:`session factory` to - have been configured. - .. versionadded:: 1.4a2 physical_path diff --git a/pyramid/csrf.py b/pyramid/csrf.py new file mode 100644 index 000000000..c373079a4 --- /dev/null +++ b/pyramid/csrf.py @@ -0,0 +1,286 @@ +from functools import partial +import uuid + +from zope.interface import implementer + +from pyramid.compat import ( + urlparse, + bytes_ +) +from pyramid.exceptions import ( + BadCSRFOrigin, + BadCSRFToken, +) +from pyramid.interfaces import ICSRFPolicy +from pyramid.settings import aslist +from pyramid.util import ( + is_same_domain, + strings_differ +) + + +@implementer(ICSRFPolicy) +class SessionCSRF(object): + """ The default CSRF implementation, which mimics the behavior from older + versions of Pyramid. The ``new_csrf_token`` and ``get_csrf_token`` methods + are indirected to the underlying session implementation. + + Note that using this CSRF implementation requires that + a :term:`session factory` is configured. + + .. versionadded :: 1.8a1 + """ + def new_csrf_token(self, request): + """ Sets a new CSRF token into the session and returns it. """ + return request.session.new_csrf_token() + + def get_csrf_token(self, request): + """ Returns the currently active CSRF token from the session, generating + a new one if needed.""" + return request.session.get_csrf_token() + + def check_csrf_token(self, request, supplied_token): + """ Returns True if supplied_token is the same value as get_csrf_token + returns for this request. """ + expected = self.get_csrf_token(request) + return not strings_differ( + bytes_(expected, 'ascii'), + bytes_(supplied_token, 'ascii'), + ) + +@implementer(ICSRFPolicy) +class CookieCSRF(object): + """ An alternative CSRF implementation that stores its information in + unauthenticated cookies, known as the 'Double Submit Cookie' method in the + OWASP CSRF guidelines. This gives some additional flexibility with regards + to scalingas the tokens can be generated and verified by a front-end server. + + .. versionadded :: 1.8a1 + """ + + def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, + domain=None, path='/'): + self.cookie_name = cookie_name + self.secure = secure + self.httponly = httponly + self.domain = domain + self.path = path + + def new_csrf_token(self, request): + """ Sets a new CSRF token into the request and returns it. """ + token = uuid.uuid4().hex + def set_cookie(request, response): + response.set_cookie( + self.cookie_name, + token, + httponly=self.httponly, + secure=self.secure, + domain=self.domain, + path=self.path, + overwrite=True, + ) + request.add_response_callback(set_cookie) + return token + + def get_csrf_token(self, request): + """ Returns the currently active CSRF token by checking the cookies + sent with the current request.""" + token = request.cookies.get(self.cookie_name) + if not token: + token = self.new_csrf_token(request) + return token + + def check_csrf_token(self, request, supplied_token): + """ Returns True if supplied_token is the same value as get_csrf_token + returns for this request. """ + expected = self.get_csrf_token(request) + return not strings_differ( + bytes_(expected, 'ascii'), + bytes_(supplied_token, 'ascii'), + ) + + +def csrf_token_template_global(event): + request = event.get('request', None) + try: + registry = request.registry + except AttributeError: + return + else: + csrf = registry.getUtility(ICSRFPolicy) + if csrf is not None: + event['get_csrf_token'] = partial(csrf.get_csrf_token, request) + + +def get_csrf_token(request): + """ Get the currently active CSRF token for the request passed, generating + a new one using ``new_csrf_token(request)`` if one does not exist. This + calls the equivalent method in the chosen CSRF protection implementation. + + .. versionadded :: 1.8a1 + """ + registry = request.registry + csrf = registry.getUtility(ICSRFPolicy) + if csrf is not None: + return csrf.get_csrf_token(request) + + +def new_csrf_token(request): + """ Generate a new CSRF token for the request passed and persist it in an + implementation defined manner. This calls the equivalent method in the + chosen CSRF protection implementation. + + .. versionadded :: 1.8a1 + """ + registry = request.registry + csrf = registry.getUtility(ICSRFPolicy) + if csrf is not None: + return csrf.new_csrf_token(request) + + +def check_csrf_token(request, + token='csrf_token', + header='X-CSRF-Token', + raises=True): + """ Check the CSRF token returned by the :meth:`pyramid.interfaces.ICSRFPolicy` + implementation against the value in ``request.POST.get(token)`` (if a POST + request) or ``request.headers.get(header)``. If a ``token`` keyword is not + supplied to this function, the string ``csrf_token`` will be used to look + up the token in ``request.POST``. If a ``header`` keyword is not supplied + to this function, the string ``X-CSRF-Token`` will be used to look up the + token in ``request.headers``. + + If the value supplied by post or by header doesn't match the value supplied + by ``impl.get_csrf_token()`` (where ``impl`` is an implementation of + :meth:`pyramid.interfaces.ICSRFPolicy`), and ``raises`` is ``True``, this + function will raise an :exc:`pyramid.exceptions.BadCSRFToken` exception. If + the values differ and ``raises`` is ``False``, this function will return + ``False``. If the CSRF check is successful, this function will return + ``True`` unconditionally. + + See :ref:`auto_csrf_checking` for information about how to secure your + application automatically against CSRF attacks. + + .. versionadded:: 1.4a2 + + .. versionchanged:: 1.7a1 + A CSRF token passed in the query string of the request is no longer + considered valid. It must be passed in either the request body or + a header. + + .. versionchanged:: 1.8a1 + Moved from pyramid.session to pyramid.csrf + """ + supplied_token = "" + # We first check the headers for a csrf token, as that is significantly + # cheaper than checking the POST body + if header is not None: + supplied_token = request.headers.get(header, "") + + # If this is a POST/PUT/etc request, then we'll check the body to see if it + # has a token. We explicitly use request.POST here because CSRF tokens + # should never appear in an URL as doing so is a security issue. We also + # explicitly check for request.POST here as we do not support sending form + # encoded data over anything but a request.POST. + if supplied_token == "" and token is not None: + supplied_token = request.POST.get(token, "") + + policy = request.registry.getUtility(ICSRFPolicy) + if not policy.check_csrf_token(request, supplied_token): + if raises: + raise BadCSRFToken('check_csrf_token(): Invalid token') + return False + return True + + +def check_csrf_origin(request, trusted_origins=None, raises=True): + """ + Check the Origin of the request to see if it is a cross site request or + not. + + If the value supplied by the Origin or Referer header isn't one of the + trusted origins and ``raises`` is ``True``, this function will raise a + :exc:`pyramid.exceptions.BadCSRFOrigin` exception but if ``raises`` is + ``False`` this function will return ``False`` instead. If the CSRF origin + checks are successful this function will return ``True`` unconditionally. + + Additional trusted origins may be added by passing a list of domain (and + ports if nonstandard like `['example.com', 'dev.example.com:8080']`) in + with the ``trusted_origins`` parameter. If ``trusted_origins`` is ``None`` + (the default) this list of additional domains will be pulled from the + ``pyramid.csrf_trusted_origins`` setting. + + Note that this function will do nothing if request.scheme is not https. + + .. versionadded:: 1.7 + + .. versionchanged:: 1.8a1 + Moved from pyramid.session to pyramid.csrf + """ + def _fail(reason): + if raises: + raise BadCSRFOrigin(reason) + else: + return False + + if request.scheme == "https": + # Suppose user visits http://example.com/ + # An active network attacker (man-in-the-middle, MITM) sends a + # POST form that targets https://example.com/detonate-bomb/ and + # submits it via JavaScript. + # + # The attacker will need to provide a CSRF cookie and token, but + # that's no problem for a MITM when we cannot make any assumptions + # about what kind of session storage is being used. So the MITM can + # circumvent the CSRF protection. This is true for any HTTP connection, + # but anyone using HTTPS expects better! For this reason, for + # https://example.com/ we need additional protection that treats + # http://example.com/ as completely untrusted. Under HTTPS, + # Barth et al. found that the Referer header is missing for + # same-domain requests in only about 0.2% of cases or less, so + # we can use strict Referer checking. + + # Determine the origin of this request + origin = request.headers.get("Origin") + if origin is None: + origin = request.referrer + + # Fail if we were not able to locate an origin at all + if not origin: + return _fail("Origin checking failed - no Origin or Referer.") + + # Parse our origin so we we can extract the required information from + # it. + originp = urlparse.urlparse(origin) + + # Ensure that our Referer is also secure. + if originp.scheme != "https": + return _fail( + "Referer checking failed - Referer is insecure while host is " + "secure." + ) + + # Determine which origins we trust, which by default will include the + # current origin. + if trusted_origins is None: + trusted_origins = aslist( + request.registry.settings.get( + "pyramid.csrf_trusted_origins", []) + ) + + if request.host_port not in set(["80", "443"]): + trusted_origins.append("{0.domain}:{0.host_port}".format(request)) + else: + trusted_origins.append(request.domain) + + # Actually check to see if the request's origin matches any of our + # trusted origins. + if not any(is_same_domain(originp.netloc, host) + for host in trusted_origins): + reason = ( + "Referer checking failed - {0} does not match any trusted " + "origins." + ) + return _fail(reason.format(origin)) + + return True diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index bbb4754e4..f58ee8b58 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -981,19 +981,30 @@ class ISession(IDict): :meth:`pyramid.interfaces.ISession.flash` """ - def new_csrf_token(): - """ Create and set into the session a new, random cross-site request - forgery protection token. Return the token. It will be a string.""" - def get_csrf_token(): - """ Return a random cross-site request forgery protection token. It - will be a string. If a token was previously added to the session via +class ICSRFPolicy(Interface): + """ An object that offers the ability to verify CSRF tokens and generate + new ones""" + + def new_csrf_token(request): + """ Create and return a new, random cross-site request forgery + protection token. Return the token. It will be a string.""" + + def get_csrf_token(request): + """ Return a cross-site request forgery protection token. It + will be a string. If a token was previously set for this user via ``new_csrf_token``, that token will be returned. If no CSRF token - was previously set into the session, ``new_csrf_token`` will be - called, which will create and set a token, and this token will be - returned. + was previously set, ``new_csrf_token`` will be called, which will + create and set a token, and this token will be returned. """ + def check_csrf_token(request, supplied_token): + """ Returns a boolean that represents if supplied_token is a valid CSRF + token for this request. Comparing strings for equality must be done + using :func:`pyramid.utils.strings_differ` to avoid timing attacks. + """ + + class IIntrospector(Interface): def get(category_name, discriminator, default=None): """ Get the IIntrospectable related to the category_name and the diff --git a/pyramid/predicates.py b/pyramid/predicates.py index 7c3a778ca..3d7bb1b4b 100644 --- a/pyramid/predicates.py +++ b/pyramid/predicates.py @@ -4,7 +4,7 @@ from pyramid.exceptions import ConfigurationError from pyramid.compat import is_nonstr_iter -from pyramid.session import check_csrf_token +from pyramid.csrf import check_csrf_token from pyramid.traversal import ( find_interface, traversal_path, diff --git a/pyramid/renderers.py b/pyramid/renderers.py index 47705d5d9..7d667ba7b 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -447,7 +447,6 @@ class RendererHelper(object): registry = self.registry registry.notify(system_values) - result = renderer(value, system_values) return result diff --git a/pyramid/session.py b/pyramid/session.py index 47b80f617..b1ad25410 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -16,19 +16,11 @@ from pyramid.compat import ( text_, bytes_, native_, - urlparse, ) -from pyramid.exceptions import ( - BadCSRFOrigin, - BadCSRFToken, -) from pyramid.interfaces import ISession -from pyramid.settings import aslist -from pyramid.util import ( - is_same_domain, - strings_differ, -) +from pyramid.util import strings_differ + def manage_accessed(wrapped): """ Decorator which causes a cookie to be renewed when an accessor @@ -109,149 +101,6 @@ def signed_deserialize(serialized, secret, hmac=hmac): return pickle.loads(pickled) -def check_csrf_origin(request, trusted_origins=None, raises=True): - """ - Check the Origin of the request to see if it is a cross site request or - not. - - If the value supplied by the Origin or Referer header isn't one of the - trusted origins and ``raises`` is ``True``, this function will raise a - :exc:`pyramid.exceptions.BadCSRFOrigin` exception but if ``raises`` is - ``False`` this function will return ``False`` instead. If the CSRF origin - checks are successful this function will return ``True`` unconditionally. - - Additional trusted origins may be added by passing a list of domain (and - ports if nonstandard like `['example.com', 'dev.example.com:8080']`) in - with the ``trusted_origins`` parameter. If ``trusted_origins`` is ``None`` - (the default) this list of additional domains will be pulled from the - ``pyramid.csrf_trusted_origins`` setting. - - Note that this function will do nothing if request.scheme is not https. - - .. versionadded:: 1.7 - """ - def _fail(reason): - if raises: - raise BadCSRFOrigin(reason) - else: - return False - - if request.scheme == "https": - # Suppose user visits http://example.com/ - # An active network attacker (man-in-the-middle, MITM) sends a - # POST form that targets https://example.com/detonate-bomb/ and - # submits it via JavaScript. - # - # The attacker will need to provide a CSRF cookie and token, but - # that's no problem for a MITM when we cannot make any assumptions - # about what kind of session storage is being used. So the MITM can - # circumvent the CSRF protection. This is true for any HTTP connection, - # but anyone using HTTPS expects better! For this reason, for - # https://example.com/ we need additional protection that treats - # http://example.com/ as completely untrusted. Under HTTPS, - # Barth et al. found that the Referer header is missing for - # same-domain requests in only about 0.2% of cases or less, so - # we can use strict Referer checking. - - # Determine the origin of this request - origin = request.headers.get("Origin") - if origin is None: - origin = request.referrer - - # Fail if we were not able to locate an origin at all - if not origin: - return _fail("Origin checking failed - no Origin or Referer.") - - # Parse our origin so we we can extract the required information from - # it. - originp = urlparse.urlparse(origin) - - # Ensure that our Referer is also secure. - if originp.scheme != "https": - return _fail( - "Referer checking failed - Referer is insecure while host is " - "secure." - ) - - # Determine which origins we trust, which by default will include the - # current origin. - if trusted_origins is None: - trusted_origins = aslist( - request.registry.settings.get( - "pyramid.csrf_trusted_origins", []) - ) - - if request.host_port not in set(["80", "443"]): - trusted_origins.append("{0.domain}:{0.host_port}".format(request)) - else: - trusted_origins.append(request.domain) - - # Actually check to see if the request's origin matches any of our - # trusted origins. - if not any(is_same_domain(originp.netloc, host) - for host in trusted_origins): - reason = ( - "Referer checking failed - {0} does not match any trusted " - "origins." - ) - return _fail(reason.format(origin)) - - return True - - -def check_csrf_token(request, - token='csrf_token', - header='X-CSRF-Token', - raises=True): - """ Check the CSRF token in the request's session against the value in - ``request.POST.get(token)`` (if a POST request) or - ``request.headers.get(header)``. If a ``token`` keyword is not supplied to - this function, the string ``csrf_token`` will be used to look up the token - in ``request.POST``. If a ``header`` keyword is not supplied to this - function, the string ``X-CSRF-Token`` will be used to look up the token in - ``request.headers``. - - If the value supplied by post or by header doesn't match the value - supplied by ``request.session.get_csrf_token()``, and ``raises`` is - ``True``, this function will raise an - :exc:`pyramid.exceptions.BadCSRFToken` exception. - If the values differ and ``raises`` is ``False``, this function will - return ``False``. If the CSRF check is successful, this function will - return ``True`` unconditionally. - - Note that using this function requires that a :term:`session factory` is - configured. - - See :ref:`auto_csrf_checking` for information about how to secure your - application automatically against CSRF attacks. - - .. versionadded:: 1.4a2 - - .. versionchanged:: 1.7a1 - A CSRF token passed in the query string of the request is no longer - considered valid. It must be passed in either the request body or - a header. - """ - supplied_token = "" - # If this is a POST/PUT/etc request, then we'll check the body to see if it - # has a token. We explicitly use request.POST here because CSRF tokens - # should never appear in an URL as doing so is a security issue. We also - # explicitly check for request.POST here as we do not support sending form - # encoded data over anything but a request.POST. - if token is not None: - supplied_token = request.POST.get(token, "") - - # If we were unable to locate a CSRF token in a request body, then we'll - # check to see if there are any headers that have a value for us. - if supplied_token == "" and header is not None: - supplied_token = request.headers.get(header, "") - - expected_token = request.session.get_csrf_token() - if strings_differ(bytes_(expected_token), bytes_(supplied_token)): - if raises: - raise BadCSRFToken('check_csrf_token(): Invalid token') - return False - return True class PickleSerializer(object): """ A serializer that uses the pickle protocol to dump Python diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 211632730..45495f1fa 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -2373,7 +2373,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): view = lambda r: 'OK' config.set_default_csrf_options(require_csrf=True) config.add_view(view, context=Exception, renderer=null_renderer) - view_intr = introspector.introspectables[1] + view_intr = introspector.introspectables[-1] self.assertTrue(view_intr.type_name, 'view') self.assertEqual(view_intr['callable'], view) derived_view = view_intr['derived_callable'] diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py new file mode 100644 index 000000000..a74d2a07b --- /dev/null +++ b/pyramid/tests/test_csrf.py @@ -0,0 +1,172 @@ +import unittest + +from pyramid.config import Configurator +from pyramid.csrf import CookieCSRF, SessionCSRF, get_csrf_token, new_csrf_token +from pyramid.events import BeforeRender +from pyramid.interfaces import ICSRFPolicy +from pyramid.tests.test_view import BaseTest as ViewBaseTest + + +class CSRFTokenTests(ViewBaseTest, unittest.TestCase): + class DummyCSRF(object): + def new_csrf_token(self, request): + return 'e5e9e30a08b34ff9842ff7d2b958c14b' + + def get_csrf_token(self, request): + return '02821185e4c94269bdc38e6eeae0a2f8' + + def test_csrf_token_passed_to_template(self): + config = Configurator() + config.set_default_csrf_options(implementation=self.DummyCSRF) + config.commit() + + request = self._makeRequest() + request.registry = config.registry + + before = BeforeRender({'request': request}, {}) + config.registry.notify(before) + self.assertIn('get_csrf_token', before) + self.assertEqual( + before['get_csrf_token'](), + '02821185e4c94269bdc38e6eeae0a2f8' + ) + + def test_simple_api_for_tokens_from_python(self): + config = Configurator() + config.set_default_csrf_options(implementation=self.DummyCSRF) + config.commit() + + request = self._makeRequest() + request.registry = config.registry + self.assertEqual( + get_csrf_token(request), + '02821185e4c94269bdc38e6eeae0a2f8' + ) + self.assertEqual( + new_csrf_token(request), + 'e5e9e30a08b34ff9842ff7d2b958c14b' + ) + + +class SessionCSRFTests(unittest.TestCase): + class MockSession(object): + def new_csrf_token(self): + return 'e5e9e30a08b34ff9842ff7d2b958c14b' + + def get_csrf_token(self): + return '02821185e4c94269bdc38e6eeae0a2f8' + + def test_session_csrf_implementation_delegates_to_session(self): + config = Configurator() + config.set_default_csrf_options(implementation=SessionCSRF) + config.commit() + + request = DummyRequest(config.registry, session=self.MockSession()) + self.assertEqual( + config.registry.getUtility(ICSRFPolicy).get_csrf_token(request), + '02821185e4c94269bdc38e6eeae0a2f8' + ) + self.assertEqual( + config.registry.getUtility(ICSRFPolicy).new_csrf_token(request), + 'e5e9e30a08b34ff9842ff7d2b958c14b' + ) + + +class CookieCSRFTests(unittest.TestCase): + + def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self): + config = Configurator() + config.set_default_csrf_options(implementation=CookieCSRF()) + config.commit() + + response = MockResponse() + request = DummyRequest(config.registry, response=response) + + token = config.registry.getUtility(ICSRFPolicy).get_csrf_token(request) + self.assertEqual( + response.called_args, + ('csrf_token', token), + ) + self.assertEqual( + response.called_kwargs, + { + 'secure': False, + 'httponly': False, + 'domain': None, + 'path': '/', + 'overwrite': True + } + ) + + def test_existing_cookie_csrf_does_not_set_cookie(self): + config = Configurator() + config.set_default_csrf_options(implementation=CookieCSRF()) + config.commit() + + response = MockResponse() + request = DummyRequest(config.registry, response=response) + request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + + token = config.registry.getUtility(ICSRFPolicy).get_csrf_token(request) + self.assertEqual( + token, + 'e6f325fee5974f3da4315a8ccf4513d2' + ) + self.assertEqual( + response.called_args, + (), + ) + self.assertEqual( + response.called_kwargs, + {} + ) + + def test_new_cookie_csrf_with_existing_cookie_sets_cookies(self): + config = Configurator() + config.set_default_csrf_options(implementation=CookieCSRF()) + config.commit() + + response = MockResponse() + request = DummyRequest(config.registry, response=response) + request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + + token = config.registry.getUtility(ICSRFPolicy).new_csrf_token(request) + self.assertEqual( + response.called_args, + ('csrf_token', token), + ) + self.assertEqual( + response.called_kwargs, + { + 'secure': False, + 'httponly': False, + 'domain': None, + 'path': '/', + 'overwrite': True + } + ) + + +class DummyRequest(object): + registry = None + session = None + cookies = {} + + def __init__(self, registry, session=None, response=None): + self.registry = registry + self.session = session + self.response = response + + def add_response_callback(self, callback): + callback(self, self.response) + + +class MockResponse(object): + def __init__(self): + self.called_args = () + self.called_kwargs = {} + + def set_cookie(self, *args, **kwargs): + self.called_args = args + self.called_kwargs = kwargs + return diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index 3a308d08b..b51dccecc 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -661,7 +661,7 @@ class Test_signed_deserialize(unittest.TestCase): class Test_check_csrf_token(unittest.TestCase): def _callFUT(self, *args, **kwargs): - from ..session import check_csrf_token + from ..csrf import check_csrf_token return check_csrf_token(*args, **kwargs) def test_success_token(self): @@ -709,7 +709,7 @@ class Test_check_csrf_token(unittest.TestCase): class Test_check_csrf_origin(unittest.TestCase): def _callFUT(self, *args, **kwargs): - from ..session import check_csrf_origin + from ..csrf import check_csrf_origin return check_csrf_origin(*args, **kwargs) def test_success_with_http(self): diff --git a/pyramid/viewderivers.py b/pyramid/viewderivers.py index 4eb0ce704..d2869b162 100644 --- a/pyramid/viewderivers.py +++ b/pyramid/viewderivers.py @@ -6,7 +6,7 @@ from zope.interface import ( ) from pyramid.security import NO_PERMISSION_REQUIRED -from pyramid.session import ( +from pyramid.csrf import ( check_csrf_origin, check_csrf_token, ) -- cgit v1.2.3 From 313c251497f6cdb3e5ca961a8092a2356aa502fc Mon Sep 17 00:00:00 2001 From: Jure Cerjak Date: Mon, 5 Dec 2016 16:06:08 +0100 Subject: Fix tests and documentation in various places, and feedback following review regarding naming of variables and code cleanup. --- docs/api/csrf.rst | 10 +- docs/api/interfaces.rst | 2 +- docs/narr/security.rst | 34 ++- docs/narr/sessions.rst | 4 +- pyramid/config/views.py | 2 +- pyramid/csrf.py | 35 ++- pyramid/tests/test_config/test_views.py | 1 + pyramid/tests/test_csrf.py | 367 +++++++++++++++++++++++++++----- pyramid/tests/test_session.py | 138 ------------ pyramid/tests/test_viewderivers.py | 1 + 10 files changed, 369 insertions(+), 225 deletions(-) diff --git a/docs/api/csrf.rst b/docs/api/csrf.rst index 3125bdac9..89fb0c4b2 100644 --- a/docs/api/csrf.rst +++ b/docs/api/csrf.rst @@ -5,14 +5,16 @@ .. automodule:: pyramid.csrf + .. autoclass:: SessionCSRF + :members: + + .. autoclass:: CookieCSRF + :members: + .. autofunction:: get_csrf_token .. autofunction:: new_csrf_token - .. autoclass:: SessionCSRF - :members: - .. autofunction:: check_csrf_origin .. autofunction:: check_csrf_token - diff --git a/docs/api/interfaces.rst b/docs/api/interfaces.rst index 2ca472616..b88209a36 100644 --- a/docs/api/interfaces.rst +++ b/docs/api/interfaces.rst @@ -44,7 +44,7 @@ Other Interfaces .. autointerface:: IRoutePregenerator :members: - .. autointerface:: ICSRF + .. autointerface:: ICSRFPolicy :members: .. autointerface:: ISession diff --git a/docs/narr/security.rst b/docs/narr/security.rst index b4fb3b8a8..6962a0fe3 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -146,7 +146,7 @@ For example, the following view declaration protects the view named # config is an instance of pyramid.config.Configurator config.add_view('mypackage.views.blog_entry_add_view', - name='add_entry.html', + name='add_entry.html', context='mypackage.resources.Blog', permission='add') @@ -725,7 +725,7 @@ object that implements the following interface: """ Return ``True`` if any of the ``principals`` is allowed the ``permission`` in the current ``context``, else return ``False`` """ - + def principals_allowed_by_permission(self, context, permission): """ Return a set of principal identifiers allowed by the ``permission`` in ``context``. This behavior is optional; if you @@ -777,11 +777,27 @@ If the URL is one that may modify or delete data, the consequences can be dire. You can avoid most of these attacks by issuing a unique token to the browser and then requiring that it be present in all potentially unsafe requests. -:app:`Pyramid` sessions provide facilities to create and check CSRF tokens. +:app:`Pyramid` provides facilities to create and check CSRF tokens. + +By default :app:`Pyramid` comes with a session-based CSRF implementation +:class:`pyramid.csrf.SessionCSRF`. To use it, you must first enable +a :term:`session factory` as described in +:ref:`using_the_default_session_factory` or +:ref:`using_alternate_session_factories`. Alternatively, you can use +a cookie-based implementation :class:`pyramid.csrf.CookieCSRF` which gives +some additional flexibility as it does not require a session for each user. +You can also define your own implementation of +:class:`pyramid.interfaces.ICSRFPolicy` and register it with the +:meth:`pyramid.config.Configurator.set_default_csrf_options` directive. -To use CSRF tokens, you must first enable a :term:`session factory` as -described in :ref:`using_the_default_session_factory` or -:ref:`using_alternate_session_factories`. +For example: + +.. code-block:: python + + from pyramid.config import Configurator + + config = Configurator() + config.set_default_csrf_options(implementation=MyCustomCSRFPolicy()) .. index:: single: csrf.get_csrf_token @@ -866,7 +882,7 @@ Checking CSRF Tokens Manually ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In request handling code, you can check the presence and validity of a CSRF -token with :func:`pyramid.session.check_csrf_token`. If the token is valid, it +token with :func:`pyramid.csrf.check_csrf_token`. If the token is valid, it will return ``True``, otherwise it will raise ``HTTPBadRequest``. Optionally, you can specify ``raises=False`` to have the check return ``False`` instead of raising an exception. @@ -876,7 +892,7 @@ named ``X-CSRF-Token``. .. code-block:: python - from pyramid.session import check_csrf_token + from pyramid.csrf import check_csrf_token def myview(request): # Require CSRF Token @@ -955,4 +971,4 @@ include ``check_csrf=True`` as a view predicate. See A mismatch of a CSRF token is treated like any other predicate miss, and the predicate system, when it doesn't find a view, raises ``HTTPNotFound`` instead of ``HTTPBadRequest``, so ``check_csrf=True`` behavior is different - from calling :func:`pyramid.session.check_csrf_token`. + from calling :func:`pyramid.csrf.check_csrf_token`. diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 90b5f4585..86fe2a139 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -12,8 +12,7 @@ application. This chapter describes how to configure sessions, what session implementations :app:`Pyramid` provides out of the box, how to store and retrieve data from -sessions, and two session-specific features: flash messages, and cross-site -request forgery attack prevention. +sessions, and a session-specific feature: flash messages. .. index:: single: session factory (default) @@ -320,4 +319,3 @@ flash storage. .. index:: single: preventing cross-site request forgery attacks single: cross-site request forgery attacks, prevention - diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 7a383be44..4ebd014de 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -643,7 +643,7 @@ class ViewsConfiguratorMixin(object): If CSRF checking is performed, the checked value will be the value of ``request.params[check_name]``. This value will be compared against - the value of ``impl.get_csrf_token()`` (where ``impl`` is an + the value of ``policy.get_csrf_token()`` (where ``policy`` is an implementation of :meth:`pyramid.interfaces.ICSRFPolicy`), and the check will pass if these two values are the same. If the check passes, the associated view will be permitted to execute. If the diff --git a/pyramid/csrf.py b/pyramid/csrf.py index c373079a4..7adbc9fee 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -53,11 +53,12 @@ class CookieCSRF(object): """ An alternative CSRF implementation that stores its information in unauthenticated cookies, known as the 'Double Submit Cookie' method in the OWASP CSRF guidelines. This gives some additional flexibility with regards - to scalingas the tokens can be generated and verified by a front-end server. - + to scaling as the tokens can be generated and verified by a front-end + server. + .. versionadded :: 1.8a1 """ - + def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, domain=None, path='/'): self.cookie_name = cookie_name @@ -108,8 +109,7 @@ def csrf_token_template_global(event): return else: csrf = registry.getUtility(ICSRFPolicy) - if csrf is not None: - event['get_csrf_token'] = partial(csrf.get_csrf_token, request) + event['get_csrf_token'] = partial(csrf.get_csrf_token, request) def get_csrf_token(request): @@ -121,8 +121,7 @@ def get_csrf_token(request): """ registry = request.registry csrf = registry.getUtility(ICSRFPolicy) - if csrf is not None: - return csrf.get_csrf_token(request) + return csrf.get_csrf_token(request) def new_csrf_token(request): @@ -134,25 +133,25 @@ def new_csrf_token(request): """ registry = request.registry csrf = registry.getUtility(ICSRFPolicy) - if csrf is not None: - return csrf.new_csrf_token(request) + return csrf.new_csrf_token(request) def check_csrf_token(request, token='csrf_token', header='X-CSRF-Token', raises=True): - """ Check the CSRF token returned by the :meth:`pyramid.interfaces.ICSRFPolicy` - implementation against the value in ``request.POST.get(token)`` (if a POST - request) or ``request.headers.get(header)``. If a ``token`` keyword is not - supplied to this function, the string ``csrf_token`` will be used to look - up the token in ``request.POST``. If a ``header`` keyword is not supplied - to this function, the string ``X-CSRF-Token`` will be used to look up the - token in ``request.headers``. + """ Check the CSRF token returned by the + :class:`pyramid.interfaces.ICSRFPolicy` implementation against the value in + ``request.POST.get(token)`` (if a POST request) or + ``request.headers.get(header)``. If a ``token`` keyword is not supplied to + this function, the string ``csrf_token`` will be used to look up the token + in ``request.POST``. If a ``header`` keyword is not supplied to this + function, the string ``X-CSRF-Token`` will be used to look up the token in + ``request.headers``. If the value supplied by post or by header doesn't match the value supplied - by ``impl.get_csrf_token()`` (where ``impl`` is an implementation of - :meth:`pyramid.interfaces.ICSRFPolicy`), and ``raises`` is ``True``, this + by ``policy.get_csrf_token()`` (where ``policy`` is an implementation of + :class:`pyramid.interfaces.ICSRFPolicy`), and ``raises`` is ``True``, this function will raise an :exc:`pyramid.exceptions.BadCSRFToken` exception. If the values differ and ``raises`` is ``False``, this function will return ``False``. If the CSRF check is successful, this function will return diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 45495f1fa..0816d9958 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -18,6 +18,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): def _makeOne(self, *arg, **kw): from pyramid.config import Configurator config = Configurator(*arg, **kw) + config.set_default_csrf_options(require_csrf=False) return config def _getViewCallable(self, config, ctx_iface=None, exc_iface=None, diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index a74d2a07b..1b3f3fc3b 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -1,54 +1,109 @@ import unittest +from zope.interface.interfaces import ComponentLookupError + +from pyramid import testing from pyramid.config import Configurator -from pyramid.csrf import CookieCSRF, SessionCSRF, get_csrf_token, new_csrf_token from pyramid.events import BeforeRender -from pyramid.interfaces import ICSRFPolicy -from pyramid.tests.test_view import BaseTest as ViewBaseTest -class CSRFTokenTests(ViewBaseTest, unittest.TestCase): - class DummyCSRF(object): - def new_csrf_token(self, request): - return 'e5e9e30a08b34ff9842ff7d2b958c14b' +class Test_get_csrf_token(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() - def get_csrf_token(self, request): - return '02821185e4c94269bdc38e6eeae0a2f8' + def _callFUT(self, *args, **kwargs): + from pyramid.csrf import get_csrf_token + return get_csrf_token(*args, **kwargs) + + def test_no_csrf_utility_registered(self): + request = testing.DummyRequest() + + with self.assertRaises(ComponentLookupError): + self._callFUT(request) + + def test_success(self): + self.config.set_default_csrf_options(implementation=DummyCSRF()) + request = testing.DummyRequest() + + csrf_token = self._callFUT(request) + + self.assertEquals(csrf_token, '02821185e4c94269bdc38e6eeae0a2f8') + + +class Test_new_csrf_token(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + def _callFUT(self, *args, **kwargs): + from pyramid.csrf import new_csrf_token + return new_csrf_token(*args, **kwargs) + + def test_no_csrf_utility_registered(self): + request = testing.DummyRequest() + + with self.assertRaises(ComponentLookupError): + self._callFUT(request) + + def test_success(self): + self.config.set_default_csrf_options(implementation=DummyCSRF()) + request = testing.DummyRequest() + + csrf_token = self._callFUT(request) + + self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b') + + +class Test_csrf_token_template_global(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + def _callFUT(self, *args, **kwargs): + from pyramid.csrf import csrf_token_template_global + return csrf_token_template_global(*args, **kwargs) + + def test_event_is_missing_request(self): + event = BeforeRender({}, {}) + + self._callFUT(event) + + self.assertNotIn('get_csrf_token', event) + + def test_request_is_missing_registry(self): + request = DummyRequest(registry=None) + del request.registry + del request.__class__.registry + event = BeforeRender({'request': request}, {}) + + self._callFUT(event) + + self.assertNotIn('get_csrf_token', event) + + def test_csrf_utility_not_registered(self): + request = testing.DummyRequest() + event = BeforeRender({'request': request}, {}) + + with self.assertRaises(ComponentLookupError): + self._callFUT(event) def test_csrf_token_passed_to_template(self): config = Configurator() - config.set_default_csrf_options(implementation=self.DummyCSRF) + config.set_default_csrf_options(implementation=DummyCSRF()) config.commit() - request = self._makeRequest() + request = testing.DummyRequest() request.registry = config.registry before = BeforeRender({'request': request}, {}) config.registry.notify(before) + self.assertIn('get_csrf_token', before) self.assertEqual( before['get_csrf_token'](), '02821185e4c94269bdc38e6eeae0a2f8' ) - def test_simple_api_for_tokens_from_python(self): - config = Configurator() - config.set_default_csrf_options(implementation=self.DummyCSRF) - config.commit() - - request = self._makeRequest() - request.registry = config.registry - self.assertEqual( - get_csrf_token(request), - '02821185e4c94269bdc38e6eeae0a2f8' - ) - self.assertEqual( - new_csrf_token(request), - 'e5e9e30a08b34ff9842ff7d2b958c14b' - ) - -class SessionCSRFTests(unittest.TestCase): +class TestSessionCSRF(unittest.TestCase): class MockSession(object): def new_csrf_token(self): return 'e5e9e30a08b34ff9842ff7d2b958c14b' @@ -56,33 +111,75 @@ class SessionCSRFTests(unittest.TestCase): def get_csrf_token(self): return '02821185e4c94269bdc38e6eeae0a2f8' - def test_session_csrf_implementation_delegates_to_session(self): + def _makeOne(self): + from pyramid.csrf import SessionCSRF + return SessionCSRF() + + def test_register_session_csrf_policy(self): + from pyramid.csrf import SessionCSRF + from pyramid.interfaces import ICSRFPolicy + config = Configurator() - config.set_default_csrf_options(implementation=SessionCSRF) + config.set_default_csrf_options(implementation=self._makeOne()) config.commit() - request = DummyRequest(config.registry, session=self.MockSession()) + policy = config.registry.queryUtility(ICSRFPolicy) + + self.assertTrue(isinstance(policy, SessionCSRF)) + + def test_session_csrf_implementation_delegates_to_session(self): + policy = self._makeOne() + request = DummyRequest(session=self.MockSession()) + self.assertEqual( - config.registry.getUtility(ICSRFPolicy).get_csrf_token(request), + policy.get_csrf_token(request), '02821185e4c94269bdc38e6eeae0a2f8' ) self.assertEqual( - config.registry.getUtility(ICSRFPolicy).new_csrf_token(request), + policy.new_csrf_token(request), 'e5e9e30a08b34ff9842ff7d2b958c14b' ) + def test_verifying_token_invalid(self): + policy = self._makeOne() + request = DummyRequest(session=self.MockSession()) -class CookieCSRFTests(unittest.TestCase): + result = policy.check_csrf_token(request, 'invalid-token') + self.assertFalse(result) + + def test_verifying_token_valid(self): + policy = self._makeOne() + request = DummyRequest(session=self.MockSession()) + + result = policy.check_csrf_token( + request, '02821185e4c94269bdc38e6eeae0a2f8') + self.assertTrue(result) + + +class TestCookieCSRF(unittest.TestCase): + def _makeOne(self): + from pyramid.csrf import CookieCSRF + return CookieCSRF() + + def test_register_cookie_csrf_policy(self): + from pyramid.csrf import CookieCSRF + from pyramid.interfaces import ICSRFPolicy - def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self): config = Configurator() - config.set_default_csrf_options(implementation=CookieCSRF()) + config.set_default_csrf_options(implementation=self._makeOne()) config.commit() + policy = config.registry.queryUtility(ICSRFPolicy) + + self.assertTrue(isinstance(policy, CookieCSRF)) + + def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self): response = MockResponse() - request = DummyRequest(config.registry, response=response) + request = DummyRequest(response=response) + + policy = self._makeOne() + token = policy.get_csrf_token(request) - token = config.registry.getUtility(ICSRFPolicy).get_csrf_token(request) self.assertEqual( response.called_args, ('csrf_token', token), @@ -99,15 +196,13 @@ class CookieCSRFTests(unittest.TestCase): ) def test_existing_cookie_csrf_does_not_set_cookie(self): - config = Configurator() - config.set_default_csrf_options(implementation=CookieCSRF()) - config.commit() - response = MockResponse() - request = DummyRequest(config.registry, response=response) + request = DummyRequest(response=response) request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} - token = config.registry.getUtility(ICSRFPolicy).get_csrf_token(request) + policy = self._makeOne() + token = policy.get_csrf_token(request) + self.assertEqual( token, 'e6f325fee5974f3da4315a8ccf4513d2' @@ -122,15 +217,13 @@ class CookieCSRFTests(unittest.TestCase): ) def test_new_cookie_csrf_with_existing_cookie_sets_cookies(self): - config = Configurator() - config.set_default_csrf_options(implementation=CookieCSRF()) - config.commit() - response = MockResponse() - request = DummyRequest(config.registry, response=response) + request = DummyRequest(response=response) request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} - token = config.registry.getUtility(ICSRFPolicy).new_csrf_token(request) + policy = self._makeOne() + token = policy.new_csrf_token(request) + self.assertEqual( response.called_args, ('csrf_token', token), @@ -146,13 +239,177 @@ class CookieCSRFTests(unittest.TestCase): } ) + def test_verifying_token_invalid_token(self): + response = MockResponse() + request = DummyRequest(response=response) + request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + + policy = self._makeOne() + self.assertFalse( + policy.check_csrf_token(request, 'invalid-token') + ) + + def test_verifying_token_against_existing_cookie(self): + response = MockResponse() + request = DummyRequest(response=response) + request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + + policy = self._makeOne() + self.assertTrue( + policy.check_csrf_token(request, 'e6f325fee5974f3da4315a8ccf4513d2') + ) + + +class Test_check_csrf_token(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + # set up CSRF (this will also register SessionCSRF policy) + self.config.set_default_csrf_options(require_csrf=False) + + def _callFUT(self, *args, **kwargs): + from ..csrf import check_csrf_token + return check_csrf_token(*args, **kwargs) + + def test_success_token(self): + request = testing.DummyRequest() + request.method = "POST" + request.POST = {'csrf_token': request.session.get_csrf_token()} + self.assertEqual(self._callFUT(request, token='csrf_token'), True) + + def test_success_header(self): + request = testing.DummyRequest() + request.headers['X-CSRF-Token'] = request.session.get_csrf_token() + self.assertEqual(self._callFUT(request, header='X-CSRF-Token'), True) + + def test_success_default_token(self): + request = testing.DummyRequest() + request.method = "POST" + request.POST = {'csrf_token': request.session.get_csrf_token()} + self.assertEqual(self._callFUT(request), True) + + def test_success_default_header(self): + request = testing.DummyRequest() + request.headers['X-CSRF-Token'] = request.session.get_csrf_token() + self.assertEqual(self._callFUT(request), True) + + def test_failure_raises(self): + from pyramid.exceptions import BadCSRFToken + request = testing.DummyRequest() + self.assertRaises(BadCSRFToken, self._callFUT, request, + 'csrf_token') + + def test_failure_no_raises(self): + request = testing.DummyRequest() + result = self._callFUT(request, 'csrf_token', raises=False) + self.assertEqual(result, False) + + def test_token_differing_types(self): + from pyramid.compat import text_ + request = testing.DummyRequest() + request.method = "POST" + request.session['_csrft_'] = text_('foo') + request.POST = {'csrf_token': b'foo'} + self.assertEqual(self._callFUT(request, token='csrf_token'), True) + + +class Test_check_csrf_origin(unittest.TestCase): + def _callFUT(self, *args, **kwargs): + from ..csrf import check_csrf_origin + return check_csrf_origin(*args, **kwargs) + + def test_success_with_http(self): + request = testing.DummyRequest() + request.scheme = "http" + self.assertTrue(self._callFUT(request)) + + def test_success_with_https_and_referrer(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = "https://example.com/login/" + request.registry.settings = {} + self.assertTrue(self._callFUT(request)) + + def test_success_with_https_and_origin(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.headers = {"Origin": "https://example.com/"} + request.referrer = "https://not-example.com/" + request.registry.settings = {} + self.assertTrue(self._callFUT(request)) + + def test_success_with_additional_trusted_host(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = "https://not-example.com/login/" + request.registry.settings = { + "pyramid.csrf_trusted_origins": ["not-example.com"], + } + self.assertTrue(self._callFUT(request)) + + def test_success_with_nonstandard_port(self): + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com:8080" + request.host_port = "8080" + request.referrer = "https://example.com:8080/login/" + request.registry.settings = {} + self.assertTrue(self._callFUT(request)) + + def test_fails_with_wrong_host(self): + from pyramid.exceptions import BadCSRFOrigin + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = "https://not-example.com/login/" + request.registry.settings = {} + self.assertRaises(BadCSRFOrigin, self._callFUT, request) + self.assertFalse(self._callFUT(request, raises=False)) + + def test_fails_with_no_origin(self): + from pyramid.exceptions import BadCSRFOrigin + request = testing.DummyRequest() + request.scheme = "https" + request.referrer = None + self.assertRaises(BadCSRFOrigin, self._callFUT, request) + self.assertFalse(self._callFUT(request, raises=False)) + + def test_fails_when_http_to_https(self): + from pyramid.exceptions import BadCSRFOrigin + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com" + request.host_port = "443" + request.referrer = "http://example.com/evil/" + request.registry.settings = {} + self.assertRaises(BadCSRFOrigin, self._callFUT, request) + self.assertFalse(self._callFUT(request, raises=False)) + + def test_fails_with_nonstandard_port(self): + from pyramid.exceptions import BadCSRFOrigin + request = testing.DummyRequest() + request.scheme = "https" + request.host = "example.com:8080" + request.host_port = "8080" + request.referrer = "https://example.com/login/" + request.registry.settings = {} + self.assertRaises(BadCSRFOrigin, self._callFUT, request) + self.assertFalse(self._callFUT(request, raises=False)) + class DummyRequest(object): registry = None session = None cookies = {} - def __init__(self, registry, session=None, response=None): + def __init__(self, registry=None, session=None, response=None): self.registry = registry self.session = session self.response = response @@ -170,3 +427,11 @@ class MockResponse(object): self.called_args = args self.called_kwargs = kwargs return + + +class DummyCSRF(object): + def new_csrf_token(self, request): + return 'e5e9e30a08b34ff9842ff7d2b958c14b' + + def get_csrf_token(self, request): + return '02821185e4c94269bdc38e6eeae0a2f8' diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py index b51dccecc..ade602799 100644 --- a/pyramid/tests/test_session.py +++ b/pyramid/tests/test_session.py @@ -659,144 +659,6 @@ class Test_signed_deserialize(unittest.TestCase): result = self._callFUT(serialized, secret.decode('latin-1')) self.assertEqual(result, '123') -class Test_check_csrf_token(unittest.TestCase): - def _callFUT(self, *args, **kwargs): - from ..csrf import check_csrf_token - return check_csrf_token(*args, **kwargs) - - def test_success_token(self): - request = testing.DummyRequest() - request.method = "POST" - request.POST = {'csrf_token': request.session.get_csrf_token()} - self.assertEqual(self._callFUT(request, token='csrf_token'), True) - - def test_success_header(self): - request = testing.DummyRequest() - request.headers['X-CSRF-Token'] = request.session.get_csrf_token() - self.assertEqual(self._callFUT(request, header='X-CSRF-Token'), True) - - def test_success_default_token(self): - request = testing.DummyRequest() - request.method = "POST" - request.POST = {'csrf_token': request.session.get_csrf_token()} - self.assertEqual(self._callFUT(request), True) - - def test_success_default_header(self): - request = testing.DummyRequest() - request.headers['X-CSRF-Token'] = request.session.get_csrf_token() - self.assertEqual(self._callFUT(request), True) - - def test_failure_raises(self): - from pyramid.exceptions import BadCSRFToken - request = testing.DummyRequest() - self.assertRaises(BadCSRFToken, self._callFUT, request, - 'csrf_token') - - def test_failure_no_raises(self): - request = testing.DummyRequest() - result = self._callFUT(request, 'csrf_token', raises=False) - self.assertEqual(result, False) - - def test_token_differing_types(self): - from pyramid.compat import text_ - request = testing.DummyRequest() - request.method = "POST" - request.session['_csrft_'] = text_('foo') - request.POST = {'csrf_token': b'foo'} - self.assertEqual(self._callFUT(request, token='csrf_token'), True) - - -class Test_check_csrf_origin(unittest.TestCase): - - def _callFUT(self, *args, **kwargs): - from ..csrf import check_csrf_origin - return check_csrf_origin(*args, **kwargs) - - def test_success_with_http(self): - request = testing.DummyRequest() - request.scheme = "http" - self.assertTrue(self._callFUT(request)) - - def test_success_with_https_and_referrer(self): - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com" - request.host_port = "443" - request.referrer = "https://example.com/login/" - request.registry.settings = {} - self.assertTrue(self._callFUT(request)) - - def test_success_with_https_and_origin(self): - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com" - request.host_port = "443" - request.headers = {"Origin": "https://example.com/"} - request.referrer = "https://not-example.com/" - request.registry.settings = {} - self.assertTrue(self._callFUT(request)) - - def test_success_with_additional_trusted_host(self): - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com" - request.host_port = "443" - request.referrer = "https://not-example.com/login/" - request.registry.settings = { - "pyramid.csrf_trusted_origins": ["not-example.com"], - } - self.assertTrue(self._callFUT(request)) - - def test_success_with_nonstandard_port(self): - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com:8080" - request.host_port = "8080" - request.referrer = "https://example.com:8080/login/" - request.registry.settings = {} - self.assertTrue(self._callFUT(request)) - - def test_fails_with_wrong_host(self): - from pyramid.exceptions import BadCSRFOrigin - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com" - request.host_port = "443" - request.referrer = "https://not-example.com/login/" - request.registry.settings = {} - self.assertRaises(BadCSRFOrigin, self._callFUT, request) - self.assertFalse(self._callFUT(request, raises=False)) - - def test_fails_with_no_origin(self): - from pyramid.exceptions import BadCSRFOrigin - request = testing.DummyRequest() - request.scheme = "https" - request.referrer = None - self.assertRaises(BadCSRFOrigin, self._callFUT, request) - self.assertFalse(self._callFUT(request, raises=False)) - - def test_fails_when_http_to_https(self): - from pyramid.exceptions import BadCSRFOrigin - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com" - request.host_port = "443" - request.referrer = "http://example.com/evil/" - request.registry.settings = {} - self.assertRaises(BadCSRFOrigin, self._callFUT, request) - self.assertFalse(self._callFUT(request, raises=False)) - - def test_fails_with_nonstandard_port(self): - from pyramid.exceptions import BadCSRFOrigin - request = testing.DummyRequest() - request.scheme = "https" - request.host = "example.com:8080" - request.host_port = "8080" - request.referrer = "https://example.com/login/" - request.registry.settings = {} - self.assertRaises(BadCSRFOrigin, self._callFUT, request) - self.assertFalse(self._callFUT(request, raises=False)) - class DummySerializer(object): def dumps(self, value): diff --git a/pyramid/tests/test_viewderivers.py b/pyramid/tests/test_viewderivers.py index 51d0bd367..6b81cc1e5 100644 --- a/pyramid/tests/test_viewderivers.py +++ b/pyramid/tests/test_viewderivers.py @@ -12,6 +12,7 @@ class TestDeriveView(unittest.TestCase): def setUp(self): self.config = testing.setUp() + self.config.set_default_csrf_options(require_csrf=False) def tearDown(self): self.config = None -- cgit v1.2.3 From 8f60e2c397a4c781d3ac2dc7fcff9321cdb16a42 Mon Sep 17 00:00:00 2001 From: Jure Cerjak Date: Wed, 7 Dec 2016 11:00:18 +0100 Subject: add to contributors list --- CONTRIBUTORS.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 566e91195..750f6d29f 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -291,6 +291,8 @@ Contributors - Mikko Ohtamaa, 2016/12/6 +- Jure Cerjak, 2016/12/7 + - Martin Frlin, 2016/12/7 - Kirill Kuzminykh, 2017/03/01 -- cgit v1.2.3 From fe0d223ad08bcab724d216b3a877b690c5795f73 Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Fri, 9 Dec 2016 11:25:03 +0100 Subject: Rename implementation to ICSRFStoragePolicy --- docs/api/interfaces.rst | 2 +- docs/narr/security.rst | 2 +- pyramid/config/security.py | 6 +++--- pyramid/config/views.py | 2 +- pyramid/csrf.py | 18 +++++++++--------- pyramid/interfaces.py | 2 +- pyramid/tests/test_csrf.py | 8 ++++---- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/api/interfaces.rst b/docs/api/interfaces.rst index b88209a36..e542a6be0 100644 --- a/docs/api/interfaces.rst +++ b/docs/api/interfaces.rst @@ -44,7 +44,7 @@ Other Interfaces .. autointerface:: IRoutePregenerator :members: - .. autointerface:: ICSRFPolicy + .. autointerface:: ICSRFStoragePolicy :members: .. autointerface:: ISession diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 6962a0fe3..04c236e0b 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -787,7 +787,7 @@ a :term:`session factory` as described in a cookie-based implementation :class:`pyramid.csrf.CookieCSRF` which gives some additional flexibility as it does not require a session for each user. You can also define your own implementation of -:class:`pyramid.interfaces.ICSRFPolicy` and register it with the +:class:`pyramid.interfaces.ICSRFStoragePolicy` and register it with the :meth:`pyramid.config.Configurator.set_default_csrf_options` directive. For example: diff --git a/pyramid/config/security.py b/pyramid/config/security.py index 102a61e0c..c8becce1f 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -3,7 +3,7 @@ from zope.interface import implementer from pyramid.interfaces import ( IAuthorizationPolicy, IAuthenticationPolicy, - ICSRFPolicy, + ICSRFStoragePolicy, IDefaultCSRFOptions, IDefaultPermission, PHASE1_CONFIG, @@ -181,7 +181,7 @@ class SecurityConfiguratorMixin(object): Set the default CSRF options used by subsequent view registrations. ``implementation`` is a class that implements the - :meth:`pyramid.interfaces.ICSRFPolicy` interface that will be used for all + :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all CSRF functionality. Default: :class:`pyramid.csrf.SessionCSRF`. ``require_csrf`` controls whether CSRF checks will be automatically @@ -220,7 +220,7 @@ class SecurityConfiguratorMixin(object): if implementation is None: implementation = SessionCSRF() def register(): - self.registry.registerUtility(implementation, ICSRFPolicy) + self.registry.registerUtility(implementation, ICSRFStoragePolicy) self.registry.registerUtility(options, IDefaultCSRFOptions) intr = self.introspectable('default csrf view options', None, diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 4ebd014de..e037f7706 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -644,7 +644,7 @@ class ViewsConfiguratorMixin(object): If CSRF checking is performed, the checked value will be the value of ``request.params[check_name]``. This value will be compared against the value of ``policy.get_csrf_token()`` (where ``policy`` is an - implementation of :meth:`pyramid.interfaces.ICSRFPolicy`), and the + implementation of :meth:`pyramid.interfaces.ICSRFStoragePolicy`), and the check will pass if these two values are the same. If the check passes, the associated view will be permitted to execute. If the check fails, the associated view will not be permitted to execute. diff --git a/pyramid/csrf.py b/pyramid/csrf.py index 7adbc9fee..b2788a764 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -11,7 +11,7 @@ from pyramid.exceptions import ( BadCSRFOrigin, BadCSRFToken, ) -from pyramid.interfaces import ICSRFPolicy +from pyramid.interfaces import ICSRFStoragePolicy from pyramid.settings import aslist from pyramid.util import ( is_same_domain, @@ -19,7 +19,7 @@ from pyramid.util import ( ) -@implementer(ICSRFPolicy) +@implementer(ICSRFStoragePolicy) class SessionCSRF(object): """ The default CSRF implementation, which mimics the behavior from older versions of Pyramid. The ``new_csrf_token`` and ``get_csrf_token`` methods @@ -48,7 +48,7 @@ class SessionCSRF(object): bytes_(supplied_token, 'ascii'), ) -@implementer(ICSRFPolicy) +@implementer(ICSRFStoragePolicy) class CookieCSRF(object): """ An alternative CSRF implementation that stores its information in unauthenticated cookies, known as the 'Double Submit Cookie' method in the @@ -108,7 +108,7 @@ def csrf_token_template_global(event): except AttributeError: return else: - csrf = registry.getUtility(ICSRFPolicy) + csrf = registry.getUtility(ICSRFStoragePolicy) event['get_csrf_token'] = partial(csrf.get_csrf_token, request) @@ -120,7 +120,7 @@ def get_csrf_token(request): .. versionadded :: 1.8a1 """ registry = request.registry - csrf = registry.getUtility(ICSRFPolicy) + csrf = registry.getUtility(ICSRFStoragePolicy) return csrf.get_csrf_token(request) @@ -132,7 +132,7 @@ def new_csrf_token(request): .. versionadded :: 1.8a1 """ registry = request.registry - csrf = registry.getUtility(ICSRFPolicy) + csrf = registry.getUtility(ICSRFStoragePolicy) return csrf.new_csrf_token(request) @@ -141,7 +141,7 @@ def check_csrf_token(request, header='X-CSRF-Token', raises=True): """ Check the CSRF token returned by the - :class:`pyramid.interfaces.ICSRFPolicy` implementation against the value in + :class:`pyramid.interfaces.ICSRFStoragePolicy` implementation against the value in ``request.POST.get(token)`` (if a POST request) or ``request.headers.get(header)``. If a ``token`` keyword is not supplied to this function, the string ``csrf_token`` will be used to look up the token @@ -151,7 +151,7 @@ def check_csrf_token(request, If the value supplied by post or by header doesn't match the value supplied by ``policy.get_csrf_token()`` (where ``policy`` is an implementation of - :class:`pyramid.interfaces.ICSRFPolicy`), and ``raises`` is ``True``, this + :class:`pyramid.interfaces.ICSRFStoragePolicy`), and ``raises`` is ``True``, this function will raise an :exc:`pyramid.exceptions.BadCSRFToken` exception. If the values differ and ``raises`` is ``False``, this function will return ``False``. If the CSRF check is successful, this function will return @@ -184,7 +184,7 @@ def check_csrf_token(request, if supplied_token == "" and token is not None: supplied_token = request.POST.get(token, "") - policy = request.registry.getUtility(ICSRFPolicy) + policy = request.registry.getUtility(ICSRFStoragePolicy) if not policy.check_csrf_token(request, supplied_token): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index f58ee8b58..aab5647a1 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -982,7 +982,7 @@ class ISession(IDict): """ -class ICSRFPolicy(Interface): +class ICSRFStoragePolicy(Interface): """ An object that offers the ability to verify CSRF tokens and generate new ones""" diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index 1b3f3fc3b..8866f3601 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -117,13 +117,13 @@ class TestSessionCSRF(unittest.TestCase): def test_register_session_csrf_policy(self): from pyramid.csrf import SessionCSRF - from pyramid.interfaces import ICSRFPolicy + from pyramid.interfaces import ICSRFStoragePolicy config = Configurator() config.set_default_csrf_options(implementation=self._makeOne()) config.commit() - policy = config.registry.queryUtility(ICSRFPolicy) + policy = config.registry.queryUtility(ICSRFStoragePolicy) self.assertTrue(isinstance(policy, SessionCSRF)) @@ -163,13 +163,13 @@ class TestCookieCSRF(unittest.TestCase): def test_register_cookie_csrf_policy(self): from pyramid.csrf import CookieCSRF - from pyramid.interfaces import ICSRFPolicy + from pyramid.interfaces import ICSRFStoragePolicy config = Configurator() config.set_default_csrf_options(implementation=self._makeOne()) config.commit() - policy = config.registry.queryUtility(ICSRFPolicy) + policy = config.registry.queryUtility(ICSRFStoragePolicy) self.assertTrue(isinstance(policy, CookieCSRF)) -- cgit v1.2.3 From f6d63a41d37b0647c49e53bb54f009f7da4d5079 Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Fri, 9 Dec 2016 12:00:17 +0100 Subject: Fix a bug where people that didn't configure CSRF protection but did configure a session and set explicit checks would see an exception --- pyramid/csrf.py | 8 +++++++- pyramid/tests/test_csrf.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pyramid/csrf.py b/pyramid/csrf.py index b2788a764..f282eb569 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -184,7 +184,13 @@ def check_csrf_token(request, if supplied_token == "" and token is not None: supplied_token = request.POST.get(token, "") - policy = request.registry.getUtility(ICSRFStoragePolicy) + policy = request.registry.queryUtility(ICSRFStoragePolicy) + if policy is None: + # There is no policy set, but we are trying to validate a CSRF token + # This means explicit validation has been asked for without configuring + # the CSRF implementation. Fall back to SessionCSRF as that is the + # default + policy = SessionCSRF() if not policy.check_csrf_token(request, supplied_token): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index 8866f3601..3994a31d4 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -313,6 +313,32 @@ class Test_check_csrf_token(unittest.TestCase): self.assertEqual(self._callFUT(request, token='csrf_token'), True) +class Test_check_csrf_token_without_defaults_configured(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + def _callFUT(self, *args, **kwargs): + from ..csrf import check_csrf_token + return check_csrf_token(*args, **kwargs) + + def test_success_token(self): + request = testing.DummyRequest() + request.method = "POST" + request.POST = {'csrf_token': request.session.get_csrf_token()} + self.assertEqual(self._callFUT(request, token='csrf_token'), True) + + def test_failure_raises(self): + from pyramid.exceptions import BadCSRFToken + request = testing.DummyRequest() + self.assertRaises(BadCSRFToken, self._callFUT, request, + 'csrf_token') + + def test_failure_no_raises(self): + request = testing.DummyRequest() + result = self._callFUT(request, 'csrf_token', raises=False) + self.assertEqual(result, False) + + class Test_check_csrf_origin(unittest.TestCase): def _callFUT(self, *args, **kwargs): from ..csrf import check_csrf_origin -- cgit v1.2.3 From 7c0f098641fda4207ea6fa50c58b289926038697 Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Wed, 12 Apr 2017 11:57:56 +0100 Subject: Use the webob CookieProfile in the Cookie implementation, rename some implemenations based on feedback, split CSRF implementation and option configuration and make the csrf token function exposed as a system default rather than a renderer event. --- docs/api/config.rst | 1 + docs/api/csrf.rst | 4 +- docs/narr/extconfig.rst | 1 + docs/narr/security.rst | 8 +-- pyramid/config/__init__.py | 1 + pyramid/config/security.py | 31 ++++++---- pyramid/csrf.py | 52 +++++++---------- pyramid/renderers.py | 4 ++ pyramid/tests/test_csrf.py | 126 +++++++--------------------------------- pyramid/tests/test_renderers.py | 8 +++ 10 files changed, 84 insertions(+), 152 deletions(-) diff --git a/docs/api/config.rst b/docs/api/config.rst index c76d3d5ff..a785b64ad 100644 --- a/docs/api/config.rst +++ b/docs/api/config.rst @@ -37,6 +37,7 @@ .. automethod:: set_authentication_policy .. automethod:: set_authorization_policy .. automethod:: set_default_csrf_options + .. automethod:: set_csrf_storage_policy .. automethod:: set_default_permission .. automethod:: add_permission diff --git a/docs/api/csrf.rst b/docs/api/csrf.rst index 89fb0c4b2..f890ee660 100644 --- a/docs/api/csrf.rst +++ b/docs/api/csrf.rst @@ -5,10 +5,10 @@ .. automodule:: pyramid.csrf - .. autoclass:: SessionCSRF + .. autoclass:: SessionCSRFStoragePolicy :members: - .. autoclass:: CookieCSRF + .. autoclass:: CookieCSRFStoragePolicy :members: .. autofunction:: get_csrf_token diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst index 4009ec1dc..c20685cbf 100644 --- a/docs/narr/extconfig.rst +++ b/docs/narr/extconfig.rst @@ -263,6 +263,7 @@ Pre-defined Phases - :meth:`pyramid.config.Configurator.override_asset` - :meth:`pyramid.config.Configurator.set_authorization_policy` - :meth:`pyramid.config.Configurator.set_default_csrf_options` +- :meth:`pyramid.config.Configurator.set_csrf_storage_policy` - :meth:`pyramid.config.Configurator.set_default_permission` - :meth:`pyramid.config.Configurator.set_view_mapper` diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 04c236e0b..e67f7b98c 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -780,15 +780,15 @@ and then requiring that it be present in all potentially unsafe requests. :app:`Pyramid` provides facilities to create and check CSRF tokens. By default :app:`Pyramid` comes with a session-based CSRF implementation -:class:`pyramid.csrf.SessionCSRF`. To use it, you must first enable +:class:`pyramid.csrf.SessionCSRFStoragePolicy`. To use it, you must first enable a :term:`session factory` as described in :ref:`using_the_default_session_factory` or :ref:`using_alternate_session_factories`. Alternatively, you can use -a cookie-based implementation :class:`pyramid.csrf.CookieCSRF` which gives +a cookie-based implementation :class:`pyramid.csrf.CookieCSRFStoragePolicy` which gives some additional flexibility as it does not require a session for each user. You can also define your own implementation of :class:`pyramid.interfaces.ICSRFStoragePolicy` and register it with the -:meth:`pyramid.config.Configurator.set_default_csrf_options` directive. +:meth:`pyramid.config.Configurator.set_csrf_storage_policy` directive. For example: @@ -797,7 +797,7 @@ For example: from pyramid.config import Configurator config = Configurator() - config.set_default_csrf_options(implementation=MyCustomCSRFPolicy()) + config.set_csrf_storage_policy(MyCustomCSRFPolicy()) .. index:: single: csrf.get_csrf_token diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py index 6c661aa59..b05effbde 100644 --- a/pyramid/config/__init__.py +++ b/pyramid/config/__init__.py @@ -380,6 +380,7 @@ class Configurator( self.add_default_view_derivers() self.add_default_route_predicates() self.add_default_tweens() + self.add_default_security() if exceptionresponse_view is not None: exceptionresponse_view = self.maybe_dotted(exceptionresponse_view) diff --git a/pyramid/config/security.py b/pyramid/config/security.py index c8becce1f..0b565e322 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -10,15 +10,17 @@ from pyramid.interfaces import ( PHASE2_CONFIG, ) -from pyramid.csrf import csrf_token_template_global -from pyramid.csrf import SessionCSRF -from pyramid.events import BeforeRender +from pyramid.csrf import SessionCSRFStoragePolicy from pyramid.exceptions import ConfigurationError from pyramid.util import action_method from pyramid.util import as_sorted_tuple class SecurityConfiguratorMixin(object): + + def add_default_security(self): + self.set_csrf_storage_policy(SessionCSRFStoragePolicy()) + @action_method def set_authentication_policy(self, policy): """ Override the :app:`Pyramid` :term:`authentication policy` in the @@ -170,7 +172,6 @@ class SecurityConfiguratorMixin(object): @action_method def set_default_csrf_options( self, - implementation=None, require_csrf=True, token='csrf_token', header='X-CSRF-Token', @@ -180,10 +181,6 @@ class SecurityConfiguratorMixin(object): """ Set the default CSRF options used by subsequent view registrations. - ``implementation`` is a class that implements the - :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all - CSRF functionality. Default: :class:`pyramid.csrf.SessionCSRF`. - ``require_csrf`` controls whether CSRF checks will be automatically enabled on each view in the application. This value is used as the fallback when ``require_csrf`` is left at the default of ``None`` on @@ -217,10 +214,7 @@ class SecurityConfiguratorMixin(object): options = DefaultCSRFOptions( require_csrf, token, header, safe_methods, callback, ) - if implementation is None: - implementation = SessionCSRF() def register(): - self.registry.registerUtility(implementation, ICSRFStoragePolicy) self.registry.registerUtility(options, IDefaultCSRFOptions) intr = self.introspectable('default csrf view options', None, @@ -232,10 +226,23 @@ class SecurityConfiguratorMixin(object): intr['safe_methods'] = as_sorted_tuple(safe_methods) intr['callback'] = callback - self.add_subscriber(csrf_token_template_global, [BeforeRender]) self.action(IDefaultCSRFOptions, register, order=PHASE1_CONFIG, introspectables=(intr,)) + @action_method + def set_csrf_storage_policy(self, policy): + """ + Set the CSRF storage policy used by subsequent view registrations. + + ``policy`` is a class that implements the + :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all + CSRF functionality. + """ + def register(): + self.registry.registerUtility(policy, ICSRFStoragePolicy) + + self.action(ICSRFStoragePolicy, register, order=PHASE1_CONFIG) + @implementer(IDefaultCSRFOptions) class DefaultCSRFOptions(object): diff --git a/pyramid/csrf.py b/pyramid/csrf.py index f282eb569..4c5a73940 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -1,8 +1,11 @@ -from functools import partial import uuid +from webob.cookies import CookieProfile from zope.interface import implementer + +from pyramid.authentication import _SimpleSerializer + from pyramid.compat import ( urlparse, bytes_ @@ -20,7 +23,7 @@ from pyramid.util import ( @implementer(ICSRFStoragePolicy) -class SessionCSRF(object): +class SessionCSRFStoragePolicy(object): """ The default CSRF implementation, which mimics the behavior from older versions of Pyramid. The ``new_csrf_token`` and ``get_csrf_token`` methods are indirected to the underlying session implementation. @@ -49,7 +52,7 @@ class SessionCSRF(object): ) @implementer(ICSRFStoragePolicy) -class CookieCSRF(object): +class CookieCSRFStoragePolicy(object): """ An alternative CSRF implementation that stores its information in unauthenticated cookies, known as the 'Double Submit Cookie' method in the OWASP CSRF guidelines. This gives some additional flexibility with regards @@ -60,25 +63,25 @@ class CookieCSRF(object): """ def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, - domain=None, path='/'): - self.cookie_name = cookie_name - self.secure = secure - self.httponly = httponly + domain=None, max_age=None, path='/'): + serializer = _SimpleSerializer() + self.cookie_profile = CookieProfile( + cookie_name=cookie_name, + secure=secure, + max_age=max_age, + httponly=httponly, + path=path, + serializer=serializer + ) self.domain = domain - self.path = path def new_csrf_token(self, request): """ Sets a new CSRF token into the request and returns it. """ token = uuid.uuid4().hex def set_cookie(request, response): - response.set_cookie( - self.cookie_name, + self.cookie_profile.set_cookies( + response, token, - httponly=self.httponly, - secure=self.secure, - domain=self.domain, - path=self.path, - overwrite=True, ) request.add_response_callback(set_cookie) return token @@ -86,7 +89,8 @@ class CookieCSRF(object): def get_csrf_token(self, request): """ Returns the currently active CSRF token by checking the cookies sent with the current request.""" - token = request.cookies.get(self.cookie_name) + bound_cookies = self.cookie_profile.bind(request) + token = bound_cookies.get_value() if not token: token = self.new_csrf_token(request) return token @@ -100,18 +104,6 @@ class CookieCSRF(object): bytes_(supplied_token, 'ascii'), ) - -def csrf_token_template_global(event): - request = event.get('request', None) - try: - registry = request.registry - except AttributeError: - return - else: - csrf = registry.getUtility(ICSRFStoragePolicy) - event['get_csrf_token'] = partial(csrf.get_csrf_token, request) - - def get_csrf_token(request): """ Get the currently active CSRF token for the request passed, generating a new one using ``new_csrf_token(request)`` if one does not exist. This @@ -188,9 +180,9 @@ def check_csrf_token(request, if policy is None: # There is no policy set, but we are trying to validate a CSRF token # This means explicit validation has been asked for without configuring - # the CSRF implementation. Fall back to SessionCSRF as that is the + # the CSRF implementation. Fall back to SessionCSRFStoragePolicy as that is the # default - policy = SessionCSRF() + policy = SessionCSRFStoragePolicy() if not policy.check_csrf_token(request, supplied_token): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') diff --git a/pyramid/renderers.py b/pyramid/renderers.py index 7d667ba7b..6019f50fb 100644 --- a/pyramid/renderers.py +++ b/pyramid/renderers.py @@ -1,3 +1,4 @@ +from functools import partial import json import os import re @@ -19,6 +20,7 @@ from pyramid.compat import ( text_type, ) +from pyramid.csrf import get_csrf_token from pyramid.decorator import reify from pyramid.events import BeforeRender @@ -428,6 +430,7 @@ class RendererHelper(object): 'context':context, 'request':request, 'req':request, + 'get_csrf_token':partial(get_csrf_token, request), } return self.render_to_response(response, system, request=request) @@ -441,6 +444,7 @@ class RendererHelper(object): 'context':getattr(request, 'context', None), 'request':request, 'req':request, + 'get_csrf_token':partial(get_csrf_token, request), } system_values = BeforeRender(system_values, value) diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index 3994a31d4..e6ae05eec 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -22,7 +22,7 @@ class Test_get_csrf_token(unittest.TestCase): self._callFUT(request) def test_success(self): - self.config.set_default_csrf_options(implementation=DummyCSRF()) + self.config.set_csrf_storage_policy(DummyCSRF()) request = testing.DummyRequest() csrf_token = self._callFUT(request) @@ -45,7 +45,7 @@ class Test_new_csrf_token(unittest.TestCase): self._callFUT(request) def test_success(self): - self.config.set_default_csrf_options(implementation=DummyCSRF()) + self.config.set_csrf_storage_policy(DummyCSRF()) request = testing.DummyRequest() csrf_token = self._callFUT(request) @@ -53,57 +53,7 @@ class Test_new_csrf_token(unittest.TestCase): self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b') -class Test_csrf_token_template_global(unittest.TestCase): - def setUp(self): - self.config = testing.setUp() - - def _callFUT(self, *args, **kwargs): - from pyramid.csrf import csrf_token_template_global - return csrf_token_template_global(*args, **kwargs) - - def test_event_is_missing_request(self): - event = BeforeRender({}, {}) - - self._callFUT(event) - - self.assertNotIn('get_csrf_token', event) - - def test_request_is_missing_registry(self): - request = DummyRequest(registry=None) - del request.registry - del request.__class__.registry - event = BeforeRender({'request': request}, {}) - - self._callFUT(event) - - self.assertNotIn('get_csrf_token', event) - - def test_csrf_utility_not_registered(self): - request = testing.DummyRequest() - event = BeforeRender({'request': request}, {}) - - with self.assertRaises(ComponentLookupError): - self._callFUT(event) - - def test_csrf_token_passed_to_template(self): - config = Configurator() - config.set_default_csrf_options(implementation=DummyCSRF()) - config.commit() - - request = testing.DummyRequest() - request.registry = config.registry - - before = BeforeRender({'request': request}, {}) - config.registry.notify(before) - - self.assertIn('get_csrf_token', before) - self.assertEqual( - before['get_csrf_token'](), - '02821185e4c94269bdc38e6eeae0a2f8' - ) - - -class TestSessionCSRF(unittest.TestCase): +class TestSessionCSRFStoragePolicy(unittest.TestCase): class MockSession(object): def new_csrf_token(self): return 'e5e9e30a08b34ff9842ff7d2b958c14b' @@ -112,20 +62,20 @@ class TestSessionCSRF(unittest.TestCase): return '02821185e4c94269bdc38e6eeae0a2f8' def _makeOne(self): - from pyramid.csrf import SessionCSRF - return SessionCSRF() + from pyramid.csrf import SessionCSRFStoragePolicy + return SessionCSRFStoragePolicy() def test_register_session_csrf_policy(self): - from pyramid.csrf import SessionCSRF + from pyramid.csrf import SessionCSRFStoragePolicy from pyramid.interfaces import ICSRFStoragePolicy config = Configurator() - config.set_default_csrf_options(implementation=self._makeOne()) + config.set_csrf_storage_policy(self._makeOne()) config.commit() policy = config.registry.queryUtility(ICSRFStoragePolicy) - self.assertTrue(isinstance(policy, SessionCSRF)) + self.assertTrue(isinstance(policy, SessionCSRFStoragePolicy)) def test_session_csrf_implementation_delegates_to_session(self): policy = self._makeOne() @@ -156,22 +106,22 @@ class TestSessionCSRF(unittest.TestCase): self.assertTrue(result) -class TestCookieCSRF(unittest.TestCase): +class TestCookieCSRFStoragePolicy(unittest.TestCase): def _makeOne(self): - from pyramid.csrf import CookieCSRF - return CookieCSRF() + from pyramid.csrf import CookieCSRFStoragePolicy + return CookieCSRFStoragePolicy() def test_register_cookie_csrf_policy(self): - from pyramid.csrf import CookieCSRF + from pyramid.csrf import CookieCSRFStoragePolicy from pyramid.interfaces import ICSRFStoragePolicy config = Configurator() - config.set_default_csrf_options(implementation=self._makeOne()) + config.set_csrf_storage_policy(self._makeOne()) config.commit() policy = config.registry.queryUtility(ICSRFStoragePolicy) - self.assertTrue(isinstance(policy, CookieCSRF)) + self.assertTrue(isinstance(policy, CookieCSRFStoragePolicy)) def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self): response = MockResponse() @@ -179,20 +129,9 @@ class TestCookieCSRF(unittest.TestCase): policy = self._makeOne() token = policy.get_csrf_token(request) - self.assertEqual( - response.called_args, - ('csrf_token', token), - ) - self.assertEqual( - response.called_kwargs, - { - 'secure': False, - 'httponly': False, - 'domain': None, - 'path': '/', - 'overwrite': True - } + response.headerlist, + [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] ) def test_existing_cookie_csrf_does_not_set_cookie(self): @@ -208,12 +147,8 @@ class TestCookieCSRF(unittest.TestCase): 'e6f325fee5974f3da4315a8ccf4513d2' ) self.assertEqual( - response.called_args, - (), - ) - self.assertEqual( - response.called_kwargs, - {} + response.headerlist, + [], ) def test_new_cookie_csrf_with_existing_cookie_sets_cookies(self): @@ -223,20 +158,9 @@ class TestCookieCSRF(unittest.TestCase): policy = self._makeOne() token = policy.new_csrf_token(request) - - self.assertEqual( - response.called_args, - ('csrf_token', token), - ) self.assertEqual( - response.called_kwargs, - { - 'secure': False, - 'httponly': False, - 'domain': None, - 'path': '/', - 'overwrite': True - } + response.headerlist, + [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] ) def test_verifying_token_invalid_token(self): @@ -264,7 +188,7 @@ class Test_check_csrf_token(unittest.TestCase): def setUp(self): self.config = testing.setUp() - # set up CSRF (this will also register SessionCSRF policy) + # set up CSRF (this will also register SessionCSRFStoragePolicy policy) self.config.set_default_csrf_options(require_csrf=False) def _callFUT(self, *args, **kwargs): @@ -446,13 +370,7 @@ class DummyRequest(object): class MockResponse(object): def __init__(self): - self.called_args = () - self.called_kwargs = {} - - def set_cookie(self, *args, **kwargs): - self.called_args = args - self.called_kwargs = kwargs - return + self.headerlist = [] class DummyCSRF(object): diff --git a/pyramid/tests/test_renderers.py b/pyramid/tests/test_renderers.py index 65bfa5582..86d8b582a 100644 --- a/pyramid/tests/test_renderers.py +++ b/pyramid/tests/test_renderers.py @@ -203,6 +203,7 @@ class TestRendererHelper(unittest.TestCase): self.assertEqual(helper.get_renderer(), factory.respond) def test_render_view(self): + import pyramid.csrf self._registerRendererFactory() self._registerResponseFactory() request = Dummy() @@ -212,6 +213,9 @@ class TestRendererHelper(unittest.TestCase): request = testing.DummyRequest() response = 'response' response = helper.render_view(request, response, view, context) + get_csrf = response.app_iter[1].pop('get_csrf_token') + self.assertEqual(get_csrf.args, (request, )) + self.assertEqual(get_csrf.func, pyramid.csrf.get_csrf_token) self.assertEqual(response.app_iter[0], 'response') self.assertEqual(response.app_iter[1], {'renderer_info': helper, @@ -242,12 +246,16 @@ class TestRendererHelper(unittest.TestCase): self.assertEqual(reg.event.__class__.__name__, 'BeforeRender') def test_render_system_values_is_None(self): + import pyramid.csrf self._registerRendererFactory() request = Dummy() context = Dummy() request.context = context helper = self._makeOne('loo.foo') result = helper.render('values', None, request=request) + get_csrf = result[1].pop('get_csrf_token') + self.assertEqual(get_csrf.args, (request, )) + self.assertEqual(get_csrf.func, pyramid.csrf.get_csrf_token) system = {'request':request, 'context':context, 'renderer_name':'loo.foo', -- cgit v1.2.3 From 67ac6c8718df02505882d08d254d7a4ab9423d18 Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Sat, 15 Apr 2017 19:23:58 +1000 Subject: Update default.py --- docs/tutorials/wiki2/src/views/tutorial/views/default.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/wiki2/src/views/tutorial/views/default.py b/docs/tutorials/wiki2/src/views/tutorial/views/default.py index 0a05b33e6..3b95e0f59 100644 --- a/docs/tutorials/wiki2/src/views/tutorial/views/default.py +++ b/docs/tutorials/wiki2/src/views/tutorial/views/default.py @@ -1,4 +1,4 @@ -import html +from pyramid.compat import escape import re from docutils.core import publish_parts @@ -31,10 +31,10 @@ def view_page(request): exists = request.dbsession.query(Page).filter_by(name=word).all() if exists: view_url = request.route_url('view_page', pagename=word) - return '%s' % (view_url, html.escape(word)) + return '%s' % (view_url, escape(word)) else: add_url = request.route_url('add_page', pagename=word) - return '%s' % (add_url, html.escape(word)) + return '%s' % (add_url, escape(word)) content = publish_parts(page.data, writer_name='html')['html_body'] content = wikiwords.sub(add_link, content) -- cgit v1.2.3 From 4b743ad895e914d31b75d446118d219e36435711 Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Sat, 15 Apr 2017 19:25:46 +1000 Subject: Update default.py --- docs/tutorials/wiki2/src/authentication/tutorial/views/default.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/wiki2/src/authentication/tutorial/views/default.py b/docs/tutorials/wiki2/src/authentication/tutorial/views/default.py index 1b071434c..2d058d874 100644 --- a/docs/tutorials/wiki2/src/authentication/tutorial/views/default.py +++ b/docs/tutorials/wiki2/src/authentication/tutorial/views/default.py @@ -1,4 +1,4 @@ -import cgi +from pyramid.compat import escape import re from docutils.core import publish_parts @@ -32,10 +32,10 @@ def view_page(request): exists = request.dbsession.query(Page).filter_by(name=word).all() if exists: view_url = request.route_url('view_page', pagename=word) - return '%s' % (view_url, cgi.escape(word)) + return '%s' % (view_url, escape(word)) else: add_url = request.route_url('add_page', pagename=word) - return '%s' % (add_url, cgi.escape(word)) + return '%s' % (add_url, escape(word)) content = publish_parts(page.data, writer_name='html')['html_body'] content = wikiwords.sub(add_link, content) -- cgit v1.2.3 From edf56847ab136c0fc358309e584edd15357c5848 Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Sat, 15 Apr 2017 19:27:24 +1000 Subject: Update default.py --- docs/tutorials/wiki2/src/authorization/tutorial/views/default.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/wiki2/src/authorization/tutorial/views/default.py b/docs/tutorials/wiki2/src/authorization/tutorial/views/default.py index 9358993ea..65c12ed3b 100644 --- a/docs/tutorials/wiki2/src/authorization/tutorial/views/default.py +++ b/docs/tutorials/wiki2/src/authorization/tutorial/views/default.py @@ -1,4 +1,4 @@ -import cgi +from pyramid.compat import escape import re from docutils.core import publish_parts @@ -25,10 +25,10 @@ def view_page(request): exists = request.dbsession.query(Page).filter_by(name=word).all() if exists: view_url = request.route_url('view_page', pagename=word) - return '%s' % (view_url, cgi.escape(word)) + return '%s' % (view_url, escape(word)) else: add_url = request.route_url('add_page', pagename=word) - return '%s' % (add_url, cgi.escape(word)) + return '%s' % (add_url, escape(word)) content = publish_parts(page.data, writer_name='html')['html_body'] content = wikiwords.sub(add_link, content) -- cgit v1.2.3 From 9d961de6cef714391683e24d4616d0db2a9e931d Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Sat, 15 Apr 2017 19:28:27 +1000 Subject: Update default.py --- docs/tutorials/wiki2/src/tests/tutorial/views/default.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/wiki2/src/tests/tutorial/views/default.py b/docs/tutorials/wiki2/src/tests/tutorial/views/default.py index 9358993ea..65c12ed3b 100644 --- a/docs/tutorials/wiki2/src/tests/tutorial/views/default.py +++ b/docs/tutorials/wiki2/src/tests/tutorial/views/default.py @@ -1,4 +1,4 @@ -import cgi +from pyramid.compat import escape import re from docutils.core import publish_parts @@ -25,10 +25,10 @@ def view_page(request): exists = request.dbsession.query(Page).filter_by(name=word).all() if exists: view_url = request.route_url('view_page', pagename=word) - return '%s' % (view_url, cgi.escape(word)) + return '%s' % (view_url, escape(word)) else: add_url = request.route_url('add_page', pagename=word) - return '%s' % (add_url, cgi.escape(word)) + return '%s' % (add_url, escape(word)) content = publish_parts(page.data, writer_name='html')['html_body'] content = wikiwords.sub(add_link, content) -- cgit v1.2.3 From 999bdae76694649b36d963b58cc1f3f91fa35ed7 Mon Sep 17 00:00:00 2001 From: Ira Lun Date: Sun, 16 Apr 2017 22:46:04 +0100 Subject: Fix typo in comment. --- pyramid/config/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index dd8e9e787..2433ccfef 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -972,7 +972,7 @@ class ViewsConfiguratorMixin(object): def register_view(classifier, request_iface, derived_view): # A multiviews is a set of views which are registered for # exactly the same context type/request type/name triad. Each - # consituent view in a multiview differs only by the + # constituent view in a multiview differs only by the # predicates which it possesses. # To find a previously registered view for a context -- cgit v1.2.3 From 8197f18c5ed634625c749db674d7bdf97d1013ef Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Mon, 17 Apr 2017 12:29:55 -0700 Subject: fix rst syntax for index entries --- docs/narr/logging.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/narr/logging.rst b/docs/narr/logging.rst index 87682158b..9cc5b4ed8 100644 --- a/docs/narr/logging.rst +++ b/docs/narr/logging.rst @@ -16,7 +16,7 @@ to send log messages to loggers that you've configured. cookiecutter which does not create these files, the configuration information in this chapter may not be applicable. -.. index: +.. index:: pair: settings; logging pair: .ini; logging pair: logging; configuration -- cgit v1.2.3 From 4375bc690f31c488610e1bd72c2f1ea18b295121 Mon Sep 17 00:00:00 2001 From: Jeremy Chen Date: Wed, 19 Apr 2017 20:32:39 +1000 Subject: Update CONTRIBUTORS.txt --- CONTRIBUTORS.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 566e91195..3fe2c2d58 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -294,3 +294,5 @@ Contributors - Martin Frlin, 2016/12/7 - Kirill Kuzminykh, 2017/03/01 + +- Jeremy(Ching-Rui) Chen, 2017/04/19 -- cgit v1.2.3 From 6ff6fa265cb48a48daa61247bb1a068852ad13c0 Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Sun, 23 Apr 2017 23:59:48 -0700 Subject: update user prompt for cookiecutter repo_name - refs: https://github.com/Pylons/pyramid-cookiecutter-starter/pull/27#issuecomment-296507821 --- docs/narr/project.rst | 2 +- docs/quick_tour.rst | 4 ++-- docs/quick_tutorial/cookiecutters.rst | 2 +- docs/tutorials/modwsgi/index.rst | 2 +- docs/tutorials/wiki/installation.rst | 2 +- docs/tutorials/wiki2/installation.rst | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/narr/project.rst b/docs/narr/project.rst index ce7e90793..9c44d4f16 100644 --- a/docs/narr/project.rst +++ b/docs/narr/project.rst @@ -94,7 +94,7 @@ If prompted for the first item, accept the default ``yes`` by hitting return. You've cloned ~/.cookiecutters/pyramid-cookiecutter-starter before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: myproject - repo_name [scaffold]: myproject + repo_name [myproject]: myproject Select template_language: 1 - jinja2 2 - chameleon diff --git a/docs/quick_tour.rst b/docs/quick_tour.rst index 02c3ff811..571dfb356 100644 --- a/docs/quick_tour.rst +++ b/docs/quick_tour.rst @@ -519,7 +519,7 @@ If prompted for the first item, accept the default ``yes`` by hitting return. You've cloned ~/.cookiecutters/pyramid-cookiecutter-starter before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: hello_world - repo_name [scaffold]: hello_world + repo_name [hello_world]: hello_world Select template_language: 1 - jinja2 2 - chameleon @@ -875,7 +875,7 @@ If prompted for the first item, accept the default ``yes`` by hitting return. You've cloned ~/.cookiecutters/pyramid-cookiecutter-alchemy before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: sqla_demo - repo_name [scaffold]: sqla_demo + repo_name [sqla_demo]: sqla_demo We then run through the following commands as before. diff --git a/docs/quick_tutorial/cookiecutters.rst b/docs/quick_tutorial/cookiecutters.rst index edfd8cd69..337a5c535 100644 --- a/docs/quick_tutorial/cookiecutters.rst +++ b/docs/quick_tutorial/cookiecutters.rst @@ -37,7 +37,7 @@ Steps You've cloned ~/.cookiecutters/pyramid-cookiecutter-starter before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: cc_starter - repo_name [scaffold]: cc_starter + repo_name [cc_starter]: cc_starter Select template_language: 1 - jinja2 2 - chameleon diff --git a/docs/tutorials/modwsgi/index.rst b/docs/tutorials/modwsgi/index.rst index 690266586..170f2ebc8 100644 --- a/docs/tutorials/modwsgi/index.rst +++ b/docs/tutorials/modwsgi/index.rst @@ -48,7 +48,7 @@ specific path information for commands and files. You've cloned ~/.cookiecutters/pyramid-cookiecutter-starter before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: myproject - repo_name [scaffold]: myproject + repo_name [myproject]: myproject Select template_language: 1 - jinja2 2 - chameleon diff --git a/docs/tutorials/wiki/installation.rst b/docs/tutorials/wiki/installation.rst index 6be826395..de057b1cc 100644 --- a/docs/tutorials/wiki/installation.rst +++ b/docs/tutorials/wiki/installation.rst @@ -50,7 +50,7 @@ If prompted for the first item, accept the default ``yes`` by hitting return. You've cloned ~/.cookiecutters/pyramid-cookiecutter-zodb before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: myproj - repo_name [scaffold]: tutorial + repo_name [myproj]: tutorial Change directory into your newly created project ------------------------------------------------ diff --git a/docs/tutorials/wiki2/installation.rst b/docs/tutorials/wiki2/installation.rst index 9eeb1711d..c61d4360d 100644 --- a/docs/tutorials/wiki2/installation.rst +++ b/docs/tutorials/wiki2/installation.rst @@ -62,7 +62,7 @@ If prompted for the first item, accept the default ``yes`` by hitting return. You've cloned ~/.cookiecutters/pyramid-cookiecutter-alchemy before. Is it okay to delete and re-clone it? [yes]: yes project_name [Pyramid Scaffold]: myproj - repo_name [scaffold]: tutorial + repo_name [myproj]: tutorial Change directory into your newly created project ------------------------------------------------ -- cgit v1.2.3 From 2ded2fc216b4caaf0d97813413943e0838b6eaaa Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Wed, 26 Apr 2017 15:41:47 +0100 Subject: Apply drafting changes to documentation. --- CHANGES.txt | 2 +- docs/glossary.rst | 5 +++++ docs/narr/security.rst | 4 ++++ docs/narr/sessions.rst | 4 ---- pyramid/config/security.py | 2 +- pyramid/config/views.py | 6 ++++++ pyramid/csrf.py | 43 ++++++++++++++++++++++--------------------- pyramid/interfaces.py | 4 ++-- 8 files changed, 41 insertions(+), 29 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 9d6264688..762550053 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -25,7 +25,7 @@ Features appropriately. See https://github.com/Pylons/pyramid/pull/2989 - A new CSRF implementation, :class:`pyramid.csrf.SessionCSRF` has been added, - which deleagates all CSRF generation to the current session, following the + which delegates all CSRF generation to the current session, following the old API for this. A ``get_csrf_token()`` method is now available in template global scope, to make it easy for template developers to get the current CSRF token without adding it to Python code. diff --git a/docs/glossary.rst b/docs/glossary.rst index 0a46fac3b..0cf96f488 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -891,6 +891,11 @@ Glossary :meth:`pyramid.config.Configurator.set_session_factory` for more information. + CSRF storage policy + A utility that implements :class:`pyramid.interfaces.ICSRFStoragePolicy` + which is responsible for allocating CSRF tokens to a user and verifying + that a provided token is acceptable. + Mako `Mako `_ is a template language which refines the familiar ideas of componentized layout and inheritance diff --git a/docs/narr/security.rst b/docs/narr/security.rst index e67f7b98c..86e5c1ef4 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -766,6 +766,10 @@ a secret across two different subsystems might drop the security of signing to zero. Keys should not be re-used across different contexts where an attacker has the possibility of providing a chosen plaintext. +.. index:: + single: preventing cross-site request forgery attacks + single: cross-site request forgery attacks, prevention + Preventing Cross-Site Request Forgery Attacks --------------------------------------------- diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 86fe2a139..7e2469d54 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -315,7 +315,3 @@ flash storage. ['info message'] >>> request.session.peek_flash() [] - -.. index:: - single: preventing cross-site request forgery attacks - single: cross-site request forgery attacks, prevention diff --git a/pyramid/config/security.py b/pyramid/config/security.py index 0b565e322..6f5b36d3a 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -232,7 +232,7 @@ class SecurityConfiguratorMixin(object): @action_method def set_csrf_storage_policy(self, policy): """ - Set the CSRF storage policy used by subsequent view registrations. + Set the :term:`CSRF storage policy` used by subsequent view registrations. ``policy`` is a class that implements the :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all diff --git a/pyramid/config/views.py b/pyramid/config/views.py index e037f7706..2fc243fac 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -651,6 +651,12 @@ class ViewsConfiguratorMixin(object): .. versionadded:: 1.4a2 + .. versionchanged:: 1.9 + This feature requires either a :term:`session factory` to have been + configured, or a :term:`CSRF storage policy` other than the default + to be in use. + + physical_path If specified, this value should be a string or a tuple representing diff --git a/pyramid/csrf.py b/pyramid/csrf.py index 4c5a73940..ffc7b5fe3 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -31,7 +31,7 @@ class SessionCSRFStoragePolicy(object): Note that using this CSRF implementation requires that a :term:`session factory` is configured. - .. versionadded :: 1.8a1 + .. versionadded :: 1.9 """ def new_csrf_token(self, request): """ Sets a new CSRF token into the session and returns it. """ @@ -43,8 +43,8 @@ class SessionCSRFStoragePolicy(object): return request.session.get_csrf_token() def check_csrf_token(self, request, supplied_token): - """ Returns True if supplied_token is the same value as get_csrf_token - returns for this request. """ + """ Returns ``True`` if ``supplied_token is`` the same value as + ``get_csrf_token(request)``.""" expected = self.get_csrf_token(request) return not strings_differ( bytes_(expected, 'ascii'), @@ -55,11 +55,11 @@ class SessionCSRFStoragePolicy(object): class CookieCSRFStoragePolicy(object): """ An alternative CSRF implementation that stores its information in unauthenticated cookies, known as the 'Double Submit Cookie' method in the - OWASP CSRF guidelines. This gives some additional flexibility with regards - to scaling as the tokens can be generated and verified by a front-end - server. + `OWASP CSRF guidelines `_. + This gives some additional flexibility with regards to scaling as the tokens + can be generated and verified by a front-end server. - .. versionadded :: 1.8a1 + .. versionadded :: 1.9 """ def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, @@ -96,8 +96,8 @@ class CookieCSRFStoragePolicy(object): return token def check_csrf_token(self, request, supplied_token): - """ Returns True if supplied_token is the same value as get_csrf_token - returns for this request. """ + """ Returns ``True`` if ``supplied_token is`` the same value as + ``get_csrf_token(request)``.""" expected = self.get_csrf_token(request) return not strings_differ( bytes_(expected, 'ascii'), @@ -109,7 +109,7 @@ def get_csrf_token(request): a new one using ``new_csrf_token(request)`` if one does not exist. This calls the equivalent method in the chosen CSRF protection implementation. - .. versionadded :: 1.8a1 + .. versionadded :: 1.9 """ registry = request.registry csrf = registry.getUtility(ICSRFStoragePolicy) @@ -121,7 +121,7 @@ def new_csrf_token(request): implementation defined manner. This calls the equivalent method in the chosen CSRF protection implementation. - .. versionadded :: 1.8a1 + .. versionadded :: 1.9 """ registry = request.registry csrf = registry.getUtility(ICSRFStoragePolicy) @@ -159,8 +159,8 @@ def check_csrf_token(request, considered valid. It must be passed in either the request body or a header. - .. versionchanged:: 1.8a1 - Moved from pyramid.session to pyramid.csrf + .. versionchanged:: 1.9 + Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` """ supplied_token = "" # We first check the headers for a csrf token, as that is significantly @@ -192,27 +192,28 @@ def check_csrf_token(request, def check_csrf_origin(request, trusted_origins=None, raises=True): """ - Check the Origin of the request to see if it is a cross site request or + Check the ``Origin`` of the request to see if it is a cross site request or not. - If the value supplied by the Origin or Referer header isn't one of the + If the value supplied by the ``Origin`` or ``Referer`` header isn't one of the trusted origins and ``raises`` is ``True``, this function will raise a - :exc:`pyramid.exceptions.BadCSRFOrigin` exception but if ``raises`` is - ``False`` this function will return ``False`` instead. If the CSRF origin + :exc:`pyramid.exceptions.BadCSRFOrigin` exception, but if ``raises`` is + ``False``, this function will return ``False`` instead. If the CSRF origin checks are successful this function will return ``True`` unconditionally. Additional trusted origins may be added by passing a list of domain (and - ports if nonstandard like `['example.com', 'dev.example.com:8080']`) in + ports if nonstandard like ``['example.com', 'dev.example.com:8080']``) in with the ``trusted_origins`` parameter. If ``trusted_origins`` is ``None`` (the default) this list of additional domains will be pulled from the ``pyramid.csrf_trusted_origins`` setting. - Note that this function will do nothing if request.scheme is not https. + Note that this function will do nothing if ``request.scheme`` is not + ``https``. .. versionadded:: 1.7 - .. versionchanged:: 1.8a1 - Moved from pyramid.session to pyramid.csrf + .. versionchanged:: 1.9 + Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` """ def _fail(reason): if raises: diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index aab5647a1..c3b6b164d 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -999,8 +999,8 @@ class ICSRFStoragePolicy(Interface): """ def check_csrf_token(request, supplied_token): - """ Returns a boolean that represents if supplied_token is a valid CSRF - token for this request. Comparing strings for equality must be done + """ Returns a boolean that represents if ``supplied_token`` is a valid + CSRF token for this request. Comparing strings for equality must be done using :func:`pyramid.utils.strings_differ` to avoid timing attacks. """ -- cgit v1.2.3 From 4b3603ad2f4850605c45e1b7bf4f077584303641 Mon Sep 17 00:00:00 2001 From: Matthew Wilkes Date: Wed, 26 Apr 2017 15:43:18 +0100 Subject: Move CSRF storage policy registration out of PHASE_1 config and simplify tests given previous improvements to CSRF. --- docs/narr/extconfig.rst | 1 - pyramid/config/security.py | 2 +- pyramid/csrf.py | 6 ------ pyramid/testing.py | 1 + pyramid/tests/test_csrf.py | 14 +++++--------- 5 files changed, 7 insertions(+), 17 deletions(-) diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst index c20685cbf..4009ec1dc 100644 --- a/docs/narr/extconfig.rst +++ b/docs/narr/extconfig.rst @@ -263,7 +263,6 @@ Pre-defined Phases - :meth:`pyramid.config.Configurator.override_asset` - :meth:`pyramid.config.Configurator.set_authorization_policy` - :meth:`pyramid.config.Configurator.set_default_csrf_options` -- :meth:`pyramid.config.Configurator.set_csrf_storage_policy` - :meth:`pyramid.config.Configurator.set_default_permission` - :meth:`pyramid.config.Configurator.set_view_mapper` diff --git a/pyramid/config/security.py b/pyramid/config/security.py index 6f5b36d3a..9d59ca78e 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -241,7 +241,7 @@ class SecurityConfiguratorMixin(object): def register(): self.registry.registerUtility(policy, ICSRFStoragePolicy) - self.action(ICSRFStoragePolicy, register, order=PHASE1_CONFIG) + self.action(ICSRFStoragePolicy, register) @implementer(IDefaultCSRFOptions) diff --git a/pyramid/csrf.py b/pyramid/csrf.py index ffc7b5fe3..5d183bb57 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -177,12 +177,6 @@ def check_csrf_token(request, supplied_token = request.POST.get(token, "") policy = request.registry.queryUtility(ICSRFStoragePolicy) - if policy is None: - # There is no policy set, but we are trying to validate a CSRF token - # This means explicit validation has been asked for without configuring - # the CSRF implementation. Fall back to SessionCSRFStoragePolicy as that is the - # default - policy = SessionCSRFStoragePolicy() if not policy.check_csrf_token(request, supplied_token): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') diff --git a/pyramid/testing.py b/pyramid/testing.py index 877b351db..69b30e83f 100644 --- a/pyramid/testing.py +++ b/pyramid/testing.py @@ -479,6 +479,7 @@ def setUp(registry=None, request=None, hook_zca=True, autocommit=True, config.add_default_view_derivers() config.add_default_route_predicates() config.add_default_tweens() + config.add_default_security() config.commit() global have_zca try: diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index e6ae05eec..fcb6333ee 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -15,11 +15,9 @@ class Test_get_csrf_token(unittest.TestCase): from pyramid.csrf import get_csrf_token return get_csrf_token(*args, **kwargs) - def test_no_csrf_utility_registered(self): + def test_no_override_csrf_utility_registered(self): request = testing.DummyRequest() - - with self.assertRaises(ComponentLookupError): - self._callFUT(request) + self._callFUT(request) def test_success(self): self.config.set_csrf_storage_policy(DummyCSRF()) @@ -38,11 +36,9 @@ class Test_new_csrf_token(unittest.TestCase): from pyramid.csrf import new_csrf_token return new_csrf_token(*args, **kwargs) - def test_no_csrf_utility_registered(self): + def test_no_override_csrf_utility_registered(self): request = testing.DummyRequest() - - with self.assertRaises(ComponentLookupError): - self._callFUT(request) + self._callFUT(request) def test_success(self): self.config.set_csrf_storage_policy(DummyCSRF()) @@ -188,7 +184,7 @@ class Test_check_csrf_token(unittest.TestCase): def setUp(self): self.config = testing.setUp() - # set up CSRF (this will also register SessionCSRFStoragePolicy policy) + # set up CSRF self.config.set_default_csrf_options(require_csrf=False) def _callFUT(self, *args, **kwargs): -- cgit v1.2.3 From de299eb1ca359a2f13b109e57cff97098fbe00ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20HUBSCHER?= Date: Thu, 27 Apr 2017 10:00:28 +0200 Subject: Fix underlined title. --- docs/narr/myproject/README.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/narr/myproject/README.txt b/docs/narr/myproject/README.txt index 41ef0ff91..2ffc0acba 100644 --- a/docs/narr/myproject/README.txt +++ b/docs/narr/myproject/README.txt @@ -1,5 +1,5 @@ MyProject -=============================== +========= Getting Started --------------- -- cgit v1.2.3 From 68f673ff520c4bdffac796c9965936ec57916c72 Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Fri, 28 Apr 2017 00:09:20 -0700 Subject: update cookiecutter README.txt throughout docs - https://github.com/Pylons/pyramid-cookiecutter-starter/pull/28 - https://github.com/Pylons/pyramid-cookiecutter-zodb/pull/7 - https://github.com/Pylons/pyramid-cookiecutter-alchemy/pull/8 --- docs/quick_tour/logging/README.txt | 2 +- docs/quick_tour/package/README.txt | 2 +- docs/quick_tour/sessions/README.txt | 2 +- docs/quick_tour/sqla_demo/README.txt | 2 +- docs/quick_tutorial/cookiecutters/README.txt | 2 +- docs/tutorials/wiki/src/authorization/README.txt | 2 +- docs/tutorials/wiki/src/basiclayout/README.txt | 2 +- docs/tutorials/wiki/src/installation/README.txt | 2 +- docs/tutorials/wiki/src/models/README.txt | 2 +- docs/tutorials/wiki/src/tests/README.txt | 2 +- docs/tutorials/wiki/src/views/README.txt | 2 +- docs/tutorials/wiki2/src/authentication/README.txt | 2 +- docs/tutorials/wiki2/src/authorization/README.txt | 2 +- docs/tutorials/wiki2/src/basiclayout/README.txt | 2 +- docs/tutorials/wiki2/src/installation/README.txt | 2 +- docs/tutorials/wiki2/src/models/README.txt | 2 +- docs/tutorials/wiki2/src/tests/README.txt | 2 +- docs/tutorials/wiki2/src/views/README.txt | 2 +- 18 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/quick_tour/logging/README.txt b/docs/quick_tour/logging/README.txt index fb7bde0a7..ff70a1354 100644 --- a/docs/quick_tour/logging/README.txt +++ b/docs/quick_tour/logging/README.txt @@ -1,5 +1,5 @@ hello_world -=============================== +=========== Getting Started --------------- diff --git a/docs/quick_tour/package/README.txt b/docs/quick_tour/package/README.txt index fb7bde0a7..ff70a1354 100644 --- a/docs/quick_tour/package/README.txt +++ b/docs/quick_tour/package/README.txt @@ -1,5 +1,5 @@ hello_world -=============================== +=========== Getting Started --------------- diff --git a/docs/quick_tour/sessions/README.txt b/docs/quick_tour/sessions/README.txt index fb7bde0a7..ff70a1354 100644 --- a/docs/quick_tour/sessions/README.txt +++ b/docs/quick_tour/sessions/README.txt @@ -1,5 +1,5 @@ hello_world -=============================== +=========== Getting Started --------------- diff --git a/docs/quick_tour/sqla_demo/README.txt b/docs/quick_tour/sqla_demo/README.txt index 1659e47ab..27bbff5a7 100644 --- a/docs/quick_tour/sqla_demo/README.txt +++ b/docs/quick_tour/sqla_demo/README.txt @@ -1,5 +1,5 @@ sqla_demo -=============================== +========= Getting Started --------------- diff --git a/docs/quick_tutorial/cookiecutters/README.txt b/docs/quick_tutorial/cookiecutters/README.txt index 4b1f31bf3..55c5dcec6 100644 --- a/docs/quick_tutorial/cookiecutters/README.txt +++ b/docs/quick_tutorial/cookiecutters/README.txt @@ -1,5 +1,5 @@ cc_starter -=============================== +========== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/authorization/README.txt b/docs/tutorials/wiki/src/authorization/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/authorization/README.txt +++ b/docs/tutorials/wiki/src/authorization/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/basiclayout/README.txt b/docs/tutorials/wiki/src/basiclayout/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/basiclayout/README.txt +++ b/docs/tutorials/wiki/src/basiclayout/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/installation/README.txt b/docs/tutorials/wiki/src/installation/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/installation/README.txt +++ b/docs/tutorials/wiki/src/installation/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/models/README.txt b/docs/tutorials/wiki/src/models/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/models/README.txt +++ b/docs/tutorials/wiki/src/models/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/tests/README.txt b/docs/tutorials/wiki/src/tests/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/tests/README.txt +++ b/docs/tutorials/wiki/src/tests/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki/src/views/README.txt b/docs/tutorials/wiki/src/views/README.txt index 98683bf8c..5ec53bf9d 100644 --- a/docs/tutorials/wiki/src/views/README.txt +++ b/docs/tutorials/wiki/src/views/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/authentication/README.txt b/docs/tutorials/wiki2/src/authentication/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/authentication/README.txt +++ b/docs/tutorials/wiki2/src/authentication/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/authorization/README.txt b/docs/tutorials/wiki2/src/authorization/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/authorization/README.txt +++ b/docs/tutorials/wiki2/src/authorization/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/basiclayout/README.txt b/docs/tutorials/wiki2/src/basiclayout/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/basiclayout/README.txt +++ b/docs/tutorials/wiki2/src/basiclayout/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/installation/README.txt b/docs/tutorials/wiki2/src/installation/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/installation/README.txt +++ b/docs/tutorials/wiki2/src/installation/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/models/README.txt b/docs/tutorials/wiki2/src/models/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/models/README.txt +++ b/docs/tutorials/wiki2/src/models/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/tests/README.txt b/docs/tutorials/wiki2/src/tests/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/tests/README.txt +++ b/docs/tutorials/wiki2/src/tests/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- diff --git a/docs/tutorials/wiki2/src/views/README.txt b/docs/tutorials/wiki2/src/views/README.txt index 5e21b8aa4..81102a869 100644 --- a/docs/tutorials/wiki2/src/views/README.txt +++ b/docs/tutorials/wiki2/src/views/README.txt @@ -1,5 +1,5 @@ myproj -=============================== +====== Getting Started --------------- -- cgit v1.2.3 From 682a9b9df6f42f8261daa077f04b47b65bf00c34 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sat, 29 Apr 2017 01:43:38 -0500 Subject: final cleanup of csrf decoupling in #2854 - Renamed `SessionCSRFStoragePolicy` to `LegacySessionCSRFStoragePolicy` for the version that uses the legacy `ISession.get_csrf_token` and `ISession.new_csrf_token` apis and set that as the default. - Added new `SessionCSRFStoragePolicy` that stores data in the session similar to how the `SessionAuthenticationPolicy` works. - `CookieCSRFStoragePolicy` did not properly return the newly generated token from `get_csrf_token` after calling `new_csrf_token`. It needed to cache the new value since the response callback does not affect the current request. - `CookieCSRFStoragePolicy` was not forwarding the `domain` value to the `CookieProfile` causing that setting to be ignored. - Removed `check_csrf_token` from the `ICSRFStoragePolicy` interface to simplify implementations of storage policies. - Added an introspectable item for the configured storage policy so that it appears on the debugtoolbar. - Added a change note on `ISession` that it no longer required the csrf methods. - Leave deprecated shims in ``pyramid.session`` for ``check_csrf_origin`` and ``check_csrf_token``. --- CHANGES.txt | 13 +++--- docs/api/csrf.rst | 3 ++ docs/narr/security.rst | 1 + docs/narr/templates.rst | 4 ++ pyramid/config/security.py | 20 ++++++--- pyramid/csrf.py | 109 +++++++++++++++++++++++++++++---------------- pyramid/interfaces.py | 30 ++++++++----- pyramid/session.py | 14 ++++++ pyramid/tests/test_csrf.py | 108 ++++++++++++++++++++++---------------------- pyramid/tests/test_util.py | 6 ++- 10 files changed, 190 insertions(+), 118 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 762550053..7d70abbb8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,12 +24,13 @@ Features can be alleviated by invoking ``config.begin()`` and ``config.end()`` appropriately. See https://github.com/Pylons/pyramid/pull/2989 -- A new CSRF implementation, :class:`pyramid.csrf.SessionCSRF` has been added, - which delegates all CSRF generation to the current session, following the - old API for this. A ``get_csrf_token()`` method is now available in template - global scope, to make it easy for template developers to get the current CSRF - token without adding it to Python code. - See https://github.com/Pylons/pyramid/pull/2854 +- A new CSRF implementation, ``pyramid.csrf.SessionCSRFStoragePolicy``, + has been added which delegates all CSRF generation to the current session, + following the old API for this. A ``pyramid.csrf.get_csrf_token()`` api is now + available in template global scope, to make it easy for template developers + to get the current CSRF token without adding it to Python code. + See https://github.com/Pylons/pyramid/pull/2854 and + https://github.com/Pylons/pyramid/pull/3019 Bug Fixes diff --git a/docs/api/csrf.rst b/docs/api/csrf.rst index f890ee660..38501546e 100644 --- a/docs/api/csrf.rst +++ b/docs/api/csrf.rst @@ -5,6 +5,9 @@ .. automodule:: pyramid.csrf + .. autoclass:: LegacySessionCSRFStoragePolicy + :members: + .. autoclass:: SessionCSRFStoragePolicy :members: diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 86e5c1ef4..ddf496b69 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -824,6 +824,7 @@ If no CSRF token previously existed for this user, then a new token will be set into the session and returned. The newly created token will be opaque and randomized. +.. _get_csrf_token_in_templates: Using the ``get_csrf_token`` global in templates ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/narr/templates.rst b/docs/narr/templates.rst index 6b3b5fcce..4eadbd2f0 100644 --- a/docs/narr/templates.rst +++ b/docs/narr/templates.rst @@ -228,6 +228,10 @@ These values are provided to the template: provided if the template is rendered as the result of a ``renderer=`` argument to the view configuration being used. +``get_csrf_token()`` + A convenience function to access the current CSRF token. See + :ref:`get_csrf_token_in_templates` for more information. + ``renderer_name`` The renderer name used to perform the rendering, e.g., ``mypackage:templates/foo.pt``. diff --git a/pyramid/config/security.py b/pyramid/config/security.py index 9d59ca78e..8e4c908d3 100644 --- a/pyramid/config/security.py +++ b/pyramid/config/security.py @@ -10,7 +10,7 @@ from pyramid.interfaces import ( PHASE2_CONFIG, ) -from pyramid.csrf import SessionCSRFStoragePolicy +from pyramid.csrf import LegacySessionCSRFStoragePolicy from pyramid.exceptions import ConfigurationError from pyramid.util import action_method from pyramid.util import as_sorted_tuple @@ -19,7 +19,7 @@ from pyramid.util import as_sorted_tuple class SecurityConfiguratorMixin(object): def add_default_security(self): - self.set_csrf_storage_policy(SessionCSRFStoragePolicy()) + self.set_csrf_storage_policy(LegacySessionCSRFStoragePolicy()) @action_method def set_authentication_policy(self, policy): @@ -232,16 +232,22 @@ class SecurityConfiguratorMixin(object): @action_method def set_csrf_storage_policy(self, policy): """ - Set the :term:`CSRF storage policy` used by subsequent view registrations. + Set the :term:`CSRF storage policy` used by subsequent view + registrations. ``policy`` is a class that implements the - :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all - CSRF functionality. + :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface and defines + how to generate and persist CSRF tokens. + """ def register(): self.registry.registerUtility(policy, ICSRFStoragePolicy) - - self.action(ICSRFStoragePolicy, register) + intr = self.introspectable('csrf storage policy', + None, + policy, + 'csrf storage policy') + intr['policy'] = policy + self.action(ICSRFStoragePolicy, register, introspectables=(intr,)) @implementer(IDefaultCSRFOptions) diff --git a/pyramid/csrf.py b/pyramid/csrf.py index 5d183bb57..1910e4ec8 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -7,8 +7,9 @@ from zope.interface import implementer from pyramid.authentication import _SimpleSerializer from pyramid.compat import ( + bytes_, urlparse, - bytes_ + text_, ) from pyramid.exceptions import ( BadCSRFOrigin, @@ -23,44 +24,79 @@ from pyramid.util import ( @implementer(ICSRFStoragePolicy) -class SessionCSRFStoragePolicy(object): - """ The default CSRF implementation, which mimics the behavior from older - versions of Pyramid. The ``new_csrf_token`` and ``get_csrf_token`` methods - are indirected to the underlying session implementation. +class LegacySessionCSRFStoragePolicy(object): + """ A CSRF storage policy that defers control of CSRF storage to the + session. + + This policy maintains compatibility with legacy ISession implementations + that know how to manage CSRF tokens themselves via + ``ISession.new_csrf_token`` and ``ISession.get_csrf_token``. Note that using this CSRF implementation requires that a :term:`session factory` is configured. - .. versionadded :: 1.9 + .. versionadded:: 1.9 + """ def new_csrf_token(self, request): """ Sets a new CSRF token into the session and returns it. """ return request.session.new_csrf_token() def get_csrf_token(self, request): - """ Returns the currently active CSRF token from the session, generating - a new one if needed.""" + """ Returns the currently active CSRF token from the session, + generating a new one if needed.""" return request.session.get_csrf_token() - def check_csrf_token(self, request, supplied_token): - """ Returns ``True`` if ``supplied_token is`` the same value as - ``get_csrf_token(request)``.""" - expected = self.get_csrf_token(request) - return not strings_differ( - bytes_(expected, 'ascii'), - bytes_(supplied_token, 'ascii'), - ) + +@implementer(ICSRFStoragePolicy) +class SessionCSRFStoragePolicy(object): + """ A CSRF storage policy that persists the CSRF token in the session. + + Note that using this CSRF implementation requires that + a :term:`session factory` is configured. + + ``key`` + + The session key where the CSRF token will be stored. + Default: `_csrft_`. + + .. versionadded:: 1.9 + + """ + _token_factory = staticmethod(lambda: text_(uuid.uuid4().hex)) + + def __init__(self, key='_csrft_'): + self.key = key + + def new_csrf_token(self, request): + """ Sets a new CSRF token into the session and returns it. """ + token = self._token_factory() + request.session[self.key] = token + return token + + def get_csrf_token(self, request): + """ Returns the currently active CSRF token from the session, + generating a new one if needed.""" + token = request.session.get(self.key, None) + if not token: + token = self.new_csrf_token(request) + return token + @implementer(ICSRFStoragePolicy) class CookieCSRFStoragePolicy(object): """ An alternative CSRF implementation that stores its information in unauthenticated cookies, known as the 'Double Submit Cookie' method in the - `OWASP CSRF guidelines `_. - This gives some additional flexibility with regards to scaling as the tokens - can be generated and verified by a front-end server. + `OWASP CSRF guidelines `_. This gives some additional flexibility with + regards to scaling as the tokens can be generated and verified by a + front-end server. + + .. versionadded:: 1.9 - .. versionadded :: 1.9 """ + _token_factory = staticmethod(lambda: text_(uuid.uuid4().hex)) def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, domain=None, max_age=None, path='/'): @@ -71,13 +107,15 @@ class CookieCSRFStoragePolicy(object): max_age=max_age, httponly=httponly, path=path, + domains=[domain], serializer=serializer ) - self.domain = domain + self.cookie_name = cookie_name def new_csrf_token(self, request): """ Sets a new CSRF token into the request and returns it. """ - token = uuid.uuid4().hex + token = self._token_factory() + request.cookies[self.cookie_name] = token def set_cookie(request, response): self.cookie_profile.set_cookies( response, @@ -95,14 +133,6 @@ class CookieCSRFStoragePolicy(object): token = self.new_csrf_token(request) return token - def check_csrf_token(self, request, supplied_token): - """ Returns ``True`` if ``supplied_token is`` the same value as - ``get_csrf_token(request)``.""" - expected = self.get_csrf_token(request) - return not strings_differ( - bytes_(expected, 'ascii'), - bytes_(supplied_token, 'ascii'), - ) def get_csrf_token(request): """ Get the currently active CSRF token for the request passed, generating @@ -133,8 +163,8 @@ def check_csrf_token(request, header='X-CSRF-Token', raises=True): """ Check the CSRF token returned by the - :class:`pyramid.interfaces.ICSRFStoragePolicy` implementation against the value in - ``request.POST.get(token)`` (if a POST request) or + :class:`pyramid.interfaces.ICSRFStoragePolicy` implementation against the + value in ``request.POST.get(token)`` (if a POST request) or ``request.headers.get(header)``. If a ``token`` keyword is not supplied to this function, the string ``csrf_token`` will be used to look up the token in ``request.POST``. If a ``header`` keyword is not supplied to this @@ -143,11 +173,12 @@ def check_csrf_token(request, If the value supplied by post or by header doesn't match the value supplied by ``policy.get_csrf_token()`` (where ``policy`` is an implementation of - :class:`pyramid.interfaces.ICSRFStoragePolicy`), and ``raises`` is ``True``, this - function will raise an :exc:`pyramid.exceptions.BadCSRFToken` exception. If - the values differ and ``raises`` is ``False``, this function will return - ``False``. If the CSRF check is successful, this function will return - ``True`` unconditionally. + :class:`pyramid.interfaces.ICSRFStoragePolicy`), and ``raises`` is + ``True``, this function will raise an + :exc:`pyramid.exceptions.BadCSRFToken` exception. If the values differ + and ``raises`` is ``False``, this function will return ``False``. If the + CSRF check is successful, this function will return ``True`` + unconditionally. See :ref:`auto_csrf_checking` for information about how to secure your application automatically against CSRF attacks. @@ -176,8 +207,8 @@ def check_csrf_token(request, if supplied_token == "" and token is not None: supplied_token = request.POST.get(token, "") - policy = request.registry.queryUtility(ICSRFStoragePolicy) - if not policy.check_csrf_token(request, supplied_token): + expected_token = get_csrf_token(request) + if strings_differ(bytes_(expected_token), bytes_(supplied_token)): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') return False diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index c3b6b164d..853e8fcdd 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -927,6 +927,13 @@ class ISession(IDict): usually accessed via ``request.session``. Keys and values of a session must be pickleable. + + .. versionchanged:: 1.9 + + Sessions are no longer required to implement ``get_csrf_token`` and + ``new_csrf_token``. CSRF token support was moved to the pluggable + :class:`pyramid.interfaces.ICSRFStoragePolicy` configuration hook. + """ # attributes @@ -984,24 +991,23 @@ class ISession(IDict): class ICSRFStoragePolicy(Interface): """ An object that offers the ability to verify CSRF tokens and generate - new ones""" + new ones.""" def new_csrf_token(request): - """ Create and return a new, random cross-site request forgery - protection token. Return the token. It will be a string.""" + """ Create and return a new, random cross-site request forgery + protection token. The token will be an ascii-compatible unicode + string. + + """ def get_csrf_token(request): """ Return a cross-site request forgery protection token. It - will be a string. If a token was previously set for this user via - ``new_csrf_token``, that token will be returned. If no CSRF token - was previously set, ``new_csrf_token`` will be called, which will - create and set a token, and this token will be returned. - """ + will be an ascii-compatible unicode string. If a token was previously + set for this user via ``new_csrf_token``, that token will be returned. + If no CSRF token was previously set, ``new_csrf_token`` will be + called, which will create and set a token, and this token will be + returned. - def check_csrf_token(request, supplied_token): - """ Returns a boolean that represents if ``supplied_token`` is a valid - CSRF token for this request. Comparing strings for equality must be done - using :func:`pyramid.utils.strings_differ` to avoid timing attacks. """ diff --git a/pyramid/session.py b/pyramid/session.py index b1ad25410..33119343b 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -17,6 +17,10 @@ from pyramid.compat import ( bytes_, native_, ) +from pyramid.csrf import ( + check_csrf_origin, + check_csrf_token, +) from pyramid.interfaces import ISession from pyramid.util import strings_differ @@ -608,3 +612,13 @@ def SignedCookieSessionFactory( reissue_time=reissue_time, set_on_exception=set_on_exception, ) + +check_csrf_origin = check_csrf_origin # api +deprecated('check_csrf_origin', + 'pyramid.session.check_csrf_origin is deprecated as of Pyramid ' + '1.9. Use pyramid.csrf.check_csrf_origin instead.') + +check_csrf_token = check_csrf_token # api +deprecated('check_csrf_token', + 'pyramid.session.check_csrf_token is deprecated as of Pyramid ' + '1.9. Use pyramid.csrf.check_csrf_token instead.') diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index fcb6333ee..cd7ba2951 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -49,7 +49,7 @@ class Test_new_csrf_token(unittest.TestCase): self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b') -class TestSessionCSRFStoragePolicy(unittest.TestCase): +class TestLegacySessionCSRFStoragePolicy(unittest.TestCase): class MockSession(object): def new_csrf_token(self): return 'e5e9e30a08b34ff9842ff7d2b958c14b' @@ -58,11 +58,11 @@ class TestSessionCSRFStoragePolicy(unittest.TestCase): return '02821185e4c94269bdc38e6eeae0a2f8' def _makeOne(self): - from pyramid.csrf import SessionCSRFStoragePolicy - return SessionCSRFStoragePolicy() + from pyramid.csrf import LegacySessionCSRFStoragePolicy + return LegacySessionCSRFStoragePolicy() def test_register_session_csrf_policy(self): - from pyramid.csrf import SessionCSRFStoragePolicy + from pyramid.csrf import LegacySessionCSRFStoragePolicy from pyramid.interfaces import ICSRFStoragePolicy config = Configurator() @@ -71,7 +71,7 @@ class TestSessionCSRFStoragePolicy(unittest.TestCase): policy = config.registry.queryUtility(ICSRFStoragePolicy) - self.assertTrue(isinstance(policy, SessionCSRFStoragePolicy)) + self.assertTrue(isinstance(policy, LegacySessionCSRFStoragePolicy)) def test_session_csrf_implementation_delegates_to_session(self): policy = self._makeOne() @@ -86,26 +86,46 @@ class TestSessionCSRFStoragePolicy(unittest.TestCase): 'e5e9e30a08b34ff9842ff7d2b958c14b' ) - def test_verifying_token_invalid(self): + +class TestSessionCSRFStoragePolicy(unittest.TestCase): + def _makeOne(self, **kw): + from pyramid.csrf import SessionCSRFStoragePolicy + return SessionCSRFStoragePolicy(**kw) + + def test_register_session_csrf_policy(self): + from pyramid.csrf import SessionCSRFStoragePolicy + from pyramid.interfaces import ICSRFStoragePolicy + + config = Configurator() + config.set_csrf_storage_policy(self._makeOne()) + config.commit() + + policy = config.registry.queryUtility(ICSRFStoragePolicy) + + self.assertTrue(isinstance(policy, SessionCSRFStoragePolicy)) + + def test_it_creates_a_new_token(self): + request = DummyRequest(session={}) + policy = self._makeOne() - request = DummyRequest(session=self.MockSession()) + policy._token_factory = lambda: 'foo' + self.assertEqual(policy.get_csrf_token(request), 'foo') - result = policy.check_csrf_token(request, 'invalid-token') - self.assertFalse(result) + def test_get_csrf_token_returns_the_new_token(self): + request = DummyRequest(session={'_csrft_': 'foo'}) - def test_verifying_token_valid(self): policy = self._makeOne() - request = DummyRequest(session=self.MockSession()) + self.assertEqual(policy.get_csrf_token(request), 'foo') - result = policy.check_csrf_token( - request, '02821185e4c94269bdc38e6eeae0a2f8') - self.assertTrue(result) + token = policy.new_csrf_token(request) + self.assertNotEqual(token, 'foo') + self.assertEqual(token, policy.get_csrf_token(request)) class TestCookieCSRFStoragePolicy(unittest.TestCase): - def _makeOne(self): + def _makeOne(self, **kw): from pyramid.csrf import CookieCSRFStoragePolicy - return CookieCSRFStoragePolicy() + return CookieCSRFStoragePolicy(**kw) def test_register_cookie_csrf_policy(self): from pyramid.csrf import CookieCSRFStoragePolicy @@ -121,18 +141,18 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase): def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self): response = MockResponse() - request = DummyRequest(response=response) + request = DummyRequest() policy = self._makeOne() token = policy.get_csrf_token(request) + request.response_callback(request, response) self.assertEqual( response.headerlist, [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] ) def test_existing_cookie_csrf_does_not_set_cookie(self): - response = MockResponse() - request = DummyRequest(response=response) + request = DummyRequest() request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} policy = self._makeOne() @@ -142,42 +162,32 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase): token, 'e6f325fee5974f3da4315a8ccf4513d2' ) - self.assertEqual( - response.headerlist, - [], - ) + self.assertIsNone(request.response_callback) def test_new_cookie_csrf_with_existing_cookie_sets_cookies(self): - response = MockResponse() - request = DummyRequest(response=response) + request = DummyRequest() request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} policy = self._makeOne() token = policy.new_csrf_token(request) + + response = MockResponse() + request.response_callback(request, response) self.assertEqual( response.headerlist, [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] ) - def test_verifying_token_invalid_token(self): - response = MockResponse() - request = DummyRequest(response=response) - request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + def test_get_csrf_token_returns_the_new_token(self): + request = DummyRequest() + request.cookies = {'csrf_token': 'foo'} policy = self._makeOne() - self.assertFalse( - policy.check_csrf_token(request, 'invalid-token') - ) - - def test_verifying_token_against_existing_cookie(self): - response = MockResponse() - request = DummyRequest(response=response) - request.cookies = {'csrf_token': 'e6f325fee5974f3da4315a8ccf4513d2'} + self.assertEqual(policy.get_csrf_token(request), 'foo') - policy = self._makeOne() - self.assertTrue( - policy.check_csrf_token(request, 'e6f325fee5974f3da4315a8ccf4513d2') - ) + token = policy.new_csrf_token(request) + self.assertNotEqual(token, 'foo') + self.assertEqual(token, policy.get_csrf_token(request)) class Test_check_csrf_token(unittest.TestCase): @@ -224,14 +234,6 @@ class Test_check_csrf_token(unittest.TestCase): result = self._callFUT(request, 'csrf_token', raises=False) self.assertEqual(result, False) - def test_token_differing_types(self): - from pyramid.compat import text_ - request = testing.DummyRequest() - request.method = "POST" - request.session['_csrft_'] = text_('foo') - request.POST = {'csrf_token': b'foo'} - self.assertEqual(self._callFUT(request, token='csrf_token'), True) - class Test_check_csrf_token_without_defaults_configured(unittest.TestCase): def setUp(self): @@ -353,15 +355,15 @@ class Test_check_csrf_origin(unittest.TestCase): class DummyRequest(object): registry = None session = None - cookies = {} + response_callback = None - def __init__(self, registry=None, session=None, response=None): + def __init__(self, registry=None, session=None): self.registry = registry self.session = session - self.response = response + self.cookies = {} def add_response_callback(self, callback): - callback(self, self.response) + self.response_callback = callback class MockResponse(object): diff --git a/pyramid/tests/test_util.py b/pyramid/tests/test_util.py index bbf6103f4..d64f0a73f 100644 --- a/pyramid/tests/test_util.py +++ b/pyramid/tests/test_util.py @@ -369,12 +369,16 @@ class Test_strings_differ(unittest.TestCase): from pyramid.util import strings_differ return strings_differ(*args, **kw) - def test_it(self): + def test_it_bytes(self): self.assertFalse(self._callFUT(b'foo', b'foo')) self.assertTrue(self._callFUT(b'123', b'345')) self.assertTrue(self._callFUT(b'1234', b'123')) self.assertTrue(self._callFUT(b'123', b'1234')) + def test_it_native_str(self): + self.assertFalse(self._callFUT('123', '123')) + self.assertTrue(self._callFUT('123', '1234')) + def test_it_with_internal_comparator(self): result = self._callFUT(b'foo', b'foo', compare_digest=None) self.assertFalse(result) -- cgit v1.2.3 From 87af11c5e33b8c03d57a8b571f0b152efe866af1 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sat, 29 Apr 2017 21:48:49 -0500 Subject: add changelog for #2874 --- CHANGES.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index c8a87f625..8868e6ff7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,6 +24,12 @@ Features can be alleviated by invoking ``config.begin()`` and ``config.end()`` appropriately. See https://github.com/Pylons/pyramid/pull/2989 +- The ``pyramid.config.Configurator`` can now be used as a context manager + which will automatically push/pop threadlocals (similar to + ``config.begin()`` and ``config.end()``). It will also automatically perform + a ``config.commit()`` and thus it is only recommended to be used at the + top-level of your app. See https://github.com/Pylons/pyramid/pull/2874 + Bug Fixes --------- -- cgit v1.2.3 From 3f14d63c009ae7f101b7aeb4525bab2dfe66fa11 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 30 Apr 2017 02:00:48 -0500 Subject: restore the ``ICSRFStoragePolicy.check_csrf_token`` api --- pyramid/csrf.py | 35 ++++++++++--- pyramid/interfaces.py | 10 ++++ pyramid/tests/test_csrf.py | 121 +++++++++++++++++++++++++++------------------ 3 files changed, 113 insertions(+), 53 deletions(-) diff --git a/pyramid/csrf.py b/pyramid/csrf.py index 1910e4ec8..c8f097777 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -47,6 +47,12 @@ class LegacySessionCSRFStoragePolicy(object): generating a new one if needed.""" return request.session.get_csrf_token() + def check_csrf_token(self, request, supplied_token): + """ Returns ``True`` if the ``supplied_token`` is valid.""" + expected_token = self.get_csrf_token(request) + return not strings_differ( + bytes_(expected_token), bytes_(supplied_token)) + @implementer(ICSRFStoragePolicy) class SessionCSRFStoragePolicy(object): @@ -82,6 +88,12 @@ class SessionCSRFStoragePolicy(object): token = self.new_csrf_token(request) return token + def check_csrf_token(self, request, supplied_token): + """ Returns ``True`` if the ``supplied_token`` is valid.""" + expected_token = self.get_csrf_token(request) + return not strings_differ( + bytes_(expected_token), bytes_(supplied_token)) + @implementer(ICSRFStoragePolicy) class CookieCSRFStoragePolicy(object): @@ -133,6 +145,12 @@ class CookieCSRFStoragePolicy(object): token = self.new_csrf_token(request) return token + def check_csrf_token(self, request, supplied_token): + """ Returns ``True`` if the ``supplied_token`` is valid.""" + expected_token = self.get_csrf_token(request) + return not strings_differ( + bytes_(expected_token), bytes_(supplied_token)) + def get_csrf_token(request): """ Get the currently active CSRF token for the request passed, generating @@ -140,6 +158,7 @@ def get_csrf_token(request): calls the equivalent method in the chosen CSRF protection implementation. .. versionadded :: 1.9 + """ registry = request.registry csrf = registry.getUtility(ICSRFStoragePolicy) @@ -152,6 +171,7 @@ def new_csrf_token(request): chosen CSRF protection implementation. .. versionadded :: 1.9 + """ registry = request.registry csrf = registry.getUtility(ICSRFStoragePolicy) @@ -171,9 +191,8 @@ def check_csrf_token(request, function, the string ``X-CSRF-Token`` will be used to look up the token in ``request.headers``. - If the value supplied by post or by header doesn't match the value supplied - by ``policy.get_csrf_token()`` (where ``policy`` is an implementation of - :class:`pyramid.interfaces.ICSRFStoragePolicy`), and ``raises`` is + If the value supplied by post or by header cannot be verified by the + :class:`pyramid.interfaces.ICSRFStoragePolicy`, and ``raises`` is ``True``, this function will raise an :exc:`pyramid.exceptions.BadCSRFToken` exception. If the values differ and ``raises`` is ``False``, this function will return ``False``. If the @@ -191,7 +210,10 @@ def check_csrf_token(request, a header. .. versionchanged:: 1.9 - Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` + Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` and updated + to use the configured :class:`pyramid.interfaces.ICSRFStoragePolicy` to + verify the CSRF token. + """ supplied_token = "" # We first check the headers for a csrf token, as that is significantly @@ -207,8 +229,8 @@ def check_csrf_token(request, if supplied_token == "" and token is not None: supplied_token = request.POST.get(token, "") - expected_token = get_csrf_token(request) - if strings_differ(bytes_(expected_token), bytes_(supplied_token)): + policy = request.registry.getUtility(ICSRFStoragePolicy) + if not policy.check_csrf_token(request, text_(supplied_token)): if raises: raise BadCSRFToken('check_csrf_token(): Invalid token') return False @@ -239,6 +261,7 @@ def check_csrf_origin(request, trusted_origins=None, raises=True): .. versionchanged:: 1.9 Moved from :mod:`pyramid.session` to :mod:`pyramid.csrf` + """ def _fail(reason): if raises: diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index 853e8fcdd..ab83813c8 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -1010,6 +1010,16 @@ class ICSRFStoragePolicy(Interface): """ + def check_csrf_token(request, token): + """ Determine if the supplied ``token`` is valid. Most implementations + should simply compare the ``token`` to the current value of + ``get_csrf_token`` but it is possible to verify the token using + any mechanism necessary using this method. + + Returns ``True`` if the ``token`` is valid, otherwise ``False``. + + """ + class IIntrospector(Interface): def get(category_name, discriminator, default=None): diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index cd7ba2951..f01780ad8 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -1,61 +1,20 @@ import unittest -from zope.interface.interfaces import ComponentLookupError - from pyramid import testing from pyramid.config import Configurator -from pyramid.events import BeforeRender - - -class Test_get_csrf_token(unittest.TestCase): - def setUp(self): - self.config = testing.setUp() - - def _callFUT(self, *args, **kwargs): - from pyramid.csrf import get_csrf_token - return get_csrf_token(*args, **kwargs) - - def test_no_override_csrf_utility_registered(self): - request = testing.DummyRequest() - self._callFUT(request) - - def test_success(self): - self.config.set_csrf_storage_policy(DummyCSRF()) - request = testing.DummyRequest() - - csrf_token = self._callFUT(request) - - self.assertEquals(csrf_token, '02821185e4c94269bdc38e6eeae0a2f8') - - -class Test_new_csrf_token(unittest.TestCase): - def setUp(self): - self.config = testing.setUp() - - def _callFUT(self, *args, **kwargs): - from pyramid.csrf import new_csrf_token - return new_csrf_token(*args, **kwargs) - - def test_no_override_csrf_utility_registered(self): - request = testing.DummyRequest() - self._callFUT(request) - - def test_success(self): - self.config.set_csrf_storage_policy(DummyCSRF()) - request = testing.DummyRequest() - - csrf_token = self._callFUT(request) - - self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b') class TestLegacySessionCSRFStoragePolicy(unittest.TestCase): class MockSession(object): + def __init__(self, current_token='02821185e4c94269bdc38e6eeae0a2f8'): + self.current_token = current_token + def new_csrf_token(self): - return 'e5e9e30a08b34ff9842ff7d2b958c14b' + self.current_token = 'e5e9e30a08b34ff9842ff7d2b958c14b' + return self.current_token def get_csrf_token(self): - return '02821185e4c94269bdc38e6eeae0a2f8' + return self.current_token def _makeOne(self): from pyramid.csrf import LegacySessionCSRFStoragePolicy @@ -86,6 +45,13 @@ class TestLegacySessionCSRFStoragePolicy(unittest.TestCase): 'e5e9e30a08b34ff9842ff7d2b958c14b' ) + def test_check_csrf_token(self): + request = DummyRequest(session=self.MockSession('foo')) + + policy = self._makeOne() + self.assertTrue(policy.check_csrf_token(request, 'foo')) + self.assertFalse(policy.check_csrf_token(request, 'bar')) + class TestSessionCSRFStoragePolicy(unittest.TestCase): def _makeOne(self, **kw): @@ -121,6 +87,16 @@ class TestSessionCSRFStoragePolicy(unittest.TestCase): self.assertNotEqual(token, 'foo') self.assertEqual(token, policy.get_csrf_token(request)) + def test_check_csrf_token(self): + request = DummyRequest(session={}) + + policy = self._makeOne() + self.assertFalse(policy.check_csrf_token(request, 'foo')) + + request.session = {'_csrft_': 'foo'} + self.assertTrue(policy.check_csrf_token(request, 'foo')) + self.assertFalse(policy.check_csrf_token(request, 'bar')) + class TestCookieCSRFStoragePolicy(unittest.TestCase): def _makeOne(self, **kw): @@ -189,6 +165,57 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase): self.assertNotEqual(token, 'foo') self.assertEqual(token, policy.get_csrf_token(request)) + def test_check_csrf_token(self): + request = DummyRequest() + + policy = self._makeOne() + self.assertFalse(policy.check_csrf_token(request, 'foo')) + + request.cookies = {'csrf_token': 'foo'} + self.assertTrue(policy.check_csrf_token(request, 'foo')) + self.assertFalse(policy.check_csrf_token(request, 'bar')) + +class Test_get_csrf_token(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + def _callFUT(self, *args, **kwargs): + from pyramid.csrf import get_csrf_token + return get_csrf_token(*args, **kwargs) + + def test_no_override_csrf_utility_registered(self): + request = testing.DummyRequest() + self._callFUT(request) + + def test_success(self): + self.config.set_csrf_storage_policy(DummyCSRF()) + request = testing.DummyRequest() + + csrf_token = self._callFUT(request) + + self.assertEquals(csrf_token, '02821185e4c94269bdc38e6eeae0a2f8') + + +class Test_new_csrf_token(unittest.TestCase): + def setUp(self): + self.config = testing.setUp() + + def _callFUT(self, *args, **kwargs): + from pyramid.csrf import new_csrf_token + return new_csrf_token(*args, **kwargs) + + def test_no_override_csrf_utility_registered(self): + request = testing.DummyRequest() + self._callFUT(request) + + def test_success(self): + self.config.set_csrf_storage_policy(DummyCSRF()) + request = testing.DummyRequest() + + csrf_token = self._callFUT(request) + + self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b') + class Test_check_csrf_token(unittest.TestCase): def setUp(self): -- cgit v1.2.3 From 69828b5aa35ed3cf19941a0771c82418a0733b7e Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Sun, 30 Apr 2017 16:37:21 -0700 Subject: standardize "non-standard" --- docs/narr/security.rst | 2 +- pyramid/csrf.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/narr/security.rst b/docs/narr/security.rst index ddf496b69..3a6bfa5e5 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -946,7 +946,7 @@ automatic CSRF checking will also check the referrer of the request to ensure that it matches one of the trusted origins. By default the only trusted origin is the current host, however additional origins may be configured by setting ``pyramid.csrf_trusted_origins`` to a list of domain names (and ports if they -are non standard). If a host in the list of domains starts with a ``.`` then +are non-standard). If a host in the list of domains starts with a ``.`` then that will allow all subdomains as well as the domain without the ``.``. If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or diff --git a/pyramid/csrf.py b/pyramid/csrf.py index c8f097777..7c836e5ad 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -249,7 +249,7 @@ def check_csrf_origin(request, trusted_origins=None, raises=True): checks are successful this function will return ``True`` unconditionally. Additional trusted origins may be added by passing a list of domain (and - ports if nonstandard like ``['example.com', 'dev.example.com:8080']``) in + ports if non-standard like ``['example.com', 'dev.example.com:8080']``) in with the ``trusted_origins`` parameter. If ``trusted_origins`` is ``None`` (the default) this list of additional domains will be pulled from the ``pyramid.csrf_trusted_origins`` setting. -- cgit v1.2.3 From 6419a30f2322157a1faf3fce5bec5122a2ca69fa Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 30 Apr 2017 19:08:50 -0500 Subject: improve csrf changelog docs --- CHANGES.txt | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 075d3ffd9..719fbd495 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -11,6 +11,7 @@ Major Features For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding. + This may change in the future. See https://github.com/Pylons/pyramid/pull/2985 @@ -42,11 +43,26 @@ Features can be alleviated by invoking ``config.begin()`` and ``config.end()`` appropriately. See https://github.com/Pylons/pyramid/pull/2989 -- A new CSRF implementation, ``pyramid.csrf.SessionCSRFStoragePolicy``, - has been added which delegates all CSRF generation to the current session, - following the old API for this. A ``pyramid.csrf.get_csrf_token()`` api is now - available in template global scope, to make it easy for template developers - to get the current CSRF token without adding it to Python code. +- CSRF support has been refactored out of sessions and into its own + independent API in the ``pyramid.csrf`` module. It supports a pluggable + ``pyramid.interfaces.ICSRFStoragePolicy`` which can be used to define your + own mechanism for generating and validating CSRF tokens. By default, + Pyramid continues to use the ``pyramid.csrf.LegacySessionCSRFStoragePolicy`` + that uses the ``request.session.get_csrf_token`` and + ``request.session.new_csrf_token`` APIs under the hood to preserve + compatibility. Two new policies are shipped as well, + ``pyramid.csrf.SessionCSRFStoragePolicy`` and + ``pyramid.csrf.CookieCSRFStoragePolicy`` which will store the CSRF tokens + in the session and in a standalone cookie, respectively. The storage policy + can be changed by using the new + ``pyramid.config.Configurator.set_csrf_storage_policy`` config directive. + + CSRF tokens should be used via the new ``pyramid.csrf.get_csrf_token``, + ``pyramid.csrf.new_csrf_token`` and ``pyramid.csrf.check_csrf_token`` APIs + in order to continue working if the storage policy is changed. Also, the + ``pyramid.csrf.get_csrf_token`` function is injected into templates to be + used conveniently in UI code. + See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 -- cgit v1.2.3 From 9028c99445d4c0a7ac24aaa84a6db499397e691a Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 30 Apr 2017 19:09:17 -0500 Subject: move csrf changes to the "major features" section --- CHANGES.txt | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 719fbd495..bcfcc3107 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -26,23 +26,6 @@ Major Features See https://github.com/Pylons/pyramid/pull/2964 -Features --------- - -- Support an ``open_url`` config setting in the ``pserve`` section of the - config file. This url is used to open a web browser when ``pserve --browser`` - is invoked. When this setting is unavailable the ``pserve`` script will - attempt to guess the port the server is using from the - ``server:`` section of the config file but there is no - requirement that the server is being run in this format so it may fail. - See https://github.com/Pylons/pyramid/pull/2984 - -- The threadlocals are now available inside any function invoked via - ``config.include``. This means the only config-time code that cannot rely - on threadlocals is code executed from non-actions inside the main. This - can be alleviated by invoking ``config.begin()`` and ``config.end()`` - appropriately. See https://github.com/Pylons/pyramid/pull/2989 - - CSRF support has been refactored out of sessions and into its own independent API in the ``pyramid.csrf`` module. It supports a pluggable ``pyramid.interfaces.ICSRFStoragePolicy`` which can be used to define your @@ -66,6 +49,23 @@ Features See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 +Features +-------- + +- Support an ``open_url`` config setting in the ``pserve`` section of the + config file. This url is used to open a web browser when ``pserve --browser`` + is invoked. When this setting is unavailable the ``pserve`` script will + attempt to guess the port the server is using from the + ``server:`` section of the config file but there is no + requirement that the server is being run in this format so it may fail. + See https://github.com/Pylons/pyramid/pull/2984 + +- The threadlocals are now available inside any function invoked via + ``config.include``. This means the only config-time code that cannot rely + on threadlocals is code executed from non-actions inside the main. This + can be alleviated by invoking ``config.begin()`` and ``config.end()`` + appropriately. See https://github.com/Pylons/pyramid/pull/2989 + - The ``pyramid.config.Configurator`` can now be used as a context manager which will automatically push/pop threadlocals (similar to ``config.begin()`` and ``config.end()``). It will also automatically perform -- cgit v1.2.3 From 847fb70980aca38b0dc415e2b433618d7e42ac8d Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 30 Apr 2017 19:10:12 -0500 Subject: improve flow of changes for configurator threadlocals --- CHANGES.txt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index bcfcc3107..e30f185f0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -60,18 +60,19 @@ Features requirement that the server is being run in this format so it may fail. See https://github.com/Pylons/pyramid/pull/2984 -- The threadlocals are now available inside any function invoked via - ``config.include``. This means the only config-time code that cannot rely - on threadlocals is code executed from non-actions inside the main. This - can be alleviated by invoking ``config.begin()`` and ``config.end()`` - appropriately. See https://github.com/Pylons/pyramid/pull/2989 - - The ``pyramid.config.Configurator`` can now be used as a context manager which will automatically push/pop threadlocals (similar to ``config.begin()`` and ``config.end()``). It will also automatically perform a ``config.commit()`` and thus it is only recommended to be used at the top-level of your app. See https://github.com/Pylons/pyramid/pull/2874 +- The threadlocals are now available inside any function invoked via + ``config.include``. This means the only config-time code that cannot rely + on threadlocals is code executed from non-actions inside the main. This + can be alleviated by invoking ``config.begin()`` and ``config.end()`` + appropriately or using the new context manager feature of the configurator. + See https://github.com/Pylons/pyramid/pull/2989 + Bug Fixes --------- -- cgit v1.2.3 From 2b9b6cab969eab9b1976a1a9a29ed2e44e92ca6d Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:10:30 -0500 Subject: update changelog and add whatsnew-1.9 --- CHANGES.txt | 27 ++++++++++++++++++--------- docs/index.rst | 1 + 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index e30f185f0..861dfa684 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -49,8 +49,8 @@ Major Features See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 -Features --------- +Minor Features +-------------- - Support an ``open_url`` config setting in the ``pserve`` section of the config file. This url is used to open a web browser when ``pserve --browser`` @@ -94,12 +94,21 @@ Bug Fixes Deprecations ------------ -Backward Incompatibilities --------------------------- +- Pyramid currently depends on ``plaster_pastedeploy`` to simplify the + transition to ``plaster`` by maintaining integrated support for INI files. + This dependency on ``plaster_pastedeploy`` should be considered subject to + Pyramid's deprecation policy and is subject to removal in the future. + Applications should depend on the appropriate plaster binding to satisfy + their needs. + +- Retrieving CSRF token from the session has been deprecated in favor of + equivalent methods in the ``pyramid.csrf`` module. The CSRF methods + (``ISession.get_csrf_token`` and ``ISession.new_csrf_token``) are no longer + required on the ``ISession`` interface except when using the default + ``pyramid.csrf.LegacySessionCSRFStoragePolicy``. -Documentation Changes ---------------------- + Also, ``pyramid.session.check_csrf_token`` is now located at + ``pyramid.csrf.check_csrf_token``. -- Retrieving CSRF token from the session has been deprecated, in favor of - equivalent methods in :mod:`pyramid.csrf`. - See https://github.com/Pylons/pyramid/pull/2854 + See https://github.com/Pylons/pyramid/pull/2854 and + https://github.com/Pylons/pyramid/pull/3019 diff --git a/docs/index.rst b/docs/index.rst index ed5b458ea..7d3393548 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -185,6 +185,7 @@ Change History .. toctree:: :maxdepth: 1 + whatsnew-1.9 whatsnew-1.8 whatsnew-1.7 whatsnew-1.6 -- cgit v1.2.3 From 4245b85e8041c87b9eb7ebd60707813d05d7e004 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:30:09 -0500 Subject: really add whatsnew-1.9 --- docs/whatsnew-1.9.rst | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 docs/whatsnew-1.9.rst diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst new file mode 100644 index 000000000..7ceefbf49 --- /dev/null +++ b/docs/whatsnew-1.9.rst @@ -0,0 +1,49 @@ +What's New in Pyramid 1.9 +========================= + +This article explains the new features in :app:`Pyramid` version 1.9 as compared to its predecessor, :app:`Pyramid` 1.8. It also documents backwards incompatibilities between the two versions and deprecations added to :app:`Pyramid` 1.9, as well as software dependency changes and notable documentation additions. + +Major Feature Additions +----------------------- + +- The file format used by all ``p*`` command line scripts such as ``pserve`` and ``pshell``, as well as the :func:`pyramid.paster.bootstrap` function is now replaceable thanks to a new dependency on `plaster `_. + + For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding library. This may change in the future so it is recommended for applications to start depending on the appropriate plaster binding for their needs. + + See https://github.com/Pylons/pyramid/pull/2985 + +- Added an :term:`execution policy` hook to the request pipeline. An execution policy has the ability to control creation and execution of the request objects before they enter the rest of the pipeline. This means for a single request environ the policy may create more than one request object. + + The execution policy can be replaced using the new :meth:`pyramid.config.Configurator.set_execution_policy` config directive. + + The first library to use this feature is `pyramid_retry `_. + + See https://github.com/Pylons/pyramid/pull/2964 + +- CSRF support has been refactored out of sessions and into its own independent API in the :mod:`pyramid.csrf` module. It supports a pluggable :class:`pyramid.interfaces.ICSRFStoragePolicy` which can be used to define your own mechanism for generating and validating CSRF tokens. By default, Pyramid continues to use the :class:`pyramid.csrf.LegacySessionCSRFStoragePolicy` that uses the ``request.session.get_csrf_token`` and ``request.session.new_csrf_token`` APIs under the hood to preserve compatibility with older Pyramid applications. Two new policies are shipped as well, :class:`pyramid.csrf.SessionCSRFStoragePolicy` and :class:`pyramid.csrf.CookieCSRFStoragePolicy` which will store the CSRF tokens in the session and in a standalone cookie, respectively. The storage policy can be changed by using the new :meth:`pyramid.config.Configurator.set_csrf_storage_policy` config directive. + + CSRF tokens should be used via the new :func:`pyramid.csrf.get_csrf_token`, :func:`pyramid.csrf.new_csrf_token` and :func:`pyramid.csrf.check_csrf_token`` APIs in order to continue working if the storage policy is changed. Also, the :func:`pyramid.csrf.get_csrf_token` function is now injected into templates to be used conveniently in UI code. + + See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 + +Minor Feature Additions +----------------------- + +- Support an ``open_url`` config setting in the ``pserve`` section of the config file. This url is used to open a web browser when ``pserve --browser`` is invoked. When this setting is unavailable the ``pserve`` script will attempt to guess the port the server is using from the ``server:`` section of the config file but there is no requirement that the server is being run in this format so it may fail. See https://github.com/Pylons/pyramid/pull/2984 + +- The :class:`pyramid.config.Configurator` can now be used as a context manager which will automatically push/pop threadlocals (similar to :meth:`pyramid.config.Configurator.begin` and `pyramid.config.Configurator.end`). It will also automatically perform a :meth:`pyramid.config.Configurator.commit` at the end and thus it is only recommended to be used at the top-level of your app. See https://github.com/Pylons/pyramid/pull/2874 + +- The threadlocals are now available inside any function invoked via :meth:`pyramid.config.Configurator.include`. This means the only config-time code that cannot rely on threadlocals is code executed from non-actions inside the main. This can be alleviated by invoking :meth:`pyramid.config.Configurator.begin` and :meth:`pyramid.config.Configurator.end` appropriately or using the new context manager feature of the configurator. See https://github.com/Pylons/pyramid/pull/2989 + +Deprecations +------------ + +- Pyramid currently depends on ``plaster_pastedeploy`` to simplify the transition to ``plaster`` by maintaining integrated support for INI files. This dependency on ``plaster_pastedeploy`` should be considered subject to Pyramid's deprecation policy and is subject to removal in the future. Applications should depend on the appropriate plaster binding to satisfy their needs. + +- Retrieving CSRF token from the session has been deprecated in favor of equivalent methods in the :mod:`pyramid.csrf` module. The CSRF methods (``ISession.get_csrf_token`` and ``ISession.new_csrf_token``) are no longer required on the :class:`pyramid.interfaces.ISession` interface except when using the default :class:`pyramid.csrf.LegacySessionCSRFStoragePolicy`. + + Also, ``pyramid.session.check_csrf_token`` is now located at + :func:`pyramid.csrf.check_csrf_token`. + + See https://github.com/Pylons/pyramid/pull/2854 and + https://github.com/Pylons/pyramid/pull/3019 -- cgit v1.2.3 From 33f1c247272e58993003777677c7c6a7a4f5e38c Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:33:36 -0500 Subject: switch readme to 1.9-branch badges --- README.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 302590fe1..5f42115df 100644 --- a/README.rst +++ b/README.rst @@ -1,16 +1,16 @@ Pyramid ======= -.. image:: https://travis-ci.org/Pylons/pyramid.png?branch=master +.. image:: https://travis-ci.org/Pylons/pyramid.png?branch=1.9-branch :target: https://travis-ci.org/Pylons/pyramid :alt: master Travis CI Status -.. image:: https://readthedocs.org/projects/pyramid/badge/?version=master - :target: http://docs.pylonsproject.org/projects/pyramid/en/master/ +.. image:: https://readthedocs.org/projects/pyramid/badge/?version=1.9-branch + :target: http://docs.pylonsproject.org/projects/pyramid/en/1.9-branch/ :alt: Master Documentation Status -.. image:: https://readthedocs.org/projects/pyramid/badge/?version=latest - :target: http://docs.pylonsproject.org/projects/pyramid/en/latest/ +.. image:: https://readthedocs.org/projects/pyramid/badge/?version=1.9-branch + :target: http://docs.pylonsproject.org/projects/pyramid/en/1.9-branch/ :alt: Latest Documentation Status .. image:: https://img.shields.io/badge/irc-freenode-blue.svg -- cgit v1.2.3 From 156c7fa32d6bf6e7786967920ca89403b0eb16fd Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:38:08 -0500 Subject: link to 1.9-branch in contributing --- contributing.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contributing.md b/contributing.md index 9deeee035..82f60e7b8 100644 --- a/contributing.md +++ b/contributing.md @@ -26,6 +26,8 @@ listed below. * [master](https://github.com/Pylons/pyramid/) - The branch on which further development takes place. The default branch on GitHub. +* [1.9-branch](https://github.com/Pylons/pyramid/tree/1.9-branch) - The branch + classified as "alpha". * [1.8-branch](https://github.com/Pylons/pyramid/tree/1.8-branch) - The branch classified as "stable" or "latest". * [1.7-branch](https://github.com/Pylons/pyramid/tree/1.7-branch) - The oldest -- cgit v1.2.3 From fdd77da6231fa9286c3f6fa494ae0731570e0134 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:42:54 -0500 Subject: link to plaster_pastedeploy --- CHANGES.txt | 5 +++-- docs/whatsnew-1.9.rst | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 861dfa684..a6cac805f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -10,8 +10,9 @@ Major Features `plaster `_. For now, Pyramid is still shipping with integrated support for the - PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding. - This may change in the future. + PasteDeploy INI format by depending on the + `plaster_pastedeploy `_. - For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the ``plaster_pastedeploy`` binding library. This may change in the future so it is recommended for applications to start depending on the appropriate plaster binding for their needs. + For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the `plaster_pastedeploy Date: Mon, 1 May 2017 21:49:28 -0500 Subject: fix rst syntax --- docs/whatsnew-1.9.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index 291f731ed..dd5ab894d 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -31,7 +31,7 @@ Minor Feature Additions - Support an ``open_url`` config setting in the ``pserve`` section of the config file. This url is used to open a web browser when ``pserve --browser`` is invoked. When this setting is unavailable the ``pserve`` script will attempt to guess the port the server is using from the ``server:`` section of the config file but there is no requirement that the server is being run in this format so it may fail. See https://github.com/Pylons/pyramid/pull/2984 -- The :class:`pyramid.config.Configurator` can now be used as a context manager which will automatically push/pop threadlocals (similar to :meth:`pyramid.config.Configurator.begin` and `pyramid.config.Configurator.end`). It will also automatically perform a :meth:`pyramid.config.Configurator.commit` at the end and thus it is only recommended to be used at the top-level of your app. See https://github.com/Pylons/pyramid/pull/2874 +- The :class:`pyramid.config.Configurator` can now be used as a context manager which will automatically push/pop threadlocals (similar to :meth:`pyramid.config.Configurator.begin` and :meth:`pyramid.config.Configurator.end`). It will also automatically perform a :meth:`pyramid.config.Configurator.commit` at the end and thus it is only recommended to be used at the top-level of your app. See https://github.com/Pylons/pyramid/pull/2874 - The threadlocals are now available inside any function invoked via :meth:`pyramid.config.Configurator.include`. This means the only config-time code that cannot rely on threadlocals is code executed from non-actions inside the main. This can be alleviated by invoking :meth:`pyramid.config.Configurator.begin` and :meth:`pyramid.config.Configurator.end` appropriately or using the new context manager feature of the configurator. See https://github.com/Pylons/pyramid/pull/2989 -- cgit v1.2.3 From 7850884719c94c0721748b5458504cb8a9d242c8 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 21:54:55 -0500 Subject: add changelog for #2993 --- CHANGES.txt | 6 ++++++ docs/whatsnew-1.9.rst | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index a6cac805f..70a2ff922 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -113,3 +113,9 @@ Deprecations See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 + +Documentation Changes +--------------------- + +- Added the execution policy to the routing diagram in the Request Processing + chatper. See https://github.com/Pylons/pyramid/pull/2993 diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index dd5ab894d..e57ed254d 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -47,3 +47,9 @@ Deprecations See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 + +Documentation Enhancements +-------------------------- + +- Added the :term:`execution policy` to the routing diagram in + :ref:`router_chapter`. See https://github.com/Pylons/pyramid/pull/2993 -- cgit v1.2.3 From 2aebc688c6a81b1baef01791e1cf3c9907c7c3ee Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 22:07:55 -0500 Subject: line length fixes in whatsnew-1.9 --- CHANGES.txt | 2 +- docs/whatsnew-1.9.rst | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 70a2ff922..85931d7f1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -98,7 +98,7 @@ Deprecations - Pyramid currently depends on ``plaster_pastedeploy`` to simplify the transition to ``plaster`` by maintaining integrated support for INI files. This dependency on ``plaster_pastedeploy`` should be considered subject to - Pyramid's deprecation policy and is subject to removal in the future. + Pyramid's deprecation policy and may be removed in the future. Applications should depend on the appropriate plaster binding to satisfy their needs. diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index e57ed254d..5f9e0e011 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -38,18 +38,15 @@ Minor Feature Additions Deprecations ------------ -- Pyramid currently depends on ``plaster_pastedeploy`` to simplify the transition to ``plaster`` by maintaining integrated support for INI files. This dependency on ``plaster_pastedeploy`` should be considered subject to Pyramid's deprecation policy and is subject to removal in the future. Applications should depend on the appropriate plaster binding to satisfy their needs. +- Pyramid currently depends on ``plaster_pastedeploy`` to simplify the transition to ``plaster`` by maintaining integrated support for INI files. This dependency on ``plaster_pastedeploy`` should be considered subject to Pyramid's deprecation policy and may be removed in the future. Applications should depend on the appropriate plaster binding to satisfy their needs. - Retrieving CSRF token from the session has been deprecated in favor of equivalent methods in the :mod:`pyramid.csrf` module. The CSRF methods (``ISession.get_csrf_token`` and ``ISession.new_csrf_token``) are no longer required on the :class:`pyramid.interfaces.ISession` interface except when using the default :class:`pyramid.csrf.LegacySessionCSRFStoragePolicy`. - Also, ``pyramid.session.check_csrf_token`` is now located at - :func:`pyramid.csrf.check_csrf_token`. + Also, ``pyramid.session.check_csrf_token`` is now located at :func:`pyramid.csrf.check_csrf_token`. - See https://github.com/Pylons/pyramid/pull/2854 and - https://github.com/Pylons/pyramid/pull/3019 + See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 Documentation Enhancements -------------------------- -- Added the :term:`execution policy` to the routing diagram in - :ref:`router_chapter`. See https://github.com/Pylons/pyramid/pull/2993 +- Added the :term:`execution policy` to the routing diagram in :ref:`router_chapter`. See https://github.com/Pylons/pyramid/pull/2993 -- cgit v1.2.3 From 840508d2a104d1e729bbb1e91f50947da6988133 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 22:51:18 -0500 Subject: typo --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 85931d7f1..d77a5e49b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -118,4 +118,4 @@ Documentation Changes --------------------- - Added the execution policy to the routing diagram in the Request Processing - chatper. See https://github.com/Pylons/pyramid/pull/2993 + chapter. See https://github.com/Pylons/pyramid/pull/2993 -- cgit v1.2.3 From 2ea4b00b1055864a1f10dd9c4c8a0b903b25080c Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 22:51:44 -0500 Subject: add the license file to the wheel's dist-info --- setup.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.cfg b/setup.cfg index 9a241ddf5..6f1f33760 100644 --- a/setup.cfg +++ b/setup.cfg @@ -13,6 +13,9 @@ docs = develop easy_install pyramid[docs] [bdist_wheel] universal = 1 +[metadata] +license_file = LICENSE.txt + [flake8] ignore = # E121: continuation line under-indented for hanging indent -- cgit v1.2.3 From c84904381f664511060b39d0937fbe76efa22f25 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 22:51:58 -0500 Subject: prep 1.9a1 --- CHANGES.txt | 4 ++-- setup.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index d77a5e49b..0513fd3c9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,5 @@ -unreleased -========== +1.9a1 (2017-05-01) +================== Major Features -------------- diff --git a/setup.py b/setup.py index 3048428aa..6aac12ff8 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ testing_extras = tests_require + [ ] setup(name='pyramid', - version='1.9.dev0', + version='1.9a1', description='The Pyramid Web Framework, a Pylons project', long_description=README + '\n\n' + CHANGES, classifiers=[ -- cgit v1.2.3 From c273cd0471afe365d9bd8a793a81897a9e713aab Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 22:57:19 -0500 Subject: fix url syntax --- docs/whatsnew-1.9.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index 5f9e0e011..b1a406a74 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -8,7 +8,7 @@ Major Feature Additions - The file format used by all ``p*`` command line scripts such as ``pserve`` and ``pshell``, as well as the :func:`pyramid.paster.bootstrap` function is now replaceable thanks to a new dependency on `plaster `_. - For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the `plaster_pastedeploy `_ binding library. This may change in the future so it is recommended for applications to start depending on the appropriate plaster binding for their needs. See https://github.com/Pylons/pyramid/pull/2985 -- cgit v1.2.3 From fbfd8191cee8536078cc01cd2256378ba0711f22 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Mon, 1 May 2017 23:05:53 -0500 Subject: fix url syntax yet again --- CHANGES.txt | 2 +- docs/conf.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0513fd3c9..2378ec883 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -11,7 +11,7 @@ Major Features For now, Pyramid is still shipping with integrated support for the PasteDeploy INI format by depending on the - `plaster_pastedeploy `_ binding library. This may change in the future. See https://github.com/Pylons/pyramid/pull/2985 diff --git a/docs/conf.py b/docs/conf.py index df58064e5..f09ae325b 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -67,6 +67,7 @@ intersphinx_mapping = { 'cookiecutter': ('https://cookiecutter.readthedocs.io/en/latest/', None), 'deform': ('http://docs.pylonsproject.org/projects/deform/en/latest', None), 'jinja2': ('http://docs.pylonsproject.org/projects/pyramid-jinja2/en/latest/', None), + 'plaster': ('http://docs.pylonsproject.org/projects/plaster/en/latest/', None), 'pylonswebframework': ('http://docs.pylonsproject.org/projects/pylons-webframework/en/latest/', None), 'python': ('https://docs.python.org/3', None), 'pytest': ('http://pytest.org/latest/', None), -- cgit v1.2.3 From 216e105262f23f7469a72265d94ed0181ec04bc3 Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Tue, 2 May 2017 00:32:30 -0700 Subject: kill off pylonsrtd --- RELEASING.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/RELEASING.txt b/RELEASING.txt index b9e5f4a6c..cd38b9871 100644 --- a/RELEASING.txt +++ b/RELEASING.txt @@ -111,8 +111,6 @@ Marketing and communications - Edit Pylons/trypyramid.com/src/templates/resources.html for major releases only. -- Edit Pylons/pylonsrtd/pylonsrtd/docs/pyramid.rst for major releases only. - - Edit `http://wiki.python.org/moin/WebFrameworks `_. -- cgit v1.2.3 From 7d1e0646d701b35d61ca8800341c07afac31bb86 Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Tue, 2 May 2017 00:34:24 -0700 Subject: add more events for updating trypyramid.com --- RELEASING.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RELEASING.txt b/RELEASING.txt index cd38b9871..58ebb2fb3 100644 --- a/RELEASING.txt +++ b/RELEASING.txt @@ -108,8 +108,8 @@ Update previous version (final releases only) Marketing and communications ---------------------------- -- Edit Pylons/trypyramid.com/src/templates/resources.html for major releases - only. +- Edit Pylons/trypyramid.com/src/templates/resources.html for major releases, + pre-releases, and once pre-releases are final. - Edit `http://wiki.python.org/moin/WebFrameworks `_. -- cgit v1.2.3 From 7936210eb8f6da693fcab9ed04dfed277874eeb0 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 2 May 2017 21:43:49 -0500 Subject: clean request.exception if the excview fails to handle the error request.exception is only not None if the response was generated by the excview fixes #3027 --- pyramid/tests/test_tweens.py | 17 ++++++++- pyramid/tweens.py | 86 +++++++++++++++++++++++--------------------- 2 files changed, 61 insertions(+), 42 deletions(-) diff --git a/pyramid/tests/test_tweens.py b/pyramid/tests/test_tweens.py index c8eada34c..2e74ad7cf 100644 --- a/pyramid/tests/test_tweens.py +++ b/pyramid/tests/test_tweens.py @@ -22,6 +22,8 @@ class Test_excview_tween_factory(unittest.TestCase): request = DummyRequest() result = tween(request) self.assertTrue(result is dummy_response) + self.assertIsNone(request.exception) + self.assertIsNone(request.exc_info) def test_it_catches_notfound(self): from pyramid.request import Request @@ -31,8 +33,11 @@ class Test_excview_tween_factory(unittest.TestCase): raise HTTPNotFound tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry result = tween(request) self.assertEqual(result.status, '404 Not Found') + self.assertIsInstance(request.exception, HTTPNotFound) + self.assertEqual(request.exception, request.exc_info[1]) def test_it_catches_with_predicate(self): from pyramid.request import Request @@ -44,8 +49,11 @@ class Test_excview_tween_factory(unittest.TestCase): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry result = tween(request) self.assertTrue(b'foo' in result.body) + self.assertIsInstance(request.exception, ValueError) + self.assertEqual(request.exception, request.exc_info[1]) def test_it_reraises_on_mismatch(self): from pyramid.request import Request @@ -55,8 +63,11 @@ class Test_excview_tween_factory(unittest.TestCase): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry request.method = 'POST' self.assertRaises(ValueError, lambda: tween(request)) + self.assertIsNone(request.exception) + self.assertIsNone(request.exc_info) def test_it_reraises_on_no_match(self): from pyramid.request import Request @@ -64,10 +75,14 @@ class Test_excview_tween_factory(unittest.TestCase): raise ValueError tween = self._makeOne(handler) request = Request.blank('/') + request.registry = self.config.registry self.assertRaises(ValueError, lambda: tween(request)) + self.assertIsNone(request.exception) + self.assertIsNone(request.exc_info) class DummyRequest: - pass + exception = None + exc_info = None class DummyResponse: pass diff --git a/pyramid/tweens.py b/pyramid/tweens.py index a842b1133..673429b06 100644 --- a/pyramid/tweens.py +++ b/pyramid/tweens.py @@ -10,6 +10,50 @@ from pyramid.interfaces import ( from zope.interface import providedBy from pyramid.view import _call_view +def _error_handler(request, exc): + # NOTE: we do not need to delete exc_info because this function + # should never be in the call stack of the exception + exc_info = sys.exc_info() + + attrs = request.__dict__ + attrs['exc_info'] = exc_info + attrs['exception'] = exc + # clear old generated request.response, if any; it may + # have been mutated by the view, and its state is not + # sane (e.g. caching headers) + if 'response' in attrs: + del attrs['response'] + # we use .get instead of .__getitem__ below due to + # https://github.com/Pylons/pyramid/issues/700 + request_iface = attrs.get('request_iface', IRequest) + provides = providedBy(exc) + try: + response = _call_view( + request.registry, + request, + exc, + provides, + '', + view_classifier=IExceptionViewClassifier, + request_iface=request_iface.combined + ) + + # if views matched but did not pass predicates then treat the + # same as not finding any matching views + except PredicateMismatch: + response = None + + # re-raise the original exception as no exception views were + # able to handle the error + if response is None: + if 'exception' in attrs: + del attrs['exception'] + if 'exc_info' in attrs: + del attrs['exc_info'] + reraise(*exc_info) + + return response + def excview_tween_factory(handler, registry): """ A :term:`tween` factory which produces a tween that catches an exception raised by downstream tweens (or the main Pyramid request @@ -17,50 +61,10 @@ def excview_tween_factory(handler, registry): :term:`exception view`.""" def excview_tween(request): - attrs = request.__dict__ try: response = handler(request) except Exception as exc: - # WARNING: do not assign the result of sys.exc_info() to a local - # var here, doing so will cause a leak. We used to actually - # explicitly delete both "exception" and "exc_info" from ``attrs`` - # in a ``finally:`` clause below, but now we do not because these - # attributes are useful to upstream tweens. This actually still - # apparently causes a reference cycle, but it is broken - # successfully by the garbage collector (see - # https://github.com/Pylons/pyramid/issues/1223). - attrs['exc_info'] = sys.exc_info() - attrs['exception'] = exc - # clear old generated request.response, if any; it may - # have been mutated by the view, and its state is not - # sane (e.g. caching headers) - if 'response' in attrs: - del attrs['response'] - # we use .get instead of .__getitem__ below due to - # https://github.com/Pylons/pyramid/issues/700 - request_iface = attrs.get('request_iface', IRequest) - provides = providedBy(exc) - try: - response = _call_view( - registry, - request, - exc, - provides, - '', - view_classifier=IExceptionViewClassifier, - request_iface=request_iface.combined - ) - - # if views matched but did not pass predicates, squash the error - # and re-raise the original exception - except PredicateMismatch: - response = None - - # re-raise the original exception as no exception views were - # able to handle the error - if response is None: - reraise(*attrs['exc_info']) - + response = _error_handler(request, exc) return response return excview_tween -- cgit v1.2.3 From 3213e20f58d1a0b339e9d5bf9378ec54593624c7 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 3 May 2017 14:05:19 -0500 Subject: add changelog for #3029 --- CHANGES.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 2378ec883..b299ed6e9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -114,6 +114,23 @@ Deprecations See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 +Backward Incompatibilities +-------------------------- + +- ``request.exception`` and ``request.exc_info`` will only be set if the + response was generated by the EXCVIEW tween. This is to avoid any confusion + where a response was generated elsewhere in the pipeline and not in + direct relation to the original exception. If anyone upstream wants to + catch and render responses for exceptions they should set + ``request.exception`` and ``request.exc_info`` themselves to indicate + the exception that was squashed when generating the response. + + This is a very minor incompatibility. Most tweens right now would give + priority to the raised exception and ignore ``request.exception``. This + change just improves and clarifies that bookkeeping by trying to be + more clear about the relationship between the response and its squashed + exception. See https://github.com/Pylons/pyramid/pull/3029 + Documentation Changes --------------------- -- cgit v1.2.3 From 3b886e6f39b1c89e78566bce2edacef9bee6d177 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Wed, 3 May 2017 22:59:52 -0500 Subject: normalize excview tween to use ``request.invoke_exception_view`` ``request.exception`` and ``request.exc_info`` are set to the exception used to render the response but they are reset to their original values if no response could be rendered minor incompatibility in that ``request.response`` is restored after the excview tween but should not be an issue because a response is returned thus request.response should be ignored by anyone who cares. --- pyramid/tests/test_view.py | 6 +++--- pyramid/tweens.py | 47 +++++----------------------------------------- pyramid/view.py | 21 ++++++++++++++++----- 3 files changed, 24 insertions(+), 50 deletions(-) diff --git a/pyramid/tests/test_view.py b/pyramid/tests/test_view.py index cab42cf48..2061515b3 100644 --- a/pyramid/tests/test_view.py +++ b/pyramid/tests/test_view.py @@ -778,11 +778,11 @@ class TestViewMethodsMixin(unittest.TestCase): orig_response = request.response = DummyResponse(b'foo') try: raise RuntimeError - except RuntimeError: + except RuntimeError as ex: response = request.invoke_exception_view() self.assertEqual(response.app_iter, [b'bar']) - self.assertTrue(request.exception is orig_exc) - self.assertTrue(request.exc_info is orig_exc_info) + self.assertTrue(request.exception is ex) + self.assertTrue(request.exc_info[1] is ex) self.assertTrue(request.response is orig_response) else: # pragma: no cover self.fail() diff --git a/pyramid/tweens.py b/pyramid/tweens.py index 673429b06..c9b3bd4b8 100644 --- a/pyramid/tweens.py +++ b/pyramid/tweens.py @@ -1,55 +1,18 @@ import sys from pyramid.compat import reraise -from pyramid.exceptions import PredicateMismatch -from pyramid.interfaces import ( - IExceptionViewClassifier, - IRequest, - ) - -from zope.interface import providedBy -from pyramid.view import _call_view +from pyramid.httpexceptions import HTTPNotFound def _error_handler(request, exc): # NOTE: we do not need to delete exc_info because this function # should never be in the call stack of the exception exc_info = sys.exc_info() - attrs = request.__dict__ - attrs['exc_info'] = exc_info - attrs['exception'] = exc - # clear old generated request.response, if any; it may - # have been mutated by the view, and its state is not - # sane (e.g. caching headers) - if 'response' in attrs: - del attrs['response'] - # we use .get instead of .__getitem__ below due to - # https://github.com/Pylons/pyramid/issues/700 - request_iface = attrs.get('request_iface', IRequest) - provides = providedBy(exc) try: - response = _call_view( - request.registry, - request, - exc, - provides, - '', - view_classifier=IExceptionViewClassifier, - request_iface=request_iface.combined - ) - - # if views matched but did not pass predicates then treat the - # same as not finding any matching views - except PredicateMismatch: - response = None - - # re-raise the original exception as no exception views were - # able to handle the error - if response is None: - if 'exception' in attrs: - del attrs['exception'] - if 'exc_info' in attrs: - del attrs['exc_info'] + response = request.invoke_exception_view(exc_info) + except HTTPNotFound: + # re-raise the original exception as no exception views were + # able to handle the error reraise(*exc_info) return response diff --git a/pyramid/view.py b/pyramid/view.py index 498bdde45..3ce984209 100644 --- a/pyramid/view.py +++ b/pyramid/view.py @@ -657,8 +657,14 @@ class ViewMethodsMixin(object): This method returns a :term:`response` object or raises :class:`pyramid.httpexceptions.HTTPNotFound` if a matching view cannot - be found.""" + be found. + If a response is generated then ``request.exception`` and + ``request.exc_info`` will be left at the values used to render the + response. Otherwise the previous values for ``request.exception`` and + ``request.exc_info`` will be restored. + + """ if request is None: request = self registry = getattr(request, 'registry', None) @@ -673,7 +679,7 @@ class ViewMethodsMixin(object): # clear old generated request.response, if any; it may # have been mutated by the view, and its state is not # sane (e.g. caching headers) - with hide_attrs(request, 'exception', 'exc_info', 'response'): + with hide_attrs(request, 'response', 'exc_info', 'exception'): attrs['exception'] = exc attrs['exc_info'] = exc_info # we use .get instead of .__getitem__ below due to @@ -690,6 +696,11 @@ class ViewMethodsMixin(object): secure=secure, request_iface=request_iface.combined, ) - if response is None: - raise HTTPNotFound - return response + + if response is None: + raise HTTPNotFound + + # successful response, overwrite exception/exc_info + attrs['exception'] = exc + attrs['exc_info'] = exc_info + return response -- cgit v1.2.3 From e2e51b35303e69b5028a84026837095b1bfe6f79 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 4 May 2017 00:23:18 -0500 Subject: add changelog for #3031 --- CHANGES.txt | 7 ++++++- pyramid/tweens.py | 13 ++++++++++++- pyramid/view.py | 5 +++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b299ed6e9..80b5003c1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -125,11 +125,16 @@ Backward Incompatibilities ``request.exception`` and ``request.exc_info`` themselves to indicate the exception that was squashed when generating the response. + Similar behavior occurs with ``request.invoke_exception_view`` in which + the exception properties are set to reflect the exception if a response + is successfully generated by the method. + This is a very minor incompatibility. Most tweens right now would give priority to the raised exception and ignore ``request.exception``. This change just improves and clarifies that bookkeeping by trying to be more clear about the relationship between the response and its squashed - exception. See https://github.com/Pylons/pyramid/pull/3029 + exception. See https://github.com/Pylons/pyramid/pull/3029 and + https://github.com/Pylons/pyramid/pull/3031 Documentation Changes --------------------- diff --git a/pyramid/tweens.py b/pyramid/tweens.py index c9b3bd4b8..740b6961c 100644 --- a/pyramid/tweens.py +++ b/pyramid/tweens.py @@ -21,7 +21,18 @@ def excview_tween_factory(handler, registry): """ A :term:`tween` factory which produces a tween that catches an exception raised by downstream tweens (or the main Pyramid request handler) and, if possible, converts it into a Response using an - :term:`exception view`.""" + :term:`exception view`. + + .. versionchanged:: 1.9 + The ``request.response`` will be remain unchanged even if the tween + handles an exception. Previously it was deleted after handling an + exception. + + Also, ``request.exception`` and ``request.exc_info`` are only set if + the tween handles an exception and returns a response otherwise they + are left at their original values. + + """ def excview_tween(request): try: diff --git a/pyramid/view.py b/pyramid/view.py index 3ce984209..0c1b8cd97 100644 --- a/pyramid/view.py +++ b/pyramid/view.py @@ -664,6 +664,11 @@ class ViewMethodsMixin(object): response. Otherwise the previous values for ``request.exception`` and ``request.exc_info`` will be restored. + .. versionchanged:: 1.9 + The ``request.exception`` and ``request.exc_info`` properties will + reflect the exception used to render the response where previously + they were reset to the values prior to invoking the method. + """ if request is None: request = self -- cgit v1.2.3 From ab8c57811d904377416c2786670ecf0e81d8ca33 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 4 May 2017 00:26:20 -0500 Subject: add incompatibilities to whatsnew --- docs/whatsnew-1.9.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index b1a406a74..f49258662 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -46,6 +46,29 @@ Deprecations See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 +Backward Incompatibilities +-------------------------- + +- ``request.exception`` and ``request.exc_info`` will only be set if the + response was generated by the EXCVIEW tween. This is to avoid any confusion + where a response was generated elsewhere in the pipeline and not in + direct relation to the original exception. If anyone upstream wants to + catch and render responses for exceptions they should set + ``request.exception`` and ``request.exc_info`` themselves to indicate + the exception that was squashed when generating the response. + + Similar behavior occurs with + :meth:`pyramid.request.Request.invoke_exception_view` in which + the exception properties are set to reflect the exception if a response + is successfully generated by the method. + + This is a very minor incompatibility. Most tweens right now would give + priority to the raised exception and ignore ``request.exception``. This + change just improves and clarifies that bookkeeping by trying to be + more clear about the relationship between the response and its squashed + exception. See https://github.com/Pylons/pyramid/pull/3029 and + https://github.com/Pylons/pyramid/pull/3031 + Documentation Enhancements -------------------------- -- cgit v1.2.3 From d1745247edae01ef934acf5bb206d29952a99dbf Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Thu, 4 May 2017 00:27:18 -0500 Subject: line length --- docs/whatsnew-1.9.rst | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/docs/whatsnew-1.9.rst b/docs/whatsnew-1.9.rst index f49258662..0ba29625c 100644 --- a/docs/whatsnew-1.9.rst +++ b/docs/whatsnew-1.9.rst @@ -49,25 +49,11 @@ Deprecations Backward Incompatibilities -------------------------- -- ``request.exception`` and ``request.exc_info`` will only be set if the - response was generated by the EXCVIEW tween. This is to avoid any confusion - where a response was generated elsewhere in the pipeline and not in - direct relation to the original exception. If anyone upstream wants to - catch and render responses for exceptions they should set - ``request.exception`` and ``request.exc_info`` themselves to indicate - the exception that was squashed when generating the response. - - Similar behavior occurs with - :meth:`pyramid.request.Request.invoke_exception_view` in which - the exception properties are set to reflect the exception if a response - is successfully generated by the method. - - This is a very minor incompatibility. Most tweens right now would give - priority to the raised exception and ignore ``request.exception``. This - change just improves and clarifies that bookkeeping by trying to be - more clear about the relationship between the response and its squashed - exception. See https://github.com/Pylons/pyramid/pull/3029 and - https://github.com/Pylons/pyramid/pull/3031 +- ``request.exception`` and ``request.exc_info`` will only be set if the response was generated by the EXCVIEW tween. This is to avoid any confusion where a response was generated elsewhere in the pipeline and not in direct relation to the original exception. If anyone upstream wants to catch and render responses for exceptions they should set ``request.exception`` and ``request.exc_info`` themselves to indicate the exception that was squashed when generating the response. + + Similar behavior occurs with :meth:`pyramid.request.Request.invoke_exception_view` in which the exception properties are set to reflect the exception if a response is successfully generated by the method. + + This is a very minor incompatibility. Most tweens right now would give priority to the raised exception and ignore ``request.exception``. This change just improves and clarifies that bookkeeping by trying to be more clear about the relationship between the response and its squashed exception. See https://github.com/Pylons/pyramid/pull/3029 and https://github.com/Pylons/pyramid/pull/3031 Documentation Enhancements -------------------------- -- cgit v1.2.3 From 1fc7eefc4ac9f5ea3d22d7a108cd3da1e73cbcfa Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 9 May 2017 01:41:12 -0500 Subject: fix changelog, added #3031 and #3029 to the wrong release version --- CHANGES.txt | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 80b5003c1..51a1e457d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,28 @@ +1.9a2 (2017-05-09) +================== + +Backward Incompatibilities +-------------------------- + +- ``request.exception`` and ``request.exc_info`` will only be set if the + response was generated by the EXCVIEW tween. This is to avoid any confusion + where a response was generated elsewhere in the pipeline and not in + direct relation to the original exception. If anyone upstream wants to + catch and render responses for exceptions they should set + ``request.exception`` and ``request.exc_info`` themselves to indicate + the exception that was squashed when generating the response. + + Similar behavior occurs with ``request.invoke_exception_view`` in which + the exception properties are set to reflect the exception if a response + is successfully generated by the method. + + This is a very minor incompatibility. Most tweens right now would give + priority to the raised exception and ignore ``request.exception``. This + change just improves and clarifies that bookkeeping by trying to be + more clear about the relationship between the response and its squashed + exception. See https://github.com/Pylons/pyramid/pull/3029 and + https://github.com/Pylons/pyramid/pull/3031 + 1.9a1 (2017-05-01) ================== @@ -114,28 +139,6 @@ Deprecations See https://github.com/Pylons/pyramid/pull/2854 and https://github.com/Pylons/pyramid/pull/3019 -Backward Incompatibilities --------------------------- - -- ``request.exception`` and ``request.exc_info`` will only be set if the - response was generated by the EXCVIEW tween. This is to avoid any confusion - where a response was generated elsewhere in the pipeline and not in - direct relation to the original exception. If anyone upstream wants to - catch and render responses for exceptions they should set - ``request.exception`` and ``request.exc_info`` themselves to indicate - the exception that was squashed when generating the response. - - Similar behavior occurs with ``request.invoke_exception_view`` in which - the exception properties are set to reflect the exception if a response - is successfully generated by the method. - - This is a very minor incompatibility. Most tweens right now would give - priority to the raised exception and ignore ``request.exception``. This - change just improves and clarifies that bookkeeping by trying to be - more clear about the relationship between the response and its squashed - exception. See https://github.com/Pylons/pyramid/pull/3029 and - https://github.com/Pylons/pyramid/pull/3031 - Documentation Changes --------------------- -- cgit v1.2.3 From 607367ace939488f787d918c60275a90d5505dbd Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 9 May 2017 01:42:09 -0500 Subject: prep 1.9a2 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 6aac12ff8..03416efe7 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ testing_extras = tests_require + [ ] setup(name='pyramid', - version='1.9a1', + version='1.9a2', description='The Pyramid Web Framework, a Pylons project', long_description=README + '\n\n' + CHANGES, classifiers=[ -- cgit v1.2.3 From a7402ad57c6bf4803286b61fd9560d8b192826b6 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 9 May 2017 14:15:01 -0400 Subject: Pytest changed their URL structure --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index f09ae325b..0fdfa7c9a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -70,7 +70,7 @@ intersphinx_mapping = { 'plaster': ('http://docs.pylonsproject.org/projects/plaster/en/latest/', None), 'pylonswebframework': ('http://docs.pylonsproject.org/projects/pylons-webframework/en/latest/', None), 'python': ('https://docs.python.org/3', None), - 'pytest': ('http://pytest.org/latest/', None), + 'pytest': ('http://pytest.org/en/latest/', None), 'sphinx': ('http://www.sphinx-doc.org/en/latest', None), 'sqla': ('http://docs.sqlalchemy.org/en/latest', None), 'tm': ('http://docs.pylonsproject.org/projects/pyramid-tm/en/latest/', None), -- cgit v1.2.3 From 0b92dfed800117595ef00fb2847c5db9970f4cac Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Tue, 9 May 2017 12:00:33 -0700 Subject: use new TLD for pytest-cov --- docs/quick_tutorial/unit_testing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/quick_tutorial/unit_testing.rst b/docs/quick_tutorial/unit_testing.rst index 7c85d5289..002c62fde 100644 --- a/docs/quick_tutorial/unit_testing.rst +++ b/docs/quick_tutorial/unit_testing.rst @@ -29,7 +29,7 @@ broken the code. As you're writing your code, you might find this more convenient than changing to your browser constantly and clicking reload. We'll also leave discussion of `pytest-cov -`_ for another section. +`_ for another section. Objectives -- cgit v1.2.3 From f46c7944b70e2204529216655bfdfac1b72e646b Mon Sep 17 00:00:00 2001 From: Steve Piercy Date: Fri, 12 May 2017 01:18:17 -0700 Subject: use https --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 0fdfa7c9a..e63019c63 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -70,7 +70,7 @@ intersphinx_mapping = { 'plaster': ('http://docs.pylonsproject.org/projects/plaster/en/latest/', None), 'pylonswebframework': ('http://docs.pylonsproject.org/projects/pylons-webframework/en/latest/', None), 'python': ('https://docs.python.org/3', None), - 'pytest': ('http://pytest.org/en/latest/', None), + 'pytest': ('https://pytest.org/en/latest/', None), 'sphinx': ('http://www.sphinx-doc.org/en/latest', None), 'sqla': ('http://docs.sqlalchemy.org/en/latest', None), 'tm': ('http://docs.pylonsproject.org/projects/pyramid-tm/en/latest/', None), -- cgit v1.2.3