From beda4b970565fa0b7563e0eaae6936faeef46018 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Tue, 27 Jul 2021 14:37:24 -0400 Subject: [PATCH] Fix ESO migration decryption for converted saved object types A bug in the algorithm neglected to differentiate when a converted object may have had its ID changed (this happens when an object exists in a non-default space and then it is converted). This commit fixes the bug and adds several more unit tests and integration tests to exercise different permutations of migrations. --- .../server/create_migration.test.ts | 124 +++++++----- .../server/create_migration.ts | 33 ++- .../encrypted_saved_objects_service.test.ts | 190 ++++++++++++++---- .../crypto/encrypted_saved_objects_service.ts | 26 ++- .../api_consumer_plugin/server/index.ts | 17 +- .../encrypted_saved_objects/data.json | 66 +++++- .../tests/encrypted_saved_objects_api.ts | 109 ++++++++-- 7 files changed, 452 insertions(+), 113 deletions(-) diff --git a/x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts b/x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts index 548340fbb6463..7301624f65094 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts @@ -170,10 +170,14 @@ describe('createMigration()', () => { describe('uses the object `namespaces` field to populate the descriptor when the migration context indicates this type is being converted', () => { const doTest = ({ objectNamespace, + objectNamespaces, decryptDescriptorNamespace, + encryptDescriptorNamespace, }: { objectNamespace: string | undefined; + objectNamespaces: string[] | undefined; decryptDescriptorNamespace: string | undefined; + encryptDescriptorNamespace: string | undefined; }) => { const instantiateServiceWithLegacyType = jest.fn(() => encryptedSavedObjectsServiceMock.create() @@ -189,56 +193,82 @@ describe('createMigration()', () => { }, (doc) => doc ); - - const attributes = { - firstAttr: 'first_attr', - }; - - encryptionSavedObjectService.decryptAttributesSync.mockReturnValueOnce(attributes); - encryptionSavedObjectService.encryptAttributesSync.mockReturnValueOnce(attributes); - - noopMigration( - { - id: '123', - type: 'known-type-1', - namespaces: objectNamespace ? [objectNamespace] : [], + const attributes = { firstAttr: 'first_attr' }; + + ['7.99.99', '8.0.0'].forEach((migrationVersion, i) => { + encryptionSavedObjectService.decryptAttributesSync.mockReturnValueOnce(attributes); + encryptionSavedObjectService.encryptAttributesSync.mockReturnValueOnce(attributes); + noopMigration( + { + id: '123', + originId: 'some-origin-id', + type: 'known-type-1', + namespace: objectNamespace, + namespaces: objectNamespaces, + attributes, + }, + migrationMocks.createContext({ + migrationVersion, // test works with any version <= 8.0.0 + convertToMultiNamespaceTypeVersion: '8.0.0', + }) + ); + expect(encryptionSavedObjectService.decryptAttributesSync).toHaveBeenNthCalledWith( + i + 1, + { id: '123', type: 'known-type-1', namespace: decryptDescriptorNamespace }, attributes, - }, - migrationMocks.createContext({ - migrationVersion: '8.0.0', - convertToMultiNamespaceTypeVersion: '8.0.0', - }) - ); - - expect(encryptionSavedObjectService.decryptAttributesSync).toHaveBeenCalledWith( - { - id: '123', - type: 'known-type-1', - namespace: decryptDescriptorNamespace, - }, - attributes, - { convertToMultiNamespaceType: true } - ); - - expect(encryptionSavedObjectService.encryptAttributesSync).toHaveBeenCalledWith( - { - id: '123', - type: 'known-type-1', - }, - attributes - ); + { convertToMultiNamespaceType: true, originId: 'some-origin-id' } + ); + expect(encryptionSavedObjectService.encryptAttributesSync).toHaveBeenNthCalledWith( + i + 1, + { id: '123', type: 'known-type-1', namespace: encryptDescriptorNamespace }, + attributes + ); + }); }; - it('when namespaces is an empty array', () => { - doTest({ objectNamespace: undefined, decryptDescriptorNamespace: undefined }); - }); - - it('when the first namespace element is "default"', () => { - doTest({ objectNamespace: 'default', decryptDescriptorNamespace: undefined }); - }); - - it('when the first namespace element is another string', () => { - doTest({ objectNamespace: 'foo', decryptDescriptorNamespace: 'foo' }); + [undefined, 'foo'].forEach((objectNamespace) => { + // In the test cases below, we test what will happen if an object has both `namespace` and `namespaces` fields. This will not happen + // in normal operation, as when Kibana converts an object from a single- to a multi-namespace type, it removes the `namespace` field + // and adds the `namespaces` field. The tests below are for completeness. + const namespaceDescription = objectNamespace ? 'defined' : undefined; + + it(`when namespaces is undefined and namespace is ${namespaceDescription}`, () => { + doTest({ + objectNamespace, + objectNamespaces: undefined, + decryptDescriptorNamespace: objectNamespace, + encryptDescriptorNamespace: objectNamespace, + }); + }); + + it(`when namespaces is an empty array and namespace is ${namespaceDescription}`, () => { + // The `namespaces` field should never be an empty array, but we test for it anyway. In this case, we fall back to attempting to + // decrypt with the `namespace` field; if that doesn't work, the ESO service will try again without it. + doTest({ + objectNamespace, + objectNamespaces: [], + decryptDescriptorNamespace: objectNamespace, + encryptDescriptorNamespace: objectNamespace, + }); + }); + + it(`when namespaces is a non-empty array (default space) and namespace is ${namespaceDescription}`, () => { + doTest({ + objectNamespace, + objectNamespaces: ['default', 'additional-spaces-are-ignored'], + decryptDescriptorNamespace: undefined, + encryptDescriptorNamespace: objectNamespace, + }); + }); + + it(`when namespaces is a non-empty array (custom space) and namespace is ${namespaceDescription}`, () => { + doTest({ + objectNamespace, + objectNamespaces: ['custom', 'additional-spaces-are-ignored'], + decryptDescriptorNamespace: 'custom', + encryptDescriptorNamespace: objectNamespace, + }); + }); }); }); }); diff --git a/x-pack/plugins/encrypted_saved_objects/server/create_migration.ts b/x-pack/plugins/encrypted_saved_objects/server/create_migration.ts index beace2b17fe08..e4e79d8e84604 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/create_migration.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/create_migration.ts @@ -5,6 +5,8 @@ * 2.0. */ +import Semver from 'semver'; + import type { SavedObjectMigrationContext, SavedObjectMigrationFn, @@ -65,19 +67,37 @@ export const getCreateMigration = ( return encryptedDoc; } + // After it is converted, the object's old ID is stored in the `originId` field. In addition, objects that are imported a certain way + // may have this field set, but it would not be necessary to use this to decrypt saved object attributes. + const originId = encryptedDoc.originId; + + // If an object is slated to be converted, it should be decrypted flexibly: + // * If this is an index migration: + // a. If there is one or more pending migration _before_ the conversion, the object will be decrypted and re-encrypted with its + // namespace in the descriptor. Then, after the conversion, the object will be decrypted with its namespace and old ID in the + // descriptor and re-encrypted with its new ID (without a namespace) in the descriptor. + // b. If there are no pending migrations before the conversion, then after the conversion the object will be decrypted with its + // namespace and old ID in the descriptor and re-encrypted with its new ID (without a namespace) in the descriptor. + // * If this is *not* an index migration, then it is a single document migration. In that case, the object will be decrypted and + // re-encrypted without a namespace in the descriptor. + // To account for these different scenarios, when this field is set, the ESO service will attempt several different permutations of + // the descriptor when decrypting the object. + const convertToMultiNamespaceType = + !!context.convertToMultiNamespaceTypeVersion && + Semver.lte(context.migrationVersion, context.convertToMultiNamespaceTypeVersion); + // If an object has been converted right before this migration function is called, it will no longer have a `namespace` field, but it // will have a `namespaces` field; in that case, the first/only element in that array should be used as the namespace in the descriptor // during decryption. - const convertToMultiNamespaceType = - context.convertToMultiNamespaceTypeVersion === context.migrationVersion; - const decryptDescriptorNamespace = convertToMultiNamespaceType - ? normalizeNamespace(encryptedDoc.namespaces?.[0]) // `namespaces` contains string values, but we need to normalize this to the namespace ID representation - : encryptedDoc.namespace; + const decryptDescriptorNamespace = + convertToMultiNamespaceType && encryptedDoc.namespaces?.length + ? normalizeNamespace(encryptedDoc.namespaces[0]) // `namespaces` contains string values, but we need to normalize this to the namespace ID representation + : encryptedDoc.namespace; const { id, type } = encryptedDoc; // These descriptors might have a `namespace` that is undefined. That is expected for multi-namespace and namespace-agnostic types. const decryptDescriptor = { id, type, namespace: decryptDescriptorNamespace }; - const encryptDescriptor = { id, type, namespace: encryptedDoc.namespace }; + const encryptDescriptor = { id, type, namespace: encryptedDoc.namespace }; // It would be preferable to rely on getDescriptorNamespace() here, but that requires the SO type registry which can only be retrieved from a promise, and this is not an async function // decrypt the attributes using the input type definition // then migrate the document @@ -87,6 +107,7 @@ export const getCreateMigration = ( mapAttributes(encryptedDoc, (inputAttributes) => inputService.decryptAttributesSync(decryptDescriptor, inputAttributes, { convertToMultiNamespaceType, + originId, }) ), context diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts index 5ac6467e8d78b..5bd1d6a5b355e 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts @@ -820,53 +820,163 @@ describe('#decryptAttributes', () => { ); }); - it('retries decryption without namespace if incorrect namespace is provided and convertToMultiNamespaceType option is enabled', async () => { - const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; + describe('with convertToMultiNamespaceType option', () => { + it('retries decryption without namespace', async () => { + const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; - service.registerType({ - type: 'known-type-1', - attributesToEncrypt: new Set(['attrThree']), + service.registerType({ + type: 'known-type-1', + attributesToEncrypt: new Set(['attrThree']), + }); + + const encryptedAttributes = service.encryptAttributesSync( + { type: 'known-type-1', id: 'object-id' }, // namespace was not included in descriptor during encryption + attributes + ); + expect(encryptedAttributes).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: expect.not.stringMatching(/^three$/), + }); + + const mockUser = mockAuthenticatedUser(); + await expect( + service.decryptAttributes( + { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, + encryptedAttributes, + { user: mockUser, convertToMultiNamespaceType: true } + ) + ).resolves.toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decrypt).toHaveBeenCalledTimes(2); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 1, // first attempted to decrypt with the namespace in the descriptor (fail) + expect.anything(), + `["object-ns","known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 2, // then attempted to decrypt without the namespace in the descriptor (success) + expect.anything(), + `["known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledTimes(1); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledWith( + ['attrThree'], + { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, + mockUser + ); }); - const encryptedAttributes = service.encryptAttributesSync( - { type: 'known-type-1', id: 'object-id' }, // namespace was not included in descriptor during encryption - attributes - ); - expect(encryptedAttributes).toEqual({ - attrOne: 'one', - attrTwo: 'two', - attrThree: expect.not.stringMatching(/^three$/), + it('retries decryption without originId (old object ID)', async () => { + const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; + + service.registerType({ + type: 'known-type-1', + attributesToEncrypt: new Set(['attrThree']), + }); + + const encryptedAttributes = service.encryptAttributesSync( + { type: 'known-type-1', id: 'object-id' }, // old object ID was not used in descriptor during encryption + attributes + ); + expect(encryptedAttributes).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: expect.not.stringMatching(/^three$/), + }); + + const mockUser = mockAuthenticatedUser(); + await expect( + service.decryptAttributes( + { type: 'known-type-1', id: 'object-id', namespace: undefined }, + encryptedAttributes, + { user: mockUser, convertToMultiNamespaceType: true, originId: 'old-object-id' } + ) + ).resolves.toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decrypt).toHaveBeenCalledTimes(2); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 1, // first attempted to decrypt with the old object ID in the descriptor (fail) + expect.anything(), + `["known-type-1","old-object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 2, // then attempted to decrypt with the current object ID in the descriptor (success) + expect.anything(), + `["known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledTimes(1); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledWith( + ['attrThree'], + { type: 'known-type-1', id: 'object-id', namespace: undefined }, + mockUser + ); }); - const mockUser = mockAuthenticatedUser(); - await expect( - service.decryptAttributes( + it('retries decryption without namespace *and* without originId (old object ID)', async () => { + const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; + + service.registerType({ + type: 'known-type-1', + attributesToEncrypt: new Set(['attrThree']), + }); + + const encryptedAttributes = service.encryptAttributesSync( + { type: 'known-type-1', id: 'object-id' }, // namespace and old object ID were not used in descriptor during encryption + attributes + ); + expect(encryptedAttributes).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: expect.not.stringMatching(/^three$/), + }); + + const mockUser = mockAuthenticatedUser(); + await expect( + service.decryptAttributes( + { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, + encryptedAttributes, + { user: mockUser, convertToMultiNamespaceType: true, originId: 'old-object-id' } + ) + ).resolves.toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decrypt).toHaveBeenCalledTimes(4); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 1, // first attempted to decrypt with the old object ID and namespace in the descriptor (fail) + expect.anything(), + `["object-ns","known-type-1","old-object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 2, // then attempted to decrypt with the old object ID and no namespace in the descriptor (fail) + expect.anything(), + `["known-type-1","old-object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 3, // first attempted to decrypt with the current object ID and namespace in the descriptor (fail) + expect.anything(), + `["object-ns","known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( + 4, // then attempted to decrypt with the current object ID and no namespace in the descriptor (success) + expect.anything(), + `["known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` + ); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledTimes(1); + expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledWith( + ['attrThree'], { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, - encryptedAttributes, - { user: mockUser, convertToMultiNamespaceType: true } - ) - ).resolves.toEqual({ - attrOne: 'one', - attrTwo: 'two', - attrThree: 'three', + mockUser + ); }); - expect(mockNodeCrypto.decrypt).toHaveBeenCalledTimes(2); - expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( - 1, // first attempted to decrypt with the namespace in the descriptor (fail) - expect.anything(), - `["object-ns","known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` - ); - expect(mockNodeCrypto.decrypt).toHaveBeenNthCalledWith( - 2, // then attempted to decrypt without the namespace in the descriptor (success) - expect.anything(), - `["known-type-1","object-id",{"attrOne":"one","attrTwo":"two"}]` - ); - expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledTimes(1); - expect(mockAuditLogger.decryptAttributesSuccess).toHaveBeenCalledWith( - ['attrThree'], - { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, - mockUser - ); }); it('decrypts even if no attributes are included into AAD', async () => { diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts index 652a2c8b6870e..1692e43a6fe19 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts @@ -71,6 +71,11 @@ interface DecryptParameters extends CommonParameters { * decrypted during object migration, the object was never encrypted with its namespace in the descriptor portion of the AAD. */ convertToMultiNamespaceType?: boolean; + /** + * If the originId (old object ID) is present and the object is being converted from a single-namespace type to a multi-namespace type, + * we will attempt to decrypt with both the old object ID and the current object ID. + */ + originId?: string; } interface EncryptedSavedObjectsServiceOptions { @@ -484,11 +489,22 @@ export class EncryptedSavedObjectsService { ); } if (!encryptionAADs.length) { - encryptionAADs.push(this.getAAD(typeDefinition, descriptor, attributes)); - if (params?.convertToMultiNamespaceType && descriptor.namespace) { - // This is happening during a migration; create an alternate AAD for decrypting the object attributes by stripping out the namespace from the descriptor. - const { namespace, ...alternateDescriptor } = descriptor; - encryptionAADs.push(this.getAAD(typeDefinition, alternateDescriptor, attributes)); + if (params?.convertToMultiNamespaceType) { + // The object is either pending conversion to a multi-namespace type, or it was just converted. We may need to attempt to decrypt + // it with several different descriptors depending upon how the migrations are structured, and whether this is a full index + // migration or a single document migration. Note that the originId is set either when the document is converted _or_ when it is + // imported with "createNewCopies: false", so we have to try with and without it. + const ids = params.originId ? [params.originId, descriptor.id] : [descriptor.id]; + for (const id of ids) { + const decryptDescriptor = { ...descriptor, id }; + encryptionAADs.push(this.getAAD(typeDefinition, decryptDescriptor, attributes)); + if (descriptor.namespace) { + const { namespace, ...alternateDescriptor } = decryptDescriptor; + encryptionAADs.push(this.getAAD(typeDefinition, alternateDescriptor, attributes)); + } + } + } else { + encryptionAADs.push(this.getAAD(typeDefinition, descriptor, attributes)); } } try { diff --git a/x-pack/test/encrypted_saved_objects_api_integration/fixtures/api_consumer_plugin/server/index.ts b/x-pack/test/encrypted_saved_objects_api_integration/fixtures/api_consumer_plugin/server/index.ts index 96a0a3b2fa427..b68ddf46a3d39 100644 --- a/x-pack/test/encrypted_saved_objects_api_integration/fixtures/api_consumer_plugin/server/index.ts +++ b/x-pack/test/encrypted_saved_objects_api_integration/fixtures/api_consumer_plugin/server/index.ts @@ -134,7 +134,8 @@ function defineTypeWithMigration(core: CoreSetup, deps: PluginsSet core.savedObjects.registerType({ name: SAVED_OBJECT_WITH_MIGRATION_TYPE, hidden: false, - namespaceType: 'single', + namespaceType: 'multiple-isolated', // in data.json, we simulate that existing objects were created with `namespaceType: 'single'` + convertToMultiNamespaceTypeVersion: '8.0.0', // in this version we convert from a single-namespace type to a "share-capable" multi-namespace isolated type mappings: { properties: { nonEncryptedAttribute: { @@ -195,6 +196,20 @@ function defineTypeWithMigration(core: CoreSetup, deps: PluginsSet }, typePriorTo790 ), + + // NOTE FOR MAINTAINERS: do not add any more migrations before 8.0.0 unless you regenerate the test data for two of the objects in + // data.json: '362828f0-eef2-11eb-9073-11359682300a' and '36448a90-eef2-11eb-9073-11359682300a. These are used in the test cases 'for + // a saved object that does not need to be migrated before it is converted'. + + // This empty migration is necessary to ensure that the saved object is decrypted with its old descriptor/ and re-encrypted with its + // new descriptor, if necessary. This is included because the saved object is being converted to `namespaceType: 'multiple-isolated'` + // in 8.0.0 (see the `convertToMultiNamespaceTypeVersion` field in the saved object type registration process). + '8.0.0': deps.encryptedSavedObjects.createMigration( + function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc { + return true; + }, + (doc) => doc // no-op + ), }, }); } diff --git a/x-pack/test/encrypted_saved_objects_api_integration/fixtures/es_archiver/encrypted_saved_objects/data.json b/x-pack/test/encrypted_saved_objects_api_integration/fixtures/es_archiver/encrypted_saved_objects/data.json index 88ec54cdf3a54..71ac4dfc974d4 100644 --- a/x-pack/test/encrypted_saved_objects_api_integration/fixtures/es_archiver/encrypted_saved_objects/data.json +++ b/x-pack/test/encrypted_saved_objects_api_integration/fixtures/es_archiver/encrypted_saved_objects/data.json @@ -223,6 +223,70 @@ } } +{ + "type": "doc", + "value": { + "id": "custom-space:saved-object-with-migration:a67c6950-eed8-11eb-9a62-032b4e4049d1", + "index": ".kibana_1", + "source": { + "saved-object-with-migration": { + "encryptedAttribute": "BIOBsx5SjLq3ZQdOJv06XeCAMY9ZrYj8K5bcGa5+wpd3TeT2sqln1+9AGblnfxT7LXRI3sLWQ900+wRQzBhJYx8PNKH+Yw+GdeESpu73PFHdWt/52cJKr+b4EPALFc00tIMEDHdT9FyQhqJ7nV8UpwtjcuTp9SA=", + "nonEncryptedAttribute": "elastic" + }, + "type": "saved-object-with-migration", + "references": [], + "namespace": "custom-space", + "migrationVersion": { + "saved-object-with-migration": "7.7.0" + }, + "updated_at": "2021-07-27T12:46:23.881Z" + } + } +} + +{ + "type": "doc", + "value": { + "id": "saved-object-with-migration:362828f0-eef2-11eb-9073-11359682300a", + "index": ".kibana_1", + "source": { + "saved-object-with-migration": { + "encryptedAttribute": "wWDAtF/5PkCb5BxjfWyRxoIoHbJXlb5cGAKg9ztZ1Bz9Zwo0/xf2yTa3Gq/CbYrvey/F9FZkZOUk03USPaqa5mfFO8FhORkfmNLQaPhgCIDNd6SbIhN8RYkqWVTYSVgcZrwes+VwiTUZ29mCJprVSHwXdyAOy4g=", + "nonEncryptedAttribute": "elastic-migrated", + "additionalEncryptedAttribute": "mszSQj0+Wv7G6kZJQsqf7CWwjJwwyriMlBcUjSHTLlj+tljbLTb7PI7gR07S9l7BXd3Lquc5PeOJifl2HvnTh8s871d/WdtIvt2K/ggwA2ae9NH6ui8A15cuPlXiGO612qccsIyBzhsftFyWJNuLBApmqeEy7HFe" + }, + "type": "saved-object-with-migration", + "references": [], + "migrationVersion": { + "saved-object-with-migration": "7.9.0" + }, + "updated_at": "2021-07-27T15:49:22.324Z" + } + } +} + +{ + "type": "doc", + "value": { + "id": "custom-space:saved-object-with-migration:36448a90-eef2-11eb-9073-11359682300a", + "index": ".kibana_1", + "source": { + "saved-object-with-migration": { + "encryptedAttribute": "33lfpnBI136UfkdcQLzovzBXdUaeDouN0Z32qkVutgZJ5SU60hMtaHWXNkaU9DGy9jtr0ptwm6FCYmZbyDrlGMwyZP2n0PzMhwW9fRcBh7he12Cm1mImWTrxgYoRtc1MX20/orbINx5VnuNl1Ide7htAm1oPRjM=", + "nonEncryptedAttribute": "elastic-migrated", + "additionalEncryptedAttribute": "e2rsxBijtMGcdw7A+WAWJNlLOhQCZnEP1sdcHxVO5aQouiUVeI1OTFcOY3h/+iZBlSGvZdGRURgimrSNc0HRicemZx3o4v1gVw0JX3RRatzdl02v3GJoFzBWfQGyf3xhNNWmkweGJrFQqr2kfdKjIHbdVmMt4LZj" + }, + "type": "saved-object-with-migration", + "references": [], + "namespace": "custom-space", + "migrationVersion": { + "saved-object-with-migration": "7.9.0" + }, + "updated_at": "2021-07-27T15:49:22.509Z" + } + } +} + { "type": "doc", "value": { @@ -367,4 +431,4 @@ "updated_at": "2020-06-17T16:29:27.563Z" } } -} \ No newline at end of file +} diff --git a/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts b/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts index 0b01f4f385da6..311228424afe3 100644 --- a/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts +++ b/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts @@ -527,20 +527,103 @@ export default function ({ getService }: FtrProviderContext) { ); }); - it('migrates unencrypted fields on saved objects', async () => { - const { body: decryptedResponse } = await supertest - .get( - `/api/saved_objects/get-decrypted-as-internal-user/saved-object-with-migration/74f3e6d7-b7bb-477d-ac28-92ee22728e6e` - ) - .expect(200); + function getGetApiUrl({ objectId, spaceId }: { objectId: string; spaceId?: string }) { + const spacePrefix = spaceId ? `/s/${spaceId}` : ''; + return `${spacePrefix}/api/saved_objects/get-decrypted-as-internal-user/saved-object-with-migration/${objectId}`; + } + + // For brevity, each encrypted saved object has the same decrypted attributes after migrations/conversion. + // An assertion based on this ensures all encrypted fields can still be decrypted after migrations/conversion have been applied. + const expectedDecryptedAttributes = { + encryptedAttribute: 'this is my secret api key', + nonEncryptedAttribute: 'elastic-migrated', // this field was migrated in 7.8.0 + additionalEncryptedAttribute: 'elastic-migrated-encrypted', // this field was added in 7.9.0 + }; + + // In these test cases, we simulate a scenario where some existing objects that are migrated when Kibana starts up. Note that when a + // document migration is triggered, the saved object "convert" transform is also applied by the Core migration algorithm. + describe('handles index migration correctly', () => { + describe('in the default space', () => { + it('for a saved object that needs to be migrated before it is converted', async () => { + const getApiUrl = getGetApiUrl({ objectId: '74f3e6d7-b7bb-477d-ac28-92ee22728e6e' }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + }); + + it('for a saved object that does not need to be migrated before it is converted', async () => { + const getApiUrl = getGetApiUrl({ objectId: '362828f0-eef2-11eb-9073-11359682300a' }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + }); + }); + + describe('in a custom space', () => { + const spaceId = 'custom-space'; + + it('for a saved object that needs to be migrated before it is converted', async () => { + const getApiUrl = getGetApiUrl({ + objectId: 'a98e22f8-530e-5d69-baf7-97526796f3a6', // This ID is not found in the data.json file, it is dynamically generated when the object is converted; the original ID is a67c6950-eed8-11eb-9a62-032b4e4049d1 + spaceId, + }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + }); + + it('for a saved object that does not need to be migrated before it is converted', async () => { + const getApiUrl = getGetApiUrl({ + objectId: '41395c74-da7a-5679-9535-412d550a6cf7', // This ID is not found in the data.json file, it is dynamically generated when the object is converted; the original ID is 36448a90-eef2-11eb-9073-11359682300a + spaceId, + }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + }); + }); + }); + + // In these test cases, we simulate a scenario where new objects are migrated upon creation. This happens because an outdated + // `migrationVersion` field is included below. Note that when a document migration is triggered, the saved object "convert" transform + // is *not* applied by the Core migration algorithm. + describe('handles document migration correctly', () => { + function getCreateApiUrl({ spaceId }: { spaceId?: string } = {}) { + const spacePrefix = spaceId ? `/s/${spaceId}` : ''; + return `${spacePrefix}/api/saved_objects/saved-object-with-migration`; + } + + const objectToCreate = { + attributes: { + encryptedAttribute: 'this is my secret api key', + nonEncryptedAttribute: 'elastic', + }, + migrationVersion: { 'saved-object-with-migration': '7.7.0' }, + }; + + it('in the default space', async () => { + const createApiUrl = getCreateApiUrl(); + const { body: savedObject } = await supertest + .post(createApiUrl) + .set('kbn-xsrf', 'xxx') + .send(objectToCreate) + .expect(200); + const { id: objectId } = savedObject; + + const getApiUrl = getGetApiUrl({ objectId }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + }); + + it('in a custom space', async () => { + const spaceId = 'custom-space'; + const createApiUrl = getCreateApiUrl({ spaceId }); + const { body: savedObject } = await supertest + .post(createApiUrl) + .set('kbn-xsrf', 'xxx') + .send(objectToCreate) + .expect(200); + const { id: objectId } = savedObject; - expect(decryptedResponse.attributes).to.eql({ - // ensures the encrypted field can still be decrypted after the migration - encryptedAttribute: 'this is my secret api key', - // ensures the non-encrypted field has been migrated in 7.8.0 - nonEncryptedAttribute: 'elastic-migrated', - // ensures the non-encrypted field has been migrated into a new encrypted field in 7.9.0 - additionalEncryptedAttribute: 'elastic-migrated-encrypted', + const getApiUrl = getGetApiUrl({ objectId, spaceId }); + const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); + expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); }); }); });