Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Security Solution] Endpoint RBAC integration with AppFeatures archit…
…ecture (#158646) # Summary This PR adapts the endpoint RBAC to the new Serverless PLI features architecture. The changes are the following: ## App Features ### New appFeatures keys for endpoint The `endpointExceptions` PLI has been added to the _Endpoint Essentials_ product tier and `endpointResponseActions` to the _Endpoint Complete_ https://github.com/elastic/kibana/blob/686bc2eeaaa87841383af5aa1959c2758e633b20/x-pack/plugins/serverless_security/common/pli/pli_config.ts#L20-L23 ### Endpoint appFeatures capabilities config The features configuration for each appFeature (PLI) has been added. They will be configured within the Security Kibana features only when the appFeature is enabled by the selected Security product type. (Note that all of them will be always added in regular ESS deployments, only in Serverless we'll have different product types) https://github.com/elastic/kibana/blob/4d9f0c3a6f0c926a733b0fc113dfc20fe1223e07/x-pack/plugins/security_solution/server/lib/app_features/security_kibana_features.ts#L170-L198 These are the capabilities that seemed relevant to me for each PLI, but I don't have enough expertise in Endpoint operations to know for sure what Kibana sub-features and capabilities need to be included in each appFeature. The PLIs are in a private spreadsheet with the following descriptions. - endpointExceptions:  - endpointResponseActions:  I'll need Endpoint team members to confirm there's no missing or wrong capability in each appFeature config. ### Host isolation capabilities It is important to mention that in the configuration above, to have some capabilities available we are adding some sub-features directly using the `subFeatureIds` entry, but for host_isolation capabilities, we are doing it in a slightly different way, using the `subFeaturesPrivileges`, this way the privileges are added to existing subFeatures. The reason is we need to have the _write_ (isolate operation) only in payment product types, but the _read_ and _delete_ (release operation) capabilities should be always available, to allow releasing previously isolated hosts after a product downgrade. To do this we always include the `host_isolation_all` and `host_isolation_exceptions_all` subFeatures in the base configuration, but they only contain _read_ and _delete_ capabilities by default, only when the product tier allows the proper appFeatures the _write_ capability is added to the same subFeatures privileges. ## Endpoint Authz module ### Remove "superuser" specific check This specific check: ``` // user is superuser, always return true if (isSuperuser) { return true; } ``` Has been removed, this has no behavioral impact, superuser has all capabilities enabled anyway. ### Remove usage of `endpointRbacEnabled` and `endpointRbacV1Enabled` experimental flags They are already enabled by default. superuser will still have the authorization to access all the features. The only change is the endpoint sub-features will always be visible in the Kibana Privilege section of the Role management page, they were hidden when these experimental flags were disabled.  ### Remove double _write_ check for _read_ authorizations: We were doing unnecessary checks for the _write_ capabilities in the _read_ authorizations, like: ``` const canReadEndpointList = canWriteEndpointList || hasKibanaPrivilege(fleetAuthz, 'readEndpointList'); ```. Sub-features already add _read_ and _write_ capabilities on the `all` privilege, so these double checks were unnecessary. ### Extract `hasHostIsolationExceptionsItems` flag This flag was used to grant _read_ and _delete_ authorization for Host Isolation Exceptions (HIE) when there is data, basically turning them free features when there is data to perform the actions. This is needed to allow users to remove HIE after a license downgrade scenario, which is good. However, we needed to do this API call from outside the auth module, in every place we needed to call `calculateEndpointAuthz`, and we were also adding the responsibility to do some auth-specific logic with licenses outside the auth module, which is not good. In addition, it is not very consistent to make authorization depend on the existence of data to perform an action. Authorization should be based only on the role capabilities and tiers/licenses, if some parts of the application want to show/hide stuff depending on the data, that's not the auth module's responsibility. I checked all the places where we use the HIE _read_ and _delete_ authorizations, and the only place where we really need them to be denied (when there is no data) is in the _links_, we need to remove the HIE link from the app in this situation. So, this PR moves the data check to the links.ts module, making the _read_ and _delete_ permissions always granted without a license (they will still be useless without data), the same way the `canUnIsolateHost` authorization works. And then doing the async data check to remove the HIE link in the _management/links.ts_ module itself, only in the last case where we really need to know it: https://github.com/elastic/kibana/blob/4d9f0c3a6f0c926a733b0fc113dfc20fe1223e07/x-pack/plugins/security_solution/public/management/links.ts#L257-L262 This flag extraction is unrelated to the integration of the new architecture, I included it only to extract complexity from the _authz_ module and simplify its usage, but this change can be rolled back if we consider it. # Testing - To start the application in ESS (non-serverless) mode, run it normally with `yarn start`. Everything should keep working as usual with all features available and capabilities should only be restricted by the user role. - To start the application in Serverless mode run with `yarn serverless-security`. It sets a random root path, so access the main URL at "http://localhost:5601/" to be redirected. By default the "Endpoint Complete" product line is selected in the _serverless.security.yml_ config, so everything should be available as in ESS with the default config. https://github.com/elastic/kibana/blob/686bc2eeaaa87841383af5aa1959c2758e633b20/config/serverless.security.yml#L11-L15 Once in Serverless mode, in order to see the difference between product types, we can change the _Endpoint_ `product_tier` to `essentials`, as per the pli_config, this change should remove all the capabilities included by the `endpointResponseActions` appFeatures config. To check how the application behaves without the `endpointExceptions` PLI, we can remove the _Endpoint_ `product_line` entirely from the product array, leaving the _Security_ `product_line` alone. # Next steps ## Upselling page The product upselling page has not been registered for endpoint pages in this PR, so when any of these pages are unauthorized because of the serverless product tier, and they are accessed directly by URL they still show the `Privileges required` screen.  This is arguably not entirely correct. However, an upselling page can be registered to display a "Buy a higher tier" message when the privilege is denied because of the product type, if it is unauthorized because of the user role the "Privileges required" page will still show. I did not include the endpoint upselling page in this PR to keep it simple, but the registry is already implemented in the main proposal, we can define and register them in a follow-up PR. ## Superuser role in authz module Almost all "superuser" role conditionals have been removed from the Endpoint authz module, but there is only one check left here: https://github.com/elastic/kibana/blob/24330f2356537771c55b07efc3144e56ce199332/x-pack/plugins/security_solution/common/endpoint/service/authz/authz.ts#L85 This `canAccessEndpointManagement` flag looks deprecated, and it seems to be used incorrectly in the few places where it is checked. If we could fix the places that it is used, checking the proper authz flag, we could definitively remove the `userRoles` parameter from the `calculateEndpointAuthz` function, this will have an impact in the different places where this function is called since they will no longer need any async logic. --------- Co-authored-by: Pablo Neves Machado <pablo.nevesmachado@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
- Loading branch information
3 people
authored
Jun 9, 2023
1 parent
ab18045
commit 352d7c9