Allow for custom env settings to be set and simple csrf support

[dariocravero]

Hey @brynary,

How're things?

Helping @Rex37 trying to come on board using Padrino we've come across the need to test out controllers protected from CSRF.

From the rack-protection specs you can tell it's not that hard to do so but it clearly becomes a burden when you have to start setting all of that extra code {'rack.session' => {:csrf => "b"}, 'HTTP_X_CSRF_TOKEN' => "b"}) for each unsafe method you test.

This PR intends to cover that along by adding a csrf helper that allows a user to do something as simple as:

before do
  csrf
end

and they can forget about csrf management for that test group until they clear it out with csrf(false).

In order to have the validation work fine, I needed two things: a header and a session value. The header was taken care by header but I just couldn't find a way of setting the session value across all of the requests.
That is why I implemented an env method that allows you to set anything on the env object. I know this might sound risky but in all fairness, whomever is implementing the tests should know how to handle that.
However, there might be another way of doing it that I'm just not aware of.

What do you think about this?
Thanks! :)

[nesquena]

Looks good to me 👍

[brynary]

The #csrf helper feels more specific than Rack-Test aims to be. We don't want to maintain helps for every common header/HTTP feature that comes along.

The #env helper makes more sense, but I'm also not a big fan of it yet. Rack-Test is a black box testing tool so I'm not sure of the need to patch the env in a systematic way. Could you just stub something in the app instead?

[dariocravero]

Yeah, I agree #csrf might be a bit too specific but since I saw the auth
helpers I thought it could make it in. I will remove it from this PR and
probably move it into its own gem called rack-test helpers or contrib.

As for #env, the problem is that if it doesn't happen at that level it
becomes pretty hard to set those values if not impossible at all for
subsequent requests. At the end of the day, the test itself should deal
with those in the same way it handles headers.

On Monday, April 15, 2013, Bryan Helmkamp wrote:

The #csrf helper feels more specific than Rack-Test aims to be. We don't
want to maintain helps for every common header/HTTP feature that comes
along.

The #env helper makes more sense, but I'm also not a big fan of it yet.
Rack-Test is a black box testing tool so I'm not sure of the need to patch
the env in a systematic way. Could you just stub something in the app
instead?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-16398840
.

Darío

[brynary]

Yeah for CSRF the different to me is that authentication is an HTTP standard, whereas CSRF is not.

For the env I guess what I'm trying to understand is, "Why does the test need to reach into the env state?" Generally rack-test is for black box testing at the HTTP layer, so that's where the functionality is focused. Thoughts? (Not saying I'm a "no" just trying to understand better.)

[dariocravero]

Sorry for the late reply, lost track of this issue and it just came back to my head today!..

I think that it's important to expose env because otherwise you can't automate certain functionality that require a particular value to be set.

The CSRF case couldn't be a better example. If you don't have access to env you have no way of forcing the expected token value for each request and therefore you can't fake the validation when there's no need to test it.

An alternative to this would be to limit the scope and only allow for session values to be set in the same way that headers are being set.

I can't think of other use cases off the top of my head. However, they might come up and instead of having to patch up new methods I reckon that it could be a good idea to leave it open.

What do you think? Thanks

[brynary]

OK, I'll take the env change. Can you please cleanup the PR to remove the csrf stuff? Thanks.

[dariocravero]

Hey @brynary, just removed the csrf code. Let me know if you want me to issue a new PR with only one commit to make it even cleaner. :) Thanks for taking this one in.

[nesquena]

Thanks!