From 71f0c08a34cf56a5501ffb0596049dd024d7a835 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Fri, 19 Oct 2018 18:35:55 +0100 Subject: [PATCH] Added edit webhook route to v2 Admin API (#10033) no issue - webhooks UI requires the ability to edit webhooks - added `edit` permission for `webhook` - added `edit` method to v2 webhook controller - added `PUT /webhooks/:id` route to v2 Admin API routes --- core/server/api/v2/webhooks.js | 30 +++++++++ .../2.3/2-add-webhook-edit-permission.js | 51 +++++++++++++++ .../server/data/schema/fixtures/fixtures.json | 5 ++ core/server/translations/en.json | 2 +- core/server/web/api/v2/admin/routes.js | 1 + .../functional/api/v2/admin/webhooks_spec.js | 62 +++++++++++++++++++ core/test/integration/migration_spec.js | 30 ++++----- core/test/unit/data/schema/integrity_spec.js | 2 +- 8 files changed, 167 insertions(+), 16 deletions(-) create mode 100644 core/server/data/migrations/versions/2.3/2-add-webhook-edit-permission.js diff --git a/core/server/api/v2/webhooks.js b/core/server/api/v2/webhooks.js index e1a182d494..cb96d1235c 100644 --- a/core/server/api/v2/webhooks.js +++ b/core/server/api/v2/webhooks.js @@ -24,6 +24,36 @@ module.exports = { }); } }, + edit: { + permissions: true, + data: [ + 'name', + 'event', + 'target_url', + 'secret', + 'api_version' + ], + options: [ + 'id' + ], + validation: { + options: { + id: { + required: true + } + } + }, + query({data, options}) { + return models.Webhook.edit(data.webhooks[0], Object.assign(options, {require: true})) + .catch(models.Webhook.NotFoundError, () => { + throw new common.errors.NotFoundError({ + message: common.i18n.t('errors.api.resource.resourceNotFound', { + resource: 'Webhook' + }) + }); + }); + } + }, destroy: { statusCode: 204, headers: {}, diff --git a/core/server/data/migrations/versions/2.3/2-add-webhook-edit-permission.js b/core/server/data/migrations/versions/2.3/2-add-webhook-edit-permission.js new file mode 100644 index 0000000000..87ddec04b0 --- /dev/null +++ b/core/server/data/migrations/versions/2.3/2-add-webhook-edit-permission.js @@ -0,0 +1,51 @@ +const _ = require('lodash'); +const utils = require('../../../schema/fixtures/utils'); +const permissions = require('../../../../services/permissions'); +const logging = require('../../../../lib/common/logging'); + +const resources = ['webhook']; +const _private = {}; + +_private.getPermissions = function getPermissions(resource) { + return utils.findModelFixtures('Permission', {object_type: resource}); +}; + +_private.printResult = function printResult(result, message) { + if (result.done === result.expected) { + logging.info(message); + } else { + logging.warn(`(${result.done}/${result.expected}) ${message}`); + } +}; + +module.exports.config = { + transaction: true +}; + +module.exports.up = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToAdd = _private.getPermissions(resource); + + return utils.addFixturesForModel(modelToAdd, localOptions) + .then(result => _private.printResult(result, `Adding permissions fixtures for ${resource}s`)) + .then(() => permissions.init(localOptions)); + }); +}; + +module.exports.down = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToRemove = _private.getPermissions(resource); + + // permission model automatically cleans up permissions_roles on .destroy() + return utils.removeFixturesForModel(modelToRemove, localOptions) + .then(result => _private.printResult(result, `Removing permissions fixtures for ${resource}s`)); + }); +}; diff --git a/core/server/data/schema/fixtures/fixtures.json b/core/server/data/schema/fixtures/fixtures.json index 25020c7c91..4d819d7305 100644 --- a/core/server/data/schema/fixtures/fixtures.json +++ b/core/server/data/schema/fixtures/fixtures.json @@ -332,6 +332,11 @@ "action_type": "add", "object_type": "webhook" }, + { + "name": "Edit webhooks", + "action_type": "edit", + "object_type": "webhook" + }, { "name": "Delete webhooks", "action_type": "destroy", diff --git a/core/server/translations/en.json b/core/server/translations/en.json index f8270401b5..9c1c76a42e 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -447,7 +447,7 @@ "notAllowedToInvite": "Not allowed to invite this role." }, "webhooks": { - "webhookAlreadyExists": "A webhook for requested event with supplied target_url already exists." + "webhookAlreadyExists": "Target URL has already been used for this event." }, "oembed": { "noUrlProvided": "No url provided.", diff --git a/core/server/web/api/v2/admin/routes.js b/core/server/web/api/v2/admin/routes.js index 2e9c1d2c1d..801460e35b 100644 --- a/core/server/web/api/v2/admin/routes.js +++ b/core/server/web/api/v2/admin/routes.js @@ -222,6 +222,7 @@ module.exports = function apiRoutes() { // ## Webhooks (RESTHooks) router.post('/webhooks', mw.authAdminAPI, apiv2.http(apiv2.webhooks.add)); + router.put('/webhooks/:id', mw.authAdminAPI, apiv2.http(apiv2.webhooks.edit)); router.del('/webhooks/:id', mw.authAdminAPI, apiv2.http(apiv2.webhooks.destroy)); // ## Oembed (fetch response from oembed provider) diff --git a/core/test/functional/api/v2/admin/webhooks_spec.js b/core/test/functional/api/v2/admin/webhooks_spec.js index 0762ef369b..201d0a6f10 100644 --- a/core/test/functional/api/v2/admin/webhooks_spec.js +++ b/core/test/functional/api/v2/admin/webhooks_spec.js @@ -90,6 +90,68 @@ describe('Webhooks API', function () { }); }); + describe('Edit', function () { + it('can succesfully edit a webhook', function (done) { + request.post(localUtils.API.getApiQuery('integrations/')) + .set('Origin', config.get('url')) + .send({ + integrations: [{ + name: 'Rubbish Integration Name' + }] + }) + .expect(201) + .end(function (err, {body}) { + if (err) { + return done(err); + } + + const [createdIntegration] = body.integrations; + + request.post(localUtils.API.getApiQuery('webhooks/')) + .set('Origin', config.get('url')) + .send({ + webhooks: [{ + name: 'Testing', + event: 'site.changed', + target_url: 'https://example.com/rebuild', + integration_id: createdIntegration.id + }] + }) + .expect(201) + .end(function (err, {body}) { + if (err) { + return done(err); + } + + const [createdWebhook] = body.webhooks; + + request.put(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`)) + .set('Origin', config.get('url')) + .send({ + webhooks: [{ + name: 'Edit Test', + event: 'subscriber.added', + target_url: 'https://example.com/new-subscriber' + }] + }) + .expect(200) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .then(({body}) => { + const [updatedWebhook] = body.webhooks; + + should.equal(updatedWebhook.id, createdWebhook.id); + should.equal(updatedWebhook.name, 'Edit Test'); + should.equal(updatedWebhook.event, 'subscriber.added'); + should.equal(updatedWebhook.target_url, 'https://example.com/new-subscriber'); + should.equal(updatedWebhook.integration_id, createdIntegration.id); + done(); + }); + }); + }); + }); + }); + describe('Delete', function () { var newWebhook = { event: 'test.create', diff --git a/core/test/integration/migration_spec.js b/core/test/integration/migration_spec.js index 15a22b5521..dc17192f91 100644 --- a/core/test/integration/migration_spec.js +++ b/core/test/integration/migration_spec.js @@ -167,32 +167,34 @@ describe('Database Migration (special functions)', function () { // Webhooks permissions[51].name.should.eql('Add webhooks'); permissions[51].should.be.AssignedToRoles(['Administrator', 'Admin Integration']); - permissions[52].name.should.eql('Delete webhooks'); + permissions[52].name.should.eql('Edit webhooks'); permissions[52].should.be.AssignedToRoles(['Administrator', 'Admin Integration']); + permissions[53].name.should.eql('Delete webhooks'); + permissions[53].should.be.AssignedToRoles(['Administrator', 'Admin Integration']); // Integrations - permissions[53].name.should.eql('Browse integrations'); - permissions[53].should.be.AssignedToRoles(['Administrator']); - permissions[54].name.should.eql('Read integrations'); + permissions[54].name.should.eql('Browse integrations'); permissions[54].should.be.AssignedToRoles(['Administrator']); - permissions[55].name.should.eql('Edit integrations'); + permissions[55].name.should.eql('Read integrations'); permissions[55].should.be.AssignedToRoles(['Administrator']); - permissions[56].name.should.eql('Add integrations'); + permissions[56].name.should.eql('Edit integrations'); permissions[56].should.be.AssignedToRoles(['Administrator']); - permissions[57].name.should.eql('Delete integrations'); + permissions[57].name.should.eql('Add integrations'); permissions[57].should.be.AssignedToRoles(['Administrator']); + permissions[58].name.should.eql('Delete integrations'); + permissions[58].should.be.AssignedToRoles(['Administrator']); // API Keys - permissions[58].name.should.eql('Browse API keys'); - permissions[58].should.be.AssignedToRoles(['Administrator']); - permissions[59].name.should.eql('Read API keys'); + permissions[59].name.should.eql('Browse API keys'); permissions[59].should.be.AssignedToRoles(['Administrator']); - permissions[60].name.should.eql('Edit API keys'); + permissions[60].name.should.eql('Read API keys'); permissions[60].should.be.AssignedToRoles(['Administrator']); - permissions[61].name.should.eql('Add API keys'); + permissions[61].name.should.eql('Edit API keys'); permissions[61].should.be.AssignedToRoles(['Administrator']); - permissions[62].name.should.eql('Delete API keys'); + permissions[62].name.should.eql('Add API keys'); permissions[62].should.be.AssignedToRoles(['Administrator']); + permissions[63].name.should.eql('Delete API keys'); + permissions[63].should.be.AssignedToRoles(['Administrator']); }); describe('Populate', function () { @@ -257,7 +259,7 @@ describe('Database Migration (special functions)', function () { result.roles.at(5).get('name').should.eql('Admin Integration'); // Permissions - result.permissions.length.should.eql(63); + result.permissions.length.should.eql(64); result.permissions.toJSON().should.be.CompletePermissions(); }); }); diff --git a/core/test/unit/data/schema/integrity_spec.js b/core/test/unit/data/schema/integrity_spec.js index 3a131b140f..eb37b67bb9 100644 --- a/core/test/unit/data/schema/integrity_spec.js +++ b/core/test/unit/data/schema/integrity_spec.js @@ -20,7 +20,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = '92cb4391c426520d2e3e80c46f6ae100'; - const currentFixturesHash = '20292edf9fd692cbd6485267a2ac8e75'; + const currentFixturesHash = '6723e0af9b55c4c2854120eab1c29ab9'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation