[7.x] [SIEM][Detections] Restrict ML rule modification to ML Admins (#65583)#66102
Merged
rylnd merged 3 commits intoelastic:7.xfrom May 12, 2020
Merged
[7.x] [SIEM][Detections] Restrict ML rule modification to ML Admins (#65583)#66102rylnd merged 3 commits intoelastic:7.xfrom
rylnd merged 3 commits intoelastic:7.xfrom
Conversation
…c#65583) * Move common ML types and functions into siem/common These revolve around capabilities/permissions checks which were previously only used on the client. Now that we have need to make similar checks on the server, we can/should move these up to be shared. * Use ML's Capabilities type in lieu of our own There was already some drift between these types preventing our helpers from being used with the ML services; this will prevent further drift. * Add authorization helpers for ML Some of this responsibility will move to ML services in the near future, but for now we still need to restrict SIEM users from performing certain actions. * Use mlAuthz on our import rule route The tests were a little tricky because the use of spyOn/clear was preventing (rather, clearing the mocks from) the use of jest.mock(). I found a workaround with mockRestore(), which was easy to verify because the mock would throw an error if it wasn't removed, and we'd import multiple rules if a default mock was used. The threading through of ML can go away if/when ML adds their services to the request handler context. * Add mlAuthz checks to remaining rule routes * Remove validateLicenseForRuleType This is now unused and redundant with the mlAuthz module. * Fix failing tests These were missed when the helpers were moved to common/, but are also unneeded. * Cleanup: fixing type errors * Clean up some types from ML A recent upstream refactor in ML added top-level exports; this uses them where possible. * Refactor mlAuthz to defer authz validation until validator is called This prevents us from unnecessarily calling ml services if e.g. we're not dealing with an ML rule. This also adds a failing test for the next-to-be-implemented feature: cashing the async validation for subsequent validator calls. * Cache validation promise The purpose of the `buildMlAuthz` function is to store state (request, license, ml). Since `validateMlAuthz` should be idempotent for the duration of this object's lifecycle, we should cache the result the first time we call it; this is effectively memoization since the arguments do not change. * Make our result caching more explicit Extracts a caching helper function. * Add additional unit tests around some edge cases This is the best form of documentation, thanks Frank! * Remove redundant test setup * Empty messages are invalid If we somehow generate an empty message string, the validation should fail as we were attempting to assign _something_ as a failure message. * Fix validity logic valid: message !== null was the opposite of what I wanted; a validation is valid if it has no message (i.e. it's undefined). * Prevent patching of ML rules by non-ML admins This required refactoring patchRules to accept the rule to be patched, so that we can check its attributes before performing the update. * Fix our update_prepackaged_rules route patchRules no longer does the fetch; we need to perform this ourselves. * Fix update_prepackaged_rules tests This notably synchronizes the entirety of the updates, as our tests were failing due to the asynchronous nature of the updates. * Remove id and ruleId from patchRules parameters Instead of fetching the rule within patchRules, we now pass it in. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
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
3 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.
Backports the following commits to 7.x: