diff --git a/CHANGES.rst b/CHANGES.rst index e2d5dbbacd..0cf66d16fe 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -120,6 +120,11 @@ Backward Incompatibilities cases and is largely backward compatible. See https://github.com/Pylons/pyramid/pull/3514 +- Removed the ``check_csrf`` predicate. Instead, use + ``pyramid.config.Configurator.set_default_csrf_options`` and the + ``require_csrf`` view option to enable automatic CSRF checking. + See https://github.com/Pylons/pyramid/pull/3521 + Documentation Changes --------------------- diff --git a/docs/narr/security.rst b/docs/narr/security.rst index 2a7034a19e..62730e7a8a 100644 --- a/docs/narr/security.rst +++ b/docs/narr/security.rst @@ -896,26 +896,3 @@ If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or 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.csrf.check_csrf_token`. diff --git a/docs/narr/viewconfig.rst b/docs/narr/viewconfig.rst index 465477b4d8..6a49e02a55 100644 --- a/docs/narr/viewconfig.rst +++ b/docs/narr/viewconfig.rst @@ -479,28 +479,6 @@ configured view. consideration when deciding whether or not to invoke the associated view callable. -``check_csrf`` - 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.POST[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. - - Note that using this feature requires a :term:`session factory` to have been - configured. - - .. versionadded:: 1.4a2 - ``physical_path`` If specified, this value should be a string or a tuple representing the :term:`physical path` of the context found via traversal for this predicate diff --git a/src/pyramid/config/views.py b/src/pyramid/config/views.py index 1abff05794..afb685f930 100644 --- a/src/pyramid/config/views.py +++ b/src/pyramid/config/views.py @@ -276,7 +276,6 @@ def add_view( mapper=None, http_cache=None, match_param=None, - check_csrf=None, require_csrf=None, exception_only=False, **view_options @@ -709,38 +708,6 @@ def wrapper(context, request): variable. If the regex matches, this predicate will be ``True``. - 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 - 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 ``policy.get_csrf_token()`` (where ``policy`` is an - 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. - - .. 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 @@ -804,6 +771,10 @@ def wrapper(context, request): Support setting view deriver options. Previously, only custom view predicate values could be supplied. + .. versionchanged:: 2.0 + + Removed support for the ``check_csrf`` predicate. + """ if custom_predicates: warnings.warn( @@ -820,19 +791,6 @@ def wrapper(context, request): 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, - ) - if accept is not None: if is_nonstr_iter(accept): raise ConfigurationError( @@ -903,7 +861,6 @@ def view(context, request): containment=containment, request_type=request_type, match_param=match_param, - check_csrf=check_csrf, custom=predvalseq(custom_predicates), ) ) @@ -963,7 +920,6 @@ def discrim_func(): header=header, path_info=path_info, match_param=match_param, - check_csrf=check_csrf, http_cache=http_cache, require_csrf=require_csrf, callable=view, @@ -1249,7 +1205,6 @@ def add_default_view_predicates(self): ('containment', p.ContainmentPredicate), ('request_type', p.RequestTypePredicate), ('match_param', p.MatchParamPredicate), - ('check_csrf', p.CheckCSRFTokenPredicate), ('physical_path', p.PhysicalPathPredicate), ('effective_principals', p.EffectivePrincipalsPredicate), ('custom', p.CustomPredicate), diff --git a/src/pyramid/predicates.py b/src/pyramid/predicates.py index a267a69a0f..a099332536 100644 --- a/src/pyramid/predicates.py +++ b/src/pyramid/predicates.py @@ -4,7 +4,6 @@ from pyramid.exceptions import ConfigurationError -from pyramid.csrf import check_csrf_token from pyramid.traversal import ( find_interface, traversal_path, @@ -252,27 +251,6 @@ def __call__(self, context, request): return True -class CheckCSRFTokenPredicate(object): - - check_csrf_token = staticmethod(check_csrf_token) # testing - - def __init__(self, val, config): - self.val = val - - def text(self): - return 'check_csrf = %s' % (self.val,) - - phash = text - - def __call__(self, context, request): - val = self.val - if val: - if val is True: - val = 'csrf_token' - return self.check_csrf_token(request, val, raises=False) - return True - - class PhysicalPathPredicate(object): def __init__(self, val, config): if is_nonstr_iter(val): diff --git a/src/pyramid/view.py b/src/pyramid/view.py index 7e54a40f61..eeac4e783c 100644 --- a/src/pyramid/view.py +++ b/src/pyramid/view.py @@ -174,7 +174,7 @@ def my_view(context, request): ``request_type``, ``route_name``, ``request_method``, ``request_param``, ``containment``, ``xhr``, ``accept``, ``header``, ``path_info``, ``custom_predicates``, ``decorator``, ``mapper``, ``http_cache``, - ``require_csrf``, ``match_param``, ``check_csrf``, ``physical_path``, and + ``require_csrf``, ``match_param``, ``physical_path``, and ``view_options``. The meanings of these arguments are the same as the arguments passed to diff --git a/tests/test_config/test_views.py b/tests/test_config/test_views.py index 28b7a9fb14..a1e9757560 100644 --- a/tests/test_config/test_views.py +++ b/tests/test_config/test_views.py @@ -1886,24 +1886,6 @@ def test_add_view_with_path_info_nomatch(self): 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.method = "POST" - request.session = DummySession({'csrf_token': 'foo'}) - request.POST = {'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 diff --git a/tests/test_predicates.py b/tests/test_predicates.py index 60e36047eb..4029faf9d2 100644 --- a/tests/test_predicates.py +++ b/tests/test_predicates.py @@ -315,55 +315,6 @@ def test_phash(self): self.assertEqual(inst.phash(), '') -class Test_CheckCSRFTokenPredicate(unittest.TestCase): - def _makeOne(self, val, config): - from pyramid.predicates import CheckCSRFTokenPredicate - - return CheckCSRFTokenPredicate(val, config) - - def test_text(self): - inst = self._makeOne(True, None) - self.assertEqual(inst.text(), 'check_csrf = True') - - def test_phash(self): - inst = self._makeOne(True, None) - self.assertEqual(inst.phash(), 'check_csrf = True') - - def test_it_call_val_True(self): - inst = self._makeOne(True, None) - request = Dummy() - - def check_csrf_token(req, val, raises=True): - self.assertEqual(req, request) - self.assertEqual(val, 'csrf_token') - self.assertEqual(raises, False) - return True - - inst.check_csrf_token = check_csrf_token - result = inst(None, request) - self.assertEqual(result, True) - - def test_it_call_val_str(self): - inst = self._makeOne('abc', None) - request = Dummy() - - def check_csrf_token(req, val, raises=True): - self.assertEqual(req, request) - self.assertEqual(val, 'abc') - self.assertEqual(raises, False) - return True - - inst.check_csrf_token = check_csrf_token - result = inst(None, request) - self.assertEqual(result, True) - - def test_it_call_val_False(self): - inst = self._makeOne(False, None) - request = Dummy() - result = inst(None, request) - self.assertEqual(result, True) - - class TestHeaderPredicate(unittest.TestCase): def _makeOne(self, val): from pyramid.predicates import HeaderPredicate