Skip to content

Commit

Permalink
Invalidate API key if any in enable API
Browse files Browse the repository at this point in the history
  • Loading branch information
mikecote committed Feb 7, 2020
1 parent 4331c37 commit f1367ec
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 5 deletions.
39 changes: 36 additions & 3 deletions x-pack/legacy/plugins/alerting/server/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,7 @@ describe('enable()', () => {

beforeEach(() => {
alertsClient = new AlertsClient(alertsClientParams);
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue(existingAlert);
savedObjectsClient.get.mockResolvedValue(existingAlert);
alertsClientParams.createAPIKey.mockResolvedValue({
apiKeysEnabled: false,
Expand All @@ -884,7 +885,11 @@ describe('enable()', () => {

test('enables an alert', async () => {
await alertsClient.enable({ id: '1' });
expect(savedObjectsClient.get).toHaveBeenCalledWith('alert', '1');
expect(savedObjectsClient.get).not.toHaveBeenCalled();
expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', {
namespace: 'default',
});
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
expect(alertsClientParams.createAPIKey).toHaveBeenCalled();
expect(savedObjectsClient.update).toHaveBeenCalledWith(
'alert',
Expand Down Expand Up @@ -919,8 +924,25 @@ describe('enable()', () => {
});
});

test('invalidates API key if ever one existed prior to updating', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue({
...existingAlert,
attributes: {
...existingAlert.attributes,
apiKey: Buffer.from('123:abc').toString('base64'),
},
});

await alertsClient.enable({ id: '1' });
expect(savedObjectsClient.get).not.toHaveBeenCalled();
expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', {
namespace: 'default',
});
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' });
});

test(`doesn't enable already enabled alerts`, async () => {
savedObjectsClient.get.mockResolvedValueOnce({
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce({
...existingAlert,
attributes: {
...existingAlert.attributes,
Expand Down Expand Up @@ -959,7 +981,18 @@ describe('enable()', () => {
);
});

test('throws error when failing to load the saved object', async () => {
test('falls back when failing to getDecryptedAsInternalUser', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail'));

await alertsClient.enable({ id: '1' });
expect(savedObjectsClient.get).toHaveBeenCalledWith('alert', '1');
expect(alertsClientParams.logger.error).toHaveBeenCalledWith(
'enable(): Failed to load API key to invalidate on alert 1: Fail'
);
});

test('throws error when failing to load the saved object using SOC', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(new Error('Fail'));
savedObjectsClient.get.mockRejectedValueOnce(new Error('Fail to get'));

await expect(alertsClient.enable({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot(
Expand Down
27 changes: 25 additions & 2 deletions x-pack/legacy/plugins/alerting/server/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,27 @@ export class AlertsClient {
}

public async enable({ id }: { id: string }) {
const { version, attributes } = await this.savedObjectsClient.get<RawAlert>('alert', id);
let apiKeyToInvalidate: string | null = null;
let attributes: RawAlert;
let version: string | undefined;

try {
const decryptedAlert = await this.encryptedSavedObjectsPlugin.getDecryptedAsInternalUser<
RawAlert
>('alert', id, { namespace: this.namespace });
apiKeyToInvalidate = decryptedAlert.attributes.apiKey;
attributes = decryptedAlert.attributes;
version = decryptedAlert.version;
} catch (e) {
// We'll skip invalidating the API key since we failed to load the decrypted saved object
this.logger.error(
`enable(): Failed to load API key to invalidate on alert ${id}: ${e.message}`
);
// Still attempt to load the attributes and version using SOC
const alert = await this.savedObjectsClient.get<RawAlert>('alert', id);
attributes = alert.attributes;
version = alert.version;
}

if (attributes.enabled === false) {
const username = await this.getUserName();
Expand All @@ -390,6 +410,9 @@ export class AlertsClient {
);
const scheduledTask = await this.scheduleAlert(id, attributes.alertTypeId);
await this.savedObjectsClient.update('alert', id, { scheduledTaskId: scheduledTask.id });
if (apiKeyToInvalidate) {
await this.invalidateApiKey({ apiKey: apiKeyToInvalidate });
}
}
}

Expand All @@ -410,7 +433,7 @@ export class AlertsClient {
this.logger.error(
`disable(): Failed to load API key to invalidate on alert ${id}: ${e.message}`
);
// Still attempt to load the scheduledTaskId using SOC
// Still attempt to load the attributes and version using SOC
const alert = await this.savedObjectsClient.get<RawAlert>('alert', id);
attributes = alert.attributes;
version = alert.version;
Expand Down

0 comments on commit f1367ec

Please sign in to comment.