Skip to content

Commit

Permalink
Fixed integration_id assignment for webhook when creating through API…
Browse files Browse the repository at this point in the history 
… key auth

refs 173e329

- The bug was initially introduced in referenced commit. When request is done with `api_key` context, there should always be an `integration` object associated with it - https://github.com/TryGhost/Ghost/blob/71c17539d816479e532685c06701d1ec2bd8cde5/core/server/services/permissions/parse-context.js#L36 . An `id` from `context.integration` not `context.api_key` has to be assigned to newly created webhook!
- The webhooks API is about to be declared stable in upcoming release, so no migration will be done
  • Loading branch information
@naz
naz committed Aug 4, 2020
1 parent 71c1753 commit 60ae9e8
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 16 deletions.
4 changes: 2 additions & 2 deletions core/server/api/canary/utils/serializers/input/webhooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ 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;
if (_.get(frame, 'options.context.integration.id')) {
frame.data.webhooks[0].integration_id = frame.options.context.integration.id;
}
}
};
2 changes: 1 addition & 1 deletion core/server/api/canary/utils/validators/input/webhooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const jsonSchema = require('../utils/json-schema');

module.exports = {
add(apiConfig, frame) {
if (!_.get(frame, 'options.context.api_key.id') && !_.get(frame.data, 'webhooks[0].integration_id')) {
if (!_.get(frame, 'options.context.integration.id') && !_.get(frame.data, 'webhooks[0].integration_id')) {
return Promise.reject(new errors.ValidationError({
message: i18n.t('notices.data.validation.index.schemaValidationFailed', {
key: 'integration_id'
Expand Down
8 changes: 4 additions & 4 deletions core/server/api/canary/webhooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module.exports = {
edit: {
permissions: {
before: (frame) => {
if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) {
if (frame.options.context && frame.options.context.integration && frame.options.context.integration.id) {
return models.Webhook.findOne({id: frame.options.id})
.then((webhook) => {
if (!webhook) {
Expand All @@ -42,7 +42,7 @@ module.exports = {
});
}

if (webhook.get('integration_id') !== frame.options.context.api_key.id) {
if (webhook.get('integration_id') !== frame.options.context.integration.id) {
throw new errors.NoPermissionError({
message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
method: 'edit'
Expand Down Expand Up @@ -100,7 +100,7 @@ module.exports = {
},
permissions: {
before: (frame) => {
if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) {
if (frame.options.context && frame.options.context.integration && frame.options.context.integration.id) {
return models.Webhook.findOne({id: frame.options.id})
.then((webhook) => {
if (!webhook) {
Expand All @@ -111,7 +111,7 @@ module.exports = {
});
}

if (webhook.get('integration_id') !== frame.options.context.api_key.id) {
if (webhook.get('integration_id') !== frame.options.context.integration.id) {
throw new errors.NoPermissionError({
message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
method: 'destroy'
Expand Down
4 changes: 2 additions & 2 deletions core/server/api/v2/utils/serializers/input/webhooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ 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;
if (_.get(frame, 'options.context.integration.id')) {
frame.data.webhooks[0].integration_id = frame.options.context.integration.id;
}
}
};
8 changes: 4 additions & 4 deletions core/server/api/v2/webhooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ module.exports = {
edit: {
permissions: {
before: (frame) => {
if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) {
if (frame.options.context && frame.options.context.integration && frame.options.context.integration.id) {
return models.Webhook.findOne({id: frame.options.id})
.then((webhook) => {
if (!webhook) {
Expand All @@ -52,7 +52,7 @@ module.exports = {
});
}

if (webhook.get('integration_id') !== frame.options.context.api_key.id) {
if (webhook.get('integration_id') !== frame.options.context.integration.id) {
throw new errors.NoPermissionError({
message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
method: 'edit'
Expand Down Expand Up @@ -110,7 +110,7 @@ module.exports = {
},
permissions: {
before: (frame) => {
if (frame.options.context && frame.options.context.api_key && frame.options.context.api_key.id) {
if (frame.options.context && frame.options.context.integration && frame.options.context.integration.id) {
return models.Webhook.findOne({id: frame.options.id})
.then((webhook) => {
if (!webhook) {
Expand All @@ -121,7 +121,7 @@ module.exports = {
});
}

if (webhook.get('integration_id') !== frame.options.context.api_key.id) {
if (webhook.get('integration_id') !== frame.options.context.integration.id) {
throw new errors.NoPermissionError({
message: i18n.t('errors.api.webhooks.noPermissionToEdit.message', {
method: 'destroy'
Expand Down
2 changes: 1 addition & 1 deletion test/regression/api/canary/admin/webhooks_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Webhooks API (canary)', function () {

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);
jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].integration_id);
jsonResponse.webhooks[0].name.should.eql('test');
jsonResponse.webhooks[0].secret.should.eql('thisissecret');
jsonResponse.webhooks[0].api_version.should.eql('v3');
Expand Down
2 changes: 1 addition & 1 deletion test/regression/api/v2/admin/webhooks_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Webhooks API (v2)', function () {

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);
jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].integration_id);

localUtils.API.checkResponse(jsonResponse.webhooks[0], 'webhook');
});
Expand Down
2 changes: 1 addition & 1 deletion test/regression/api/v3/admin/webhooks_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Webhooks API (v3)', function () {

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);
jsonResponse.webhooks[0].integration_id.should.eql(testUtils.DataGenerator.Content.api_keys[0].integration_id);

localUtils.API.checkResponse(jsonResponse.webhooks[0], 'webhook');
});
Expand Down

0 comments on commit 60ae9e8

Please sign in to comment.