From f9627d5b9eea029b1ec37e8955774fba2dcb47f8 Mon Sep 17 00:00:00 2001 From: Amardeepsingh Siglani Date: Tue, 31 Jan 2023 10:22:53 -0800 Subject: [PATCH] Backport PRs #377, #379 (#390) * Feature/add role edit cypress tests (#377) * [FEATURE] Detector must have at least one alert set #288 Signed-off-by: Jovan Cvetkovic * [FEATURE] Implement additional integration test cases #104 Signed-off-by: Jovan Cvetkovic * [FEATURE] Implement additional integration test cases #104 Signed-off-by: Jovan Cvetkovic * [FEATURE] Implement additional integration test cases #104 Signed-off-by: Jovan Cvetkovic Signed-off-by: Jovan Cvetkovic * Bugfix/interval field can be empty (#379) * [FEATURE] Detector must have at least one alert set #288 Signed-off-by: Jovan Cvetkovic * [BUG] Create detector | Interval field can be empty #378 Signed-off-by: Jovan Cvetkovic * fix PR failed tests Signed-off-by: Jovan Cvetkovic * fix PR failed tests Signed-off-by: Jovan Cvetkovic * unit tests Signed-off-by: Jovan Cvetkovic * unit tests Signed-off-by: Jovan Cvetkovic * testing github-action v5 Signed-off-by: Jovan Cvetkovic * testing github-action v5 Signed-off-by: Jovan Cvetkovic * testing github-action v5 Signed-off-by: Jovan Cvetkovic --------- Signed-off-by: Jovan Cvetkovic --------- Signed-off-by: Jovan Cvetkovic Co-authored-by: Jovan Cvetkovic --- .github/workflows/cypress-workflow.yml | 4 +- cypress.json | 6 +- cypress/integration/1_detectors.spec.js | 21 +- cypress/integration/2_rules.spec.js | 211 ++++++++++++------ cypress/integration/3_alerts.spec.js | 3 + cypress/support/detectors.js | 3 + cypress/support/indexes.js | 10 +- cypress/support/rules.js | 6 +- cypress/support/typings.js | 2 +- .../components/DetectorSchedule/Interval.tsx | 22 +- .../containers/DefineDetector.tsx | 3 +- 11 files changed, 207 insertions(+), 84 deletions(-) diff --git a/.github/workflows/cypress-workflow.yml b/.github/workflows/cypress-workflow.yml index 9d134ef38..1b5a3c34f 100644 --- a/.github/workflows/cypress-workflow.yml +++ b/.github/workflows/cypress-workflow.yml @@ -111,10 +111,12 @@ jobs: with: path: ${{ matrix.cypress_cache_folder }} key: cypress-cache-v2-${{ runner.os }}-${{ hashFiles('**/package.json') }} + restore-keys: | + cypress-cache-v2-${{ runner.os }}-${{ hashFiles('**/package.json') }} # for now just chrome, use matrix to do all browsers later - name: Cypress tests - uses: cypress-io/github-action@v2 + uses: cypress-io/github-action@v5 with: working-directory: OpenSearch-Dashboards/plugins/security-analytics-dashboards-plugin command: yarn run cypress run diff --git a/cypress.json b/cypress.json index 56235296e..48f248ec8 100644 --- a/cypress.json +++ b/cypress.json @@ -1,8 +1,10 @@ { "viewportHeight": 900, "viewportWidth": 1440, - "defaultCommandTimeout": 20000, - "retries": 1, + "defaultCommandTimeout": 30000, + "requestTimeout": 300000, + "responseTimeout": 300000, + "baseUrl": "http://localhost:5601", "env": { "opensearch_url": "localhost:9200", "opensearch_dashboards": "http://localhost:5601", diff --git a/cypress/integration/1_detectors.spec.js b/cypress/integration/1_detectors.spec.js index 74dd7119b..4f5a80090 100644 --- a/cypress/integration/1_detectors.spec.js +++ b/cypress/integration/1_detectors.spec.js @@ -38,14 +38,29 @@ describe('Detectors', () => { before(() => { cy.cleanUpTests(); // Create test index - cy.createIndex(indexName, sample_index_settings); + cy.createIndex(indexName, sample_index_settings).then(() => + cy + .request('POST', '_plugins/_security_analytics/rules/_search?prePackaged=true', { + from: 0, + size: 5000, + query: { + nested: { + path: 'rule', + query: { bool: { must: [{ match: { 'rule.category': 'windows' } }] } }, + }, + }, + }) + .should('have.property', 'status', 200) + ); cy.contains(detectorName).should('not.exist'); }); beforeEach(() => { + cy.intercept('/detectors/_search').as('detectorsSearch'); // Visit Detectors page cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/detectors`); + cy.wait('@detectorsSearch').should('have.property', 'state', 'Complete'); // Check that correct page is showing cy.waitForPageLoad('detectors', { @@ -85,6 +100,10 @@ describe('Detectors', () => { // Open Detection rules accordion cy.get('[data-test-subj="detection-rules-btn"]').click({ force: true, timeout: 5000 }); + cy.contains('table tr', 'Windows', { + timeout: 120000, + }); + // find search, type USB cy.get(`input[placeholder="Search..."]`).ospSearch('USB Device Plugged'); diff --git a/cypress/integration/2_rules.spec.js b/cypress/integration/2_rules.spec.js index 70fe548ba..e0d34deda 100644 --- a/cypress/integration/2_rules.spec.js +++ b/cypress/integration/2_rules.spec.js @@ -48,11 +48,93 @@ const YAML_RULE_LINES = [ ...SAMPLE_RULE.detection.replaceAll(' ', '').replaceAll('{backspace}', '').split('\n'), ]; +const checkRulesFlyout = () => { + // Search for the rule + cy.get(`input[placeholder="Search rules"]`).ospSearch(SAMPLE_RULE.name); + + // Click the rule link to open the details flyout + cy.get(`[data-test-subj="rule_link_${SAMPLE_RULE.name}"]`).click({ force: true }); + + // Confirm the flyout contains the expected values + cy.get(`[data-test-subj="rule_flyout_${SAMPLE_RULE.name}"]`) + .click({ force: true }) + .within(() => { + // Validate name + cy.get('[data-test-subj="rule_flyout_rule_name"]').contains(SAMPLE_RULE.name); + + // Validate log type + cy.get('[data-test-subj="rule_flyout_rule_log_type"]').contains(SAMPLE_RULE.logType); + + // Validate description + cy.get('[data-test-subj="rule_flyout_rule_description"]').contains(SAMPLE_RULE.description); + + // Validate author + cy.get('[data-test-subj="rule_flyout_rule_author"]').contains(SAMPLE_RULE.author); + + // Validate source is "custom" + cy.get('[data-test-subj="rule_flyout_rule_source"]').contains('Custom'); + + // Validate severity + cy.get('[data-test-subj="rule_flyout_rule_severity"]').contains(SAMPLE_RULE.severity); + + // Validate tags + SAMPLE_RULE.tags.forEach((tag) => + cy.get('[data-test-subj="rule_flyout_rule_tags"]').contains(tag) + ); + + // Validate references + cy.get('[data-test-subj="rule_flyout_rule_references"]').contains(SAMPLE_RULE.references); + + // Validate false positives + cy.get('[data-test-subj="rule_flyout_rule_false_positives"]').contains( + SAMPLE_RULE.falsePositive + ); + + // Validate status + cy.get('[data-test-subj="rule_flyout_rule_status"]').contains(SAMPLE_RULE.status); + + // Validate detection + SAMPLE_RULE.detectionLine.forEach((line) => + cy.get('[data-test-subj="rule_flyout_rule_detection"]').contains(line) + ); + + cy.get('[data-test-subj="change-editor-type"] label:nth-child(2)').click({ + force: true, + }); + + cy.get('[data-test-subj="rule_flyout_yaml_rule"]') + .get('[class="euiCodeBlock__line"]') + .each((lineElement, lineIndex) => { + if (lineIndex >= YAML_RULE_LINES.length) { + return; + } + let line = lineElement.text().replaceAll('\n', '').trim(); + let expectedLine = YAML_RULE_LINES[lineIndex]; + + // The document ID field is generated when the document is added to the index, + // so this test just checks that the line starts with the ID key. + if (expectedLine.startsWith('id:')) { + expectedLine = 'id:'; + expect(line, `Sigma rule line ${lineIndex}`).to.contain(expectedLine); + } else { + expect(line, `Sigma rule line ${lineIndex}`).to.equal(expectedLine); + } + }); + + // Close the flyout + cy.get('[data-test-subj="close-rule-details-flyout"]').click({ + force: true, + }); + }); +}; + describe('Rules', () => { before(() => cy.cleanUpTests()); beforeEach(() => { + cy.intercept('/rules/_search').as('rulesSearch'); // Visit Rules page cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/rules`); + cy.wait('@rulesSearch').should('have.property', 'state', 'Complete'); // Check that correct page is showing cy.waitForPageLoad('rules', { @@ -114,89 +196,73 @@ describe('Rules', () => { force: true, }); + cy.wait('@getRules'); + cy.waitForPageLoad('rules', { contains: 'Rules', }); - cy.wait('@getRules'); + checkRulesFlyout(); + }); - // Search for the rule - cy.get(`input[placeholder="Search rules"]`).ospSearch(SAMPLE_RULE.name); + it('...can be edited', () => { + cy.waitForPageLoad('rules', { + contains: 'Rules', + }); - // Click the rule link to open the details flyout + cy.get(`input[placeholder="Search rules"]`).ospSearch(SAMPLE_RULE.name); cy.get(`[data-test-subj="rule_link_${SAMPLE_RULE.name}"]`).click({ force: true }); - // Confirm the flyout contains the expected values cy.get(`[data-test-subj="rule_flyout_${SAMPLE_RULE.name}"]`) + .find('button') + .contains('Action') .click({ force: true }) - .within(() => { - // Validate name - cy.get('[data-test-subj="rule_flyout_rule_name"]').contains(SAMPLE_RULE.name); - - // Validate log type - cy.get('[data-test-subj="rule_flyout_rule_log_type"]').contains(SAMPLE_RULE.logType); - - // Validate description - cy.get('[data-test-subj="rule_flyout_rule_description"]').contains(SAMPLE_RULE.description); - - // Validate author - cy.get('[data-test-subj="rule_flyout_rule_author"]').contains(SAMPLE_RULE.author); + .then(() => { + // Confirm arrival at detectors page + cy.get('.euiPopover__panel').find('button').contains('Edit').click(); + }); - // Validate source is "custom" - cy.get('[data-test-subj="rule_flyout_rule_source"]').contains('Custom'); + const ruleNameSelector = '[data-test-subj="rule_name_field"]'; + cy.get(ruleNameSelector).clear(); - // Validate severity - cy.get('[data-test-subj="rule_flyout_rule_severity"]').contains(SAMPLE_RULE.severity); + SAMPLE_RULE.name += ' edited'; + cy.get(ruleNameSelector).type(SAMPLE_RULE.name); + cy.get(ruleNameSelector).should('have.value', SAMPLE_RULE.name); - // Validate tags - SAMPLE_RULE.tags.forEach((tag) => - cy.get('[data-test-subj="rule_flyout_rule_tags"]').contains(tag) - ); + // Enter the log type + const logSelector = '[data-test-subj="rule_type_dropdown"]'; + cy.get(logSelector).within(() => cy.get('.euiFormControlLayoutClearButton').click()); + SAMPLE_RULE.logType = 'dns'; + YAML_RULE_LINES[2] = `product: ${SAMPLE_RULE.logType}`; + YAML_RULE_LINES[3] = `title: ${SAMPLE_RULE.name}`; + cy.get(logSelector).type(SAMPLE_RULE.logType).type('{enter}'); + cy.get(logSelector).contains(SAMPLE_RULE.logType, { + matchCase: false, + }); - // Validate references - cy.get('[data-test-subj="rule_flyout_rule_references"]').contains(SAMPLE_RULE.references); + const ruleDescriptionSelector = '[data-test-subj="rule_description_field"]'; + SAMPLE_RULE.description += ' edited'; + YAML_RULE_LINES[4] = `description: ${SAMPLE_RULE.description}`; + cy.get(ruleDescriptionSelector).clear(); + cy.get(ruleDescriptionSelector).type(SAMPLE_RULE.description); + cy.get(ruleDescriptionSelector).should('have.value', SAMPLE_RULE.description); - // Validate false positives - cy.get('[data-test-subj="rule_flyout_rule_false_positives"]').contains( - SAMPLE_RULE.falsePositive - ); + cy.intercept({ + url: '/rules', + }).as('getRules'); - // Validate status - cy.get('[data-test-subj="rule_flyout_rule_status"]').contains(SAMPLE_RULE.status); + // Click "create" button + cy.get('[data-test-subj="submit_rule_form_button"]').click({ + force: true, + }); - // Validate detection - SAMPLE_RULE.detectionLine.forEach((line) => - cy.get('[data-test-subj="rule_flyout_rule_detection"]').contains(line) - ); + cy.waitForPageLoad('rules', { + contains: 'Rules', + }); - cy.get('[data-test-subj="change-editor-type"] label:nth-child(2)').click({ - force: true, - }); + cy.wait('@getRules'); - cy.get('[data-test-subj="rule_flyout_yaml_rule"]') - .get('[class="euiCodeBlock__line"]') - .each((lineElement, lineIndex) => { - if (lineIndex >= YAML_RULE_LINES.length) { - return; - } - let line = lineElement.text().replaceAll('\n', '').trim(); - let expectedLine = YAML_RULE_LINES[lineIndex]; - - // The document ID field is generated when the document is added to the index, - // so this test just checks that the line starts with the ID key. - if (expectedLine.startsWith('id:')) { - expectedLine = 'id:'; - expect(line, `Sigma rule line ${lineIndex}`).to.contain(expectedLine); - } else { - expect(line, `Sigma rule line ${lineIndex}`).to.equal(expectedLine); - } - }); - - // Close the flyout - cy.get('[data-test-subj="close-rule-details-flyout"]').click({ - force: true, - }); - }); + checkRulesFlyout(); }); it('...can be deleted', () => { @@ -209,20 +275,17 @@ describe('Rules', () => { // Click the rule link to open the details flyout cy.get(`[data-test-subj="rule_link_${SAMPLE_RULE.name}"]`).click({ force: true }); - cy.get('.euiButton') + cy.get(`[data-test-subj="rule_flyout_${SAMPLE_RULE.name}"]`) + .find('button') .contains('Action') .click({ force: true }) .then(() => { // Confirm arrival at detectors page - cy.get( - '.euiFlexGroup > :nth-child(3) > .euiButtonEmpty > .euiButtonContent > .euiButtonEmpty__text' - ) - .click({ - force: true, - }) - .then(() => { - cy.get('.euiButton').contains('Delete').click(); - }); + cy.get('.euiPopover__panel') + .find('button') + .contains('Delete') + .click() + .then(() => cy.get('.euiModalFooter > .euiButton').contains('Delete').click()); cy.wait('@deleteRule'); diff --git a/cypress/integration/3_alerts.spec.js b/cypress/integration/3_alerts.spec.js index 0dfb6e0cd..77f125384 100644 --- a/cypress/integration/3_alerts.spec.js +++ b/cypress/integration/3_alerts.spec.js @@ -84,7 +84,10 @@ describe('Alerts', () => { beforeEach(() => { // Visit Alerts table page + cy.intercept('/detectors/_search').as('detectorsSearch'); + // Visit Detectors page cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/alerts`); + cy.wait('@detectorsSearch').should('have.property', 'state', 'Complete'); // Wait for page to load cy.waitForPageLoad('alerts', { diff --git a/cypress/support/detectors.js b/cypress/support/detectors.js index b0ec6c60f..22f53c45b 100644 --- a/cypress/support/detectors.js +++ b/cypress/support/detectors.js @@ -68,5 +68,8 @@ Cypress.Commands.add('deleteAllDetectors', () => { method: 'DELETE', url: `${Cypress.env('opensearch')}/.opensearch-sap-detectors-config`, failOnStatusCode: false, + }).as('deleteAllDetectors'); + cy.get('@deleteAllDetectors').should((response) => { + expect(response.status).to.be.oneOf([200, 404]); }); }); diff --git a/cypress/support/indexes.js b/cypress/support/indexes.js index 1ffc22116..1e9f5b498 100644 --- a/cypress/support/indexes.js +++ b/cypress/support/indexes.js @@ -6,7 +6,11 @@ const { NODE_API } = require('./constants'); Cypress.Commands.add('createIndex', (index, settings = {}) => { - cy.request('PUT', `${Cypress.env('opensearch')}/${index}`, settings); + cy.request('PUT', `${Cypress.env('opensearch')}/${index}`, settings).should( + 'have.property', + 'status', + 200 + ); }); Cypress.Commands.add('createIndexTemplate', (name, template) => { @@ -43,5 +47,9 @@ Cypress.Commands.add('deleteAllIndices', () => { method: 'DELETE', url: `${Cypress.env('opensearch')}/index*,sample*,opensearch_dashboards*,test*,cypress*`, failOnStatusCode: false, + }).as('deleteAllIndices'); + cy.get('@deleteAllIndices').should((response) => { + // Both statuses are a pass, 200 means deleted successfully and 404 there was no index to delete + expect(response.status).to.be.oneOf([200, 404]); }); }); diff --git a/cypress/support/rules.js b/cypress/support/rules.js index 46f01b3b7..8dada4fd1 100644 --- a/cypress/support/rules.js +++ b/cypress/support/rules.js @@ -51,10 +51,14 @@ Cypress.Commands.add('deleteRule', (ruleName) => { }); Cypress.Commands.add('deleteAllCustomRules', () => { + const url = `${Cypress.env('opensearch')}/.opensearch-sap-custom-rules-config`; cy.request({ method: 'DELETE', - url: `${Cypress.env('opensearch')}/.opensearch-sap-custom-rules-config`, + url: url, failOnStatusCode: false, body: { query: { match_all: {} } }, + }).as('deleteAllCustomRules'); + cy.get('@deleteAllCustomRules').should((response) => { + expect(response.status).to.be.oneOf([200, 404]); }); }); diff --git a/cypress/support/typings.js b/cypress/support/typings.js index 4345d7e0b..347c47c78 100644 --- a/cypress/support/typings.js +++ b/cypress/support/typings.js @@ -9,7 +9,7 @@ Cypress.Commands.add( prevSubject: true, }, (subject, text) => { - return cy.get(subject).ospType(text).realPress('Enter'); + return cy.get(subject).clear().ospType(text).realPress('Enter'); } ); diff --git a/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx b/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx index 1c9b2530d..171b32711 100644 --- a/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx +++ b/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx @@ -20,14 +20,25 @@ export interface IntervalProps { onDetectorScheduleChange(schedule: PeriodSchedule): void; } +export interface IntervalState { + isIntervalValid: boolean; +} + const unitOptions: EuiSelectOption[] = [ { value: 'MINUTES', text: 'Minutes' }, { value: 'HOURS', text: 'Hours' }, { value: 'DAYS', text: 'Days' }, ]; -export class Interval extends React.Component { +export class Interval extends React.Component { + state = { + isIntervalValid: true, + }; + onTimeIntervalChange = (event: React.ChangeEvent) => { + this.setState({ + isIntervalValid: !!event.target.value, + }); this.props.onDetectorScheduleChange({ period: { ...this.props.detector.schedule.period, @@ -46,9 +57,14 @@ export class Interval extends React.Component { }; render() { + const { isIntervalValid } = this.state; const { period } = this.props.detector.schedule; return ( - }> + } + isInvalid={!isIntervalValid} + error={'Enter schedule interval.'} + > { value={period.interval} onChange={this.onTimeIntervalChange} data-test-subj={'detector-schedule-number-select'} + required={true} + isInvalid={!isIntervalValid} /> diff --git a/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx b/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx index 36ad37b3e..858e68250 100644 --- a/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx +++ b/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx @@ -43,7 +43,8 @@ export default class DefineDetector extends Component= MIN_NUM_DATA_SOURCES; + detector.inputs[0].detector_input.indices.length >= MIN_NUM_DATA_SOURCES && + !!detector.schedule.period.interval; this.props.changeDetector(detector); this.props.updateDataValidState(DetectorCreationStep.DEFINE_DETECTOR, isDataValid); }