Prevent creating saved objects with empty IDs (#120693) (#120914)

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>
This commit is contained in:
Kibana Machine 2021-12-09 10:53:17 -05:00 committed by GitHub
parent cff37e2917
commit b3a3c626c9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 2 deletions

View file

@ -2272,7 +2272,16 @@ describe('SavedObjectsRepository', () => {
it(`self-generates an id if none is provided`, async () => {
await createSuccess(type, attributes);
expect(client.create).toHaveBeenCalledWith(
expect(client.create).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
id: expect.objectContaining(/index-pattern:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/),
}),
expect.anything()
);
await createSuccess(type, attributes, { id: '' });
expect(client.create).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
id: expect.objectContaining(/index-pattern:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/),
}),
@ -4161,6 +4170,13 @@ describe('SavedObjectsRepository', () => {
await test({});
});
it(`throws when id is empty`, async () => {
await expect(
savedObjectsRepository.incrementCounter(type, '', counterFields)
).rejects.toThrowError(createBadRequestError('id cannot be empty'));
expect(client.update).not.toHaveBeenCalled();
});
it(`throws when counterField is not CounterField type`, async () => {
const test = async (field: unknown[]) => {
await expect(
@ -4687,6 +4703,13 @@ describe('SavedObjectsRepository', () => {
expect(client.update).not.toHaveBeenCalled();
});
it(`throws when id is empty`, async () => {
await expect(savedObjectsRepository.update(type, '', attributes)).rejects.toThrowError(
createBadRequestError('id cannot be empty')
);
expect(client.update).not.toHaveBeenCalled();
});
it(`throws when ES is unable to find the document during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(

View file

@ -303,7 +303,6 @@ export class SavedObjectsRepository {
options: SavedObjectsCreateOptions = {}
): Promise<SavedObject<T>> {
const {
id = SavedObjectsUtils.generateId(),
migrationVersion,
coreMigrationVersion,
overwrite = false,
@ -313,6 +312,7 @@ export class SavedObjectsRepository {
initialNamespaces,
version,
} = options;
const id = options.id || SavedObjectsUtils.generateId();
const namespace = normalizeNamespace(options.namespace);
this.validateInitialNamespaces(type, initialNamespaces);
@ -1231,6 +1231,9 @@ export class SavedObjectsRepository {
if (!this._allowedTypes.includes(type)) {
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
}
if (!id) {
throw SavedObjectsErrorHelpers.createBadRequestError('id cannot be empty'); // prevent potentially upserting a saved object with an empty ID
}
const { version, references, upsert, refresh = DEFAULT_REFRESH_SETTING } = options;
const namespace = normalizeNamespace(options.namespace);
@ -1754,6 +1757,10 @@ export class SavedObjectsRepository {
upsertAttributes,
} = options;
if (!id) {
throw SavedObjectsErrorHelpers.createBadRequestError('id cannot be empty'); // prevent potentially upserting a saved object with an empty ID
}
const normalizedCounterFields = counterFields.map((counterField) => {
/**
* no counterField configs provided, instead a field name string was passed.