From 523b9d47a05580b4774985b92863c6b000b1fef2 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 30 Jan 2025 12:47:42 +0000 Subject: [PATCH] Added support for gating access to blocks of post content (#22069) ref https://linear.app/ghost/issue/PLG-327 - updated post output serializer's gating functions to add gating of specific content blocks - uses regex to look for specific strings in the HTML for speed compared to fully parsing the HTML - content gating blocks look like `...gated content...` - parsing of params is limited to `nonMember` with a true/false value and `memberSegment` with a string value containing a limited set of supported filters - occurs at the API level so that content is correctly gated in Content API output and front-end website - added `checkGatedBlockAccess()` to members-service content-gating methods to keep the underlying member checks co-located --- apps/admin-x-settings/package.json | 2 +- ghost/admin/package.json | 4 +- .../serializers/output/utils/post-gating.js | 100 ++++++- .../server/services/members/content-gating.js | 27 ++ ghost/core/package.json | 6 +- ghost/core/test/e2e-api/content/posts.test.js | 16 ++ .../__snapshots__/cards.test.js.snap | 4 +- .../services/email-service/cards.test.js | 1 + .../utils/serializers/input/posts.test.js | 2 +- .../output/utils/post-gating.test.js | 272 ++++++++++++++---- .../services/members/content-gating.test.js | 37 ++- yarn.lock | 58 ++-- 12 files changed, 433 insertions(+), 96 deletions(-) diff --git a/apps/admin-x-settings/package.json b/apps/admin-x-settings/package.json index ce0283e3a9..b5c05e1157 100644 --- a/apps/admin-x-settings/package.json +++ b/apps/admin-x-settings/package.json @@ -36,7 +36,7 @@ "dependencies": { "@codemirror/lang-html": "6.4.9", "@tryghost/color-utils": "0.2.2", - "@tryghost/kg-unsplash-selector": "0.2.7", + "@tryghost/kg-unsplash-selector": "0.2.8", "@tryghost/limit-service": "1.2.14", "@tryghost/nql": "0.12.7", "@tryghost/timezone-data": "0.4.4", diff --git a/ghost/admin/package.json b/ghost/admin/package.json index 936f2bcb99..68855795de 100644 --- a/ghost/admin/package.json +++ b/ghost/admin/package.json @@ -50,7 +50,7 @@ "@tryghost/helpers": "1.1.90", "@tryghost/kg-clean-basic-html": "4.1.5", "@tryghost/kg-converters": "1.0.8", - "@tryghost/koenig-lexical": "1.4.0", + "@tryghost/koenig-lexical": "1.5.1", "@tryghost/limit-service": "1.2.14", "@tryghost/members-csv": "0.0.0", "@tryghost/nql": "0.12.7", @@ -217,4 +217,4 @@ } } } -} \ No newline at end of file +} diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js index 07655f311a..372aaf75ad 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/post-gating.js @@ -1,6 +1,86 @@ const membersService = require('../../../../../../services/members'); const htmlToPlaintext = require('@tryghost/html-to-plaintext'); +const {PERMIT_ACCESS} = membersService.contentGating; + +// Match the start of a gated block - fast regex as a pre-check before doing full regex+loop +const HAS_GATED_BLOCKS_REGEX = /...gated content +const GATED_BLOCK_REGEX = /\s*([\s\S]*?)\s*/g; +// Match the key-value pairs (with optional quotes around the value) in the gated-block:begin comment +const GATED_BLOCK_PARAM_REGEX = /\b(?\w+):["']?(?[^"'\s]+)["']?/g; + +const ALLOWED_GATED_BLOCK_PARAMS = { + nonMember: {type: 'boolean'}, + memberSegment: {type: 'string', allowedValues: ['', 'status:free,status:-free', 'status:free', 'status:-free']} +}; +const ALLOWED_GATED_BLOCK_KEYS = Object.keys(ALLOWED_GATED_BLOCK_PARAMS); + +const parseGatedBlockParams = function (paramsString) { + const params = {}; + + const matches = paramsString.matchAll(GATED_BLOCK_PARAM_REGEX); + for (const match of matches) { + const key = match.groups.key; + let value = match.groups.value; + + if (!ALLOWED_GATED_BLOCK_KEYS.includes(key)) { + continue; + } + + // Convert "true"/"false" strings to booleans, otherwise keep as string + if (value === 'true') { + value = true; + } else if (value === 'false') { + value = false; + } + + if (typeof value !== ALLOWED_GATED_BLOCK_PARAMS[key].type) { + continue; + } + + if (ALLOWED_GATED_BLOCK_PARAMS[key].allowedValues && !ALLOWED_GATED_BLOCK_PARAMS[key].allowedValues.includes(value)) { + continue; + } + + params[key] = value; + } + + return params; +}; + +/** + * @param {string} html - The HTML to strip gated blocks from + * @param {object} member - The member who's access should be checked + * @returns {string} HTML with gated blocks stripped + */ +const stripGatedBlocks = function (html, member) { + return html.replace(GATED_BLOCK_REGEX, (match, params, content) => { + const gatedBlockParams = module.exports.parseGatedBlockParams(params); + const checkResult = membersService.contentGating.checkGatedBlockAccess(gatedBlockParams, member); + + if (checkResult === PERMIT_ACCESS) { + // return content rather than match to avoid rendering gated block wrapping comments + return content; + } else { + return ''; + } + }); +}; + +function _updatePlaintext(attrs) { + if (attrs.html) { + attrs.plaintext = htmlToPlaintext.excerpt(attrs.html); + } +} + +function _updateExcerpt(attrs) { + if (!attrs.custom_excerpt && attrs.excerpt) { + attrs.excerpt = htmlToPlaintext.excerpt(attrs.html).substring(0, 500); + } +} + // @TODO: reconsider the location of this - it's part of members and adds a property to the API const forPost = (attrs, frame) => { // CASE: Access always defaults to true, unless members is enabled and the member does not have access @@ -15,11 +95,8 @@ const forPost = (attrs, frame) => { if (paywallIndex !== -1) { attrs.html = attrs.html.slice(0, paywallIndex); - attrs.plaintext = htmlToPlaintext.excerpt(attrs.html); - - if (!attrs.custom_excerpt && attrs.excerpt) { - attrs.excerpt = attrs.plaintext.substring(0, 500); - } + _updatePlaintext(attrs); + _updateExcerpt(attrs); } else { ['plaintext', 'html', 'excerpt'].forEach((field) => { if (attrs[field] !== undefined) { @@ -29,6 +106,13 @@ const forPost = (attrs, frame) => { } } + const hasGatedBlocks = HAS_GATED_BLOCKS_REGEX.test(attrs.html); + if (hasGatedBlocks) { + attrs.html = module.exports.stripGatedBlocks(attrs.html, frame.original.context.member); + _updatePlaintext(attrs); + _updateExcerpt(attrs); + } + if (!Object.prototype.hasOwnProperty.call(frame.options, 'columns') || (frame.options.columns.includes('access'))) { attrs.access = memberHasAccess; } @@ -36,4 +120,8 @@ const forPost = (attrs, frame) => { return attrs; }; -module.exports.forPost = forPost; +module.exports = { + parseGatedBlockParams, + stripGatedBlocks, + forPost +}; diff --git a/ghost/core/core/server/services/members/content-gating.js b/ghost/core/core/server/services/members/content-gating.js index 206a71fc19..de980951b4 100644 --- a/ghost/core/core/server/services/members/content-gating.js +++ b/ghost/core/core/server/services/members/content-gating.js @@ -61,8 +61,35 @@ function checkPostAccess(post, member) { return BLOCK_ACCESS; } +function checkGatedBlockAccess(gatedBlockParams, member) { + const {nonMember, memberSegment} = gatedBlockParams; + const isLoggedIn = !!member; + + if (nonMember && !isLoggedIn) { + return PERMIT_ACCESS; + } + + if (!memberSegment && isLoggedIn) { + return BLOCK_ACCESS; + } + + if (memberSegment && member) { + const nqlQuery = nql(memberSegment, {expansions: MEMBER_NQL_EXPANSIONS, transformer: rejectUnknownKeys}); + + // if we only have unknown keys the NQL query will be empty and "pass" for all members + // we should block access in this case to match the memberSegment:"" behaviour + const parsedQuery = nqlQuery.parse(); + if (Object.keys(parsedQuery).length > 0) { + return nqlQuery.queryJSON(member) ? PERMIT_ACCESS : BLOCK_ACCESS; + } + } + + return BLOCK_ACCESS; +} + module.exports = { checkPostAccess, + checkGatedBlockAccess, PERMIT_ACCESS, BLOCK_ACCESS }; diff --git a/ghost/core/package.json b/ghost/core/package.json index f1f47a8dfa..603b016a2d 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -106,9 +106,9 @@ "@tryghost/kg-converters": "1.0.8", "@tryghost/kg-default-atoms": "5.0.4", "@tryghost/kg-default-cards": "10.0.10", - "@tryghost/kg-default-nodes": "1.2.3", - "@tryghost/kg-html-to-lexical": "1.1.23", - "@tryghost/kg-lexical-html-renderer": "1.1.25", + "@tryghost/kg-default-nodes": "1.3.0", + "@tryghost/kg-html-to-lexical": "1.1.24", + "@tryghost/kg-lexical-html-renderer": "1.2.0", "@tryghost/kg-mobiledoc-html-renderer": "7.0.7", "@tryghost/limit-service": "1.2.14", "@tryghost/link-redirects": "0.0.0", diff --git a/ghost/core/test/e2e-api/content/posts.test.js b/ghost/core/test/e2e-api/content/posts.test.js index 51647c17b1..a301469f8c 100644 --- a/ghost/core/test/e2e-api/content/posts.test.js +++ b/ghost/core/test/e2e-api/content/posts.test.js @@ -430,4 +430,20 @@ describe('Posts Content API', function () { assert(!query.sql.includes('*'), 'Query should not select *'); } }); + + it('Strips out gated blocks not viewable by anonymous viewers ', async function () { + const post = await models.Post.add({ + title: 'title', + status: 'published', + slug: 'gated-blocks', + lexical: JSON.stringify({root: {children: [{type: 'html',version: 1,html: '

Visible to free/paid members

',visibility: {web: {nonMember: false,memberSegment: 'status:free,status:-free'},email: {memberSegment: ''}}},{type: 'html',version: 1,html: '

Visible to anonymous viewers

',visibility: {web: {nonMember: true,memberSegment: ''},email: {memberSegment: ''}}},{children: [],direction: null,format: '',indent: 0,type: 'paragraph',version: 1}],direction: null,format: '',indent: 0,type: 'root',version: 1}}) + }, {context: {internal: true}}); + + const response = await agent + .get(`posts/${post.id}/`) + .expectStatus(200); + + assert.doesNotMatch(response.body.posts[0].html, /Visible to free\/paid members/); + assert.match(response.body.posts[0].html, /Visible to anonymous viewers/); + }); }); diff --git a/ghost/core/test/integration/services/email-service/__snapshots__/cards.test.js.snap b/ghost/core/test/integration/services/email-service/__snapshots__/cards.test.js.snap index 9b55b64890..08f2e843db 100644 --- a/ghost/core/test/integration/services/email-service/__snapshots__/cards.test.js.snap +++ b/ghost/core/test/integration/services/email-service/__snapshots__/cards.test.js.snap @@ -1834,12 +1834,12 @@ Ghost: Independent technology for modern publishingBeautiful, modern publishing

This is just a simple paragraph, no frills.

This is block quote

This is a...different block quote

This is a heading!

Here's a smaller heading.

\\"Cows
A lovely cow

A heading

and a paragraph (in markdown!)

-
+

A paragraph inside an HTML card.

And another one, with some bold text.

-

A gallery.

+

A gallery.

diff --git a/ghost/core/test/integration/services/email-service/cards.test.js b/ghost/core/test/integration/services/email-service/cards.test.js index 1ccf953cb0..95be60c6a9 100644 --- a/ghost/core/test/integration/services/email-service/cards.test.js +++ b/ghost/core/test/integration/services/email-service/cards.test.js @@ -191,6 +191,7 @@ describe('Can send cards via email', function () { 'extended-text', // not a card 'extended-quote', // not a card 'extended-heading', // not a card + 'call-to-action', // behind the contentVisibilityAlpha labs flag // not a card and shouldn't be present in published posts / emails 'tk', 'at-link', diff --git a/ghost/core/test/unit/api/canary/utils/serializers/input/posts.test.js b/ghost/core/test/unit/api/canary/utils/serializers/input/posts.test.js index 961f1591f7..53f374ac60 100644 --- a/ghost/core/test/unit/api/canary/utils/serializers/input/posts.test.js +++ b/ghost/core/test/unit/api/canary/utils/serializers/input/posts.test.js @@ -353,7 +353,7 @@ describe('Unit: endpoints/utils/serializers/input/posts', function () { serializers.input.posts.edit(apiConfig, frame); let postData = frame.data.posts[0]; - postData.lexical.should.equal('{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"this is great feature","type":"extended-text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1},{"type":"html","version":1,"html":"
My Custom HTML
","visibility":{"showOnEmail":true,"showOnWeb":true,"segment":""}},{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"custom html preserved!","type":"extended-text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'); + postData.lexical.should.equal('{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"this is great feature","type":"extended-text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1},{"type":"html","version":1,"html":"
My Custom HTML
","visibility":{"web":{"nonMember":true,"memberSegment":"status:free,status:-free"},"email":{"memberSegment":"status:free,status:-free"}}},{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"custom html preserved!","type":"extended-text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'); }); it('throws error when HTML conversion fails', function () { diff --git a/ghost/core/test/unit/api/canary/utils/serializers/output/utils/post-gating.test.js b/ghost/core/test/unit/api/canary/utils/serializers/output/utils/post-gating.test.js index f9b1e9d1c8..95515db178 100644 --- a/ghost/core/test/unit/api/canary/utils/serializers/output/utils/post-gating.test.js +++ b/ghost/core/test/unit/api/canary/utils/serializers/output/utils/post-gating.test.js @@ -1,8 +1,25 @@ -const should = require('should'); +const assert = require('assert/strict'); +const sinon = require('sinon'); const gating = require('../../../../../../../../core/server/api/endpoints/utils/serializers/output/utils/post-gating'); +const membersContentGating = require('../../../../../../../../core/server/services/members/content-gating'); describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function () { + afterEach(function () { + sinon.restore(); + }); + describe('for post', function () { + let frame; + + beforeEach(function () { + frame = { + options: {}, + original: { + context: {} + } + }; + }); + it('should NOT hide content attributes when visibility is public', function () { const attrs = { visibility: 'public', @@ -10,16 +27,9 @@ describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function html: '

I am here to stay

' }; - const frame = { - options: {}, - original: { - context: {} - } - }; - gating.forPost(attrs, frame); - attrs.plaintext.should.eql('no touching'); + assert.equal(attrs.plaintext, 'no touching'); }); it('should hide content attributes when visibility is "members"', function () { @@ -29,17 +39,10 @@ describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function html: '

I am here to stay

' }; - const frame = { - options: {}, - original: { - context: {} - } - }; - gating.forPost(attrs, frame); - attrs.plaintext.should.eql(''); - attrs.html.should.eql(''); + assert.equal(attrs.plaintext, ''); + assert.equal(attrs.html, ''); }); it('should NOT hide content attributes when visibility is "members" and member is present', function () { @@ -49,19 +52,12 @@ describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function html: '

What\'s the matter?

' }; - const frame = { - options: {}, - original: { - context: { - member: {} - } - } - }; + frame.original.context.member = {}; gating.forPost(attrs, frame); - attrs.plaintext.should.eql('I see dead people'); - attrs.html.should.eql('

What\'s the matter?

'); + assert.equal(attrs.plaintext, 'I see dead people'); + assert.equal(attrs.html, '

What\'s the matter?

'); }); it('should hide content attributes when visibility is "paid" and member has status of "free"', function () { @@ -71,21 +67,12 @@ describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function html: '

What\'s the matter?

' }; - const frame = { - options: {}, - original: { - context: { - member: { - status: 'free' - } - } - } - }; + frame.original.context.member = {status: 'free'}; gating.forPost(attrs, frame); - attrs.plaintext.should.eql(''); - attrs.html.should.eql(''); + assert.equal(attrs.plaintext, ''); + assert.equal(attrs.html, ''); }); it('should NOT hide content attributes when visibility is "paid" and member has status of "paid"', function () { @@ -95,21 +82,204 @@ describe('Unit: endpoints/utils/serializers/output/utils/post-gating', function html: '

Can read this

' }; - const frame = { - options: {}, - original: { - context: { - member: { - status: 'paid' - } - } - } + frame.original.context.member = {status: 'paid'}; + + gating.forPost(attrs, frame); + + assert.equal(attrs.plaintext, 'Secret paid content'); + assert.equal(attrs.html, '

Can read this

'); + }); + + it('does not call stripGatedBlocks when a post has no gated blocks', function () { + const attrs = { + visibility: 'public', + html: '

no gated blocks

' + }; + + const stripGatedBlocksStub = sinon.stub(gating, 'stripGatedBlocks'); + gating.forPost(attrs, frame); + sinon.assert.notCalled(stripGatedBlocksStub); + }); + + it('calls stripGatedBlocks when a post has gated blocks', function () { + const attrs = { + visibility: 'public', + html: '

gated block

' + }; + + const stripGatedBlocksStub = sinon.stub(gating, 'stripGatedBlocks'); + gating.forPost(attrs, frame); + sinon.assert.calledOnce(stripGatedBlocksStub); + }); + + it('updates html, plaintext, and excerpt when a post has gated blocks', function () { + const attrs = { + visibility: 'public', + html: ` +

Members only.

+

Everyone can see this.

+

Anonymous only.

+ `, + plaintext: 'Members only. Everyone can see this. Anonymous only.', + excerpt: 'Members only. Everyone can see this. Anonymous only.' }; gating.forPost(attrs, frame); - attrs.plaintext.should.eql('Secret paid content'); - attrs.html.should.eql('

Can read this

'); + assert.match(attrs.html, /

Everyone can see this\.<\/p>\n\s+

Anonymous only.<\/p>/); + assert.match(attrs.plaintext, /^\n+Everyone can see this.\n+Anonymous only.\n$/); + assert.match(attrs.excerpt, /^\n+Everyone can see this.\n+Anonymous only.\n$/); + }); + }); + + describe('parseGatedBlockParams', function () { + function testFn(input, expected) { + const params = gating.parseGatedBlockParams(input); + assert.deepEqual(params, expected); + } + + const validTestCases = [{ + input: 'nonMember:true', + output: {nonMember: true} + }, { + input: 'nonMember:false', + output: {nonMember: false} + }, { + input: 'nonMember:\'true\'', + output: {nonMember: true} + }, { + input: 'nonMember:\'false\'', + output: {nonMember: false} + }, { + input: 'nonMember:"true"', + output: {nonMember: true} + }, { + input: 'memberSegment:\'\'', + output: {} + }, { + input: 'memberSegment:"status:free"', + output: {memberSegment: 'status:free'} + }, { + input: 'nonMember:true memberSegment:"status:free"', + output: {nonMember: true, memberSegment: 'status:free'} + }, { + input: 'memberSegment:"status:free" nonMember:true', + output: {nonMember: true, memberSegment: 'status:free'} + }]; + + validTestCases.forEach(function (testCase) { + it(`should parse ${testCase.input} correctly`, function () { + testFn(testCase.input, testCase.output); + }); + }); + + // we only support known keys and values with the correct types and allowed values + // we should also handle malformed input gracefully + const invalidTestCases = [{ + input: 'unknownKey:true nonMember:false', + output: {nonMember: false} + }, { + input: 'nonMember:invalid', + output: {} + }, { + input: 'nonMember: memberSegment:"status:free"', + output: {memberSegment: 'status:free'} + }, { + input: 'memberSegment:"status:paid"', + output: {} + }, { + input: 'nonMember:memberSegment:"status:free"', + output: {} + }, { + input: 'memberSegment', + output: {} + }]; + + invalidTestCases.forEach(function (testCase) { + it(`should handle unexpected input ${testCase.input} correctly`, function () { + testFn(testCase.input, testCase.output); + }); + }); + }); + + describe('stripGatedBlocks', function () { + function stubCheckGatedBlockAccess(permitAccess) { + return sinon.stub(membersContentGating, 'checkGatedBlockAccess').returns(permitAccess); + } + + it('handles content with no gated blocks', function () { + const checkGatedBlockAccessStub = stubCheckGatedBlockAccess(true); + const html = '

no gated blocks

'; + const result = gating.stripGatedBlocks(html, {}); + assert.equal(result, html); + sinon.assert.notCalled(checkGatedBlockAccessStub); + }); + + it('handles content with only a denied gated block', function () { + const checkGatedBlockAccessStub = stubCheckGatedBlockAccess(false); + const html = '

gated blocks

'; + const result = gating.stripGatedBlocks(html, {}); + sinon.assert.calledWith(checkGatedBlockAccessStub, {nonMember: false}, {}); + assert.equal(result, ''); + }); + + it('handles content with only a permitted gated block', function () { + const checkGatedBlockAccessStub = stubCheckGatedBlockAccess(true); + const html = '

gated blocks

'; + const result = gating.stripGatedBlocks(html, {}); + sinon.assert.calledWith(checkGatedBlockAccessStub, {nonMember: true}, {}); + assert.equal(result, '

gated blocks

'); + }); + + it('handles content with multiple permitted blocks', function () { + const checkGatedBlockAccessStub = stubCheckGatedBlockAccess(true); + const html = ` +

gated block 1

+

Non-gated block

+

gated block 2

+ `; + const result = gating.stripGatedBlocks(html, {}); + sinon.assert.calledTwice(checkGatedBlockAccessStub); + assert.equal(result, ` +

gated block 1

+

Non-gated block

+

gated block 2

+ `); + }); + + it('handles mix of permitted and denied blocks', function () { + const checkGatedBlockAccessStub = sinon.stub(membersContentGating, 'checkGatedBlockAccess') + .onFirstCall().returns(false) + .onSecondCall().returns(true); + const html = ` +

gated block 1

+

Non-gated block

+

gated block 2

+ `; + const result = gating.stripGatedBlocks(html, null); + sinon.assert.calledTwice(checkGatedBlockAccessStub); + assert.equal(result.trim(), ` +

Non-gated block

+

gated block 2

+ `.trim()); + }); + + it('handles malformed gated block comments', function () { + const checkGatedBlockAccessStub = stubCheckGatedBlockAccess(true); + const html = ` + +

Non-gated block

+

valid gated block

+ `; + const result = gating.stripGatedBlocks(html, null); + sinon.assert.calledOnce(checkGatedBlockAccessStub); + assert.equal(result.trim(), ` + +

Non-gated block

+