Skip to content

Commit

Permalink
Update cache-control header (#62014)
Browse files Browse the repository at this point in the history
* update cache-control header

* update tests

* update test run config

* remove custom cache-control header for authentication resources

* address test flakiness

* address PR feedback

* revert changes to endpoint test

* revert changes for real this time

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
legrego and elasticmachine authored Apr 6, 2020
1 parent d26372c commit 5003179
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/core/server/http/http_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
host: config.host,
port: config.port,
routes: {
cache: {
privacy: 'private',
otherwise: 'private, no-cache, no-store, must-revalidate',
},
cors: config.cors,
payload: {
maxBytes: config.maxPayload.getValueInBytes(),
Expand Down
32 changes: 32 additions & 0 deletions src/core/server/http/integration_tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,38 @@ describe('Options', () => {
});
});

describe('Cache-Control', () => {
it('does not allow responses to be cached by default', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');

router.get({ path: '/', validate: false, options: {} }, (context, req, res) => res.ok());
await server.start();

await supertest(innerServer.listener)
.get('/')
.expect('Cache-Control', 'private, no-cache, no-store, must-revalidate');
});

it('allows individual responses override the default cache-control header', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
const router = createRouter('/');

router.get({ path: '/', validate: false, options: {} }, (context, req, res) =>
res.ok({
headers: {
'Cache-Control': 'public, max-age=1200',
},
})
);
await server.start();

await supertest(innerServer.listener)
.get('/')
.expect('Cache-Control', 'public, max-age=1200');
});
});

describe('Handler', () => {
it("Doesn't expose error details if handler throws", async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
Expand Down
1 change: 0 additions & 1 deletion src/plugins/bfetch/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ export class BfetchServerPlugin
'Content-Type': 'application/x-ndjson',
Connection: 'keep-alive',
'Transfer-Encoding': 'chunked',
'Cache-Control': 'no-cache',
};
return response.ok({
headers,
Expand Down
2 changes: 2 additions & 0 deletions tasks/config/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ module.exports = function(grunt) {
'test/server_integration/http/ssl/config.js',
'--config',
'test/server_integration/http/ssl_redirect/config.js',
'--config',
'test/server_integration/http/cache/config.js',
'--bail',
'--debug',
'--kibana-install-dir',
Expand Down
33 changes: 33 additions & 0 deletions test/server_integration/http/cache/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export default async function({ readConfigFile }) {
const httpConfig = await readConfigFile(require.resolve('../../config'));

return {
testFiles: [require.resolve('./')],
services: httpConfig.get('services'),
servers: httpConfig.get('servers'),
junit: {
reportName: 'Http Cache-Control Integration Tests',
},
esTestCluster: httpConfig.get('esTestCluster'),
kbnTestServer: httpConfig.get('kbnTestServer'),
};
}
46 changes: 46 additions & 0 deletions test/server_integration/http/cache/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

// eslint-disable-next-line import/no-default-export
export default function({ getService }) {
const supertest = getService('supertest');

describe('kibana server cache-control', () => {
it('properly marks responses as private, with directives to disable caching', async () => {
await supertest
.get('/api/status')
.expect('Cache-Control', 'private, no-cache, no-store, must-revalidate')
.expect(200);
});

it('allows translation bundles to be cached', async () => {
await supertest
.get('/translations/en.json')
.expect('Cache-Control', 'must-revalidate')
.expect(200);
});

it('allows the bootstrap bundles to be cached', async () => {
await supertest
.get('/bundles/app/any-old-id-works/bootstrap.js')
.expect('Cache-Control', 'must-revalidate')
.expect(200);
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export function createCustomResourceResponse(body: string, contentType: string,
body,
headers: {
'content-type': contentType,
'cache-control': 'private, no-cache, no-store',
'content-security-policy': cspHeader,
},
statusCode: 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ export default function({ getService }: FtrProviderContext) {

// Check that proxy page is returned with proper headers.
expect(response.headers['content-type']).to.be('text/html; charset=utf-8');
expect(response.headers['cache-control']).to.be('private, no-cache, no-store');
expect(response.headers['cache-control']).to.be(
'private, no-cache, no-store, must-revalidate'
);
expect(response.headers['content-security-policy']).to.be(
`script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`
);
Expand Down
4 changes: 3 additions & 1 deletion x-pack/test/saml_api_integration/apis/security/saml_login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ export default function({ getService }: FtrProviderContext) {

// Check that proxy page is returned with proper headers.
expect(response.headers['content-type']).to.be('text/html; charset=utf-8');
expect(response.headers['cache-control']).to.be('private, no-cache, no-store');
expect(response.headers['cache-control']).to.be(
'private, no-cache, no-store, must-revalidate'
);
expect(response.headers['content-security-policy']).to.be(
`script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`
);
Expand Down

0 comments on commit 5003179

Please sign in to comment.