From 9e9fa9ac40bdd79fbce69f94a13d705e40f3d458 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 22 Mar 2016 01:02:05 -0500 Subject: add a csrf_view to the view pipeline supporting a require_csrf option --- docs/narr/hooks.rst | 28 ++++++++++++++-------- pyramid/config/views.py | 34 ++++++++++++++++++++++++++- pyramid/tests/test_config/test_views.py | 41 +++++++++++++++++++++++++++++++++ pyramid/tests/test_viewderivers.py | 31 +++++++++++++++++++++++++ pyramid/viewderivers.py | 15 ++++++++++++ 5 files changed, 139 insertions(+), 10 deletions(-) diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst index 2c3782387..e7db97565 100644 --- a/docs/narr/hooks.rst +++ b/docs/narr/hooks.rst @@ -1580,6 +1580,11 @@ There are several built-in view derivers that :app:`Pyramid` will automatically apply to any view. Below they are defined in order from furthest to closest to the user-defined :term:`view callable`: +``csrf_view`` + + Used to check the CSRF token provided in the request. This element is a + no-op if ``require_csrf`` is not defined. + ``secured_view`` Enforce the ``permission`` defined on the view. This element is a no-op if no @@ -1656,27 +1661,32 @@ View derivers are unique in that they have access to most of the options passed to :meth:`pyramid.config.Configurator.add_view` in order to decide what to do, and they have a chance to affect every view in the application. -Let's look at one more example which will protect views by requiring a CSRF -token unless ``disable_csrf=True`` is passed to the view: +Let's override the default CSRF checker to default to on instead of off and +only check ``POST`` requests: .. code-block:: python :linenos: from pyramid.response import Response from pyramid.session import check_csrf_token + from pyramid.viewderivers import INGRESS - def require_csrf_view(view, info): + def csrf_view(view, info): + val = info.options.get('require_csrf', True) wrapper_view = view - if not info.options.get('disable_csrf', False): - def wrapper_view(context, request): + if val: + if val is True: + val = 'csrf_token' + def csrf_view(context, request): if request.method == 'POST': - check_csrf_token(request) + check_csrf_token(request, val, raises=True) return view(context, request) + wrapper_view = csrf_view return wrapper_view - require_csrf_view.options = ('disable_csrf',) + csrf_view.options = ('require_csrf',) - config.add_view_deriver(require_csrf_view) + config.add_view_deriver(csrf_view, 'csrf_view', over='secured_view', under=INGRESS) def protected_view(request): return Response('protected') @@ -1685,7 +1695,7 @@ token unless ``disable_csrf=True`` is passed to the view: return Response('unprotected') config.add_view(protected_view, name='safe') - config.add_view(unprotected_view, name='unsafe', disable_csrf=True) + config.add_view(unprotected_view, name='unsafe', require_csrf=False) Navigating to ``/safe`` with a POST request will then fail when the call to :func:`pyramid.session.check_csrf_token` raises a diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 3f6a9080d..58fdbfd06 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -213,6 +213,7 @@ class ViewsConfiguratorMixin(object): http_cache=None, match_param=None, check_csrf=None, + require_csrf=None, **view_options): """ Add a :term:`view configuration` to the current configuration state. Arguments to ``add_view`` are broken @@ -366,6 +367,32 @@ class ViewsConfiguratorMixin(object): before returning the response from the view. This effectively disables any HTTP caching done by ``http_cache`` for that response. + require_csrf + + .. versionadded:: 1.7 + + If specified, this value should be one of ``None``, ``True``, + ``False``, or a string representing the 'check name'. If the value + is ``True`` or a string, CSRF checking will be performed. If the + value is ``False`` or ``None``, CSRF checking will not be performed. + + If the value provided is a string, that string will be used as the + '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 + check fails, the associated view will not be permitted to execute + and a :class:`pyramid.exceptions.BadCSRFToken` exception will + be raised. This exception may be caught and handled by an + :term:`exception view`. + + Note that using this feature requires a :term:`session factory` to + have been configured. + wrapper The :term:`view name` of a different :term:`view @@ -805,6 +832,8 @@ class ViewsConfiguratorMixin(object): path_info=path_info, match_param=match_param, check_csrf=check_csrf, + http_cache=http_cache, + require_csrf=require_csrf, callable=view, mapper=mapper, decorator=decorator, @@ -860,6 +889,7 @@ class ViewsConfiguratorMixin(object): decorator=decorator, mapper=mapper, http_cache=http_cache, + require_csrf=require_csrf, extra_options=ovals, ) derived_view.__discriminator__ = lambda *arg: discriminator @@ -1183,6 +1213,7 @@ class ViewsConfiguratorMixin(object): def add_default_view_derivers(self): d = pyramid.viewderivers derivers = [ + ('csrf_view', d.csrf_view), ('secured_view', d.secured_view), ('owrapped_view', d.owrapped_view), ('http_cached_view', d.http_cached_view), @@ -1284,7 +1315,7 @@ class ViewsConfiguratorMixin(object): viewname=None, accept=None, order=MAX_ORDER, phash=DEFAULT_PHASH, decorator=None, mapper=None, http_cache=None, context=None, - extra_options=None): + require_csrf=None, extra_options=None): view = self.maybe_dotted(view) mapper = self.maybe_dotted(mapper) if isinstance(renderer, string_types): @@ -1311,6 +1342,7 @@ class ViewsConfiguratorMixin(object): mapper=mapper, decorator=decorator, http_cache=http_cache, + require_csrf=require_csrf, ) if extra_options: options.update(extra_options) diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index b2513c42c..55ead55c2 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -1570,6 +1570,43 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view2) self.assertRaises(ConfigurationConflictError, config.commit) + def test_add_view_with_csrf_header(self): + from pyramid.renderers import null_renderer + def view(request): + return 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view, require_csrf=True, renderer=null_renderer) + view = self._getViewCallable(config) + request = self._makeRequest(config) + request.headers = {'X-CSRF-Token': 'foo'} + request.session = DummySession({'csrf_token': 'foo'}) + self.assertEqual(view(None, request), 'OK') + + def test_add_view_with_csrf_param(self): + from pyramid.renderers import null_renderer + def view(request): + return 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view, require_csrf='st', renderer=null_renderer) + view = self._getViewCallable(config) + request = self._makeRequest(config) + request.params = {'st': 'foo'} + request.headers = {} + request.session = DummySession({'csrf_token': 'foo'}) + self.assertEqual(view(None, request), 'OK') + + def test_add_view_with_missing_csrf_header(self): + from pyramid.exceptions import BadCSRFToken + from pyramid.renderers import null_renderer + def view(request): return 'OK' + config = self._makeOne(autocommit=True) + config.add_view(view, require_csrf=True, renderer=null_renderer) + view = self._getViewCallable(config) + request = self._makeRequest(config) + request.headers = {} + request.session = DummySession({'csrf_token': 'foo'}) + self.assertRaises(BadCSRFToken, lambda: view(None, request)) + def test_add_view_with_permission(self): from pyramid.renderers import null_renderer view1 = lambda *arg: 'OK' @@ -3233,3 +3270,7 @@ class DummyIntrospector(object): return self.getval def relate(self, a, b): self.related.append((a, b)) + +class DummySession(dict): + def get_csrf_token(self): + return self['csrf_token'] diff --git a/pyramid/tests/test_viewderivers.py b/pyramid/tests/test_viewderivers.py index 1823beb4d..0dd70b74a 100644 --- a/pyramid/tests/test_viewderivers.py +++ b/pyramid/tests/test_viewderivers.py @@ -1090,6 +1090,28 @@ class TestDeriveView(unittest.TestCase): self.assertRaises(ConfigurationError, self.config._derive_view, view, http_cache=(None,)) + def test_csrf_view_requires_header(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.session = DummySession({'csrf_token': 'foo'}) + request.headers = {'X-CSRF-Token': 'foo'} + view = self.config._derive_view(inner_view, require_csrf=True) + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_requires_param(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.session = DummySession({'csrf_token': 'foo'}) + request.params['DUMMY'] = 'foo' + view = self.config._derive_view(inner_view, require_csrf='DUMMY') + result = view(None, request) + self.assertTrue(result is response) + class TestDerivationOrder(unittest.TestCase): def setUp(self): @@ -1110,6 +1132,7 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ + 'csrf_view', 'secured_view', 'owrapped_view', 'http_cached_view', @@ -1132,6 +1155,7 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ + 'csrf_view', 'secured_view', 'owrapped_view', 'http_cached_view', @@ -1152,6 +1176,7 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ + 'csrf_view', 'secured_view', 'owrapped_view', 'http_cached_view', @@ -1173,6 +1198,7 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ + 'csrf_view', 'secured_view', 'owrapped_view', 'http_cached_view', @@ -1408,6 +1434,7 @@ class DummyRequest: self.environ = environ self.params = {} self.cookies = {} + self.headers = {} self.response = DummyResponse() class DummyLogger: @@ -1428,6 +1455,10 @@ class DummySecurityPolicy: def permits(self, context, principals, permission): return self.permitted +class DummySession(dict): + def get_csrf_token(self): + return self['csrf_token'] + def parse_httpdate(s): import datetime # cannot use %Z, must use literal GMT; Jython honors timezone diff --git a/pyramid/viewderivers.py b/pyramid/viewderivers.py index 8061e5d4a..7560fa67f 100644 --- a/pyramid/viewderivers.py +++ b/pyramid/viewderivers.py @@ -6,6 +6,7 @@ from zope.interface import ( ) from pyramid.security import NO_PERMISSION_REQUIRED +from pyramid.session import check_csrf_token from pyramid.response import Response from pyramid.interfaces import ( @@ -455,5 +456,19 @@ def decorated_view(view, info): decorated_view.options = ('decorator',) +def csrf_view(view, info): + val = info.options.get('require_csrf') + wrapped_view = view + if val: + if val is True: + val = 'csrf_token' + def csrf_view(context, request): + check_csrf_token(request, val, raises=True) + return view(context, request) + wrapped_view = csrf_view + return wrapped_view + +csrf_view.options = ('require_csrf',) + VIEW = 'VIEW' INGRESS = 'INGRESS' -- cgit v1.2.3 From 6b35eb6ca3b271e2943d37307c925c5733e082d9 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 10 Apr 2016 20:50:10 -0500 Subject: rewrite csrf checks to support a global setting to turn it on - only check csrf on POST - support "pyramid.require_default_csrf" setting - support "require_csrf=True" to fallback to the global setting to determine the token name --- docs/glossary.rst | 8 ++ docs/narr/hooks.rst | 52 ++----------- docs/narr/sessions.rst | 42 ++++++++++- pyramid/config/settings.py | 7 +- pyramid/config/views.py | 37 +++++---- pyramid/settings.py | 7 +- pyramid/tests/test_config/test_views.py | 17 +++-- pyramid/tests/test_viewderivers.py | 129 +++++++++++++++++++++++++++++++- pyramid/viewderivers.py | 34 ++++++++- 9 files changed, 246 insertions(+), 87 deletions(-) diff --git a/docs/glossary.rst b/docs/glossary.rst index 039665926..ef9c66b99 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -1098,3 +1098,11 @@ Glossary implementing the :class:`pyramid.interfaces.IViewDeriver` interface. Examples of built-in derivers including view mapper, the permission checker, and applying a renderer to a dictionary returned from the view. + + truthy string + A string represeting a value of ``True``. Acceptable values are + ``t``, ``true``, ``y``, ``yes``, ``on`` and ``1``. + + falsey string + A string represeting a value of ``False``. Acceptable values are + ``f``, ``false``, ``n``, ``no``, ``off`` and ``0``. diff --git a/docs/narr/hooks.rst b/docs/narr/hooks.rst index e7db97565..28d1e09d5 100644 --- a/docs/narr/hooks.rst +++ b/docs/narr/hooks.rst @@ -1580,11 +1580,6 @@ There are several built-in view derivers that :app:`Pyramid` will automatically apply to any view. Below they are defined in order from furthest to closest to the user-defined :term:`view callable`: -``csrf_view`` - - Used to check the CSRF token provided in the request. This element is a - no-op if ``require_csrf`` is not defined. - ``secured_view`` Enforce the ``permission`` defined on the view. This element is a no-op if no @@ -1595,6 +1590,12 @@ the user-defined :term:`view callable`: This element will also output useful debugging information when ``pyramid.debug_authorization`` is enabled. +``csrf_view`` + + Used to check the CSRF token provided in the request. This element is a + no-op if both the ``require_csrf`` view option and the + ``pyramid.require_default_csrf`` setting are disabled. + ``owrapped_view`` Invokes the wrapped view defined by the ``wrapper`` option. @@ -1661,47 +1662,6 @@ View derivers are unique in that they have access to most of the options passed to :meth:`pyramid.config.Configurator.add_view` in order to decide what to do, and they have a chance to affect every view in the application. -Let's override the default CSRF checker to default to on instead of off and -only check ``POST`` requests: - -.. code-block:: python - :linenos: - - from pyramid.response import Response - from pyramid.session import check_csrf_token - from pyramid.viewderivers import INGRESS - - def csrf_view(view, info): - val = info.options.get('require_csrf', True) - wrapper_view = view - if val: - if val is True: - val = 'csrf_token' - def csrf_view(context, request): - if request.method == 'POST': - check_csrf_token(request, val, raises=True) - return view(context, request) - wrapper_view = csrf_view - return wrapper_view - - csrf_view.options = ('require_csrf',) - - config.add_view_deriver(csrf_view, 'csrf_view', over='secured_view', under=INGRESS) - - def protected_view(request): - return Response('protected') - - def unprotected_view(request): - return Response('unprotected') - - config.add_view(protected_view, name='safe') - config.add_view(unprotected_view, name='unsafe', require_csrf=False) - -Navigating to ``/safe`` with a POST request will then fail when the call to -:func:`pyramid.session.check_csrf_token` raises a -:class:`pyramid.exceptions.BadCSRFToken` exception. However, ``/unsafe`` will -not error. - Ordering View Derivers ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index db554a93b..3baed1cb8 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -389,8 +389,43 @@ header named ``X-CSRF-Token``. # ... -.. index:: - single: session.new_csrf_token +.. _auto_csrf_checking: + +Checking CSRF Tokens Automatically +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: 1.7 + +:app:`Pyramid` supports automatically checking CSRF tokens on POST requests. +Any other request may be checked manually. This feature can be turned on +globally for an application using the ``pyramid.require_default_csrf`` setting. + +If the ``pyramid.required_default_csrf`` setting is a :term:`truthy string` or +``True`` then the default CSRF token parameter will be ``csrf_token``. If a +different token is desired, it may be passed as the value. Finally, a +:term:`falsey string` or ``False`` will turn off automatic CSRF checking +globally on every POST request. + +No matter what, CSRF checking may be explicitly enabled or disabled on a +per-view basis using the ``require_csrf`` view option. This option is of the +same format as the ``pyramid.require_default_csrf`` setting, accepting strings +or boolean values. + +If ``require_csrf`` is ``True`` but does not explicitly define a token to +check, then the token name is pulled from whatever was set in the +``pyramid.require_default_csrf`` setting. Finally, if that setting does not +explicitly define a token, then ``csrf_token`` is the token required. This token +name will be required in ``request.params`` which is a combination of the +query string and a submitted form body. + +It is always possible to pass the token in the ``X-CSRF-Token`` header as well. +There is currently no way to define an alternate name for this header without +performing CSRF checking manually. + +If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` 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`` +resposne being sent to the client. Checking CSRF Tokens with a View Predicate ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -411,6 +446,9 @@ include ``check_csrf=True`` as a view predicate. See instead of ``HTTPBadRequest``, so ``check_csrf=True`` behavior is different from calling :func:`pyramid.session.check_csrf_token`. +.. index:: + single: session.new_csrf_token + Using the ``session.new_csrf_token`` Method ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pyramid/config/settings.py b/pyramid/config/settings.py index 492b7d524..78b61e4ef 100644 --- a/pyramid/config/settings.py +++ b/pyramid/config/settings.py @@ -6,6 +6,7 @@ from zope.interface import implementer from pyramid.interfaces import ISettings from pyramid.settings import asbool +from pyramid.settings import truthy class SettingsConfiguratorMixin(object): def _set_settings(self, mapping): @@ -122,6 +123,8 @@ class Settings(dict): config_prevent_cachebust) eff_prevent_cachebust = asbool(eget('PYRAMID_PREVENT_CACHEBUST', config_prevent_cachebust)) + require_default_csrf = self.get('pyramid.require_default_csrf') + eff_require_default_csrf = require_default_csrf update = { 'debug_authorization': eff_debug_all or eff_debug_auth, @@ -134,6 +137,7 @@ class Settings(dict): 'default_locale_name':eff_locale_name, 'prevent_http_cache':eff_prevent_http_cache, 'prevent_cachebust':eff_prevent_cachebust, + 'require_default_csrf':eff_require_default_csrf, 'pyramid.debug_authorization': eff_debug_all or eff_debug_auth, 'pyramid.debug_notfound': eff_debug_all or eff_debug_notfound, @@ -145,7 +149,8 @@ class Settings(dict): 'pyramid.default_locale_name':eff_locale_name, 'pyramid.prevent_http_cache':eff_prevent_http_cache, 'pyramid.prevent_cachebust':eff_prevent_cachebust, - } + 'pyramid.require_default_csrf':eff_require_default_csrf, + } self.update(update) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 58fdbfd06..8b066bc1e 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -371,27 +371,26 @@ class ViewsConfiguratorMixin(object): .. versionadded:: 1.7 - If specified, this value should be one of ``None``, ``True``, - ``False``, or a string representing the 'check name'. If the value - is ``True`` or a string, CSRF checking will be performed. If the - value is ``False`` or ``None``, CSRF checking will not be performed. + CSRF checks only affect POST requests. Any other request methods + will pass untouched. This option is used in combination with the + ``pyramid.require_default_csrf`` setting to control which + request parameters are checked for CSRF tokens. - If the value provided is a string, that string will be used as the - 'check name'. If the value provided is ``True``, ``csrf_token`` will - be used as the check name. + This feature requires a configured :term:`session factory`. - 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 - check fails, the associated view will not be permitted to execute - and a :class:`pyramid.exceptions.BadCSRFToken` exception will - be raised. This exception may be caught and handled by an - :term:`exception view`. + If this option is set to ``True`` then CSRF checks will be enabled + for POST requests to this view. The required token will be whatever + was specified by the ``pyramid.require_default_csrf`` setting, or + will fallback to ``csrf_token``. - Note that using this feature requires a :term:`session factory` to - have been configured. + If this option is set to a string then CSRF checks will be enabled + and it will be used as the required token regardless of the + ``pyramid.require_default_csrf`` setting. + + If this option is set to ``False`` then CSRF checks will be disabled + regardless of the ``pyramid.require_default_csrf`` setting. + + See :ref:`auto_csrf_checking` for more information. wrapper @@ -1213,8 +1212,8 @@ class ViewsConfiguratorMixin(object): def add_default_view_derivers(self): d = pyramid.viewderivers derivers = [ - ('csrf_view', d.csrf_view), ('secured_view', d.secured_view), + ('csrf_view', d.csrf_view), ('owrapped_view', d.owrapped_view), ('http_cached_view', d.http_cached_view), ('decorated_view', d.decorated_view), diff --git a/pyramid/settings.py b/pyramid/settings.py index e2cb3cb3c..8a498d572 100644 --- a/pyramid/settings.py +++ b/pyramid/settings.py @@ -1,13 +1,12 @@ from pyramid.compat import string_types truthy = frozenset(('t', 'true', 'y', 'yes', 'on', '1')) +falsey = frozenset(('f', 'false', 'n', 'no', 'off', '0')) def asbool(s): """ Return the boolean value ``True`` if the case-lowered value of string - input ``s`` is any of ``t``, ``true``, ``y``, ``on``, or ``1``, otherwise - return the boolean value ``False``. If ``s`` is the value ``None``, - return ``False``. If ``s`` is already one of the boolean values ``True`` - or ``False``, return it.""" + input ``s`` is a :term:`truthy string`. If ``s`` is already one of the + boolean values ``True`` or ``False``, return it.""" if s is None: return False if isinstance(s, bool): diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 55ead55c2..f3c51f985 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -1570,28 +1570,30 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view2) self.assertRaises(ConfigurationConflictError, config.commit) - def test_add_view_with_csrf_header(self): + def test_add_view_with_csrf_param(self): from pyramid.renderers import null_renderer def view(request): return 'OK' config = self._makeOne(autocommit=True) - config.add_view(view, require_csrf=True, renderer=null_renderer) + config.add_view(view, require_csrf='st', renderer=null_renderer) view = self._getViewCallable(config) request = self._makeRequest(config) - request.headers = {'X-CSRF-Token': 'foo'} + request.method = 'POST' + request.params = {'st': 'foo'} + request.headers = {} request.session = DummySession({'csrf_token': 'foo'}) self.assertEqual(view(None, request), 'OK') - def test_add_view_with_csrf_param(self): + def test_add_view_with_csrf_header(self): from pyramid.renderers import null_renderer def view(request): return 'OK' config = self._makeOne(autocommit=True) - config.add_view(view, require_csrf='st', renderer=null_renderer) + config.add_view(view, require_csrf=True, renderer=null_renderer) view = self._getViewCallable(config) request = self._makeRequest(config) - request.params = {'st': 'foo'} - request.headers = {} + request.method = 'POST' + request.headers = {'X-CSRF-Token': 'foo'} request.session = DummySession({'csrf_token': 'foo'}) self.assertEqual(view(None, request), 'OK') @@ -1603,6 +1605,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view, require_csrf=True, renderer=null_renderer) view = self._getViewCallable(config) request = self._makeRequest(config) + request.method = 'POST' request.headers = {} request.session = DummySession({'csrf_token': 'foo'}) self.assertRaises(BadCSRFToken, lambda: view(None, request)) diff --git a/pyramid/tests/test_viewderivers.py b/pyramid/tests/test_viewderivers.py index 0dd70b74a..c8fbe6f36 100644 --- a/pyramid/tests/test_viewderivers.py +++ b/pyramid/tests/test_viewderivers.py @@ -1090,11 +1090,36 @@ class TestDeriveView(unittest.TestCase): self.assertRaises(ConfigurationError, self.config._derive_view, view, http_cache=(None,)) + def test_csrf_view_requires_bool_or_str_in_require_csrf(self): + def view(request): pass + try: + self.config._derive_view(view, require_csrf=object()) + except ConfigurationError as ex: + self.assertEqual( + 'View option "require_csrf" must be a string or boolean value', + ex.args[0]) + else: # pragma: no cover + raise AssertionError + + def test_csrf_view_requires_bool_or_str_in_config_setting(self): + def view(request): pass + self.config.add_settings({'pyramid.require_default_csrf': object()}) + try: + self.config._derive_view(view) + except ConfigurationError as ex: + self.assertEqual( + 'Config setting "pyramid.require_csrf_default" must be a ' + 'string or boolean value', + ex.args[0]) + else: # pragma: no cover + raise AssertionError + def test_csrf_view_requires_header(self): response = DummyResponse() def inner_view(request): return response request = self._makeRequest() + request.method = 'POST' request.session = DummySession({'csrf_token': 'foo'}) request.headers = {'X-CSRF-Token': 'foo'} view = self.config._derive_view(inner_view, require_csrf=True) @@ -1106,12 +1131,108 @@ class TestDeriveView(unittest.TestCase): def inner_view(request): return response request = self._makeRequest() + request.method = 'POST' request.session = DummySession({'csrf_token': 'foo'}) request.params['DUMMY'] = 'foo' view = self.config._derive_view(inner_view, require_csrf='DUMMY') result = view(None, request) self.assertTrue(result is response) + def test_csrf_view_ignores_GET(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'GET' + view = self.config._derive_view(inner_view, require_csrf=True) + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_fails_on_bad_POST_param(self): + from pyramid.exceptions import BadCSRFToken + def inner_view(request): pass + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['DUMMY'] = 'bar' + view = self.config._derive_view(inner_view, require_csrf='DUMMY') + self.assertRaises(BadCSRFToken, lambda: view(None, request)) + + def test_csrf_view_fails_on_bad_POST_header(self): + from pyramid.exceptions import BadCSRFToken + def inner_view(request): pass + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.headers = {'X-CSRF-Token': 'bar'} + view = self.config._derive_view(inner_view, require_csrf='DUMMY') + self.assertRaises(BadCSRFToken, lambda: view(None, request)) + + def test_csrf_view_uses_config_setting_truthy(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['csrf_token'] = 'foo' + self.config.add_settings({'pyramid.require_default_csrf': 'yes'}) + view = self.config._derive_view(inner_view) + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_uses_config_setting_with_custom_token(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['DUMMY'] = 'foo' + self.config.add_settings({'pyramid.require_default_csrf': 'DUMMY'}) + view = self.config._derive_view(inner_view) + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_uses_config_setting_falsey(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['csrf_token'] = 'foo' + self.config.add_settings({'pyramid.require_default_csrf': 'no'}) + view = self.config._derive_view(inner_view) + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_uses_view_option_override(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['DUMMY'] = 'foo' + self.config.add_settings({'pyramid.require_default_csrf': 'yes'}) + view = self.config._derive_view(inner_view, require_csrf='DUMMY') + result = view(None, request) + self.assertTrue(result is response) + + def test_csrf_view_uses_config_setting_when_view_option_is_true(self): + response = DummyResponse() + def inner_view(request): + return response + request = self._makeRequest() + request.method = 'POST' + request.session = DummySession({'csrf_token': 'foo'}) + request.params['DUMMY'] = 'foo' + self.config.add_settings({'pyramid.require_default_csrf': 'DUMMY'}) + view = self.config._derive_view(inner_view, require_csrf=True) + result = view(None, request) + self.assertTrue(result is response) + class TestDerivationOrder(unittest.TestCase): def setUp(self): @@ -1132,8 +1253,8 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ - 'csrf_view', 'secured_view', + 'csrf_view', 'owrapped_view', 'http_cached_view', 'decorated_view', @@ -1155,8 +1276,8 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ - 'csrf_view', 'secured_view', + 'csrf_view', 'owrapped_view', 'http_cached_view', 'decorated_view', @@ -1176,8 +1297,8 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ - 'csrf_view', 'secured_view', + 'csrf_view', 'owrapped_view', 'http_cached_view', 'decorated_view', @@ -1198,8 +1319,8 @@ class TestDerivationOrder(unittest.TestCase): derivers_sorted = derivers.sorted() dlist = [d for (d, _) in derivers_sorted] self.assertEqual([ - 'csrf_view', 'secured_view', + 'csrf_view', 'owrapped_view', 'http_cached_view', 'decorated_view', diff --git a/pyramid/viewderivers.py b/pyramid/viewderivers.py index 7560fa67f..41102319d 100644 --- a/pyramid/viewderivers.py +++ b/pyramid/viewderivers.py @@ -19,6 +19,7 @@ from pyramid.interfaces import ( ) from pyramid.compat import ( + string_types, is_bound_method, is_unbound_method, ) @@ -34,6 +35,10 @@ from pyramid.exceptions import ( PredicateMismatch, ) from pyramid.httpexceptions import HTTPForbidden +from pyramid.settings import ( + falsey, + truthy, +) from pyramid.util import object_description from pyramid.view import render_view_to_response from pyramid import renderers @@ -456,14 +461,35 @@ def decorated_view(view, info): decorated_view.options = ('decorator',) +def _parse_csrf_setting(val, error_source): + if val: + if isinstance(val, string_types): + if val.lower() in truthy: + val = True + elif val.lower() in falsey: + val = False + elif not isinstance(val, bool): + raise ConfigurationError( + '{0} must be a string or boolean value' + .format(error_source)) + return val + def csrf_view(view, info): - val = info.options.get('require_csrf') + default_val = _parse_csrf_setting( + info.settings.get('pyramid.require_default_csrf'), + 'Config setting "pyramid.require_csrf_default"') + val = _parse_csrf_setting( + info.options.get('require_csrf'), + 'View option "require_csrf"') + if (val is True and default_val) or val is None: + val = default_val + if val is True: + val = 'csrf_token' wrapped_view = view if val: - if val is True: - val = 'csrf_token' def csrf_view(context, request): - check_csrf_token(request, val, raises=True) + if request.method == 'POST': + check_csrf_token(request, val, raises=True) return view(context, request) wrapped_view = csrf_view return wrapped_view -- cgit v1.2.3 From 15b97dc81c8bcdc039f8f2293f85812f68a076da Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 10 Apr 2016 20:51:23 -0500 Subject: deprecate the check_csrf predicate --- docs/narr/sessions.rst | 4 ++++ pyramid/config/views.py | 18 +++++++++++++++++- pyramid/tests/test_config/test_views.py | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 3baed1cb8..4e8f6db88 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -430,6 +430,10 @@ resposne 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`. diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 8b066bc1e..6fe31fd4a 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -613,6 +613,11 @@ class ViewsConfiguratorMixin(object): check_csrf + .. deprecated:: 1.7 + Use the ``require_csrf`` option or see :ref:`auto_csrf_checking` + instead to have :class:`pyramid.exceptions.BadCSRFToken` + exceptions raised. + If specified, this value should be one of ``None``, ``True``, ``False``, or a string representing the 'check name'. If the value is ``True`` or a string, CSRF checking will be performed. If the @@ -708,7 +713,18 @@ class ViewsConfiguratorMixin(object): 'Predicate" in the "Hooks" chapter of the documentation ' 'for more information.'), DeprecationWarning, - stacklevel=4 + stacklevel=4, + ) + + if check_csrf is not None: + warnings.warn( + ('The "check_csrf" argument to Configurator.add_view is ' + 'deprecated as of Pyramid 1.7. Use the "require_csrf" option ' + 'instead or see "Checking CSRF Tokens Automatically" in the ' + '"Sessions" chapter of the documentation for more ' + 'information.'), + DeprecationWarning, + stacklevel=4, ) view = self.maybe_dotted(view) diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index f3c51f985..0bf0bd0b3 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -1491,6 +1491,22 @@ class TestViewsConfigurationMixin(unittest.TestCase): request.upath_info = text_('/') self._assertNotFound(wrapper, None, request) + def test_add_view_with_check_csrf_predicates_match(self): + import warnings + from pyramid.renderers import null_renderer + view = lambda *arg: 'OK' + config = self._makeOne(autocommit=True) + with warnings.catch_warnings(record=True) as w: + warnings.filterwarnings('always') + config.add_view(view=view, check_csrf=True, renderer=null_renderer) + self.assertEqual(len(w), 1) + wrapper = self._getViewCallable(config) + request = self._makeRequest(config) + request.session = DummySession({'csrf_token': 'foo'}) + request.params = {'csrf_token': 'foo'} + request.headers = {} + self.assertEqual(wrapper(None, request), 'OK') + def test_add_view_with_custom_predicates_match(self): import warnings from pyramid.renderers import null_renderer -- cgit v1.2.3 From 769da1215a0287f4161e58f36d8d4b7650154202 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 10 Apr 2016 21:14:22 -0500 Subject: cleanup some references in the docs --- docs/narr/sessions.rst | 32 ++++++++++++++++---------------- docs/narr/viewconfig.rst | 26 ++++++++++++++++++++++++++ pyramid/config/settings.py | 1 - pyramid/session.py | 3 +++ pyramid/view.py | 3 ++- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 4e8f6db88..d66e86258 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -367,6 +367,21 @@ Or include it as a header in a jQuery AJAX request: 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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -425,7 +440,7 @@ performing CSRF checking manually. If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` 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`` -resposne being sent to the client. +response being sent to the client. Checking CSRF Tokens with a View Predicate ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -449,18 +464,3 @@ include ``check_csrf=True`` as a view predicate. See 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`. - -.. 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() diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 0bd52b6e2..e645185f5 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -192,6 +192,32 @@ Non-Predicate Arguments only influence ``Cache-Control`` headers, pass a tuple as ``http_cache`` with the first element of ``None``, i.e., ``(None, {'public':True})``. + +``require_csrf`` + + CSRF checks only affect POST requests. Any other request methods will pass + untouched. This option is used in combination with the + ``pyramid.require_default_csrf`` setting to control which request parameters + are checked for CSRF tokens. + + This feature requires a configured :term:`session factory`. + + If this option is set to ``True`` then CSRF checks will be enabled for POST + requests to this view. The required token will be whatever was specified by + the ``pyramid.require_default_csrf`` setting, or will fallback to + ``csrf_token``. + + If this option is set to a string then CSRF checks will be enabled and it + will be used as the required token regardless of the + ``pyramid.require_default_csrf`` setting. + + If this option is set to ``False`` then CSRF checks will be disabled + regardless of the ``pyramid.require_default_csrf`` setting. + + See :ref:`auto_csrf_checking` for more information. + + .. versionadded:: 1.7 + ``wrapper`` The :term:`view name` of a different :term:`view configuration` which will receive the response body of this view as the ``request.wrapped_body`` diff --git a/pyramid/config/settings.py b/pyramid/config/settings.py index 78b61e4ef..b66986327 100644 --- a/pyramid/config/settings.py +++ b/pyramid/config/settings.py @@ -6,7 +6,6 @@ from zope.interface import implementer from pyramid.interfaces import ISettings from pyramid.settings import asbool -from pyramid.settings import truthy class SettingsConfiguratorMixin(object): def _set_settings(self, mapping): diff --git a/pyramid/session.py b/pyramid/session.py index a4cdf910d..fd7b5f8d5 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -123,6 +123,9 @@ def check_csrf_token(request, 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 """ supplied_token = request.params.get(token, request.headers.get(header, "")) diff --git a/pyramid/view.py b/pyramid/view.py index 0129526ce..62ac5310e 100644 --- a/pyramid/view.py +++ b/pyramid/view.py @@ -169,7 +169,8 @@ class view_config(object): ``request_type``, ``route_name``, ``request_method``, ``request_param``, ``containment``, ``xhr``, ``accept``, ``header``, ``path_info``, ``custom_predicates``, ``decorator``, ``mapper``, ``http_cache``, - ``match_param``, ``check_csrf``, ``physical_path``, and ``predicates``. + ``require_csrf``, ``match_param``, ``check_csrf``, ``physical_path``, and + ``view_options``. The meanings of these arguments are the same as the arguments passed to :meth:`pyramid.config.Configurator.add_view`. If any argument is left -- cgit v1.2.3