-
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
[Serverless][Security Solution][Endpoint] Gate endpoint exceptions on rule details and API changes #165613
[Serverless][Security Solution][Endpoint] Gate endpoint exceptions on rule details and API changes #165613
Conversation
68c2808
to
126ce01
Compare
@elasticmachin a merge upstream |
898cfa4
to
088c76a
Compare
62b6bd7
to
fa1e8eb
Compare
f6c7f36
to
c03a8ed
Compare
37b70bf
to
bc87d99
Compare
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.
Hey @ashokaditya, thank you for implementing these PLI checks. From the Rule Management side, we don't really own the exceptions functionality, so I was primarily looking into the diff and overall code of the Rule Details page. I have a few suggestions for relatively simple refactoring that could improve code ownership for endpoint exceptions.
I will do some basic testing based on the PR description, thank you for providing these instructions.
...e_details_ui/pages/rule_details/hooks/use_get_endpoint_exceptions_unavailablle_component.tsx
Outdated
Show resolved
Hide resolved
...e_details_ui/pages/rule_details/hooks/use_get_endpoint_exceptions_unavailablle_component.tsx
Outdated
Show resolved
Hide resolved
...ugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/index.tsx
Outdated
Show resolved
Hide resolved
...olution/public/detection_engine/rule_details_ui/pages/rule_details/use_rule_details_tabs.tsx
Outdated
Show resolved
Hide resolved
I did some testing of the changes on the Rule Details page and it LGTM 👍 @ashokaditya I wasn't able to run Serverless Kibana using the provided instructions in the description. That's probably my lack of knowledge of how to run Kibana in Serverless. I had to comment out everything in The Endpoint exceptions tab on the Rule Details page worked fine with the following config: xpack.securitySolutionServerless.productTypes:
[
{ product_line: 'security', product_tier: 'complete' },
# { product_line: 'endpoint', product_tier: 'complete' },
] I was able to open this "tab" using its direct URL, but it showed that I should upgrade to Endpoint Essentials. The tab itself was hidden 👍 Nit: the UI is slightly different for the endpoint list page |
review suggestions @banderror
Thanks for the review @banderror. I've managed to refactor as you suggested. Here are some thoughts on testing locally.
You tested it the correct way @banderror. For now, I'm testing this locally by
Note: To be able to do the above switch one also needs to comment out this block I know this is tedious for manual testing, but it works reliably for now. Feel free to try it out if you like.
Superb 👍🏼.
Excellent. This is expected.
Quite right! We will update this later along with the text info. These changes only affect the serverless UX anyway. |
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.
Thank you for addressing my comments @ashokaditya, LGTM 👍
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ashokaditya |
…k to check access to`.lists-*` (elastic#171412) ## Summary Instead of using `useListsConfig` this PR uses `useListPrivileges` to verify access to `.lists-*` index pattern. follow up of elastic/pull/165613 related elastic/pull/170671 (closed in favour of this) fixes elastic/issues/169268 ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 8c3322e)
…k to check access to`.lists-*` (elastic#171412) ## Summary Instead of using `useListsConfig` this PR uses `useListPrivileges` to verify access to `.lists-*` index pattern. follow up of elastic/pull/165613 related elastic/pull/170671 (closed in favour of this) fixes elastic/issues/169268 ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <[email protected]>
…to check access to`.lists-*` for endpoint exceptions access (elastic#171412) (elastic#171794) # Backport This will backport the following commits from `main` to `8.11`: - [[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)](elastic#171412) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ash","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-11-22T19:10:50Z","message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","OLM Sprint","v8.11.0","v8.12.0","v8.11.1"],"number":171412,"url":"https://github.com/elastic/kibana/pull/171412","mergeCommit":{"message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/171412","number":171412,"mergeCommit":{"message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8"}}]}] BACKPORT--> Co-authored-by: Ash <[email protected]>
What this PR changes
Follow up of /pull/164107/
For serverless ES/Kibana, it gates exception list API for endpoint exceptions and restricts endpoint exceptions tab on Endpoint Security rule details based on project PLIs. If no endpoint PLIs, endpoint exceptions should not be accessible.
app/security/exceptions/details/endpoint_list
pageHow to review
Best to follow along commits for a code review. Below are details to manually test the changes.
yarn es serverless --kill --clean --license trial -E xpack.security.authc.api_key.enabled=true
on a terminal window to start ES.config/serverless.security.yml
toconfig/serverless.security.dev.yml
yarn serverless-security --no-base-path
on another terminal window to start kibana in serverless modeserverless_security
user.Tests (Serverless)
This needs to be tested with a custom user/role and not
elastic_serverless
which hassuperuser
role.PLI configs
{ product_line: 'security', product_tier: 'essentials' }
or{ product_line: 'security', product_tier: 'complete' }
and
{ product_line: 'endpoint', product_tier: 'essentials' }
or{ product_line: 'endpoint', product_tier: 'complete' }
UX
http://localhost:5601/app/security/rules/
. Click onAdd Elastic rules
.Endpoint Security
rule.Endpoint Security
and navigate to the rules details page, and you should seeEndpoint exceptions
tab. The tabs visible areAlerts
,Endpoint exceptions
,Rule exceptions
,Execution results
.app/security/exceptions/details/endpoint_list
and you should be able to see the page with any added endpoint exceptions.API requests (with user
serverless_security
)200
onPOST api/exception_lists/items
200
onPOST api/exception_lists/_export?id=endpoint_list&list_id=endpoint_list&namespace_type=agnostic&include_expired_exceptions=true
200
onPUT api/exception_lists/items
200
onDELETE api/exception_lists/items
200
onGET api/exception_lists/items/_find?list_id=endpoint_list&namespace_type=agnostic
PLI configs
{ product_line: 'security', product_tier: 'essentials' }
or{ product_line: 'security', product_tier: 'complete' }
UX
http://localhost:5601/app/security/rules/
. Click onAdd Elastic rules
.Endpoint Security
rule.Endpoint Security
and navigate to the rules details page, and you should not seeEndpoint exceptions
tab. The only tabs visible areAlerts
,Rule exceptions
,Execution results
.app/security/exceptions/details/endpoint_list
and you should see an upsell message.API requests
403
onPOST api/exception_lists/items
403
onPOST api/exception_lists/_export?id=endpoint_list&list_id=endpoint_list&namespace_type=agnostic&include_expired_exceptions=true
403
onPUT api/exception_lists/items
403
onDELETE api/exception_lists/items
403
onGET api/exception_lists/items/_find?list_id=endpoint_list&namespace_type=agnostic
Flaky FTRs
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3248
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3255
Checklist
Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers