Skip to content

Commit

Permalink
Make the update alert API key API work when AAD is out of sync (#56640)…
Browse files Browse the repository at this point in the history
… (#57258)

* Make the update API key API work when AAD is out of sync

* Make updateAPIKey only load SOC where possible

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
mikecote and elasticmachine committed Feb 10, 2020
1 parent 6df3df0 commit 00a296b
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 37 deletions.
106 changes: 76 additions & 30 deletions x-pack/legacy/plugins/alerting/server/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2544,63 +2544,109 @@ describe('update()', () => {
});

describe('updateApiKey()', () => {
test('updates the API key for the alert', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce({
id: '1',
type: 'alert',
attributes: {
schedule: { interval: '10s' },
alertTypeId: '2',
enabled: true,
},
version: '123',
references: [],
});
let alertsClient: AlertsClient;
const existingAlert = {
id: '1',
type: 'alert',
attributes: {
schedule: { interval: '10s' },
alertTypeId: '2',
enabled: true,
},
version: '123',
references: [],
};
const existingEncryptedAlert = {
...existingAlert,
attributes: {
...existingAlert.attributes,
apiKey: Buffer.from('123:abc').toString('base64'),
},
};

beforeEach(() => {
alertsClient = new AlertsClient(alertsClientParams);
savedObjectsClient.get.mockResolvedValue(existingAlert);
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValue(existingEncryptedAlert);
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '123', api_key: 'abc' },
result: { id: '234', api_key: 'abc' },
});
});

test('updates the API key for the alert', async () => {
await alertsClient.updateApiKey({ id: '1' });
expect(savedObjectsClient.get).not.toHaveBeenCalled();
expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', {
namespace: 'default',
});
expect(savedObjectsClient.update).toHaveBeenCalledWith(
'alert',
'1',
{
schedule: { interval: '10s' },
alertTypeId: '2',
enabled: true,
apiKey: Buffer.from('123:abc').toString('base64'),
apiKey: Buffer.from('234:abc').toString('base64'),
apiKeyOwner: 'elastic',
updatedBy: 'elastic',
},
{ version: '123' }
);
expect(alertsClientParams.invalidateAPIKey).toHaveBeenCalledWith({ id: '123' });
});

test('swallows error when invalidate API key throws', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
alertsClientParams.invalidateAPIKey.mockRejectedValue(new Error('Fail'));
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce({
id: '1',
type: 'alert',
attributes: {
test('falls back to SOC when getDecryptedAsInternalUser throws an error', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail'));

await alertsClient.updateApiKey({ id: '1' });
expect(savedObjectsClient.get).toHaveBeenCalledWith('alert', '1');
expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', {
namespace: 'default',
});
expect(savedObjectsClient.update).toHaveBeenCalledWith(
'alert',
'1',
{
schedule: { interval: '10s' },
alertTypeId: '2',
enabled: true,
apiKey: Buffer.from('123:abc').toString('base64'),
apiKey: Buffer.from('234:abc').toString('base64'),
apiKeyOwner: 'elastic',
updatedBy: 'elastic',
},
version: '123',
references: [],
});
alertsClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '123', api_key: 'abc' },
});
{ version: '123' }
);
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
});

test('swallows error when invalidate API key throws', async () => {
alertsClientParams.invalidateAPIKey.mockRejectedValue(new Error('Fail'));

await alertsClient.updateApiKey({ id: '1' });
expect(alertsClientParams.logger.error).toHaveBeenCalledWith(
'Failed to invalidate API Key: Fail'
);
expect(savedObjectsClient.update).toHaveBeenCalled();
});

test('swallows error when getting decrypted object throws', async () => {
encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValueOnce(new Error('Fail'));

await alertsClient.updateApiKey({ id: '1' });
expect(alertsClientParams.logger.error).toHaveBeenCalledWith(
'updateApiKey(): Failed to load API key to invalidate on alert 1: Fail'
);
expect(savedObjectsClient.update).toHaveBeenCalled();
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
});

test('throws when savedObjectsClient update fails', async () => {
savedObjectsClient.update.mockRejectedValueOnce(new Error('Fail'));

await expect(alertsClient.updateApiKey({ id: '1' })).rejects.toThrowErrorMatchingInlineSnapshot(
`"Fail"`
);
expect(alertsClientParams.invalidateAPIKey).not.toHaveBeenCalled();
});
});
31 changes: 24 additions & 7 deletions x-pack/legacy/plugins/alerting/server/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,27 @@ export class AlertsClient {
}

public async updateApiKey({ id }: { id: string }) {
const {
version,
attributes,
} = await this.encryptedSavedObjectsPlugin.getDecryptedAsInternalUser<RawAlert>('alert', id, {
namespace: this.namespace,
});
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(
`updateApiKey(): 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;
}

const username = await this.getUserName();
await this.savedObjectsClient.update(
Expand All @@ -363,7 +378,9 @@ export class AlertsClient {
{ version }
);

await this.invalidateApiKey({ apiKey: attributes.apiKey });
if (apiKeyToInvalidate) {
await this.invalidateApiKey({ apiKey: apiKeyToInvalidate });
}
}

private async invalidateApiKey({ apiKey }: { apiKey: string | null }): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,60 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte
}
});

it('should still be able to update API key when AAD is broken', async () => {
const { body: createdAlert } = await supertest
.post(`${getUrlPrefix(space.id)}/api/alert`)
.set('kbn-xsrf', 'foo')
.send(getTestAlertData())
.expect(200);
objectRemover.add(space.id, createdAlert.id, 'alert');

await supertest
.put(`${getUrlPrefix(space.id)}/api/saved_objects/alert/${createdAlert.id}`)
.set('kbn-xsrf', 'foo')
.send({
attributes: {
name: 'bar',
},
})
.expect(200);

const response = await alertUtils.getUpdateApiKeyRequest(createdAlert.id);

switch (scenario.id) {
case 'no_kibana_privileges at space1':
case 'space_1_all at space2':
case 'global_read at space1':
expect(response.statusCode).to.eql(404);
expect(response.body).to.eql({
statusCode: 404,
error: 'Not Found',
message: 'Not Found',
});
break;
case 'superuser at space1':
case 'space_1_all at space1':
expect(response.statusCode).to.eql(204);
expect(response.body).to.eql('');
const { body: updatedAlert } = await supertestWithoutAuth
.get(`${getUrlPrefix(space.id)}/api/alert/${createdAlert.id}`)
.set('kbn-xsrf', 'foo')
.auth(user.username, user.password)
.expect(200);
expect(updatedAlert.apiKeyOwner).to.eql(user.username);
// Ensure AAD isn't broken
await checkAAD({
supertest,
spaceId: space.id,
type: 'alert',
id: createdAlert.id,
});
break;
default:
throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`);
}
});

it(`shouldn't update alert api key from another space`, async () => {
const { body: createdAlert } = await supertest
.post(`${getUrlPrefix('other')}/api/alert`)
Expand Down

0 comments on commit 00a296b

Please sign in to comment.