github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 17:59:23 -04:00
Code Issues Projects Releases Packages Wiki Activity

Fix scenario where orphaned API keys can exist when SO operations fail (#78843)

Browse source 
* Fix scenario where orphaned API keys can exist

* Add test for enable API
This commit is contained in:
Mike Côté 2020-10-01 11:24:54 -04:00 • committed by GitHub
parent 758f537708
commit e59c78c2b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 135 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

67
x-pack/plugins/alerts/server/alerts_client.test.ts
View file

@ -778,8 +778,12 @@ describe('create()', () => {
expect(taskManager.schedule).not.toHaveBeenCalled();
});
test('throws error if create saved object fails', async () => {
test('throws error and invalidates API key when create saved object fails', async () => {
const data = getMockData();
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '123', name: '123', api_key: 'abc' },
});
unsecuredSavedObjectsClient.bulkGet.mockResolvedValueOnce({
saved_objects: [
{
@ -798,6 +802,7 @@ describe('create()', () => {
`"Test failure"`
);
expect(taskManager.schedule).not.toHaveBeenCalled();
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' });
});
test('attempts to remove saved object if scheduling failed', async () => {
@ -1422,6 +1427,10 @@ describe('enable()', () => {
});
test('throws error when failing to update the first time', async () => {
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '123', name: '123', api_key: 'abc' },
});
unsecuredSavedObjectsClient.update.mockReset();
unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail to update'));
@ -1430,6 +1439,7 @@ describe('enable()', () => {
);
expect(alertsClientParams.getUserName).toHaveBeenCalled();
expect(alertsClientParams.createAPIKey).toHaveBeenCalled();
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' });
expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledTimes(1);
expect(taskManager.schedule).not.toHaveBeenCalled();
});
@ -3926,6 +3936,52 @@ describe('update()', () => {
);
});
test('throws when unsecuredSavedObjectsClient update fails and invalidates newly created API key', async () => {
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '234', name: '234', api_key: 'abc' },
});
unsecuredSavedObjectsClient.bulkGet.mockResolvedValueOnce({
saved_objects: [
{
id: '1',
type: 'action',
attributes: {
actions: [],
actionTypeId: 'test',
},
references: [],
},
],
});
unsecuredSavedObjectsClient.create.mockRejectedValue(new Error('Fail'));
await expect(
alertsClient.update({
id: '1',
data: {
schedule: { interval: '10s' },
name: 'abc',
tags: ['foo'],
params: {
bar: true,
},
throttle: null,
actions: [
{
group: 'default',
id: '1',
params: {
foo: true,
},
},
],
},
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' });
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' });
});
describe('updating an alert schedule', () => {
function mockApiCalls(
alertId: string,
@ -4360,13 +4416,18 @@ describe('updateApiKey()', () => {
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
});
test('throws when unsecuredSavedObjectsClient update fails', async () => {
test('throws when unsecuredSavedObjectsClient update fails and invalidates newly created API key', async () => {
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '234', name: '234', api_key: 'abc' },
});
unsecuredSavedObjectsClient.update.mockRejectedValueOnce(new Error('Fail'));
await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot(
`"Fail"`
);
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalledWith({ id: '123' });
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '234' });
});
describe('authorization', () => {

125
x-pack/plugins/alerts/server/alerts_client.ts
View file

@ -229,14 +229,21 @@ export class AlertsClient {
muteAll: false,
mutedInstanceIds: [],
};
const createdAlert = await this.unsecuredSavedObjectsClient.create(
'alert',
this.updateMeta(rawAlert),
{
...options,
references,
}
);
let createdAlert: SavedObject<RawAlert>;
try {
createdAlert = await this.unsecuredSavedObjectsClient.create(
'alert',
this.updateMeta(rawAlert),
{
...options,
references,
}
);
} catch (e) {
// Avoid unused API key
this.invalidateApiKey({ apiKey: rawAlert.apiKey });
throw e;
}
if (data.enabled) {
let scheduledTask;
try {
@ -498,23 +505,31 @@ export class AlertsClient {
: null;
const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
const updatedObject = await this.unsecuredSavedObjectsClient.create<RawAlert>(
'alert',
this.updateMeta({
...attributes,
...data,
...apiKeyAttributes,
params: validatedAlertTypeParams as RawAlert['params'],
actions,
updatedBy: username,
}),
{
id,
overwrite: true,
version,
references,
}
);
let updatedObject: SavedObject<RawAlert>;
const createAttributes = this.updateMeta({
...attributes,
...data,
...apiKeyAttributes,
params: validatedAlertTypeParams as RawAlert['params'],
actions,
updatedBy: username,
});
try {
updatedObject = await this.unsecuredSavedObjectsClient.create<RawAlert>(
'alert',
createAttributes,
{
id,
overwrite: true,
version,
references,
}
);
} catch (e) {
// Avoid unused API key
this.invalidateApiKey({ apiKey: createAttributes.apiKey });
throw e;
}
return this.getPartialAlertFromRaw(
id,
@ -580,19 +595,21 @@ export class AlertsClient {
}
const username = await this.getUserName();
await this.unsecuredSavedObjectsClient.update(
'alert',
id,
this.updateMeta({
...attributes,
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)),
username
),
updatedBy: username,
}),
{ version }
);
const updateAttributes = this.updateMeta({
...attributes,
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)),
username
),
updatedBy: username,
});
try {
await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version });
} catch (e) {
// Avoid unused API key
this.invalidateApiKey({ apiKey: updateAttributes.apiKey });
throw e;
}
if (apiKeyToInvalidate) {
await this.invalidateApiKey({ apiKey: apiKeyToInvalidate });
@ -658,22 +675,22 @@ export class AlertsClient {
if (attributes.enabled === false) {
const username = await this.getUserName();
await this.unsecuredSavedObjectsClient.update(
'alert',
id,
this.updateMeta({
...attributes,
enabled: true,
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(
this.generateAPIKeyName(attributes.alertTypeId, attributes.name)
),
username
),
updatedBy: username,
}),
{ version }
);
const updateAttributes = this.updateMeta({
...attributes,
enabled: true,
...this.apiKeyAsAlertAttributes(
await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)),
username
),
updatedBy: username,
});
try {
await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version });
} catch (e) {
// Avoid unused API key
this.invalidateApiKey({ apiKey: updateAttributes.apiKey });
throw e;
}
const scheduledTask = await this.scheduleAlert(id, attributes.alertTypeId);
await this.unsecuredSavedObjectsClient.update('alert', id, {
scheduledTaskId: scheduledTask.id,