Skip to content
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

[EDR Workflows] The host isolation exception tab is hidden on the basic license if no artifacts #192562

Merged
merged 14 commits into from
Sep 23, 2024

Conversation

szwarckonrad
Copy link
Contributor

@szwarckonrad szwarckonrad commented Sep 11, 2024

This PR updates how the Host Isolation Exceptions tab is displayed based on the user’s permissions and license. The tab is always visible to platinum+ users. For lower-tier licenses, a check is performed: if a user has previously defined host isolation exceptions, they will see the tab and be able to view or remove existing exceptions. If they haven’t, the tab will be hidden, and the functionality will be inaccessible.

Previously, even if a user didn’t have access to host isolation exceptions, they could still see and enter the Host Isolation Exceptions tab.

To test locally:
ESS:

  1. Start ES + Kibana the regular way, with the default trial license.
  2. Add HIE
  3. Downgrade license (https://github.com/elastic/pzl-es-tools)
  4. Verify that the license had been downgraded

Serverless:

  1. Start Serverless ES yarn es serverless --clean --teardown --kill -E xpack.security.authc.api_key.enabled=true -E http.host=0.0.0.0 --projectType security
  2. Start Serverless Kibana yarn serverless-security
  3. Add HIE
  4. Modify config/serverless.security.yml to security and endpoint essential
  5. Wait for Kibana to reload

ESS:

Screen.Recording.2024-09-11.at.15.27.26.mov

Serverless:

Screen.Recording.2024-09-11.at.15.47.46.mov

@szwarckonrad szwarckonrad added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0 labels Sep 11, 2024
@szwarckonrad szwarckonrad self-assigned this Sep 11, 2024
@szwarckonrad szwarckonrad marked this pull request as ready for review September 11, 2024 15:18
@szwarckonrad szwarckonrad requested a review from a team as a code owner September 11, 2024 15:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

Copy link
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work, @szwarckonrad, thanks for all the cleanup as well! 👏 tested with both ESS and serverless, works well

i have only some nitpicks - see below -, other than that, i have a question regarding the feature (cc @dasansol92 @caitlinbetz)

with Basic licence, if there's a HIE after downgrade

  • we show the HIE page amongst the management page, that's clear bc there the user sees all HIE entries
  • but, we also show the HIE tab on the Policy Details page regardless if anything is displayed there. so the question, do we want to show the HIE tab for every integration, including the ones that does not have a HIE assigned (and there's no global HIE)?

with this modification, on Basic license the user sees this if there are some HIE left but none of those are global or assigned to the opened integration:
image

Copy link
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last nitpicks are below, but it's looking real good, thanks for the changes! 🚀

(also quickly re-tested only with ESS, still works well 👍 )

@@ -68,7 +68,7 @@ describe('useHostIsolationExceptionsAccess', () => {

await waitFor(() => {
expect(checkArtifactHasData).not.toHaveBeenCalled();
expect(result.current).toBe(true);
expect(result.current.hasAccessToHostIsolationExceptions).toBe(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think a test case for isHostIsolationExceptionsAccessLoading: true would be useful, both here and in policy_details.test.tsx

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work fixing this. 🚀 Tested it out and works as expected. I left some suggestions for you to consider.

@@ -10,7 +10,7 @@ import type {
ExceptionListItemSchema,
UpdateExceptionListItemSchema,
} from '@kbn/securitysolution-io-ts-list-types';
import { ENDPOINT_BLOCKLISTS_LIST_ID } from '@kbn/securitysolution-list-constants';
import { ENDPOINT_ARTIFACT_LISTS } from '@kbn/securitysolution-list-constants';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up the deprecations 🥇

const redirectHostIsolationException =
isInHostIsolationExceptionsTab &&
(!canReadHostIsolationExceptions ||
(!isHostIsolationExceptionsAccessLoading && !hasAccessToHostIsolationExceptions));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!isHostIsolationExceptionsAccessLoading check is redundant here.

return;
}

const redirectHostIsolationException =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const redirectHostIsolationException =
const shouldNotSeeHostIsolationExceptions =

@@ -68,7 +68,7 @@ describe('useHostIsolationExceptionsAccess', () => {

await waitFor(() => {
expect(checkArtifactHasData).not.toHaveBeenCalled();
expect(result.current).toBe(true);
expect(result.current.hasAccessToHostIsolationExceptions).toBe(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

@szwarckonrad szwarckonrad added backport:version Backport to applied version labels v9.0.0 and removed backport:skip This commit does not require backporting labels Sep 20, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #42 / Journey[many_fields_discover] Expand the first document

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5814 5815 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 20.4MB 20.4MB +1.9KB
Unknown metric groups

References to deprecated APIs

id before after diff
securitySolution 470 458 -12

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @szwarckonrad

@szwarckonrad szwarckonrad merged commit 636baad into elastic:main Sep 23, 2024
43 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 23, 2024
…ic license if no artifacts (elastic#192562)

This PR updates how the Host Isolation Exceptions tab is displayed based
on the user’s permissions and license. The tab is always visible to
platinum+ users. For lower-tier licenses, a check is performed: if a
user has previously defined host isolation exceptions, they will see the
tab and be able to view or remove existing exceptions. If they haven’t,
the tab will be hidden, and the functionality will be inaccessible.

Previously, even if a user didn’t have access to host isolation
exceptions, they could still see and enter the Host Isolation Exceptions
tab.

To test locally:
ESS:
1. Start ES + Kibana the regular way, with the default `trial` license.
2. Add HIE
3. Downgrade license (https://github.com/elastic/pzl-es-tools)
4. Verify that the license had been downgraded

Serverless:
1. Start Serverless ES `yarn es serverless --clean --teardown --kill -E
xpack.security.authc.api_key.enabled=true -E http.host=0.0.0.0
--projectType security`
2. Start Serverless Kibana `yarn serverless-security`
3. Add HIE
4. Modify `config/serverless.security.yml` to security and endpoint
essential
5. Wait for Kibana to reload

ESS:

https://github.com/user-attachments/assets/75527af7-9d06-4da7-9e86-6ce6b22ac147

Serverless:

https://github.com/user-attachments/assets/e89bd642-9e99-4a22-8b42-5997f7333ea6

---------

Co-authored-by: Ash <[email protected]>
(cherry picked from commit 636baad)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 23, 2024
…he basic license if no artifacts (#192562) (#193711)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[EDR Workflows] The host isolation exception tab is hidden on the
basic license if no artifacts
(#192562)](#192562)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Konrad
Szwarc","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-23T12:00:15Z","message":"[EDR
Workflows] The host isolation exception tab is hidden on the basic
license if no artifacts (#192562)\n\nThis PR updates how the Host
Isolation Exceptions tab is displayed based\r\non the user’s permissions
and license. The tab is always visible to\r\nplatinum+ users. For
lower-tier licenses, a check is performed: if a\r\nuser has previously
defined host isolation exceptions, they will see the\r\ntab and be able
to view or remove existing exceptions. If they haven’t,\r\nthe tab will
be hidden, and the functionality will be
inaccessible.\r\n\r\nPreviously, even if a user didn’t have access to
host isolation\r\nexceptions, they could still see and enter the Host
Isolation Exceptions\r\ntab.\r\n\r\nTo test locally:\r\nESS:\r\n1. Start
ES + Kibana the regular way, with the default `trial` license. \r\n2.
Add HIE\r\n3. Downgrade license
(https://github.com/elastic/pzl-es-tools) \r\n4. Verify that the license
had been downgraded\r\n\r\nServerless:\r\n1. Start Serverless ES `yarn
es serverless --clean --teardown --kill
-E\r\nxpack.security.authc.api_key.enabled=true -E
http.host=0.0.0.0\r\n--projectType security`\r\n2. Start Serverless
Kibana `yarn serverless-security` \r\n3. Add HIE\r\n4. Modify
`config/serverless.security.yml` to security and
endpoint\r\nessential\r\n5. Wait for Kibana to
reload\r\n\r\nESS:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/75527af7-9d06-4da7-9e86-6ce6b22ac147\r\n\r\nServerless:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e89bd642-9e99-4a22-8b42-5997f7333ea6\r\n\r\n---------\r\n\r\nCo-authored-by:
Ash
<[email protected]>","sha":"636baadfa278bf831ad21457be4abde4b9c837c9","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Defend
Workflows","v8.16.0","backport:version"],"title":"[EDR Workflows] The
host isolation exception tab is hidden on the basic license if no
artifacts","number":192562,"url":"https://github.com/elastic/kibana/pull/192562","mergeCommit":{"message":"[EDR
Workflows] The host isolation exception tab is hidden on the basic
license if no artifacts (#192562)\n\nThis PR updates how the Host
Isolation Exceptions tab is displayed based\r\non the user’s permissions
and license. The tab is always visible to\r\nplatinum+ users. For
lower-tier licenses, a check is performed: if a\r\nuser has previously
defined host isolation exceptions, they will see the\r\ntab and be able
to view or remove existing exceptions. If they haven’t,\r\nthe tab will
be hidden, and the functionality will be
inaccessible.\r\n\r\nPreviously, even if a user didn’t have access to
host isolation\r\nexceptions, they could still see and enter the Host
Isolation Exceptions\r\ntab.\r\n\r\nTo test locally:\r\nESS:\r\n1. Start
ES + Kibana the regular way, with the default `trial` license. \r\n2.
Add HIE\r\n3. Downgrade license
(https://github.com/elastic/pzl-es-tools) \r\n4. Verify that the license
had been downgraded\r\n\r\nServerless:\r\n1. Start Serverless ES `yarn
es serverless --clean --teardown --kill
-E\r\nxpack.security.authc.api_key.enabled=true -E
http.host=0.0.0.0\r\n--projectType security`\r\n2. Start Serverless
Kibana `yarn serverless-security` \r\n3. Add HIE\r\n4. Modify
`config/serverless.security.yml` to security and
endpoint\r\nessential\r\n5. Wait for Kibana to
reload\r\n\r\nESS:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/75527af7-9d06-4da7-9e86-6ce6b22ac147\r\n\r\nServerless:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e89bd642-9e99-4a22-8b42-5997f7333ea6\r\n\r\n---------\r\n\r\nCo-authored-by:
Ash
<[email protected]>","sha":"636baadfa278bf831ad21457be4abde4b9c837c9"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192562","number":192562,"mergeCommit":{"message":"[EDR
Workflows] The host isolation exception tab is hidden on the basic
license if no artifacts (#192562)\n\nThis PR updates how the Host
Isolation Exceptions tab is displayed based\r\non the user’s permissions
and license. The tab is always visible to\r\nplatinum+ users. For
lower-tier licenses, a check is performed: if a\r\nuser has previously
defined host isolation exceptions, they will see the\r\ntab and be able
to view or remove existing exceptions. If they haven’t,\r\nthe tab will
be hidden, and the functionality will be
inaccessible.\r\n\r\nPreviously, even if a user didn’t have access to
host isolation\r\nexceptions, they could still see and enter the Host
Isolation Exceptions\r\ntab.\r\n\r\nTo test locally:\r\nESS:\r\n1. Start
ES + Kibana the regular way, with the default `trial` license. \r\n2.
Add HIE\r\n3. Downgrade license
(https://github.com/elastic/pzl-es-tools) \r\n4. Verify that the license
had been downgraded\r\n\r\nServerless:\r\n1. Start Serverless ES `yarn
es serverless --clean --teardown --kill
-E\r\nxpack.security.authc.api_key.enabled=true -E
http.host=0.0.0.0\r\n--projectType security`\r\n2. Start Serverless
Kibana `yarn serverless-security` \r\n3. Add HIE\r\n4. Modify
`config/serverless.security.yml` to security and
endpoint\r\nessential\r\n5. Wait for Kibana to
reload\r\n\r\nESS:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/75527af7-9d06-4da7-9e86-6ce6b22ac147\r\n\r\nServerless:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e89bd642-9e99-4a22-8b42-5997f7333ea6\r\n\r\n---------\r\n\r\nCo-authored-by:
Ash
<[email protected]>","sha":"636baadfa278bf831ad21457be4abde4b9c837c9"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Konrad Szwarc <[email protected]>
weizijun added a commit to weizijun/kibana that referenced this pull request Sep 23, 2024
* main: (176 commits)
  [ML][Rules] Fixes deletion in Check interval input for anomaly detection rule (elastic#193420)
  Bump maximum supported package spec version to 3.2 (elastic#193574)
  [ES|QL] new pattern for `SORT` autocomplete (elastic#193595)
  [Inventory][ECO] Entities page search bar (elastic#193546)
  [Synthetics] Remove extra overview route (elastic#192449)
  [Obs Alerts table] Fix error on clicking alert reason message (elastic#193693)
  [Migrations] Remove tests that are not applicable in 9.x (elastic#193699)
  [EDR Workflows] Set Agent Tamper Protection to false on policy unassignment (elastic#193017)
  [Inventory][ECO] Enable elastic entity model from inventory (elastic#193557)
  [EDR Workflows] The host isolation exception tab is hidden on the basic license if no artifacts (elastic#192562)
  [Entity Analytics] Ensuring definition transforms are managed (elastic#193408)
  [Automatic Import] Do not remove message field for unstructured logs (elastic#193678)
  [Fleet] Add missing permissions for connector package (elastic#193573)
  [Fleet] using @kbn/config-schema part 2 (outputs and other apis)  (elastic#193326)
  [Migrations] Provide testing archives + tooling for migrations integration tests (elastic#193328)
  [ES|QL] Renames the textbased editor to esql editor (elastic#193521)
  [ES|QL] Update function metadata (elastic#193662)
  [Security Solution][Entity Analytics] Scoping the entity store to spaces (elastic#193303)
  [Docs] Update Sharing docs (elastic#190318)
  [ML] AIOps: Move Log Rate Analysis results callout to help popover. (elastic#192243)
  ...

# Conflicts:
#	x-pack/plugins/search_inference_endpoints/public/components/all_inference_endpoints/render_table_columns/render_endpoint/endpoint_info.test.tsx
#	x-pack/plugins/search_inference_endpoints/public/components/all_inference_endpoints/render_table_columns/render_endpoint/endpoint_info.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EDR Workflows] The host isolation exception tab is available inconsistently on the basic license
6 participants