From 9d5c1cb907d0ae177b376ac012525dece51442b0 Mon Sep 17 00:00:00 2001 From: Mark Hopkin Date: Tue, 16 May 2023 09:55:56 +0100 Subject: [PATCH] [Fleet] Update secret values (API only) (#156806) ## Summary Part of https://github.com/elastic/kibana/issues/154731 Allow secrets to be updated via the API. When a secret value is updated, the secret reference is replaced with a "raw" value we detect this on the API and create a new secret document. Once a secret reference is updated, we clean up the old secret document if it is not in use by another policy. This check is a simple lookup of the secret_references array on policies. API integration tests updated. --- .../services/validate_package_policy.test.ts | 48 ++++ .../services/validate_package_policy.ts | 17 ++ .../fleet/server/services/package_policy.ts | 79 +++++- .../fleet/server/services/secrets.test.ts | 234 +++++++++++++++++- .../plugins/fleet/server/services/secrets.ts | 220 ++++++++++++++-- .../1.0.0/data_stream/log/manifest.yml | 1 - .../test_packages/secrets/1.0.0/manifest.yml | 1 - .../test/fleet_api_integration/apis/index.js | 16 +- .../apis/policy_secrets.ts | 83 ++++++- 9 files changed, 642 insertions(+), 57 deletions(-) diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts index abf38ea7db0c5..7f5c6999bce9c 100644 --- a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts +++ b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts @@ -1030,5 +1030,53 @@ describe('Fleet - validatePackagePolicyConfig', () => { expect(res).toBeNull(); }); + it('should accept a secret ref instead of a text value for a secret field', () => { + const res = validatePackagePolicyConfig( + { + value: { isSecretRef: true, id: 'secret1' }, + }, + { + name: 'secret_variable', + type: 'text', + secret: true, + }, + 'secret_variable', + safeLoad + ); + + expect(res).toBeNull(); + }); + it('secret refs should always have an id', () => { + const res = validatePackagePolicyConfig( + { + value: { isSecretRef: true }, + }, + { + name: 'secret_variable', + type: 'text', + secret: true, + }, + 'secret_variable', + safeLoad + ); + + expect(res).toEqual(['Secret reference is invalid, id must be a string']); + }); + it('secret ref id should be a string', () => { + const res = validatePackagePolicyConfig( + { + value: { isSecretRef: true, id: 123 }, + }, + { + name: 'secret_variable', + type: 'text', + secret: true, + }, + 'secret_variable', + safeLoad + ); + + expect(res).toEqual(['Secret reference is invalid, id must be a string']); + }); }); }); diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.ts index 9e85652d75e96..15aa58d547a84 100644 --- a/x-pack/plugins/fleet/common/services/validate_package_policy.ts +++ b/x-pack/plugins/fleet/common/services/validate_package_policy.ts @@ -234,6 +234,23 @@ export const validatePackagePolicyConfig = ( } } + if (varDef.secret === true && parsedValue && parsedValue.isSecretRef === true) { + if ( + parsedValue.id === undefined || + parsedValue.id === '' || + typeof parsedValue.id !== 'string' + ) { + errors.push( + i18n.translate('xpack.fleet.packagePolicyValidation.invalidSecretReference', { + defaultMessage: 'Secret reference is invalid, id must be a string', + }) + ); + + return errors; + } + return null; + } + if (varDef.type === 'yaml') { try { parsedValue = safeLoadYaml(value); diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 5a73c694a2f51..cf253fd221d28 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -113,7 +113,11 @@ import { updateDatastreamExperimentalFeatures } from './epm/packages/update'; import type { PackagePolicyClient, PackagePolicyService } from './package_policy_service'; import { installAssetsForInputPackagePolicy } from './epm/packages/install'; import { auditLoggingService } from './audit_logging'; -import { extractAndWriteSecrets } from './secrets'; +import { + extractAndUpdateSecrets, + extractAndWriteSecrets, + deleteSecretsIfNotReferenced as deleteSecrets, +} from './secrets'; export type InputsOverride = Partial & { vars?: Array; @@ -242,15 +246,15 @@ class PackagePolicyClientImpl implements PackagePolicyClient { const { secretsStorage: secretsStorageEnabled } = appContextService.getExperimentalFeatures(); if (secretsStorageEnabled) { const secretsRes = await extractAndWriteSecrets({ - packagePolicy: enrichedPackagePolicy, + packagePolicy: { ...enrichedPackagePolicy, inputs }, packageInfo: pkgInfo, esClient, }); enrichedPackagePolicy = secretsRes.packagePolicy; - secretReferences = secretsRes.secret_references; + secretReferences = secretsRes.secretReferences; - inputs = getInputsWithStreamIds(enrichedPackagePolicy, packagePolicyId); + inputs = enrichedPackagePolicy.inputs as PackagePolicyInput[]; } inputs = await _compilePackagePolicyInputs(pkgInfo, enrichedPackagePolicy.vars || {}, inputs); @@ -644,6 +648,8 @@ class PackagePolicyClientImpl implements PackagePolicyClient { }); let enrichedPackagePolicy: UpdatePackagePolicy; + let secretReferences: PolicySecretReference[] | undefined; + let secretsToDelete: PolicySecretReference[] | undefined; try { enrichedPackagePolicy = await packagePolicyService.runExternalCallbacks( @@ -661,7 +667,6 @@ class PackagePolicyClientImpl implements PackagePolicyClient { const packagePolicy = { ...enrichedPackagePolicy, name: enrichedPackagePolicy.name.trim() }; const oldPackagePolicy = await this.get(soClient, id); - const { version, ...restOfPackagePolicy } = packagePolicy; if (packagePolicyUpdate.is_managed && !options?.force) { throw new PackagePolicyRestrictionRelatedError(`Cannot update package policy ${id}`); @@ -678,6 +683,8 @@ class PackagePolicyClientImpl implements PackagePolicyClient { await requireUniqueName(soClient, enrichedPackagePolicy, id); } + // eslint-disable-next-line prefer-const + let { version, ...restOfPackagePolicy } = packagePolicy; let inputs = getInputsWithStreamIds(restOfPackagePolicy, oldPackagePolicy.id); inputs = enforceFrozenInputs(oldPackagePolicy.inputs, inputs, options?.force); @@ -697,12 +704,31 @@ class PackagePolicyClientImpl implements PackagePolicyClient { }); validatePackagePolicyOrThrow(packagePolicy, pkgInfo); - inputs = await _compilePackagePolicyInputs(pkgInfo, packagePolicy.vars || {}, inputs); + const { secretsStorage: secretsStorageEnabled } = appContextService.getExperimentalFeatures(); + if (secretsStorageEnabled) { + const secretsRes = await extractAndUpdateSecrets({ + oldPackagePolicy, + packagePolicyUpdate: { ...restOfPackagePolicy, inputs }, + packageInfo: pkgInfo, + esClient, + }); + + restOfPackagePolicy = secretsRes.packagePolicyUpdate; + secretReferences = secretsRes.secretReferences; + secretsToDelete = secretsRes.secretsToDelete; + inputs = restOfPackagePolicy.inputs as PackagePolicyInput[]; + } + + inputs = await _compilePackagePolicyInputs(pkgInfo, restOfPackagePolicy.vars || {}, inputs); elasticsearchPrivileges = pkgInfo.elasticsearch?.privileges; } // Handle component template/mappings updates for experimental features, e.g. synthetic source - await handleExperimentalDatastreamFeatureOptIn({ soClient, esClient, packagePolicy }); + await handleExperimentalDatastreamFeatureOptIn({ + soClient, + esClient, + packagePolicy: restOfPackagePolicy, + }); await soClient.update( SAVED_OBJECT_TYPE, @@ -714,6 +740,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { : {}), inputs, ...(elasticsearchPrivileges && { elasticsearch: { privileges: elasticsearchPrivileges } }), + ...(secretReferences?.length && { secret_references: secretReferences }), revision: oldPackagePolicy.revision + 1, updated_at: new Date().toISOString(), updated_by: options?.user?.username ?? 'system', @@ -767,7 +794,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient { pkgName: newPolicy.package!.name, currentVersion: newPolicy.package!.version, }); - await Promise.all([bumpPromise, assetRemovePromise]); + const deleteSecretsPromise = secretsToDelete?.length + ? deleteSecrets({ esClient, soClient, ids: secretsToDelete.map((s) => s.id) }) + : Promise.resolve(); + + await Promise.all([bumpPromise, assetRemovePromise, deleteSecretsPromise]); sendUpdatePackagePolicyTelemetryEvent(soClient, [packagePolicyUpdate], [oldPackagePolicy]); @@ -778,8 +809,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { soClient: SavedObjectsClientContract, esClient: ElasticsearchClient, packagePolicyUpdates: Array, - options?: { user?: AuthenticatedUser; force?: boolean }, - currentVersion?: string + options?: { user?: AuthenticatedUser; force?: boolean } ): Promise<{ updatedPolicies: PackagePolicy[] | null; failedPolicies: Array<{ @@ -804,6 +834,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { } const packageInfos = await getPackageInfoForPackagePolicies(packagePolicyUpdates, soClient); + const allSecretsToDelete: PolicySecretReference[] = []; const policiesToUpdate: Array> = []; const failedPolicies: Array<{ @@ -820,8 +851,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient { throw new Error('Package policy not found'); } + let secretReferences: PolicySecretReference[] | undefined; + // id and version are not part of the saved object attributes - const { version, id: _id, ...restOfPackagePolicy } = packagePolicy; + // eslint-disable-next-line prefer-const + let { version, id: _id, ...restOfPackagePolicy } = packagePolicy; if (packagePolicyUpdate.is_managed && !options?.force) { throw new PackagePolicyRestrictionRelatedError(`Cannot update package policy ${id}`); @@ -837,7 +871,21 @@ class PackagePolicyClientImpl implements PackagePolicyClient { ); if (pkgInfo) { validatePackagePolicyOrThrow(packagePolicy, pkgInfo); - + const { secretsStorage: secretsStorageEnabled } = + appContextService.getExperimentalFeatures(); + if (secretsStorageEnabled) { + const secretsRes = await extractAndUpdateSecrets({ + oldPackagePolicy, + packagePolicyUpdate: { ...restOfPackagePolicy, inputs }, + packageInfo: pkgInfo, + esClient, + }); + + restOfPackagePolicy = secretsRes.packagePolicyUpdate; + secretReferences = secretsRes.secretReferences; + allSecretsToDelete.push(...secretsRes.secretsToDelete); + inputs = restOfPackagePolicy.inputs as PackagePolicyInput[]; + } inputs = await _compilePackagePolicyInputs(pkgInfo, packagePolicy.vars || {}, inputs); elasticsearchPrivileges = pkgInfo.elasticsearch?.privileges; } @@ -858,6 +906,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { ...(elasticsearchPrivileges && { elasticsearch: { privileges: elasticsearchPrivileges }, }), + ...(secretReferences?.length && { secret_references: secretReferences }), revision: oldPackagePolicy.revision + 1, updated_at: new Date().toISOString(), updated_by: options?.user?.username ?? 'system', @@ -901,7 +950,11 @@ class PackagePolicyClientImpl implements PackagePolicyClient { }); }); - await Promise.all([bumpPromise, removeAssetPromise]); + const deleteSecretsPromise = allSecretsToDelete.length + ? deleteSecrets({ esClient, soClient, ids: allSecretsToDelete.map((s) => s.id) }) + : Promise.resolve(); + + await Promise.all([bumpPromise, removeAssetPromise, deleteSecretsPromise]); sendUpdatePackagePolicyTelemetryEvent(soClient, packagePolicyUpdates, oldPackagePolicies); diff --git a/x-pack/plugins/fleet/server/services/secrets.test.ts b/x-pack/plugins/fleet/server/services/secrets.test.ts index 1568599ace2a2..66b13638085d3 100644 --- a/x-pack/plugins/fleet/server/services/secrets.test.ts +++ b/x-pack/plugins/fleet/server/services/secrets.test.ts @@ -14,7 +14,7 @@ import type { NewPackagePolicy, PackageInfo } from '../types'; -import { getPolicySecretPaths } from './secrets'; +import { getPolicySecretPaths, diffSecretPaths } from './secrets'; describe('getPolicySecretPaths', () => { describe('integration package with one policy template', () => { @@ -100,6 +100,25 @@ describe('getPolicySecretPaths', () => { }, ]); }); + it('policy with package level secret vars and only one set', () => { + const packagePolicy = { + vars: { + 'pkg-secret-1': { + value: 'pkg-secret-1-val', + }, + }, + inputs: [], + } as unknown as NewPackagePolicy; + + expect(getPolicySecretPaths(packagePolicy, mockIntegrationPackage)).toEqual([ + { + path: 'vars.pkg-secret-1', + value: { + value: 'pkg-secret-1-val', + }, + }, + ]); + }); it('policy with input level secret vars', () => { const packagePolicy = { inputs: [ @@ -567,3 +586,216 @@ describe('getPolicySecretPaths', () => { }); }); }); + +describe('diffSecretPaths', () => { + it('should return empty array if no secrets', () => { + expect(diffSecretPaths([], [])).toEqual({ + toCreate: [], + toDelete: [], + noChange: [], + }); + }); + it('should return empty array if single secret not changed', () => { + const paths = [ + { + path: 'somepath', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + ]; + expect(diffSecretPaths(paths, paths)).toEqual({ + toCreate: [], + toDelete: [], + noChange: paths, + }); + }); + it('should return empty array if multiple secrets not changed', () => { + const paths = [ + { + path: 'somepath', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + { + path: 'somepath2', + value: { + value: { + isSecretRef: true, + id: 'secret-2', + }, + }, + }, + { + path: 'somepath3', + value: { + value: { + isSecretRef: true, + id: 'secret-3', + }, + }, + }, + ]; + + expect(diffSecretPaths(paths, paths.slice().reverse())).toEqual({ + toCreate: [], + toDelete: [], + noChange: paths, + }); + }); + it('single secret modified', () => { + const paths1 = [ + { + path: 'somepath1', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + { + path: 'somepath2', + value: { + value: { isSecretRef: true, id: 'secret-2' }, + }, + }, + ]; + + const paths2 = [ + paths1[0], + { + path: 'somepath2', + value: { value: 'newvalue' }, + }, + ]; + + expect(diffSecretPaths(paths1, paths2)).toEqual({ + toCreate: [ + { + path: 'somepath2', + value: { value: 'newvalue' }, + }, + ], + toDelete: [ + { + path: 'somepath2', + value: { + value: { + isSecretRef: true, + id: 'secret-2', + }, + }, + }, + ], + noChange: [paths1[0]], + }); + }); + it('double secret modified', () => { + const paths1 = [ + { + path: 'somepath1', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + { + path: 'somepath2', + value: { + value: { + isSecretRef: true, + id: 'secret-2', + }, + }, + }, + ]; + + const paths2 = [ + { + path: 'somepath1', + value: { value: 'newvalue1' }, + }, + { + path: 'somepath2', + value: { value: 'newvalue2' }, + }, + ]; + + expect(diffSecretPaths(paths1, paths2)).toEqual({ + toCreate: [ + { + path: 'somepath1', + value: { value: 'newvalue1' }, + }, + { + path: 'somepath2', + value: { value: 'newvalue2' }, + }, + ], + toDelete: [ + { + path: 'somepath1', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + { + path: 'somepath2', + value: { + value: { + isSecretRef: true, + id: 'secret-2', + }, + }, + }, + ], + noChange: [], + }); + }); + + it('single secret added', () => { + const paths1 = [ + { + path: 'somepath1', + value: { + value: { + isSecretRef: true, + id: 'secret-1', + }, + }, + }, + ]; + + const paths2 = [ + paths1[0], + { + path: 'somepath2', + value: { value: 'newvalue' }, + }, + ]; + + expect(diffSecretPaths(paths1, paths2)).toEqual({ + toCreate: [ + { + path: 'somepath2', + value: { value: 'newvalue' }, + }, + ], + toDelete: [], + noChange: [paths1[0]], + }); + }); +}); diff --git a/x-pack/plugins/fleet/server/services/secrets.ts b/x-pack/plugins/fleet/server/services/secrets.ts index 240773655f0f1..bd70e8435b02a 100644 --- a/x-pack/plugins/fleet/server/services/secrets.ts +++ b/x-pack/plugins/fleet/server/services/secrets.ts @@ -5,8 +5,8 @@ * 2.0. */ -import type { ElasticsearchClient } from '@kbn/core/server'; -import type { BulkResponse, DeleteResponse } from '@elastic/elasticsearch/lib/api/types'; +import type { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server'; +import type { BulkResponse } from '@elastic/elasticsearch/lib/api/types'; import { keyBy, partition } from 'lodash'; import { set } from '@kbn/safer-lodash-set'; @@ -17,7 +17,9 @@ import type { NewPackagePolicy, PackagePolicyConfigRecordEntry, RegistryStream, + UpdatePackagePolicy, } from '../../common'; +import { SO_SEARCH_LIMIT } from '../../common'; import { doesPackageHaveIntegrations, @@ -40,6 +42,7 @@ import { SECRETS_INDEX } from '../constants'; import { auditLoggingService } from './audit_logging'; import { appContextService } from './app_context'; +import { packagePolicyService } from './package_policy'; interface SecretPath { path: string; @@ -102,47 +105,138 @@ export async function createSecrets(opts: { } } -export async function deleteSecret(opts: { +export async function deleteSecretsIfNotReferenced(opts: { esClient: ElasticsearchClient; - id: string; -}): Promise { - const { esClient, id } = opts; - let res: DeleteResponse; + soClient: SavedObjectsClientContract; + ids: string[]; +}): Promise { + const { esClient, soClient, ids } = opts; + const logger = appContextService.getLogger(); + const packagePoliciesUsingSecrets = await findPackagePoliciesUsingSecrets({ + soClient, + ids, + }); + + if (packagePoliciesUsingSecrets.length) { + packagePoliciesUsingSecrets.forEach(({ id, policyIds }) => { + logger.debug( + `Not deleting secret with id ${id} is still in use by package policies: ${policyIds.join( + ', ' + )}` + ); + }); + } + + const secretsToDelete = ids.filter((id) => { + return !packagePoliciesUsingSecrets.some((packagePolicy) => packagePolicy.id === id); + }); + + if (!secretsToDelete.length) { + return; + } try { - res = await esClient.delete({ - index: getSecretsIndex(), - id, + await _deleteSecrets({ + esClient, + ids: secretsToDelete, }); + } catch (e) { + logger.warn(`Error cleaning up secrets ${ids.join(', ')}: ${e}`); + } +} + +export async function findPackagePoliciesUsingSecrets(opts: { + soClient: SavedObjectsClientContract; + ids: string[]; +}): Promise> { + const { soClient, ids } = opts; + const packagePolicies = await packagePolicyService.list(soClient, { + kuery: `ingest-package-policies.secret_references.id: (${ids.join(' or ')})`, + perPage: SO_SEARCH_LIMIT, + page: 1, + }); + + if (!packagePolicies.total) { + return []; + } + + // create a map of secret_references.id to package policy id + const packagePoliciesBySecretId = packagePolicies.items.reduce((acc, packagePolicy) => { + packagePolicy?.secret_references?.forEach((secretReference) => { + if (!acc[secretReference.id]) { + acc[secretReference.id] = []; + } + acc[secretReference.id].push(packagePolicy.id); + }); + return acc; + }, {} as Record); + + const res = []; - auditLoggingService.writeCustomAuditLog({ - message: `secret deleted: ${id}`, - event: { - action: 'secret_delete', - category: ['database'], - type: ['access'], - outcome: 'success', - }, + for (const id of ids) { + if (packagePoliciesBySecretId[id]) { + res.push({ + id, + policyIds: packagePoliciesBySecretId[id], + }); + } + } + + return res; +} + +export async function _deleteSecrets(opts: { + esClient: ElasticsearchClient; + ids: string[]; +}): Promise { + const { esClient, ids } = opts; + const logger = appContextService.getLogger(); + const body = ids.flatMap((id) => [ + { + delete: { _index: getSecretsIndex(), _id: id }, + }, + ]); + + let res: BulkResponse; + + try { + res = await esClient.bulk({ + body, }); + + const [errorItems, successItems] = partition(res.items, (a) => a.delete?.error); + + successItems.forEach((item) => { + auditLoggingService.writeCustomAuditLog({ + message: `secret deleted: ${item.delete!._id}`, + event: { + action: 'secret_delete', + category: ['database'], + type: ['access'], + outcome: 'success', + }, + }); + }); + + if (errorItems.length) { + throw new Error(JSON.stringify(errorItems)); + } } catch (e) { - const logger = appContextService.getLogger(); - const msg = `Error deleting secret '${id}' from ${getSecretsIndex()} index: ${e}`; + const msg = `Error deleting secrets from ${getSecretsIndex()} index: ${e}`; logger.error(msg); throw new FleetError(msg); } - - return res.result; } export async function extractAndWriteSecrets(opts: { packagePolicy: NewPackagePolicy; packageInfo: PackageInfo; esClient: ElasticsearchClient; -}): Promise<{ packagePolicy: NewPackagePolicy; secret_references: PolicySecretReference[] }> { +}): Promise<{ packagePolicy: NewPackagePolicy; secretReferences: PolicySecretReference[] }> { const { packagePolicy, packageInfo, esClient } = opts; const secretPaths = getPolicySecretPaths(packagePolicy, packageInfo); if (!secretPaths.length) { - return { packagePolicy, secret_references: [] }; + return { packagePolicy, secretReferences: [] }; } const secrets = await createSecrets({ @@ -157,7 +251,49 @@ export async function extractAndWriteSecrets(opts: { return { packagePolicy: policyWithSecretRefs, - secret_references: secrets.map(({ id }) => ({ id })), + secretReferences: secrets.map(({ id }) => ({ id })), + }; +} + +export async function extractAndUpdateSecrets(opts: { + oldPackagePolicy: PackagePolicy; + packagePolicyUpdate: UpdatePackagePolicy; + packageInfo: PackageInfo; + esClient: ElasticsearchClient; +}): Promise<{ + packagePolicyUpdate: UpdatePackagePolicy; + secretReferences: PolicySecretReference[]; + secretsToDelete: PolicySecretReference[]; +}> { + const { oldPackagePolicy, packagePolicyUpdate, packageInfo, esClient } = opts; + const oldSecretPaths = getPolicySecretPaths(oldPackagePolicy, packageInfo); + const updatedSecretPaths = getPolicySecretPaths(packagePolicyUpdate, packageInfo); + + if (!oldSecretPaths.length && !updatedSecretPaths.length) { + return { packagePolicyUpdate, secretReferences: [], secretsToDelete: [] }; + } + + const { toCreate, toDelete, noChange } = diffSecretPaths(oldSecretPaths, updatedSecretPaths); + + const createdSecrets = await createSecrets({ + esClient, + values: toCreate.map((secretPath) => secretPath.value.value), + }); + + const policyWithSecretRefs = JSON.parse(JSON.stringify(packagePolicyUpdate)); + toCreate.forEach((secretPath, i) => { + set(policyWithSecretRefs, secretPath.path + '.value', toVarSecretRef(createdSecrets[i].id)); + }); + + const secretReferences = [ + ...noChange.map((secretPath) => ({ id: secretPath.value.value.id })), + ...createdSecrets.map(({ id }) => ({ id })), + ]; + + return { + packagePolicyUpdate: policyWithSecretRefs, + secretReferences, + secretsToDelete: toDelete.map((secretPath) => ({ id: secretPath.value.value.id })), }; } @@ -179,10 +315,42 @@ export function toCompiledSecretRef(id: string) { return `$co.elastic.secret{${id}}`; } +export function diffSecretPaths( + oldPaths: SecretPath[], + newPaths: SecretPath[] +): { toCreate: SecretPath[]; toDelete: SecretPath[]; noChange: SecretPath[] } { + const toCreate: SecretPath[] = []; + const toDelete: SecretPath[] = []; + const noChange: SecretPath[] = []; + const newPathsByPath = keyBy(newPaths, 'path'); + + for (const oldPath of oldPaths) { + if (!newPathsByPath[oldPath.path]) { + toDelete.push(oldPath); + } + + const newPath = newPathsByPath[oldPath.path]; + if (newPath && newPath.value.value) { + const newValue = newPath.value?.value; + if (!newValue?.isSecretRef) { + toCreate.push(newPath); + toDelete.push(oldPath); + } else { + noChange.push(newPath); + } + delete newPathsByPath[oldPath.path]; + } + } + + const remainingNewPaths = Object.values(newPathsByPath); + + return { toCreate: [...toCreate, ...remainingNewPaths], toDelete, noChange }; +} + // Given a package policy and a package, // returns an array of lodash style paths to all secrets and their current values export function getPolicySecretPaths( - packagePolicy: PackagePolicy | NewPackagePolicy, + packagePolicy: PackagePolicy | NewPackagePolicy | UpdatePackagePolicy, packageInfo: PackageInfo ): SecretPath[] { const packageLevelVarPaths = _getPackageLevelSecretPaths(packagePolicy, packageInfo); diff --git a/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/data_stream/log/manifest.yml b/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/data_stream/log/manifest.yml index 505ca53d69613..8ecffc0e4e7d4 100644 --- a/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/data_stream/log/manifest.yml +++ b/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/data_stream/log/manifest.yml @@ -8,6 +8,5 @@ streams: type: text title: Stream Var Secret multi: false - required: true show_user: true secret: true diff --git a/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/manifest.yml b/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/manifest.yml index fc39e04d8e0dc..50cc075161e7d 100644 --- a/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/manifest.yml +++ b/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/secrets/1.0.0/manifest.yml @@ -47,6 +47,5 @@ policy_templates: type: text title: Input Var Secret multi: false - required: true show_user: true secret: true \ No newline at end of file diff --git a/x-pack/test/fleet_api_integration/apis/index.js b/x-pack/test/fleet_api_integration/apis/index.js index 5b2b39ef2574f..a7d0797da731b 100644 --- a/x-pack/test/fleet_api_integration/apis/index.js +++ b/x-pack/test/fleet_api_integration/apis/index.js @@ -17,20 +17,8 @@ export default function ({ loadTestFile, getService }) { // Fleet setup loadTestFile(require.resolve('./fleet_setup')); // ~ 6s - // Enrollment API keys - loadTestFile(require.resolve('./enrollment_api_keys/crud')); - - // Package policies - loadTestFile(require.resolve('./policy_secrets')); - loadTestFile(require.resolve('./package_policy/create')); - loadTestFile(require.resolve('./package_policy/update')); - loadTestFile(require.resolve('./package_policy/get')); - loadTestFile(require.resolve('./package_policy/delete')); - loadTestFile(require.resolve('./package_policy/upgrade')); - loadTestFile(require.resolve('./package_policy/input_package_create_upgrade')); - - // Agent policies - loadTestFile(require.resolve('./agent_policy')); + loadTestFile(require.resolve('./policy_secrets')); // ~40s + loadTestFile(require.resolve('./enrollment_api_keys/crud')); // ~ 20s // Data Streams diff --git a/x-pack/test/fleet_api_integration/apis/policy_secrets.ts b/x-pack/test/fleet_api_integration/apis/policy_secrets.ts index 0164b0dadeef0..148ec919da964 100644 --- a/x-pack/test/fleet_api_integration/apis/policy_secrets.ts +++ b/x-pack/test/fleet_api_integration/apis/policy_secrets.ts @@ -21,6 +21,22 @@ const arrayIdsEqual = (a: Array<{ id: string }>, b: Array<{ id: string }>) => { return a.every(({ id }) => b.find(({ id: bid }) => bid === id)); }; +function createdPolicyToUpdatePolicy(policy: any) { + const updatedPolicy = JSON.parse(JSON.stringify(policy)); + delete updatedPolicy.id; + delete updatedPolicy.revision; + delete updatedPolicy.secret_references; + delete updatedPolicy.created_at; + delete updatedPolicy.created_by; + delete updatedPolicy.updated_at; + delete updatedPolicy.updated_by; + delete updatedPolicy.inputs[0].compiled_input; + delete updatedPolicy.inputs[0].streams[0].compiled_stream; + delete updatedPolicy.package.title; + + return updatedPolicy; +} + export default function (providerContext: FtrProviderContext) { describe('fleet policy secrets', () => { const { getService } = providerContext; @@ -85,8 +101,10 @@ export default function (providerContext: FtrProviderContext) { }); return res.hits.hits[0]._source as any as { data: FullAgentPolicy }; }; + let createdPackagePolicy: any; let createdPackagePolicyId: string; let packageVarId: string; + let updatedPackageVarId: string; let inputVarId: string; let streamVarId: string; let expectedCompiledStream: any; @@ -173,7 +191,7 @@ export default function (providerContext: FtrProviderContext) { }) .expect(200); - const createdPackagePolicy = createResBody.item; + createdPackagePolicy = createResBody.item; createdPackagePolicyId = createdPackagePolicy.id; packageVarId = createdPackagePolicy.vars.package_var_secret.value.id; expect(packageVarId).to.be.an('string'); @@ -267,5 +285,68 @@ export default function (providerContext: FtrProviderContext) { expectCompiledPolicyVars(agentPolicy); }); + + it('should allow secret values to be updated (single policy update API)', async () => { + const updatedPolicy = createdPolicyToUpdatePolicy(createdPackagePolicy); + updatedPolicy.vars.package_var_secret.value = 'new_package_secret_val'; + const updateRes = await supertest + .put(`/api/fleet/package_policies/${createdPackagePolicyId}`) + .set('kbn-xsrf', 'xxxx') + .send(updatedPolicy) + .expect(200); + + const updatedPackagePolicy = updateRes.body.item; + + updatedPackageVarId = updatedPackagePolicy.vars.package_var_secret.value.id; + expect(updatedPackageVarId).to.be.an('string'); + expect( + arrayIdsEqual(updatedPackagePolicy.secret_references, [ + { id: updatedPackageVarId }, + { id: streamVarId }, + { id: inputVarId }, + ]) + ).to.eql(true); + expect(updatedPackagePolicy.inputs[0].streams[0].compiled_stream).to.eql({ + 'config.version': 2, + package_var_secret: secretVar(updatedPackageVarId), + input_var_secret: secretVar(inputVarId), + stream_var_secret: secretVar(streamVarId), + }); + expect(updatedPackagePolicy.inputs[0].compiled_input).to.eql({ + package_var_secret: secretVar(updatedPackageVarId), + input_var_secret: secretVar(inputVarId), + }); + expect(updatedPackagePolicy.vars.package_var_secret.value.isSecretRef).to.eql(true); + expect(updatedPackagePolicy.vars.package_var_secret.value.id).eql(updatedPackageVarId); + expect(updatedPackagePolicy.inputs[0].vars.input_var_secret.value.isSecretRef).to.eql(true); + expect(updatedPackagePolicy.inputs[0].vars.input_var_secret.value.id).eql(inputVarId); + expect( + updatedPackagePolicy.inputs[0].streams[0].vars.stream_var_secret.value.isSecretRef + ).to.eql(true); + expect(updatedPackagePolicy.inputs[0].streams[0].vars.stream_var_secret.value.id).eql( + streamVarId + ); + }); + + it('should have correctly deleted the secrets', async () => { + const searchRes = await es.search({ + index: '.fleet-test-secrets', + body: { + query: { + match_all: {}, + }, + }, + }); + + expect(searchRes.hits.hits.length).to.eql(3); // should have created 1 and deleted 1 doc + + const secretValuesById = searchRes.hits.hits.reduce((acc: any, secret: any) => { + acc[secret._id] = secret._source.value; + return acc; + }, {}); + expect(secretValuesById[updatedPackageVarId]).to.eql('new_package_secret_val'); + expect(secretValuesById[inputVarId]).to.eql('input_secret_val'); + expect(secretValuesById[streamVarId]).to.eql('stream_secret_val'); + }); }); }