Skip to content

Commit

Permalink
preserve 401 errors from legacy es client (#71234) (#71475)
Browse files Browse the repository at this point in the history
* preserve 401 errors from legacy es client

* use exact import to resolve mocked import issue
  • Loading branch information
pgayvallet authored Jul 13, 2020
1 parent a46732a commit 1a869fb
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/core/server/elasticsearch/legacy/cluster_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
34 changes: 32 additions & 2 deletions src/core/server/http/integration_tests/core_services.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -352,7 +356,7 @@ describe('http service', () => {
});
});
});
describe('elasticsearch', () => {
describe('legacy elasticsearch client', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;
beforeEach(async () => {
root = kbnTestServer.createRoot({ plugins: { initialize: false } });
Expand Down Expand Up @@ -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');
});
});
});
5 changes: 5 additions & 0 deletions src/core/server/http/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
}
Expand Down

0 comments on commit 1a869fb

Please sign in to comment.