diff --git a/src/core/server/saved_objects/serialization/serializer.test.ts b/src/core/server/saved_objects/serialization/serializer.test.ts index 6c7fcc88cc902..3fdeb4aa088e1 100644 --- a/src/core/server/saved_objects/serialization/serializer.test.ts +++ b/src/core/server/saved_objects/serialization/serializer.test.ts @@ -158,7 +158,6 @@ describe('#rawToSavedObject', () => { _id: 'foo:bar', _source: { type: 'foo', - hello: {}, }, }); expect(actual).not.toHaveProperty('version'); @@ -171,7 +170,6 @@ describe('#rawToSavedObject', () => { _primary_term: 1, _source: { type: 'foo', - hello: {}, }, }); expect(actual).toHaveProperty('version', encodeVersion(4, 1)); @@ -184,7 +182,6 @@ describe('#rawToSavedObject', () => { _seq_no: 4, _source: { type: 'foo', - hello: {}, }, }) ).toThrowErrorMatchingInlineSnapshot(`"_primary_term from elasticsearch must be an integer"`); @@ -197,7 +194,6 @@ describe('#rawToSavedObject', () => { _primary_term: 1, _source: { type: 'foo', - hello: {}, }, }) ).toThrowErrorMatchingInlineSnapshot(`"_seq_no from elasticsearch must be an integer"`); @@ -249,7 +245,7 @@ describe('#rawToSavedObject', () => { test('it does not pass unknown properties through', () => { const actual = singleNamespaceSerializer.rawToSavedObject({ - _id: 'universe', + _id: 'hello:universe', _source: { type: 'hello', hello: { @@ -270,7 +266,7 @@ describe('#rawToSavedObject', () => { test('it does not create attributes if [type] is missing', () => { const actual = singleNamespaceSerializer.rawToSavedObject({ - _id: 'universe', + _id: 'hello:universe', _source: { type: 'hello', }, @@ -285,14 +281,14 @@ describe('#rawToSavedObject', () => { test('it fails for documents which do not specify a type', () => { expect(() => singleNamespaceSerializer.rawToSavedObject({ - _id: 'universe', + _id: 'hello:universe', _source: { hello: { world: 'earth', }, } as any, }) - ).toThrow(/Expected "undefined" to be a saved object type/); + ).toThrow(`Raw document 'hello:universe' is missing _source.type field`); }); test('it is complimentary with savedObjectToRaw', () => { @@ -325,29 +321,30 @@ describe('#rawToSavedObject', () => { ).toEqual(raw); }); - test('it handles unprefixed ids', () => { - const actual = singleNamespaceSerializer.rawToSavedObject({ - _id: 'universe', - _source: { - type: 'hello', - }, - }); - - expect(actual).toHaveProperty('id', 'universe'); + test('fails for documents which do not have a type prefix in their _id', () => { + expect(() => + singleNamespaceSerializer.rawToSavedObject({ + _id: 'universe', + _source: { + type: 'hello', + }, + }) + ).toThrow(`Raw document 'universe' does not start with expected prefix 'hello:'`); }); describe('namespace-agnostic type with a namespace', () => { - const raw = createSampleDoc({ _source: { namespace: 'baz' } }); + const raw = createSampleDoc({ _source: { namespace: 'baz' } }); // namespace field should be ignored const actual = namespaceAgnosticSerializer.rawToSavedObject(raw); test(`removes type prefix from _id`, () => { expect(actual).toHaveProperty('id', 'bar'); }); - test(`copies _id to id if prefixed by namespace and type`, () => { + test(`fails for documents which have a namespace prefix in their _id`, () => { const _id = `${raw._source.namespace}:${raw._id}`; - const _actual = namespaceAgnosticSerializer.rawToSavedObject({ ...raw, _id }); - expect(_actual).toHaveProperty('id', _id); + expect(() => namespaceAgnosticSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'baz:foo:bar' does not start with expected prefix 'foo:'` + ); }); test(`doesn't copy _source.namespace to namespace`, () => { @@ -372,10 +369,11 @@ describe('#rawToSavedObject', () => { expect(actual).toHaveProperty('id', 'bar'); }); - test(`copies _id to id if prefixed by random prefix and type`, () => { + test(`fails for documents which have any extra prefix in their _id`, () => { const _id = `random:${raw._id}`; - const _actual = singleNamespaceSerializer.rawToSavedObject({ ...raw, _id }); - expect(_actual).toHaveProperty('id', _id); + expect(() => singleNamespaceSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'random:foo:bar' does not start with expected prefix 'foo:'` + ); }); test(`doesn't specify namespace`, () => { @@ -385,23 +383,28 @@ describe('#rawToSavedObject', () => { describe('single-namespace type with a namespace', () => { const namespace = 'baz'; - const raw = createSampleDoc({ _source: { namespace } }); + const raw = createSampleDoc({ + _id: `${namespace}:${sampleTemplate._id}`, + _source: { namespace }, + }); const actual = singleNamespaceSerializer.rawToSavedObject(raw); test(`removes type and namespace prefix from _id`, () => { - const _id = `${namespace}:${raw._id}`; - const _actual = singleNamespaceSerializer.rawToSavedObject({ ...raw, _id }); - expect(_actual).toHaveProperty('id', 'bar'); + expect(actual).toHaveProperty('id', 'bar'); }); - test(`copies _id to id if prefixed only by type`, () => { - expect(actual).toHaveProperty('id', raw._id); + test(`fails for documents which do not have a namespace prefix in their _id`, () => { + const _id = sampleTemplate._id; + expect(() => singleNamespaceSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'foo:bar' does not start with expected prefix 'baz:foo:'` + ); }); - test(`copies _id to id if prefixed by random prefix and type`, () => { + test(`fails for documents which have any extra prefix in their _id`, () => { const _id = `random:${raw._id}`; - const _actual = singleNamespaceSerializer.rawToSavedObject({ ...raw, _id }); - expect(_actual).toHaveProperty('id', _id); + expect(() => singleNamespaceSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'random:baz:foo:bar' does not start with expected prefix 'baz:foo:'` + ); }); test(`copies _source.namespace to namespace`, () => { @@ -419,17 +422,25 @@ describe('#rawToSavedObject', () => { }); describe('multi-namespace type with a namespace', () => { - const raw = createSampleDoc({ _source: { namespace: 'baz' } }); + const raw = createSampleDoc({ _source: { namespace: 'baz' } }); // namespace should be ignored const actual = multiNamespaceSerializer.rawToSavedObject(raw); test(`removes type prefix from _id`, () => { expect(actual).toHaveProperty('id', 'bar'); }); - test(`copies _id to id if prefixed by namespace and type`, () => { + test(`fails for documents which have a namespace prefix in their _id`, () => { const _id = `${raw._source.namespace}:${raw._id}`; - const _actual = multiNamespaceSerializer.rawToSavedObject({ ...raw, _id }); - expect(_actual).toHaveProperty('id', _id); + expect(() => multiNamespaceSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'baz:foo:bar' does not start with expected prefix 'foo:'` + ); + }); + + test(`fails for documents which have any extra prefix in their _id`, () => { + const _id = `random:${raw._id}`; + expect(() => multiNamespaceSerializer.rawToSavedObject({ ...raw, _id })).toThrow( + `Raw document 'random:foo:bar' does not start with expected prefix 'foo:'` + ); }); test(`doesn't copy _source.namespace to namespace`, () => { @@ -785,7 +796,7 @@ describe('#isRawSavedObject', () => { ).toBeFalsy(); }); - test('is false if there is no [type] attribute', () => { + test('is true if there is no [type] attribute', () => { expect( singleNamespaceSerializer.isRawSavedObject({ _id: 'hello:world', @@ -794,7 +805,7 @@ describe('#isRawSavedObject', () => { jam: {}, }, }) - ).toBeFalsy(); + ).toBeTruthy(); }); }); @@ -1014,7 +1025,7 @@ describe('#isRawSavedObject', () => { ).toBeFalsy(); }); - test('is false if there is no [type] attribute', () => { + test('is true if there is no [type] attribute', () => { expect( multiNamespaceSerializer.isRawSavedObject({ _id: 'hello:world', @@ -1024,7 +1035,7 @@ describe('#isRawSavedObject', () => { namespace: 'foo', }, }) - ).toBeFalsy(); + ).toBeTruthy(); }); }); @@ -1107,7 +1118,7 @@ describe('#isRawSavedObject', () => { ).toBeFalsy(); }); - test('is false if there is no [type] attribute', () => { + test('is true if there is no [type] attribute', () => { expect( namespaceAgnosticSerializer.isRawSavedObject({ _id: 'hello:world', @@ -1117,7 +1128,7 @@ describe('#isRawSavedObject', () => { namespace: 'foo', }, }) - ).toBeFalsy(); + ).toBeTruthy(); }); }); }); diff --git a/src/core/server/saved_objects/serialization/serializer.ts b/src/core/server/saved_objects/serialization/serializer.ts index cd6c68a6de076..4b955032939b3 100644 --- a/src/core/server/saved_objects/serialization/serializer.ts +++ b/src/core/server/saved_objects/serialization/serializer.ts @@ -39,14 +39,35 @@ export class SavedObjectsSerializer { * @param {SavedObjectsRawDocParseOptions} options - Options for parsing the raw document. */ public isRawSavedObject(doc: SavedObjectsRawDoc, options: SavedObjectsRawDocParseOptions = {}) { + try { + this.checkIsRawSavedObject(doc, options); + return true; + } catch (error) { + // do nothing + } + return false; + } + + private checkIsRawSavedObject( + doc: SavedObjectsRawDoc, + options: SavedObjectsRawDocParseOptions = {} + ) { const { namespaceTreatment = 'strict' } = options; const { _id, _source } = doc; const { type, namespace } = _source; if (!type) { - return false; + throw new Error(`Raw document '${_id}' is missing _source.type field`); } - const { idMatchesPrefix } = this.parseIdPrefix(namespace, type, _id, namespaceTreatment); - return idMatchesPrefix && _source.hasOwnProperty(type); + const { idMatchesPrefix, prefix } = this.parseIdPrefix( + namespace, + type, + _id, + namespaceTreatment + ); + if (!idMatchesPrefix) { + throw new Error(`Raw document '${_id}' does not start with expected prefix '${prefix}'`); + } + return idMatchesPrefix; } /** @@ -59,6 +80,8 @@ export class SavedObjectsSerializer { doc: SavedObjectsRawDoc, options: SavedObjectsRawDocParseOptions = {} ): SavedObjectSanitizedDoc { + this.checkIsRawSavedObject(doc, options); // throws a descriptive error if the document is not a saved object + const { namespaceTreatment = 'strict' } = options; const { _id, _source, _seq_no, _primary_term } = doc; const { diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/ui_counters/data.json b/test/api_integration/fixtures/es_archiver/saved_objects/ui_counters/data.json index 80071fe422780..8192c3c161f93 100644 --- a/test/api_integration/fixtures/es_archiver/saved_objects/ui_counters/data.json +++ b/test/api_integration/fixtures/es_archiver/saved_objects/ui_counters/data.json @@ -77,7 +77,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "uiCounter:09042021:count:myApp:some_app_event", + "id": "usage-counters:uiCounter:09042021:count:myApp:some_app_event", "source": { "usage-counters": { "count": 2, @@ -95,7 +95,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "anotherDomainId:09042021:count:some_event_name", + "id": "usage-counters:anotherDomainId:09042021:count:some_event_name", "source": { "usage-counters": { "count": 2, diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/usage_counters/data.json b/test/api_integration/fixtures/es_archiver/saved_objects/usage_counters/data.json index 16e0364b24fda..cd8d63dffa04e 100644 --- a/test/api_integration/fixtures/es_archiver/saved_objects/usage_counters/data.json +++ b/test/api_integration/fixtures/es_archiver/saved_objects/usage_counters/data.json @@ -2,7 +2,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "uiCounter:20112020:count:myApp:some_app_event", + "id": "usage-counters:uiCounter:20112020:count:myApp:some_app_event", "source": { "usage-counters": { "count": 2, @@ -20,7 +20,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "anotherDomainId:20112020:count:some_event_name", + "id": "usage-counters:anotherDomainId:20112020:count:some_event_name", "source": { "usage-counters": { "count": 3, @@ -38,7 +38,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "anotherDomainId:09042021:count:some_event_name", + "id": "usage-counters:anotherDomainId:09042021:count:some_event_name", "source": { "usage-counters": { "count": 2, @@ -56,7 +56,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "anotherDomainId2:09042021:count:some_event_name", + "id": "usage-counters:anotherDomainId2:09042021:count:some_event_name", "source": { "usage-counters": { "count": 1, @@ -74,7 +74,7 @@ "type": "doc", "value": { "index": ".kibana", - "id": "anotherDomainId3:09042021:custom_type:zero_count", + "id": "usage-counters:anotherDomainId3:09042021:custom_type:zero_count", "source": { "usage-counters": { "count": 0,