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

check_csrf=True attribute view_config on invalid csrf token causes 500 Internal Server Error instead of 400 HTTPBadRequest Error #1532

Closed
ashayshub opened this issue Jan 20, 2015 · 13 comments

Comments

@ashayshub
Copy link

Platform I am using:

Mac Air, Yosemite 10.10.1
Python 3.3.3
Babel 1.3
Beaker 1.6.4
beautifulsoup4 4.3.2
Chameleon 2.19
Cython 0.21.2
lingua 3.7
Mako 1.0.0
MarkupSafe 0.23
oursql 0.9.4
passlib 1.6.2
PasteDeploy 1.5.2
pip 1.5.2
polib 1.0.6
pycrypto 2.6.1
Pygments 2.0.1
pymongo 2.7.2
pyramid 1.5.2
pyramid-beaker 0.8
pyramid-chameleon 0.3
pyramid-debugtoolbar 2.3
pyramid-mako 1.0.2
pyramid-tm 0.10
pytz 2014.10
repoze.lru 0.6
requests 2.5.1
setuptools 2.1
six 1.9.0
SQLAlchemy 0.9.8
SQLAlchemy-Utils 0.29.1
teacup 0.0
transaction 1.4.3
translationstring 1.3
venusian 1.0
waitress 0.8.9
WebOb 1.4
WebTest 2.0.17
zope.deprecation 4.1.1
zope.interface 4.1.2

zope.sqlalchemy 0.7.5

How to reproduce:

I use a class based view_config with check_csrf=True enabled, anticipating a 403 forbidden. I use pyramid_beaker file based session. On correct csrf token everything seems to work fine. But on an invalid csrf token it gives Internal Server Error. Below is the view configuration I use on pastebin (edited).

http://pastebin.com/WsEfTbd6


Error:

Below is the edited output of the 500 error that I get where in I should receive a 403 forbidden on the client side (edited).

http://pastebin.com/k6vy917c

@ashayshub
Copy link
Author

In case the pastebin code expires, here is the above pastebin code/errors under markdown:

Class based view configuration I use (edited):

import logging
from pyramid.view import view_config

_logger = logging

class Search(object):
    """Login View"""

    def __init__(self, request):
        """Init function """
        self.request = request
        self.session = self.request.session

    @view_config(route_name='Search', renderer='json', request_method='GET', check_csrf=True)
    def my_search(self):
        """
        :type request: pyramid.request.Request
        :return: Response object
        """
        json_resp = list()
        for i in range(10):
            a = {
                "rid": i+1,
            }
            json_resp.append(a)

        return json_resp

Here is the Error traceback that I received:

pydev debugger: process 66272 is connecting

Connected to pydev debugger (build 139.1001)
Starting server in PID 66272.
serving on http://0.0.0.0:6543
2015-01-20 11:25:12,183 ERROR [pyramid_debugtoolbar][Dummy-6] Exception at http://localhost:6543/a/search/XYZ/PQR/
traceback url: http://localhost:6543/_debug_toolbar/exception?token=62275c7831645c7864665c7839665c7863622977355c7864635c7838305c78666627&tb=4420701072
Traceback (most recent call last):
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_debugtoolbar-2.3-py3.3.egg/pyramid_debugtoolbar/toolbar.py", line 178, in toolbar_tween
    response = _handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_debugtoolbar-2.3-py3.3.egg/pyramid_debugtoolbar/panels/performance.py", line 57, in resource_timer_handler
    result = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/tweens.py", line 21, in excview_tween
    response = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/__init__.py", line 95, in tm_tween
    reraise(*exc_info)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/compat.py", line 15, in reraise
    raise value
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/__init__.py", line 76, in tm_tween
    response = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/router.py", line 163, in handle_request
    response = view_callable(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 596, in __call__
    return view(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 385, in viewresult_to_response
    result = view(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 475, in _class_requestonly_view
    response = inst()
TypeError: 'Search' object is not callable

@mmerickel
Copy link
Member

Unfortunately I cannot see anything wrong with the code you've pasted so far. Do you think you can make a complete example I can run that demonstrates the error? The check_csrf predicate should simply cause the view to not be found, which will invoke a 404 (not a 403 or a 500). If you'd like to raise an error then you should call the p.sessions.check_csrf function yourself.

@ashayshub
Copy link
Author

Hi mmerickel,

  1. Something is wrong in the predicate check_csrf under view_config. Everything works fine if you remove the predicate and use check_csrf_token() imported from pyramid.session. I like the predicate implementation, since it's more clean so would like it/like to fix.
  2. My mistake that I mentioned 403 on error. The ideal error that should be thrown by either of the above two functions, in case of csrf match fails, is a HTTPBadRequest , 400; Refer pyramid.exceptions.BadCSRFToken. Yet, the predicate version throws back a 500 Internal Server Error, saying Search object is not callable as something in predicate breaks. This is not acceptable behaviour.
  3. Use basic implementation of two views with Beaker Session.
    a) 1st view should implement a predicate way (check_csrf) of checking csrf.
    b) 2nd view should implement check_csrf_token() way of checking the csrf.
    You should be able to figure out the issue I am highlighting. Apart from this if you need any specific file, I will be glad to provide it to you.

@digitalresistor
Copy link
Member

@ashayshub

check_csrf under view_config() works without issues for a huge variety of projects that I use it on. We need more information to better understand why you are receiving the error.

@ashayshub
Copy link
Author

@bertjwregeer

I have already mentioned the various versions of python and its packages I am using, a sample view configuration and error message.

As per point #3, if you can be more specific as to what you want then I would be glad to provide you with details.

@ashayshub ashayshub changed the title check_csrf=True attribute view_config on invalid csrf token causes 500 Internal Server Error instead of 403 Forbidden Error check_csrf=True attribute view_config on invalid csrf token causes 500 Internal Server Error instead of 400 HTTPBadRequest Error Jan 25, 2015
@ashayshub
Copy link
Author

@bertjwregeer and @mmerickel

Here is a view I have created for you to test, I am posting all details from the very basics:

considering csrf token is retrieved from session, session details in development.ini :

#pyramid_beaker settings
session.type = file
session.data_dir = %(here)s/data/sessions/data
session.lock_dir = %(here)s/data/sessions/lock
session.key = CustomerKey
session.secret = CusomterSecret
session.domain = localhost
session.path = /

partial project/project/init.py details:

from pyramid_beaker import session_factory_from_settings
import Loader

...more...

def main(global_config, **settings):
    """ This function returns a Pyramid WSGI application.
    """

    engine = engine_from_config(settings, 'sqlalchemy.')
    DBSession.configure(bind=engine)
    Base.metadata.bind = engine
    session_factory = session_factory_from_settings(settings)

    config = Configurator(settings=settings, locale_negotiator=my_locale_negotiator)

    #Route
    config.add_route('Test', '/test/')
     #View
    config.add_view(Loader, route_name='Test')

...more...

Inside Loader class:

from pyramid.view import view_config

class Loader(object):
    """Loader View"""

    def __init__(self, request):
        """Init function """
        self.request = request
        self.session = self.request.session

    @view_config(route_name='Test', renderer='json', request_method='GET', check_csrf=True)
    def my_test(self):
        return {'test': self.session.get_csrf_token()}

How to make the HTTP call?

from any browser call below url, without check_csrf=True predicate in view_config:

http://localhost:6543/test/

You will get something like:

{"test": "a9877ff550f8b06a32aec2b5063dfe9ee5166381"}

Now use this csrf details with check_csrf=True predicate in view_config once and everything should work fine. Next deliberately add say 1 to the csrf token to check for a wrong csrf token.

http://localhost:6543/test/?csrf_token=a9877ff550f8b06a32aec2b5063dfe9ee51663811

Output:
In case where you use the http call without check_csrf predicate or with correct csrf token there should be absolutely no error at all.

{"test": "a9877ff550f8b06a32aec2b5063dfe9ee5166381"}

In case you use a wrong csrf token:

This should result in front end

500 Internal Server Error

and in the backend, you should get error:

pydev debugger: process 11153 is connecting

Connected to pydev debugger (build 139.1001)
Starting server in PID 11153.
serving on http://0.0.0.0:6543
2015-01-25 13:36:51,600 ERROR [pyramid_debugtoolbar][Dummy-5] Exception at http://localhost:6543/test/?csrf_token=a9877ff550f8b06a32aec2b5063dfe9ee51663811
traceback url: http://localhost:6543/_debug_toolbar/exception?token=62275c7861325c7839632d4d597b5c7863665c7865625c7861382d27&tb=4398367312
Traceback (most recent call last):
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_debugtoolbar-2.3-py3.3.egg/pyramid_debugtoolbar/toolbar.py", line 178, in toolbar_tween
    response = _handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_debugtoolbar-2.3-py3.3.egg/pyramid_debugtoolbar/panels/performance.py", line 57, in resource_timer_handler
    result = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/tweens.py", line 21, in excview_tween
    response = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/__init__.py", line 95, in tm_tween
    reraise(*exc_info)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/compat.py", line 15, in reraise
    raise value
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid_tm-0.10-py3.3.egg/pyramid_tm/__init__.py", line 76, in tm_tween
    response = handler(request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/router.py", line 163, in handle_request
    response = view_callable(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 596, in __call__
    return view(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 385, in viewresult_to_response
    result = view(context, request)
  File "/Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py", line 475, in _class_requestonly_view
    response = inst()
TypeError: 'Loader' object is not callable

Hope this is enough for you to debug at your end. Also note that I use python 3.3.3. If you need anything more from me, I'll be glad to help, though would appreciate if you are more specific.

@ashayshub
Copy link
Author

I guess I have figured the issue. I haven't tested it by fixing it as I don't want to currently make changes in libraries outside my project. This seems to be a bug but more clarity would come from the author himself.

Here is how it goes:

The predicate check_csrf=True is registered through below flow (just brief references, go through actual files for more details):

  1. Initial Configurator.
    File: /Users/Home/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/__init__.py

  2. Registering predicates.
    File: /Users/Ashay/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/views.py

    def add_default_view_predicates(self):
        p = pyramid.config.predicates
        for (name, factory) in (
            ('xhr', p.XHRPredicate),
            ('request_method', p.RequestMethodPredicate),
            ('path_info', p.PathInfoPredicate),
            ('request_param', p.RequestParamPredicate),
            ('header', p.HeaderPredicate),
            ('accept', p.AcceptPredicate),
            ('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),
            ):
  3. Predicate classes. Here note the self.check_csrf_token call, esp. raises=False
    File: /Users/Ashay/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/config/predicates.py

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
  1. Elsewhere, when we directly call self.request.session.check_csrf_token(self.request) we do not usually include raises parameter. The default value taken in such case by raises is True. I do not see the reason for raises being False here. This causes BadCSRFToken to not process and thus HTTPBadRequest exception to not raise as seen below.

File: /Users/Ashay/Programming/Python/3.x/pyramid_projs/env2/lib/python3.3/site-packages/pyramid/session.py

```python
def check_csrf_token(request,
                 token='csrf_token',
                 header='X-CSRF-Token',
                 raises=True):
     supplied_token = request.params.get(token, request.headers.get(header))
     if supplied_token != request.session.get_csrf_token():
         if raises:
              raise BadCSRFToken('check_csrf_token(): Invalid token')
          return False
     return True
```

Also would appreciate if you tag this issue as a bug after consulting with the author and fix it in the future release if found correct. I would appreciate the author's comments on this issue.

@mmerickel
Copy link
Member

Ok thank you for posting your example. This is not a bug, but it is understandable that you have had difficulty tracking down the issue.

The following is the problem. In the first code example below you are registering one view, with no predicates, for the Loader class. If you remove this view then everything will start working as expected (explained at the bottom in [1]).

#View
config.add_view(Loader, route_name='Test')

The reason that this is breaking everything is because you probably do not realize that this is adding a second view to your route, and when you add a class-based view without an attr argument, a __call__(self) is expected to be found on Loader, and it's obviously not in your setup. So when check_csrf=True fails, or you pass a non-GET request, then the my_test method will not match the request predicates and it will not be invoked. Instead pyramid will fallback to the view above (which is not valid since you haven't defined a __call__ making it callable.

[1] And by expected, I mean that you will see a 404 when attempting to access the view and check_csrf=True is on the view, because as I explained earlier check_csrf=True is a predicate for view matching. It means the view will not match, and no view will be found for that route, and thus a 404 is raised. If you want a 400 (bad csrf) error then you should use check_csrf_token directly.

@ashayshub
Copy link
Author

Ok, Thanks for the explanation.

@ashayshub
Copy link
Author

This is not a bug, confirmed by removing view_config decorators and placing the view predicate and non-predicate parameters solely under add_view. Also the error is HTTP 404 as mentioned by you.

@rskumar
Copy link

rskumar commented Apr 8, 2016

Proper error code should be 403. Its confusing, since the URL exists but its access is forbidden due to improper or missing csrf.

404 does not make sense here. Would it be possible to show message in debug mode so developer can know the reason too.

@digitalresistor
Copy link
Member

@rskumar Unfortunately due to the fact that check_csrf is a predicate, it can't directly affect the type of error code that is returned, because there is no view that matches, it truly is a 404 error.

With the new view derivations coming, with the use of #2413 you will get a BadCRSFToken that is raised, and then you can handle it as appropriate.

@rskumar
Copy link

rskumar commented Apr 8, 2016

wow, nice.. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants