Pow: Use `no-cache` In `cache-control` header for session and registration form pages

[danschultzer]

Per elixirforum.com:

True, but the subtle difference here is how the back button works in browsers. Unfortunately browsers don’t respect expiry or max-age directives in Cache-Control when it comes to history-lists. There is a good longer-form explanation here.

So, say a user while logged in, types out session/new in the address bar and presses return, the browser notices that the login page is stale since max-age is 0 and sends a request to the server, at which point phoenix knows they’re already logged in, and redirects them appropriately. This works as expected.

Instead, If the user clicks the back button and goes to session/new somewhere in their history, they will just see the login page again and they can interact with the form, resubmit it etc…, we do not want that to happen. I can make gif to show this behavior when I get home.

I think this happens because, the private directive tells the browser that /session/new can be cached by the browser locally, and history doesn’t respect max-age being 0, so no request is triggered and the form shows up again.

If we set no-store directives, then this doesn’t happen. Nothing from the response is cached, so even if the user goes to session/new via back-button, the browser still has to make a fresh request at which point phoenix will redirect appropriately.

Just to be sure, I checked how github handles this, and they do have no-store, no-cache in Cache-Control.

It makes a lot of sense to prevent cache for the session/registration form pages.

I would like to see what’s standard with most large websites, and then go with that. Also this would work best as a fallback, so if there’s a way to detect whether the cache-control header has been explicitly set or not, this would be the ideal way of handling it. Some users may prefer to set their own cache-control header for Pow or the whole app.

[danschultzer]

There may also be some security considerations: https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Session_Management_Cheat_Sheet.md#web-content-caching

[ryloric]

Hi!,

I'm pretty new to Elixir, but I think would it be more reasonable to make cache control an option for Pow.Phoenix.Router

Something along the lines of :no_cache which is a bool, true by default. It would be easy to customize, since it's right at the top of router.ex. I'm not entirely sure, but it feels a little more natural than hiding it as a fallback.

[danschultzer]

I prefer explicit calls in the controllers for plug methods. Implicit handling through configuration always felt wrong to me, I avoid them as much as possible. With elixir I think it’s much easier to understand the data transformation if you can see what flow the data goes through without having to figure out what configuration setting is turned on. It’s less magic, and easier to debug since the flow is just code rather than configuration settings.

In this case it’s also better than using a plug method in router, since it’s just one action in each controller we want the hit. The routes would be more complex if we handled it in router.

Also any customization of “cache-control” is already handled here, since the no cache header will only be set if the current cache header is the default one set by plug.

[danschultzer]

Oh, the above is in reference to the proposed PR in #213. I see you were probably just talking about what I wrote here 😁

[ryloric]

I prefer explicit calls in the controllers for plug methods. Implicit handling through configuration always felt wrong to me, I avoid them as much as possible. With elixir I think it’s much easier to understand the data transformation if you can see what flow the data goes through without having to figure out what configuration setting is turned on. It’s less magic, and easier to debug since the flow is just code rather than configuration settings.

Yeah, this makes much more sense now. Sorry, I'm still figuring out the common idioms and patterns of Elixir :smile:

[danschultzer]

I think the current implementation is good, so I've merged it to master, and this will be released in v1.0.10. Thanks for the suggestion and help @ryloric!