Allow predefined ids for encrypted saved objects (elastic#83482)

* Allow predefined ids for encrypted saved objects

* Fix mock

* fix tests

* Added suggestions from code review

* added jsdocs params

* Fixed jsdocs

x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts

@@ -113,3 +113,18 @@ 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);
+});

x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts

@@ -15,6 +15,7 @@ export class EncryptedSavedObjectAttributesDefinition {
   public readonly attributesToEncrypt: ReadonlySet<string>;
   private readonly attributesToExcludeFromAAD: ReadonlySet<string> | undefined;
   private readonly attributesToStrip: ReadonlySet<string>;
+  public readonly allowPredefinedID: boolean;
 
   constructor(typeRegistration: EncryptedSavedObjectTypeRegistration) {
     const attributesToEncrypt = new Set<string>();
@@ -34,6 +35,7 @@ export class EncryptedSavedObjectAttributesDefinition {
     this.attributesToEncrypt = attributesToEncrypt;
     this.attributesToStrip = attributesToStrip;
     this.attributesToExcludeFromAAD = typeRegistration.attributesToExcludeFromAAD;
+    this.allowPredefinedID = !!typeRegistration.allowPredefinedID;
   }
 
   /**

x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts

@@ -13,6 +13,7 @@ import {
 function createEncryptedSavedObjectsServiceMock() {
   return ({
     isRegistered: jest.fn(),
+    canSpecifyID: jest.fn(),
     stripOrDecryptAttributes: jest.fn(),
     encryptAttributes: jest.fn(),
     decryptAttributes: jest.fn(),
@@ -52,6 +53,12 @@ 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,

x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts

@@ -89,6 +89,45 @@ 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' };

x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts

@@ -31,6 +31,7 @@ export interface EncryptedSavedObjectTypeRegistration {
   readonly type: string;
   readonly attributesToEncrypt: ReadonlySet<string | AttributeToEncrypt>;
   readonly attributesToExcludeFromAAD?: ReadonlySet<string>;
+  readonly allowPredefinedID?: boolean;
 }
 
 /**
@@ -144,6 +145,25 @@ 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

x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts

@@ -30,6 +30,11 @@ beforeEach(() => {
         { key: 'attrNotSoSecret', dangerouslyExposeValue: true },
       ]),
     },
+    {
+      type: 'known-type-predefined-id',
+      attributesToEncrypt: new Set(['attrSecret']),
+      allowPredefinedID: true,
+    },
   ]);
 
   wrapper = new EncryptedSavedObjectsClientWrapper({
@@ -72,16 +77,36 @@ describe('#create', () => {
     expect(mockBaseClient.create).toHaveBeenCalledWith('unknown-type', attributes, options);
   });
 
-  it('fails if type is registered and ID is specified', async () => {
+  it('fails if type is registered without allowPredefinedID 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 saved objects with encrypted attributes.'
+      'Predefined IDs are not allowed for encrypted saved objects of type "known-type".'
     );
 
     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',
@@ -299,7 +324,7 @@ describe('#bulkCreate', () => {
     );
   });
 
-  it('fails if ID is specified for registered type', async () => {
+  it('fails if ID is specified for registered type without allowPredefinedID', async () => {
     const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };
 
     const bulkCreateParams = [
@@ -308,12 +333,48 @@ describe('#bulkCreate', () => {
     ];
 
     await expect(wrapper.bulkCreate(bulkCreateParams)).rejects.toThrowError(
-      'Predefined IDs are not allowed for saved objects with encrypted attributes.'
+      'Predefined IDs are not allowed for encrypted saved objects of type "known-type".'
     );
 
     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',

x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts

@@ -68,16 +68,14 @@ 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.
-
-    // 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) {
+    // 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)
+    ) {
       throw new Error(
-        'Predefined IDs are not allowed for saved objects with encrypted attributes.'
+        `Predefined IDs are not allowed for encrypted saved objects of type "${type}".`
       );
     }
 
@@ -118,10 +116,12 @@ 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.
-        const canSpecifyID = options?.overwrite && object.version;
-        if (object.id && !canSpecifyID) {
+        if (
+          object.id &&
+          !this.options.service.canSpecifyID(object.type, object.version, options?.overwrite)
+        ) {
           throw new Error(
-            'Predefined IDs are not allowed for saved objects with encrypted attributes.'
+            `Predefined IDs are not allowed for encrypted saved objects of type "${object.type}".`
           );
         }