Skip to content

Commit

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

* preserve 401 errors from legacy es client

* use exact import to resolve mocked import issue
# Conflicts:
#	src/core/server/elasticsearch/legacy/cluster_client.test.ts
#	src/core/server/http/integration_tests/core_service.test.mocks.ts

* adapt for 7.8

* use specific import to avoid cyclic deps
  • Loading branch information
pgayvallet authored Jul 13, 2020
1 parent 68adcaf commit 8853066
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
*/
import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock';

export const clusterClientInstanceMock = elasticsearchServiceMock.createScopedClusterClient();
export const clusterClientMock = jest.fn();
jest.doMock('../../elasticsearch/scoped_cluster_client', () => ({
ScopedClusterClient: clusterClientMock.mockImplementation(function () {
return elasticsearchServiceMock.createScopedClusterClient();
return clusterClientInstanceMock;
}),
}));

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 { ElasticsearchErrorHelpers } from '../../elasticsearch/errors';

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 @@ -416,5 +420,31 @@ describe('http service', () => {
const [, , dataClientHeaders] = dataClient;
expect(dataClientHeaders).toEqual({ authorization: authorizationHeader });
});

it('forwards 401 errors returned from elasticsearch', async () => {
const { http } = await root.setup();
const { createRouter } = http;

const authenticationError = ElasticsearchErrorHelpers.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.adminClient.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 { ElasticsearchErrorHelpers } from '../../elasticsearch/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 (ElasticsearchErrorHelpers.isNotAuthorizedError(e)) {
return e;
}
return hapiResponseAdapter.toInternalError();
}
}
Expand Down

0 comments on commit 8853066

Please sign in to comment.