Skip to content

Commit

Permalink
do not fetch agent policies without privileges - fixes "Forbidden" issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@gergoabraham
gergoabraham committed Nov 22, 2024
1 parent c7021b3 commit a3311ad
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 34 deletions.
67 changes: 66 additions & 1 deletion x-pack/plugins/security_solution/common/endpoint/service/authz/authz.test.ts
View file
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,
canFetchAgentPolicies,
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 @@ -336,4 +341,64 @@ describe('Endpoint Authz service', () => {
});
});
});

describe('canFetchAgentPolicies()', () => {
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(canFetchAgentPolicies(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(canFetchAgentPolicies(capabilities as Capabilities)).toBe(result);
}
);
});
});
});
22 changes: 22 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/service/authz/authz.ts
View file
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 @@ -198,3 +199,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 canFetchAgentPolicies = (capabilities: Capabilities): boolean => {
const canReadPolicyManagement = (capabilities.siem?.readPolicyManagement ?? false) as boolean;

const fleetv2 = capabilities.fleetv2;
const canReadFleetAgentPolicies = (fleetv2?.read &&
(fleetv2?.agent_policies_read === true ||
fleetv2?.agent_policies_read === undefined)) as boolean;

const canReadIntegrations = capabilities.fleet?.read as boolean;

return canReadPolicyManagement || (canReadFleetAgentPolicies && canReadIntegrations);
};
101 changes: 84 additions & 17 deletions ...plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import {
} from '../../../../common/lib/endpoint/endpoint_isolation/mocks';
import { endpointPageHttpMock, failedTransformStateMock } from '../mocks';
import { HOST_METADATA_LIST_ROUTE } from '../../../../../common/endpoint/constants';
import { INGEST_API_PACKAGE_POLICIES } from '../../../services/policies/ingest';
import { canFetchAgentPolicies } from '../../../../../common/endpoint/service/authz/authz';

jest.mock('../../../services/policies/ingest', () => ({
sendGetAgentConfigList: () => Promise.resolve({ items: [] }),
Expand All @@ -57,6 +59,12 @@ jest.mock('rxjs', () => ({
firstValueFrom: () => mockFirstValueFrom(),
}));

jest.mock('../../../../../common/endpoint/service/authz/authz', () => ({
...jest.requireActual('../../../../../common/endpoint/service/authz/authz'),
canFetchAgentPolicies: jest.fn(),
}));
const canFetchAgentPoliciesMock = canFetchAgentPolicies as jest.Mock;

type EndpointListStore = Store<Immutable<EndpointState>, Immutable<AppAction>>;

describe('endpoint list middleware', () => {
Expand All @@ -71,8 +79,10 @@ describe('endpoint list middleware', () => {
let actionSpyMiddleware;
let history: History<never>;

const getEndpointListApiResponse = (): MetadataListResponse => {
return mockEndpointResultList({ pageSize: 1, page: 0, total: 10 });
const getEndpointListApiResponse = (
options: Partial<Parameters<typeof mockEndpointResultList>[0]> = {}
): MetadataListResponse => {
return mockEndpointResultList({ pageSize: 1, page: 0, total: 10, ...options });
};

const dispatchUserChangedUrlToEndpointList = (locationOverrides: Partial<Location> = {}) => {
Expand All @@ -99,25 +109,82 @@ describe('endpoint list middleware', () => {
dispatch = store.dispatch;
history = createBrowserHistory();
getKibanaServicesMock.mockReturnValue(fakeCoreStart);
canFetchAgentPoliciesMock.mockReturnValue(false);
});

it('handles `userChangedUrl`', async () => {
endpointPageHttpMock(fakeHttpServices);
const apiResponse = getEndpointListApiResponse();
fakeHttpServices.get.mockResolvedValue(apiResponse);
expect(fakeHttpServices.get).not.toHaveBeenCalled();
describe('handles `userChangedUrl`', () => {
it('should not fetch agent policies if there are hosts', async () => {
endpointPageHttpMock(fakeHttpServices);
const apiResponse = getEndpointListApiResponse();
fakeHttpServices.get.mockResolvedValue(apiResponse);

dispatchUserChangedUrlToEndpointList();

await Promise.all([
waitForAction('serverReturnedEndpointList'),
waitForAction('serverReturnedEndpointExistValue', {
validate: ({ payload }) => payload === true,
}),
waitForAction('serverCancelledPolicyItemsLoading'),
]);
expect(fakeHttpServices.get).toHaveBeenNthCalledWith(1, HOST_METADATA_LIST_ROUTE, {
query: {
page: '0',
pageSize: '10',
kuery: '',
},
version: '2023-10-31',
});
expect(listData(getState())).toEqual(apiResponse.data);
expect(fakeHttpServices.get).not.toHaveBeenCalledWith(
INGEST_API_PACKAGE_POLICIES,
expect.objectContaining({})
);
});

dispatchUserChangedUrlToEndpointList();
await waitForAction('serverReturnedEndpointList');
expect(fakeHttpServices.get).toHaveBeenNthCalledWith(1, HOST_METADATA_LIST_ROUTE, {
query: {
page: '0',
pageSize: '10',
kuery: '',
},
version: '2023-10-31',
describe('when there are no hosts', () => {
beforeEach(() => {
endpointPageHttpMock(fakeHttpServices);
const apiResponse = getEndpointListApiResponse({ total: 0 });
fakeHttpServices.get.mockResolvedValue(apiResponse);
});

it('should NOT fetch agent policies without required privileges', async () => {
canFetchAgentPoliciesMock.mockReturnValue(false);

dispatchUserChangedUrlToEndpointList();

await Promise.all([
waitForAction('serverReturnedEndpointList'),
waitForAction('serverReturnedEndpointExistValue', {
validate: ({ payload }) => payload === false,
}),
waitForAction('serverCancelledPolicyItemsLoading'),
]);
expect(fakeHttpServices.get).not.toHaveBeenCalledWith(
INGEST_API_PACKAGE_POLICIES,
expect.objectContaining({})
);
});

it('should fetch agent policies with required privileges', async () => {
canFetchAgentPoliciesMock.mockReturnValue(true);

dispatchUserChangedUrlToEndpointList();

await Promise.all([
waitForAction('serverReturnedEndpointList'),
waitForAction('serverReturnedEndpointExistValue', {
validate: ({ payload }) => payload === false,
}),
waitForAction('serverReturnedPoliciesForOnboarding'),
]);
expect(fakeHttpServices.get).toHaveBeenCalledWith(
INGEST_API_PACKAGE_POLICIES,
expect.objectContaining({})
);
});
});
expect(listData(getState())).toEqual(apiResponse.data);
});

it('handles `appRequestedEndpointList`', async () => {
Expand Down
39 changes: 23 additions & 16 deletions x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
IndexFieldsStrategyRequest,
IndexFieldsStrategyResponse,
} from '@kbn/timelines-plugin/common';
import { canFetchAgentPolicies } from '../../../../../common/endpoint/service/authz/authz';
import type {
IsolationRouteRequestBody,
UnisolationRouteRequestBody,
Expand Down Expand Up @@ -366,25 +367,31 @@ async function endpointListMiddleware({
payload: false,
});

try {
const policyDataResponse: GetPolicyListResponse =
await sendGetEndpointSpecificPackagePolicies(http, {
query: {
perPage: 50, // Since this is an onboarding flow, we'll cap at 50 policies.
page: 1,
if (canFetchAgentPolicies(coreStart.application.capabilities)) {
try {
const policyDataResponse: GetPolicyListResponse =
await sendGetEndpointSpecificPackagePolicies(http, {
query: {
perPage: 50, // Since this is an onboarding flow, we'll cap at 50 policies.
page: 1,
},
});

dispatch({
type: 'serverReturnedPoliciesForOnboarding',
payload: {
policyItems: policyDataResponse.items,
},
});

dispatch({
type: 'serverReturnedPoliciesForOnboarding',
payload: {
policyItems: policyDataResponse.items,
},
});
} catch (error) {
} catch (error) {
dispatch({
type: 'serverFailedToReturnPoliciesForOnboarding',
payload: error.body ?? error,
});
}
} else {
dispatch({
type: 'serverFailedToReturnPoliciesForOnboarding',
payload: error.body ?? error,
type: 'serverCancelledPolicyItemsLoading',
});
}
} else {
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ describe('when on the endpoint list page', () => {
// to use services that we have in our test `mockedContext`
(useToasts as jest.Mock).mockReturnValue(coreStart.notifications.toasts);
(useKibana as jest.Mock).mockReturnValue({ services: mockedContext.startServices });

coreStart.application.capabilities = {
...coreStart.application.capabilities,
siem: { readPolicyManagement: true },
};
});

it('should NOT display timeline', async () => {
Expand Down

0 comments on commit a3311ad

Please sign in to comment.