From 1cc9d52596193fd2833e52cc4732a11791c249c1 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Mon, 16 Nov 2020 18:01:15 -0800 Subject: [PATCH] Use encryptedSavedObjectClient --- .../server/alerts_client/tests/create.test.ts | 4 ++-- .../server/alerts_client/tests/delete.test.ts | 8 ++++---- .../server/alerts_client/tests/disable.test.ts | 12 ++++++------ .../server/alerts_client/tests/enable.test.ts | 10 +++++----- .../server/alerts_client/tests/update.test.ts | 10 +++++----- .../alerts_client/tests/update_api_key.test.ts | 10 +++++----- .../mark_api_key_for_invalidation.test.ts | 16 +++++++++++++++- .../mark_api_key_for_invalidation.ts | 4 +--- .../server/invalidate_pending_api_keys/task.ts | 15 +++++++++++---- .../plugins/alerts/server/saved_objects/index.ts | 6 ++++++ .../fixtures/plugins/alerts/server/routes.ts | 2 -- .../security_and_spaces/tests/alerting/update.ts | 11 ----------- 12 files changed, 60 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index 1041cc13ba6d5..ee407b1a6d50c 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -735,7 +735,7 @@ describe('create()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -746,7 +746,7 @@ describe('create()', () => { expect(taskManager.schedule).not.toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(2); expect(unsecuredSavedObjectsClient.create.mock.calls[1][1]).toStrictEqual({ - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts index 1a56e96c2a140..e7b975aec8eb0 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/delete.test.ts @@ -97,7 +97,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -121,7 +121,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -189,7 +189,7 @@ describe('delete()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index f20c49992b521..11ce0027f82d8 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -112,7 +112,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -156,7 +156,7 @@ describe('disable()', () => { expect(taskManager.remove).toHaveBeenCalledWith('task-123'); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); }); test('falls back when getDecryptedAsInternalUser throws an error', async () => { @@ -165,7 +165,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -225,7 +225,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -242,7 +242,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -258,7 +258,7 @@ describe('disable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 6e9275c8c503a..16e83c42d8930 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -162,7 +162,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -239,7 +239,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -252,7 +252,7 @@ describe('enable()', () => { }); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); }); test(`doesn't enable already enabled alerts`, async () => { @@ -345,7 +345,7 @@ describe('enable()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt, }, references: [], @@ -358,7 +358,7 @@ describe('enable()', () => { expect(alertsClientParams.createAPIKey).toHaveBeenCalled(); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MTIz'); + ).toBe('123'); expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1); expect(taskManager.schedule).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 4fe3b7a9f58e9..ad58e36ade722 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -164,7 +164,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -388,7 +388,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -397,7 +397,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -560,7 +560,7 @@ describe('update()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -1002,7 +1002,7 @@ describe('update()', () => { ).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`); expect( (unsecuredSavedObjectsClient.create.mock.calls[1][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MjM0'); + ).toBe('234'); }); describe('updating an alert schedule', () => { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index 365a8526f3198..af178a1fac5f5 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -84,7 +84,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -141,7 +141,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MTIz', + apiKeyId: '123', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -199,7 +199,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -223,7 +223,7 @@ describe('updateApiKey()', () => { id: '1', type: 'api_key_pending_invalidation', attributes: { - apiKeyId: 'MjM0', + apiKeyId: '234', createdAt: '2019-02-12T21:01:22.479Z', }, references: [], @@ -234,7 +234,7 @@ describe('updateApiKey()', () => { ); expect( (unsecuredSavedObjectsClient.create.mock.calls[0][1] as InvalidatePendingApiKey).apiKeyId - ).toBe('MjM0'); + ).toBe('234'); }); describe('authorization', () => { diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts index 13f5a7dcd7eac..7b30c22c47f8a 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.test.ts @@ -19,7 +19,7 @@ describe('markApiKeyForInvalidation', () => { }, references: [], }); - markApiKeyForInvalidation( + await markApiKeyForInvalidation( { apiKey: Buffer.from('123:abc').toString('base64') }, loggingSystemMock.create().get(), unsecuredSavedObjectsClient @@ -30,4 +30,18 @@ describe('markApiKeyForInvalidation', () => { 'api_key_pending_invalidation' ); }); + + test('should log the proper error when savedObjectsClient create failed', async () => { + const logger = loggingSystemMock.create().get(); + const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); + unsecuredSavedObjectsClient.create.mockRejectedValueOnce(new Error('Fail')); + await markApiKeyForInvalidation( + { apiKey: Buffer.from('123').toString('base64') }, + logger, + unsecuredSavedObjectsClient + ); + expect(logger.error).toHaveBeenCalledWith( + 'Failed to mark for API key [id="MTIz"] for invalidation: Fail' + ); + }); }); diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts index d145e8e33fa3d..db25f5b3e19eb 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/mark_api_key_for_invalidation.ts @@ -14,9 +14,7 @@ export const markApiKeyForInvalidation = async ( return; } try { - const apiKeyId = Buffer.from(Buffer.from(apiKey, 'base64').toString().split(':')[0]).toString( - 'base64' - ); + const apiKeyId = Buffer.from(apiKey, 'base64').toString().split(':')[0]; await savedObjectsClient.create('api_key_pending_invalidation', { apiKeyId, createdAt: new Date().toISOString(), diff --git a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts index dd2f9684dccbf..77cbb9f4a4a85 100644 --- a/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts +++ b/x-pack/plugins/alerts/server/invalidate_pending_api_keys/task.ts @@ -11,6 +11,7 @@ import { KibanaRequest, SavedObjectsClientContract, } from 'kibana/server'; +import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; import { InvalidateAPIKeyParams, SecurityPluginSetup } from '../../../security/server'; import { RunContext, @@ -129,7 +130,7 @@ function taskRunner( let totalInvalidated = 0; const configResult = await config; try { - const [{ savedObjects, http }] = await coreStartServices; + const [{ savedObjects, http }, { encryptedSavedObjects }] = await coreStartServices; const savedObjectsClient = savedObjects.getScopedClient( getFakeKibanaRequest(http.basePath.serverBasePath), { @@ -137,7 +138,9 @@ function taskRunner( excludedWrappers: ['security'], } ); - + const encryptedSavedObjectsClient = encryptedSavedObjects.getClient({ + includedHiddenTypes: ['api_key_pending_invalidation'], + }); const configuredDelay = configResult.invalidateApiKeysTask.removalDelay; const delay = timePeriodBeforeDate(new Date(), configuredDelay).toISOString(); @@ -156,6 +159,7 @@ function taskRunner( logger, savedObjectsClient, apiKeysToInvalidate, + encryptedSavedObjectsClient, securityPluginSetup ); @@ -192,13 +196,16 @@ async function invalidateApiKeys( logger: Logger, savedObjectsClient: SavedObjectsClientContract, apiKeysToInvalidate: SavedObjectsFindResponse, + encryptedSavedObjectsClient: EncryptedSavedObjectsClient, securityPluginSetup?: SecurityPluginSetup ) { let totalInvalidated = 0; await Promise.all( apiKeysToInvalidate.saved_objects.map(async (apiKeyObj) => { - const apiKeyId = Buffer.from(apiKeyObj.attributes.apiKeyId, 'base64').toString(); - + const decryptedApiKey = await encryptedSavedObjectsClient.getDecryptedAsInternalUser< + InvalidatePendingApiKey + >('api_key_pending_invalidation', apiKeyObj.id); + const apiKeyId = decryptedApiKey.attributes.apiKeyId; const response = await invalidateAPIKey({ id: apiKeyId }, securityPluginSetup); if (response.apiKeysEnabled === true && response.result.error_count > 0) { logger.error(`Failed to invalidate API Key [id="${apiKeyObj.attributes.apiKeyId}"]`); diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index bbdc121f8f855..da30273e93c6b 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -64,4 +64,10 @@ export function setupSavedObjects( attributesToEncrypt: new Set(['apiKey']), attributesToExcludeFromAAD: new Set(AlertAttributesExcludedFromAAD), }); + + // Encrypted attributes + encryptedSavedObjects.registerType({ + type: 'api_key_pending_invalidation', + attributesToEncrypt: new Set(['apiKeyId']), + }); } diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index a4ea3421b1580..d4c7635315f22 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts @@ -204,8 +204,6 @@ export function defineRoutes( const findResult = await savedObjectsWithTasksAndAlerts.find({ type: 'api_key_pending_invalidation', }); - // eslint-disable-next-line no-console - console.log(findResult); return res.ok({ body: { apiKeysToInvalidate: findResult.saved_objects }, }); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 3609b08cd3a37..6164669a31a40 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -31,13 +31,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { .then((response: SupertestResponse) => response.body); } - function getAlertingApiKeysToInvalidate() { - return supertest - .get(`/api/alerts_fixture/api_keys_pending_invalidation`) - .expect(200) - .then((response: SupertestResponse) => response.body); - } - describe('update', () => { const objectRemover = new ObjectRemover(supertest); @@ -859,7 +852,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { }) .expect(200); objectRemover.add(space.id, createdAlert.id, 'alert', 'alerts'); - const updatedData = { name: 'bcd', tags: ['bar'], @@ -910,9 +902,6 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { expect(alertTask.status).to.eql('idle'); // ensure the alert is rescheduled to a minute from now ensureDatetimeIsWithinRange(Date.parse(alertTask.runAt), 60 * 1000); - - const apiKeyIds = await getAlertingApiKeysToInvalidate(); - expect(apiKeyIds.apiKeysToInvalidate.length).to.be(0); }); break; default: