Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove check_csrf view predicate #3521

Merged
merged 1 commit into from Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.rst
Expand Up @@ -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
---------------------

Expand Down
23 changes: 0 additions & 23 deletions docs/narr/security.rst
Expand Up @@ -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`.
22 changes: 0 additions & 22 deletions docs/narr/viewconfig.rst
Expand Up @@ -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
Expand Down
53 changes: 4 additions & 49 deletions src/pyramid/config/views.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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),
)
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
22 changes: 0 additions & 22 deletions src/pyramid/predicates.py
Expand Up @@ -4,7 +4,6 @@

from pyramid.exceptions import ConfigurationError

from pyramid.csrf import check_csrf_token
from pyramid.traversal import (
find_interface,
traversal_path,
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/pyramid/view.py
Expand Up @@ -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
Expand Down
18 changes: 0 additions & 18 deletions tests/test_config/test_views.py
Expand Up @@ -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
Expand Down
49 changes: 0 additions & 49 deletions tests/test_predicates.py
Expand Up @@ -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
Expand Down