-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ResponseOps] Granular Connector RBAC #203503
Merged
+790
−286
Merged
Changes from 47 commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
36a8907
Adding EDR feature privilege
doakalexi fdf7212
Updating the ui
doakalexi 5384fa9
Rename to action instead of connector
doakalexi 96ccb6d
Fix boolean
doakalexi c274951
Removing changes from email connector
doakalexi 4f29da8
Updating canExecute
doakalexi ef4ba21
Merge branch 'main' into poc/connector-rbac
doakalexi 5b79db1
Merge branch 'main' of github.com:elastic/kibana into poc/connector-rbac
doakalexi 30c7260
Fixing a bad change from merge conflicts
doakalexi 3e5f43f
Update EDR connectors to only allow testing for one sub action
doakalexi 394a8af
Adding validation to rule creation
doakalexi a5cefc0
Merge branch 'main' of github.com:elastic/kibana into poc/connector-rbac
doakalexi 613381a
Removing changes from merge conflicts
doakalexi 6403e6b
Adding api key info to the event log
doakalexi a2d2079
Removing some changes that arent needed
doakalexi 372012a
Removing consumer
doakalexi 243cacc
Adding back rbac
doakalexi bf19980
Fixing typo
doakalexi be23b06
Merge branch 'main' of github.com:elastic/kibana into poc/connector-rbac
doakalexi 5a7b3dd
Creating a new sub-feature field in the connector type
doakalexi 7b34ca5
Adding back the api key
doakalexi 68dc60f
Adding tests
doakalexi 3830696
Removing api key code
doakalexi f7c006d
Adding functional tests
doakalexi 886abec
Merge branch 'main' of github.com:elastic/kibana into connector-rbac
doakalexi 3e4853d
Updating the subfeaturetype
doakalexi e40c39a
Fixing lint
doakalexi cf0884a
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine cf1b80c
Removing circular dependency
doakalexi 524649b
Merge branch 'main' of github.com:elastic/kibana into connector-rbac
doakalexi 68d725f
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine 8f11152
Merge branch 'main' of github.com:elastic/kibana into connector-rbac
doakalexi d29771b
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi 2fad33b
Removing from tsconfig
doakalexi cb380ea
Fixing type check failures
doakalexi 36c03ab
Fixing type check again
doakalexi 936cab9
Fixing test failures
doakalexi 0903096
Merge branch 'main' into connector-rbac
doakalexi 2151a50
Fixing update rule tests
doakalexi 988484c
Removing UI changes
doakalexi cde862f
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi 71ba4f9
Merge branch 'main' into connector-rbac
doakalexi 923fc9a
Fixing bulk edit test
doakalexi b60de78
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi 60b0f65
Merge branch 'main' into connector-rbac
doakalexi 11ad095
Fixing ai assistant privileges tests
doakalexi 12a0c82
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi f62d3e8
Removing so from subfeature
doakalexi 83203e5
Merge branch 'main' into connector-rbac
doakalexi 09f56a5
Merge branch 'main' of github.com:elastic/kibana into connector-rbac
doakalexi d12aa20
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi 7d000b3
Merge branch 'main' into connector-rbac
doakalexi a0a14ac
Merge branch 'main' of github.com:elastic/kibana into connector-rbac
doakalexi be33125
Merge branch 'main' into connector-rbac
doakalexi aa216e6
Renaming from PR feedback
doakalexi b1701a9
Merge branch 'main' into connector-rbac
doakalexi c8754b9
Fixing test failure
doakalexi 6855741
Merge branch 'connector-rbac' of github.com:doakalexi/kibana into con…
doakalexi 66103c8
Fixing test failure
doakalexi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: What does this change mean for users who previously had the
Observability AI Assistant
all privilege? Will users with this privilege still be able to do everything they could before this change? Do we have any functional/api integration tests for this functionality?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for asking! I should have left a note about the changes.
I removed this code because this test, Observability AI Assistant Functional tests feature_controls/assistant_security.spec.ts ai assistant privileges no actions privileges loads conversations UI with connector error message, was failing in
x-pack/test/observability_ai_assistant_functional/tests/feature_controls/assistant_security.spec·ts
. The test is checking that the user see's an error in the UI when they don't have actions privileges.In the PR I removed a check in the actions authorization code with bidirectional connectors that would return an
execute-basic
privilege, and this change is what caused the test to fail. We think this happened bc theObservability AI Assistant
was granting actions privileges through the SO types listed above. I removed them, bc based on the test I don't think that is expected behavior. I think everything should function the same, bc you need bothObservability AI Assistant
privileges andActions
privileges. cc @mikecoteThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks for clarifying this. That sounds plausible to me, as I can’t think of any reasonable use case where a feature completely unrelated to the Actions feature would need to grant direct access to the underlying Actions & Connectors SOs without requiring user to have proper Actions privileges. However, when it comes to AI assistants, I can never be 100% certain, and I assume the Obs AI Assistant team will need to validate our understanding 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed this came up before (https://elastic.slack.com/archives/CHSSGF015/p1692709333499069?thread_ts=1692299953.427369&cid=CHSSGF015), I've neglected to fix this, so fine with this change, but is this not a breaking change to users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, I tested locally and it looks like the UI functions the same, it's showing an error that you need the actions and connectors privileges when you don't have it which is the same as the test above was expecting.
I can add a tooltip to the obs ai assistant feature privilege to help indicate that there is a dependency.