Kibana request headers (elastic#79218)

* Adding X-Kibana header for the legacy client requests * Specifying `X-Kibana: true` header for all requests coming from Kibana * Dev Tools now override the X-Kibana header to false * DevTools doesn't need to do anything, it's not using the ES client * Switching from `X-Kibana: true` to `User-Agent: Kibana` * Switching from `X-Kibana: true` to `User-Agent: Kibana` * Adding a constant * Starting to add unit tests... We currently allow end-users to set whatever headers they'd like to be forwarded to Elasticsearch with `elasticsearch.customHeaders` and `elasticsearch.requestHeadersWhitelist`. This is potentially problematic with us always specifying `User-Agent: kibana` as it could interfere with what our end-users have done... * Switching from user-agent to X-elastic-product-origin * Adding some tests * Adding and updating the legacy client unit tests T# * /s/KIBANA_HEADERS/DEFAULT_HEADERS and a deepFreeze * Adding comment for why `x-elastic-product-origin` exists
cjcenizal · Oct 6, 2020 · 1ad68fd · 1ad68fd
1 parent 9794188
commit 1ad68fd
Show file tree

Hide file tree

Showing 7 changed files with 470 additions and 309 deletions.
diff --git a/src/core/server/elasticsearch/client/client_config.test.ts b/src/core/server/elasticsearch/client/client_config.test.ts
@@ -19,6 +19,7 @@
 
 import { duration } from 'moment';
 import { ElasticsearchClientConfig, parseClientOptions } from './client_config';
+import { DEFAULT_HEADERS } from '../default_headers';
 
 const createConfig = (
   parts: Partial<ElasticsearchClientConfig> = {}
@@ -36,6 +37,18 @@ const createConfig = (
 };
 
 describe('parseClientOptions', () => {
+  it('includes headers designing the HTTP request as originating from Kibana by default', () => {
+    const config = createConfig({});
+
+    expect(parseClientOptions(config, false)).toEqual(
+      expect.objectContaining({
+        headers: {
+          ...DEFAULT_HEADERS,
+        },
+      })
+    );
+  });
+
   describe('basic options', () => {
     it('`customHeaders` option', () => {
       const config = createConfig({
@@ -48,13 +61,33 @@ describe('parseClientOptions', () => {
       expect(parseClientOptions(config, false)).toEqual(
         expect.objectContaining({
           headers: {
+            ...DEFAULT_HEADERS,
             foo: 'bar',
             hello: 'dolly',
           },
         })
       );
     });
 
+    it('`customHeaders` take precedence to default kibana headers', () => {
+      const customHeader = {
+        [Object.keys(DEFAULT_HEADERS)[0]]: 'foo',
+      };
+      const config = createConfig({
+        customHeaders: {
+          ...customHeader,
+        },
+      });
+
+      expect(parseClientOptions(config, false)).toEqual(
+        expect.objectContaining({
+          headers: {
+            ...customHeader,
+          },
+        })
+      );
+    });
+
     it('`keepAlive` option', () => {
       expect(parseClientOptions(createConfig({ keepAlive: true }), false)).toEqual(
         expect.objectContaining({ agent: { keepAlive: true } })

diff --git a/src/core/server/elasticsearch/client/client_config.ts b/src/core/server/elasticsearch/client/client_config.ts
@@ -22,6 +22,7 @@ import { URL } from 'url';
 import { Duration } from 'moment';
 import { ClientOptions, NodeOptions } from '@elastic/elasticsearch';
 import { ElasticsearchConfig } from '../elasticsearch_config';
+import { DEFAULT_HEADERS } from '../default_headers';
 
 /**
  * Configuration options to be used to create a {@link IClusterClient | cluster client} using the
@@ -61,7 +62,10 @@ export function parseClientOptions(
   const clientOptions: ClientOptions = {
     sniffOnStart: config.sniffOnStart,
     sniffOnConnectionFault: config.sniffOnConnectionFault,
-    headers: config.customHeaders,
+    headers: {
+      ...DEFAULT_HEADERS,
+      ...config.customHeaders,
+    },
   };
 
   if (config.pingTimeout != null) {

diff --git a/src/core/server/elasticsearch/client/cluster_client.test.ts b/src/core/server/elasticsearch/client/cluster_client.test.ts
@@ -24,6 +24,7 @@ import { GetAuthHeaders } from '../../http';
 import { elasticsearchClientMock } from './mocks';
 import { ClusterClient } from './cluster_client';
 import { ElasticsearchClientConfig } from './client_config';
+import { DEFAULT_HEADERS } from '../default_headers';
 
 const createConfig = (
   parts: Partial<ElasticsearchClientConfig> = {}
@@ -127,7 +128,7 @@ describe('ClusterClient', () => {
 
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
-        headers: { foo: 'bar', 'x-opaque-id': expect.any(String) },
+        headers: { ...DEFAULT_HEADERS, foo: 'bar', 'x-opaque-id': expect.any(String) },
       });
     });
 
@@ -147,7 +148,7 @@ describe('ClusterClient', () => {
 
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
-        headers: { authorization: 'auth', 'x-opaque-id': expect.any(String) },
+        headers: { ...DEFAULT_HEADERS, authorization: 'auth', 'x-opaque-id': expect.any(String) },
       });
     });
 
@@ -171,7 +172,7 @@ describe('ClusterClient', () => {
 
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
-        headers: { authorization: 'auth', 'x-opaque-id': expect.any(String) },
+        headers: { ...DEFAULT_HEADERS, authorization: 'auth', 'x-opaque-id': expect.any(String) },
       });
     });
 
@@ -193,6 +194,7 @@ describe('ClusterClient', () => {
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
         headers: {
+          ...DEFAULT_HEADERS,
           foo: 'bar',
           hello: 'dolly',
           'x-opaque-id': expect.any(String),
@@ -214,6 +216,7 @@ describe('ClusterClient', () => {
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
         headers: {
+          ...DEFAULT_HEADERS,
           'x-opaque-id': 'my-fake-id',
         },
       });
@@ -239,6 +242,7 @@ describe('ClusterClient', () => {
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
         headers: {
+          ...DEFAULT_HEADERS,
           foo: 'auth',
           hello: 'dolly',
           'x-opaque-id': expect.any(String),
@@ -266,13 +270,60 @@ describe('ClusterClient', () => {
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
         headers: {
+          ...DEFAULT_HEADERS,
           foo: 'request',
           hello: 'dolly',
           'x-opaque-id': expect.any(String),
         },
       });
     });
 
+    it('respect the precedence of config headers over default headers', () => {
+      const headerKey = Object.keys(DEFAULT_HEADERS)[0];
+      const config = createConfig({
+        customHeaders: {
+          [headerKey]: 'foo',
+        },
+      });
+      getAuthHeaders.mockReturnValue({});
+
+      const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
+      const request = httpServerMock.createKibanaRequest();
+
+      clusterClient.asScoped(request);
+
+      expect(scopedClient.child).toHaveBeenCalledTimes(1);
+      expect(scopedClient.child).toHaveBeenCalledWith({
+        headers: {
+          [headerKey]: 'foo',
+          'x-opaque-id': expect.any(String),
+        },
+      });
+    });
+
+    it('respect the precedence of request headers over default headers', () => {
+      const headerKey = Object.keys(DEFAULT_HEADERS)[0];
+      const config = createConfig({
+        requestHeadersWhitelist: [headerKey],
+      });
+      getAuthHeaders.mockReturnValue({});
+
+      const clusterClient = new ClusterClient(config, logger, getAuthHeaders);
+      const request = httpServerMock.createKibanaRequest({
+        headers: { [headerKey]: 'foo' },
+      });
+
+      clusterClient.asScoped(request);
+
+      expect(scopedClient.child).toHaveBeenCalledTimes(1);
+      expect(scopedClient.child).toHaveBeenCalledWith({
+        headers: {
+          [headerKey]: 'foo',
+          'x-opaque-id': expect.any(String),
+        },
+      });
+    });
+
     it('respect the precedence of x-opaque-id header over config headers', () => {
       const config = createConfig({
         customHeaders: {
@@ -292,6 +343,7 @@ describe('ClusterClient', () => {
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
         headers: {
+          ...DEFAULT_HEADERS,
           'x-opaque-id': 'from request',
         },
       });
@@ -315,7 +367,7 @@ describe('ClusterClient', () => {
 
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
-        headers: { authorization: 'auth' },
+        headers: { ...DEFAULT_HEADERS, authorization: 'auth' },
       });
     });
 
@@ -339,7 +391,7 @@ describe('ClusterClient', () => {
 
       expect(scopedClient.child).toHaveBeenCalledTimes(1);
       expect(scopedClient.child).toHaveBeenCalledWith({
-        headers: { foo: 'bar' },
+        headers: { ...DEFAULT_HEADERS, foo: 'bar' },
       });
     });
   });

diff --git a/src/core/server/elasticsearch/client/cluster_client.ts b/src/core/server/elasticsearch/client/cluster_client.ts
@@ -26,6 +26,7 @@ import { ElasticsearchClient } from './types';
 import { configureClient } from './configure_client';
 import { ElasticsearchClientConfig } from './client_config';
 import { ScopedClusterClient, IScopedClusterClient } from './scoped_cluster_client';
+import { DEFAULT_HEADERS } from '../default_headers';
 
 const noop = () => undefined;
 
@@ -108,6 +109,7 @@ export class ClusterClient implements ICustomClusterClient {
     }
 
     return {
+      ...DEFAULT_HEADERS,
       ...this.config.customHeaders,
       ...scopedHeaders,
     };

diff --git a/src/core/server/elasticsearch/default_headers.ts b/src/core/server/elasticsearch/default_headers.ts
@@ -0,0 +1,27 @@
+/*
+ * 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.
+ */
+
+import { deepFreeze } from '@kbn/std';
+
+export const DEFAULT_HEADERS = deepFreeze({
+  // Elasticsearch uses this to identify when a request is coming from Kibana, to allow Kibana to
+  // access system indices using the standard ES APIs without logging a warning. After migrating to
+  // use the new system index APIs, this header can be removed.
+  'x-elastic-product-origin': 'kibana',
+});