Add "no-store" to Cache-Control header to prevent history caching of admin resources

[soulseekah]

The current implementation of ​wp_get_nocache_headers does not take into account history caching, which results in a browser serving a cached copy of pages from history (by pressing the Back button) even if the user has long logged out.

​RFC 2616 14.9.2 no-store describes this cache directive.

To repoduce: login to dashboard, logout, press the back button.
Expected: the login screen.
Reality: a copy of the previous page.

By adding the "no-store" directive to all non-cachable resources the behavior was mitigated successfully in Chrome 21, Firefox 15. Fails on Opera 12 (they chose to disregard "no-store" when applied to history, RFC allows this).

[soulseekah]

"no-store" please

[toscho]

That needs good tests. For example the ​behavior after POST requests can be quite annoying. Performance might be an issue too.

[soulseekah]

Replying to toscho:

That needs good tests. For example the ​behavior after POST requests can be quite annoying. Performance might be an issue too.

Agreed, under certain circumstances this could be a nuisance, especially if users are logged in and the admin bar is shown. As for performance, it seems that all of twitter is served with "no-store" and it does appear (to me) to be quite jagged navigation-wise.

Without a proper step-by-step testing plan in mind, I guess I'll use the patch in production for a bit to see if it causes any unexpected issues in general.

[nacin]

The other issue is sometimes you want the back button to be where you were last, without a complete reload. How many times have you lost a comment or some other content by clicking away (in WP or not) and scrambled to recover it?

[johnbillion]

Related: #19444.

[nevma]

It seems that Firefox actually needs the no-store as well, in order to not cache a page. Otherwise, it serves it from the ​BFCache. The no-store should be added to the wp_get_nocache_headers function in the Cache-Control header. This is what the ​MDN documentation in Using Firefox Caching mentions, too.

[dingo_bastard]

Is this something that will be implemented or not? Since there exists a nocache_headers filter, if user wants to modify the Cache-Control all he/she has to do is unset it and then add no-store in it with the filter.

IMO this ticket can be closed.

[vikram6]

Hi. Are there any plans to fix this or should we workaround it on our side?

[johnbillion]

Moving to 6.3 along with #57627

[johnbillion]

#57938 was marked as a duplicate.

Add the no-store keyword to the default nocache headers.

Trac ticket: #21938

Trac ticket: https://core.trac.wordpress.org/ticket/21938, https://core.trac.wordpress.org/ticket/57627

[johnbillion]

Thanks for the PR @luehrsen! I've been working on this in combination with #57627 and the PR at ​https://github.com/WordPress/wordpress-develop/pull/4570 should address both of these tickets by conditionally adding the no-store and private directives when the user is logged in. Would you like to do some testing?

While logged in, visiting the frontend: Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private

While logged in, visiting the backend: Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private

While logged out, visiting the frontend: No Cache-Control header is present.

While logged out, visiting the backend and redirected to wp-login: Cache-Control: no-cache, must-revalidate, max-age=0

While logged in, visiting the backend, performing a logout, pressing the BACK button in the browser: No admin page is shown, but the login page.

LGTM!

[Dharm1025]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/4570

Environment

• OS: macOS Ventura 13.0
• Web Server: nginx/1.25.0
• PHP: 7.4.33
• WordPress: 6.3-alpha-55505-src
• Browser: Chrome Version 113.0.5672.126 (Official Build) (arm64)
• Theme: Twenty Twenty-Three
• Active Plugins: -

Test Results

✅ Works as expected with a patch.

I have tested the patch as per testing instructions and it works as expected.

Before Patch:

Cache-Control Header:

1. Front-end (logged in): Cache-Control: no-cache, must-revalidate, max-age=0
2. Front-end (not logged in): No Cache-Control present
3. Back-end (logged in): Cache-Control: no-cache, must-revalidate, max-age=0
4. wp-login.php page: Cache-Control: no-cache, must-revalidate, max-age=0

Login to wp-admin, then logout and press the back button in the browser shows the previous wp-admin page.

After Patch:

Cache-Control Header:

1. Front-end (logged in): Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private
2. Front-end (not logged in): No Cache-Control present
3. Back-end (logged in): Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private
4. wp-login.php Page: Cache-Control: no-cache, must-revalidate, max-age=0

Login to wp-admin, then logout and press the back button in the browser shows the wp-login page.

Thanks

Change looks good - looking at the ​spec it seems to make sense to have both.

[johnbillion]

In 55968:

Administration: Add the no-store and private directives to the Cache-Control header when preventing caching for logged in users.

The intention behind this change is to prevent sensitive data in responses for logged in users being cached and available to others, for example via the browser history after the user logs out.

The no-store directive instructs caches in the browser or within proxies not to store the response in the cache. This is subtly different from the no-cache directive which means the response can be cached but must be revalidated before re-use. WordPress does not use ETag headers by default therefore this does not achieve the same result.

The private directive complements the no-store directive by specifying that the response contains private information that should not be stored in a public cache. Som
e proxy caches may ignore the no-store directive but respect the private directive, thus it is included.

The existing Cache-Control header for users who are not logged in remains unchanged, and the existing cache prevention directives remain in place for backwards compatib
ility.

Props soulseekah, luehrsen, Dharm1025, markdoliner, rutviksavsani, ayeshrajans, paulkevan, clorith, andy786, johnbillion

Fixes #21938, Fixes #57627

Committed in https://core.trac.wordpress.org/changeset/55968

Committed in https://core.trac.wordpress.org/changeset/55968

[noisysocks]

@soulseekah @johnbillion: Do either of you have any guesses as to why no-cache would affect E2E tests in WordPress and Gutenberg?

See:
https://core.trac.wordpress.org/ticket/58592
​https://github.com/WordPress/gutenberg/pull/51826

[soulseekah]

@noisysocks the issues outlined in the pull seem to be timeout-related. Most probably, due to the no-cache calls caching has been disabled and load has been significantly increased (especially if tests are being run in parallel). I'm unfortunately unable to provide any more insight as I'm not familiar with how the E2E tests are developed and run.

[johnbillion]

The load should not increase significantly during tests because the no-store directive affects whether a response can be fetched from the browser cache when navigating the browser history. If the tests aren't making use of the browser history then there should be no change to the load on the server. The browser should still continue to cache CSS and JS files that are loaded on the page.

There could well be a bug in the test driver or test suite which means this is not the case and additional load is being placed on the server, therefore increasing response times and causing timeouts. I'm also not familiar enough with the e2e test suite to know for sure.

I'm going to reopen this ticket and tag it with the performance focus in case there is a degradation or regression that's gone unnoticed.

Performance testing steps

• Request multiple successive pages in the wp-admin area
• Ensure that JS, CSS, and image assets loaded on the pages remain cached appropriately between successive requests
• Ensure the browser back button causes the page to be re-requested from the server and not served from the cache, but that JS, CSS, and image assets are served from the cache
• Test both with SCRIPT_DEBUG enabled and disabled

[oglekler]

Right now we don't have more time for testing, so, because there are doubts and this ticket still needs testing, I am moving this into 6.4. Let's plan it handling before Beta 1 of the next release, or better — much sooner.

[johnbillion]

This has already been committed for 6.3. The change itself is working as expected, I added the testing notes above in case an anomaly was found in a certain browser configuration or in the e2e test runner.

I'd like to keep this open for now and in the 6.3 milestone while the investigations into the e2e test failures are ongoing.

@oglekler Any objection to moving this back to 6.3? If so I'll have to revert.

[oglekler]

@johnbillion sorry, from the whole your comment it looked like it was not ready, and I didn't notice that changes weren't reverted. But I am not sure that this is the right way to open a ticket for watching if some side effect will happen, or we need to do something additional. Possibly for such things, we need to open another one or make and additional status for ticket. I believe that all tickets needs to be rechecked and tested after the Beta 1, but it looks like a pink pony world. I also believe that people who are checking that things are working after ticket is gone into trunk needs also be appreciated. This should be a normal practice - if you don't find any problem, if it doesn't mean that your efforts are not valuable. But this question it out of the scope of this ticket 😅

[johnbillion]

Ok I am going to open a PR with this change reverted and see what happens with the e2e tests. Stand by.

[johnbillion]

Ok the test run on ​https://github.com/WordPress/wordpress-develop/actions/runs/5392522701 has produced the same e2e test failures as we have in trunk. Therefore it appears that the introduction of the no-store cache control directive is a red herring.

I'll re-close this ticket as fixed for 6.3.

This adjusts the "No private directive present in cache control when user not logged in" test so they won't fail if the headers don't contain a cache-control key at all.

While trying to debug why E2E tests have been flaky, I noticed that the logged out test for cache control headers was often failing locally because the response headers don't have a cache-control key at all, resulting in a Matcher error: received value must not be null nor undefined failure. This adjusts the tests to use expect.not.objectContaining() instead.

Trac ticket: https://core.trac.wordpress.org/ticket/21938

[joemcgill]

Reopening to consider adjusting the unit tests. The main functionality that has already been committed still applies.

The tests are running fine for me locally with this change, but CI still isn't happy. I'll get this in anyway.

[johnbillion]

In 56134:

Administration: Adjust the "No private directive present in cache control when user not logged in" test so the assertions won't fail if the headers don't contain a cache-control key at all.

Props joemcgill

Fixes #21938

https://core.trac.wordpress.org/changeset/56134

Thanks @johnbillion. Agree with you that this doesn't fix the timeouts that we're seeing in CI, but this does make it easier to run these tests locally while debugging.

[stevenlinx]

Added to misc dev note.
draft: ​https://make.wordpress.org/core/?p=106236&preview=1&_ppp=2977223417

[westonruter]

Coincidentally, I've been looking into removing use of the unload event (#55491) because Chrome intends to ​deprecate it, and more importantly because ​it prevents bfcache. But something else that blocks bfcache is Cache-Control: no-store, which this ticket is all about. Adding no-store wouldn't have caused any performance regression in the admin in 6.3 because wp-heartbeat uses the unload event. However, with this removed, the introduction of no-store holds back the performance of page navigations in the admin and the frontend by disabling bfcache.

The question I have is whether the increase to security/privacy by disabling bfcache for logged-in users is worth the performance hit for the 80% of users. If not, perhaps adding no-store should be a privacy/security enhancement that site owners install via a plugin when a site is accessed by users who use shared computers? Alternatively, perhaps no-store should only be used by default when a user does not check the "remember me" checkbox when logging-in? POC plugin: ​https://gist.github.com/westonruter/8c19d87a80a36e8f24db910750162628