[huntr] adding cache control headers to the admin area #3097
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clarkwinkelmann
approved these changes
Oct 7, 2021
davwheat
reviewed
Oct 7, 2021
There was a problem hiding this comment.
Using this header on all routes seems overkill, does it not? From what I've seen online, it should be possible to only apply these headers on the logout route, and that will invalidate the cache from the past too.
We also need extra headers to make sure all browsers invalidate the cache correctly. See this from when I started to work on this.
|
Using the header on the admin routes is fine. Because that is what generates the HTML. Applying them only on the logout route will invalidate cache on that route, but it already has no cache. It's the HTML that might be cached and share data from the logged out admin. |
|
Gotcha. Looks good to me. |
davwheat
approved these changes
Oct 7, 2021
tankerkiller125
approved these changes
Oct 7, 2021
askvortsov1
approved these changes
Oct 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes https://huntr.dev/bounties/73176569-c102-47ad-9e95-17dd344a8847/
Changes proposed in this pull request:
This PR forces the
Cache-Control: no-store, max-age=0header to the response in the Admin Area. This forces cache to be ignored upon browsing back and forth between pages using the browser controls. Although absolutely no fail safe, it should provide better protection against serving cached pages once an admin has signed out.Reviewers should focus on:
For now, I don't think we need to push this onto the Forum frontend, but we do need to start looking into cache control soon. Perhaps I can tease @BartVB to write some directions in the headers to apply for us as cache control is quite an expertise.