Skip to content

Commit ef0676b

Browse files
[SIEM][Detection Engine] Fixes return codes where some were rule_id instead of id (#58191)
## Summary Fixes some return error codes where they were `rule_id` when they should have been `id` - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
1 parent 7c371a2 commit ef0676b

File tree

8 files changed

+72
-26
lines changed

8 files changed

+72
-26
lines changed

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe('import_rules_route', () => {
210210
message: 'Unexpected token : in JSON at position 8',
211211
status_code: 400,
212212
},
213-
rule_id: '(unknown)',
213+
rule_id: '(unknown id)',
214214
},
215215
],
216216
success: false,
@@ -329,7 +329,7 @@ describe('import_rules_route', () => {
329329
message: 'Unexpected token : in JSON at position 8',
330330
status_code: 400,
331331
},
332-
rule_id: '(unknown)',
332+
rule_id: '(unknown id)',
333333
},
334334
],
335335
success: false,

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ export const createImportRulesRoute = (
9595
// early with the error and an (unknown) for the ruleId
9696
resolve(
9797
createBulkErrorObject({
98-
ruleId: '(unknown)',
9998
statusCode: 400,
10099
message: parsedRule.message,
101100
})

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/utils.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -794,10 +794,20 @@ describe('utils', () => {
794794
});
795795

796796
describe('getIdBulkError', () => {
797+
test('outputs message about id and rule_id not being found if both are not null', () => {
798+
const error = getIdBulkError({ id: '123', ruleId: '456' });
799+
const expected: BulkError = {
800+
id: '123',
801+
rule_id: '456',
802+
error: { message: 'id: "123" and rule_id: "456" not found', status_code: 404 },
803+
};
804+
expect(error).toEqual(expected);
805+
});
806+
797807
test('outputs message about id not being found if only id is defined and ruleId is undefined', () => {
798808
const error = getIdBulkError({ id: '123', ruleId: undefined });
799809
const expected: BulkError = {
800-
rule_id: '123',
810+
id: '123',
801811
error: { message: 'id: "123" not found', status_code: 404 },
802812
};
803813
expect(error).toEqual(expected);
@@ -806,7 +816,7 @@ describe('utils', () => {
806816
test('outputs message about id not being found if only id is defined and ruleId is null', () => {
807817
const error = getIdBulkError({ id: '123', ruleId: null });
808818
const expected: BulkError = {
809-
rule_id: '123',
819+
id: '123',
810820
error: { message: 'id: "123" not found', status_code: 404 },
811821
};
812822
expect(error).toEqual(expected);

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/utils.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,16 @@ export const getIdBulkError = ({
6262
id: string | undefined | null;
6363
ruleId: string | undefined | null;
6464
}): BulkError => {
65-
if (id != null) {
65+
if (id != null && ruleId != null) {
66+
return createBulkErrorObject({
67+
id,
68+
ruleId,
69+
statusCode: 404,
70+
message: `id: "${id}" and rule_id: "${ruleId}" not found`,
71+
});
72+
} else if (id != null) {
6673
return createBulkErrorObject({
67-
ruleId: id,
74+
id,
6875
statusCode: 404,
6976
message: `id: "${id}" not found`,
7077
});
@@ -76,7 +83,6 @@ export const getIdBulkError = ({
7683
});
7784
} else {
7885
return createBulkErrorObject({
79-
ruleId: '(unknown id)',
8086
statusCode: 404,
8187
message: `id or rule_id should have been defined`,
8288
});

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/utils.ts

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export const transformError = (err: Error & { statusCode?: number }): OutputErro
4444
};
4545

4646
export interface BulkError {
47-
rule_id: string;
47+
id?: string;
48+
rule_id?: string;
4849
error: {
4950
status_code: number;
5051
message: string;
@@ -53,24 +54,54 @@ export interface BulkError {
5354

5455
export const createBulkErrorObject = ({
5556
ruleId,
57+
id,
5658
statusCode,
5759
message,
5860
}: {
59-
ruleId: string;
61+
ruleId?: string;
62+
id?: string;
6063
statusCode: number;
6164
message: string;
6265
}): BulkError => {
63-
return {
64-
rule_id: ruleId,
65-
error: {
66-
status_code: statusCode,
67-
message,
68-
},
69-
};
66+
if (id != null && ruleId != null) {
67+
return {
68+
id,
69+
rule_id: ruleId,
70+
error: {
71+
status_code: statusCode,
72+
message,
73+
},
74+
};
75+
} else if (id != null) {
76+
return {
77+
id,
78+
error: {
79+
status_code: statusCode,
80+
message,
81+
},
82+
};
83+
} else if (ruleId != null) {
84+
return {
85+
rule_id: ruleId,
86+
error: {
87+
status_code: statusCode,
88+
message,
89+
},
90+
};
91+
} else {
92+
return {
93+
rule_id: '(unknown id)',
94+
error: {
95+
status_code: statusCode,
96+
message,
97+
},
98+
};
99+
}
70100
};
71101

72102
export interface ImportRuleResponse {
73-
rule_id: string;
103+
rule_id?: string;
104+
id?: string;
74105
status_code?: number;
75106
message?: string;
76107
error?: {

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/delete_rules_bulk.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export default ({ getService }: FtrProviderContext): void => {
129129
message: 'id: "fake_id" not found',
130130
status_code: 404,
131131
},
132-
rule_id: 'fake_id', // TODO This is a known issue where it should be id and not rule_id
132+
id: 'fake_id',
133133
},
134134
]);
135135
});
@@ -152,7 +152,7 @@ export default ({ getService }: FtrProviderContext): void => {
152152
const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body[0]);
153153
expect([bodyToCompare, body[1]]).to.eql([
154154
getSimpleRuleOutputWithoutRuleId(),
155-
{ rule_id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
155+
{ id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
156156
]);
157157
});
158158
});
@@ -262,7 +262,7 @@ export default ({ getService }: FtrProviderContext): void => {
262262
message: 'id: "fake_id" not found',
263263
status_code: 404,
264264
},
265-
rule_id: 'fake_id', // TODO This is a known issue where it should be id and not rule_id
265+
id: 'fake_id',
266266
},
267267
]);
268268
});
@@ -285,7 +285,7 @@ export default ({ getService }: FtrProviderContext): void => {
285285
const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body[0]);
286286
expect([bodyToCompare, body[1]]).to.eql([
287287
getSimpleRuleOutputWithoutRuleId(),
288-
{ rule_id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
288+
{ id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
289289
]);
290290
});
291291
});

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules_bulk.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ export default ({ getService }: FtrProviderContext) => {
263263
.expect(200);
264264

265265
expect(body).to.eql([
266-
{ rule_id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
266+
{ id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
267267
]);
268268
});
269269

@@ -347,7 +347,7 @@ export default ({ getService }: FtrProviderContext) => {
347347
message: 'id: "fake_id" not found',
348348
status_code: 404,
349349
},
350-
rule_id: 'fake_id', // TODO: This should be id and not rule_id in the codebase
350+
id: 'fake_id',
351351
},
352352
]);
353353
});

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ export default ({ getService }: FtrProviderContext) => {
277277
.expect(200);
278278

279279
expect(body).to.eql([
280-
{ rule_id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
280+
{ id: 'fake_id', error: { status_code: 404, message: 'id: "fake_id" not found' } },
281281
]);
282282
});
283283

@@ -377,7 +377,7 @@ export default ({ getService }: FtrProviderContext) => {
377377
message: 'id: "fake_id" not found',
378378
status_code: 404,
379379
},
380-
rule_id: 'fake_id', // TODO: This should be id and not rule_id in the codebase
380+
id: 'fake_id',
381381
},
382382
]);
383383
});

0 commit comments

Comments
 (0)