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 Solution][Endpoint] Add RBAC to Endpoint Policy List and Policy Details pages #146480

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Nov 28, 2022

Summary

  • The following changes were done in support of RBAC for the policy management from security solution:
    • Pages are only accessible if user has read permissions
    • If user does not have read or write permissions, the link to the Policy list is remove from the Security Solution management page
    • If user ONLY has read, then the Policy Details save button is removed and all form controls (ex. switches, checkboxes, etc) are disabled
    • If user does not have read permissions to the Endpoint list page, then the policy list Endpoints column is displayed as plain text (no link)
  • Fixes a bug with the Cancel button on the Policy Details, which was redirecting the user by default to the Endpoint List - correct behavior is to redirect to the policy list by default

ℹ️ NOTE: Until the Fleet APIs can be updated to support Endpoint RBAC, this PR must be tested with fleet:all and integrations:all kibana privileges must be set

olm-4416-rbac-policy-management

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.7.0 labels Nov 28, 2022
@paul-tavares paul-tavares self-assigned this Nov 28, 2022
@paul-tavares paul-tavares marked this pull request as ready for review November 29, 2022 21:40
@paul-tavares paul-tavares requested a review from a team as a code owner November 29, 2022 21:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@paul-tavares paul-tavares requested review from gergoabraham, dasansol92 and kevinlog and removed request for pzl and parkiino November 29, 2022 21:40
@paul-tavares paul-tavares force-pushed the task/olm-4416-rbac-policy-list-and-details branch from f67c59e to 395616c Compare November 30, 2022 20:50
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.

looks and works great! 👏

@@ -16,10 +16,11 @@ import { getEndpointListPath, getPoliciesPath } from '../../../../common/routing
import { APP_UI_ID } from '../../../../../../common/constants';

/**
* Returns a link component that navigates to the endpoint list page filtered by a specific policy
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! 🔥

@@ -95,106 +111,101 @@ describe('links', () => {
});

describe('Host Isolation Exception', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome! thanks a lot for updating these tests!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

I just finished merging in your PR and how I did not loose any of your changes (I had merge conflicts 🤞 )

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

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

Looks great! Just left a question on a FF usage.
Also, I saw in the description gif that Host Isolation Exceptions tab appears in Policy Details page after saving it, is it expected?

@@ -61,12 +64,23 @@ export const PolicyFormLayout = React.memo(() => {
const [showConfirm, setShowConfirm] = useState<boolean>(false);
const [routeState, setRouteState] = useState<PolicyDetailsRouteState>();
const policyName = policyItem?.name ?? '';
const hostListRouterPath = getEndpointListPath({ name: 'endpointList' });
const isPolicyListEnabled = useIsExperimentalFeatureEnabled('policyListEnabled');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to check this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this feature flag is still there, so we need it to determine where the user will be redirected to when they click the cancel button. If this feature is on then the default should be policy list page. Else, it should be the Endpoint list (that is what it was before).

I'm hoping that we can clean up several FF in this 8.7 release

…policy-list-and-details

# Conflicts:
#	x-pack/plugins/security_solution/public/management/links.test.ts
#	x-pack/plugins/security_solution/public/management/links.ts
@paul-tavares
Copy link
Contributor Author

paul-tavares commented Dec 1, 2022

Thanks @dasansol92 .

Re: I saw in the description gif that Host Isolation Exceptions tab appears in Policy Details page after saving it, is it expected?

I'm not sure. Probably not, but I did not touch that because there is an issue to track updating those tabs. My guess is that the authz.loading is not being checked before displaying the tabs/page. maybe.

@paul-tavares paul-tavares enabled auto-merge (squash) December 1, 2022 15:32
@paul-tavares paul-tavares merged commit d3b4d39 into elastic:main Dec 1, 2022
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #2 / Alerts detection rules table auto-refresh should disable auto refresh when any rule selected and enable it after rules unselected

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 9.6MB 9.7MB +59.6KB

Page load bundle

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

id before after diff
securitySolution 50.8KB 50.9KB +93.0B
Unknown metric groups

async chunk count

id before after diff
securitySolution 35 36 +1

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 109 115 +6
securitySolution 442 448 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 68 74 +6
osquery 110 117 +7
securitySolution 519 525 +6
total +21

History

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

cc @paul-tavares

@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 1, 2022
@paul-tavares paul-tavares deleted the task/olm-4416-rbac-policy-list-and-details branch December 1, 2022 20:00
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 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants