From 1b449f4f5346009568eb3be9684bc2a7705f5ee3 Mon Sep 17 00:00:00 2001 From: Nazar Gargol Date: Mon, 3 Aug 2020 23:08:47 +1200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20500=20error=20in=20webho?= =?UTF-8?q?oks=20API=20when=20modifying=20non-existing=20webhooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #12064 - Handled permission check bug by returning 404, same way it is returned in other permissions related places when handling non-existing resource. Example - https://github.com/TryGhost/Ghost/blob/60907a7ae430647a6b639e57419c78ad7daf56f5/core/server/models/relations/authors.js#L355-L358 --- core/server/api/canary/webhooks.js | 16 ++++++++++++++++ core/server/api/v2/webhooks.js | 16 ++++++++++++++++ .../api/canary/admin/webhooks_spec.js | 17 +++++++++++++++++ test/regression/api/v2/admin/webhooks_spec.js | 17 +++++++++++++++++ test/regression/api/v3/admin/webhooks_spec.js | 17 +++++++++++++++++ 5 files changed, 83 insertions(+) diff --git a/core/server/api/canary/webhooks.js b/core/server/api/canary/webhooks.js index be39ec07bc..88fd81d1f0 100644 --- a/core/server/api/canary/webhooks.js +++ b/core/server/api/canary/webhooks.js @@ -34,6 +34,14 @@ module.exports = { if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) { return models.Webhook.findOne({id: frame.options.id}) .then((webhook) => { + if (!webhook) { + throw new errors.NotFoundError({ + message: i18n.t('errors.api.resource.resourceNotFound', { + resource: 'Webhook' + }) + }); + } + if (webhook.get('integration_id') !== frame.options.context.api_key.id) { throw new errors.NoPermissionError({ message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', { @@ -95,6 +103,14 @@ module.exports = { if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) { return models.Webhook.findOne({id: frame.options.id}) .then((webhook) => { + if (!webhook) { + throw new errors.NotFoundError({ + message: i18n.t('errors.api.resource.resourceNotFound', { + resource: 'Webhook' + }) + }); + } + if (webhook.get('integration_id') !== frame.options.context.api_key.id) { throw new errors.NoPermissionError({ message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', { diff --git a/core/server/api/v2/webhooks.js b/core/server/api/v2/webhooks.js index c0afb9dacc..6b638f917d 100644 --- a/core/server/api/v2/webhooks.js +++ b/core/server/api/v2/webhooks.js @@ -44,6 +44,14 @@ module.exports = { if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) { return models.Webhook.findOne({id: frame.options.id}) .then((webhook) => { + if (!webhook) { + throw new errors.NotFoundError({ + message: i18n.t('errors.api.resource.resourceNotFound', { + resource: 'Webhook' + }) + }); + } + if (webhook.get('integration_id') !== frame.options.context.api_key.id) { throw new errors.NoPermissionError({ message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', { @@ -105,6 +113,14 @@ module.exports = { if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) { return models.Webhook.findOne({id: frame.options.id}) .then((webhook) => { + if (!webhook) { + throw new errors.NotFoundError({ + message: i18n.t('errors.api.resource.resourceNotFound', { + resource: 'Webhook' + }) + }); + } + if (webhook.get('integration_id') !== frame.options.context.api_key.id) { throw new errors.NoPermissionError({ message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', { diff --git a/test/regression/api/canary/admin/webhooks_spec.js b/test/regression/api/canary/admin/webhooks_spec.js index cdca0ce8dd..038166c784 100644 --- a/test/regression/api/canary/admin/webhooks_spec.js +++ b/test/regression/api/canary/admin/webhooks_spec.js @@ -154,6 +154,23 @@ describe('Webhooks API (canary)', function () { }); }); + it('Integration editing non-existing webhook returns 404', function () { + return request.put(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615da/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .send({ + webhooks: [{ + name: 'Edit Test' + }] + }) + .expect(404); + }); + + it('Integration deleting non-existing webhook returns 404', function () { + return request.delete(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615db/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .expect(404); + }); + it('Cannot edit webhooks using content api keys', function () { let webhookData = { event: 'post.create', diff --git a/test/regression/api/v2/admin/webhooks_spec.js b/test/regression/api/v2/admin/webhooks_spec.js index 617f35f0ea..0aa28a7eb8 100644 --- a/test/regression/api/v2/admin/webhooks_spec.js +++ b/test/regression/api/v2/admin/webhooks_spec.js @@ -103,6 +103,23 @@ describe('Webhooks API (v2)', function () { }); }); + it('Integration editing non-existing webhook returns 404', function () { + return request.put(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615da/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .send({ + webhooks: [{ + name: 'Edit Test' + }] + }) + .expect(404); + }); + + it('Integration deleting non-existing webhook returns 404', function () { + return request.delete(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615db/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .expect(404); + }); + it('Cannot edit webhooks using content api keys', function () { let webhookData = { event: 'post.create', diff --git a/test/regression/api/v3/admin/webhooks_spec.js b/test/regression/api/v3/admin/webhooks_spec.js index 409922039e..796b160f54 100644 --- a/test/regression/api/v3/admin/webhooks_spec.js +++ b/test/regression/api/v3/admin/webhooks_spec.js @@ -103,6 +103,23 @@ describe('Webhooks API (v3)', function () { }); }); + it('Integration editing non-existing webhook returns 404', function () { + return request.put(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615da/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .send({ + webhooks: [{ + name: 'Edit Test' + }] + }) + .expect(404); + }); + + it('Integration deleting non-existing webhook returns 404', function () { + return request.delete(localUtils.API.getApiQuery(`webhooks/5f27d0287c75da744d8615db/`)) + .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[0])}`) + .expect(404); + }); + it('Cannot edit webhooks using content api keys', function () { let webhookData = { event: 'post.create',