From eac0f8d98d5ec8dfcb0ae3d6a6176a9640a5a9ad Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 13 Jul 2020 18:03:49 +0200 Subject: [PATCH] preserve 401 errors from legacy es client (#71234) * preserve 401 errors from legacy es client * use exact import to resolve mocked import issue --- .../legacy/cluster_client.test.ts | 4 +-- .../core_service.test.mocks.ts | 5 ++- .../integration_tests/core_services.test.ts | 34 +++++++++++++++++-- src/core/server/http/router/router.ts | 5 +++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/core/server/elasticsearch/legacy/cluster_client.test.ts b/src/core/server/elasticsearch/legacy/cluster_client.test.ts index 2f0f80728c707..fd57d06e61eee 100644 --- a/src/core/server/elasticsearch/legacy/cluster_client.test.ts +++ b/src/core/server/elasticsearch/legacy/cluster_client.test.ts @@ -130,7 +130,7 @@ describe('#callAsInternalUser', () => { expect(mockEsClientInstance.security.authenticate).toHaveBeenLastCalledWith(mockParams); }); - test('does not wrap errors if `wrap401Errors` is not set', async () => { + test('does not wrap errors if `wrap401Errors` is set to `false`', async () => { const mockError = { message: 'some error' }; mockEsClientInstance.ping.mockRejectedValue(mockError); @@ -146,7 +146,7 @@ describe('#callAsInternalUser', () => { ).rejects.toBe(mockAuthenticationError); }); - test('wraps only 401 errors by default or when `wrap401Errors` is set', async () => { + test('wraps 401 errors when `wrap401Errors` is set to `true` or unspecified', async () => { const mockError = { message: 'some error' }; mockEsClientInstance.ping.mockRejectedValue(mockError); diff --git a/src/core/server/http/integration_tests/core_service.test.mocks.ts b/src/core/server/http/integration_tests/core_service.test.mocks.ts index f7ebd18b9c488..c23724b7d332f 100644 --- a/src/core/server/http/integration_tests/core_service.test.mocks.ts +++ b/src/core/server/http/integration_tests/core_service.test.mocks.ts @@ -19,10 +19,9 @@ import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock'; export const clusterClientMock = jest.fn(); +export const clusterClientInstanceMock = elasticsearchServiceMock.createLegacyScopedClusterClient(); jest.doMock('../../elasticsearch/legacy/scoped_cluster_client', () => ({ - LegacyScopedClusterClient: clusterClientMock.mockImplementation(function () { - return elasticsearchServiceMock.createLegacyScopedClusterClient(); - }), + LegacyScopedClusterClient: clusterClientMock.mockImplementation(() => clusterClientInstanceMock), })); jest.doMock('elasticsearch', () => { diff --git a/src/core/server/http/integration_tests/core_services.test.ts b/src/core/server/http/integration_tests/core_services.test.ts index ba39effa77016..0ee53a04d9f87 100644 --- a/src/core/server/http/integration_tests/core_services.test.ts +++ b/src/core/server/http/integration_tests/core_services.test.ts @@ -16,9 +16,13 @@ * specific language governing permissions and limitations * under the License. */ + +import { clusterClientMock, clusterClientInstanceMock } from './core_service.test.mocks'; + import Boom from 'boom'; import { Request } from 'hapi'; -import { clusterClientMock } from './core_service.test.mocks'; +import { errors as esErrors } from 'elasticsearch'; +import { LegacyElasticsearchErrorHelpers } from '../../elasticsearch/legacy'; import * as kbnTestServer from '../../../../test_utils/kbn_server'; @@ -352,7 +356,7 @@ describe('http service', () => { }); }); }); - describe('elasticsearch', () => { + describe('legacy elasticsearch client', () => { let root: ReturnType; beforeEach(async () => { root = kbnTestServer.createRoot({ plugins: { initialize: false } }); @@ -410,5 +414,31 @@ describe('http service', () => { const [, , clientHeaders] = client; expect(clientHeaders).toEqual({ authorization: authorizationHeader }); }); + + it('forwards 401 errors returned from elasticsearch', async () => { + const { http } = await root.setup(); + const { createRouter } = http; + + const authenticationError = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError( + new (esErrors.AuthenticationException as any)('Authentication Exception', { + body: { error: { header: { 'WWW-Authenticate': 'authenticate header' } } }, + statusCode: 401, + }) + ); + + clusterClientInstanceMock.callAsCurrentUser.mockRejectedValue(authenticationError); + + const router = createRouter('/new-platform'); + router.get({ path: '/', validate: false }, async (context, req, res) => { + await context.core.elasticsearch.legacy.client.callAsCurrentUser('ping'); + return res.ok(); + }); + + await root.start(); + + const response = await kbnTestServer.request.get(root, '/new-platform/').expect(401); + + expect(response.header['www-authenticate']).toEqual('authenticate header'); + }); }); }); diff --git a/src/core/server/http/router/router.ts b/src/core/server/http/router/router.ts index 69402a74eda5f..35eec746163ce 100644 --- a/src/core/server/http/router/router.ts +++ b/src/core/server/http/router/router.ts @@ -22,6 +22,7 @@ import Boom from 'boom'; import { isConfigSchema } from '@kbn/config-schema'; import { Logger } from '../../logging'; +import { LegacyElasticsearchErrorHelpers } from '../../elasticsearch/legacy/errors'; import { KibanaRequest } from './request'; import { KibanaResponseFactory, kibanaResponseFactory, IKibanaResponse } from './response'; import { RouteConfig, RouteConfigOptions, RouteMethod, validBodyOutput } from './route'; @@ -263,6 +264,10 @@ export class Router implements IRouter { return hapiResponseAdapter.handle(kibanaResponse); } catch (e) { this.log.error(e); + // forward 401 (boom) error from ES + if (LegacyElasticsearchErrorHelpers.isNotAuthorizedError(e)) { + return e; + } return hapiResponseAdapter.toInternalError(); } }