Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow predefined ids for encrypted saved objects #83482

Merged
merged 8 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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).toBe(false);

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 @@ -13,6 +13,7 @@ import {
function createEncryptedSavedObjectsServiceMock() {
return ({
isRegistered: jest.fn(),
canSpecifyID: jest.fn(),
stripOrDecryptAttributes: jest.fn(),
encryptAttributes: jest.fn(),
decryptAttributes: jest.fn(),
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
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,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
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,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',
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 @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}".`
);
}

Expand Down Expand Up @@ -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}".`
);
}

Expand Down