From 27e4523aec3790d2280fbc267d178e145f8b9079 Mon Sep 17 00:00:00 2001 From: Chris Raible Date: Thu, 4 May 2023 16:04:58 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Improved=20error=20message=20for?= =?UTF-8?q?=20unauthorized=20YouTube=20embeds=20(#16374)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs TryGhost/Ghost#16048 - When attempting to embed a Youtube video that has had embedding disabled by its owner/author, Ghost displayed a generic error message that didn't indicate the reason for the failed emebed. - This change updated the error message when Youtube (or any provider) returns 401: Unauthorized to indicate that the owner of the resource has explicitly disabled embedding. --- ghost/admin/.lint-todo | 3 ++ .../addon/components/koenig-card-embed.hbs | 9 ++++++ .../addon/components/koenig-card-embed.js | 6 ++++ .../server/services/oembed/twitter-embed.js | 2 +- ghost/core/package.json | 2 +- ghost/core/test/e2e-api/admin/oembed.test.js | 31 +++++++++++++++++++ ghost/oembed-service/lib/oembed-service.js | 19 ++++++++---- package.json | 1 - yarn.lock | 9 +++++- 9 files changed, 72 insertions(+), 10 deletions(-) diff --git a/ghost/admin/.lint-todo b/ghost/admin/.lint-todo index 1790d5edfa..b0f3d880b4 100644 --- a/ghost/admin/.lint-todo +++ b/ghost/admin/.lint-todo @@ -578,3 +578,6 @@ add|ember-template-lint|no-action|25|125|25|125|ba0f8b6ba2697f1b071200d1a3dae9c3 add|ember-template-lint|no-action|48|46|48|46|2f3118270fbb1ff6e5da6b0d482ccd21e69df3b5|1681862400000|1692230400000|1697414400000|app/components/modal-post-history.hbs add|ember-template-lint|require-valid-alt-text|13|20|13|20|41dff435a7aba8088be689c6d9b1e76bef081d17|1682035200000|1692403200000|1697587200000|app/components/modal-post-history.hbs add|ember-template-lint|require-valid-alt-text|20|20|20|20|bc0bb4f51567cea7289bfcb30d02932f0f57d0d9|1682035200000|1692403200000|1697587200000|app/components/modal-post-history.hbs +add|ember-template-lint|no-action|55|71|55|71|76726a13a086d82dab219df12e86db1773a9de32|1678147200000|1688511600000|1693695600000|lib/koenig-editor/addon/components/koenig-card-embed.hbs +add|ember-template-lint|no-action|56|85|56|85|bb78ad59bc384ea0de5e9459da9d85f1735ce141|1678147200000|1688511600000|1693695600000|lib/koenig-editor/addon/components/koenig-card-embed.hbs +add|ember-template-lint|no-action|57|38|57|38|3ad187464ff78253a0ea4dd17dcfcf0423f66864|1678147200000|1688511600000|1693695600000|lib/koenig-editor/addon/components/koenig-card-embed.hbs diff --git a/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.hbs b/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.hbs index 7ba8638792..af19c7a943 100644 --- a/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.hbs +++ b/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.hbs @@ -40,6 +40,15 @@
  
+ {{else if this.isUnauthorized}} +
+ The owner of this URL has disabled embedding. + + + +
{{else if this.hasError}}
There was an error when parsing the URL. diff --git a/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.js b/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.js index ca9d2b0c43..5ad02e8195 100644 --- a/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.js +++ b/ghost/admin/lib/koenig-editor/addon/components/koenig-card-embed.js @@ -6,6 +6,7 @@ import {NO_CURSOR_MOVEMENT} from './koenig-editor'; import {action, computed, set} from '@ember/object'; import {utils as ghostHelperUtils} from '@tryghost/helpers'; import {isBlank} from '@ember/utils'; +import {isUnauthorizedError} from 'ember-ajax/errors'; import {run} from '@ember/runloop'; import {task} from 'ember-concurrency'; @@ -23,6 +24,7 @@ export default class KoenigCardEmbed extends Component { // internal properties hasError = false; + isUnauthorized = false; // closure actions selectCard() {} @@ -114,6 +116,7 @@ export default class KoenigCardEmbed extends Component { @action retry() { this.set('hasError', false); + this.set('isUnauthorized', false); } @action @@ -208,6 +211,9 @@ export default class KoenigCardEmbed extends Component { return; } this.set('hasError', true); + if (isUnauthorizedError(err)) { + this.set('isUnauthorized', true); + } } }).drop()) convertUrl; diff --git a/ghost/core/core/server/services/oembed/twitter-embed.js b/ghost/core/core/server/services/oembed/twitter-embed.js index 28bb27d44f..ac6a8b9f20 100644 --- a/ghost/core/core/server/services/oembed/twitter-embed.js +++ b/ghost/core/core/server/services/oembed/twitter-embed.js @@ -1,4 +1,4 @@ -const {extract} = require('oembed-parser'); +const {extract} = require('@extractus/oembed-extractor'); const logging = require('@tryghost/logging'); /** diff --git a/ghost/core/package.json b/ghost/core/package.json index d5059061ce..145e30224d 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -60,6 +60,7 @@ "dependencies": { "@sentry/node": "7.50.0", "@tryghost/adapter-base-cache": "0.1.5", + "@extractus/oembed-extractor": "^3.1.8", "@tryghost/adapter-cache-redis": "0.0.0", "@tryghost/adapter-manager": "0.0.0", "@tryghost/admin-api-schema": "4.3.0", @@ -202,7 +203,6 @@ "mysql2": "3.2.0", "nconf": "0.12.0", "node-jose": "2.2.0", - "oembed-parser": "1.4.9", "path-match": "1.2.4", "probe-image-size": "7.2.3", "rss": "1.2.2", diff --git a/ghost/core/test/e2e-api/admin/oembed.test.js b/ghost/core/test/e2e-api/admin/oembed.test.js index cb6c9e1372..72dbaa6f75 100644 --- a/ghost/core/test/e2e-api/admin/oembed.test.js +++ b/ghost/core/test/e2e-api/admin/oembed.test.js @@ -58,6 +58,37 @@ describe('Oembed API', function () { should.exist(res.body.html); }); + it('errors with a useful message when embedding is disabled', async function () { + const requestMock = nock('https://www.youtube.com') + .get('/oembed') + .query(true) + .reply(401, { + errors: [ + { + message: 'Authorisation error, cannot read oembed.', + context: 'URL contains a private resource.', + type: 'UnauthorizedError', + details: null, + property: null, + help: null, + code: null, + id: 'c51228a0-921a-11ed-8abe-6babfda4d18a', + ghostErrorCode: null + } + ] + }); + + const res = await request.get(localUtils.API.getApiQuery('oembed/?url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DE5yFcdPAGv0')) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(401); + + requestMock.isDone().should.be.true(); + should.exist(res.body.errors); + res.body.errors[0].context.should.match(/URL contains a private resource/i); + }); + describe('type: bookmark', function () { it('can fetch a bookmark with ?type=bookmark', async function () { const pageMock = nock('http://example.com') diff --git a/ghost/oembed-service/lib/oembed-service.js b/ghost/oembed-service/lib/oembed-service.js index 026a641869..e20b6511c3 100644 --- a/ghost/oembed-service/lib/oembed-service.js +++ b/ghost/oembed-service/lib/oembed-service.js @@ -1,7 +1,7 @@ const errors = require('@tryghost/errors'); const tpl = require('@tryghost/tpl'); const logging = require('@tryghost/logging'); -const {extract, hasProvider} = require('oembed-parser'); +const {extract, hasProvider} = require('@extractus/oembed-extractor'); const cheerio = require('cheerio'); const _ = require('lodash'); const charset = require('charset'); @@ -10,7 +10,8 @@ const iconv = require('iconv-lite'); const messages = { noUrlProvided: 'No url provided.', insufficientMetadata: 'URL contains insufficient metadata.', - unknownProvider: 'No provider found for supplied URL.' + unknownProvider: 'No provider found for supplied URL.', + unauthorized: 'URL contains a private resource.' }; /** @@ -53,7 +54,7 @@ const findUrlWithProvider = (url) => { /** * @typedef {object} ICustomProvider * @prop {(url: URL) => Promise} canSupportRequest - * @prop {(url: URL, externalRequest: IExternalRequest) => Promise} getOEmbedData + * @prop {(url: URL, externalRequest: IExternalRequest) => Promise} getOEmbedData */ class OEmbed { @@ -97,9 +98,15 @@ class OEmbed { try { return await extract(url); } catch (err) { - throw new errors.InternalServerError({ - message: err.message - }); + if (err.message === 'Request failed with error code 401') { + throw new errors.UnauthorizedError({ + message: messages.unauthorized + }); + } else { + throw new errors.InternalServerError({ + message: err.message + }); + } } } diff --git a/package.json b/package.json index 0f51881ff6..1da684eede 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "intl-messageformat", "moment", "moment-timezone", - "oembed-parser", "simple-dom", "ember-drag-drop", "normalize.css", diff --git a/yarn.lock b/yarn.lock index 205e5e3e04..59c3bf27d0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2589,6 +2589,13 @@ resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.37.0.tgz#cf1b5fa24217fe007f6487a26d765274925efa7d" integrity sha512-x5vzdtOOGgFVDCUs81QRB2+liax8rFg3+7hqM+QhBG0/G3F1ZsoYl97UrqgHgQ9KKT7G6c4V+aTUCgu/n22v1A== +"@extractus/oembed-extractor@^3.1.8": + version "3.1.8" + resolved "https://registry.yarnpkg.com/@extractus/oembed-extractor/-/oembed-extractor-3.1.8.tgz#79ea7ed65c7688bdf9ee673a0ac5aa122cef5e4e" + integrity sha512-k6p8des8ISJY2fuuQDyiUOTlcuPOzWETM2ewF1aywFNSS3EvgGWRwoMsvBKhCrLuvb8NyEKDAJB0SWgt0L793w== + dependencies: + cross-fetch "^3.1.5" + "@faker-js/faker@7.6.0": version "7.6.0" resolved "https://registry.yarnpkg.com/@faker-js/faker/-/faker-7.6.0.tgz#9ea331766084288634a9247fcd8b84f16ff4ba07" @@ -11749,7 +11756,7 @@ cron-validate@1.4.5, cron-validate@^1.4.1: dependencies: yup "0.32.9" -cross-fetch@3.1.5, cross-fetch@^3.1.4: +cross-fetch@3.1.5, cross-fetch@^3.1.4, cross-fetch@^3.1.5: version "3.1.5" resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.5.tgz#e1389f44d9e7ba767907f7af8454787952ab534f" integrity sha512-lvb1SBsI0Z7GDwmuid+mU3kWVBwTVUbe7S0H52yaaAdQOXq2YktTCZdlAcNKFzE6QtRz0snpw9bNiPeOIkkQvw==