Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
74ba729
api changes
parkiino Nov 23, 2021
b195533
swap out services url from ingest to our own endpoint api
parkiino Nov 23, 2021
a0e7470
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Nov 29, 2021
b4cbe40
add request schema, try catch bloack
parkiino Nov 29, 2021
074a51f
copy fleet package policy request schema to security solution
parkiino Nov 30, 2021
efb6b85
add endpoint filtering to handler, throw error if needed
parkiino Nov 30, 2021
06fc5cb
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 1, 2021
dbd2072
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 2, 2021
e8497a1
modify schema to follow endpoint api patterns, revert policy/service …
parkiino Dec 3, 2021
8df1bc9
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 6, 2021
7f672d3
adjust kuery to include parentheses
parkiino Dec 6, 2021
c4b4845
a single passing unit test
parkiino Dec 7, 2021
d52464a
add another expect, remove unnecessary lines?
parkiino Dec 7, 2021
c885bb5
adding package policy service back, deleting unnecessary code
parkiino Dec 8, 2021
7405ead
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 8, 2021
7c189d1
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 8, 2021
d1e827a
finished tests
parkiino Dec 8, 2021
14980a1
Merge remote-tracking branch 'upstream/main' into task/get-endpoint-p…
parkiino Dec 14, 2021
7a7ae9b
fix change and add parens
parkiino Dec 14, 2021
1695276
fix jest test
parkiino Dec 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions x-pack/plugins/security_solution/common/endpoint/schema/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,21 @@ export const GetAgentPolicySummaryRequestSchema = {
policy_id: schema.nullable(schema.string()),
}),
};

const ListWithKuerySchema = schema.object({
Copy link
Copy Markdown
Member

@joeypoon joeypoon Dec 9, 2021

Choose a reason for hiding this comment

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

would be good to add tests for these. I have a suspicion that these default values might not work since they're wrapped in a maybe.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

page: schema.maybe(schema.number({ defaultValue: 1 })),
pageSize: schema.maybe(schema.number({ defaultValue: 20 })),
sort: schema.maybe(schema.string()),
sortOrder: schema.maybe(schema.oneOf([schema.literal('desc'), schema.literal('asc')])),
showUpgradeable: schema.maybe(schema.boolean()),
kuery: schema.maybe(
schema.oneOf([
schema.string(),
schema.any(), // KueryNode
])
),
});

export const GetEndpointPackagePolicyRequestSchema = {
query: ListWithKuerySchema,
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,17 @@ import {
createMockEndpointAppContextServiceStartContract,
createRouteHandlerContext,
} from '../../mocks';
import { createMockAgentClient, createMockAgentService } from '../../../../../fleet/server/mocks';
import { getHostPolicyResponseHandler, getAgentPolicySummaryHandler } from './handlers';
import {
createMockAgentClient,
createMockAgentService,
createPackagePolicyServiceMock,
} from '../../../../../fleet/server/mocks';
import { PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../../../../../fleet/common';
import {
getHostPolicyResponseHandler,
getAgentPolicySummaryHandler,
getPolicyListHandler,
} from './handlers';
import {
KibanaResponseFactory,
SavedObjectsClientContract,
Expand All @@ -33,6 +42,7 @@ import { AgentClient, AgentService } from '../../../../../fleet/server/services'
import { get } from 'lodash';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ScopedClusterClientMock } from '../../../../../../../src/core/server/elasticsearch/client/mocks';
import { PackagePolicyServiceInterface } from '../../../../../fleet/server';

describe('test policy response handler', () => {
let endpointAppContextService: EndpointAppContextService;
Expand Down Expand Up @@ -236,6 +246,80 @@ describe('test policy response handler', () => {
});
});
});
describe('test GET policy list handler', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let mockPackagePolicyService: jest.Mocked<PackagePolicyServiceInterface>;
let policyHandler: ReturnType<typeof getPolicyListHandler>;

beforeEach(() => {
mockScopedClient = elasticsearchServiceMock.createScopedClusterClient();
mockSavedObjectClient = savedObjectsClientMock.create();
mockResponse = httpServerMock.createResponseFactory();
mockPackagePolicyService = createPackagePolicyServiceMock();
mockPackagePolicyService.list.mockImplementation(() => {
return Promise.resolve({
items: [],
total: 0,
page: 1,
perPage: 10,
});
});
endpointAppContextService = new EndpointAppContextService();
endpointAppContextService.setup(createMockEndpointAppContextServiceSetupContract());
endpointAppContextService.start({
...createMockEndpointAppContextServiceStartContract(),
...{ packagePolicyService: mockPackagePolicyService },
});
policyHandler = getPolicyListHandler({
logFactory: loggingSystemMock.create(),
service: endpointAppContextService,
config: () => Promise.resolve(createMockConfig()),
experimentalFeatures: parseExperimentalConfigValue(createMockConfig().enableExperimental),
});
});

afterEach(() => endpointAppContextService.stop());

it('should return a list of endpoint package policies', async () => {
const mockRequest = httpServerMock.createKibanaRequest({
query: {},
});

await policyHandler(
createRouteHandlerContext(mockScopedClient, mockSavedObjectClient),
mockRequest,
mockResponse
);
expect(mockPackagePolicyService.list.mock.calls[0][1]).toEqual({
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name: endpoint`,
perPage: undefined,
sortField: undefined,
});
expect(mockResponse.ok).toBeCalled();
expect(mockResponse.ok.mock.calls[0][0]?.body).toEqual({
items: [],
total: 0,
page: 1,
perPage: 10,
});
});

it('should add endpoint-specific kuery to the requests kuery', async () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would be good to also add tests for the other query params too.

const mockRequest = httpServerMock.createKibanaRequest({
query: { kuery: 'some query' },
});

await policyHandler(
createRouteHandlerContext(mockScopedClient, mockSavedObjectClient),
mockRequest,
mockResponse
);
expect(mockPackagePolicyService.list.mock.calls[0][1]).toEqual({
kuery: `(some query) and ${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name: endpoint`,
perPage: undefined,
sortField: undefined,
});
});
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ import { policyIndexPattern } from '../../../../common/endpoint/constants';
import {
GetPolicyResponseSchema,
GetAgentPolicySummaryRequestSchema,
GetEndpointPackagePolicyRequestSchema,
} from '../../../../common/endpoint/schema/policy';
import { EndpointAppContext } from '../../types';
import { getAgentPolicySummary, getPolicyResponseByAgentId } from './service';
import { GetAgentSummaryResponse } from '../../../../common/endpoint/types';
import { wrapErrorIfNeeded } from '../../utils';
import { PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../../../../../fleet/common';

export const getHostPolicyResponseHandler = function (): RequestHandler<
undefined,
Expand Down Expand Up @@ -64,3 +67,33 @@ export const getAgentPolicySummaryHandler = function (
});
};
};

export const getPolicyListHandler = function (
endpointAppContext: EndpointAppContext
): RequestHandler<
undefined,
TypeOf<typeof GetEndpointPackagePolicyRequestSchema.query>,
undefined
> {
return async (context, request, response) => {
const soClient = context.core.savedObjects.client;
const fleetServices = endpointAppContext.service.getScopedFleetServices(request);
const endpointFilteredKuery = `${
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does not look correct - you need to ensure that both side of the AND are wrapped in parentheses

Suggested change
const endpointFilteredKuery = `${
const endpointFilteredKuery = `${request?.query?.kuery ? `(${request.query.kuery}) AND ` : ''
}(${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name: endpoint)`

Here is a test that you can do to see why this is important (note: i did not actually execute this, but you should be able to and get the results I'm thinking you should get):

  • call this API (prior to my suggestion above) with: query.kuery: '(ingest-package-policies.package.name: nginx) OR '

This will likely match and return package policies for nginx (if you have any) instead of only looking for endpoint. Thats because the parentheses are evaluated first and if true, then the other side of the OR will never be eval'd and thus the API can be abused to actually query for non-endpoint stuff.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I worked with @parkiino yesterday on this and we validated that what i thought was going to happen (retrieval of non-endpoint package policies) did not actually happen. that's because even if a user of the API ends their kql with 0R, when we append the AND package.name: endpoint to it, you get a KQL parsing error (...OR AND...). So I think we're good, especially since parentheses were added around the request's kuery value (if any).

request?.query?.kuery ? `(${request.query.kuery}) and ` : ''
}${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name: endpoint`;
try {
const listResponse = await fleetServices.packagePolicy.list(soClient, {
...request.query,
perPage: request.query.pageSize,
sortField: request.query.sort,
kuery: endpointFilteredKuery,
});

return response.ok({
body: listResponse,
});
} catch (error) {
throw wrapErrorIfNeeded(error);
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@ import { EndpointAppContext } from '../../types';
import {
GetPolicyResponseSchema,
GetAgentPolicySummaryRequestSchema,
GetEndpointPackagePolicyRequestSchema,
} from '../../../../common/endpoint/schema/policy';
import { getHostPolicyResponseHandler, getAgentPolicySummaryHandler } from './handlers';
import {
getHostPolicyResponseHandler,
getAgentPolicySummaryHandler,
getPolicyListHandler,
} from './handlers';
import {
AGENT_POLICY_SUMMARY_ROUTE,
BASE_POLICY_ROUTE,
BASE_POLICY_RESPONSE_ROUTE,
} from '../../../../common/endpoint/constants';

Expand All @@ -37,4 +43,13 @@ export function registerPolicyRoutes(router: IRouter, endpointAppContext: Endpoi
},
getAgentPolicySummaryHandler(endpointAppContext)
);

router.get(
{
path: BASE_POLICY_ROUTE,
validate: GetEndpointPackagePolicyRequestSchema,
options: { authRequired: true },
},
getPolicyListHandler(endpointAppContext)
);
}