Grant access to machine learning features when base privileges are used#115444
Grant access to machine learning features when base privileges are used#115444jportner merged 19 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/kibana-security (Team:Security) |
|
@XavierM could you please give a bit more detail on what user facing changes this will introduce? |
|
@jgowdyelastic sorry this should have been marked as draft for the time being. We opened this to see what the impact of the change would be on CI, so we could start addressing test failures. The intent here is for access to ML to be granted when the base |
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/ml/permissions/no_ml_access·ts.machine learning permissions for user with no ML access (ft_ml_unauthorized) should not allow access to the ML appStandard OutStack TraceKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/ml/permissions/no_ml_access·ts.machine learning permissions for user with no ML access (ft_ml_unauthorized) should not allow access to the ML appStandard OutStack TraceKibana Pipeline / general / Chrome X-Pack UI Functional Tests Basic License.x-pack/test/functional_basic/apps/ml/permissions/no_ml_access·ts.apps machine learning basic license permissions for user with no ML access (ft_ml_unauthorized) should not allow access to the ML appStandard OutStack Traceand 1 more failures, only showing the first 3. Metrics [docs]
History
To update your PR or re-run it, just comment with: |
|
@elasticmachine merge upstream |
jportner
left a comment
There was a problem hiding this comment.
Nice job on this, I think all your changes look great!
I wonder if we should add some more tests though -- it looks like we effectively don't have any ML integration tests that make assertions for users that have base privileges.
we have tests for these authorized users:
ML_POWERUSER(ft_ml_poweruser) - access granted to ML via themachine_learning_adminreserved roleML_POWERUSER_SPACES(ft_ml_poweruser_spaces) - explicit access to ML in the default spaceML_POWERUSER_SPACE1(ft_ml_poweruser_space1) - explicit access to ML in the space1 spaceML_POWERUSER_ALL_SPACES(ft_ml_poweruser_all_spaces) - explicit access to ML in all spacesML_VIEWER(ft_ml_viewer) - access granted via themachine_learning_userreserved roleML_VIEWER_SPACES(ft_ml_viewer_spaces) - explicit access to ML in the default spacML_VIEWER_SPACE1(ft_ml_viewer_space1) - explicit access to ML in the space1 spaceML_VIEWER_ALL_SPACES(ft_ml_viewer_all_spaces) - explicit access to ML in all spaces
I don't think we should make two additional users to test implicit access (base privileges). Instead, I think we should change ML_POWERUSER_ALL_SPACES and ML_VIEWER_ALL_SPACES to use base privileges. E.g., change these two roles to the following:
{
name: 'ft_all_spaces_ml_all',
elasticsearch: { cluster: [], indices: [], run_as: [] },
kibana: [
{
base: ['all'],
feature: {},
spaces: ['*'],
},
],
},
...
{
name: 'ft_all_spaces_ml_read',
elasticsearch: { cluster: [], indices: [], run_as: [] },
kibana: [
{
base: ['read'],
feature: {},
spaces: ['*'],
},
],
},WDYT?
| }); | ||
| }); | ||
|
|
||
| describe('ml read', () => { |
There was a problem hiding this comment.
I like these tests that you added for explicit read / explicit none
|
I like your suggestion about these two roles |
|
cc @pheyos regarding tests and user changes |
|
I'm not sure what is causing the module integration test failures. It seems that the "ft_ml_poweruser" user does not have enough privileges to access the "apache_data_stream" and "nginx_data_stream" modules, but I'm not sure why. I attempted adding extra feature privileges to the "ft_all_space_ml_none" role ( |
|
To summarize the failing test investigation and slack discussion: |
|
There's another test failure (I think this was hidden by a different failure before). We also have a test where we modify an index pattern / data view. I haven't tested it, but I think this means we also need |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
…-migrate-away-from-injected-css-js * 'master' of github.com:elastic/kibana: (61 commits) [ML] Nodes overview for the Model Management page (elastic#116361) [Uptime] Uptime index config using kibana.yml (elastic#115775) [Controls] Dashboard Integration (elastic#115991) skip flaky suite (elastic#104260) Include Files in GitHub UI (elastic#115956) skip flaky suite (elastic#116060) [Canvas] By-Value Embeddables (elastic#113827) Skip failing test (elastic#115366) [Osquery] Fix live query search doesn't return relevant results for agents (elastic#116332) [Integrations] Added link in old Add Data description and fixed alignment in cards (elastic#116213) [Actions] Extended ActionTypeRegistry with connector validation to validate config with secrets (elastic#116079) skip flaky suite (elastic#109329) Grant access to machine learning features when base privileges are used (elastic#115444) Skipping failing test (elastic#84957) [RAC][Security Solution] Adds migration to new SecuritySolution rule types (elastic#112113) skip flaky suite (elastic#115366) [Fleet] Marking API spec as experimental (elastic#116331) [Docs] Cleaning up the versions in the upgrade paths. Closes elastic#116223 (elastic#116228) [Reporting] Suppress debug logs in the mock logger (elastic#116012) [Metrics UI] Clear threshold alert groups state when filterQuery changes (elastic#116205) ... # Conflicts: # src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx # src/plugins/dashboard/public/types.ts
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
1 similar comment
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |

Summary
The Machine Learning feature should be included as part of the base privileges for the 8.0 release.
This is considered a breaking change since it would grant considerable privileges to existing users, and this may not be desirable for certain installations.
Resolves #71422
Checklist
Delete any items that are not applicable to this PR.