Skip to content

Commit

Permalink
Gracefully handle decryption errors during ESO migrations (#105968)
Browse files Browse the repository at this point in the history 
* Updating unit tests

* Fixing types

* Updating readme and adding warning message

* Updating README

* PR fixes

* collapsing args to create migration fn

* Adding functional tests

* Adding comment to functional test

* Adding stripOrDecryptAttributesSync

* Using stripOrDecryptAttributesSync

* Fixing unit tests

* PR fixes

* PR fixes

* Moving validation of apikey existence in alerting task runner

* Cleanup

* Reverting changes to alerting task runner

* PR fixes

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@ymao1 @kibanamachine
ymao1 and kibanamachine committed Jul 28, 2021
1 parent 30a1020 commit 81a0267
Show file tree
Hide file tree
Showing 18 changed files with 4,362 additions and 964 deletions.
227 changes: 138 additions & 89 deletions x-pack/plugins/actions/server/saved_objects/migrations.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,121 +15,170 @@ import { migrationMocks } from 'src/core/server/mocks';
const context = migrationMocks.createContext();
const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup();

describe('7.10.0', () => {
describe('successful migrations', () => {
beforeEach(() => {
jest.resetAllMocks();
encryptedSavedObjectsSetup.createMigration.mockImplementation(
(shouldMigrateWhenPredicate, migration) => migration
);
encryptedSavedObjectsSetup.createMigration.mockImplementation(({ migration }) => migration);
});

test('add hasAuth config property for .email actions', () => {
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
const action = getMockDataForEmail({});
const migratedAction = migration710(action, context);
expect(migratedAction.attributes.config).toEqual({
hasAuth: true,
});
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: true,
describe('7.10.0', () => {
test('add hasAuth config property for .email actions', () => {
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
const action = getMockDataForEmail({});
const migratedAction = migration710(action, context);
expect(migratedAction.attributes.config).toEqual({
hasAuth: true,
});
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: true,
},
},
},
});
});
});

test('rename cases configuration object', () => {
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
const action = getCasesMockData({});
const migratedAction = migration710(action, context);
expect(migratedAction.attributes.config).toEqual({
incidentConfiguration: { mapping: [] },
});
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
config: {
incidentConfiguration: { mapping: [] },
test('rename cases configuration object', () => {
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
const action = getCasesMockData({});
const migratedAction = migration710(action, context);
expect(migratedAction.attributes.config).toEqual({
incidentConfiguration: { mapping: [] },
});
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
config: {
incidentConfiguration: { mapping: [] },
},
},
},
});
});
});
});

describe('7.11.0', () => {
beforeEach(() => {
jest.resetAllMocks();
encryptedSavedObjectsSetup.createMigration.mockImplementation(
(shouldMigrateWhenPredicate, migration) => migration
);
});

test('add hasAuth = true for .webhook actions with user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, true);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: true,
describe('7.11.0', () => {
test('add hasAuth = true for .webhook actions with user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, true);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: true,
},
},
},
});
});
});

test('add hasAuth = false for .webhook actions without user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, false);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: false,
test('add hasAuth = false for .webhook actions without user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, false);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: false,
},
},
},
});
});
});
test('remove cases mapping object', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockData({
config: { incidentConfiguration: { mapping: [] }, isCaseOwned: true, another: 'value' },
test('remove cases mapping object', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockData({
config: { incidentConfiguration: { mapping: [] }, isCaseOwned: true, another: 'value' },
});
expect(migration711(action, context)).toEqual({
...action,
attributes: {
...action.attributes,
config: {
another: 'value',
},
},
});
});
expect(migration711(action, context)).toEqual({
...action,
attributes: {
...action.attributes,
config: {
another: 'value',
});

describe('7.14.0', () => {
test('add isMissingSecrets property for actions', () => {
const migration714 = getMigrations(encryptedSavedObjectsSetup)['7.14.0'];
const action = getMockData({ isMissingSecrets: undefined });
const migratedAction = migration714(action, context);
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
isMissingSecrets: false,
},
},
});
});
});
});

describe('7.14.0', () => {
describe('handles errors during migrations', () => {
beforeEach(() => {
jest.resetAllMocks();
encryptedSavedObjectsSetup.createMigration.mockImplementation(
(shouldMigrateWhenPredicate, migration) => migration
);
encryptedSavedObjectsSetup.createMigration.mockImplementation(() => () => {
throw new Error(`Can't migrate!`);
});
});

describe('7.10.0 throws if migration fails', () => {
test('should show the proper exception', () => {
const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
const action = getMockDataForEmail({});
expect(() => {
migration710(action, context);
}).toThrowError(`Can't migrate!`);
expect(context.log.error).toHaveBeenCalledWith(
`encryptedSavedObject 7.10.0 migration failed for action ${action.id} with error: Can't migrate!`,
{
migrations: {
actionDocument: action,
},
}
);
});
});

describe('7.11.0 throws if migration fails', () => {
test('should show the proper exception', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForEmail({});
expect(() => {
migration711(action, context);
}).toThrowError(`Can't migrate!`);
expect(context.log.error).toHaveBeenCalledWith(
`encryptedSavedObject 7.11.0 migration failed for action ${action.id} with error: Can't migrate!`,
{
migrations: {
actionDocument: action,
},
}
);
});
});

test('add isMissingSecrets property for actions', () => {
const migration714 = getMigrations(encryptedSavedObjectsSetup)['7.14.0'];
const action = getMockData({ isMissingSecrets: undefined });
const migratedAction = migration714(action, context);
expect(migratedAction).toEqual({
...action,
attributes: {
...action.attributes,
isMissingSecrets: false,
},
describe('7.14.0 throws if migration fails', () => {
test('should show the proper exception', () => {
const migration714 = getMigrations(encryptedSavedObjectsSetup)['7.14.0'];
const action = getMockDataForEmail({});
expect(() => {
migration714(action, context);
}).toThrowError(`Can't migrate!`);
expect(context.log.error).toHaveBeenCalledWith(
`encryptedSavedObject 7.14.0 migration failed for action ${action.id} with error: Can't migrate!`,
{
migrations: {
actionDocument: action,
},
}
);
});
});
});
Expand Down
26 changes: 21 additions & 5 deletions x-pack/plugins/actions/server/saved_objects/migrations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../../../../src/core/server';
import { RawAction } from '../types';
import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server';
import type { IsMigrationNeededPredicate } from '../../../encrypted_saved_objects/server';

interface ActionsLogMeta extends LogMeta {
migrations: { actionDocument: SavedObjectUnsanitizedDoc<RawAction> };
Expand All @@ -23,25 +24,40 @@ type ActionMigration = (
doc: SavedObjectUnsanitizedDoc<RawAction>
) => SavedObjectUnsanitizedDoc<RawAction>;

function createEsoMigration(
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup,
isMigrationNeededPredicate: IsMigrationNeededPredicate<RawAction, RawAction>,
migrationFunc: ActionMigration
) {
return encryptedSavedObjects.createMigration<RawAction, RawAction>({
isMigrationNeededPredicate,
migration: migrationFunc,
shouldMigrateIfDecryptionFails: true, // shouldMigrateIfDecryptionFails flag that applies the migration to undecrypted document if decryption fails
});
}

export function getMigrations(
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup
): SavedObjectMigrationMap {
const migrationActionsTen = encryptedSavedObjects.createMigration<RawAction, RawAction>(
const migrationActionsTen = createEsoMigration(
encryptedSavedObjects,
(doc): doc is SavedObjectUnsanitizedDoc<RawAction> =>
doc.attributes.config?.hasOwnProperty('casesConfiguration') ||
doc.attributes.actionTypeId === '.email',
pipeMigrations(renameCasesConfigurationObject, addHasAuthConfigurationObject)
);

const migrationActionsEleven = encryptedSavedObjects.createMigration<RawAction, RawAction>(
const migrationActionsEleven = createEsoMigration(
encryptedSavedObjects,
(doc): doc is SavedObjectUnsanitizedDoc<RawAction> =>
doc.attributes.config?.hasOwnProperty('isCaseOwned') ||
doc.attributes.config?.hasOwnProperty('incidentConfiguration') ||
doc.attributes.actionTypeId === '.webhook',
pipeMigrations(removeCasesFieldMappings, addHasAuthConfigurationObject)
);

const migrationActionsFourteen = encryptedSavedObjects.createMigration<RawAction, RawAction>(
const migrationActionsFourteen = createEsoMigration(
encryptedSavedObjects,
(doc): doc is SavedObjectUnsanitizedDoc<RawAction> => true,
pipeMigrations(addisMissingSecretsField)
);
Expand Down Expand Up @@ -69,8 +85,8 @@ function executeMigrationWithErrorHandling(
},
}
);
throw ex;
}
return doc;
};
}

Expand Down Expand Up @@ -120,7 +136,7 @@ const addHasAuthConfigurationObject = (
if (doc.attributes.actionTypeId !== '.email' && doc.attributes.actionTypeId !== '.webhook') {
return doc;
}
const hasAuth = !!doc.attributes.secrets.user || !!doc.attributes.secrets.password;
const hasAuth = !!doc.attributes.secrets?.user || !!doc.attributes.secrets?.password;
return {
...doc,
attributes: {
Expand Down
Loading

0 comments on commit 81a0267

Please sign in to comment.