Skip to content

Commit

Permalink
[Security Solution] "Data view" selector is shown in "Edit filter" vi…
Browse files Browse the repository at this point in the history 
…ew on the Rule Editing page (#174026) (#174922)

## Summary

Addresses #174026

These changes fix the issue with filter editing on the rule's editing
page when using index patterns instead of data view.

**Steps to reproduce**:
1. Create a custom query rule and add a filter
2. Save the rule
3. Edit the rule
4. Edit the filter

**Current behaviour**:
Right now when user tries to edit the filter the data view picking UI
appears even though index patterns were not modified.

<img width="1261" alt="Screenshot 2024-01-16 at 15 14 23"
src="https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25">

**Expected behaviour**:
Data view picking UI should not be present and previously set field and
value options should be shown in the filter editing dialog.

<img width="1252" alt="Screenshot 2024-01-16 at 15 16 07"
src="https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c">

**Cause**:
The behaviour for the filter editing on rule’s editing page changed in
`8.11` with these changes #166318.
We convert `DataViewBase` object without ID set to a `DataView` object
with auto-generated ID. This happens each time we try to edit the rule
and leads to a different ID which is saved in `filter.meta.index`.
Unified search internally checks those IDs to verify whether the filter
belongs to provided data view.

**Solution**:
To solve this issue, we set the data view id explicitly on creating an
in-memory data view that represents index patterns and update
`filter.meta.index` to use the same ID.

~~**Known issue**:
This does not resolve the issue for existing filters. In this case, user
will need to update their filters manually.~~ (This was fixed by
updating `filter.meta.index` field on rule editing)

**Flaky test runner**
[ESS 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)
[Serverless 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@e40pud @kibanamachine
e40pud and kibanamachine authored Jan 24, 2024
1 parent 4cc7583 commit f0f6274
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 59 deletions.
10 changes: 8 additions & 2 deletions x-pack/plugins/security_solution/public/common/components/query_bar/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,14 @@ export const QueryBar = memo<QueryBarComponentProps>(
setDataView(indexPattern);
} else if (!isEsql) {
const createDataView = async () => {
dv = await data.dataViews.create({ title: indexPattern.title });
dv = await data.dataViews.create({ id: indexPattern.title, title: indexPattern.title });
setDataView(dv);

/**
* We update filters and set new data view id to make sure that SearchBar does not show data view picker
* More details in https://github.com/elastic/kibana/issues/174026
*/
filters.forEach((filter) => (filter.meta.index = indexPattern.title));
};
createDataView();
}
Expand All @@ -136,7 +142,7 @@ export const QueryBar = memo<QueryBarComponentProps>(
data.dataViews.clearInstanceCache(dv?.id);
}
};
}, [data.dataViews, indexPattern, isEsql]);
}, [data.dataViews, filters, indexPattern, isEsql]);

const timeHistory = useMemo(() => new TimeHistory(new Storage(localStorage)), []);
const arrDataView = useMemo(() => (dataView != null ? [dataView] : []), [dataView]);
Expand Down
176 changes: 119 additions & 57 deletions ...cypress/cypress/e2e/detection_response/detection_engine/rule_edit/custom_query_rule.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
* 2.0.
*/

import {
ADD_FILTER_FORM_FIELD_INPUT,
ADD_FILTER_FORM_OPERATOR_FIELD,
GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM,
GLOBAL_SEARCH_BAR_FILTER_ITEM,
} from '../../../../screens/search_bar';
import { getExistingRule, getEditedRule } from '../../../../objects/rule';

import {
Expand Down Expand Up @@ -63,81 +69,137 @@ describe('Custom query rules', { tags: ['@ess', '@serverless', '@brokenInServerl
deleteConnectors();
deleteAlertsAndRules();
login();
createRule(getExistingRule({ rule_id: 'rule1', enabled: true })).then((createdRule) => {
visitEditRulePage(createdRule.body.id);
});
});

it('Allows a rule to be edited', () => {
const existingRule = getExistingRule();
context('Basics', () => {
beforeEach(() => {
createRule(getExistingRule({ rule_id: 'rule1', enabled: true })).then((createdRule) => {
visitEditRulePage(createdRule.body.id);
});
});

it('Allows a rule to be edited', () => {
const existingRule = getExistingRule();

// expect define step to populate
cy.get(CUSTOM_QUERY_INPUT).should('have.value', existingRule.query);

// expect define step to populate
cy.get(CUSTOM_QUERY_INPUT).should('have.value', existingRule.query);
cy.get(DEFINE_INDEX_INPUT).should('have.text', existingRule.index?.join(''));

cy.get(DEFINE_INDEX_INPUT).should('have.text', existingRule.index?.join(''));
goToAboutStepTab();

goToAboutStepTab();
// expect about step to populate
cy.get(RULE_NAME_INPUT).invoke('val').should('eql', existingRule.name);
cy.get(RULE_DESCRIPTION_INPUT).should('have.text', existingRule.description);
cy.get(TAGS_FIELD).should('have.text', existingRule.tags?.join(''));
cy.get(SEVERITY_DROPDOWN).should('have.text', 'High');
cy.get(DEFAULT_RISK_SCORE_INPUT).invoke('val').should('eql', `${existingRule.risk_score}`);

// expect about step to populate
cy.get(RULE_NAME_INPUT).invoke('val').should('eql', existingRule.name);
cy.get(RULE_DESCRIPTION_INPUT).should('have.text', existingRule.description);
cy.get(TAGS_FIELD).should('have.text', existingRule.tags?.join(''));
cy.get(SEVERITY_DROPDOWN).should('have.text', 'High');
cy.get(DEFAULT_RISK_SCORE_INPUT).invoke('val').should('eql', `${existingRule.risk_score}`);
goToScheduleStepTab();

goToScheduleStepTab();
// expect schedule step to populate
const interval = existingRule.interval;
const intervalParts = interval != null && interval.match(/[0-9]+|[a-zA-Z]+/g);
if (intervalParts) {
const [amount, unit] = intervalParts;
cy.get(SCHEDULE_INTERVAL_AMOUNT_INPUT).invoke('val').should('eql', amount);
cy.get(SCHEDULE_INTERVAL_UNITS_INPUT).invoke('val').should('eql', unit);
} else {
throw new Error('Cannot assert scheduling info on a rule without an interval');
}

// expect schedule step to populate
const interval = existingRule.interval;
const intervalParts = interval != null && interval.match(/[0-9]+|[a-zA-Z]+/g);
if (intervalParts) {
const [amount, unit] = intervalParts;
cy.get(SCHEDULE_INTERVAL_AMOUNT_INPUT).invoke('val').should('eql', amount);
cy.get(SCHEDULE_INTERVAL_UNITS_INPUT).invoke('val').should('eql', unit);
} else {
throw new Error('Cannot assert scheduling info on a rule without an interval');
}
goToActionsStepTab();

goToActionsStepTab();
addEmailConnectorAndRuleAction('[email protected]', 'Subject');

addEmailConnectorAndRuleAction('[email protected]', 'Subject');
cy.get(ACTIONS_SUMMARY_BUTTON).should('have.text', 'Summary of alerts');
cy.get(ACTIONS_NOTIFY_WHEN_BUTTON).should('have.text', 'Per rule run');

cy.get(ACTIONS_SUMMARY_BUTTON).should('have.text', 'Summary of alerts');
cy.get(ACTIONS_NOTIFY_WHEN_BUTTON).should('have.text', 'Per rule run');
goToAboutStepTab();
cy.get(TAGS_CLEAR_BUTTON).click();
fillAboutRule(getEditedRule());

goToAboutStepTab();
cy.get(TAGS_CLEAR_BUTTON).click();
fillAboutRule(getEditedRule());
cy.intercept('GET', '/api/detection_engine/rules?id*').as('getRule');

cy.intercept('GET', '/api/detection_engine/rules?id*').as('getRule');
saveEditedRule();

saveEditedRule();
cy.wait('@getRule').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
// ensure that editing rule does not modify max_signals
cy.wrap(response?.body.max_signals).should('eql', existingRule.max_signals);
});

cy.wait('@getRule').then(({ response }) => {
cy.wrap(response?.statusCode).should('eql', 200);
// ensure that editing rule does not modify max_signals
cy.wrap(response?.body.max_signals).should('eql', existingRule.max_signals);
cy.get(RULE_NAME_HEADER).should('contain', `${getEditedRule().name}`);
cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', getEditedRule().description);
cy.get(ABOUT_DETAILS).within(() => {
getDetails(SEVERITY_DETAILS).should('have.text', 'Medium');
getDetails(RISK_SCORE_DETAILS).should('have.text', `${getEditedRule().risk_score}`);
getDetails(TAGS_DETAILS).should('have.text', expectedEditedtags);
});
cy.get(INVESTIGATION_NOTES_TOGGLE).click();
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', getEditedRule().note);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should(
'have.text',
expectedEditedIndexPatterns?.join('')
);
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', getEditedRule().query);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
});
if (getEditedRule().interval) {
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', getEditedRule().interval);
});
}
});
});

cy.get(RULE_NAME_HEADER).should('contain', `${getEditedRule().name}`);
cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', getEditedRule().description);
cy.get(ABOUT_DETAILS).within(() => {
getDetails(SEVERITY_DETAILS).should('have.text', 'Medium');
getDetails(RISK_SCORE_DETAILS).should('have.text', `${getEditedRule().risk_score}`);
getDetails(TAGS_DETAILS).should('have.text', expectedEditedtags);
context('With filters', () => {
beforeEach(() => {
createRule(
getExistingRule({
rule_id: 'rule1',
enabled: true,
filters: [
{
meta: {
disabled: false,
negate: false,
alias: null,
index: expectedEditedIndexPatterns?.join(','),
key: 'host.name',
field: 'host.name',
type: 'exists',
value: 'exists',
},
query: {
exists: {
field: 'host.name',
},
},
$state: {
store: 'appState',
},
},
],
})
).then((createdRule) => {
visitEditRulePage(createdRule.body.id);
});
});
cy.get(INVESTIGATION_NOTES_TOGGLE).click();
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', getEditedRule().note);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', expectedEditedIndexPatterns?.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', getEditedRule().query);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Query');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');

it('Filter properly stores index information', () => {
// Check that filter exists on rule edit page
cy.get(GLOBAL_SEARCH_BAR_FILTER_ITEM).should('have.text', 'host.name: exists');

// Edit the filter
cy.get(GLOBAL_SEARCH_BAR_FILTER_ITEM).click();
cy.get(GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM).click();

// Check that correct values are propagated in the filter editing dialog
cy.get(ADD_FILTER_FORM_FIELD_INPUT).should('have.value', 'host.name');
cy.get(ADD_FILTER_FORM_OPERATOR_FIELD).should('have.value', 'exists');
});
if (getEditedRule().interval) {
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should('have.text', getEditedRule().interval);
});
}
});
});
2 changes: 2 additions & 0 deletions x-pack/test/security_solution_cypress/cypress/screens/search_bar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export const GLOBAL_SEARCH_BAR_FILTER_ITEM_DELETE = '#popoverFor_filter0 button[

export const GLOBAL_SEARCH_BAR_PINNED_FILTER = '.globalFilterItem-isPinned';

export const GLOBAL_SEARCH_BAR_EDIT_FILTER_MENU_ITEM = '[data-test-subj="editFilter"]';

export const LOCAL_KQL_INPUT = `[data-test-subj="unifiedQueryInput"] textarea`;

export const GLOBAL_KQL_INPUT = `[data-test-subj="filters-global-container"] ${LOCAL_KQL_INPUT}`;
Expand Down

0 comments on commit f0f6274

Please sign in to comment.