Skip to content

Commit

Permalink
[ReaponseOps] Add name property to audit logs SO (#193323)
Browse files Browse the repository at this point in the history
Issue: elastic/enhancements#19823

So the purpose of this PR is to add a rule name to each audit log in
alerting API.
Previously if with a rule was done some action (like create, delete,
etc.), the user could see it in an audit log. But this log included only
rule SO id, but not name. Users wanted to see a rule name associated
with the audit log.
So here I added it.

The principle I follow here to accelerate development (agreed with
@cnasikas): if it is easy (name easy to extract in the code the
`savedObject`) to pass it. If it is not do not.

### 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
  • Loading branch information
guskovaue committed Sep 25, 2024
1 parent 1f8a91b commit 45b4089
Show file tree
Hide file tree
Showing 52 changed files with 344 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
uiSettings: uiSettingsServiceMock.createStartContract(),
};

const fakeRuleName = 'fakeRuleName';

const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
Expand All @@ -80,7 +82,7 @@ const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
duration: '12h',
enabled: true,
rule: {
name: 'my rule name',
name: fakeRuleName,
tags: ['foo'],
alertTypeId: 'myType',
// @ts-expect-error
Expand Down Expand Up @@ -149,10 +151,11 @@ describe('deleteBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User is deleting ad hoc run for ad_hoc_run_params [id=1]',
message:
'User is deleting ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
expect(unsecuredSavedObjectsClient.delete).toHaveBeenLastCalledWith(
AD_HOC_RUN_SAVED_OBJECT_TYPE,
Expand Down Expand Up @@ -212,10 +215,11 @@ describe('deleteBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'Failed attempt to delete ad hoc run for ad_hoc_run_params [id=1]',
message:
'Failed attempt to delete ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
});

Expand All @@ -229,6 +233,7 @@ describe('deleteBackfill()', () => {
message: 'Unable to get',
statusCode: 404,
},
attributes: { rule: { name: fakeRuleName } },
});

await expect(rulesClient.deleteBackfill('1')).rejects.toThrowErrorMatchingInlineSnapshot(
Expand All @@ -246,8 +251,15 @@ describe('deleteBackfill()', () => {
outcome: 'failure',
type: ['deletion'],
},
kibana: { saved_object: { id: '1', type: AD_HOC_RUN_SAVED_OBJECT_TYPE } },
message: 'Failed attempt to delete ad hoc run for ad_hoc_run_params [id=1]',
kibana: {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: 'backfill for rule "fakeRuleName"',
},
},
message:
'Failed attempt to delete ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ async function deleteWithOCC(context: RulesClientContext, { id }: { id: string }
context.auditLogger?.log(
adHocRunAuditEvent({
action: AdHocRunAuditAction.DELETE,
savedObject: { type: AD_HOC_RUN_SAVED_OBJECT_TYPE, id },
savedObject: {
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
id,
name: `backfill for rule "${result.attributes.rule.name}"`,
},
error: new Error(result.error.message),
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
uiSettings: uiSettingsServiceMock.createStartContract(),
};

const fakeRuleName = 'fakeRuleName';

const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
Expand All @@ -181,7 +183,7 @@ const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
duration: '12h',
enabled: true,
rule: {
name: 'my rule name',
name: fakeRuleName,
tags: ['foo'],
alertTypeId: 'myType',
// @ts-expect-error
Expand Down Expand Up @@ -266,10 +268,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -311,10 +314,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -374,10 +378,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -437,10 +442,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -516,10 +522,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -597,10 +604,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down Expand Up @@ -648,10 +656,11 @@ describe('findBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'User has found ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has found ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});

expect(result).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
uiSettings: uiSettingsServiceMock.createStartContract(),
};

const fakeRuleName = 'fakeRuleName';

const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
Expand All @@ -80,7 +82,7 @@ const mockAdHocRunSO: SavedObject<AdHocRunSO> = {
duration: '12h',
enabled: true,
rule: {
name: 'my rule name',
name: fakeRuleName,
tags: ['foo'],
alertTypeId: 'myType',
// @ts-expect-error
Expand Down Expand Up @@ -148,10 +150,11 @@ describe('getBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: `backfill for rule "fakeRuleName"`,
},
},
message: 'User has got ad hoc run for ad_hoc_run_params [id=1]',
message:
'User has got ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
expect(logger.error).not.toHaveBeenCalled();

Expand Down Expand Up @@ -194,10 +197,11 @@ describe('getBackfill()', () => {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: `backfill for rule "my rule name"`,
name: 'backfill for rule "fakeRuleName"',
},
},
message: 'Failed attempt to get ad hoc run for ad_hoc_run_params [id=1]',
message:
'Failed attempt to get ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
});

Expand All @@ -211,6 +215,7 @@ describe('getBackfill()', () => {
message: 'Unable to get',
statusCode: 404,
},
attributes: { rule: { name: fakeRuleName } },
});

await expect(rulesClient.getBackfill('1')).rejects.toThrowErrorMatchingInlineSnapshot(
Expand All @@ -228,8 +233,15 @@ describe('getBackfill()', () => {
outcome: 'failure',
type: ['access'],
},
kibana: { saved_object: { id: '1', type: AD_HOC_RUN_SAVED_OBJECT_TYPE } },
message: 'Failed attempt to get ad hoc run for ad_hoc_run_params [id=1]',
kibana: {
saved_object: {
id: '1',
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
name: 'backfill for rule "fakeRuleName"',
},
},
message:
'Failed attempt to get ad hoc run for ad_hoc_run_params [id=1] backfill for rule "fakeRuleName"',
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ export async function getBackfill(context: RulesClientContext, id: string): Prom
context.auditLogger?.log(
adHocRunAuditEvent({
action: AdHocRunAuditAction.GET,
savedObject: { type: AD_HOC_RUN_SAVED_OBJECT_TYPE, id },
savedObject: {
type: AD_HOC_RUN_SAVED_OBJECT_TYPE,
id,
name: `backfill for rule "${result.attributes.rule.name}"`,
},
error: new Error(result.error.message),
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ const rulesClientParams: jest.Mocked<ConstructorOptions> = {
uiSettings: uiSettingsServiceMock.createStartContract(),
};

const fakeRuleName = 'fakeRuleName';

const existingRule = {
id: '1',
type: RULE_SAVED_OBJECT_TYPE,
Expand All @@ -99,7 +101,7 @@ const existingRule = {
notifyWhen: null,
actions: [],
systemActions: [],
name: 'my rule name',
name: fakeRuleName,
revision: 0,
},
references: [],
Expand Down Expand Up @@ -382,8 +384,8 @@ describe('scheduleBackfill()', () => {
outcome: 'success',
type: ['access'],
},
kibana: { saved_object: { id: '1', type: RULE_SAVED_OBJECT_TYPE } },
message: 'User has scheduled backfill for rule [id=1]',
kibana: { saved_object: { id: '1', type: RULE_SAVED_OBJECT_TYPE, name: 'fakeRuleName' } },
message: 'User has scheduled backfill for rule [id=1] [name=fakeRuleName]',
});
expect(auditLogger.log).toHaveBeenNthCalledWith(2, {
event: {
Expand All @@ -392,8 +394,8 @@ describe('scheduleBackfill()', () => {
outcome: 'success',
type: ['access'],
},
kibana: { saved_object: { id: '2', type: RULE_SAVED_OBJECT_TYPE } },
message: 'User has scheduled backfill for rule [id=2]',
kibana: { saved_object: { id: '2', type: RULE_SAVED_OBJECT_TYPE, name: 'fakeRuleName' } },
message: 'User has scheduled backfill for rule [id=2] [name=fakeRuleName]',
});

expect(backfillClient.bulkQueue).toHaveBeenCalledWith({
Expand Down Expand Up @@ -578,6 +580,7 @@ describe('scheduleBackfill()', () => {
await expect(
rulesClient.scheduleBackfill(mockData)
).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized"`);

expect(auditLogger?.log).toHaveBeenCalledWith({
error: { code: 'Error', message: 'Unauthorized' },
event: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ export async function scheduleBackfill(
context.auditLogger?.log(
ruleAuditEvent({
action: RuleAuditAction.SCHEDULE_BACKFILL,
savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: rule.id },
savedObject: {
type: RULE_SAVED_OBJECT_TYPE,
id: rule.id,
name: rule.attributes.name,
},
})
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,12 +595,12 @@ describe('bulkDelete', () => {
expect(auditLogger.log.mock.calls[0][0]?.event?.action).toEqual('rule_delete');
expect(auditLogger.log.mock.calls[0][0]?.event?.outcome).toEqual('unknown');
expect(auditLogger.log.mock.calls[0][0]?.kibana).toEqual({
saved_object: { id: 'id1', type: RULE_SAVED_OBJECT_TYPE },
saved_object: { id: 'id1', type: RULE_SAVED_OBJECT_TYPE, name: 'fakeName' },
});
expect(auditLogger.log.mock.calls[1][0]?.event?.action).toEqual('rule_delete');
expect(auditLogger.log.mock.calls[1][0]?.event?.outcome).toEqual('unknown');
expect(auditLogger.log.mock.calls[1][0]?.kibana).toEqual({
saved_object: { id: 'id2', type: RULE_SAVED_OBJECT_TYPE },
saved_object: { id: 'id2', type: RULE_SAVED_OBJECT_TYPE, name: 'fakeName' },
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,9 @@ const bulkDeleteWithOCC = async (
if (rule.attributes.apiKey && !rule.attributes.apiKeyCreatedByUser) {
apiKeyToRuleIdMapping[rule.id] = rule.attributes.apiKey;
}
if (rule.attributes.name) {
ruleNameToRuleIdMapping[rule.id] = rule.attributes.name;
const ruleName = rule.attributes.name;
if (ruleName) {
ruleNameToRuleIdMapping[rule.id] = ruleName;
}
if (rule.attributes.scheduledTaskId) {
taskIdToRuleIdMapping[rule.id] = rule.attributes.scheduledTaskId;
Expand All @@ -179,7 +180,11 @@ const bulkDeleteWithOCC = async (
ruleAuditEvent({
action: RuleAuditAction.DELETE,
outcome: 'unknown',
savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: rule.id },
savedObject: {
type: RULE_SAVED_OBJECT_TYPE,
id: rule.id,
name: ruleName,
},
})
);
}
Expand Down
Loading

0 comments on commit 45b4089

Please sign in to comment.