From 9742a52e59153902b24ad4e978ca4204f8d3bf47 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 29 Jul 2021 12:02:26 -0400 Subject: [PATCH 1/2] Fix ESO migration decryption for converted saved object types (#106897) --- ...tmigrationcontext.issinglenamespacetype.md | 13 + ...core-server.savedobjectmigrationcontext.md | 1 + .../migrations/core/document_migrator.ts | 1 + .../server/saved_objects/migrations/mocks.ts | 3 + .../server/saved_objects/migrations/types.ts | 4 + src/core/server/server.api.md | 1 + .../server/create_migration.test.ts | 234 ++++++++--- .../server/create_migration.ts | 47 ++- .../encrypted_saved_objects_service.test.ts | 384 ++++++++++++++---- .../crypto/encrypted_saved_objects_service.ts | 29 +- .../api_consumer_plugin/server/index.ts | 15 +- .../encrypted_saved_objects/data.json | 66 ++- .../tests/encrypted_saved_objects_api.ts | 109 ++++- 13 files changed, 733 insertions(+), 174 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md new file mode 100644 index 00000000000000..528be67f029c64 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectMigrationContext](./kibana-plugin-core-server.savedobjectmigrationcontext.md) > [isSingleNamespaceType](./kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md) + +## SavedObjectMigrationContext.isSingleNamespaceType property + +Whether this is a single-namespace type or not + +Signature: + +```typescript +readonly isSingleNamespaceType: boolean; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.md index c8a291e5028453..21ca234fde185c 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectmigrationcontext.md @@ -17,6 +17,7 @@ export interface SavedObjectMigrationContext | Property | Type | Description | | --- | --- | --- | | [convertToMultiNamespaceTypeVersion](./kibana-plugin-core-server.savedobjectmigrationcontext.converttomultinamespacetypeversion.md) | string | The version in which this object type is being converted to a multi-namespace type | +| [isSingleNamespaceType](./kibana-plugin-core-server.savedobjectmigrationcontext.issinglenamespacetype.md) | boolean | Whether this is a single-namespace type or not | | [log](./kibana-plugin-core-server.savedobjectmigrationcontext.log.md) | SavedObjectsMigrationLogger | logger instance to be used by the migration handler | | [migrationVersion](./kibana-plugin-core-server.savedobjectmigrationcontext.migrationversion.md) | string | The migration version that this migration function is defined for | diff --git a/src/core/server/saved_objects/migrations/core/document_migrator.ts b/src/core/server/saved_objects/migrations/core/document_migrator.ts index de8adc23996fd2..23f5d075d72e37 100644 --- a/src/core/server/saved_objects/migrations/core/document_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/document_migrator.ts @@ -667,6 +667,7 @@ function wrapWithTry( log: new MigrationLogger(log), migrationVersion: version, convertToMultiNamespaceTypeVersion: type.convertToMultiNamespaceTypeVersion, + isSingleNamespaceType: type.namespaceType === 'single', }); return function tryTransformDoc(doc: SavedObjectUnsanitizedDoc) { diff --git a/src/core/server/saved_objects/migrations/mocks.ts b/src/core/server/saved_objects/migrations/mocks.ts index 4a62fcc95997bb..ef806aa5f0fca1 100644 --- a/src/core/server/saved_objects/migrations/mocks.ts +++ b/src/core/server/saved_objects/migrations/mocks.ts @@ -24,14 +24,17 @@ export const createSavedObjectsMigrationLoggerMock = (): jest.Mocked => { const mock = { log: createSavedObjectsMigrationLoggerMock(), migrationVersion, convertToMultiNamespaceTypeVersion, + isSingleNamespaceType, }; return mock; }; diff --git a/src/core/server/saved_objects/migrations/types.ts b/src/core/server/saved_objects/migrations/types.ts index 570315e780ebe8..fe5a79dac12c36 100644 --- a/src/core/server/saved_objects/migrations/types.ts +++ b/src/core/server/saved_objects/migrations/types.ts @@ -65,6 +65,10 @@ export interface SavedObjectMigrationContext { * The version in which this object type is being converted to a multi-namespace type */ readonly convertToMultiNamespaceTypeVersion?: string; + /** + * Whether this is a single-namespace type or not + */ + readonly isSingleNamespaceType: boolean; } /** diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 47782c2f98d3a9..fbc51acfdc8ef6 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2268,6 +2268,7 @@ export interface SavedObjectExportBaseOptions { // @public export interface SavedObjectMigrationContext { readonly convertToMultiNamespaceTypeVersion?: string; + readonly isSingleNamespaceType: boolean; readonly log: SavedObjectsMigrationLogger; readonly migrationVersion: string; } 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 32aac4a37e4c21..be05825192d356 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 @@ -16,8 +16,23 @@ afterEach(() => { jest.clearAllMocks(); }); +interface ConversionTestOptions { + /** Migration version(s) to test */ + migrationVersions: string[]; + /** The `namespace` field of the object that is being migrated */ + objectNamespace: string | undefined; + /** The `namespaces` field of the object that is being migrated */ + objectNamespaces: string[] | undefined; + /** The expected namespace in the decrypt descriptor */ + expectedDecryptDescriptorNamespace: string | undefined; + /** The expected namespace in the encrypt descriptor */ + expectedEncryptDescriptorNamespace: string | undefined; + /** The expected value for the `isTypeBeingConverted` field in the decrypt params */ + expectedIsTypeBeingConverted: boolean; +} + describe('createMigration()', () => { - const migrationContext = migrationMocks.createContext(); + const migrationContext = migrationMocks.createContext({ isSingleNamespaceType: true }); const inputType = { type: 'known-type-1', attributesToEncrypt: new Set(['firstAttr']) }; const migrationType = { type: 'known-type-1', @@ -100,7 +115,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(encryptionSavedObjectService.encryptAttributesSync).toHaveBeenCalledWith( @@ -157,7 +172,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).not.toHaveBeenCalled(); @@ -209,7 +224,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(encryptionSavedObjectService.stripOrDecryptAttributesSync).not.toHaveBeenCalled(); @@ -273,7 +288,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).toHaveBeenCalled(); @@ -331,7 +346,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).toHaveBeenCalled(); @@ -383,7 +398,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).toHaveBeenCalled(); @@ -435,7 +450,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).toHaveBeenCalled(); @@ -495,7 +510,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(migrationFunc).toHaveBeenCalled(); @@ -551,7 +566,7 @@ describe('createMigration()', () => { namespace: 'namespace', }, attributes, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(encryptionSavedObjectService.encryptAttributesSync).toHaveBeenCalledWith( @@ -564,14 +579,15 @@ describe('createMigration()', () => { ); }); - describe('uses the object `namespaces` field to populate the descriptor when the migration context indicates this type is being converted', () => { + describe('handles conversion to a multi-namespace type (convertToMultiNamespaceVersion migration context field)', () => { const doTest = ({ + migrationVersions, objectNamespace, - decryptDescriptorNamespace, - }: { - objectNamespace: string | undefined; - decryptDescriptorNamespace: string | undefined; - }) => { + objectNamespaces, + expectedDecryptDescriptorNamespace, + expectedEncryptDescriptorNamespace, + expectedIsTypeBeingConverted, + }: ConversionTestOptions) => { const instantiateServiceWithLegacyType = jest.fn(() => encryptedSavedObjectsServiceMock.create() ); @@ -586,56 +602,150 @@ describe('createMigration()', () => { }, migration: (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' }; + + migrationVersions.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, // Any migrationVersion <= convertToMultiNamespaceTypeVersion (8.0.0) indicates this type is being converted + convertToMultiNamespaceTypeVersion: '8.0.0', + isSingleNamespaceType: false, // in practice, this can never be true if convertToMultiNamespaceTypeVersion is defined + }) + ); + expect(encryptionSavedObjectService.decryptAttributesSync).toHaveBeenNthCalledWith( + i + 1, + { id: '123', type: 'known-type-1', namespace: expectedDecryptDescriptorNamespace }, 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 - ); + { isTypeBeingConverted: expectedIsTypeBeingConverted, originId: 'some-origin-id' } // decrypt parameters + ); + expect(encryptionSavedObjectService.encryptAttributesSync).toHaveBeenNthCalledWith( + i + 1, + { id: '123', type: 'known-type-1', namespace: expectedEncryptDescriptorNamespace }, + 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 }); + describe('when migrationVersion <= convertToMultiNamespaceVersion, it sets the descriptors and decrypt parameters appropriately', () => { + const migrationVersions = ['7.99.99', '8.0.0']; + [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({ + migrationVersions, + objectNamespace, + objectNamespaces: undefined, + expectedDecryptDescriptorNamespace: objectNamespace, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: true, + }); + }); + + 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({ + migrationVersions, + objectNamespace, + objectNamespaces: [], + expectedDecryptDescriptorNamespace: objectNamespace, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: true, + }); + }); + + it(`when namespaces is a non-empty array (default space) and namespace is ${namespaceDescription}`, () => { + doTest({ + migrationVersions, + objectNamespace, + objectNamespaces: ['default', 'additional-spaces-are-ignored'], + expectedDecryptDescriptorNamespace: undefined, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: true, + }); + }); + + it(`when namespaces is a non-empty array (custom space) and namespace is ${namespaceDescription}`, () => { + doTest({ + migrationVersions, + objectNamespace, + objectNamespaces: ['custom', 'additional-spaces-are-ignored'], + expectedDecryptDescriptorNamespace: 'custom', + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: true, + }); + }); + }); }); - it('when the first namespace element is another string', () => { - doTest({ objectNamespace: 'foo', decryptDescriptorNamespace: 'foo' }); + describe('when migrationVersion > convertToMultiNamespaceVersion, it sets the descriptors and decrypt parameters appropriately', () => { + const migrationVersions = ['8.0.1']; + [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({ + migrationVersions, + objectNamespace, + objectNamespaces: undefined, + expectedDecryptDescriptorNamespace: undefined, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: false, + }); + }); + + 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({ + migrationVersions, + objectNamespace, + objectNamespaces: [], + expectedDecryptDescriptorNamespace: undefined, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: false, + }); + }); + + it(`when namespaces is a non-empty array (default space) and namespace is ${namespaceDescription}`, () => { + doTest({ + migrationVersions, + objectNamespace, + objectNamespaces: ['default', 'additional-spaces-are-ignored'], + expectedDecryptDescriptorNamespace: undefined, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: false, + }); + }); + + it(`when namespaces is a non-empty array (custom space) and namespace is ${namespaceDescription}`, () => { + doTest({ + migrationVersions, + objectNamespace, + objectNamespaces: ['custom', 'additional-spaces-are-ignored'], + expectedDecryptDescriptorNamespace: undefined, + expectedEncryptDescriptorNamespace: undefined, + expectedIsTypeBeingConverted: false, + }); + }); + }); }); }); }); @@ -754,7 +864,7 @@ describe('createMigration()', () => { firstAttr: '#####', nonEncryptedAttr: 'non encrypted', }, - { convertToMultiNamespaceType: false } + { isTypeBeingConverted: false } ); expect(serviceWithMigrationLegacyType.encryptAttributesSync).toHaveBeenCalledWith( 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 b9e6dcf7109245..18786e0f9d1c63 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, @@ -77,19 +79,44 @@ 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 { type, id, originId } = encryptedDoc; + + // 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 isTypeBeingConverted = + !!context.convertToMultiNamespaceTypeVersion && + semver.lte(context.migrationVersion, context.convertToMultiNamespaceTypeVersion); + + // This approximates the behavior of getDescriptorNamespace(); the intent is that if there is ever a case where a multi-namespace object + // has the `namespace` field, it will not be encrypted with that field in its descriptor. 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 + const descriptorNamespace = context.isSingleNamespaceType ? encryptedDoc.namespace : undefined; + let decryptDescriptorNamespace = descriptorNamespace; + // 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; + if (isTypeBeingConverted) { + decryptDescriptorNamespace = 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: descriptorNamespace }; // decrypt the attributes using the input type definition // if an error occurs during decryption, use the shouldMigrateIfDecryptionFails flag @@ -98,7 +125,8 @@ export const getCreateMigration = ( const documentToMigrate = mapAttributes(encryptedDoc, (inputAttributes) => { try { return inputService.decryptAttributesSync(decryptDescriptor, inputAttributes, { - convertToMultiNamespaceType, + isTypeBeingConverted, + originId, }); } catch (err) { if (!shouldMigrateIfDecryptionFails || !(err instanceof EncryptionError)) { @@ -109,7 +137,8 @@ export const getCreateMigration = ( `Decryption failed for encrypted Saved Object "${encryptedDoc.id}" of type "${encryptedDoc.type}" with error: ${err.message}. Encrypted attributes have been stripped from the original document and migration will be applied but this may cause errors later on.` ); return inputService.stripOrDecryptAttributesSync(decryptDescriptor, inputAttributes, { - convertToMultiNamespaceType, + isTypeBeingConverted, + originId, }).attributes; } }); 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 0625199eeed63d..15de21999fba32 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 @@ -1024,53 +1024,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 isTypeBeingConverted 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, isTypeBeingConverted: 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, isTypeBeingConverted: 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, isTypeBeingConverted: 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 () => { @@ -1289,7 +1399,7 @@ describe('#decryptAttributes', () => { service.decryptAttributes( { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, encryptedAttributes, - { user: mockUser, convertToMultiNamespaceType: true } + { user: mockUser, isTypeBeingConverted: true } ) ).rejects.toThrowError(EncryptionError); expect(mockNodeCrypto.decrypt).toHaveBeenCalledTimes(2); @@ -2002,53 +2112,163 @@ describe('#decryptAttributesSync', () => { }); }); - it('retries decryption without namespace if incorrect namespace is provided and convertToMultiNamespaceType option is enabled', () => { - const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; + describe('with isTypeBeingConverted option', () => { + it('retries decryption without namespace', () => { + 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(); + expect( + service.decryptAttributesSync( + { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, + encryptedAttributes, + { user: mockUser, isTypeBeingConverted: true } + ) + ).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decryptSync).toHaveBeenCalledTimes(2); + expect(mockNodeCrypto.decryptSync).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.decryptSync).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)', () => { + 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(); + expect( + service.decryptAttributesSync( + { type: 'known-type-1', id: 'object-id', namespace: undefined }, + encryptedAttributes, + { user: mockUser, isTypeBeingConverted: true, originId: 'old-object-id' } + ) + ).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decryptSync).toHaveBeenCalledTimes(2); + expect(mockNodeCrypto.decryptSync).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.decryptSync).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(); - expect( - service.decryptAttributesSync( + it('retries decryption without namespace *and* without originId (old object ID)', () => { + 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(); + expect( + service.decryptAttributesSync( + { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, + encryptedAttributes, + { user: mockUser, isTypeBeingConverted: true, originId: 'old-object-id' } + ) + ).toEqual({ + attrOne: 'one', + attrTwo: 'two', + attrThree: 'three', + }); + expect(mockNodeCrypto.decryptSync).toHaveBeenCalledTimes(4); + expect(mockNodeCrypto.decryptSync).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.decryptSync).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.decryptSync).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.decryptSync).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 } - ) - ).toEqual({ - attrOne: 'one', - attrTwo: 'two', - attrThree: 'three', + mockUser + ); }); - expect(mockNodeCrypto.decryptSync).toHaveBeenCalledTimes(2); - expect(mockNodeCrypto.decryptSync).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.decryptSync).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', () => { @@ -2214,7 +2434,7 @@ describe('#decryptAttributesSync', () => { service.decryptAttributesSync( { type: 'known-type-1', id: 'object-id', namespace: 'object-ns' }, encryptedAttributes, - { user: mockUser, convertToMultiNamespaceType: true } + { user: mockUser, isTypeBeingConverted: true } ) ).toThrowError(EncryptionError); expect(mockNodeCrypto.decryptSync).toHaveBeenCalledTimes(2); 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 cc1a8414924c39..0cf775b88e7d5b 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 @@ -70,7 +70,12 @@ interface DecryptParameters extends CommonParameters { * the object was previously encrypted with its namespace in the descriptor portion of the AAD; on the other hand, if the object is being * decrypted during object migration, the object was never encrypted with its namespace in the descriptor portion of the AAD. */ - convertToMultiNamespaceType?: boolean; + isTypeBeingConverted?: 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 { @@ -528,11 +533,23 @@ 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?.isTypeBeingConverted) { + // 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 decryptDescriptors = params.originId + ? [{ ...descriptor, id: params.originId }, descriptor] + : [descriptor]; + for (const decryptDescriptor of decryptDescriptors) { + 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 10846442d1c84b..e1cf24521d1b1e 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: { @@ -199,6 +200,18 @@ function defineTypeWithMigration(core: CoreSetup, deps: PluginsSet }, inputType: 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({ + isMigrationNeededPredicate: (doc): doc is SavedObjectUnsanitizedDoc => true, + migration: (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 88ec54cdf3a54e..71ac4dfc974d43 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 b4f24c45d4daa2..c4363d8659dbf5 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 @@ -528,20 +528,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); }); }); }); From 6c1dd274d609b4e602e726198b77f53a5cffe08d Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 29 Jul 2021 16:53:48 -0400 Subject: [PATCH 2/2] Get rid of integration test changes The integration test changes can only be applied in 8.0 or later. We can safely remove that from the 7.x branch. --- .../api_consumer_plugin/server/index.ts | 15 +-- .../encrypted_saved_objects/data.json | 66 +---------- .../tests/encrypted_saved_objects_api.ts | 109 +++--------------- 3 files changed, 15 insertions(+), 175 deletions(-) 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 e1cf24521d1b1e..10846442d1c84b 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,8 +134,7 @@ function defineTypeWithMigration(core: CoreSetup, deps: PluginsSet core.savedObjects.registerType({ name: SAVED_OBJECT_WITH_MIGRATION_TYPE, hidden: false, - 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 + namespaceType: 'single', mappings: { properties: { nonEncryptedAttribute: { @@ -200,18 +199,6 @@ function defineTypeWithMigration(core: CoreSetup, deps: PluginsSet }, inputType: 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({ - isMigrationNeededPredicate: (doc): doc is SavedObjectUnsanitizedDoc => true, - migration: (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 71ac4dfc974d43..88ec54cdf3a54e 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,70 +223,6 @@ } } -{ - "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": { @@ -431,4 +367,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 c4363d8659dbf5..b4f24c45d4daa2 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 @@ -528,103 +528,20 @@ export default function ({ getService }: FtrProviderContext) { ); }); - 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; + 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); - const getApiUrl = getGetApiUrl({ objectId, spaceId }); - const { body: decryptedResponse } = await supertest.get(getApiUrl).expect(200); - expect(decryptedResponse.attributes).to.eql(expectedDecryptedAttributes); + 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', }); }); });