Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security solution] [Endpoint] Remove linked policy from trusted apps when removing endpoint integration #108347

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b8550b0
Remove policy from trusted app when this is removed from fleet
dasansol92 Aug 3, 2021
d7a3d91
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
dasansol92 Aug 9, 2021
6ab3659
Use exceptionsList client instead of saveObject client
dasansol92 Aug 9, 2021
745c43e
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
dasansol92 Aug 11, 2021
40e16ee
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
dasansol92 Aug 11, 2021
7be7809
Skip actions when FF is not enabled
dasansol92 Aug 12, 2021
561fe39
Adds missing file on last commit - skip remove action if FF is not en…
dasansol92 Aug 12, 2021
97bcda9
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
dasansol92 Aug 12, 2021
06c62d8
Tries to add a unit test, needs to be completed/fixed
dasansol92 Aug 12, 2021
5e7b936
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
kibanamachine Aug 12, 2021
74e89b4
Merge branch 'master' into feat/olm-any_policy_specific_trusted_apps_…
dasansol92 Aug 12, 2021
b0ba9c8
Fixes type check error
dasansol92 Aug 12, 2021
2b0efcf
Fixes some errors and adds a unit test. There is a casting needs to b…
dasansol92 Aug 13, 2021
aca93ba
Unlink policy from TA when removing the entire policy. Also fixes an …
dasansol92 Aug 13, 2021
26f90e8
Move some code inside before each in order to have fresh instance on …
dasansol92 Aug 13, 2021
ad0278f
Fix error on lists, use push instead concat
dasansol92 Aug 13, 2021
8161e4c
log error itself apart from the error message
dasansol92 Aug 13, 2021
968368d
Adjust types in Security Solution fleet integration handlers
paul-tavares Aug 16, 2021
4248532
Merge remote-tracking branch 'upstream/master' into pr/david/olm-any_…
paul-tavares Aug 16, 2021
d03a556
Fleet: refactor running the delete package policy callbacks so that i…
paul-tavares Aug 16, 2021
ce7f1bb
Merge remote-tracking branch 'upstream/master' into pr/david/olm-any_…
paul-tavares Aug 17, 2021
faa0e24
Fix mock
paul-tavares Aug 18, 2021
d76e230
Merge remote-tracking branch 'upstream/master' into pr/david/olm-any_…
paul-tavares Aug 18, 2021
36bb904
Allow all delete external callbacks to be executed even if one return…
paul-tavares Aug 18, 2021
73c9a00
Fix fleet integration tests
paul-tavares Aug 18, 2021
4afb3e5
AgentPolicy delete tests to ensure `packagePolicy.runDeleteExternalCa…
paul-tavares Aug 18, 2021
9f487ff
tests for `PackagePolicyService.runDeleteExternalCallbacks()`
paul-tavares Aug 18, 2021
49bf709
code review feedback
paul-tavares Aug 18, 2021
88bd73a
Merge remote-tracking branch 'upstream/master' into pr/david/olm-any_…
paul-tavares Aug 18, 2021
e9b4fb4
Fix types in APM registerFleetPolicyCallbacks()
paul-tavares Aug 18, 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

import { APMPlugin, APMRouteHandlerResources } from '../..';
import {
ExternalCallback,
PostPackagePolicyDeleteCallback,
PostPackagePolicyCreateCallback,
PutPackagePolicyUpdateCallback,
} from '../../../../fleet/server';
import {
Expand Down Expand Up @@ -60,7 +59,9 @@ export async function registerFleetPolicyCallbacks({
});
}

type ExternalCallbackParams = Parameters<ExternalCallback[1]>;
type ExternalCallbackParams =
| Parameters<PostPackagePolicyCreateCallback>
| Parameters<PutPackagePolicyUpdateCallback>;
export type PackagePolicy = NewPackagePolicy | UpdatePackagePolicy;
type Context = ExternalCallbackParams[1];
type Request = ExternalCallbackParams[2];
Expand All @@ -81,7 +82,7 @@ function registerPackagePolicyExternalCallback({
logger: NonNullable<APMPlugin['logger']>;
}) {
const callbackFn:
| PostPackagePolicyDeleteCallback
| PostPackagePolicyCreateCallback
| PutPackagePolicyUpdateCallback = async (
packagePolicy: PackagePolicy,
context: Context,
Expand Down
13 changes: 12 additions & 1 deletion x-pack/plugins/fleet/common/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { NewPackagePolicy, PackagePolicy } from './types';
import type { NewPackagePolicy, PackagePolicy, DeletePackagePoliciesResponse } from './types';

export const createNewPackagePolicyMock = (): NewPackagePolicy => {
return {
Expand Down Expand Up @@ -45,3 +45,14 @@ export const createPackagePolicyMock = (): PackagePolicy => {
],
};
};

export const deletePackagePolicyMock = (): DeletePackagePoliciesResponse => {
const newPackagePolicy = createNewPackagePolicyMock();
return [
{
id: 'c6d16e42-c32d-4dce-8a88-113cfe276ad1',
success: true,
package: newPackagePolicy.package,
},
];
};
5 changes: 3 additions & 2 deletions x-pack/plugins/fleet/server/mocks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const xpackMocks = {
createRequestHandlerContext: createCoreRequestHandlerContextMock,
};

export const createPackagePolicyServiceMock = () => {
export const createPackagePolicyServiceMock = (): jest.Mocked<PackagePolicyServiceInterface> => {
return {
compilePackagePolicyInputs: jest.fn(),
buildPackagePolicyFromPackage: jest.fn(),
Expand All @@ -75,10 +75,11 @@ export const createPackagePolicyServiceMock = () => {
listIds: jest.fn(),
update: jest.fn(),
runExternalCallbacks: jest.fn(),
runDeleteExternalCallbacks: jest.fn(),
upgrade: jest.fn(),
getUpgradeDryRunDiff: jest.fn(),
getUpgradePackagePolicyInfo: jest.fn(),
} as jest.Mocked<PackagePolicyServiceInterface>;
};
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export const deletePackagePolicyHandler: RequestHandler<
} catch (error) {
const logger = appContextService.getLogger();
logger.error(`An error occurred executing external callback: ${error}`);
logger.error(error);
}
return response.ok({
body,
Expand Down
35 changes: 35 additions & 0 deletions x-pack/plugins/fleet/server/services/agent_policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import type { AgentPolicy, NewAgentPolicy, Output } from '../types';
import { agentPolicyService } from './agent_policy';
import { agentPolicyUpdateEventHandler } from './agent_policy_update';

import { getAgentsByKuery } from './agents';
import { packagePolicyService } from './package_policy';

function getSavedObjectMock(agentPolicyAttributes: any) {
const mock = savedObjectsClientMock.create();
mock.get.mockImplementation(async (type: string, id: string) => {
Expand Down Expand Up @@ -63,6 +66,8 @@ jest.mock('./output', () => {
});

jest.mock('./agent_policy_update');
jest.mock('./agents');
jest.mock('./package_policy');

function getAgentPolicyUpdateMock() {
return (agentPolicyUpdateEventHandler as unknown) as jest.Mock<
Expand Down Expand Up @@ -123,6 +128,36 @@ describe('agent policy', () => {
});
});

describe('delete', () => {
let soClient: ReturnType<typeof savedObjectsClientMock.create>;
let esClient: ReturnType<typeof elasticsearchServiceMock.createClusterClient>['asInternalUser'];

beforeEach(() => {
soClient = getSavedObjectMock({ revision: 1, package_policies: ['package-1'] });
esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;

(getAgentsByKuery as jest.Mock).mockResolvedValue({
agents: [],
total: 0,
page: 1,
perPage: 10,
});

(packagePolicyService.delete as jest.Mock).mockResolvedValue([
{
id: 'package-1',
},
]);
});

it('should run package policy delete external callbacks', async () => {
await agentPolicyService.delete(soClient, esClient, 'mocked');
expect(packagePolicyService.runDeleteExternalCallbacks).toHaveBeenCalledWith([
{ id: 'package-1' },
]);
});
});

describe('bumpRevision', () => {
it('should call agentPolicyUpdateEventHandler with updated event once', async () => {
const soClient = getSavedObjectMock({
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type {
FleetServerPolicy,
Installation,
Output,
DeletePackagePoliciesResponse,
} from '../../common';
import { AgentPolicyNameExistsError, HostedAgentPolicyRestrictionRelatedError } from '../errors';
import {
Expand Down Expand Up @@ -616,14 +617,21 @@ class AgentPolicyService {
}

if (agentPolicy.package_policies && agentPolicy.package_policies.length) {
await packagePolicyService.delete(
const deletedPackagePolicies: DeletePackagePoliciesResponse = await packagePolicyService.delete(
soClient,
esClient,
agentPolicy.package_policies as string[],
{
skipUnassignFromAgentPolicies: true,
}
);
try {
await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies);
} catch (error) {
const logger = appContextService.getLogger();
logger.error(`An error occurred executing external callback: ${error}`);
logger.error(error);
}
}

if (agentPolicy.is_preconfigured) {
Expand Down
78 changes: 78 additions & 0 deletions x-pack/plugins/fleet/server/services/package_policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ import type { PutPackagePolicyUpdateCallback, PostPackagePolicyCreateCallback }

import { createAppContextStartContractMock, xpackMocks } from '../mocks';

import type { PostPackagePolicyDeleteCallback } from '../types';

import type { DeletePackagePoliciesResponse } from '../../common';

import { IngestManagerError } from '../errors';

import { packagePolicyService } from './package_policy';
import { appContextService } from './app_context';

Expand Down Expand Up @@ -815,6 +821,78 @@ describe('Package policy service', () => {
});
});

describe('runDeleteExternalCallbacks', () => {
let callbackOne: jest.MockedFunction<PostPackagePolicyDeleteCallback>;
let callbackTwo: jest.MockedFunction<PostPackagePolicyDeleteCallback>;
let callingOrder: string[];
let deletedPackagePolicies: DeletePackagePoliciesResponse;

beforeEach(() => {
appContextService.start(createAppContextStartContractMock());
callingOrder = [];
deletedPackagePolicies = [
{ id: 'a', success: true },
{ id: 'a', success: true },
];
callbackOne = jest.fn(async (deletedPolicies) => {
callingOrder.push('one');
});
callbackTwo = jest.fn(async (deletedPolicies) => {
callingOrder.push('two');
});
appContextService.addExternalCallback('postPackagePolicyDelete', callbackOne);
appContextService.addExternalCallback('postPackagePolicyDelete', callbackTwo);
});

afterEach(() => {
appContextService.stop();
});

it('should execute external callbacks', async () => {
await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies);

expect(callbackOne).toHaveBeenCalledWith(deletedPackagePolicies);
expect(callbackTwo).toHaveBeenCalledWith(deletedPackagePolicies);
expect(callingOrder).toEqual(['one', 'two']);
});

it("should execute all external callbacks even if one throw's", async () => {
callbackOne.mockImplementation(async (deletedPolicies) => {
callingOrder.push('one');
throw new Error('foo');
});
await expect(
packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies)
).rejects.toThrow(IngestManagerError);
expect(callingOrder).toEqual(['one', 'two']);
});

it('should provide an array of errors encountered by running external callbacks', async () => {
let error: IngestManagerError;
const callbackOneError = new Error('foo 1');
const callbackTwoError = new Error('foo 2');

callbackOne.mockImplementation(async (deletedPolicies) => {
callingOrder.push('one');
throw callbackOneError;
});
callbackTwo.mockImplementation(async (deletedPolicies) => {
callingOrder.push('two');
throw callbackTwoError;
});

await packagePolicyService.runDeleteExternalCallbacks(deletedPackagePolicies).catch((e) => {
error = e;
});

expect(error!.message).toEqual(
'2 encountered while executing package delete external callbacks'
);
expect(error!.meta).toEqual([callbackOneError, callbackTwoError]);
expect(callingOrder).toEqual(['one', 'two']);
});
});

describe('runExternalCallbacks', () => {
let context: ReturnType<typeof xpackMocks.createRequestHandlerContext>;
let request: KibanaRequest;
Expand Down
45 changes: 33 additions & 12 deletions x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,9 @@ class PackagePolicyService {
name: packagePolicy.name,
success: true,
package: {
name: packagePolicy.name,
title: '',
version: packagePolicy.version || '',
name: packagePolicy.package?.name || '',
title: packagePolicy.package?.title || '',
version: packagePolicy.package?.version || '',
},
});
} catch (error) {
Expand Down Expand Up @@ -642,7 +642,9 @@ class PackagePolicyService {

public async runExternalCallbacks<A extends ExternalCallback[0]>(
externalCallbackType: A,
packagePolicy: NewPackagePolicy | DeletePackagePoliciesResponse,
packagePolicy: A extends 'postPackagePolicyDelete'
? DeletePackagePoliciesResponse
: NewPackagePolicy,
context: RequestHandlerContext,
request: KibanaRequest
): Promise<A extends 'postPackagePolicyDelete' ? void : NewPackagePolicy>;
Expand All @@ -653,14 +655,7 @@ class PackagePolicyService {
request: KibanaRequest
): Promise<NewPackagePolicy | void> {
if (externalCallbackType === 'postPackagePolicyDelete') {
const externalCallbacks = appContextService.getExternalCallbacks(externalCallbackType);
if (externalCallbacks && externalCallbacks.size > 0) {
for (const callback of externalCallbacks) {
if (Array.isArray(packagePolicy)) {
await callback(packagePolicy, context, request);
}
}
}
return await this.runDeleteExternalCallbacks(packagePolicy as DeletePackagePoliciesResponse);
} else {
if (!Array.isArray(packagePolicy)) {
let newData = packagePolicy;
Expand All @@ -682,6 +677,32 @@ class PackagePolicyService {
}
}
}

public async runDeleteExternalCallbacks(
deletedPackagePolicies: DeletePackagePoliciesResponse
): Promise<void> {
const externalCallbacks = appContextService.getExternalCallbacks('postPackagePolicyDelete');
const errorsThrown: Error[] = [];

if (externalCallbacks && externalCallbacks.size > 0) {
for (const callback of externalCallbacks) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

// Failures from an external callback should not prevent other external callbacks from being
// executed. Errors (if any) will be collected and `throw`n after processing the entire set
try {
await callback(deletedPackagePolicies);
} catch (error) {
errorsThrown.push(error);
}
}

if (errorsThrown.length > 0) {
throw new IngestManagerError(
`${errorsThrown.length} encountered while executing package delete external callbacks`,
errorsThrown
);
}
}
}
}

function assignStreamIdToInput(packagePolicyId: string, input: NewPackagePolicyInput) {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/fleet/server/types/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@

import type { KibanaRequest, RequestHandlerContext } from 'kibana/server';

import type { DeepReadonly } from 'utility-types';

import type {
DeletePackagePoliciesResponse,
NewPackagePolicy,
UpdatePackagePolicy,
} from '../../common';

export type PostPackagePolicyDeleteCallback = (
deletedPackagePolicies: DeletePackagePoliciesResponse,
context: RequestHandlerContext,
request: KibanaRequest
deletedPackagePolicies: DeepReadonly<DeletePackagePoliciesResponse>
) => Promise<void>;

export type PostPackagePolicyCreateCallback = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { PluginStartContract as AlertsPluginStartContract } from '../../../alert
import {
getPackagePolicyCreateCallback,
getPackagePolicyUpdateCallback,
getPackagePolicyDeleteCallback,
} from '../fleet_integration/fleet_integration';
import { ManifestManager } from './services/artifacts';
import { AppClientFactory } from '../client';
Expand Down Expand Up @@ -102,6 +103,11 @@ export class EndpointAppContextService {
'packagePolicyUpdate',
getPackagePolicyUpdateCallback(dependencies.logger, dependencies.licenseService)
);

dependencies.registerIngestCallback(
'postPackagePolicyDelete',
getPackagePolicyDeleteCallback(dependencies.exceptionListsClient, this.experimentalFeatures)
);
}
}

Expand Down
Loading