fix(customer-routes): Cache-Control: no-store on customer endpoints (#470)
The trigger: PR #458 mounted requireCustomerPortalEnabled which 410'd every /api/customer/* + /api/admin/customers/* request when the master toggle was off. Some browsers cached that 410 (no Cache-Control header was set, so heuristic freshness applied — the wrong default for an authenticated/sensitive surface). PR #470 reverted the middleware, but a customer whose tab cached the 410 still saw 410s until they hard-refreshed. Add noStoreCache middleware and mount it in front of both route groups. Every response (200, 4xx, 5xx) now carries `Cache-Control: no-store, no-cache, must-revalidate, private` plus the HTTP/1.0 Pragma + Expires fallbacks. Any future transient error from these endpoints can no longer get pinned in browser or proxy caches and outlive its cause. Cost is one setHeader per request; applied per route group rather than globally so static assets + galleries keep their own caching strategy unchanged. Includes a dedicated unit test pinning the header set so a future cleanup pass can't quietly drop it and re-introduce the bug.
This commit is contained in:
+11
-3
@@ -593,11 +593,19 @@ app.use('/api/admin/users', require('./src/routes/adminUsers'));
|
|||||||
// Putting the global flag in the kill-switch role was a mistake — a
|
// Putting the global flag in the kill-switch role was a mistake — a
|
||||||
// stray click in Settings → Features would lock every paying
|
// stray click in Settings → Features would lock every paying
|
||||||
// customer out at once. PR-revert moved the gate back to per-record.
|
// customer out at once. PR-revert moved the gate back to per-record.
|
||||||
app.use('/api/admin/customers', require('./src/routes/adminCustomers'));
|
//
|
||||||
|
// `noStoreCache` belt-and-braces the cache-control story for both
|
||||||
|
// surfaces: any response — 200, 4xx, 5xx — carries `Cache-Control:
|
||||||
|
// no-store` so a transient error (the now-reverted #458 410, a
|
||||||
|
// permission flip mid-session, a backend restart) can't get pinned
|
||||||
|
// in browser or intermediate caches and outlive its cause. See the
|
||||||
|
// PR #458 → #470 history in the middleware file for context.
|
||||||
|
const { noStoreCache } = require('./src/middleware/noStoreCache');
|
||||||
|
app.use('/api/admin/customers', noStoreCache, require('./src/routes/adminCustomers'));
|
||||||
// Customer-side surface (#354). Strictly separate from /api/admin/* —
|
// Customer-side surface (#354). Strictly separate from /api/admin/* —
|
||||||
// distinct token type, distinct cookie, distinct middleware.
|
// distinct token type, distinct cookie, distinct middleware.
|
||||||
app.use('/api/customer/auth', require('./src/routes/customerAuth'));
|
app.use('/api/customer/auth', noStoreCache, require('./src/routes/customerAuth'));
|
||||||
app.use('/api/customer', require('./src/routes/customer'));
|
app.use('/api/customer', noStoreCache, require('./src/routes/customer'));
|
||||||
app.use('/api/admin/event-types', require('./src/routes/adminEventTypes'));
|
app.use('/api/admin/event-types', require('./src/routes/adminEventTypes'));
|
||||||
app.use('/api/admin/api-tokens', require('./src/routes/adminApiTokens'));
|
app.use('/api/admin/api-tokens', require('./src/routes/adminApiTokens'));
|
||||||
app.use('/api/admin/webhooks', require('./src/routes/adminWebhooks'));
|
app.use('/api/admin/webhooks', require('./src/routes/adminWebhooks'));
|
||||||
|
|||||||
@@ -0,0 +1,48 @@
|
|||||||
|
/**
|
||||||
|
* Unit test for the noStoreCache middleware (#470 follow-up).
|
||||||
|
*
|
||||||
|
* The middleware exists because of a real production-class bug: when
|
||||||
|
* #458 mounted a 410-returning kill-switch in front of customer
|
||||||
|
* endpoints, browsers cached the 410 (no Cache-Control was set) and
|
||||||
|
* kept serving it after #470 reverted the middleware. This test pins
|
||||||
|
* the contract so a future cleanup pass doesn't quietly drop the
|
||||||
|
* header set and re-introduce the bug.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const { noStoreCache } = require('../middleware/noStoreCache');
|
||||||
|
|
||||||
|
function makeRes() {
|
||||||
|
const headers = {};
|
||||||
|
return {
|
||||||
|
setHeader: (k, v) => { headers[k] = v; },
|
||||||
|
headers,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('noStoreCache middleware', () => {
|
||||||
|
it('sets Cache-Control: no-store + private and calls next()', () => {
|
||||||
|
const res = makeRes();
|
||||||
|
const next = jest.fn();
|
||||||
|
|
||||||
|
noStoreCache({}, res, next);
|
||||||
|
|
||||||
|
expect(res.headers['Cache-Control']).toBe(
|
||||||
|
'no-store, no-cache, must-revalidate, private',
|
||||||
|
);
|
||||||
|
// HTTP/1.0 fallbacks — old proxies in front of customer-facing
|
||||||
|
// surfaces (corporate VPN gateways, legacy CDNs) honour these.
|
||||||
|
expect(res.headers.Pragma).toBe('no-cache');
|
||||||
|
expect(res.headers.Expires).toBe('0');
|
||||||
|
expect(next).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('runs as middleware regardless of response status', () => {
|
||||||
|
// The header must land on EVERY response coming from the route
|
||||||
|
// group — including 4xx/5xx — so a stale 410 from a
|
||||||
|
// now-reverted middleware can't get pinned in browser cache like
|
||||||
|
// it did in the #458 → #470 sequence.
|
||||||
|
const res = makeRes();
|
||||||
|
noStoreCache({}, res, () => {});
|
||||||
|
expect(res.headers['Cache-Control']).toContain('no-store');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,39 @@
|
|||||||
|
/**
|
||||||
|
* Cache-Control: no-store helper for sensitive endpoints.
|
||||||
|
*
|
||||||
|
* Why a dedicated middleware: shipping the wrong cache-control header
|
||||||
|
* on a session-bearing endpoint is a class-of-bug that bites long
|
||||||
|
* after the original mistake. The PR #458 / PR #470 history is the
|
||||||
|
* concrete trigger:
|
||||||
|
*
|
||||||
|
* - #458 mounted requireCustomerPortalEnabled which 410'd every
|
||||||
|
* /api/customer/* and /api/admin/customers/* request when the
|
||||||
|
* master toggle was off.
|
||||||
|
* - Some browsers cached the 410 (the response carried no explicit
|
||||||
|
* Cache-Control header, so heuristic freshness applied — for an
|
||||||
|
* authenticated/sensitive surface that's the wrong default).
|
||||||
|
* - #470 reverted the middleware, but a customer whose tab cached
|
||||||
|
* the 410 still saw 410s until they hard-refreshed.
|
||||||
|
*
|
||||||
|
* Mounting `noStoreCache` in front of these routes belt-and-braces
|
||||||
|
* the future: any 4xx/5xx (or 200) response from these endpoints
|
||||||
|
* carries `Cache-Control: no-store`, so a transient kill-switch,
|
||||||
|
* permission flip, or backend restart can never get pinned in
|
||||||
|
* intermediate caches.
|
||||||
|
*
|
||||||
|
* No-op cost (one setHeader per request); applied per route group
|
||||||
|
* rather than globally so static assets + galleries keep their
|
||||||
|
* own caching strategy.
|
||||||
|
*/
|
||||||
|
|
||||||
|
function noStoreCache(req, res, next) {
|
||||||
|
// `no-store` is the strongest signal — no cache, no revalidation,
|
||||||
|
// no offline retention. Pair with `private` so any well-behaved
|
||||||
|
// intermediate proxy treats the response as user-specific.
|
||||||
|
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate, private');
|
||||||
|
res.setHeader('Pragma', 'no-cache'); // HTTP/1.0 fallback for older proxies
|
||||||
|
res.setHeader('Expires', '0');
|
||||||
|
next();
|
||||||
|
}
|
||||||
|
|
||||||
|
module.exports = { noStoreCache };
|
||||||
Reference in New Issue
Block a user