diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts index f1e06a0cec03d..f528843cf9ea3 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts @@ -113,18 +113,3 @@ it('correctly determines attribute properties', () => { } } }); - -it('it correctly sets allowPredefinedID', () => { - const defaultTypeDefinition = new EncryptedSavedObjectAttributesDefinition({ - type: 'so-type', - attributesToEncrypt: new Set(['attr#1', 'attr#2']), - }); - expect(defaultTypeDefinition.allowPredefinedID).toBe(false); - - const typeDefinitionWithPredefinedIDAllowed = new EncryptedSavedObjectAttributesDefinition({ - type: 'so-type', - attributesToEncrypt: new Set(['attr#1', 'attr#2']), - allowPredefinedID: true, - }); - expect(typeDefinitionWithPredefinedIDAllowed.allowPredefinedID).toBe(true); -}); diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts index 398a64585411a..849a2888b6e1a 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts @@ -15,7 +15,6 @@ export class EncryptedSavedObjectAttributesDefinition { public readonly attributesToEncrypt: ReadonlySet; private readonly attributesToExcludeFromAAD: ReadonlySet | undefined; private readonly attributesToStrip: ReadonlySet; - public readonly allowPredefinedID: boolean; constructor(typeRegistration: EncryptedSavedObjectTypeRegistration) { const attributesToEncrypt = new Set(); @@ -35,7 +34,6 @@ export class EncryptedSavedObjectAttributesDefinition { this.attributesToEncrypt = attributesToEncrypt; this.attributesToStrip = attributesToStrip; this.attributesToExcludeFromAAD = typeRegistration.attributesToExcludeFromAAD; - this.allowPredefinedID = !!typeRegistration.allowPredefinedID; } /** diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts index 0138e929ca1ca..c692d8698771f 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts @@ -13,7 +13,6 @@ import { function createEncryptedSavedObjectsServiceMock() { return ({ isRegistered: jest.fn(), - canSpecifyID: jest.fn(), stripOrDecryptAttributes: jest.fn(), encryptAttributes: jest.fn(), decryptAttributes: jest.fn(), @@ -53,12 +52,6 @@ export const encryptedSavedObjectsServiceMock = { mock.isRegistered.mockImplementation( (type) => registrations.findIndex((r) => r.type === type) >= 0 ); - mock.canSpecifyID.mockImplementation((type, version, overwrite) => { - const registration = registrations.find((r) => r.type === type); - return ( - registration === undefined || registration.allowPredefinedID || !!(version && overwrite) - ); - }); mock.encryptAttributes.mockImplementation(async (descriptor, attrs) => processAttributes( descriptor, 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 6bc4a392064e4..88d57072697fe 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 @@ -89,45 +89,6 @@ describe('#isRegistered', () => { }); }); -describe('#canSpecifyID', () => { - it('returns true for unknown types', () => { - expect(service.canSpecifyID('unknown-type')).toBe(true); - }); - - it('returns true for types registered setting allowPredefinedID to true', () => { - service.registerType({ - type: 'known-type-1', - attributesToEncrypt: new Set(['attr-1']), - allowPredefinedID: true, - }); - expect(service.canSpecifyID('known-type-1')).toBe(true); - }); - - it('returns true when overwriting a saved object with a version specified even when allowPredefinedID is not set', () => { - service.registerType({ - type: 'known-type-1', - attributesToEncrypt: new Set(['attr-1']), - }); - expect(service.canSpecifyID('known-type-1', '2', true)).toBe(true); - expect(service.canSpecifyID('known-type-1', '2', false)).toBe(false); - expect(service.canSpecifyID('known-type-1', undefined, true)).toBe(false); - }); - - it('returns false for types registered without setting allowPredefinedID', () => { - service.registerType({ type: 'known-type-1', attributesToEncrypt: new Set(['attr-1']) }); - expect(service.canSpecifyID('known-type-1')).toBe(false); - }); - - it('returns false for types registered setting allowPredefinedID to false', () => { - service.registerType({ - type: 'known-type-1', - attributesToEncrypt: new Set(['attr-1']), - allowPredefinedID: false, - }); - expect(service.canSpecifyID('known-type-1')).toBe(false); - }); -}); - describe('#stripOrDecryptAttributes', () => { it('does not strip attributes from unknown types', async () => { const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' }; 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 8d2ebb575c35e..1f1093a179538 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 @@ -31,7 +31,6 @@ export interface EncryptedSavedObjectTypeRegistration { readonly type: string; readonly attributesToEncrypt: ReadonlySet; readonly attributesToExcludeFromAAD?: ReadonlySet; - readonly allowPredefinedID?: boolean; } /** @@ -145,25 +144,6 @@ export class EncryptedSavedObjectsService { return this.typeDefinitions.has(type); } - /** - * Checks whether ID can be specified for the provided saved object. - * - * If the type isn't registered as an encrypted saved object, or when overwriting an existing - * saved object with a version specified, this will return "true". - * - * @param type Saved object type. - * @param version Saved object version number which changes on each successful write operation. - * Can be used in conjunction with `overwrite` for implementing optimistic concurrency - * control. - * @param overwrite Overwrite existing documents. - */ - public canSpecifyID(type: string, version?: string, overwrite?: boolean) { - const typeDefinition = this.typeDefinitions.get(type); - return ( - typeDefinition === undefined || typeDefinition.allowPredefinedID || !!(version && overwrite) - ); - } - /** * Takes saved object attributes for the specified type and, depending on the type definition, * either decrypts or strips encrypted attributes (e.g. in case AAD or encryption key has changed diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts index f773aef13d63b..262ce45863fcd 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts @@ -32,11 +32,6 @@ beforeEach(() => { { key: 'attrNotSoSecret', dangerouslyExposeValue: true }, ]), }, - { - type: 'known-type-predefined-id', - attributesToEncrypt: new Set(['attrSecret']), - allowPredefinedID: true, - }, ]); wrapper = new EncryptedSavedObjectsClientWrapper({ @@ -79,36 +74,16 @@ describe('#create', () => { expect(mockBaseClient.create).toHaveBeenCalledWith('unknown-type', attributes, options); }); - it('fails if type is registered without allowPredefinedID and ID is specified', async () => { + it('fails if type is registered and ID is specified', async () => { const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' }; await expect(wrapper.create('known-type', attributes, { id: 'some-id' })).rejects.toThrowError( - 'Predefined IDs are not allowed for encrypted saved objects of type "known-type".' + 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); expect(mockBaseClient.create).not.toHaveBeenCalled(); }); - it('succeeds if type is registered with allowPredefinedID and ID is specified', async () => { - const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' }; - const mockedResponse = { - id: 'some-id', - type: 'known-type-predefined-id', - attributes: { attrOne: 'one', attrSecret: '*secret*', attrThree: 'three' }, - references: [], - }; - - mockBaseClient.create.mockResolvedValue(mockedResponse); - await expect( - wrapper.create('known-type-predefined-id', attributes, { id: 'some-id' }) - ).resolves.toEqual({ - ...mockedResponse, - attributes: { attrOne: 'one', attrThree: 'three' }, - }); - - expect(mockBaseClient.create).toHaveBeenCalled(); - }); - it('allows a specified ID when overwriting an existing object', async () => { const attributes = { attrOne: 'one', @@ -326,7 +301,7 @@ describe('#bulkCreate', () => { ); }); - it('fails if ID is specified for registered type without allowPredefinedID', async () => { + it('fails if ID is specified for registered type', async () => { const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' }; const bulkCreateParams = [ @@ -335,48 +310,12 @@ describe('#bulkCreate', () => { ]; await expect(wrapper.bulkCreate(bulkCreateParams)).rejects.toThrowError( - 'Predefined IDs are not allowed for encrypted saved objects of type "known-type".' + 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); expect(mockBaseClient.bulkCreate).not.toHaveBeenCalled(); }); - it('succeeds if ID is specified for registered type with allowPredefinedID', async () => { - const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' }; - const options = { namespace: 'some-namespace' }; - const mockedResponse = { - saved_objects: [ - { - id: 'some-id', - type: 'known-type-predefined-id', - attributes, - references: [], - }, - { - id: 'some-id', - type: 'unknown-type', - attributes, - references: [], - }, - ], - }; - mockBaseClient.bulkCreate.mockResolvedValue(mockedResponse); - - const bulkCreateParams = [ - { id: 'some-id', type: 'known-type-predefined-id', attributes }, - { type: 'unknown-type', attributes }, - ]; - - await expect(wrapper.bulkCreate(bulkCreateParams, options)).resolves.toEqual({ - saved_objects: [ - { ...mockedResponse.saved_objects[0], attributes: { attrOne: 'one', attrThree: 'three' } }, - mockedResponse.saved_objects[1], - ], - }); - - expect(mockBaseClient.bulkCreate).toHaveBeenCalled(); - }); - it('allows a specified ID when overwriting an existing object', async () => { const attributes = { attrOne: 'one', diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts index 2002a779ba873..e2984f7e7c34f 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts @@ -60,14 +60,16 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon } // Saved objects with encrypted attributes should have IDs that are hard to guess especially - // since IDs are part of the AAD used during encryption. Types can opt-out of this restriction, - // when necessary, but it's much safer for this wrapper to generate them. - if ( - options.id && - !this.options.service.canSpecifyID(type, options.version, options.overwrite) - ) { + // since IDs are part of the AAD used during encryption, that's why we control them within this + // wrapper and don't allow consumers to specify their own IDs directly. + + // only allow a specified ID if we're overwriting an existing ESO with a Version + // this helps us ensure that the document really was previously created using ESO + // and not being used to get around the specified ID limitation + const canSpecifyID = options.overwrite && options.version; + if (options.id && !canSpecifyID) { throw new Error( - `Predefined IDs are not allowed for encrypted saved objects of type "${type}".` + 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); } @@ -108,12 +110,10 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon // Saved objects with encrypted attributes should have IDs that are hard to guess especially // since IDs are part of the AAD used during encryption, that's why we control them within this // wrapper and don't allow consumers to specify their own IDs directly unless overwriting the original document. - if ( - object.id && - !this.options.service.canSpecifyID(object.type, object.version, options?.overwrite) - ) { + const canSpecifyID = options?.overwrite && object.version; + if (object.id && !canSpecifyID) { throw new Error( - `Predefined IDs are not allowed for encrypted saved objects of type "${object.type}".` + 'Predefined IDs are not allowed for saved objects with encrypted attributes.' ); }