[Alerting] Split alerting feature privilege between rules and alerts and handle subfeature privilege specification #100127
Conversation
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
…horization client on plugin start contract
…ing/refactor-alerts-authorization
…ing/refactor-alerts-authorization
…ing/refactor-alerts-authorization
…ing/refactor-alerts-authorization
…dd-subfeature-privilege
botelastic
bot
added
the
Team:Uptime - DEPRECATED
|
Pinging @elastic/uptime (Team:uptime) |
|
Pinging @elastic/apm-ui (Team:apm) |
4 tasks
justinkambic
approved these changes
May 25, 2021
legrego
reviewed
May 25, 2021
| @@ -46,8 +46,14 @@ describe('featurePrivilegeIterator', () => { | |||
| read: ['read-type'], | |||
| }, | |||
| alerting: { | |||
| all: ['alerting-all-type'], | |||
| read: ['alerting-read-type'], | |||
| rule: { | |||
There was a problem hiding this comment.
I feel your pain having to update this test suite 🙏
| @@ -414,6 +460,111 @@ features.registerKibanaFeature({ | |||
| In the above example, note that instead of denying users with the `read` role any access to the `my-application-id.my-restricted-rule-type` type, we've decided that these users _should_ be granted `read` privileges over the _restricted_ rule type. | |||
| As part of that same change, we also decided that not only should they be allowed to `read` the _restricted_ rule type, but actually, despite having `read` privileges to the feature as a whole, we do actually want to allow them to create our basic 'my-application-id.my-rule-type' rule type, as we consider it an extension of _reading_ data in our feature, rather than _writing_ it. | |||
|
|
|||
| ### Subfeature privileges | |||
| 'test.throw', | ||
| 'test.longRunning', | ||
| ], | ||
| rule: { |
There was a problem hiding this comment.
Would it make sense to add an integration test that grants access to alerting.alert? If I'm following along correctly, all of the test fixtures grant alerting.rule, but they don't exercise any of the alerting.alert paths.
There was a problem hiding this comment.
@legrego This is a little tricky because we are adding this granularity to support privileges on rules and alerts but right now the alerting framework is only CRUD-ing rules. We haven't hooked up any of the stack rules (that the alerting framework is responsible for) to actually write out the alerts that this feature privilege might grant access to. Getting the stack rules to write out alerts using the alert data service is something we do plan to do so maybe the functional tests would make more sense in that context when we have actual data to control access to?
@yctercero Are you adding functional tests in your RBAC PR?
There was a problem hiding this comment.
@ymao1 makes sense, I'm happy to defer this until we have actual data to control access to
yctercero
reviewed
May 25, 2021
yctercero
approved these changes
May 25, 2021
…ing/add-subfeature-privilege
legrego
approved these changes
May 25, 2021
|
@elasticmachine merge upstream |
ymao1
added
the
auto-backport
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsReferences to deprecated APIs
History
To update your PR or re-run it, just comment with: cc @ymao1 |
kibanamachine
added a commit
to kibanamachine/kibana
that referenced
this pull request
May 27, 2021
…and handle subfeature privilege specification (elastic#100127) * WIP - creating alerting authorization client factory and exposing authorization client on plugin start contract * Updating alerting feature privilege builder to handle different alerting types * Passing in alerting authorization type to AlertingActions class string builder * Passing in authorization type in each function call * Passing in exempt consumer ids. Adding authorization type to audit logger * Changing alertType to ruleType * Changing alertType to ruleType * Updating unit tests * Updating unit tests * Passing field names into authorization query builder. Adding kql/es dsl option * Converting to es query if requested * Fixing functional tests * Removing ability to specify feature privilege name in constructor * Fixing some types and tests * Consolidating alerting authorization kuery filter options * Cleanup and tests * Cleanup and tests * Initial commit with changes needed for subfeature privilege * Throwing error when AlertingAuthorizationClientFactory is not defined * Renaming authorizationType to entity * Renaming AlertsAuthorization to AlertingAuthorization * Fixing unit tests * Changing schema of alerting feature privilege * Changing schema of alerting feature privilege * Updating feature privilege iterator * Updating feature privilege builder * Fixing types check * Updating privilege string terminology * Updating privilege string terminology * Wip * Fixing unit tests * Unit tests * Updating README and removing stack subfeature privilege changes * Fixing README Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
10 tasks
kibanamachine
added a commit
that referenced
this pull request
May 27, 2021
…and handle subfeature privilege specification (#100127) (#100817) * WIP - creating alerting authorization client factory and exposing authorization client on plugin start contract * Updating alerting feature privilege builder to handle different alerting types * Passing in alerting authorization type to AlertingActions class string builder * Passing in authorization type in each function call * Passing in exempt consumer ids. Adding authorization type to audit logger * Changing alertType to ruleType * Changing alertType to ruleType * Updating unit tests * Updating unit tests * Passing field names into authorization query builder. Adding kql/es dsl option * Converting to es query if requested * Fixing functional tests * Removing ability to specify feature privilege name in constructor * Fixing some types and tests * Consolidating alerting authorization kuery filter options * Cleanup and tests * Cleanup and tests * Initial commit with changes needed for subfeature privilege * Throwing error when AlertingAuthorizationClientFactory is not defined * Renaming authorizationType to entity * Renaming AlertsAuthorization to AlertingAuthorization * Fixing unit tests * Changing schema of alerting feature privilege * Changing schema of alerting feature privilege * Updating feature privilege iterator * Updating feature privilege builder * Fixing types check * Updating privilege string terminology * Updating privilege string terminology * Wip * Fixing unit tests * Unit tests * Updating README and removing stack subfeature privilege changes * Fixing README Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: ymao1 <ying.mao@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #99940
Summary
ruleandalert.Note that this PR does not change the behavior of any roles. It will be up to individual producers to update their feature privilege definitions to add subfeature definition if they wish.
Checklist
Delete any items that are not applicable to this PR.