Skip to content

Commit

Permalink
[ResponseOps] Allow users authenticated with an API keys to manage al…
Browse files Browse the repository at this point in the history
…erting rules (#154189)

Resolves #152140

## Summary
Updates the following functions in the Rules Client to re-use the API
key in context and avoid having the system invalidate them when no
longer in use:

- bulk_delete
- bulk_edit
- clone
- create
- delete
- update
- update_api_key

Also adds a new field to the rule SO to help determine when whether an
api key was created by a user or created by us.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### To verify

- Follow these
[instructions](https://www.elastic.co/guide/en/kibana/master/api-keys.html#create-api-key)
to create an api key. Make sure to copy your api key
- Run the following 
```
curl -X POST "http://localhost:5601/api/alerting/rule/" -H 'Authorization: ApiKey ${API_KEY}' -H 'kbn-xsrf: true' -H 'Content-Type: application/json' -d'
{
  "rule_type_id": "example.pattern",
  "name": "pattern",
  "schedule": {
    "interval": "5s"
  },
  "actions": [
  ],
  "consumer": "alerts",
  "tags": [],
  "notify_when": "onActionGroupChange",
  "params": {
    "patterns": {
      "instA": " a - - a "
    }
  }
}'
```
- Verify that the request returns a rule
with`"api_key_created_by_user":true`
- Try this with the other rules clients functions listed above to verify
that you can manage alerting rules when authenticated with an api key
- Verify that `"api_key_created_by_user":false` when you remove the api
key header and add `-u ${USERNAME}:${PASSWORD}` to authenticate
  • Loading branch information
doakalexi authored Apr 11, 2023
1 parent 60fe5af commit 331eb60
Show file tree
Hide file tree
Showing 70 changed files with 1,191 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
Object {
"action": "6cfc277ed3211639e37546ac625f4a68f2494215",
"action_task_params": "5f419caba96dd8c77d0f94013e71d43890e3d5d6",
"alert": "1e4cd6941f1eb39c729c646e91fbfb9700de84b9",
"alert": "7bec97d7775a025ecf36a33baf17386b9e7b4c3c",
"api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee",
"apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2",
"apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export interface Rule<Params extends RuleTypeParams = never> {
updatedAt: Date;
apiKey: string | null;
apiKeyOwner: string | null;
apiKeyCreatedByUser?: boolean | null;
throttle?: string | null;
muteAll: boolean;
notifyWhen?: RuleNotifyWhenType | null;
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/alerting/public/lib/common_transformations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function transformRule(input: ApiRule): Rule {
updated_at: updatedAt,
api_key: apiKey,
api_key_owner: apiKeyOwner,
api_key_created_by_user: apiKeyCreatedByUser,
notify_when: notifyWhen,
mute_all: muteAll,
muted_alert_ids: mutedInstanceIds,
Expand Down Expand Up @@ -140,6 +141,8 @@ export function transformRule(input: ApiRule): Rule {
...(nextRun ? { nextRun: new Date(nextRun) } : {}),
...(monitoring ? { monitoring: transformMonitoring(monitoring) } : {}),
...(lastRun ? { lastRun: transformLastRun(lastRun) } : {}),
...(apiKeyCreatedByUser !== undefined ? { apiKeyCreatedByUser } : {}),

...rest,
};
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/clone_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -75,6 +76,7 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
: {}),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});

export const cloneRuleRoute = (
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/create_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -103,6 +104,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
actions: rewriteActionsRes(actions),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});

export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOptions) => {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/get_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -78,6 +79,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(viewInAppRelativeUrl ? { view_in_app_relative_url: viewInAppRelativeUrl } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});

interface BuildGetRulesRouteParams {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const rewriteRule = ({
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -76,4 +77,5 @@ export const rewriteRule = ({
})),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/resolve_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const rewriteBodyRes: RewriteResponseCase<ResolvedSanitizedRule<RuleTypeParams>>
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -62,6 +63,7 @@ const rewriteBodyRes: RewriteResponseCase<ResolvedSanitizedRule<RuleTypeParams>>
actions: rewriteActionsRes(actions),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});

export const resolveRuleRoute = (
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/update_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
createdAt,
updatedAt,
apiKeyOwner,
apiKeyCreatedByUser,
notifyWhen,
muteAll,
mutedInstanceIds,
Expand Down Expand Up @@ -113,6 +114,7 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
: {}),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}),
});

export const updateRuleRoute = (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { apiKeyAsAlertAttributes } from './api_key_as_alert_attributes';

describe('apiKeyAsAlertAttributes', () => {
test('return attributes', () => {
expect(
apiKeyAsAlertAttributes(
{
apiKeysEnabled: true,
result: {
id: '123',
name: '123',
api_key: 'abc',
},
},
'test',
false
)
).toEqual({
apiKey: 'MTIzOmFiYw==',
apiKeyOwner: 'test',
apiKeyCreatedByUser: false,
});
});

test('returns null attributes when api keys are not enabled', () => {
expect(
apiKeyAsAlertAttributes(
{
apiKeysEnabled: false,
},
'test',
false
)
).toEqual({
apiKey: null,
apiKeyOwner: null,
apiKeyCreatedByUser: null,
});
});

test('returns apiKeyCreatedByUser as true when createdByUser is passed in', () => {
expect(
apiKeyAsAlertAttributes(
{
apiKeysEnabled: true,
result: {
id: '123',
name: '123',
api_key: 'abc',
},
},
'test',
true
)
).toEqual({
apiKey: 'MTIzOmFiYw==',
apiKeyOwner: 'test',
apiKeyCreatedByUser: true,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ import { CreateAPIKeyResult } from '../types';

export function apiKeyAsAlertAttributes(
apiKey: CreateAPIKeyResult | null,
username: string | null
): Pick<RawRule, 'apiKey' | 'apiKeyOwner'> {
username: string | null,
createdByUser: boolean
): Pick<RawRule, 'apiKey' | 'apiKeyOwner' | 'apiKeyCreatedByUser'> {
return apiKey && apiKey.apiKeysEnabled
? {
apiKeyOwner: username,
apiKey: Buffer.from(`${apiKey.result.id}:${apiKey.result.api_key}`).toString('base64'),
apiKeyCreatedByUser: createdByUser,
}
: {
apiKeyOwner: null,
apiKey: null,
apiKeyCreatedByUser: null,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { savedObjectsClientMock, loggingSystemMock } from '@kbn/core/server/mocks';
import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
import { ruleTypeRegistryMock } from '../../rule_type_registry.mock';
import { alertingAuthorizationMock } from '../../authorization/alerting_authorization.mock';
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
import { actionsAuthorizationMock } from '@kbn/actions-plugin/server/mocks';
import { AlertingAuthorization } from '../../authorization/alerting_authorization';
import { ActionsAuthorization } from '@kbn/actions-plugin/server';
import { RawRule } from '../../types';
import { getBeforeSetup, mockedDateString } from '../tests/lib';
import { createNewAPIKeySet } from './create_new_api_key_set';
import { RulesClientContext } from '../types';

const taskManager = taskManagerMock.createStart();
const ruleTypeRegistry = ruleTypeRegistryMock.create();
const unsecuredSavedObjectsClient = savedObjectsClientMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createClient();
const authorization = alertingAuthorizationMock.create();
const actionsAuthorization = actionsAuthorizationMock.create();

const kibanaVersion = 'v8.0.0';
const rulesClientParams: jest.Mocked<RulesClientContext> = {
taskManager,
ruleTypeRegistry,
unsecuredSavedObjectsClient,
authorization: authorization as unknown as AlertingAuthorization,
actionsAuthorization: actionsAuthorization as unknown as ActionsAuthorization,
spaceId: 'default',
getUserName: jest.fn(),
createAPIKey: jest.fn(),
logger: loggingSystemMock.create().get(),
encryptedSavedObjectsClient: encryptedSavedObjects,
getActionsClient: jest.fn(),
getEventLogClient: jest.fn(),
kibanaVersion,
minimumScheduleInterval: { value: '1m', enforce: false },
minimumScheduleIntervalInMs: 1,
fieldsToExcludeFromPublicApi: [],
isAuthenticationTypeAPIKey: jest.fn(),
getAuthenticationAPIKey: jest.fn(),
};

const username = 'test';
const attributes: RawRule = {
enabled: true,
name: 'rule-name',
tags: ['tag-1', 'tag-2'],
alertTypeId: '123',
consumer: 'rule-consumer',
legacyId: null,
schedule: { interval: '1s' },
actions: [],
params: {},
createdBy: null,
updatedBy: null,
createdAt: mockedDateString,
updatedAt: mockedDateString,
apiKey: null,
apiKeyOwner: null,
throttle: null,
notifyWhen: null,
muteAll: false,
mutedInstanceIds: [],
executionStatus: {
status: 'unknown',
lastExecutionDate: '2020-08-20T19:23:38Z',
error: null,
warning: null,
},
revision: 0,
};

describe('createNewAPIKeySet', () => {
beforeEach(() => {
getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry);
});

test('create new api keys', async () => {
rulesClientParams.createAPIKey.mockResolvedValueOnce({
apiKeysEnabled: true,
result: { id: '123', name: '123', api_key: 'abc' },
});
const apiKey = await createNewAPIKeySet(rulesClientParams, {
id: attributes.alertTypeId,
ruleName: attributes.name,
username,
shouldUpdateApiKey: true,
});
expect(apiKey).toEqual({
apiKey: 'MTIzOmFiYw==',
apiKeyCreatedByUser: undefined,
apiKeyOwner: 'test',
});
expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(1);
});

test('should get api key from the request if the user is authenticated using api keys', async () => {
rulesClientParams.getAuthenticationAPIKey.mockReturnValueOnce({
apiKeysEnabled: true,
result: { id: '123', name: '123', api_key: 'abc' },
});
rulesClientParams.isAuthenticationTypeAPIKey.mockReturnValueOnce(true);
const apiKey = await createNewAPIKeySet(rulesClientParams, {
id: attributes.alertTypeId,
ruleName: attributes.name,
username,
shouldUpdateApiKey: true,
});
expect(apiKey).toEqual({
apiKey: 'MTIzOmFiYw==',
apiKeyCreatedByUser: true,
apiKeyOwner: 'test',
});
expect(rulesClientParams.getAuthenticationAPIKey).toHaveBeenCalledTimes(1);
expect(rulesClientParams.isAuthenticationTypeAPIKey).toHaveBeenCalledTimes(1);
});

test('should throw an error if getting the api key fails', async () => {
rulesClientParams.createAPIKey.mockRejectedValueOnce(new Error('Test failure'));
await expect(
async () =>
await createNewAPIKeySet(rulesClientParams, {
id: attributes.alertTypeId,
ruleName: attributes.name,
username,
shouldUpdateApiKey: true,
})
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Error creating API key for rule - Test failure"`
);
});

test('should throw an error if getting the api key fails and an error message is passed in', async () => {
rulesClientParams.createAPIKey.mockRejectedValueOnce(new Error('Test failure'));
await expect(
async () =>
await createNewAPIKeySet(rulesClientParams, {
id: attributes.alertTypeId,
ruleName: attributes.name,
username,
shouldUpdateApiKey: true,
errorMessage: 'Error updating rule: could not create API key',
})
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Error updating rule: could not create API key - Test failure"`
);
});

test('should return null if shouldUpdateApiKey: false', async () => {
const apiKey = await createNewAPIKeySet(rulesClientParams, {
id: attributes.alertTypeId,
ruleName: attributes.name,
username,
shouldUpdateApiKey: false,
});
expect(apiKey).toEqual({
apiKey: null,
apiKeyCreatedByUser: null,
apiKeyOwner: null,
});
});
});
Loading

0 comments on commit 331eb60

Please sign in to comment.