Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Fix custom URLs processing for security app #76957

Merged
merged 6 commits into from
Sep 14, 2020

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Sep 8, 2020

Summary

Closes #76789

Fixes token replacement for queries with spaces around the field-value separator.

Checklist

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

// Split query string by AND operator.
.split(/\sand\s/i)
// Get property name from `influencerField:$influencerField$` string.
.map((v) => v.split(':')[0]);
.map((v) => String(v.split(/:(.+)?\$/)[0]).trim());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested a custom URL on the special characters data set and am seeing this error when opening a link to Dashboard:

image

Job config has:

  "analysis_config": {
    "bucket_span": "15m",
    "detectors": [
      {
        "detector_description": "mean(\"metric%$£&!{(]field\") partitionfield=\"at@name\"",
        "function": "mean",
        "field_name": "metric%$£&!{(]field",
        "partition_field_name": "at@name",
        "detector_index": 0
      }
    ],
    "influencers": [
      "at@name",
      "singlequote'name"
    ]
  },

And opening a URL which uses the influencers from this anomaly:
image

I think it is the singlequote'name field name which is causing the problem here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 57fa5a0

@peteharverson
Copy link
Contributor

Found another issue with the special characters data, when the field value is contains a ' quote:

image

Opens dashboard with this in the query bar:
at@name:"contains a !' quote" AND singlequote'name:"contains a !' quote" (with an ! going in before the quote).

custom URL I am testing is

      {
        "url_name": "Show dashboard",
        "time_range": "auto",
        "url_value": """dashboards#/view/351de820-f2bb-11ea-ab06-cb93221707e9?_a=(filters:!(),query:(language:kuery,query:'at@name:"$at@name$" and singlequote!'name:"$singlequote!'name$"'))&_g=(filters:!(),time:(from:'$earliest$',mode:absolute,to:'$latest$'))"""
      },

test('return expected url for Security app', () => {
const urlConfig = {
url_name: 'Hosts Overview by process name',
url_value:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth adding a test for the slightly more complicated 'Host details by process name' custom URL, which uses host.name and process.name:

      {
        "url_name": "Host Details by process name",
        "url_value": "security/hosts/ml-hosts/$host.name$?_g=()&query=(query:'process.name%20:%20%22$process.name$%22',language:kuery)&timerange=(global:(linkTo:!(timeline),timerange:(from:'$earliest$',kind:absolute,to:'$latest$')),timeline:(linkTo:!(global),timerange:(from:'$earliest$',kind:absolute,to:'$latest$')))"
      },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in e8b935e

@darnautov
Copy link
Contributor Author

Found another issue with the special characters data, when the field value is contains a ' quote:

image

Opens dashboard with this in the query bar:
at@name:"contains a !' quote" AND singlequote'name:"contains a !' quote" (with an ! going in before the quote).

custom URL I am testing is

      {
        "url_name": "Show dashboard",
        "time_range": "auto",
        "url_value": """dashboards#/view/351de820-f2bb-11ea-ab06-cb93221707e9?_a=(filters:!(),query:(language:kuery,query:'at@name:"$at@name$" and singlequote!'name:"$singlequote!'name$"'))&_g=(filters:!(),time:(from:'$earliest$',mode:absolute,to:'$latest$'))"""
      },

was happening because of an extra escaping for a rison string, changed in e8b935e, added a unit test in 5b34682

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested latest edits against siem_auditbeat, siem_winlogbeat and special_characters jobs and LGTM.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
ml 8.0MB +3.7KB 8.0MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@qn895
Copy link
Member

qn895 commented Sep 14, 2020

Changes LGTM 💯

@darnautov darnautov merged commit c5358f3 into elastic:master Sep 14, 2020
@darnautov darnautov deleted the ML-76789-fix-secutiry-custom-urls branch September 14, 2020 14:59
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
* master: (26 commits)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  [ML] Functional tests - increase wait time for DFA start (elastic#77307)
  [UiActions][Drilldowns] Fix actions sorting in context menu (elastic#77162)
  [Drilldowns] Wire up new links to new docs (elastic#77154)
  Fix APM issue template
  [Ingest Pipelines] Drop into an empty tree (elastic#76885)
  ...
darnautov added a commit to darnautov/kibana that referenced this pull request Sep 14, 2020
* [ML] fix custom urls processing for security app

* [ML] improve query string parsing

* [ML] remove escaping with !, adjust a unit test for security app

* [ML] unit test

* [ML] unit test
darnautov added a commit to darnautov/kibana that referenced this pull request Sep 14, 2020
* [ML] fix custom urls processing for security app

* [ML] improve query string parsing

* [ML] remove escaping with !, adjust a unit test for security app

* [ML] unit test

* [ML] unit test
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
darnautov added a commit that referenced this pull request Sep 15, 2020
* [ML] fix custom urls processing for security app

* [ML] improve query string parsing

* [ML] remove escaping with !, adjust a unit test for security app

* [ML] unit test

* [ML] unit test
darnautov added a commit that referenced this pull request Sep 15, 2020
* [ML] Fix custom URLs processing for security app (#76957)

* [ML] fix custom urls processing for security app

* [ML] improve query string parsing

* [ML] remove escaping with !, adjust a unit test for security app

* [ML] unit test

* [ML] unit test

* [ML] update check for kibana urls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Tokens in custom URLs to security plugin are not being substituted with anomaly values
5 participants