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

[Security Solutions] Add PLI authorisation for Advanced Insights (Entity Risk) #161190

Merged
merged 9 commits into from
Jul 26, 2023

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented Jul 4, 2023

Summary

Add PLI authorization checks for Entity Analytics features.
This PR only restricts access to the features but doesn't implement PLG/Upselling. It will be added later when we have defined the UX for it.

The advancedInsights PLI was already configured, so I only had to add extra checks to make sure users can't see the Risk score on other components.
Updated components:

  • "All hosts" table on the Hosts page
  • "All users" table on the Users page
  • Host overview on the Host details page and Host details flyout
  • User overview on the User details page and User details flyout
  • Alerts flyout
  • Remove sample Upselling components config

Not included

  • Upselling/PLG
  • I left empty tabs/pages where the Upselling component will be added

How to test it?

ESS

  • Run ESS with a basic license
  • Run ESS with a platinum

Serverless

  • Run Serverless with security essentials (serverless.security.yml)
xpack.serverless.security.productTypes:
  [
    { product_line: 'security', product_tier: 'essentials' }
  ]
  • Run Serverless with security complete (kibana/config/serverless.security.yml)
xpack.serverless.security.productTypes:
  [
    { product_line: 'security', product_tier: 'complete' },
  ]
 
risk.score.PLI.mp4

Checklist

Delete any items that are not applicable to this PR.

@machadoum machadoum self-assigned this Jul 5, 2023
@machadoum machadoum added Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore labels Jul 5, 2023
@machadoum machadoum marked this pull request as ready for review July 6, 2023 07:50
@machadoum machadoum requested review from a team as code owners July 6, 2023 07:50
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@machadoum machadoum added Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes labels Jul 6, 2023
@machadoum machadoum marked this pull request as draft July 6, 2023 07:53
@machadoum machadoum added v8.10.0 ci:cloud-deploy Create or update a Cloud deployment labels Jul 7, 2023
@machadoum
Copy link
Member Author

@elasticmachine merge upstream

@machadoum machadoum marked this pull request as ready for review July 24, 2023 13:42
Copy link
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

Looks good to me. But Left a comment regarding the risk score tabs.

if (RiskScoreUpsell) {
return <RiskScoreUpsell />;
if (!isAuthorized) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it makes sense to hide the complete Tab itself till we have the upsell design ? Empty Tab feels like an error.

I noticed that in ESS when we have basic license, we remove the tab completely. Shouldn't we follow the same pattern in serverless as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left the tab there so it is easier for those who add the Upsell component. And it also feels like a waste to add extra code for hiding the tab that will be deleted very soon. But I understand your point. It would be more consistent while we don't have the Upsell component. As a compromise, I added a TODO message.
Screenshot 2023-07-26 at 10 22 04

@machadoum machadoum enabled auto-merge (squash) July 26, 2023 08:42
@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 26, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #3 / Detection rules, Prebuilt Rules Installation and Update Notifications Notifications Rule installation available and rule update available notifications should notify user about prebuilt rules available for installation and for upgrade should notify user about prebuilt rules available for installation and for upgrade

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolutionServerless 233 230 -3

Async chunks

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

id before after diff
securitySolution 15.6MB 15.6MB +2.0KB
securitySolutionServerless 104.9KB 101.9KB -3.0KB
total -1.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolutionServerless 23.6KB 23.1KB -496.0B
Unknown metric groups

async chunk count

id before after diff
securitySolutionServerless 21 19 -2

History

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

cc @machadoum

@machadoum machadoum merged commit a074c06 into elastic:main Jul 26, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 26, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
…ity Risk) (elastic#161190)

## Summary

Add PLI authorization checks for Entity Analytics features.
*This PR only restricts access to the features* but doesn't implement
PLG/Upselling. It will be added later when we have defined the UX for
it.

The `advancedInsights` PLI was already configured, so I only had to add
extra checks to make sure users can't see the Risk score on other
components.
Updated components:
* "All hosts" table on the Hosts page
* "All users" table on the Users page
* Host overview on the Host details page and Host details flyout
* User overview on the User details page and User details flyout
* Alerts flyout
* Remove sample Upselling components config

### Not included
* Upselling/PLG
* I left empty tabs/pages where the Upselling component will be added

### How to test it?
#### ESS
* Run ESS with a basic license
* Run ESS with a platinum

#### Serverless
* Run Serverless with security essentials (serverless.security.yml)
```
xpack.serverless.security.productTypes:
  [
    { product_line: 'security', product_tier: 'essentials' }
  ]
```
* Run Serverless with security complete
(kibana/config/serverless.security.yml)
```
xpack.serverless.security.productTypes:
  [
    { product_line: 'security', product_tier: 'complete' },
  ]
 
 ```


https://github.com/elastic/kibana/assets/1490444/1ab84134-bee1-497c-9b41-a9ec398bd921

### Checklist

Delete any items that are not applicable to this PR.

- [x] [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: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants