mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-01-06 22:40:14 -05:00
Added check for parent integration_id when creating a webhook
refs https://github.com/TryGhost/Ghost/issues/12033 refs https://github.com/TryGhost/Ghost/issues/10567 - Creating a webhook without valid parent integration leads to orphaned webhook records, which shoult not ever happen - This scenario is only possible for non-integration authentication, because in case of integration being authenticated it's id is automatically assigned to creatd webhook
This commit is contained in:
parent
bbcc83dadb
commit
6f1abc610a
3 changed files with 42 additions and 15 deletions
|
@ -14,20 +14,35 @@ module.exports = {
|
||||||
options: [],
|
options: [],
|
||||||
data: [],
|
data: [],
|
||||||
permissions: true,
|
permissions: true,
|
||||||
query(frame) {
|
async query(frame) {
|
||||||
return models.Webhook.getByEventAndTarget(
|
const isIntegrationRequest = frame.options.context && frame.options.context.integration && frame.options.context.integration.id;
|
||||||
|
|
||||||
|
// NOTE: this check can be removed once `webhooks.integration_id` gets foreigh ke constraint (Ghost 4.0)
|
||||||
|
if (!isIntegrationRequest && frame.data.webhooks[0].integration_id) {
|
||||||
|
const integration = await models.Integration.findOne({id: frame.data.webhooks[0].integration_id}, {context: {internal: true}});
|
||||||
|
|
||||||
|
if (!integration) {
|
||||||
|
throw new errors.ValidationError({
|
||||||
|
message: i18n.t('notices.data.validation.index.schemaValidationFailed', {
|
||||||
|
key: 'integration_id'
|
||||||
|
}),
|
||||||
|
context: i18n.t('errors.api.webhooks.nonExistingIntegrationIdProvided.context'),
|
||||||
|
help: i18n.t('errors.api.webhooks.nonExistingIntegrationIdProvided.help')
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const webhook = await models.Webhook.getByEventAndTarget(
|
||||||
frame.data.webhooks[0].event,
|
frame.data.webhooks[0].event,
|
||||||
frame.data.webhooks[0].target_url,
|
frame.data.webhooks[0].target_url,
|
||||||
frame.options
|
frame.options
|
||||||
).then((webhook) => {
|
|
||||||
if (webhook) {
|
|
||||||
return Promise.reject(
|
|
||||||
new errors.ValidationError({message: i18n.t('errors.api.webhooks.webhookAlreadyExists')})
|
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if (webhook) {
|
||||||
|
throw new errors.ValidationError({message: i18n.t('errors.api.webhooks.webhookAlreadyExists')});
|
||||||
}
|
}
|
||||||
|
|
||||||
return models.Webhook.add(frame.data.webhooks[0], frame.options);
|
return models.Webhook.add(frame.data.webhooks[0], frame.options);
|
||||||
});
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
|
@ -486,6 +486,10 @@
|
||||||
"noIntegrationIdProvided": {
|
"noIntegrationIdProvided": {
|
||||||
"context": "You may only create webhooks with 'integration_id' when using session authentication."
|
"context": "You may only create webhooks with 'integration_id' when using session authentication."
|
||||||
},
|
},
|
||||||
|
"nonExistingIntegrationIdProvided": {
|
||||||
|
"context": "'integration_id' value does not match any existing integration.",
|
||||||
|
"help": "Provide 'integration_id' of existing integration."
|
||||||
|
},
|
||||||
"webhookAlreadyExists": "Target URL has already been used for this event."
|
"webhookAlreadyExists": "Target URL has already been used for this event."
|
||||||
},
|
},
|
||||||
"oembed": {
|
"oembed": {
|
||||||
|
@ -711,7 +715,7 @@
|
||||||
"valueIsNotInteger": "Value in [{tableName}.{columnKey}] is not an integer.",
|
"valueIsNotInteger": "Value in [{tableName}.{columnKey}] is not an integer.",
|
||||||
"themeCannotBeActivated": "{themeName} cannot be activated because it is not currently installed.",
|
"themeCannotBeActivated": "{themeName} cannot be activated because it is not currently installed.",
|
||||||
"validationFailed": "Validation ({validationName}) failed for {key}",
|
"validationFailed": "Validation ({validationName}) failed for {key}",
|
||||||
"schemaValidationFailed": "Validation failed for '{key}'",
|
"schemaValidationFailed": "Validation failed for '{key}'.",
|
||||||
"validationFailedTypes": {
|
"validationFailedTypes": {
|
||||||
"isLength": "Value in [{tableName}.{key}] exceeds maximum length of {max} characters."
|
"isLength": "Value in [{tableName}.{key}] exceeds maximum length of {max} characters."
|
||||||
}
|
}
|
||||||
|
|
|
@ -17,18 +17,18 @@ describe('Webhooks API', function () {
|
||||||
request = supertest.agent(config.get('url'));
|
request = supertest.agent(config.get('url'));
|
||||||
})
|
})
|
||||||
.then(function () {
|
.then(function () {
|
||||||
return localUtils.doAuth(request);
|
return localUtils.doAuth(request, 'integrations');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('Can creates a webhook', function () {
|
it('Can create a webhook', function () {
|
||||||
let webhookData = {
|
let webhookData = {
|
||||||
event: 'test.create',
|
event: 'test.create',
|
||||||
target_url: 'http://example.com/webhooks/test/extra/1',
|
target_url: 'http://example.com/webhooks/test/extra/1',
|
||||||
name: 'test',
|
name: 'test',
|
||||||
secret: 'thisissecret',
|
secret: 'thisissecret',
|
||||||
api_version: 'v2',
|
api_version: 'v2',
|
||||||
integration_id: '12345'
|
integration_id: testUtils.DataGenerator.Content.integrations[0].id
|
||||||
};
|
};
|
||||||
|
|
||||||
return request.post(localUtils.API.getApiQuery('webhooks/'))
|
return request.post(localUtils.API.getApiQuery('webhooks/'))
|
||||||
|
@ -52,6 +52,14 @@ describe('Webhooks API', function () {
|
||||||
jsonResponse.webhooks[0].integration_id.should.equal(webhookData.integration_id);
|
jsonResponse.webhooks[0].integration_id.should.equal(webhookData.integration_id);
|
||||||
|
|
||||||
should.not.exist(res.headers.location);
|
should.not.exist(res.headers.location);
|
||||||
|
})
|
||||||
|
.then(() => {
|
||||||
|
return request.post(localUtils.API.getApiQuery('webhooks/'))
|
||||||
|
.set('Origin', config.get('url'))
|
||||||
|
.send({webhooks: [webhookData]})
|
||||||
|
.expect('Content-Type', /json/)
|
||||||
|
.expect('Cache-Control', testUtils.cacheRules.private)
|
||||||
|
.expect(422);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -114,7 +122,7 @@ describe('Webhooks API', function () {
|
||||||
event: 'test.create',
|
event: 'test.create',
|
||||||
// a different target_url from above is needed to avoid an "already exists" error
|
// a different target_url from above is needed to avoid an "already exists" error
|
||||||
target_url: 'http://example.com/webhooks/test/2',
|
target_url: 'http://example.com/webhooks/test/2',
|
||||||
integration_id: '123423'
|
integration_id: testUtils.DataGenerator.Content.integrations[0].id
|
||||||
};
|
};
|
||||||
|
|
||||||
// create the webhook that is to be deleted
|
// create the webhook that is to be deleted
|
||||||
|
|
Loading…
Reference in a new issue