🌊 Streams: Permission handling#217353
Merged
flash1293 merged 6 commits intoelastic:mainfrom Apr 8, 2025
Merged
Conversation
flash1293
added
release_note:skip
Contributor
Author
|
/ci |
Contributor
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
Contributor
|
Did you consider using the core |
Contributor
Author
|
@tonyghiani Maybe I'm misunderstanding how the privileges work, but aren't they global for the user and about the Kibana level of things? I'm checking for privileges on the Elasticsearch level which can be different per data stream. I chatted with @yngrdyn about the approach and it's mirroring which we do for the data set quality page which is pretty similar. We will need a Kibana level feature for streams eventually to allow disabling the whole feature on-prem, but we have a different issue for that (and it's not relevant for serverless) |
Contributor
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
|
tonyghiani
approved these changes
Apr 8, 2025
Contributor
tonyghiani
left a comment
tonyghiani
left a comment
There was a problem hiding this comment.
Overall looks good and limits the available features when privileges are lacking. As discussed offline, not sure if there is a more established practice to consume ES privileges client side than attaching them to the API response, but at this point of our needs it looks ok.
Contributor
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/14335116322 |
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
flash1293
added a commit
to flash1293/kibana
that referenced
this pull request
Apr 8, 2025
Currently, the streams UI doesn't deal well with partial permissions.
This PR improves that. As a lot of things come together in streams, we
could do even better, but I think it's OK to draw a line somewhere.
The logic is now as follows:
When reading a stream, the privileges of the current user are returned
along with the stream itself. These are grouped like this:
```
interface IngestStreamPrivileges {
// User can change everything about the stream
manage: boolean;
// User can read stats (like size in bytes) about the stream
monitor: boolean;
// User can change the retention policy of the stream
lifecycle: boolean;
// User can simulate changes to the processing or the mapping of the stream
simulate: boolean;
}
```
This is part of the definition response and is passed around to the
components and disabled buttons and similar in the places where this is
necessary.
The "advanced" tab is only shown when full `manage` permissions are
present - there constellations of permissions that would allow some
access but not all (e.g. having `read_pipelines` but not
`manage_index_templates`), but these should be rather rare and not worth
the additional effort.
## Conditions
In the following places privileges are checked:
* Overview
* Without `monitor`, the overall stats are not shown
* Enrichment
* Without `manage`, you can't save changes
* Without `simulate`, the UI is readonly
* Partitioning
* Without `manage`, you can't save changes
* Without `simulate`, the UI is readonly
* Schema editor
* Without `manage`, the UI is readonly
* Retention
* Without `monitor`, the ingest stats are not shown
* Without `lifecycle`, the retention can't be changed and ILM breakdown
is not rendered
* Advanced
* Without `manage`, the tab is hidden completely
## Drive-by fix
I noticed that we still register the app header action menu which adds
an empty bar on serverless, removed that code.
## Testing
Check
https://github.com/elastic/kibana/pull/217353/files#diff-d8f33d7021058bf90cbeea908bf399da2af50d8b8bfac8a07f160ddc0cdff12bR747
for which Elasticsearch level privileges you need for different
permutations. Then set up a role and a user and log in as that user.
Also test the different pre-defined roles on serverless.
(cherry picked from commit fd37446)
# Conflicts:
# x-pack/platform/plugins/shared/streams/server/routes/streams/crud/read_stream.ts
flash1293
added a commit
that referenced
this pull request
Apr 8, 2025
# Backport This will backport the following commits from `main` to `8.x`: - [🌊 Streams: Permission handling (#217353)](#217353) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Joe Reuter","email":"johannes.reuter@elastic.co"},"sourceCommit":{"committedDate":"2025-04-08T13:42:29Z","message":"🌊 Streams: Permission handling (#217353)\n\nCurrently, the streams UI doesn't deal well with partial permissions.\nThis PR improves that. As a lot of things come together in streams, we\ncould do even better, but I think it's OK to draw a line somewhere.\n\nThe logic is now as follows:\nWhen reading a stream, the privileges of the current user are returned\nalong with the stream itself. These are grouped like this:\n```\ninterface IngestStreamPrivileges {\n // User can change everything about the stream\n manage: boolean;\n // User can read stats (like size in bytes) about the stream\n monitor: boolean;\n // User can change the retention policy of the stream\n lifecycle: boolean;\n // User can simulate changes to the processing or the mapping of the stream\n simulate: boolean;\n}\n```\n\nThis is part of the definition response and is passed around to the\ncomponents and disabled buttons and similar in the places where this is\nnecessary.\n\nThe \"advanced\" tab is only shown when full `manage` permissions are\npresent - there constellations of permissions that would allow some\naccess but not all (e.g. having `read_pipelines` but not\n`manage_index_templates`), but these should be rather rare and not worth\nthe additional effort.\n\n## Conditions\n\nIn the following places privileges are checked:\n* Overview\n * Without `monitor`, the overall stats are not shown\n* Enrichment\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Partitioning\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Schema editor\n * Without `manage`, the UI is readonly\n* Retention\n * Without `monitor`, the ingest stats are not shown\n* Without `lifecycle`, the retention can't be changed and ILM breakdown\nis not rendered\n* Advanced\n * Without `manage`, the tab is hidden completely\n\n## Drive-by fix\n\nI noticed that we still register the app header action menu which adds\nan empty bar on serverless, removed that code.\n\n## Testing\n\nCheck\nhttps://github.com//pull/217353/files#diff-d8f33d7021058bf90cbeea908bf399da2af50d8b8bfac8a07f160ddc0cdff12bR747\nfor which Elasticsearch level privileges you need for different\npermutations. Then set up a role and a user and log in as that user.\n\nAlso test the different pre-defined roles on serverless.","sha":"fd374463f74caac17b07120c34d2fc6c8e5e2754","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:obs-ux-logs","backport:version","Feature:Streams","v9.1.0","v8.19.0"],"title":"🌊 Streams: Permission handling","number":217353,"url":"https://github.com/elastic/kibana/pull/217353","mergeCommit":{"message":"🌊 Streams: Permission handling (#217353)\n\nCurrently, the streams UI doesn't deal well with partial permissions.\nThis PR improves that. As a lot of things come together in streams, we\ncould do even better, but I think it's OK to draw a line somewhere.\n\nThe logic is now as follows:\nWhen reading a stream, the privileges of the current user are returned\nalong with the stream itself. These are grouped like this:\n```\ninterface IngestStreamPrivileges {\n // User can change everything about the stream\n manage: boolean;\n // User can read stats (like size in bytes) about the stream\n monitor: boolean;\n // User can change the retention policy of the stream\n lifecycle: boolean;\n // User can simulate changes to the processing or the mapping of the stream\n simulate: boolean;\n}\n```\n\nThis is part of the definition response and is passed around to the\ncomponents and disabled buttons and similar in the places where this is\nnecessary.\n\nThe \"advanced\" tab is only shown when full `manage` permissions are\npresent - there constellations of permissions that would allow some\naccess but not all (e.g. having `read_pipelines` but not\n`manage_index_templates`), but these should be rather rare and not worth\nthe additional effort.\n\n## Conditions\n\nIn the following places privileges are checked:\n* Overview\n * Without `monitor`, the overall stats are not shown\n* Enrichment\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Partitioning\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Schema editor\n * Without `manage`, the UI is readonly\n* Retention\n * Without `monitor`, the ingest stats are not shown\n* Without `lifecycle`, the retention can't be changed and ILM breakdown\nis not rendered\n* Advanced\n * Without `manage`, the tab is hidden completely\n\n## Drive-by fix\n\nI noticed that we still register the app header action menu which adds\nan empty bar on serverless, removed that code.\n\n## Testing\n\nCheck\nhttps://github.com//pull/217353/files#diff-d8f33d7021058bf90cbeea908bf399da2af50d8b8bfac8a07f160ddc0cdff12bR747\nfor which Elasticsearch level privileges you need for different\npermutations. Then set up a role and a user and log in as that user.\n\nAlso test the different pre-defined roles on serverless.","sha":"fd374463f74caac17b07120c34d2fc6c8e5e2754"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/217353","number":217353,"mergeCommit":{"message":"🌊 Streams: Permission handling (#217353)\n\nCurrently, the streams UI doesn't deal well with partial permissions.\nThis PR improves that. As a lot of things come together in streams, we\ncould do even better, but I think it's OK to draw a line somewhere.\n\nThe logic is now as follows:\nWhen reading a stream, the privileges of the current user are returned\nalong with the stream itself. These are grouped like this:\n```\ninterface IngestStreamPrivileges {\n // User can change everything about the stream\n manage: boolean;\n // User can read stats (like size in bytes) about the stream\n monitor: boolean;\n // User can change the retention policy of the stream\n lifecycle: boolean;\n // User can simulate changes to the processing or the mapping of the stream\n simulate: boolean;\n}\n```\n\nThis is part of the definition response and is passed around to the\ncomponents and disabled buttons and similar in the places where this is\nnecessary.\n\nThe \"advanced\" tab is only shown when full `manage` permissions are\npresent - there constellations of permissions that would allow some\naccess but not all (e.g. having `read_pipelines` but not\n`manage_index_templates`), but these should be rather rare and not worth\nthe additional effort.\n\n## Conditions\n\nIn the following places privileges are checked:\n* Overview\n * Without `monitor`, the overall stats are not shown\n* Enrichment\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Partitioning\n * Without `manage`, you can't save changes\n * Without `simulate`, the UI is readonly\n* Schema editor\n * Without `manage`, the UI is readonly\n* Retention\n * Without `monitor`, the ingest stats are not shown\n* Without `lifecycle`, the retention can't be changed and ILM breakdown\nis not rendered\n* Advanced\n * Without `manage`, the tab is hidden completely\n\n## Drive-by fix\n\nI noticed that we still register the app header action menu which adds\nan empty bar on serverless, removed that code.\n\n## Testing\n\nCheck\nhttps://github.com//pull/217353/files#diff-d8f33d7021058bf90cbeea908bf399da2af50d8b8bfac8a07f160ddc0cdff12bR747\nfor which Elasticsearch level privileges you need for different\npermutations. Then set up a role and a user and log in as that user.\n\nAlso test the different pre-defined roles on serverless.","sha":"fd374463f74caac17b07120c34d2fc6c8e5e2754"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, the streams UI doesn't deal well with partial permissions. This PR improves that. As a lot of things come together in streams, we could do even better, but I think it's OK to draw a line somewhere.
The logic is now as follows:
When reading a stream, the privileges of the current user are returned along with the stream itself. These are grouped like this:
This is part of the definition response and is passed around to the components and disabled buttons and similar in the places where this is necessary.
The "advanced" tab is only shown when full
managepermissions are present - there constellations of permissions that would allow some access but not all (e.g. havingread_pipelinesbut notmanage_index_templates), but these should be rather rare and not worth the additional effort.Conditions
In the following places privileges are checked:
monitor, the overall stats are not shownmanage, you can't save changessimulate, the UI is readonlymanage, you can't save changessimulate, the UI is readonlymanage, the UI is readonlymonitor, the ingest stats are not shownlifecycle, the retention can't be changed and ILM breakdown is not renderedmanage, the tab is hidden completelyDrive-by fix
I noticed that we still register the app header action menu which adds an empty bar on serverless, removed that code.
Testing
Check https://github.com/elastic/kibana/pull/217353/files#diff-d8f33d7021058bf90cbeea908bf399da2af50d8b8bfac8a07f160ddc0cdff12bR747 for which Elasticsearch level privileges you need for different permutations. Then set up a role and a user and log in as that user.
Also test the different pre-defined roles on serverless.