Skip to content

Commit

Permalink
adds non customizable field validation
Browse files Browse the repository at this point in the history
  • Loading branch information
dplumlee committed Oct 7, 2024
1 parent e4bb435 commit 9ef9c35
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,27 @@ describe('DetectionRulesClient.patchRule', () => {
expect(rulesClient.create).not.toHaveBeenCalled();
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Mock the existing rule
const existingRule = {
...getRulesSchemaMock(),
rule_source: { type: 'external', is_customized: true },
};
(getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule);

// Mock the rule update
const rulePatch = getCreateRulesSchemaMock('query-rule-id');
rulePatch.license = 'new license';

// Mock the rule returned after update; not used for this test directly but
// needed so that the patchRule method does not throw
rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams()));

await expect(detectionRulesClient.patchRule({ rulePatch })).rejects.toThrow(
'Cannot update "license" field for prebuilt rules'
);
});

describe('actions', () => {
it("updates the rule's actions if provided", async () => {
// Mock the existing rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,5 +498,26 @@ describe('DetectionRulesClient.updateRule', () => {
})
);
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Mock the existing rule
const existingRule = {
...getRulesSchemaMock(),
rule_source: { type: 'external', is_customized: true },
};

(getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule);

// Mock the rule update
const ruleUpdate = { ...getCreateRulesSchemaMock(), author: ['new user'] };

// Mock the rule returned after update; not used for this test directly but
// needed so that the patchRule method does not throw
rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams()));

await expect(detectionRulesClient.updateRule({ ruleUpdate })).rejects.toThrow(
'Cannot update "author" field for prebuilt rules'
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { MlAuthz } from '../../../../../machine_learning/authz';
import type { IPrebuiltRuleAssetsClient } from '../../../../prebuilt_rules/logic/rule_assets/prebuilt_rule_assets_client';
import { applyRulePatch } from '../mergers/apply_rule_patch';
import { getIdError } from '../../../utils/utils';
import { validateNonCustomizablePatchFields } from '../../../utils/validate';
import { convertAlertingRuleToRuleResponse } from '../converters/convert_alerting_rule_to_rule_response';
import { convertRuleResponseToAlertingRule } from '../converters/convert_rule_response_to_alerting_rule';
import { ClientError, toggleRuleEnabledOnUpdate, validateMlAuth } from '../utils';
Expand Down Expand Up @@ -51,6 +52,11 @@ export const patchRule = async ({

await validateMlAuth(mlAuthz, rulePatch.type ?? existingRule.type);

// We don't allow non-customizable fields to be changed for prebuilt rules
if (existingRule.rule_source && existingRule.rule_source.type === 'external') {
validateNonCustomizablePatchFields(rulePatch, existingRule);
}

const patchedRule = await applyRulePatch({
prebuiltRuleAssetClient,
existingRule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { RuleResponse } from '../../../../../../../common/api/detection_eng
import type { MlAuthz } from '../../../../../machine_learning/authz';
import { applyRuleUpdate } from '../mergers/apply_rule_update';
import { getIdError } from '../../../utils/utils';
import { validateNonCustomizableUpdateFields } from '../../../utils/validate';
import { convertRuleResponseToAlertingRule } from '../converters/convert_rule_response_to_alerting_rule';

import { ClientError, toggleRuleEnabledOnUpdate, validateMlAuth } from '../utils';
Expand Down Expand Up @@ -50,6 +51,11 @@ export const updateRule = async ({
throw new ClientError(error.message, error.statusCode);
}

// We don't allow non-customizable fields to be changed for prebuilt rules
if (existingRule.rule_source && existingRule.rule_source.type === 'external') {
validateNonCustomizableUpdateFields(ruleUpdate, existingRule);
}

const ruleWithUpdates = await applyRuleUpdate({
prebuiltRuleAssetClient,
existingRule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
EsqlRule,
NewTermsRule,
QueryRule,
RulePatchProps,
} from '../../../../../common/api/detection_engine';
import {
type ResponseAction,
Expand All @@ -38,6 +39,7 @@ import {
} from '../../rule_schema';
import { type BulkError, createBulkErrorObject } from '../../routes/utils';
import { internalRuleToAPIResponse } from '../logic/detection_rules_client/converters/internal_rule_to_api_response';
import { ClientError } from '../logic/detection_rules_client/utils';

export const transformValidateBulkError = (
ruleId: string,
Expand Down Expand Up @@ -128,3 +130,25 @@ function ruleObjectContainsResponseActions(
): rule is Rule<UnifiedQueryRuleParams | EsqlRuleParams | EqlRuleParams | NewTermsRuleParams> {
return rule != null && 'params' in rule && 'responseActions' in rule?.params;
}

export const validateNonCustomizableUpdateFields = (
ruleUpdate: RuleUpdateProps,
existingRule: RuleResponse
) => {
if (!isEqual(ruleUpdate.author, existingRule.author)) {
throw new ClientError(`Cannot update "author" field for prebuilt rules`, 400);
} else if (ruleUpdate.license !== existingRule.license) {
throw new ClientError(`Cannot update "license" field for prebuilt rules`, 400);
}
};

export const validateNonCustomizablePatchFields = (
rulePatch: RulePatchProps,
existingRule: RuleResponse
) => {
if (rulePatch.author && !isEqual(rulePatch.author, existingRule.author)) {
throw new ClientError(`Cannot update "author" field for prebuilt rules`, 400);
} else if (rulePatch.license && rulePatch.license !== existingRule.license) {
throw new ClientError(`Cannot update "license" field for prebuilt rules`, 400);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import {
removeServerGeneratedPropertiesIncludingRuleId,
getSimpleRuleOutputWithoutRuleId,
updateUsername,
createHistoricalPrebuiltRuleAssetSavedObjects,
installPrebuiltRules,
createRuleAssetSavedObject,
} from '../../../utils';
import {
createAlertsIndex,
Expand Down Expand Up @@ -238,6 +241,25 @@ export default ({ getService }: FtrProviderContext) => {
});
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Install base prebuilt detection rule
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
createRuleAssetSavedObject({ rule_id: 'rule-1', author: ['elastic'] }),
]);
await installPrebuiltRules(es, supertest);

const { body } = await securitySolutionApi
.patchRule({
body: {
rule_id: 'rule-1',
author: ['new user'],
},
})
.expect(400);

expect(body.message).toEqual('Cannot update "author" field for prebuilt rules');
});

describe('max signals', () => {
afterEach(async () => {
await deleteAllRules(supertest, log);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import {
getSimpleRuleOutputWithoutRuleId,
removeServerGeneratedPropertiesIncludingRuleId,
updateUsername,
createHistoricalPrebuiltRuleAssetSavedObjects,
installPrebuiltRules,
createRuleAssetSavedObject,
} from '../../../utils';
import {
createAlertsIndex,
Expand Down Expand Up @@ -347,6 +350,41 @@ export default ({ getService }: FtrProviderContext) => {
},
]);
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Install base prebuilt detection rule
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
createRuleAssetSavedObject({ rule_id: 'rule-1', author: ['elastic'] }),
createRuleAssetSavedObject({ rule_id: 'rule-2', license: 'basic' }),
]);
await installPrebuiltRules(es, supertest);

const { body } = await securitySolutionApi
.bulkPatchRules({
body: [
{ rule_id: 'rule-1', author: ['new user'] },
{ rule_id: 'rule-2', license: 'new license' },
],
})
.expect(200);

expect([body[0], body[1]]).toEqual([
{
error: {
message: 'Cannot update "author" field for prebuilt rules',
status_code: 400,
},
rule_id: 'rule-1',
},
{
error: {
message: 'Cannot update "license" field for prebuilt rules',
status_code: 400,
},
rule_id: 'rule-2',
},
]);
});
});
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import {
getSimpleMlRuleUpdate,
getSimpleRule,
updateUsername,
createHistoricalPrebuiltRuleAssetSavedObjects,
installPrebuiltRules,
createRuleAssetSavedObject,
} from '../../../utils';
import {
createAlertsIndex,
Expand Down Expand Up @@ -309,6 +312,33 @@ export default ({ getService }: FtrProviderContext) => {
expect(updatedRuleResponse).toMatchObject(expectedRule);
});
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Install base prebuilt detection rule
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
createRuleAssetSavedObject({ rule_id: 'rule-1', license: 'elastic' }),
]);
await installPrebuiltRules(es, supertest);

const { body: existingRule } = await securitySolutionApi
.readRule({
query: { rule_id: 'rule-1' },
})
.expect(200);

const { body } = await securitySolutionApi
.updateRule({
body: getCustomQueryRuleParams({
...existingRule,
rule_id: 'rule-1',
id: undefined,
license: 'new license',
}),
})
.expect(400);

expect(body.message).toEqual('Cannot update "license" field for prebuilt rules');
});
});
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
getSimpleRuleUpdate,
getSimpleRule,
updateUsername,
createHistoricalPrebuiltRuleAssetSavedObjects,
installPrebuiltRules,
createRuleAssetSavedObject,
} from '../../../utils';
import {
createAlertsIndex,
Expand Down Expand Up @@ -370,6 +373,30 @@ export default ({ getService }: FtrProviderContext) => {
},
]);
});

it('throws an error if rule has external rule source and non-customizable fields are changed', async () => {
// Install base prebuilt detection rule
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
createRuleAssetSavedObject({ rule_id: 'rule-1', author: ['elastic'] }),
]);
await installPrebuiltRules(es, supertest);

const { body } = await securitySolutionApi
.bulkUpdateRules({
body: [getCustomQueryRuleParams({ rule_id: 'rule-1', author: ['new user'] })],
})
.expect(200);

expect([body[0]]).toEqual([
{
error: {
message: 'Cannot update "author" field for prebuilt rules',
status_code: 400,
},
rule_id: 'rule-1',
},
]);
});
});
});
};

0 comments on commit 9ef9c35

Please sign in to comment.