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

[Logs ML] Check permissions before granting access to Logs ML pages #195278

Conversation

tonyghiani
Copy link
Contributor

@tonyghiani tonyghiani commented Oct 7, 2024

📓 Summary

Closes #191206

This work fixes issues while accessing the Logs Anomalies and Logs Categories pages due to a lack of user privileges.

The privileges were correctly handled until #168234 was merged, which introduced a call to retrieve ml formats information higher in the React hierarchy before the privileges could be asserted for the logged user.
This was resulting in the call failing and letting the user stack in loading states or erroneous error pages.

These changes lift the license + ML read privileges checks higher in the hierarchy so we can display the right prompts before calling the ml formats API, which will resolve correctly if the user has the right privileges.

User without valid license

Screenshot 2024-10-07 at 17 01 17

User with a valid license (or Trial), but no ML privileges

Screenshot 2024-10-07 at 17 03 48

User with a valid license (or Trial) and only Read ML privileges

Screenshot 2024-10-07 at 17 04 21

User with a valid license (or Trial) and All ML privileges, which are the requirements to work with ML Logs features

Screenshot 2024-10-07 at 17 04 52

@reakaleek
Copy link
Member

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@tonyghiani tonyghiani added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Logs UI Logs UI feature Team:obs-ux-logs Observability Logs User Experience Team labels Oct 7, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #20 / Controls Dashboard control group apply button range slider selections making selection enables apply button
  • [job] [logs] Jest Tests #6 / Create renders correctly with optional fields
  • [job] [logs] x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group6.ts / discover/esql discover esql view switch modal should not show switch modal when switching to a data view while a saved search is open
  • [job] [logs] Jest Integration Tests #5 / Http1BasePathProxyServer shouldRedirect it will do a redirect if it detects what looks like a stale or previously used base path

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.6MB 1.6MB +233.0B

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

@tonyghiani tonyghiani marked this pull request as ready for review October 8, 2024 08:54
@tonyghiani tonyghiani requested a review from a team as a code owner October 8, 2024 08:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@tonyghiani tonyghiani added the release_note:skip Skip the PR/issue when compiling release notes label Oct 8, 2024
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Oct 8, 2024
@tonyghiani
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

There are no new commits on the base branch.

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 9, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: bcb49f0
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-195278-bcb49f091c91

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.6MB 1.6MB +217.0B

History

Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

LGTM 🙌

@tonyghiani tonyghiani merged commit e0e4ec1 into elastic:main Oct 10, 2024
23 checks passed
@tonyghiani tonyghiani deleted the 191206-logs-anomalies--and-categories-access branch October 10, 2024 12:33
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11274096553

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 10, 2024
…lastic#195278)

## 📓 Summary

Closes elastic#191206

This work fixes issues while accessing the Logs Anomalies and Logs
Categories pages due to a lack of user privileges.

The privileges were correctly handled until
elastic#168234 was merged, which
introduced a call to retrieve ml formats information higher in the React
hierarchy before the privileges could be asserted for the logged user.
This was resulting in the call failing and letting the user stack in
loading states or erroneous error pages.

These changes lift the license + ML read privileges checks higher in the
hierarchy so we can display the right prompts before calling the ml
formats API, which will resolve correctly if the user has the right
privileges.

### User without valid license

<img width="3008" alt="Screenshot 2024-10-07 at 17 01 17"
src="https://github.com/user-attachments/assets/bf6478ce-b007-4f15-9538-c7959c497e8a">

### User with a valid license (or Trial), but no ML privileges

<img width="3003" alt="Screenshot 2024-10-07 at 17 03 48"
src="https://github.com/user-attachments/assets/c5a82159-b4e8-4f22-9531-23d5e5a9377f">

### User with a valid license (or Trial) and only Read ML privileges

<img width="3003" alt="Screenshot 2024-10-07 at 17 04 21"
src="https://github.com/user-attachments/assets/990f4695-e07e-46a2-9214-d0de3628caf7">

### User with a valid license (or Trial) and All ML privileges, which
are the requirements to work with ML Logs features

<img width="3000" alt="Screenshot 2024-10-07 at 17 04 52"
src="https://github.com/user-attachments/assets/c9b4d832-d3c8-4337-9e17-8a220e7be084">

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
(cherry picked from commit e0e4ec1)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 10, 2024
…ages (#195278) (#195759)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Logs ML] Check permissions before granting access to Logs ML pages
(#195278)](#195278)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco Antonio
Ghiani","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-10T12:33:18Z","message":"[Logs
ML] Check permissions before granting access to Logs ML pages
(#195278)\n\n## 📓 Summary\r\n\r\nCloses #191206 \r\n\r\nThis work fixes
issues while accessing the Logs Anomalies and Logs\r\nCategories pages
due to a lack of user privileges.\r\n\r\nThe privileges were correctly
handled until\r\nhttps://github.com//pull/168234 was
merged, which\r\nintroduced a call to retrieve ml formats information
higher in the React\r\nhierarchy before the privileges could be asserted
for the logged user.\r\nThis was resulting in the call failing and
letting the user stack in\r\nloading states or erroneous error
pages.\r\n\r\nThese changes lift the license + ML read privileges checks
higher in the\r\nhierarchy so we can display the right prompts before
calling the ml\r\nformats API, which will resolve correctly if the user
has the right\r\nprivileges.\r\n\r\n### User without valid
license\r\n\r\n<img width=\"3008\" alt=\"Screenshot 2024-10-07 at 17 01
17\"\r\nsrc=\"https://github.com/user-attachments/assets/bf6478ce-b007-4f15-9538-c7959c497e8a\">\r\n\r\n###
User with a valid license (or Trial), but no ML privileges\r\n\r\n<img
width=\"3003\" alt=\"Screenshot 2024-10-07 at 17 03
48\"\r\nsrc=\"https://github.com/user-attachments/assets/c5a82159-b4e8-4f22-9531-23d5e5a9377f\">\r\n\r\n###
User with a valid license (or Trial) and only Read ML
privileges\r\n\r\n<img width=\"3003\" alt=\"Screenshot 2024-10-07 at 17
04
21\"\r\nsrc=\"https://github.com/user-attachments/assets/990f4695-e07e-46a2-9214-d0de3628caf7\">\r\n\r\n###
User with a valid license (or Trial) and All ML privileges, which\r\nare
the requirements to work with ML Logs features\r\n\r\n<img
width=\"3000\" alt=\"Screenshot 2024-10-07 at 17 04
52\"\r\nsrc=\"https://github.com/user-attachments/assets/c9b4d832-d3c8-4337-9e17-8a220e7be084\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"e0e4ec10e3c329f933bed0a01dbeaecdf79cfa99","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Logs
UI","release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-logs"],"title":"[Logs
ML] Check permissions before granting access to Logs ML
pages","number":195278,"url":"https://github.com/elastic/kibana/pull/195278","mergeCommit":{"message":"[Logs
ML] Check permissions before granting access to Logs ML pages
(#195278)\n\n## 📓 Summary\r\n\r\nCloses #191206 \r\n\r\nThis work fixes
issues while accessing the Logs Anomalies and Logs\r\nCategories pages
due to a lack of user privileges.\r\n\r\nThe privileges were correctly
handled until\r\nhttps://github.com//pull/168234 was
merged, which\r\nintroduced a call to retrieve ml formats information
higher in the React\r\nhierarchy before the privileges could be asserted
for the logged user.\r\nThis was resulting in the call failing and
letting the user stack in\r\nloading states or erroneous error
pages.\r\n\r\nThese changes lift the license + ML read privileges checks
higher in the\r\nhierarchy so we can display the right prompts before
calling the ml\r\nformats API, which will resolve correctly if the user
has the right\r\nprivileges.\r\n\r\n### User without valid
license\r\n\r\n<img width=\"3008\" alt=\"Screenshot 2024-10-07 at 17 01
17\"\r\nsrc=\"https://github.com/user-attachments/assets/bf6478ce-b007-4f15-9538-c7959c497e8a\">\r\n\r\n###
User with a valid license (or Trial), but no ML privileges\r\n\r\n<img
width=\"3003\" alt=\"Screenshot 2024-10-07 at 17 03
48\"\r\nsrc=\"https://github.com/user-attachments/assets/c5a82159-b4e8-4f22-9531-23d5e5a9377f\">\r\n\r\n###
User with a valid license (or Trial) and only Read ML
privileges\r\n\r\n<img width=\"3003\" alt=\"Screenshot 2024-10-07 at 17
04
21\"\r\nsrc=\"https://github.com/user-attachments/assets/990f4695-e07e-46a2-9214-d0de3628caf7\">\r\n\r\n###
User with a valid license (or Trial) and All ML privileges, which\r\nare
the requirements to work with ML Logs features\r\n\r\n<img
width=\"3000\" alt=\"Screenshot 2024-10-07 at 17 04
52\"\r\nsrc=\"https://github.com/user-attachments/assets/c9b4d832-d3c8-4337-9e17-8a220e7be084\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"e0e4ec10e3c329f933bed0a01dbeaecdf79cfa99"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195278","number":195278,"mergeCommit":{"message":"[Logs
ML] Check permissions before granting access to Logs ML pages
(#195278)\n\n## 📓 Summary\r\n\r\nCloses #191206 \r\n\r\nThis work fixes
issues while accessing the Logs Anomalies and Logs\r\nCategories pages
due to a lack of user privileges.\r\n\r\nThe privileges were correctly
handled until\r\nhttps://github.com//pull/168234 was
merged, which\r\nintroduced a call to retrieve ml formats information
higher in the React\r\nhierarchy before the privileges could be asserted
for the logged user.\r\nThis was resulting in the call failing and
letting the user stack in\r\nloading states or erroneous error
pages.\r\n\r\nThese changes lift the license + ML read privileges checks
higher in the\r\nhierarchy so we can display the right prompts before
calling the ml\r\nformats API, which will resolve correctly if the user
has the right\r\nprivileges.\r\n\r\n### User without valid
license\r\n\r\n<img width=\"3008\" alt=\"Screenshot 2024-10-07 at 17 01
17\"\r\nsrc=\"https://github.com/user-attachments/assets/bf6478ce-b007-4f15-9538-c7959c497e8a\">\r\n\r\n###
User with a valid license (or Trial), but no ML privileges\r\n\r\n<img
width=\"3003\" alt=\"Screenshot 2024-10-07 at 17 03
48\"\r\nsrc=\"https://github.com/user-attachments/assets/c5a82159-b4e8-4f22-9531-23d5e5a9377f\">\r\n\r\n###
User with a valid license (or Trial) and only Read ML
privileges\r\n\r\n<img width=\"3003\" alt=\"Screenshot 2024-10-07 at 17
04
21\"\r\nsrc=\"https://github.com/user-attachments/assets/990f4695-e07e-46a2-9214-d0de3628caf7\">\r\n\r\n###
User with a valid license (or Trial) and All ML privileges, which\r\nare
the requirements to work with ML Logs features\r\n\r\n<img
width=\"3000\" alt=\"Screenshot 2024-10-07 at 17 04
52\"\r\nsrc=\"https://github.com/user-attachments/assets/c9b4d832-d3c8-4337-9e17-8a220e7be084\">\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"e0e4ec10e3c329f933bed0a01dbeaecdf79cfa99"}}]}]
BACKPORT-->

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project Feature:Logs UI Logs UI feature release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Logs] Error when accessing Anomalies and Categories without ML privileges
6 participants