tgext.routes changes

[Thomas De Schampheleire]

Hi Søren,

Thanks for the feedback.

On Apr 12, 2016 12:23, "Søren Løvborg" <sorenl at unity3d.com> wrote:
>
> I would definitely not recommend copying the current _method handling. :-)
>
> The correct thing must be an early pass where the _method override is applied to the REQUEST_METHOD. All code beyond that point should just see REQUEST_METHOD, and never know the difference between fake and real HTTP verbs. Which sounds like exactly what tgext.routes 0.1.2 does.

So this means updating Kallithea. Do you happen to be interested and
available for such change? You have shown to be very knowledgeable
about this area :-)


>
> (Aside: I did not look at the tgext.routes code, but I assume the override support is opt-in? Enabling it automatically for all applications could cause security issues for applications that don't have CSRF protection.)

AFAIU there is no opt-in currently: the following lines were added
unconditionally:

# routes middleware overrides methods using _method param.
if environ['REQUEST_METHOD'] == 'GET' and '_method' in state.request.GET:
    environ['REQUEST_METHOD'] = state.request.GET['_method'].upper()
elif environ['REQUEST_METHOD'] == 'POST' and '_method' in state.request.POST:
    environ['REQUEST_METHOD'] = state.request.POST['_method'].upper()


>
>
> > If I temporarily change that code so that 'DELETE' is also part of the
> > accepted request methods, then test_delete fails due to 403 Forbidden
> > while it expects 405 Method Not Allowed, and
> > test_delete_browser_fakeout fails with 403 too (CSRF protection, I
> > guess?)
>
> Yes, and that behavior is arguably the correct one. test_delete should be changed to expect 403, since the DELETE method is in fact allowed (but the CSRF check is expected to fail).

For test_delete, I agree that CSRF failure is expected.
However, for test_delete_browser_fakeout, a CSRF authentication token
is actually supplied in the test, so I would expect it to succeed.
Right?

/Thomas