Disable caching when response uses antiforgery

[JunTaoLuo]

When setting the cookie token and headers on a response that uses antiforgery, caching should be disabled to prevent the content from being cached.

Adding the cache control headers Cache-Control: no-cache, no-store should be sufficient.

cc @sebastienros @Tratcher

[muratg]

@Eilon @danroth27 could you guys triage this? Response caching middleware will otherwise cache the tokens. See aspnet/ResponseCaching#69

[Tratcher]

@sebastienros discussed offline. No-cache is set when there is a cookie, but it's not set if the cookie already exists and the antiforgery token is used somewhere in a form. The caller that inserts the token into the form should mark the response as non-cachable.

Note response caching won't cache by default, it requires a direct opt-in Cache-Control: public. There is only a problem if a user directly marks an Action as cachable and then uses anti-forgery in the same action. You have to make sure the caching is evaluated in right order so it gets disabled. This will likely be addressed in MVC.

[Eilon]

Is this bug really just saying that any component that sets cookies should disable response caching? And that antiforgery happens to set cookies? Should components such as session state and cookie auth middleware also do this? (Or do they already do this?)

[JunTaoLuo]

Setting cookies will disable response caching since only responses without the Set-Cookies header are deemed cacheable. The issue with antiforgery is that Set-Cookie is sent the first time. Subsequent responses will not contain that header and will be deemed cacheable by the middleware. As @Tratcher pointed out, in case the action uses antiforgery, for validation for example, and is explicitly marked as cacheable, the cached response for a different antiforgery token will be served and this is wrong. Sometimes it's not obvious that antiforgery is in play, for example forms can implicitly generate them, and this can lead to subtle failures.

The proposal is to disable caching when antiforgery is used regardless of whether cookies are being set. The question is who should disable caching. Should it be done in the antiforgery middleware or in MVC where the form was added?

[Eilon]

Oh I see, that makes sense.

cc @rynowak to help answer the question in the last paragraph of your recent comment.

[rynowak]

The proposal is to disable caching when antiforgery is used regardless of whether cookies are being set. The question is who should disable caching. Should it be done in the antiforgery middleware or in MVC where the form was added?

The simple answer to your question is that there isn't an antiforgery middleware. This should be done inside the IAntiforgery service, which already does a bunch of stuff, and gets called by MVC regardless of whether or not you already have the cookie.

[Tratcher]

That may not be practical. We have to look at where it's being consumed and how it may conflict with the user setting cache headers.

[muratg]

So what are we doing for 1.1?

[Eilon]

@kichalla can you investigate for 1.1.0? Please work with @rynowak, @Tratcher, @JunTaoLuo to figure this out.

[muratg]

@JunTaoLuo Can you verify this (response caching + anti forgery).

[JunTaoLuo]

Tested with @sebastienros and it's been fixed.

[shaunluttin]

Adding the cache control headers Cache-Control: no-cache, no-store should be sufficient.

The commit only sets no-cache. Please also set no-store to prevent the browser back button from serving a cached page.