Don't serve cached pages to logged-in users

[hirasso]

Adds an additional RewriteCond to the .htaccess that will check if a user is logged-in before serving cached files, by testing for the existence of the kirby_session cookie.

Drive-By

• Add syntax highlighting (apacheconf) to the snippet

[lukasbestle]

That's an excellent idea for the 99% case, thanks for sharing.

Would the following also work?

RewriteCond %{HTTP_COOKIE} !kirby_session

[hirasso]

@lukasbestle yes, your snippet also works! I'll change my PR accordingly, much easier to read.

Maybe it's worth to note that while testing this more thoroughly, I noticed that the kirby_session cookie is being created as soon as I visit /panel, even if I don't log in. It would probably be more robust to introduce a dedicated cookie for identifying logged-in users sometime in the future. WordPress does it like this, and WP Super Cache (the caching plugin developed by WP's Automattic) is where I got the idea to check for the cookie from. They also do a few more nice things, for example checking for a query string before serving a cached file:

RewriteCond %{QUERY_STRING} ^$

What do you think, should we also include this in the readme of staticache? I'll certainly use it in my setup :)

[lukasbestle]

Thanks for updating the PR.

Maybe it's worth to note that while testing this more thoroughly, I noticed that the kirby_session cookie is being created as soon as I visit /panel, even if I don't log in. It would probably be more robust to introduce a dedicated cookie for identifying logged-in users sometime in the future.

It's not just visiting the login page (which creates a session because it stores a CSRF token in the session). Also other custom functionality in the site frontend could and will create a session.

Kirby's PHP-based caching takes the actual use of the session into account. So even if there is an active session, a page will only be excluded from caching if the template has accessed the session (= if the response somehow depends on the session contents).

In Staticache we can of course not do it in such a thorough way because we want to do as little processing on request as possible, which is the main advantage of Staticache compared to the PHP-based caching.

I think it's fine to at least offer the condition added by this PR as an option. Of course it will reduce the cache hit ratio because in some false-positive cases, a request is routed to Kirby that could have been responded by Staticache. But Staticache will still be able to serve a large proportion of requests for most sites that don't heavily rely on sessions.

Before we merge this PR, I think it would be good to add the same feature to the other server configs (nginx and Caddy) as well as to the simple PHP loader. Would it be possible for you to work on that?

They also do a few more nice things, for example checking for a query string before serving a cached file:

RewriteCond %{QUERY_STRING} ^$

What do you think, should we also include this in the readme of staticache? I'll certainly use it in my setup :)

Requests with a query string are not cached by Kirby in the first place and if there's nothing cached, Staticache can also not serve anything. Or am I mistaken?

[hirasso]

Before we merge this PR, I think it would be good to add the same feature to the other server configs (nginx and Caddy) as well as to the simple PHP loader. Would it be possible for you to work on that?

I don't have any experience with either nginx or caddy – so I can't contribute here. But feel free to push to this PR if you know your way around these :)

Requests with a query string are not cached by Kirby in the first place and if there's nothing cached, Staticache can also not serve anything. Or am I mistaken?

I have the following use case in mind: Imagine a page /contact that contains a contact form. This form could include a name field, for example:

<form action="/contact?success=1" method="POST">
  <input type="text" name="name"></input>
  <!-- other fields -->
</form>

Submitting this form would lead to /contact?success=1, that could print something like:

<p>Hello <?= esc($_POST['name']) ?? 'there' ?>, thank you for your message!</p>

I would expect the first page (/contact) to be served from the cache, while the second one should stay dynamic, so that I can print out the personalized message to the user. Detecting a query string before serving a cached file would help achieve exactly that.

[lukasbestle]

I would expect the first page (/contact) to be served from the cache, while the second one should stay dynamic, so that I can print out the personalized message to the user. Detecting a query string before serving a cached file would help achieve exactly that.

You are right. Kirby does cache the page without query string and that would then overshadow the requests with a query string. So we do in fact need the condition you suggested.

We also need one for Kirby params (like /contact/param:value). If the request path contains a colon, the request should also be routed to PHP and not served from cache.

I don't have any experience with either nginx or caddy – so I can't contribute here. But feel free to push to this PR if you know your way around these :)

I can work on that, but it will take some time. My todo list is a bit long at the moment. :)