Pylons / pyramid_simpleform

Bindings to help integrate formencode with Pyramid.
Other
15 stars 25 forks source link

CSRF method uses existing form value over correct session csrf value. #6

Open edelooff opened 11 years ago

edelooff commented 11 years ago

I have a schema that defines a single non-empty text field and sets the controls that allow additional fields and filters them. The schema looks like this:

class Message(Schema):
  """Message reply."""
  allow_extra_fields = True
  filter_extra_fields = True

  message = validators.UnicodeString(not_empty=True, max=500)

It is then processed in the following view code:

@view_config(request_method='POST', renderer='message.mak')
def claim_update(context, request):
  thread = context.data
  form = pyramid_simpleform.Form(
      request, schema=schemas.Message)
  if form.validate():
    # ... persist and redirect
    return exc.HTTPSeeOther('/message')
  return {'thread': thread, 'form': FormRenderer(form)}

The form renderer adds a csrf token and then the form is submitted by the user. The view code runs again and after validating (and stripping of attributes not in my schema) the form.data dictionary looks like this:

{'_csrf': u'6921efe037911dfe28991802462034c227173a06',
 'message': u'',
 'submit': u'Add message'}

The attributes that should have been stripped out have not been.

Performing a standalone to_python on my Message schema does work as expected (only the message gets returned, and when empty, Invalid is raised.

This causes me trouble because I'm resetting the csrf token on every successful post, but the old csrf value keeps coming back.

edelooff commented 11 years ago

Looking over the code, the problem seems to occur at line 191 in init.py:

self.data.update(params)

        if self.schema:
            try:
                self.data = self.schema.to_python(decoded, self.state)
            except Invalid, e:
                self.errors = e.unpack_errors(self.variable_decode,
                                              self.dict_char,
                                              self.list_char)

Just before the validation code, all form values are added to the data attribute.

Thinking about it though this makes sense because otherwise one value being invalid would cause all form data being removed.

This does mean that the CSRF checking code should not get the value, but pop it from the MultiDict. This way, it will not be present when the form gets rendered and the csrf placing code will do its intended action.

Although, the better solution would probably be to correc the csrf method to not render through the hidden method which does a lookup of the _csrf key through value where the correct CSRF key becomes a default and not the override value.