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

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