0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-11 02:12:21 -05:00

Fixed handling requests with mismatching version and missing key

fix https://linear.app/tryghost/issue/SLO-88/typeerror-cannot-read-properties-of-null-reading-relations

- in the event that we make it through the version mismatch code, but
  without a key, which is possible if you send a request like POST
  /ghost/api/v2/content/posts/`, then the version mismatch code will try
  and look up the API key attached to a null key, which won't work
- we should handle this case and soft return, to avoid trying to read
  `.relations` from `null`
- I'm not entirely convinced by how this code works in general, it seems
  quite confusing to reason about, but this commit should solve the HTTP
  500 we've been seeing from this
- perhaps in the future we can return earlier in the flow if we receive
  a `null` key
This commit is contained in:
Daniel Lockyer 2024-05-02 12:51:27 +02:00 committed by Daniel Lockyer
parent ec626bd0cf
commit 6c7b230efe
4 changed files with 71 additions and 2 deletions

View file

@ -41,10 +41,17 @@ class APIVersionCompatibilityService {
*/ */
async handleMismatch({acceptVersion, contentVersion, apiKeyValue, apiKeyType, requestURL, userAgent = ''}) { async handleMismatch({acceptVersion, contentVersion, apiKeyValue, apiKeyType, requestURL, userAgent = ''}) {
if (!await this.versionNotificationsDataService.fetchNotification(acceptVersion)) { if (!await this.versionNotificationsDataService.fetchNotification(acceptVersion)) {
const integration = await this.versionNotificationsDataService.getIntegration(apiKeyValue, apiKeyType);
// We couldn't find the integration
if (!integration) {
return;
}
const { const {
name: integrationName, name: integrationName,
type: integrationType type: integrationType
} = await this.versionNotificationsDataService.getIntegration(apiKeyValue, apiKeyType); } = integration;
// @NOTE: "internal" or "core" integrations (https://ghost.notion.site/Data-Types-e5dc54dd0078443f9afd6b2abda443c4) // @NOTE: "internal" or "core" integrations (https://ghost.notion.site/Data-Types-e5dc54dd0078443f9afd6b2abda443c4)
// are maintained by Ghost team, so there is no sense notifying the instance owner about it's incompatibility. // are maintained by Ghost team, so there is no sense notifying the instance owner about it's incompatibility.

View file

@ -184,6 +184,43 @@ describe('APIVersionCompatibilityService', function () {
assert.equal(sendEmail.calledTwice, false); assert.equal(sendEmail.calledTwice, false);
}); });
it('Does not send email when unknown integration is detected', async function () {
const sendEmail = sinon.spy();
const findOneStub = sinon.stub();
findOneStub
.withArgs({
id: 'unknown_key'
}, {
withRelated: ['integration']
})
.resolves(null);
ApiKeyModel = {
findOne: findOneStub
};
const compatibilityService = new APIVersionCompatibilityService({
sendEmail,
ApiKeyModel,
UserModel,
settingsService,
getSiteUrl,
getSiteTitle
});
await compatibilityService.handleMismatch({
acceptVersion: 'v4.5',
contentVersion: 'v5.1',
userAgent: 'GhostAdminSDK/2.4.0',
requestURL: 'https://amazeballsghostsite.com/ghost/api/admin/posts/dew023d9203se4',
apiKeyValue: 'unknown_key',
apiKeyType: 'content'
});
assert.equal(sendEmail.called, false);
});
it('Does NOT send an email to the instance owner when "internal" or "core" integration triggered version mismatch', async function () { it('Does NOT send an email to the instance owner when "internal" or "core" integration triggered version mismatch', async function () {
const sendEmail = sinon.spy(); const sendEmail = sinon.spy();
const findOneStub = sinon.stub(); const findOneStub = sinon.stub();

View file

@ -57,7 +57,7 @@ class VersionNotificationsDataService {
* *
* @param {String} key - api key identification value, it's "secret" in case of Content API key and "id" for Admin API * @param {String} key - api key identification value, it's "secret" in case of Content API key and "id" for Admin API
* @param {String} type - one of "content" or "admin" values * @param {String} type - one of "content" or "admin" values
* @returns {Promise<Object>} Integration JSON object * @returns {Promise<Object | null>} Integration JSON object
*/ */
async getIntegration(key, type) { async getIntegration(key, type) {
let queryOptions = null; let queryOptions = null;
@ -69,6 +69,9 @@ class VersionNotificationsDataService {
} }
const apiKey = await this.ApiKeyModel.findOne(queryOptions, {withRelated: ['integration']}); const apiKey = await this.ApiKeyModel.findOne(queryOptions, {withRelated: ['integration']});
if (!apiKey) {
return null;
}
return apiKey.relations.integration.toJSON(); return apiKey.relations.integration.toJSON();
} }

View file

@ -190,5 +190,27 @@ describe('Version Notification Data Service', function () {
assert.equal(integrationName, 'Tri Hita Karana'); assert.equal(integrationName, 'Tri Hita Karana');
assert.equal(integrationType, 'core'); assert.equal(integrationType, 'core');
}); });
it('Returns null when the api-key/integration is not found', async function () {
const ApiKeyModel = {
findOne: sinon
.stub()
.withArgs({
id: 'key_id'
}, {
withRelated: ['integration']
})
.resolves(null)
};
const versionNotificationsDataService = new VersionNotificationsDataService({
UserModel: {},
ApiKeyModel,
settingsService: {}
});
const integration = await versionNotificationsDataService.getIntegration('key_id', 'admin');
assert.equal(integration, null);
});
}); });
}); });