Skip to content

Commit

Permalink
[EDR Workflows] Fix Endpoint list RBAC problems (elastic#199803)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes multiple Endpoint list privilege issues. It can be
reviewed commit-by-commit so the fixes are mostly separated (although
some solutions and tests are reused, hence the reason to have them in
one pr):
- a3311ad fixes issue when during
onboarding (no hosts, policies are indiferent) calls are made to `GET
api/fleet/package_policies` without correct privilege (needs policy
management READ or fleet:READ+integration:READ), and causes `Forbidden`
page. ([issue](elastic/security-team#10581))
_UI_: we display the usual 'onboarding without correct privileges' UI
for users
<img width="1958" alt="image"
src="https://github.com/user-attachments/assets/9e1701cc-9c3d-4a80-9c7a-df792d88dab3">


- 63ca011 fixes issue when during
onboarding (no hosts, no policies) the `Add Elastic Defend` button was
shown when user had `Fleet:ALL` and `Integrations:READ` privilege, while
both should be `ALL` in order to be able to create an integration policy
([issue](elastic/security-team#10765))
_UI_: the 'Add Elastic Defend' button is hidden, so the result is the
same as above


https://github.com/user-attachments/assets/87fe3a95-131d-484b-8ca0-d06c4caafee1


- ffafa14 fixes issue when after having
hosts in Endpoint list and we're calling `POST
api/fleet/package_policies/_bulk_get` without privilege (needs policy
management READ or fleet:READ+integration:READ), which does not cause
any visible issue, but is logged to dev console
([issue](elastic/security-team#10580))

some additions:
- c7021b3 adds an acceptance test for
all 3 issues above, with failing test run
[here](https://buildkite.com/elastic/kibana-pull-request/builds/250428#019320cf-c433-4979-a998-d0f8b8f7be16).
- 8e10847 enables policy list
integration test, this closes elastic#169133


### 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
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
gergoabraham and elasticmachine authored Dec 10, 2024
1 parent a0f5a7f commit 2fa8f47
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 2fa8f47

Please sign in to comment.