Exception: KeyError: CSRF_TOKEN

[joar]

Trying to view a video breaks stuff.

BROKEN (videos)

http://mg.wandborg.se/u/joar/m/fluid-box-colourful/ http://mg.wandborg.se/u/joar/m/new-deploy-test/ http://mg.wandborg.se/u/joar/m/sintel/

WORKING (images)

http://mg.wandborg.se/u/joar/m/photo-maj-09-8-06-07-em/ http://mg.wandborg.se/u/joar/m/vasaparken/ http://mg.wandborg.se/u/joar/m/4ec9534946102a5b6f00002c-tove/

BACKTRACE

File '/srv/mg.wandborg.se/mediagoblin/lib/python2.7/site-packages/Paste-1.7.5.1-py2.7.egg/paste/exceptions/errormiddleware.py', line 144 in __call__
  app_iter = self.application(environ, sr_checker)
File '/srv/mg.wandborg.se/mediagoblin/lib/python2.7/site-packages/Paste-1.7.5.1-py2.7.egg/paste/urlmap.py', line 203 in __call__
  return app(environ, start_response)
File '/srv/mg.wandborg.se/mediagoblin/lib/python2.7/site-packages/Beaker-1.6.1-py2.7.egg/beaker/middleware.py', line 155 in __call__
  return self.wrap_app(environ, session_start_response)
File '/srv/mg.wandborg.se/mediagoblin/mediagoblin/app.py', line 158 in __call__
  return render_404(request)(environ, start_response)
File '/srv/mg.wandborg.se/mediagoblin/mediagoblin/tools/response.py', line 33 in render_404
  request, 'mediagoblin/404.html', {}, status=400)
File '/srv/mg.wandborg.se/mediagoblin/mediagoblin/tools/response.py', line 24 in render_to_response
  render_template(request, template, context),
File '/srv/mg.wandborg.se/mediagoblin/mediagoblin/tools/template.py', line 82 in render_template
  context['csrf_token'] = render_csrf_form_token(request)
File '/srv/mg.wandborg.se/mediagoblin/mediagoblin/meddleware/csrf.py', line 53 in render_csrf_form_token
  form = CsrfForm(csrf_token=request.environ['CSRF_TOKEN'])
KeyError: 'CSRF_TOKEN'

BRANCH master

WORKING TREE STATE (latest commit)

commit 72567762e36c849ffe8172b6cea4ca1be682e511
Merge: a3663b4 ca9ebfe
Author: Elrond <well@this-is-an-email-address.example.org>
Date:   Mon Nov 28 18:40:45 2011 +0100

    Merge remote branch 'remotes/nyergler/issue-680-csrf-optout'

    * remotes/nyergler/issue-680-csrf-optout:
      Issue 680 Allow decorating views to prevent CSRF protection.
      Issue 680: Dispatch meddleware request processing post-routing

[Elrond]

The most interesting part is the 404. So somehow video pages request an invalid URL, I think. I hope Joar can investigate that part.

Reproducing this issue is thus very simple: Give GMG an invalid URL, like /frob/ or so.

Adding Nathan Yergler, as he made the latest changes to the meddlware.

The problem is, that the process_request hook is called after the render_404 in app.py:MediaGoblinApp.**call**. And the render_404 wants to setup a complete context for rendering a 404 page. That context wants to contain a CSRF token.

I think, we have three options

1. Move the meddlware a bit up, just before the 404 stuff. This might not be very straight forward, as the route might not exist and so no controller might exist, and so all those controller/route based things in the meddlware must be skipped.
2. Make the render_404 more autonomous. That is: Make it not depend on the normal tools and work "in any case".
3. Split meddlware stuff in "very early" hook for putting the csrf token in the context, and "after routing" for actual checking and controller based disabling.

I know, I don't like option 1, because it makes the meddlware messy. But I don't know, if I like option 2 or 3 better, or if I would even prefer both. Opinions?

[nyergler]

I'm not sure this is actually a meddleware positioning issue. I think the problem is that we're trying to always make the csrf token available to templates, and because the process_request half of the CSRF meddleware never fired, the token never got generated.

I'd probably just handle this in render_template and render_csrf_form_token (only provide the CSRF token if it exists).

[nyergler]

We should probably also have a unit test along the lines of

assert_equal(test_app.get("/does-not-exist/").retval_as_int, 404)

This should fail before we apply a fix for this bug.

[Elrond]

Okay, I have a failing unit test in my local tree on the line of this assert_equal thing. I'll probably push it tomorrow.

[Elrond]

Okay, pushed failing unit test.

[Elrond]

Thanks Nathan Yergler for the suggestions!

I just implemented that and pushed it. Works fine!

[Will Kahn-Greene]

The original url for this bug was ​http://bugs.foocorp.net/issues/685 .