Skip to content

Commit

Permalink
馃悰 Fixed 500 error in webhooks API when modifying non-existing webhooks
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@naz
naz committed Aug 3, 2020
1 parent 60907a7 commit 1b449f4
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 0 deletions.
16 changes: 16 additions & 0 deletions core/server/api/canary/webhooks.js
View file
Expand Up @@ -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', {
Expand Down Expand Up @@ -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', {
Expand Down
16 changes: 16 additions & 0 deletions core/server/api/v2/webhooks.js
View file
Expand Up @@ -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', {
Expand Down Expand Up @@ -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', {
Expand Down
17 changes: 17 additions & 0 deletions test/regression/api/canary/admin/webhooks_spec.js
View file
Expand Up @@ -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',
Expand Down
17 changes: 17 additions & 0 deletions test/regression/api/v2/admin/webhooks_spec.js
View file
Expand Up @@ -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',
Expand Down
17 changes: 17 additions & 0 deletions test/regression/api/v3/admin/webhooks_spec.js
View file
Expand Up @@ -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',
Expand Down

0 comments on commit 1b449f4

Please sign in to comment.