cachethq / Docker

A Dockerized version of Cachet.
https://cachethq.io
BSD 3-Clause "New" or "Revised" License
416 stars 281 forks source link

Remove caching logic from nginx-site.conf #311

Closed michaelkaye closed 5 years ago

michaelkaye commented 6 years ago

Fixes #272

This basically rips out all of the caching logic from the nginx config, as a rough cut to make this docker container safe to deploy on public sites. I think we could go back to caching some pages, but I think some of this lies upstream with cachet/Cachet to address, to enable a front page that is safe to cache aggressively (as a status page should be).

Can I request that this get merged and a container pushed quickly, and this advertised as a urgent update for those using this docker container, as full access to the dashboard for non-logged in users is quite a severe problem.

I've tested this locally and it seems to have the right behaviour - I've not managed to share between systems yet, but absence of proof is not proof of absence.

michaelkaye commented 6 years ago

Have pushed my local image to https://hub.docker.com/r/michaelka/cachet-docker/ if anyone wishes to test.

NB: I am NOT planning on maintaining this image beyond this PR request - please no-one start depending on it in production.

thomasleveil commented 6 years ago

@michaelkaye, you could also give a try to fastcgi_hide_header "Set-Cookie";

michaelkaye commented 6 years ago

To do this properly, we need to be able to cache by being logged in or not, as otherwise non-logged in users could still see the edit link because the page that was cached included it, even if they can't then actually use it to edit anything or visit the dashboard.

Happy to open a feature request for somebody* to make the caching possible for this, but at the moment, I want to remove all of it so that we are at least secure, if not fast.

djdefi commented 6 years ago

@jbrooksuk I am no longer on the CachetHQ/Docker team to be able to approve this. Could you add me back possibly 🙏

shuichiro-makigaki commented 6 years ago

@jbrooksuk ping

This patch works fine in my side. LGTM.

conf commented 6 years ago

Any news on this? This thing breaks personal subscription management page (/subscribe/manage/<token>) for us (page shows stale results after update because of too aggressive caching).

djdefi commented 6 years ago

@jbrooksuk this one is good to merge 👍

jbrooksuk commented 5 years ago

Thanks everyone!