[Security Solution][Artifacts] Refactor endpoint Artifact manifest processing (#95846)

* Remove references to class `ArtifactClient` and replace with EndpointArtifactClientInterface
* refactor artifact client tests to use new class
* Added additional test to Fleet Artifacts create service
* remove SavedObject type wrapper from getArtifact response
This commit is contained in:
Paul Tavares 2021-04-01 09:31:17 -04:00 committed by GitHub
parent b29ccdcac1
commit 6238ef7bad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 103 additions and 181 deletions

View file

@ -9,6 +9,8 @@ import { elasticsearchServiceMock } from 'src/core/server/mocks';
import { ResponseError } from '@elastic/elasticsearch/lib/errors';
import type { ApiResponse } from '@elastic/elasticsearch/lib/Transport';
import { FLEET_SERVER_ARTIFACTS_INDEX } from '../../../common';
import { ArtifactsElasticsearchError } from '../../errors';
@ -100,6 +102,14 @@ describe('When using the artifacts services', () => {
});
});
it('should ignore 409 errors from elasticsearch', async () => {
const error = new ResponseError({ statusCode: 409 } as ApiResponse);
// Unclear why `mockRejectedValue()` has the params value type set to `never`
// @ts-expect-error
esClientMock.create.mockRejectedValue(error);
await expect(() => createArtifact(esClientMock, newArtifact)).not.toThrow();
});
it('should throw an ArtifactElasticsearchError if one is encountered', async () => {
setEsClientMethodResponseToError(esClientMock, 'create');
await expect(createArtifact(esClientMock, newArtifact)).rejects.toBeInstanceOf(

View file

@ -15,7 +15,7 @@ import {
export const ArtifactConstants = {
GLOBAL_ALLOWLIST_NAME: 'endpoint-exceptionlist',
/**
* Saved objects no longer used for storing artifacts. Value
* Saved objects no longer used for storing artifacts
* @deprecated
*/
SAVED_OBJECT_TYPE: 'endpoint:user-artifact',

View file

@ -171,7 +171,7 @@ describe('test alerts route', () => {
// and this entire test file refactored to start using fleet's exposed FleetArtifactClient class.
endpointAppContextService!
.getManifestManager()!
.getArtifactsClient().getArtifact = jest.fn().mockResolvedValue(soFindResp);
.getArtifactsClient().getArtifact = jest.fn().mockResolvedValue(soFindResp.attributes);
[routeConfig, routeHandler] = routerMock.get.mock.calls.find(([{ path }]) =>
path.startsWith('/api/endpoint/artifacts/download')

View file

@ -91,9 +91,9 @@ export function registerDownloadArtifactRoute(
return res.notFound({ body: `No artifact found for ${id}` });
}
const bodyBuffer = Buffer.from(artifact.attributes.body, 'base64');
const bodyBuffer = Buffer.from(artifact.body, 'base64');
cache.set(id, bodyBuffer);
return buildAndValidateResponse(artifact.attributes.identifier, bodyBuffer);
return buildAndValidateResponse(artifact.identifier, bodyBuffer);
}
}
);

View file

@ -5,48 +5,49 @@
* 2.0.
*/
import { savedObjectsClientMock } from 'src/core/server/mocks';
import { ArtifactConstants, getArtifactId } from '../../lib/artifacts';
import { getInternalArtifactMock } from '../../schemas/artifacts/saved_objects.mock';
import { ArtifactClient } from './artifact_client';
import { EndpointArtifactClient } from './artifact_client';
import { createArtifactsClientMock } from '../../../../../fleet/server/mocks';
describe('artifact_client', () => {
describe('ArtifactClient sanity checks', () => {
let fleetArtifactClient: ReturnType<typeof createArtifactsClientMock>;
let artifactClient: EndpointArtifactClient;
beforeEach(() => {
fleetArtifactClient = createArtifactsClientMock();
artifactClient = new EndpointArtifactClient(fleetArtifactClient);
});
test('can create ArtifactClient', () => {
const artifactClient = new ArtifactClient(savedObjectsClientMock.create());
expect(artifactClient).toBeInstanceOf(ArtifactClient);
expect(artifactClient).toBeInstanceOf(EndpointArtifactClient);
});
test('can get artifact', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const artifactClient = new ArtifactClient(savedObjectsClient);
await artifactClient.getArtifact('abcd');
expect(savedObjectsClient.get).toHaveBeenCalled();
expect(fleetArtifactClient.listArtifacts).toHaveBeenCalled();
});
test('can create artifact', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const artifactClient = new ArtifactClient(savedObjectsClient);
const artifact = await getInternalArtifactMock('linux', 'v1');
const artifact = await getInternalArtifactMock('linux', 'v1', { compress: true });
await artifactClient.createArtifact(artifact);
expect(savedObjectsClient.create).toHaveBeenCalledWith(
ArtifactConstants.SAVED_OBJECT_TYPE,
{
...artifact,
created: expect.any(Number),
},
{ id: getArtifactId(artifact) }
);
expect(fleetArtifactClient.createArtifact).toHaveBeenCalledWith({
identifier: artifact.identifier,
type: 'exceptionlist',
content:
'{"entries":[{"type":"simple","entries":[{"entries":[{"field":"some.nested.field","operator":"included","type":"exact_cased","value":"some value"}],' +
'"field":"some.parentField","type":"nested"},{"field":"some.not.nested.field","operator":"included","type":"exact_cased","value":"some value"}]},' +
'{"type":"simple","entries":[{"field":"some.other.not.nested.field","operator":"included","type":"exact_cased","value":"some other value"}]}]}',
});
});
test('can delete artifact', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const artifactClient = new ArtifactClient(savedObjectsClient);
await artifactClient.deleteArtifact('abcd');
expect(savedObjectsClient.delete).toHaveBeenCalledWith(
ArtifactConstants.SAVED_OBJECT_TYPE,
'abcd'
);
await artifactClient.deleteArtifact('endpoint-trustlist-linux-v1-sha26hash');
expect(fleetArtifactClient.listArtifacts).toHaveBeenCalledWith({
kuery: `decoded_sha256: "sha26hash" AND identifier: "endpoint-trustlist-linux-v1"`,
perPage: 1,
});
expect(fleetArtifactClient.deleteArtifact).toHaveBeenCalledWith('123');
});
});
});

View file

@ -5,64 +5,23 @@
* 2.0.
*/
/* eslint-disable max-classes-per-file */
import { inflate as _inflate } from 'zlib';
import { promisify } from 'util';
import { SavedObject, SavedObjectsClientContract } from 'src/core/server';
import { ArtifactConstants, getArtifactId } from '../../lib/artifacts';
import {
InternalArtifactCompleteSchema,
InternalArtifactCreateSchema,
} from '../../schemas/artifacts';
import { InternalArtifactCompleteSchema } from '../../schemas/artifacts';
import { Artifact, ArtifactsClientInterface } from '../../../../../fleet/server';
const inflateAsync = promisify(_inflate);
export interface EndpointArtifactClientInterface {
getArtifact(id: string): Promise<SavedObject<InternalArtifactCompleteSchema> | undefined>;
getArtifact(id: string): Promise<InternalArtifactCompleteSchema | undefined>;
createArtifact(
artifact: InternalArtifactCompleteSchema
): Promise<SavedObject<InternalArtifactCompleteSchema>>;
createArtifact(artifact: InternalArtifactCompleteSchema): Promise<InternalArtifactCompleteSchema>;
deleteArtifact(id: string): Promise<void>;
}
export class ArtifactClient implements EndpointArtifactClientInterface {
private savedObjectsClient: SavedObjectsClientContract;
constructor(savedObjectsClient: SavedObjectsClientContract) {
this.savedObjectsClient = savedObjectsClient;
}
public async getArtifact(id: string): Promise<SavedObject<InternalArtifactCompleteSchema>> {
return this.savedObjectsClient.get<InternalArtifactCompleteSchema>(
ArtifactConstants.SAVED_OBJECT_TYPE,
id
);
}
public async createArtifact(
artifact: InternalArtifactCompleteSchema
): Promise<SavedObject<InternalArtifactCompleteSchema>> {
return this.savedObjectsClient.create<InternalArtifactCreateSchema>(
ArtifactConstants.SAVED_OBJECT_TYPE,
{
...artifact,
created: Date.now(),
},
{ id: getArtifactId(artifact) }
);
}
public async deleteArtifact(id: string) {
await this.savedObjectsClient.delete(ArtifactConstants.SAVED_OBJECT_TYPE, id);
}
}
/**
* Endpoint specific artifact managment client which uses FleetArtifactsClient to persist artifacts
* Endpoint specific artifact management client which uses FleetArtifactsClient to persist artifacts
* to the Fleet artifacts index (then used by Fleet Server)
*/
export class EndpointArtifactClient implements EndpointArtifactClientInterface {
@ -91,15 +50,12 @@ export class EndpointArtifactClient implements EndpointArtifactClientInterface {
return;
}
// FIXME:PT change method signature so that it returns back only the `InternalArtifactCompleteSchema`
return ({
attributes: artifacts.items[0],
} as unknown) as SavedObject<InternalArtifactCompleteSchema>;
return artifacts.items[0];
}
async createArtifact(
artifact: InternalArtifactCompleteSchema
): Promise<SavedObject<InternalArtifactCompleteSchema>> {
): Promise<InternalArtifactCompleteSchema> {
// FIXME:PT refactor to make this more efficient by passing through the uncompressed artifact content
// Artifact `.body` is compressed/encoded. We need it decoded and as a string
const artifactContent = await inflateAsync(Buffer.from(artifact.body, 'base64'));
@ -110,15 +66,13 @@ export class EndpointArtifactClient implements EndpointArtifactClientInterface {
type: this.parseArtifactId(artifact.identifier).type,
});
return ({
attributes: createdArtifact,
} as unknown) as SavedObject<InternalArtifactCompleteSchema>;
return createdArtifact;
}
async deleteArtifact(id: string) {
// Ignoring the `id` not being in the type until we can refactor the types in endpoint.
// @ts-ignore
const artifactId = (await this.getArtifact(id)).attributes?.id;
const artifactId = (await this.getArtifact(id))?.id!;
return this.fleetArtifacts.deleteArtifact(artifactId);
}
}

View file

@ -20,8 +20,7 @@ import {
getMockArtifactsWithDiff,
getEmptyMockArtifacts,
} from '../../../lib/artifacts/mocks';
import { ArtifactClient } from '../artifact_client';
import { getManifestClientMock } from '../mocks';
import { createEndpointArtifactClientMock, getManifestClientMock } from '../mocks';
import { ManifestManager, ManifestManagerContext } from './manifest_manager';
export const createExceptionListResponse = (data: ExceptionListItemSchema[], total?: number) => ({
@ -84,7 +83,7 @@ export const buildManifestManagerContextMock = (
return {
...fullOpts,
artifactClient: new ArtifactClient(fullOpts.savedObjectsClient),
artifactClient: createEndpointArtifactClientMock(),
logger: loggingSystemMock.create().get() as jest.Mocked<Logger>,
};
};

View file

@ -6,7 +6,6 @@
*/
import { inflateSync } from 'zlib';
import { SavedObjectsErrorHelpers } from 'src/core/server';
import { savedObjectsClientMock } from 'src/core/server/mocks';
import { ENDPOINT_LIST_ID, ENDPOINT_TRUSTED_APPS_LIST_ID } from '../../../../../../lists/common';
import { getExceptionListItemSchemaMock } from '../../../../../../lists/common/schemas/response/exception_list_item_schema.mock';
@ -23,7 +22,6 @@ import {
toArtifactRecords,
} from '../../../lib/artifacts/mocks';
import {
ArtifactConstants,
ManifestConstants,
getArtifactId,
isCompressed,
@ -37,6 +35,7 @@ import {
} from './manifest_manager.mock';
import { ManifestManager } from './manifest_manager';
import { EndpointArtifactClientInterface } from '../artifact_client';
const uncompressData = async (data: Buffer) => JSON.parse(await inflateSync(data).toString());
@ -145,9 +144,8 @@ describe('ManifestManager', () => {
test('Retrieves non empty manifest successfully', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const manifestManager = new ManifestManager(
buildManifestManagerContextMock({ savedObjectsClient })
);
const manifestManagerContext = buildManifestManagerContextMock({ savedObjectsClient });
const manifestManager = new ManifestManager(manifestManagerContext);
savedObjectsClient.get = jest
.fn()
@ -169,13 +167,17 @@ describe('ManifestManager', () => {
},
version: '2.0.0',
};
} else if (objectType === ArtifactConstants.SAVED_OBJECT_TYPE) {
return { attributes: ARTIFACTS_BY_ID[id], version: '2.1.1' };
} else {
return null;
}
});
(manifestManagerContext.artifactClient as jest.Mocked<EndpointArtifactClientInterface>).getArtifact.mockImplementation(
async (id) => {
return ARTIFACTS_BY_ID[id];
}
);
const manifest = await manifestManager.getLastComputedManifest();
expect(manifest?.getSchemaVersion()).toStrictEqual('v1');
@ -418,8 +420,6 @@ describe('ManifestManager', () => {
const context = buildManifestManagerContextMock({});
const manifestManager = new ManifestManager(context);
context.savedObjectsClient.delete = jest.fn().mockResolvedValue({});
await expect(
manifestManager.deleteArtifacts([
ARTIFACT_ID_EXCEPTIONS_MACOS,
@ -427,32 +427,27 @@ describe('ManifestManager', () => {
])
).resolves.toStrictEqual([]);
expect(context.savedObjectsClient.delete).toHaveBeenNthCalledWith(
expect(context.artifactClient.deleteArtifact).toHaveBeenNthCalledWith(
1,
ArtifactConstants.SAVED_OBJECT_TYPE,
ARTIFACT_ID_EXCEPTIONS_MACOS
);
expect(context.savedObjectsClient.delete).toHaveBeenNthCalledWith(
expect(context.artifactClient.deleteArtifact).toHaveBeenNthCalledWith(
2,
ArtifactConstants.SAVED_OBJECT_TYPE,
ARTIFACT_ID_EXCEPTIONS_WINDOWS
);
});
test('Returns errors for partial failures', async () => {
const context = buildManifestManagerContextMock({});
const artifactClient = context.artifactClient as jest.Mocked<EndpointArtifactClientInterface>;
const manifestManager = new ManifestManager(context);
const error = new Error();
context.savedObjectsClient.delete = jest
.fn()
.mockImplementation(async (type: string, id: string) => {
if (id === ARTIFACT_ID_EXCEPTIONS_WINDOWS) {
throw error;
} else {
return {};
}
});
artifactClient.deleteArtifact.mockImplementation(async (id) => {
if (id === ARTIFACT_ID_EXCEPTIONS_WINDOWS) {
throw error;
}
});
await expect(
manifestManager.deleteArtifacts([
@ -461,46 +456,35 @@ describe('ManifestManager', () => {
])
).resolves.toStrictEqual([error]);
expect(context.savedObjectsClient.delete).toHaveBeenCalledTimes(2);
expect(context.savedObjectsClient.delete).toHaveBeenNthCalledWith(
expect(artifactClient.deleteArtifact).toHaveBeenCalledTimes(2);
expect(artifactClient.deleteArtifact).toHaveBeenNthCalledWith(
1,
ArtifactConstants.SAVED_OBJECT_TYPE,
ARTIFACT_ID_EXCEPTIONS_MACOS
);
expect(context.savedObjectsClient.delete).toHaveBeenNthCalledWith(
expect(artifactClient.deleteArtifact).toHaveBeenNthCalledWith(
2,
ArtifactConstants.SAVED_OBJECT_TYPE,
ARTIFACT_ID_EXCEPTIONS_WINDOWS
);
});
});
describe('pushArtifacts', () => {
test('Successfully invokes saved objects client and stores in the cache', async () => {
test('Successfully invokes artifactClient and stores in the cache', async () => {
const context = buildManifestManagerContextMock({});
const artifactClient = context.artifactClient as jest.Mocked<EndpointArtifactClientInterface>;
const manifestManager = new ManifestManager(context);
context.savedObjectsClient.create = jest
.fn()
.mockImplementation((type: string, artifact: InternalArtifactCompleteSchema) => artifact);
await expect(
manifestManager.pushArtifacts([ARTIFACT_EXCEPTIONS_MACOS, ARTIFACT_EXCEPTIONS_WINDOWS])
).resolves.toStrictEqual([]);
expect(context.savedObjectsClient.create).toHaveBeenCalledTimes(2);
expect(context.savedObjectsClient.create).toHaveBeenNthCalledWith(
1,
ArtifactConstants.SAVED_OBJECT_TYPE,
{ ...ARTIFACT_EXCEPTIONS_MACOS, created: expect.anything() },
{ id: ARTIFACT_ID_EXCEPTIONS_MACOS }
);
expect(context.savedObjectsClient.create).toHaveBeenNthCalledWith(
2,
ArtifactConstants.SAVED_OBJECT_TYPE,
{ ...ARTIFACT_EXCEPTIONS_WINDOWS, created: expect.anything() },
{ id: ARTIFACT_ID_EXCEPTIONS_WINDOWS }
);
expect(artifactClient.createArtifact).toHaveBeenCalledTimes(2);
expect(artifactClient.createArtifact).toHaveBeenNthCalledWith(1, {
...ARTIFACT_EXCEPTIONS_MACOS,
});
expect(artifactClient.createArtifact).toHaveBeenNthCalledWith(2, {
...ARTIFACT_EXCEPTIONS_WINDOWS,
});
expect(
await uncompressData(context.cache.get(getArtifactId(ARTIFACT_EXCEPTIONS_MACOS))!)
).toStrictEqual(await uncompressArtifact(ARTIFACT_EXCEPTIONS_MACOS));
@ -511,19 +495,20 @@ describe('ManifestManager', () => {
test('Returns errors for partial failures', async () => {
const context = buildManifestManagerContextMock({});
const artifactClient = context.artifactClient as jest.Mocked<EndpointArtifactClientInterface>;
const manifestManager = new ManifestManager(context);
const error = new Error();
const { body, ...incompleteArtifact } = ARTIFACT_TRUSTED_APPS_MACOS;
context.savedObjectsClient.create = jest
.fn()
.mockImplementation(async (type: string, artifact: InternalArtifactCompleteSchema) => {
artifactClient.createArtifact.mockImplementation(
async (artifact: InternalArtifactCompleteSchema) => {
if (getArtifactId(artifact) === ARTIFACT_ID_EXCEPTIONS_WINDOWS) {
throw error;
} else {
return artifact;
}
});
}
);
await expect(
manifestManager.pushArtifacts([
@ -536,45 +521,15 @@ describe('ManifestManager', () => {
new Error(`Incomplete artifact: ${ARTIFACT_ID_TRUSTED_APPS_MACOS}`),
]);
expect(context.savedObjectsClient.create).toHaveBeenCalledTimes(2);
expect(context.savedObjectsClient.create).toHaveBeenNthCalledWith(
1,
ArtifactConstants.SAVED_OBJECT_TYPE,
{ ...ARTIFACT_EXCEPTIONS_MACOS, created: expect.anything() },
{ id: ARTIFACT_ID_EXCEPTIONS_MACOS }
);
expect(artifactClient.createArtifact).toHaveBeenCalledTimes(2);
expect(artifactClient.createArtifact).toHaveBeenNthCalledWith(1, {
...ARTIFACT_EXCEPTIONS_MACOS,
});
expect(
await uncompressData(context.cache.get(getArtifactId(ARTIFACT_EXCEPTIONS_MACOS))!)
).toStrictEqual(await uncompressArtifact(ARTIFACT_EXCEPTIONS_MACOS));
expect(context.cache.get(getArtifactId(ARTIFACT_EXCEPTIONS_WINDOWS))).toBeUndefined();
});
test('Tolerates saved objects client conflict', async () => {
const context = buildManifestManagerContextMock({});
const manifestManager = new ManifestManager(context);
context.savedObjectsClient.create = jest
.fn()
.mockRejectedValue(
SavedObjectsErrorHelpers.createConflictError(
ArtifactConstants.SAVED_OBJECT_TYPE,
ARTIFACT_ID_EXCEPTIONS_MACOS
)
);
await expect(
manifestManager.pushArtifacts([ARTIFACT_EXCEPTIONS_MACOS])
).resolves.toStrictEqual([]);
expect(context.savedObjectsClient.create).toHaveBeenCalledTimes(1);
expect(context.savedObjectsClient.create).toHaveBeenNthCalledWith(
1,
ArtifactConstants.SAVED_OBJECT_TYPE,
{ ...ARTIFACT_EXCEPTIONS_MACOS, created: expect.anything() },
{ id: ARTIFACT_ID_EXCEPTIONS_MACOS }
);
expect(context.cache.get(getArtifactId(ARTIFACT_EXCEPTIONS_MACOS))).toBeUndefined();
});
});
describe('commit', () => {

View file

@ -32,7 +32,7 @@ import {
InternalArtifactCompleteSchema,
internalArtifactCompleteSchema,
} from '../../../schemas/artifacts';
import { ArtifactClient } from '../artifact_client';
import { EndpointArtifactClientInterface } from '../artifact_client';
import { ManifestClient } from '../manifest_client';
interface ArtifactsBuildResult {
@ -76,7 +76,7 @@ const iterateAllListItems = async <T>(
export interface ManifestManagerContext {
savedObjectsClient: SavedObjectsClientContract;
artifactClient: ArtifactClient;
artifactClient: EndpointArtifactClientInterface;
exceptionListClient: ExceptionListClient;
packagePolicyService: PackagePolicyServiceInterface;
logger: Logger;
@ -92,7 +92,7 @@ const manifestsEqual = (manifest1: ManifestSchema, manifest2: ManifestSchema) =>
isEqual(new Set(getArtifactIds(manifest1)), new Set(getArtifactIds(manifest2)));
export class ManifestManager {
protected artifactClient: ArtifactClient;
protected artifactClient: EndpointArtifactClientInterface;
protected exceptionListClient: ExceptionListClient;
protected packagePolicyService: PackagePolicyServiceInterface;
protected savedObjectsClient: SavedObjectsClientContract;
@ -290,10 +290,13 @@ export class ManifestManager {
);
for (const entry of manifestSo.attributes.artifacts) {
manifest.addEntry(
(await this.artifactClient.getArtifact(entry.artifactId)).attributes,
entry.policyId
);
const artifact = await this.artifactClient.getArtifact(entry.artifactId);
if (!artifact) {
throw new Error(`artifact id [${entry.artifactId}] not found!`);
}
manifest.addEntry(artifact, entry.policyId);
}
return manifest;
@ -462,7 +465,7 @@ export class ManifestManager {
});
}
public getArtifactsClient(): ArtifactClient {
public getArtifactsClient(): EndpointArtifactClientInterface {
return this.artifactClient;
}
}

View file

@ -59,7 +59,7 @@ import { registerEndpointRoutes } from './endpoint/routes/metadata';
import { registerLimitedConcurrencyRoutes } from './endpoint/routes/limited_concurrency';
import { registerResolverRoutes } from './endpoint/routes/resolver';
import { registerPolicyRoutes } from './endpoint/routes/policy';
import { ArtifactClient, EndpointArtifactClient, ManifestManager } from './endpoint/services';
import { EndpointArtifactClient, ManifestManager } from './endpoint/services';
import { EndpointAppContextService } from './endpoint/endpoint_app_context_services';
import { EndpointAppContext } from './endpoint/types';
import { registerDownloadArtifactRoute } from './endpoint/routes/artifacts';
@ -352,9 +352,9 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
const fleetServerEnabled = parseExperimentalConfigValue(this.config.enableExperimental)
.fleetServerEnabled;
const exceptionListClient = this.lists.getExceptionListClient(savedObjectsClient, 'kibana');
const artifactClient = (new EndpointArtifactClient(
const artifactClient = new EndpointArtifactClient(
plugins.fleet.createArtifactsClient('endpoint')
) as unknown) as ArtifactClient;
);
manifestManager = new ManifestManager(
{