Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] preserve 401 errors from legacy es client (#71234) #71475

Merged
merged 1 commit into from
Jul 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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