Migrate Console to use Node http instead of Hapi to support GET reque…

…sts with bodies (elastic#46200) * Cleaned up use of es.send API - Converted Elasticsearch proxy config to TS (now we can see the types with https.AgentOptions) - Wrap request in util.promisify and refactor - Use 'url' lib for parsing URLs - Remove rejectUnauthorized from proxy_route.js (this is a TLS setting handled in agent setup) * Retained original proxying behavior * Re-enable support for setting rejectUnauthorized via proxy config settings * Updated tests.
XavierM · Sep 30, 2019 · 3316d44 · 3316d44
1 parent e27b65f
commit 3316d44
Show file tree

Hide file tree

Showing 11 changed files with 272 additions and 105 deletions.
diff --git a/src/legacy/core_plugins/console/public/quarantined/src/es.js b/src/legacy/core_plugins/console/public/quarantined/src/es.js
@@ -35,11 +35,6 @@ export function getContentType(body) {
 export function send(method, path, data) {
   const wrappedDfd = $.Deferred();  // eslint-disable-line new-cap
 
-  const isGetRequest = /^get$/i.test(method);
-  if (data && isGetRequest) {
-    method = 'POST';
-  }
-
   const options = {
     url: '../api/console/proxy?' + formatQueryString({ path, method }),
     data,
@@ -50,7 +45,6 @@ export function send(method, path, data) {
     dataType: 'text', // disable automatic guessing
   };
 
-
   $.ajax(options).then(
     function (data, textStatus, jqXHR) {
       wrappedDfd.resolveWith(this, [data, textStatus, jqXHR]);

diff --git a/src/legacy/core_plugins/console/public/quarantined/src/mappings.js b/src/legacy/core_plugins/console/public/quarantined/src/mappings.js
@@ -267,7 +267,7 @@ function retrieveSettings(settingsKey, settingsToRetrieve) {
 
   // Fetch autocomplete info if setting is set to true, and if user has made changes.
   if (currentSettings[settingsKey] && settingsToRetrieve[settingsKey]) {
-    return es.send('GET', settingKeyToPathMap[settingsKey], null, null, true);
+    return es.send('GET', settingKeyToPathMap[settingsKey], null);
   } else {
     const settingsPromise = new $.Deferred();
     // If a user has saved settings, but a field remains checked and unchanged, no need to make changes

diff --git a/src/legacy/core_plugins/console/server/__tests__/proxy_route/body.js b/src/legacy/core_plugins/console/server/__tests__/proxy_route/body.js
@@ -18,23 +18,21 @@
  */
 
 import sinon from 'sinon';
-import Wreck from '@hapi/wreck';
 import expect from '@kbn/expect';
 import { Server } from 'hapi';
-
+import { createResponseStub } from './stubs';
 import { createProxyRoute } from '../../';
-
-import { createWreckResponseStub } from './stubs';
+import * as requestModule from '../../request';
 
 describe('Console Proxy Route', () => {
   const sandbox = sinon.createSandbox();
   const teardowns = [];
   let request;
 
+
   beforeEach(() => {
     request = async (method, path, response) => {
-      sandbox.stub(Wreck, 'request').callsFake(createWreckResponseStub(response));
-
+      sandbox.stub(requestModule, 'sendRequest').callsFake(createResponseStub(response));
       const server = new Server();
       server.route(
         createProxyRoute({

diff --git a/src/legacy/core_plugins/console/server/__tests__/proxy_route/headers.js b/src/legacy/core_plugins/console/server/__tests__/proxy_route/headers.js
@@ -20,21 +20,21 @@
 import { request } from 'http';
 
 import sinon from 'sinon';
-import Wreck from '@hapi/wreck';
 import expect from '@kbn/expect';
 import { Server } from 'hapi';
+import * as requestModule from '../../request';
 
 import { createProxyRoute } from '../../';
 
-import { createWreckResponseStub } from './stubs';
+import { createResponseStub } from './stubs';
 
 describe('Console Proxy Route', () => {
   const sandbox = sinon.createSandbox();
   const teardowns = [];
   let setup;
 
   beforeEach(() => {
-    sandbox.stub(Wreck, 'request').callsFake(createWreckResponseStub());
+    sandbox.stub(requestModule, 'sendRequest').callsFake(createResponseStub());
 
     setup = () => {
       const server = new Server();
@@ -77,8 +77,8 @@ describe('Console Proxy Route', () => {
 
       resp.destroy();
 
-      sinon.assert.calledOnce(Wreck.request);
-      const { headers } = Wreck.request.getCall(0).args[2];
+      sinon.assert.calledOnce(requestModule.sendRequest);
+      const { headers } = requestModule.sendRequest.getCall(0).args[0];
       expect(headers)
         .to.have.property('x-forwarded-for')
         .and.not.be('');

diff --git a/src/legacy/core_plugins/console/server/__tests__/proxy_route/params.js b/src/legacy/core_plugins/console/server/__tests__/proxy_route/params.js
@@ -20,21 +20,21 @@
 import { Agent } from 'http';
 
 import sinon from 'sinon';
-import Wreck from '@hapi/wreck';
+import * as requestModule from '../../request';
 import expect from '@kbn/expect';
 import { Server } from 'hapi';
 
 import { createProxyRoute } from '../../';
 
-import { createWreckResponseStub } from './stubs';
+import { createResponseStub } from './stubs';
 
 describe('Console Proxy Route', () => {
   const sandbox = sinon.createSandbox();
   const teardowns = [];
   let setup;
 
   beforeEach(() => {
-    sandbox.stub(Wreck, 'request').callsFake(createWreckResponseStub());
+    sandbox.stub(requestModule, 'sendRequest').callsFake(createResponseStub());
 
     setup = () => {
       const server = new Server();
@@ -72,6 +72,7 @@ describe('Console Proxy Route', () => {
           const { server } = setup();
           server.route(
             createProxyRoute({
+              baseUrl: 'http://localhost:9200',
               pathFilters: [/^\/foo\//, /^\/bar\//],
             })
           );
@@ -82,14 +83,15 @@ describe('Console Proxy Route', () => {
           });
 
           expect(statusCode).to.be(200);
-          sinon.assert.calledOnce(Wreck.request);
+          sinon.assert.calledOnce(requestModule.sendRequest);
         });
       });
       describe('all match', () => {
         it('allows the request', async () => {
           const { server } = setup();
           server.route(
             createProxyRoute({
+              baseUrl: 'http://localhost:9200',
               pathFilters: [/^\/foo\//, /^\/bar\//],
             })
           );
@@ -100,7 +102,7 @@ describe('Console Proxy Route', () => {
           });
 
           expect(statusCode).to.be(200);
-          sinon.assert.calledOnce(Wreck.request);
+          sinon.assert.calledOnce(requestModule.sendRequest);
         });
       });
     });
@@ -111,7 +113,7 @@ describe('Console Proxy Route', () => {
 
         const getConfigForReq = sinon.stub().returns({});
 
-        server.route(createProxyRoute({ getConfigForReq }));
+        server.route(createProxyRoute({ baseUrl: 'http://localhost:9200', getConfigForReq }));
         await server.inject({
           method: 'POST',
           url: '/api/console/proxy?method=HEAD&path=/index/type/id',
@@ -124,10 +126,10 @@ describe('Console Proxy Route', () => {
         expect(args[0])
           .to.have.property('query')
           .eql({ method: 'HEAD', path: '/index/type/id' });
-        expect(args[1]).to.be('/index/type/id?pretty');
+        expect(args[1]).to.be('http://localhost:9200/index/type/id?pretty=true');
       });
 
-      it('sends the returned timeout, rejectUnauthorized, agent, and base headers to Wreck', async () => {
+      it('sends the returned timeout, agent, and base headers to request', async () => {
         const { server } = setup();
 
         const timeout = Math.round(Math.random() * 10000);
@@ -140,11 +142,12 @@ describe('Console Proxy Route', () => {
 
         server.route(
           createProxyRoute({
+            baseUrl: 'http://localhost:9200',
             getConfigForReq: () => ({
               timeout,
               agent,
-              rejectUnauthorized,
               headers,
+              rejectUnauthorized,
             }),
           })
         );
@@ -154,8 +157,8 @@ describe('Console Proxy Route', () => {
           url: '/api/console/proxy?method=HEAD&path=/index/type/id',
         });
 
-        sinon.assert.calledOnce(Wreck.request);
-        const opts = Wreck.request.getCall(0).args[2];
+        sinon.assert.calledOnce(requestModule.sendRequest);
+        const opts = requestModule.sendRequest.getCall(0).args[0];
         expect(opts).to.have.property('timeout', timeout);
         expect(opts).to.have.property('agent', agent);
         expect(opts).to.have.property('rejectUnauthorized', rejectUnauthorized);

diff --git a/src/legacy/core_plugins/console/server/__tests__/proxy_route/query_string.js b/src/legacy/core_plugins/console/server/__tests__/proxy_route/query_string.js
@@ -18,21 +18,21 @@
  */
 
 import sinon from 'sinon';
-import Wreck from '@hapi/wreck';
+import * as requestModule from '../../request';
 import expect from '@kbn/expect';
 import { Server } from 'hapi';
 
 import { createProxyRoute } from '../../';
 
-import { createWreckResponseStub } from './stubs';
+import { createResponseStub } from './stubs';
 
 describe('Console Proxy Route', () => {
   const sandbox = sinon.createSandbox();
   const teardowns = [];
   let request;
 
   beforeEach(() => {
-    sandbox.stub(Wreck, 'request').callsFake(createWreckResponseStub());
+    sandbox.stub(requestModule, 'sendRequest').callsFake(createResponseStub());
 
     request = async (method, path) => {
       const server = new Server();
@@ -64,40 +64,40 @@ describe('Console Proxy Route', () => {
       describe('contains full url', () => {
         it('treats the url as a path', async () => {
           await request('GET', 'http://evil.com/test');
-          sinon.assert.calledOnce(Wreck.request);
-          const args = Wreck.request.getCall(0).args;
-          expect(args[1]).to.be('http://localhost:9200/http://evil.com/test?pretty');
+          sinon.assert.calledOnce(requestModule.sendRequest);
+          const args = requestModule.sendRequest.getCall(0).args;
+          expect(args[0].uri.href).to.be('http://localhost:9200/http://evil.com/test?pretty=true');
         });
       });
       describe('is missing', () => {
         it('returns a 400 error', async () => {
           const { statusCode } = await request('GET', undefined);
           expect(statusCode).to.be(400);
-          sinon.assert.notCalled(Wreck.request);
+          sinon.assert.notCalled(requestModule.sendRequest);
         });
       });
       describe('is empty', () => {
         it('returns a 400 error', async () => {
           const { statusCode } = await request('GET', '');
           expect(statusCode).to.be(400);
-          sinon.assert.notCalled(Wreck.request);
+          sinon.assert.notCalled(requestModule.sendRequest);
         });
       });
       describe('starts with a slash', () => {
         it('combines well with the base url', async () => {
           await request('GET', '/index/type/id');
-          sinon.assert.calledOnce(Wreck.request);
-          expect(Wreck.request.getCall(0).args[1]).to.be(
-            'http://localhost:9200/index/type/id?pretty'
+          sinon.assert.calledOnce(requestModule.sendRequest);
+          expect(requestModule.sendRequest.getCall(0).args[0].uri.href).to.be(
+            'http://localhost:9200/index/type/id?pretty=true'
           );
         });
       });
       describe(`doesn't start with a slash`, () => {
         it('combines well with the base url', async () => {
           await request('GET', 'index/type/id');
-          sinon.assert.calledOnce(Wreck.request);
-          expect(Wreck.request.getCall(0).args[1]).to.be(
-            'http://localhost:9200/index/type/id?pretty'
+          sinon.assert.calledOnce(requestModule.sendRequest);
+          expect(requestModule.sendRequest.getCall(0).args[0].uri.href).to.be(
+            'http://localhost:9200/index/type/id?pretty=true'
           );
         });
       });
@@ -107,29 +107,29 @@ describe('Console Proxy Route', () => {
         it('returns a 400 error', async () => {
           const { statusCode } = await request(null, '/');
           expect(statusCode).to.be(400);
-          sinon.assert.notCalled(Wreck.request);
+          sinon.assert.notCalled(requestModule.sendRequest);
         });
       });
       describe('is empty', () => {
         it('returns a 400 error', async () => {
           const { statusCode } = await request('', '/');
           expect(statusCode).to.be(400);
-          sinon.assert.notCalled(Wreck.request);
+          sinon.assert.notCalled(requestModule.sendRequest);
         });
       });
       describe('is an invalid http method', () => {
         it('returns a 400 error', async () => {
           const { statusCode } = await request('foo', '/');
           expect(statusCode).to.be(400);
-          sinon.assert.notCalled(Wreck.request);
+          sinon.assert.notCalled(requestModule.sendRequest);
         });
       });
       describe('is mixed case', () => {
         it('sends a request with the exact method', async () => {
           const { statusCode } = await request('HeAd', '/');
           expect(statusCode).to.be(200);
-          sinon.assert.calledOnce(Wreck.request);
-          expect(Wreck.request.getCall(0).args[0]).to.be('HeAd');
+          sinon.assert.calledOnce(requestModule.sendRequest);
+          expect(requestModule.sendRequest.getCall(0).args[0].method).to.be('HeAd');
         });
       });
     });

diff --git a/src/legacy/core_plugins/console/server/__tests__/proxy_route/stubs.js b/src/legacy/core_plugins/console/server/__tests__/proxy_route/stubs.js
@@ -19,7 +19,7 @@
 
 import { Readable } from 'stream';
 
-export function createWreckResponseStub(response) {
+export function createResponseStub(response) {
   return async () => {
     const resp = new Readable({
       read() {

diff --git a/...sole/server/elasticsearch_proxy_config.js → ...sole/server/elasticsearch_proxy_config.ts b/...sole/server/elasticsearch_proxy_config.js → ...sole/server/elasticsearch_proxy_config.ts
@@ -23,13 +23,13 @@ import http from 'http';
 import https from 'https';
 import url from 'url';
 
-const readFile = (file) => readFileSync(file, 'utf8');
+const readFile = (file: string) => readFileSync(file, 'utf8');
 
-const createAgent = (legacyConfig) => {
+const createAgent = (legacyConfig: any) => {
   const target = url.parse(_.head(legacyConfig.hosts));
-  if (!/^https/.test(target.protocol)) return new http.Agent();
+  if (!/^https/.test(target.protocol || '')) return new http.Agent();
 
-  const agentOptions = {};
+  const agentOptions: https.AgentOptions = {};
 
   const verificationMode = legacyConfig.ssl && legacyConfig.ssl.verificationMode;
   switch (verificationMode) {
@@ -40,7 +40,7 @@ const createAgent = (legacyConfig) => {
       agentOptions.rejectUnauthorized = true;
 
       // by default, NodeJS is checking the server identify
-      agentOptions.checkServerIdentity = _.noop;
+      agentOptions.checkServerIdentity = _.noop as any;
       break;
     case 'full':
       agentOptions.rejectUnauthorized = true;
@@ -49,8 +49,11 @@ const createAgent = (legacyConfig) => {
       throw new Error(`Unknown ssl verificationMode: ${verificationMode}`);
   }
 
-  if (legacyConfig.ssl && Array.isArray(legacyConfig.ssl.certificateAuthorities)
-      && legacyConfig.ssl.certificateAuthorities.length > 0) {
+  if (
+    legacyConfig.ssl &&
+    Array.isArray(legacyConfig.ssl.certificateAuthorities) &&
+    legacyConfig.ssl.certificateAuthorities.length > 0
+  ) {
     agentOptions.ca = legacyConfig.ssl.certificateAuthorities.map(readFile);
   }
 
@@ -68,9 +71,9 @@ const createAgent = (legacyConfig) => {
   return new https.Agent(agentOptions);
 };
 
-export const getElasticsearchProxyConfig = (legacyConfig) => {
+export const getElasticsearchProxyConfig = (legacyConfig: any) => {
   return {
     timeout: legacyConfig.requestTimeout.asMilliseconds(),
-    agent: createAgent(legacyConfig)
+    agent: createAgent(legacyConfig),
   };
 };