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 {