From 20eefbcc76616c97b214dd886ab99fa303b4b17d Mon Sep 17 00:00:00 2001 From: Thom Heymann Date: Thu, 20 Aug 2020 14:55:46 +0100 Subject: [PATCH] rebase fixes --- .../audit_trail/audit_trail_service.test.ts | 9 +- .../elasticsearch_service.test.ts | 1 - .../legacy/cluster_client.test.ts | 143 +++--------------- .../elasticsearch/legacy/cluster_client.ts | 2 +- .../legacy/scoped_cluster_client.test.ts | 45 ------ 5 files changed, 28 insertions(+), 172 deletions(-) diff --git a/src/core/server/audit_trail/audit_trail_service.test.ts b/src/core/server/audit_trail/audit_trail_service.test.ts index d8dbb5211f2a2..1deba4cacc831 100644 --- a/src/core/server/audit_trail/audit_trail_service.test.ts +++ b/src/core/server/audit_trail/audit_trail_service.test.ts @@ -75,13 +75,10 @@ describe('AuditTrailService', () => { const { asScoped } = auditTrail.start(); const kibanaRequest = httpServerMock.createKibanaRequest(); const auditor = asScoped(kibanaRequest); - const message = { - type: 'foo', - message: 'bar', - }; - auditor.add(() => message); + const eventDecorator = jest.fn(); + auditor.add(eventDecorator); - expect(addEventMock).toHaveBeenLastCalledWith(message); + expect(addEventMock).toHaveBeenLastCalledWith(eventDecorator); }); describe('return the same auditor instance for the same KibanaRequest', () => { diff --git a/src/core/server/elasticsearch/elasticsearch_service.test.ts b/src/core/server/elasticsearch/elasticsearch_service.test.ts index 49f5c8dd98790..41e82bf367f5a 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.test.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.test.ts @@ -113,7 +113,6 @@ describe('#setup', () => { expect(MockLegacyClusterClient).toHaveBeenCalledWith( expect.objectContaining(customConfig), expect.objectContaining({ context: ['elasticsearch', 'some-custom-type'] }), - expect.any(Function), expect.any(Function) ); }); diff --git a/src/core/server/elasticsearch/legacy/cluster_client.test.ts b/src/core/server/elasticsearch/legacy/cluster_client.test.ts index 73d941053e84b..13d6e72063e71 100644 --- a/src/core/server/elasticsearch/legacy/cluster_client.test.ts +++ b/src/core/server/elasticsearch/legacy/cluster_client.test.ts @@ -27,7 +27,6 @@ import { import { errors } from 'elasticsearch'; import { get } from 'lodash'; -import { auditTrailServiceMock } from '../../audit_trail/audit_trail_service.mock'; import { Logger } from '../../logging'; import { loggingSystemMock } from '../../logging/logging_system.mock'; import { httpServerMock } from '../../http/http_server.mocks'; @@ -43,11 +42,7 @@ test('#constructor creates client with parsed config', () => { const mockEsConfig = { apiVersion: 'es-version' } as any; const mockLogger = logger.get(); - const clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + const clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); expect(clusterClient).toBeDefined(); expect(mockParseElasticsearchClientConfig).toHaveBeenCalledTimes(1); @@ -73,11 +68,7 @@ describe('#callAsInternalUser', () => { }; MockClient.mockImplementation(() => mockEsClientInstance); - clusterClient = new LegacyClusterClient( - { apiVersion: 'es-version' } as any, - logger.get(), - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient({ apiVersion: 'es-version' } as any, logger.get()); }); test('fails if cluster client is closed', async () => { @@ -246,11 +237,7 @@ describe('#asScoped', () => { requestHeadersWhitelist: ['one', 'two'], } as any; - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); jest.clearAllMocks(); }); @@ -285,11 +272,7 @@ describe('#asScoped', () => { test('properly configures `ignoreCertAndKey` for various configurations', () => { // Config without SSL. - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); mockParseElasticsearchClientConfig.mockClear(); clusterClient.asScoped(httpServerMock.createRawRequest({ headers: { one: '1' } })); @@ -302,11 +285,7 @@ describe('#asScoped', () => { // Config ssl.alwaysPresentCertificate === false mockEsConfig = { ...mockEsConfig, ssl: { alwaysPresentCertificate: false } } as any; - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); mockParseElasticsearchClientConfig.mockClear(); clusterClient.asScoped(httpServerMock.createRawRequest({ headers: { one: '1' } })); @@ -319,11 +298,7 @@ describe('#asScoped', () => { // Config ssl.alwaysPresentCertificate === true mockEsConfig = { ...mockEsConfig, ssl: { alwaysPresentCertificate: true } } as any; - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); mockParseElasticsearchClientConfig.mockClear(); clusterClient.asScoped(httpServerMock.createRawRequest({ headers: { one: '1' } })); @@ -344,8 +319,7 @@ describe('#asScoped', () => { expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { one: '1', two: '2' }, - expect.any(Object) + { one: '1', two: '2' } ); }); @@ -358,8 +332,7 @@ describe('#asScoped', () => { expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { 'x-opaque-id': 'alpha' }, - expect.any(Object) + { 'x-opaque-id': 'alpha' } ); }); @@ -381,142 +354,75 @@ describe('#asScoped', () => { }); test('does not fail when scope to not defined request', async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); clusterClient.asScoped(); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - {}, - undefined + {} ); }); test('does not fail when scope to a request without headers', async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); clusterClient.asScoped({} as any); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - {}, - undefined + {} ); }); test('calls getAuthHeaders and filters results for a real request', async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory, - () => ({ - one: '1', - three: '3', - }) - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, () => ({ + one: '1', + three: '3', + })); clusterClient.asScoped(httpServerMock.createRawRequest({ headers: { two: '2' } })); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { one: '1', two: '2' }, - expect.any(Object) + { one: '1', two: '2' } ); }); test('getAuthHeaders results rewrite extends a request headers', async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory, - () => ({ one: 'foo' }) - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, () => ({ one: 'foo' })); clusterClient.asScoped(httpServerMock.createRawRequest({ headers: { one: '1', two: '2' } })); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { one: 'foo', two: '2' }, - expect.any(Object) + { one: 'foo', two: '2' } ); }); test("doesn't call getAuthHeaders for a fake request", async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory, - () => ({}) - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, () => ({})); clusterClient.asScoped({ headers: { one: 'foo' } }); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { one: 'foo' }, - undefined + { one: 'foo' } ); }); test('filters a fake request headers', async () => { - clusterClient = new LegacyClusterClient( - mockEsConfig, - mockLogger, - auditTrailServiceMock.createAuditorFactory - ); + clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger); clusterClient.asScoped({ headers: { one: '1', two: '2', three: '3' } }); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); expect(MockScopedClusterClient).toHaveBeenCalledWith( expect.any(Function), expect.any(Function), - { one: '1', two: '2' }, - undefined + { one: '1', two: '2' } ); }); - - describe('Auditor', () => { - it('creates Auditor for KibanaRequest', async () => { - const auditor = auditTrailServiceMock.createAuditor(); - const auditorFactory = auditTrailServiceMock.createAuditorFactory(); - auditorFactory.asScoped.mockReturnValue(auditor); - clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, () => auditorFactory); - clusterClient.asScoped(httpServerMock.createKibanaRequest()); - - expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); - expect(MockScopedClusterClient).toHaveBeenCalledWith( - expect.any(Function), - expect.any(Function), - expect.objectContaining({ 'x-opaque-id': expect.any(String) }), - auditor - ); - }); - - it("doesn't create Auditor for a fake request", async () => { - const getAuthHeaders = jest.fn(); - clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, getAuthHeaders); - clusterClient.asScoped({ headers: { one: '1', two: '2', three: '3' } }); - - expect(getAuthHeaders).not.toHaveBeenCalled(); - }); - - it("doesn't create Auditor when no request passed", async () => { - const getAuthHeaders = jest.fn(); - clusterClient = new LegacyClusterClient(mockEsConfig, mockLogger, getAuthHeaders); - clusterClient.asScoped(); - - expect(getAuthHeaders).not.toHaveBeenCalled(); - }); - }); }); describe('#close', () => { @@ -534,8 +440,7 @@ describe('#close', () => { clusterClient = new LegacyClusterClient( { apiVersion: 'es-version', requestHeadersWhitelist: [] } as any, - logger.get(), - auditTrailServiceMock.createAuditorFactory + logger.get() ); }); diff --git a/src/core/server/elasticsearch/legacy/cluster_client.ts b/src/core/server/elasticsearch/legacy/cluster_client.ts index 83937e4b59ecc..00417e3bef4f4 100644 --- a/src/core/server/elasticsearch/legacy/cluster_client.ts +++ b/src/core/server/elasticsearch/legacy/cluster_client.ts @@ -20,7 +20,7 @@ import { Client } from 'elasticsearch'; import { get } from 'lodash'; import { LegacyElasticsearchErrorHelpers } from './errors'; -import { GetAuthHeaders, KibanaRequest, isKibanaRequest, isRealRequest } from '../../http'; +import { GetAuthHeaders, isKibanaRequest, isRealRequest } from '../../http'; import { filterHeaders, ensureRawRequest } from '../../http/router'; import { Logger } from '../../logging'; import { ScopeableRequest } from '../types'; diff --git a/src/core/server/elasticsearch/legacy/scoped_cluster_client.test.ts b/src/core/server/elasticsearch/legacy/scoped_cluster_client.test.ts index f1096d5d602f4..2eb8cefb564ae 100644 --- a/src/core/server/elasticsearch/legacy/scoped_cluster_client.test.ts +++ b/src/core/server/elasticsearch/legacy/scoped_cluster_client.test.ts @@ -18,7 +18,6 @@ */ import { LegacyScopedClusterClient } from './scoped_cluster_client'; -import { auditTrailServiceMock } from '../../audit_trail/audit_trail_service.mock'; let internalAPICaller: jest.Mock; let scopedAPICaller: jest.Mock; @@ -84,28 +83,6 @@ describe('#callAsInternalUser', () => { expect(scopedAPICaller).not.toHaveBeenCalled(); }); - - describe('Auditor', () => { - it('does not fail when no auditor provided', () => { - const clusterClientWithoutAuditor = new LegacyScopedClusterClient(jest.fn(), jest.fn()); - expect(() => clusterClientWithoutAuditor.callAsInternalUser('endpoint')).not.toThrow(); - }); - it('creates an audit record if auditor provided', () => { - const auditor = auditTrailServiceMock.createAuditor(); - const clusterClientWithoutAuditor = new LegacyScopedClusterClient( - jest.fn(), - jest.fn(), - {}, - auditor - ); - clusterClientWithoutAuditor.callAsInternalUser('endpoint'); - expect(auditor.add).toHaveBeenCalledTimes(1); - expect(auditor.add).toHaveBeenLastCalledWith({ - message: 'endpoint', - type: 'elasticsearch.call.internalUser', - }); - }); - }); }); describe('#callAsCurrentUser', () => { @@ -229,26 +206,4 @@ describe('#callAsCurrentUser', () => { expect(internalAPICaller).not.toHaveBeenCalled(); }); - - describe('Auditor', () => { - it('does not fail when no auditor provided', () => { - const clusterClientWithoutAuditor = new LegacyScopedClusterClient(jest.fn(), jest.fn()); - expect(() => clusterClientWithoutAuditor.callAsCurrentUser('endpoint')).not.toThrow(); - }); - it('creates an audit record if auditor provided', () => { - const auditor = auditTrailServiceMock.createAuditor(); - const clusterClientWithoutAuditor = new LegacyScopedClusterClient( - jest.fn(), - jest.fn(), - {}, - auditor - ); - clusterClientWithoutAuditor.callAsCurrentUser('endpoint'); - expect(auditor.add).toHaveBeenCalledTimes(1); - expect(auditor.add).toHaveBeenLastCalledWith({ - message: 'endpoint', - type: 'elasticsearch.call.currentUser', - }); - }); - }); });