Skip to content

Commit

Permalink
[8.x] [EDR Workflows] Fix Endpoint list RBAC problems (#199803) (#203534
Browse files Browse the repository at this point in the history
)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[EDR Workflows] Fix Endpoint list RBAC problems
(#199803)](#199803)

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

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

<!--BACKPORT [{"author":{"name":"Gergő
Ábrahám","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-10T10:46:03Z","message":"[EDR
Workflows] Fix Endpoint list RBAC problems (#199803)\n\n##
Summary\r\n\r\nThis PR fixes multiple Endpoint list privilege issues. It
can be\r\nreviewed commit-by-commit so the fixes are mostly separated
(although\r\nsome solutions and tests are reused, hence the reason to
have them in\r\none pr):\r\n- a3311ad
fixes issue when during\r\nonboarding (no hosts, policies are
indiferent) calls are made to `GET\r\napi/fleet/package_policies`
without correct privilege (needs policy\r\nmanagement READ or
fleet:READ+integration:READ), and causes `Forbidden`\r\npage.
([issue](https://github.com/elastic/security-team/issues/10581))\r\n_UI_:
we display the usual 'onboarding without correct privileges' UI\r\nfor
users\r\n<img width=\"1958\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9e1701cc-9c3d-4a80-9c7a-df792d88dab3\">\r\n\r\n\r\n-
63ca011 fixes issue when
during\r\nonboarding (no hosts, no policies) the `Add Elastic Defend`
button was\r\nshown when user had `Fleet:ALL` and `Integrations:READ`
privilege, while\r\nboth should be `ALL` in order to be able to create
an integration
policy\r\n([issue](https://github.com/elastic/security-team/issues/10765))\r\n_UI_:
the 'Add Elastic Defend' button is hidden, so the result is the\r\nsame
as
above\r\n\r\n\r\nhttps://github.com/user-attachments/assets/87fe3a95-131d-484b-8ca0-d06c4caafee1\r\n\r\n\r\n-
ffafa14 fixes issue when after
having\r\nhosts in Endpoint list and we're calling
`POST\r\napi/fleet/package_policies/_bulk_get` without privilege (needs
policy\r\nmanagement READ or fleet:READ+integration:READ), which does
not cause\r\nany visible issue, but is logged to dev
console\r\n([issue](https://github.com/elastic/security-team/issues/10580))\r\n\r\nsome
additions:\r\n- c7021b3 adds an
acceptance test for\r\nall 3 issues above, with failing test
run\r\n[here](https://buildkite.com/elastic/kibana-pull-request/builds/250428#019320cf-c433-4979-a998-d0f8b8f7be16).\r\n-
8e10847 enables policy
list\r\nintegration test, this closes #169133\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"2fa8f47064c9aeac378f9c547dc13482de7cb566","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Defend
Workflows","backport:prev-minor"],"title":"[EDR Workflows] Fix Endpoint
list RBAC
problems","number":199803,"url":"https://github.com/elastic/kibana/pull/199803","mergeCommit":{"message":"[EDR
Workflows] Fix Endpoint list RBAC problems (#199803)\n\n##
Summary\r\n\r\nThis PR fixes multiple Endpoint list privilege issues. It
can be\r\nreviewed commit-by-commit so the fixes are mostly separated
(although\r\nsome solutions and tests are reused, hence the reason to
have them in\r\none pr):\r\n- a3311ad
fixes issue when during\r\nonboarding (no hosts, policies are
indiferent) calls are made to `GET\r\napi/fleet/package_policies`
without correct privilege (needs policy\r\nmanagement READ or
fleet:READ+integration:READ), and causes `Forbidden`\r\npage.
([issue](https://github.com/elastic/security-team/issues/10581))\r\n_UI_:
we display the usual 'onboarding without correct privileges' UI\r\nfor
users\r\n<img width=\"1958\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9e1701cc-9c3d-4a80-9c7a-df792d88dab3\">\r\n\r\n\r\n-
63ca011 fixes issue when
during\r\nonboarding (no hosts, no policies) the `Add Elastic Defend`
button was\r\nshown when user had `Fleet:ALL` and `Integrations:READ`
privilege, while\r\nboth should be `ALL` in order to be able to create
an integration
policy\r\n([issue](https://github.com/elastic/security-team/issues/10765))\r\n_UI_:
the 'Add Elastic Defend' button is hidden, so the result is the\r\nsame
as
above\r\n\r\n\r\nhttps://github.com/user-attachments/assets/87fe3a95-131d-484b-8ca0-d06c4caafee1\r\n\r\n\r\n-
ffafa14 fixes issue when after
having\r\nhosts in Endpoint list and we're calling
`POST\r\napi/fleet/package_policies/_bulk_get` without privilege (needs
policy\r\nmanagement READ or fleet:READ+integration:READ), which does
not cause\r\nany visible issue, but is logged to dev
console\r\n([issue](https://github.com/elastic/security-team/issues/10580))\r\n\r\nsome
additions:\r\n- c7021b3 adds an
acceptance test for\r\nall 3 issues above, with failing test
run\r\n[here](https://buildkite.com/elastic/kibana-pull-request/builds/250428#019320cf-c433-4979-a998-d0f8b8f7be16).\r\n-
8e10847 enables policy
list\r\nintegration test, this closes #169133\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"2fa8f47064c9aeac378f9c547dc13482de7cb566"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199803","number":199803,"mergeCommit":{"message":"[EDR
Workflows] Fix Endpoint list RBAC problems (#199803)\n\n##
Summary\r\n\r\nThis PR fixes multiple Endpoint list privilege issues. It
can be\r\nreviewed commit-by-commit so the fixes are mostly separated
(although\r\nsome solutions and tests are reused, hence the reason to
have them in\r\none pr):\r\n- a3311ad
fixes issue when during\r\nonboarding (no hosts, policies are
indiferent) calls are made to `GET\r\napi/fleet/package_policies`
without correct privilege (needs policy\r\nmanagement READ or
fleet:READ+integration:READ), and causes `Forbidden`\r\npage.
([issue](https://github.com/elastic/security-team/issues/10581))\r\n_UI_:
we display the usual 'onboarding without correct privileges' UI\r\nfor
users\r\n<img width=\"1958\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9e1701cc-9c3d-4a80-9c7a-df792d88dab3\">\r\n\r\n\r\n-
63ca011 fixes issue when
during\r\nonboarding (no hosts, no policies) the `Add Elastic Defend`
button was\r\nshown when user had `Fleet:ALL` and `Integrations:READ`
privilege, while\r\nboth should be `ALL` in order to be able to create
an integration
policy\r\n([issue](https://github.com/elastic/security-team/issues/10765))\r\n_UI_:
the 'Add Elastic Defend' button is hidden, so the result is the\r\nsame
as
above\r\n\r\n\r\nhttps://github.com/user-attachments/assets/87fe3a95-131d-484b-8ca0-d06c4caafee1\r\n\r\n\r\n-
ffafa14 fixes issue when after
having\r\nhosts in Endpoint list and we're calling
`POST\r\napi/fleet/package_policies/_bulk_get` without privilege (needs
policy\r\nmanagement READ or fleet:READ+integration:READ), which does
not cause\r\nany visible issue, but is logged to dev
console\r\n([issue](https://github.com/elastic/security-team/issues/10580))\r\n\r\nsome
additions:\r\n- c7021b3 adds an
acceptance test for\r\nall 3 issues above, with failing test
run\r\n[here](https://buildkite.com/elastic/kibana-pull-request/builds/250428#019320cf-c433-4979-a998-d0f8b8f7be16).\r\n-
8e10847 enables policy
list\r\nintegration test, this closes #169133\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n---------\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"2fa8f47064c9aeac378f9c547dc13482de7cb566"}}]}]
BACKPORT-->

Co-authored-by: Gergő Ábrahám <[email protected]>
  • Loading branch information
kibanamachine and gergoabraham authored Dec 10, 2024
1 parent 6ed3337 commit cd14a9c
Show file tree
Hide file tree
Showing 9 changed files with 568 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
* 2.0.
*/

import { calculateEndpointAuthz, getEndpointAuthzInitialState } from './authz';
import {
calculateEndpointAuthz,
canFetchPackageAndAgentPolicies,
getEndpointAuthzInitialState,
} from './authz';
import type { FleetAuthz } from '@kbn/fleet-plugin/common';
import { createFleetAuthzMock } from '@kbn/fleet-plugin/common/mocks';
import { createLicenseServiceMock } from '../../../license/mocks';
Expand All @@ -15,6 +19,7 @@ import {
RESPONSE_CONSOLE_ACTION_COMMANDS_TO_RBAC_FEATURE_CONTROL,
type ResponseConsoleRbacControls,
} from '../response_actions/constants';
import type { Capabilities } from '@kbn/core-capabilities-common';

describe('Endpoint Authz service', () => {
let licenseService: ReturnType<typeof createLicenseServiceMock>;
Expand Down Expand Up @@ -91,48 +96,53 @@ describe('Endpoint Authz service', () => {
);
});

it('should not give canAccessFleet if `fleet.all` is false', () => {
fleetAuthz.fleet.all = false;
expect(calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canAccessFleet).toBe(
false
);
});
describe('Fleet', () => {
[true, false].forEach((value) => {
it(`should set canAccessFleet to ${value} if \`fleet.all\` is ${value}`, () => {
fleetAuthz.fleet.all = value;
expect(calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canAccessFleet).toBe(
value
);
});

it('should not give canReadFleetAgents if `fleet.readAgents` is false', () => {
fleetAuthz.fleet.readAgents = false;
expect(calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canReadFleetAgents).toBe(
false
);
});
it(`should set canReadFleetAgents to ${value} if \`fleet.readAgents\` is ${value}`, () => {
fleetAuthz.fleet.readAgents = value;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canReadFleetAgents
).toBe(value);
});

it('should not give canWriteFleetAgents if `fleet.allAgents` is false', () => {
fleetAuthz.fleet.allAgents = false;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canWriteFleetAgents
).toBe(false);
});
it(`should set canWriteFleetAgents to ${value} if \`fleet.allAgents\` is ${value}`, () => {
fleetAuthz.fleet.allAgents = value;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canWriteFleetAgents
).toBe(value);
});

it('should not give canReadFleetAgentPolicies if `fleet.readAgentPolicies` is false', () => {
fleetAuthz.fleet.readAgentPolicies = false;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canReadFleetAgentPolicies
).toBe(false);
it(`should set canReadFleetAgentPolicies to ${value} if \`fleet.readAgentPolicies\` is ${value}`, () => {
fleetAuthz.fleet.readAgentPolicies = value;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canReadFleetAgentPolicies
).toBe(value);
});

it(`should set canWriteIntegrationPolicies to ${value} if \`integrations.writeIntegrationPolicies\` is ${value}`, () => {
fleetAuthz.integrations.writeIntegrationPolicies = value;
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles)
.canWriteIntegrationPolicies
).toBe(value);
});
});
});

it('should not give canAccessEndpointManagement if not superuser', () => {
it('should set canAccessEndpointManagement if not superuser', () => {
userRoles = [];
expect(
calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canAccessEndpointManagement
).toBe(false);
});

it('should give canAccessFleet if `fleet.all` is true', () => {
fleetAuthz.fleet.all = true;
expect(calculateEndpointAuthz(licenseService, fleetAuthz, userRoles).canAccessFleet).toBe(
true
);
});

it('should give canAccessEndpointManagement if superuser', () => {
userRoles = ['superuser'];
expect(
Expand Down Expand Up @@ -303,6 +313,7 @@ describe('Endpoint Authz service', () => {
canReadFleetAgentPolicies: false,
canReadFleetAgents: false,
canWriteFleetAgents: false,
canWriteIntegrationPolicies: false,
canAccessEndpointActionsLogManagement: false,
canAccessEndpointManagement: false,
canCreateArtifactsByPolicy: false,
Expand Down Expand Up @@ -336,4 +347,64 @@ describe('Endpoint Authz service', () => {
});
});
});

describe('canFetchPackageAndAgentPolicies()', () => {
describe('without granular Fleet permissions', () => {
it.each`
readFleet | readIntegrations | readPolicyManagement | result
${false} | ${false} | ${false} | ${false}
${true} | ${false} | ${false} | ${false}
${false} | ${true} | ${false} | ${false}
${true} | ${true} | ${false} | ${true}
${false} | ${false} | ${true} | ${true}
${true} | ${false} | ${true} | ${true}
${false} | ${true} | ${true} | ${true}
${true} | ${true} | ${true} | ${true}
`(
'should return $result when readFleet is $readFleet, readIntegrations is $readIntegrations and readPolicyManagement is $readPolicyManagement',
({ readFleet, readIntegrations, readPolicyManagement, result }) => {
const capabilities: Partial<Capabilities> = {
siem: { readPolicyManagement },
fleetv2: { read: readFleet },
fleet: { read: readIntegrations },
};

expect(canFetchPackageAndAgentPolicies(capabilities as Capabilities)).toBe(result);
}
);
});

describe('with granular Fleet permissions', () => {
it.each`
readFleet | readAgentPolicies | readIntegrations | readPolicyManagement | result
${false} | ${false} | ${false} | ${false} | ${false}
${false} | ${false} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${true} | ${true}
${false} | ${false} | ${true} | ${true} | ${true}
${false} | ${true} | ${false} | ${false} | ${false}
${false} | ${true} | ${true} | ${false} | ${false}
${false} | ${true} | ${false} | ${true} | ${true}
${false} | ${true} | ${true} | ${true} | ${true}
${true} | ${false} | ${false} | ${false} | ${false}
${true} | ${false} | ${true} | ${false} | ${false}
${true} | ${false} | ${false} | ${true} | ${true}
${true} | ${false} | ${true} | ${true} | ${true}
${true} | ${true} | ${false} | ${false} | ${false}
${true} | ${true} | ${true} | ${false} | ${true}
${true} | ${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${true} | ${true} | ${true}
`(
'should return $result when readAgentPolicies is $readAgentPolicies, readFleet is $readFleet, readIntegrations is $readIntegrations and readPolicyManagement is $readPolicyManagement',
({ readAgentPolicies, readFleet, readIntegrations, readPolicyManagement, result }) => {
const capabilities: Partial<Capabilities> = {
siem: { readPolicyManagement },
fleetv2: { read: readFleet, agent_policies_read: readAgentPolicies },
fleet: { read: readIntegrations },
};

expect(canFetchPackageAndAgentPolicies(capabilities as Capabilities)).toBe(result);
}
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { ENDPOINT_PRIVILEGES, FleetAuthz } from '@kbn/fleet-plugin/common';

import { omit } from 'lodash';
import type { Capabilities } from '@kbn/core-capabilities-common';
import type { ProductFeaturesService } from '../../../../server/lib/product_features_service';
import { RESPONSE_CONSOLE_ACTION_COMMANDS_TO_REQUIRED_AUTHZ } from '../response_actions/constants';
import type { LicenseService } from '../../../license';
Expand Down Expand Up @@ -99,10 +100,19 @@ export const calculateEndpointAuthz = (
const authz: EndpointAuthz = {
canWriteSecuritySolution,
canReadSecuritySolution,

// ---------------------------------------------------------
// Coming from Fleet authz
// ---------------------------------------------------------
canAccessFleet: fleetAuthz?.fleet.all ?? false,
canReadFleetAgentPolicies: fleetAuthz?.fleet.readAgentPolicies ?? false,
canWriteFleetAgents: fleetAuthz?.fleet.allAgents ?? false,
canReadFleetAgents: fleetAuthz?.fleet.readAgents ?? false,
canWriteIntegrationPolicies: fleetAuthz?.integrations.writeIntegrationPolicies ?? false,

// ---------------------------------------------------------
// Endpoint & policy management
// ---------------------------------------------------------
canAccessEndpointManagement: hasEndpointManagementAccess, // TODO: is this one deprecated? it is the only place we need to check for superuser.
canCreateArtifactsByPolicy: isPlatinumPlusLicense,
canWriteEndpointList,
Expand Down Expand Up @@ -166,6 +176,7 @@ export const getEndpointAuthzInitialState = (): EndpointAuthz => {
canReadFleetAgentPolicies: false,
canReadFleetAgents: false,
canWriteFleetAgents: false,
canWriteIntegrationPolicies: false,
canAccessEndpointActionsLogManagement: false,
canAccessEndpointManagement: false,
canCreateArtifactsByPolicy: false,
Expand Down Expand Up @@ -198,3 +209,24 @@ export const getEndpointAuthzInitialState = (): EndpointAuthz => {
canWriteEndpointExceptions: false,
};
};

/**
* Duplicate logic to calculate if user has privilege to fetch Agent Policies,
* working only with Capabilities, in order to be able to use it e.g. in middleware.
*
* The logic works with Fleet granular privileges (`subfeaturePrivileges`) both enabled and disabled.
*
* @param capabilities Capabilities from coreStart.application
*/
export const canFetchPackageAndAgentPolicies = (capabilities: Capabilities): boolean => {
const canReadPolicyManagement = Boolean(capabilities.siem?.readPolicyManagement);

const fleetv2 = capabilities.fleetv2;
const canReadFleetAgentPolicies = Boolean(
fleetv2?.read && (fleetv2?.agent_policies_read ?? true)
);

const canReadIntegrations = Boolean(capabilities.fleet?.read);

return canReadPolicyManagement || (canReadFleetAgentPolicies && canReadIntegrations);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export interface EndpointAuthz {
canReadFleetAgents: boolean;
/** If the user has permissions to write Fleet Agents */
canWriteFleetAgents: boolean;
/** If the user has permissions to write Integration policies in the Fleet app */
canWriteIntegrationPolicies: boolean;
/** If the user has permissions to access Endpoint management (includes check to ensure they also have access to fleet) */
canAccessEndpointManagement: boolean;
/** If the user has permissions to access Actions Log management and also has a platinum license (used for endpoint details flyout) */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ const PolicyEmptyState = React.memo<{
policyEntryPoint?: boolean;
}>(({ loading, onActionClick, actionDisabled, policyEntryPoint = false }) => {
const docLinks = useKibana().services.docLinks;
const { canAccessFleet, loading: authzLoading } = useUserPrivileges().endpointPrivileges;
const {
canAccessFleet,
canWriteIntegrationPolicies,
loading: authzLoading,
} = useUserPrivileges().endpointPrivileges;

return (
<div data-test-subj="emptyPolicyTable">
Expand Down Expand Up @@ -134,7 +138,7 @@ const PolicyEmptyState = React.memo<{

{authzLoading && <EuiSkeletonText lines={1} />}

{!authzLoading && canAccessFleet && (
{!authzLoading && canAccessFleet && canWriteIntegrationPolicies && (
<>
<EuiSpacer size="s" />
<EuiFlexGroup>
Expand All @@ -156,7 +160,9 @@ const PolicyEmptyState = React.memo<{
</>
)}

{!authzLoading && !canAccessFleet && <MissingFleetAccessInfo />}
{!authzLoading && !(canAccessFleet && canWriteIntegrationPolicies) && (
<MissingFleetAccessInfo />
)}
</EuiFlexItem>

<EuiFlexItem grow={2}>
Expand Down
Loading

0 comments on commit cd14a9c

Please sign in to comment.