Fixed integration_id handling in Webhooks Admin API

no issue

- Changes introduced to both API v3 and v2
- Makes sure to use the same integration_id as authenticated integration for the webhook's data.
-  Makde it is impossible to create orphaned webhooks using token authentication
- Allowed only parent integration to edit it's children webhooks. Throwing permission error otherwise

core/server/api/canary/utils/serializers/input/index.js

@@ -29,5 +29,9 @@ module.exports = {
 
     get members() {
         return require('./members');
+    },
+
+    get webhooks() {
+        return require('./webhooks');
     }
 };

core/server/api/canary/utils/serializers/input/webhooks.js

@@ -0,0 +1,12 @@
+const _ = require('lodash');
+const debug = require('ghost-ignition').debug('api:canary:utils:serializers:input:webhooks');
+
+module.exports = {
+    add(apiConfig, frame) {
+        debug('add');
+
+        if (_.get(frame, 'options.context.api_key.id')) {
+            frame.data.webhooks[0].integration_id = frame.options.context.api_key.id;
+        }
+    }
+};

core/server/api/canary/webhooks.js

@@ -39,7 +39,25 @@ module.exports = {
     },
 
     edit: {
-        permissions: true,
+        permissions: {
+            before: (frame) => {
+                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.get('integration_id') !== frame.options.context.api_key.id) {
+                                throw new errors.NoPermissionError({
+                                    message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
+                                        method: 'edit'
+                                    }),
+                                    context: i18n.t('errors.api.webhooks.noPermissionToEdit.context', {
+                                        method: 'edit'
+                                    })
+                                });
+                            }
+                        });
+                }
+            }
+        },
         data: [
             'name',
             'event',
@@ -82,7 +100,25 @@ module.exports = {
                 }
             }
         },
-        permissions: true,
+        permissions: {
+            before: (frame) => {
+                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.get('integration_id') !== frame.options.context.api_key.id) {
+                                throw new errors.NoPermissionError({
+                                    message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
+                                        method: 'destory'
+                                    }),
+                                    context: i18n.t('errors.api.webhooks.noPermissionToEdit.context', {
+                                        method: 'destroy'
+                                    })
+                                });
+                            }
+                        });
+                }
+            }
+        },
         query(frame) {
             frame.options.require = true;
 

core/server/api/v2/utils/serializers/input/index.js

@@ -25,5 +25,9 @@ module.exports = {
 
     get tags() {
         return require('./tags');
+    },
+
+    get webhooks() {
+        return require('./webhooks');
     }
 };

core/server/api/v2/utils/serializers/input/webhooks.js

@@ -0,0 +1,12 @@
+const _ = require('lodash');
+const debug = require('ghost-ignition').debug('api:canary:utils:serializers:input:webhooks');
+
+module.exports = {
+    add(apiConfig, frame) {
+        debug('add');
+
+        if (_.get(frame, 'options.context.api_key.id')) {
+            frame.data.webhooks[0].integration_id = frame.options.context.api_key.id;
+        }
+    }
+};

core/server/api/v2/webhooks.js

@@ -39,7 +39,25 @@ module.exports = {
     },
 
     edit: {
-        permissions: true,
+        permissions: {
+            before: (frame) => {
+                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.get('integration_id') !== frame.options.context.api_key.id) {
+                                throw new errors.NoPermissionError({
+                                    message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
+                                        method: 'edit'
+                                    }),
+                                    context: i18n.t('errors.api.webhooks.noPermissionToEdit.context', {
+                                        method: 'edit'
+                                    })
+                                });
+                            }
+                        });
+                }
+            }
+        },
         data: [
             'name',
             'event',
@@ -82,7 +100,25 @@ module.exports = {
                 }
             }
         },
-        permissions: true,
+        permissions: {
+            before: (frame) => {
+                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.get('integration_id') !== frame.options.context.api_key.id) {
+                                throw new errors.NoPermissionError({
+                                    message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
+                                        method: 'destory'
+                                    }),
+                                    context: i18n.t('errors.api.webhooks.noPermissionToEdit.context', {
+                                        method: 'destroy'
+                                    })
+                                });
+                            }
+                        });
+                }
+            }
+        },
         query(frame) {
             frame.options.require = true;
             return models.Webhook.destroy(frame.options)

core/server/translations/en.json

@@ -448,6 +448,10 @@
                 "notAllowedToInvite": "Not allowed to invite this role."
             },
             "webhooks": {
+                "noPermissionToEdit": {
+                    "message": "You do not have permission to {method} this webhook.",
+                    "context": "You may only {method} webhooks that belong to the authenticated integration. Check the supplied Admin API Key."
+                },
                 "webhookAlreadyExists": "Target URL has already been used for this event."
             },
             "oembed": {

core/server/web/api/canary/admin/middleware.js

@@ -25,7 +25,7 @@ const notImplemented = function (req, res, next) {
         themes: ['POST', 'PUT'],
         members: ['GET', 'PUT', 'DELETE', 'POST'],
         config: ['GET'],
-        webhooks: ['POST', 'DELETE'],
+        webhooks: ['POST', 'PUT', 'DELETE'],
         schedules: ['PUT'],
         db: ['POST']
     };

core/server/web/api/v2/admin/middleware.js

@@ -22,7 +22,7 @@ const notImplemented = function (req, res, next) {
         users: ['GET'],
         themes: ['POST', 'PUT'],
         config: ['GET'],
-        webhooks: ['POST', 'DELETE'],
+        webhooks: ['POST', 'PUT', 'DELETE'],
         schedules: ['PUT'],
         db: ['POST']
     };

test/regression/api/canary/admin/webhooks_spec.js

@@ -0,0 +1,119 @@
+const should = require('should');
+const supertest = require('supertest');
+const testUtils = require('../../../../utils');
+const config = require('../../../../../core/shared/config');
+const localUtils = require('./utils');
+
+const ghost = testUtils.startGhost;
+
+describe('Webhooks API (canary)', function () {
+    let request;
+
+    before(function () {
+        return ghost()
+            .then(function () {
+                request = supertest.agent(config.get('url'));
+            })
+            .then(function () {
+                return localUtils.doAuth(request, 'api_keys', 'webhooks');
+            });
+    });
+
+    it('Can create a webhook using integration', function () {
+        let webhookData = {
+            event: 'test.create',
+            target_url: 'http://example.com/webhooks/test/extra/canary',
+            integration_id: 'ignore_me',
+            name: 'test',
+            secret: 'thisissecret',
+            api_version: 'v3'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(201)
+            .then((res) => {
+                should.not.exist(res.headers['x-cache-invalidate']);
+                const jsonResponse = res.body;
+
+                should.exist(jsonResponse);
+                should.exist(jsonResponse.webhooks);
+                should.exist(jsonResponse.webhooks[0].event);
+                should.exist(jsonResponse.webhooks[0].target_url);
+
+                jsonResponse.webhooks[0].event.should.eql('test.create');
+                jsonResponse.webhooks[0].target_url.should.eql('http://example.com/webhooks/test/extra/canary');
+                jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].id);
+
+                localUtils.API.checkResponse(jsonResponse.webhooks[0], 'webhook');
+            });
+    });
+
+    it('Integration cannot edit or delete other integration\'s webhook', function () {
+        let createdIntegration;
+        let createdWebhook;
+
+        return Promise.resolve()
+            .then(() => {
+                return request.post(localUtils.API.getApiQuery('integrations/'))
+                    .set('Origin', config.get('url'))
+                    .send({
+                        integrations: [{
+                            name: 'Rubbish Integration Name'
+                        }]
+                    })
+                    .expect(201)
+                    .then(({body}) => {
+                        [createdIntegration] = body.integrations;
+
+                        return 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);
+                    });
+            })
+            .then(({body}) => {
+                [createdWebhook] = body.webhooks;
+
+                return request.put(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .send({
+                        webhooks: [{
+                            name: 'Edit Test',
+                            event: 'subscriber.added',
+                            target_url: 'https://example.com/new-subscriber'
+                        }]
+                    })
+                    .expect(403);
+            })
+            .then(() => {
+                return request.del(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .expect(403);
+            });
+    });
+
+    it('Cannot edit webhooks using content api keys', function () {
+        let webhookData = {
+            event: 'post.create',
+            target_url: 'http://example.com/webhooks/test/extra/2'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/canary/admin/', testUtils.DataGenerator.Content.api_keys[1])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(401);
+    });
+});

test/regression/api/v2/admin/webhooks_spec.js

@@ -0,0 +1,119 @@
+const should = require('should');
+const supertest = require('supertest');
+const testUtils = require('../../../../utils');
+const config = require('../../../../../core/shared/config');
+const localUtils = require('./utils');
+
+const ghost = testUtils.startGhost;
+
+describe('Webhooks API (v2)', function () {
+    let request;
+
+    before(function () {
+        return ghost()
+            .then(function () {
+                request = supertest.agent(config.get('url'));
+            })
+            .then(function () {
+                return localUtils.doAuth(request, 'api_keys', 'webhooks');
+            });
+    });
+
+    it('Can create a webhook using integration', function () {
+        let webhookData = {
+            event: 'test.create',
+            target_url: 'http://example.com/webhooks/test/extra/v2',
+            integration_id: 'ignore_me',
+            name: 'test',
+            secret: 'thisissecret',
+            api_version: 'v2'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(201)
+            .then((res) => {
+                should.not.exist(res.headers['x-cache-invalidate']);
+                const jsonResponse = res.body;
+
+                should.exist(jsonResponse);
+                should.exist(jsonResponse.webhooks);
+                should.exist(jsonResponse.webhooks[0].event);
+                should.exist(jsonResponse.webhooks[0].target_url);
+
+                jsonResponse.webhooks[0].event.should.eql('test.create');
+                jsonResponse.webhooks[0].target_url.should.eql('http://example.com/webhooks/test/extra/v2');
+                jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].id);
+
+                localUtils.API.checkResponse(jsonResponse.webhooks[0], 'webhook');
+            });
+    });
+
+    it('Integration cannot edit or delete other integration\'s webhook', function () {
+        let createdIntegration;
+        let createdWebhook;
+
+        return Promise.resolve()
+            .then(() => {
+                return request.post(localUtils.API.getApiQuery('integrations/'))
+                    .set('Origin', config.get('url'))
+                    .send({
+                        integrations: [{
+                            name: 'Rubbish Integration Name'
+                        }]
+                    })
+                    .expect(201)
+                    .then(({body}) => {
+                        [createdIntegration] = body.integrations;
+
+                        return 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);
+                    });
+            })
+            .then(({body}) => {
+                [createdWebhook] = body.webhooks;
+
+                return request.put(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .send({
+                        webhooks: [{
+                            name: 'Edit Test',
+                            event: 'subscriber.added',
+                            target_url: 'https://example.com/new-subscriber'
+                        }]
+                    })
+                    .expect(403);
+            })
+            .then(() => {
+                return request.del(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .expect(403);
+            });
+    });
+
+    it('Cannot edit webhooks using content api keys', function () {
+        let webhookData = {
+            event: 'post.create',
+            target_url: 'http://example.com/webhooks/test/extra/2'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v2/admin/', testUtils.DataGenerator.Content.api_keys[1])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(401);
+    });
+});

test/regression/api/v3/admin/webhooks_spec.js

@@ -0,0 +1,119 @@
+const should = require('should');
+const supertest = require('supertest');
+const testUtils = require('../../../../utils');
+const config = require('../../../../../core/shared/config');
+const localUtils = require('./utils');
+
+const ghost = testUtils.startGhost;
+
+describe('Webhooks API (v3)', function () {
+    let request;
+
+    before(function () {
+        return ghost()
+            .then(function () {
+                request = supertest.agent(config.get('url'));
+            })
+            .then(function () {
+                return localUtils.doAuth(request, 'api_keys', 'webhooks');
+            });
+    });
+
+    it('Can create a webhook using integration', function () {
+        let webhookData = {
+            event: 'test.create',
+            target_url: 'http://example.com/webhooks/test/extra/v3',
+            integration_id: 'ignore_me',
+            name: 'test',
+            secret: 'thisissecret',
+            api_version: 'v3'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(201)
+            .then((res) => {
+                should.not.exist(res.headers['x-cache-invalidate']);
+                const jsonResponse = res.body;
+
+                should.exist(jsonResponse);
+                should.exist(jsonResponse.webhooks);
+                should.exist(jsonResponse.webhooks[0].event);
+                should.exist(jsonResponse.webhooks[0].target_url);
+
+                jsonResponse.webhooks[0].event.should.eql('test.create');
+                jsonResponse.webhooks[0].target_url.should.eql('http://example.com/webhooks/test/extra/v3');
+                jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].id);
+
+                localUtils.API.checkResponse(jsonResponse.webhooks[0], 'webhook');
+            });
+    });
+
+    it('Integration cannot edit or delete other integration\'s webhook', function () {
+        let createdIntegration;
+        let createdWebhook;
+
+        return Promise.resolve()
+            .then(() => {
+                return request.post(localUtils.API.getApiQuery('integrations/'))
+                    .set('Origin', config.get('url'))
+                    .send({
+                        integrations: [{
+                            name: 'Rubbish Integration Name'
+                        }]
+                    })
+                    .expect(201)
+                    .then(({body}) => {
+                        [createdIntegration] = body.integrations;
+
+                        return 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);
+                    });
+            })
+            .then(({body}) => {
+                [createdWebhook] = body.webhooks;
+
+                return request.put(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .send({
+                        webhooks: [{
+                            name: 'Edit Test',
+                            event: 'subscriber.added',
+                            target_url: 'https://example.com/new-subscriber'
+                        }]
+                    })
+                    .expect(403);
+            })
+            .then(() => {
+                return request.del(localUtils.API.getApiQuery(`webhooks/${createdWebhook.id}/`))
+                    .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[0])}`)
+                    .expect(403);
+            });
+    });
+
+    it('Cannot edit webhooks using content api keys', function () {
+        let webhookData = {
+            event: 'post.create',
+            target_url: 'http://example.com/webhooks/test/extra/2'
+        };
+
+        return request.post(localUtils.API.getApiQuery('webhooks/'))
+            .set('Authorization', `Ghost ${localUtils.getValidAdminToken('/v3/admin/', testUtils.DataGenerator.Content.api_keys[1])}`)
+            .send({webhooks: [webhookData]})
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(401);
+    });
+});