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] Adding endpoint capability checks #64662

Merged

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Apr 28, 2020

Adds capabilities checks to all of our kibana endpoints.
Introduces a canAccessML capability to cover access to the ML app on basic or above licenses.

Part of #64172

@jgowdyelastic jgowdyelastic self-assigned this Apr 28, 2020
@jgowdyelastic jgowdyelastic added :ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes review labels Apr 28, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@jgowdyelastic jgowdyelastic marked this pull request as ready for review April 28, 2020 17:59
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner April 28, 2020 17:59
@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

x-pack/plugins/ml/server/routes/annotations.ts Outdated Show resolved Hide resolved
x-pack/plugins/ml/server/routes/annotations.ts Outdated Show resolved Hide resolved
x-pack/plugins/ml/server/routes/datafeeds.ts Outdated Show resolved Hide resolved
x-pack/plugins/ml/server/routes/datafeeds.ts Outdated Show resolved Hide resolved
x-pack/plugins/ml/server/routes/job_service.ts Outdated Show resolved Hide resolved
@@ -150,7 +156,11 @@ export function systemRoutes(
{
path: '/api/ml/ml_node_count',
validate: false,
options: {
tags: ['access:ml:canGetJobs'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious as to why this isn't canAccessML like the other routes in here?

Copy link
Member Author

Choose a reason for hiding this comment

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

canAccessML means it's also available on basic.
An end user shouldn't need to query the ml node count if they only have a basic license.

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

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.

Latest edits LGTM.

Don't think this is related to these changes, but could we be hiding the ML Jobs List item from the Management page for the 'user' role? It is shown currently, but clicking on it gives the ML access_denied page

image

Also the 'Clone job' link for the ML viewer on the data frame analytics jobs page is still enabled.

@jgowdyelastic jgowdyelastic merged commit d8c15f5 into elastic:master Apr 29, 2020
@jgowdyelastic jgowdyelastic deleted the adding-endpoint-capability-checks branch April 29, 2020 17:31
jgowdyelastic added a commit to jgowdyelastic/kibana that referenced this pull request Apr 29, 2020
* [ML] Adding endpoint capability checks

* adding missing capability checks

* fixing test

* removing commented code

* fixing functional test

* fixing functional tests

* changes based on review

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jgowdyelastic added a commit that referenced this pull request Apr 29, 2020
* [ML] Adding endpoint capability checks

* adding missing capability checks

* fixing test

* removing commented code

* fixing functional test

* fixing functional tests

* changes based on review

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 4, 2020
…bana into pipeline-editor-part-mvp-2

* 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (90 commits)
  remove unused import
  address review feedback
  [Ingest pipelines] Cleanup (elastic#64794)
  [Ingest] Edit datasource UI (elastic#64727)
  [Lens] Bind all time fields to the time picker (elastic#63874)
  [Lens] Use suggestion system in chart switcher for subtypes (elastic#64613)
  Improve alpha messaging (elastic#64692)
  [Ingest] Allow to enable monitoring of elastic agent (elastic#63598)
  [Metrics UI] Fix alerting when a filter query is present (elastic#64575)
  skip flaky suite (elastic#64812) (elastic#64723)
  [Maps] do not display EMS or kibana layer wizards when not configured (elastic#64554)
  [Reporting/Test] Convert functional test code to Typescript (elastic#64601)
  make inserting timestamp with navigate methods optional with default true (elastic#64655)
  [EPM] Update UI to handle package versions and updates (elastic#64689)
  Minimize dependencies required by our telemetry middleware (elastic#64665)
  [Telemetry] oss api tests (elastic#64602)
  [ML] Adding endpoint capability checks (elastic#64662)
  Update jest config for coverage (elastic#64648)
  [SIEM][NP] Fixes bug in ML signals promotion (elastic#64720)
  share single data plugin bundle (elastic#64549)
  ...
spong pushed a commit that referenced this pull request Aug 13, 2020
…ssions (#74582)

## Summary

Addresses #73567.

ML Users (role: `machine_learning_user`) were previously able to invoke the ML Recognizer API, which we use to get not-yet-installed ML Jobs relevant to our index patterns. As of #64662 this is not true, and so we receive errors from components using the underlying hook, `useSiemJobs`.

To solve this I've created two separate hooks to replace `useSiemJobs`:

* `useSecurityJobs`
  * used on ML Popover
  * includes uninstalled ML Jobs
  * checks (and returns) `isMlAdmin` before fetching data
* `useInstalledSecurityJobs`
  * used on ML Jobs Dropdown and Anomalies Table
  * includes only installed ML Jobs
  * checks (and returns) `isMlUser` before fetching data

Note that we while we now receive the knowledge to do so, we do not always inform the user in the case of invalid permissions, and instead have the following behaviors:

#### User has insufficient license
* ML Popover:  shows an upgrade CTA
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled, shows upgrade CTA
* Rule Details: ML Job Id is displayed as text
#### User is ML User
* ML Popover:  not shown
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled
* Rule Details: ML Job Id is displayed as text
#### User is ML Admin
* ML Popover:  shown
* Anomalies Tables: show data __for installed ML Jobs__
  * This is the same as previous logic, but worth calling out that you can't view historical anomalies
* Rule Creation: ML Rule option is enabled, all ML Jobs available
* Rule Details: ML Job Id is displayed as hyperlink, job status badge shown

### Checklist

- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong pushed a commit to spong/kibana that referenced this pull request Aug 13, 2020
…ssions (elastic#74582)

## Summary

Addresses elastic#73567.

ML Users (role: `machine_learning_user`) were previously able to invoke the ML Recognizer API, which we use to get not-yet-installed ML Jobs relevant to our index patterns. As of elastic#64662 this is not true, and so we receive errors from components using the underlying hook, `useSiemJobs`.

To solve this I've created two separate hooks to replace `useSiemJobs`:

* `useSecurityJobs`
  * used on ML Popover
  * includes uninstalled ML Jobs
  * checks (and returns) `isMlAdmin` before fetching data
* `useInstalledSecurityJobs`
  * used on ML Jobs Dropdown and Anomalies Table
  * includes only installed ML Jobs
  * checks (and returns) `isMlUser` before fetching data

Note that we while we now receive the knowledge to do so, we do not always inform the user in the case of invalid permissions, and instead have the following behaviors:

#### User has insufficient license
* ML Popover:  shows an upgrade CTA
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled, shows upgrade CTA
* Rule Details: ML Job Id is displayed as text
#### User is ML User
* ML Popover:  not shown
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled
* Rule Details: ML Job Id is displayed as text
#### User is ML Admin
* ML Popover:  shown
* Anomalies Tables: show data __for installed ML Jobs__
  * This is the same as previous logic, but worth calling out that you can't view historical anomalies
* Rule Creation: ML Rule option is enabled, all ML Jobs available
* Rule Details: ML Job Id is displayed as hyperlink, job status badge shown

### Checklist

- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong added a commit that referenced this pull request Aug 13, 2020
…ssions (#74582) (#74919)

## Summary

Addresses #73567.

ML Users (role: `machine_learning_user`) were previously able to invoke the ML Recognizer API, which we use to get not-yet-installed ML Jobs relevant to our index patterns. As of #64662 this is not true, and so we receive errors from components using the underlying hook, `useSiemJobs`.

To solve this I've created two separate hooks to replace `useSiemJobs`:

* `useSecurityJobs`
  * used on ML Popover
  * includes uninstalled ML Jobs
  * checks (and returns) `isMlAdmin` before fetching data
* `useInstalledSecurityJobs`
  * used on ML Jobs Dropdown and Anomalies Table
  * includes only installed ML Jobs
  * checks (and returns) `isMlUser` before fetching data

Note that we while we now receive the knowledge to do so, we do not always inform the user in the case of invalid permissions, and instead have the following behaviors:

#### User has insufficient license
* ML Popover:  shows an upgrade CTA
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled, shows upgrade CTA
* Rule Details: ML Job Id is displayed as text
#### User is ML User
* ML Popover:  not shown
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled
* Rule Details: ML Job Id is displayed as text
#### User is ML Admin
* ML Popover:  shown
* Anomalies Tables: show data __for installed ML Jobs__
  * This is the same as previous logic, but worth calling out that you can't view historical anomalies
* Rule Creation: ML Rule option is enabled, all ML Jobs available
* Rule Details: ML Job Id is displayed as hyperlink, job status badge shown

### Checklist

- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

Co-authored-by: Ryland Herrick <ryalnd@gmail.com>
spong pushed a commit to spong/kibana that referenced this pull request Aug 18, 2020
…ssions (elastic#74582)

## Summary

Addresses elastic#73567.

ML Users (role: `machine_learning_user`) were previously able to invoke the ML Recognizer API, which we use to get not-yet-installed ML Jobs relevant to our index patterns. As of elastic#64662 this is not true, and so we receive errors from components using the underlying hook, `useSiemJobs`.

To solve this I've created two separate hooks to replace `useSiemJobs`:

* `useSecurityJobs`
  * used on ML Popover
  * includes uninstalled ML Jobs
  * checks (and returns) `isMlAdmin` before fetching data
* `useInstalledSecurityJobs`
  * used on ML Jobs Dropdown and Anomalies Table
  * includes only installed ML Jobs
  * checks (and returns) `isMlUser` before fetching data

Note that we while we now receive the knowledge to do so, we do not always inform the user in the case of invalid permissions, and instead have the following behaviors:

#### User has insufficient license
* ML Popover:  shows an upgrade CTA
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled, shows upgrade CTA
* Rule Details: ML Job Id is displayed as text
#### User is ML User
* ML Popover:  not shown
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled
* Rule Details: ML Job Id is displayed as text
#### User is ML Admin
* ML Popover:  shown
* Anomalies Tables: show data __for installed ML Jobs__
  * This is the same as previous logic, but worth calling out that you can't view historical anomalies
* Rule Creation: ML Rule option is enabled, all ML Jobs available
* Rule Details: ML Job Id is displayed as hyperlink, job status badge shown

### Checklist

- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
spong added a commit that referenced this pull request Aug 18, 2020
…ssions (#74582) (#75287)

## Summary

Addresses #73567.

ML Users (role: `machine_learning_user`) were previously able to invoke the ML Recognizer API, which we use to get not-yet-installed ML Jobs relevant to our index patterns. As of #64662 this is not true, and so we receive errors from components using the underlying hook, `useSiemJobs`.

To solve this I've created two separate hooks to replace `useSiemJobs`:

* `useSecurityJobs`
  * used on ML Popover
  * includes uninstalled ML Jobs
  * checks (and returns) `isMlAdmin` before fetching data
* `useInstalledSecurityJobs`
  * used on ML Jobs Dropdown and Anomalies Table
  * includes only installed ML Jobs
  * checks (and returns) `isMlUser` before fetching data

Note that we while we now receive the knowledge to do so, we do not always inform the user in the case of invalid permissions, and instead have the following behaviors:

#### User has insufficient license
* ML Popover:  shows an upgrade CTA
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled, shows upgrade CTA
* Rule Details: ML Job Id is displayed as text
#### User is ML User
* ML Popover:  not shown
* Anomalies Tables: show no data
* Rule Creation: ML Rule option is disabled
* Rule Details: ML Job Id is displayed as text
#### User is ML Admin
* ML Popover:  shown
* Anomalies Tables: show data __for installed ML Jobs__
  * This is the same as previous logic, but worth calling out that you can't view historical anomalies
* Rule Creation: ML Rule option is enabled, all ML Jobs available
* Rule Details: ML Job Id is displayed as hyperlink, job status badge shown

### Checklist

- [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

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

Co-authored-by: Ryland Herrick <ryalnd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes review v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants