Skip to content

Commit

Permalink
preserve 401 errors from new es client (#71248)
Browse files Browse the repository at this point in the history
* intercept 401 error from new client in routing layer

* improvements

* lint

* fix mocked client construction due to 7.9-rc1 bump

* use default WWW-Authenticate value when not provided by ES 401
  • Loading branch information
pgayvallet authored Jul 21, 2020
1 parent 20c6d9f commit 517c34a
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 15 deletions.
82 changes: 82 additions & 0 deletions src/core/server/elasticsearch/client/errors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* 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 {
ResponseError,
ConnectionError,
ConfigurationError,
} from '@elastic/elasticsearch/lib/errors';
import { ApiResponse } from '@elastic/elasticsearch';
import { isResponseError, isUnauthorizedError } from './errors';

const createApiResponseError = ({
statusCode = 200,
headers = {},
body = {},
}: {
statusCode?: number;
headers?: Record<string, string>;
body?: Record<string, any>;
} = {}): ApiResponse => {
return {
body,
statusCode,
headers,
warnings: [],
meta: {} as any,
};
};

describe('isResponseError', () => {
it('returns `true` when the input is a `ResponseError`', () => {
expect(isResponseError(new ResponseError(createApiResponseError()))).toBe(true);
});

it('returns `false` when the input is not a `ResponseError`', () => {
expect(isResponseError(new Error('foo'))).toBe(false);
expect(isResponseError(new ConnectionError('error', createApiResponseError()))).toBe(false);
expect(isResponseError(new ConfigurationError('foo'))).toBe(false);
});
});

describe('isUnauthorizedError', () => {
it('returns true when the input is a `ResponseError` and statusCode === 401', () => {
expect(
isUnauthorizedError(new ResponseError(createApiResponseError({ statusCode: 401 })))
).toBe(true);
});

it('returns false when the input is a `ResponseError` and statusCode !== 401', () => {
expect(
isUnauthorizedError(new ResponseError(createApiResponseError({ statusCode: 200 })))
).toBe(false);
expect(
isUnauthorizedError(new ResponseError(createApiResponseError({ statusCode: 403 })))
).toBe(false);
expect(
isUnauthorizedError(new ResponseError(createApiResponseError({ statusCode: 500 })))
).toBe(false);
});

it('returns `false` when the input is not a `ResponseError`', () => {
expect(isUnauthorizedError(new Error('foo'))).toBe(false);
expect(isUnauthorizedError(new ConnectionError('error', createApiResponseError()))).toBe(false);
expect(isUnauthorizedError(new ConfigurationError('foo'))).toBe(false);
});
});
32 changes: 32 additions & 0 deletions src/core/server/elasticsearch/client/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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 { ResponseError } from '@elastic/elasticsearch/lib/errors';

export type UnauthorizedError = ResponseError & {
statusCode: 401;
};

export function isResponseError(error: any): error is ResponseError {
return Boolean(error.body && error.statusCode && error.headers);
}

export function isUnauthorizedError(error: any): error is UnauthorizedError {
return isResponseError(error) && error.statusCode === 401;
}
6 changes: 6 additions & 0 deletions src/core/server/elasticsearch/client/mocks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ describe('Mocked client', () => {
expectMocked(client.close);
});

it('used EventEmitter functions should be mocked', () => {
expectMocked(client.on);
expectMocked(client.off);
expectMocked(client.once);
});

it('`child` should be mocked and return a mocked Client', () => {
expectMocked(client.child);

Expand Down
15 changes: 11 additions & 4 deletions src/core/server/elasticsearch/client/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,20 @@ const createInternalClientMock = (): DeeplyMockedKeys<Client> => {

mockify(client, omittedProps);

client.transport = {
// client got some read-only (getter) properties
// so we need to extend it to override the getter-only props.
const mock: any = { ...client };

mock.transport = {
request: jest.fn(),
};
client.close = jest.fn().mockReturnValue(Promise.resolve());
client.child = jest.fn().mockImplementation(() => createInternalClientMock());
mock.close = jest.fn().mockReturnValue(Promise.resolve());
mock.child = jest.fn().mockImplementation(() => createInternalClientMock());
mock.on = jest.fn();
mock.off = jest.fn();
mock.once = jest.fn();

return (client as unknown) as DeeplyMockedKeys<Client>;
return (mock as unknown) as DeeplyMockedKeys<Client>;
};

export type ElasticSearchClientMock = DeeplyMockedKeys<ElasticsearchClient>;
Expand Down
17 changes: 14 additions & 3 deletions src/core/server/http/integration_tests/core_service.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
*/
import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock';

export const clusterClientMock = jest.fn();
export const clusterClientInstanceMock = elasticsearchServiceMock.createLegacyScopedClusterClient();
export const MockLegacyScopedClusterClient = jest.fn();
export const legacyClusterClientInstanceMock = elasticsearchServiceMock.createLegacyScopedClusterClient();
jest.doMock('../../elasticsearch/legacy/scoped_cluster_client', () => ({
LegacyScopedClusterClient: clusterClientMock.mockImplementation(() => clusterClientInstanceMock),
LegacyScopedClusterClient: MockLegacyScopedClusterClient.mockImplementation(
() => legacyClusterClientInstanceMock
),
}));

jest.doMock('elasticsearch', () => {
Expand All @@ -34,3 +36,12 @@ jest.doMock('elasticsearch', () => {
},
};
});

export const MockElasticsearchClient = jest.fn();
jest.doMock('@elastic/elasticsearch', () => {
const real = jest.requireActual('@elastic/elasticsearch');
return {
...real,
Client: MockElasticsearchClient,
};
});
117 changes: 111 additions & 6 deletions src/core/server/http/integration_tests/core_services.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,21 @@
* under the License.
*/

import { clusterClientMock, clusterClientInstanceMock } from './core_service.test.mocks';
import {
MockLegacyScopedClusterClient,
MockElasticsearchClient,
legacyClusterClientInstanceMock,
} from './core_service.test.mocks';

import Boom from 'boom';
import { Request } from 'hapi';
import { errors as esErrors } from 'elasticsearch';
import { LegacyElasticsearchErrorHelpers } from '../../elasticsearch/legacy';

import { elasticsearchClientMock } from '../../elasticsearch/client/mocks';
import { ResponseError } from '@elastic/elasticsearch/lib/errors';
import * as kbnTestServer from '../../../../test_utils/kbn_server';
import { InternalElasticsearchServiceStart } from '../../elasticsearch';

interface User {
id: string;
Expand All @@ -44,6 +51,17 @@ const cookieOptions = {
};

describe('http service', () => {
let esClient: ReturnType<typeof elasticsearchClientMock.createInternalClient>;

beforeEach(async () => {
esClient = elasticsearchClientMock.createInternalClient();
MockElasticsearchClient.mockImplementation(() => esClient);
}, 30000);

afterEach(async () => {
MockElasticsearchClient.mockClear();
});

describe('auth', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;
beforeEach(async () => {
Expand Down Expand Up @@ -200,7 +218,7 @@ describe('http service', () => {
}, 30000);

afterEach(async () => {
clusterClientMock.mockClear();
MockLegacyScopedClusterClient.mockClear();
await root.shutdown();
});

Expand Down Expand Up @@ -363,7 +381,7 @@ describe('http service', () => {
}, 30000);

afterEach(async () => {
clusterClientMock.mockClear();
MockLegacyScopedClusterClient.mockClear();
await root.shutdown();
});

Expand All @@ -386,7 +404,7 @@ describe('http service', () => {
await kbnTestServer.request.get(root, '/new-platform/').expect(200);

// client contains authHeaders for BWC with legacy platform.
const [client] = clusterClientMock.mock.calls;
const [client] = MockLegacyScopedClusterClient.mock.calls;
const [, , clientHeaders] = client;
expect(clientHeaders).toEqual(authHeaders);
});
Expand All @@ -410,7 +428,7 @@ describe('http service', () => {
.set('Authorization', authorizationHeader)
.expect(200);

const [client] = clusterClientMock.mock.calls;
const [client] = MockLegacyScopedClusterClient.mock.calls;
const [, , clientHeaders] = client;
expect(clientHeaders).toEqual({ authorization: authorizationHeader });
});
Expand All @@ -426,7 +444,7 @@ describe('http service', () => {
})
);

clusterClientInstanceMock.callAsCurrentUser.mockRejectedValue(authenticationError);
legacyClusterClientInstanceMock.callAsCurrentUser.mockRejectedValue(authenticationError);

const router = createRouter('/new-platform');
router.get({ path: '/', validate: false }, async (context, req, res) => {
Expand All @@ -441,4 +459,91 @@ describe('http service', () => {
expect(response.header['www-authenticate']).toEqual('authenticate header');
});
});

describe('elasticsearch client', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;

beforeEach(async () => {
root = kbnTestServer.createRoot({ plugins: { initialize: false } });
}, 30000);

afterEach(async () => {
MockElasticsearchClient.mockClear();
await root.shutdown();
});

it('forwards unauthorized errors from elasticsearch', async () => {
const { http } = await root.setup();
const { createRouter } = http;
// eslint-disable-next-line prefer-const
let elasticsearch: InternalElasticsearchServiceStart;

esClient.ping.mockImplementation(() =>
elasticsearchClientMock.createClientError(
new ResponseError({
statusCode: 401,
body: {
error: {
type: 'Unauthorized',
},
},
warnings: [],
headers: {
'WWW-Authenticate': 'content',
},
meta: {} as any,
})
)
);

const router = createRouter('/new-platform');
router.get({ path: '/', validate: false }, async (context, req, res) => {
await elasticsearch.client.asScoped(req).asInternalUser.ping();
return res.ok();
});

const coreStart = await root.start();
elasticsearch = coreStart.elasticsearch;

const { header } = await kbnTestServer.request.get(root, '/new-platform/').expect(401);

expect(header['www-authenticate']).toEqual('content');
});

it('uses a default value for `www-authenticate` header when ES 401 does not specify it', async () => {
const { http } = await root.setup();
const { createRouter } = http;
// eslint-disable-next-line prefer-const
let elasticsearch: InternalElasticsearchServiceStart;

esClient.ping.mockImplementation(() =>
elasticsearchClientMock.createClientError(
new ResponseError({
statusCode: 401,
body: {
error: {
type: 'Unauthorized',
},
},
warnings: [],
headers: {},
meta: {} as any,
})
)
);

const router = createRouter('/new-platform');
router.get({ path: '/', validate: false }, async (context, req, res) => {
await elasticsearch.client.asScoped(req).asInternalUser.ping();
return res.ok();
});

const coreStart = await root.start();
elasticsearch = coreStart.elasticsearch;

const { header } = await kbnTestServer.request.get(root, '/new-platform/').expect(401);

expect(header['www-authenticate']).toEqual('Basic realm="Authorization Required"');
});
});
});
Loading

0 comments on commit 517c34a

Please sign in to comment.