Should anti-forgery token be kept after valid POST?

[mainej]

My question is... is the test in this diff properly or improperly failing?

Actual

This test fails. The response session is {"resp" "foo"} and is missing ::af/anti-forgery-token. The makes the following sequence inevitable:

1. receive a POST, with valid anti-forgery tokens (in the session and the headers/form params)
2. respond with session data (perhaps the user logged in, and you are storing their identity in the session)
3. forget to copy ::af/anti-forgery-token from the request session to the response session
4. the next POST cannot possibly be valid, because it will not have ::af/anti-forgery-token in the session

I can think of two ways to make the test pass:

Solution 1

The "ring-session way". Require that the inner-most handler copy ::af/anti-forgery-token from the request into the response session. There are two ways to do this, both of which have downsides.

• Copy the entire request session into the response before assoc-ing our new session data. This would copy ::af/anti-forgery-token, but of course would also copy any other keys. In the case of our test, this means the response session would also contain {"req" "foo"}, which may or may not be harmless, depending on the application.
• Or, explicitly copy only ::af/anti-forgery-token from the request to the response session. This feels like it leaks knowledge about ring-anti-forgery into inner layers of the middleware stack.

I think that the first method of copying is what this library expects. However, that means that almost all examples in the wild of using ring-session are wrong, at least when ring-anti-forgery is also enabled. Instead of writing handlers that just set the session, you should almost always do what this library and other similar libraries do.

If this is so, I suggest documenting it in the README, with an example of what the handler should do.

Solution 2

Have ring-anti-forgery copy the token from the request to the response. This is not the "ring-session way", but is there any downside? I suppose it means that inner layers of the middleware stack can't modify or remove the anti-forgery token. But isn't that OK, or even desirable?

In the end, whether the answer is Solution 1 or Solution 2, I'd be happy to provide patches to improve the documentation or change the code.

[weavejester]

The test should fail, because the handler is wiping the anti-forgery token from the session. Perhaps this could be added to the README as a potential "gotcha", if people are replacing rather than updating the session.

[mainej]

Failing test removed. "Gotcha" added to README.