Skip to content

Commit

Permalink
Allow predefined ids for encrypted saved objects
Browse files Browse the repository at this point in the history
  • Loading branch information
thomheymann committed Nov 16, 2020
1 parent 3ba7758 commit 2bf55d8
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,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).toBeFalsy();

const typeDefinitionWithPredefinedIDAllowed = new EncryptedSavedObjectAttributesDefinition({
type: 'so-type',
attributesToEncrypt: new Set(['attr#1', 'attr#2']),
allowPredefinedID: true,
});
expect(typeDefinitionWithPredefinedIDAllowed.allowPredefinedID).toBe(true);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -34,6 +35,7 @@ export class EncryptedSavedObjectAttributesDefinition {
this.attributesToEncrypt = attributesToEncrypt;
this.attributesToStrip = attributesToStrip;
this.attributesToExcludeFromAAD = typeRegistration.attributesToExcludeFromAAD;
this.allowPredefinedID = typeRegistration.allowPredefinedID;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ export const encryptedSavedObjectsServiceMock = {
mock.isRegistered.mockImplementation(
(type) => registrations.findIndex((r) => r.type === type) >= 0
);
mock.allowPredefinedID.mockImplementation((type) => {
const registration = registrations.find((r) => r.type === type);
if (!registration) {
return true;
}
return registration.allowPredefinedID === true;
});
mock.encryptAttributes.mockImplementation(async (descriptor, attrs) =>
processAttributes(
descriptor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,35 @@ describe('#isRegistered', () => {
});
});

describe('#allowPredefinedID', () => {
it('returns true for unknown types', () => {
expect(service.allowPredefinedID('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.allowPredefinedID('known-type-1')).toBe(true);
});

it('returns false for types registered without setting allowPredefinedID', () => {
service.registerType({ type: 'known-type-1', attributesToEncrypt: new Set(['attr-1']) });
expect(service.allowPredefinedID('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.allowPredefinedID('known-type-1')).toBe(false);
});
});

describe('#stripOrDecryptAttributes', () => {
it('does not strip attributes from unknown types', async () => {
const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface EncryptedSavedObjectTypeRegistration {
readonly type: string;
readonly attributesToEncrypt: ReadonlySet<string | AttributeToEncrypt>;
readonly attributesToExcludeFromAAD?: ReadonlySet<string>;
readonly allowPredefinedID?: boolean;
}

/**
Expand Down Expand Up @@ -144,6 +145,19 @@ export class EncryptedSavedObjectsService {
return this.typeDefinitions.has(type);
}

/**
* Checks whether specified saved object type supports predefined IDs. If the type isn't registered
* as an encrypted saved object type, this will return "true".
* @param type Saved object type.
*/
public allowPredefinedID(type: string) {
const typeDefinitions = this.typeDefinitions.get(type);
if (typeDefinitions === undefined) {
return true;
}
return typeDefinitions.allowPredefinedID === true;
}

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ beforeEach(() => {
{ key: 'attrNotSoSecret', dangerouslyExposeValue: true },
]),
},
{
type: 'known-type-predefined-id',
attributesToEncrypt: new Set(['attrSecret']),
allowPredefinedID: true,
},
]);

wrapper = new EncryptedSavedObjectsClientWrapper({
Expand Down Expand Up @@ -72,7 +77,7 @@ 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(
Expand All @@ -82,6 +87,26 @@ describe('#create', () => {
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',
Expand Down Expand Up @@ -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 = [
Expand All @@ -314,6 +339,42 @@ describe('#bulkCreate', () => {
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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,17 @@ 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.
// 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.

// 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;
const canSpecifyID =
(options.overwrite && options.version) || this.options.service.allowPredefinedID(type);
if (options.id && !canSpecifyID) {
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}".`
);
}

Expand Down Expand Up @@ -118,10 +119,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;
const canSpecifyID =
(options?.overwrite && object.version) ||
this.options.service.allowPredefinedID(object.type);
if (object.id && !canSpecifyID) {
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}".`
);
}

Expand Down

0 comments on commit 2bf55d8

Please sign in to comment.