From 258f56ded9297f45a2299588223cab15f75516ac Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Mon, 3 Oct 2022 12:05:58 +0100 Subject: [PATCH] Added regression test for publishing as editor when over member limit (#15518) closes https://github.com/TryGhost/Team/issues/2010 refs https://github.com/TryGhost/Ghost/commit/a67cb265fc3ab56e1b34b4a035981d3ba4d4e0b3 - wrapped all members endpoints in a permission check that returns a 403 response if the logged in member is not an admin - added a publish-flow acceptance test that goes through the flow as an Editor user that fails if the stats endpoint is hit and throws a permissions error - removed some unnecessary waits in members acceptance test that were added for earlier versions of Ember --- ghost/admin/mirage/config/members.js | 64 ++++++++++++----- .../acceptance/editor/publish-flow-test.js | 71 ++++++++++++++++++- ghost/admin/tests/acceptance/members-test.js | 14 +--- ghost/admin/tests/helpers/login-as-role.js | 2 +- 4 files changed, 116 insertions(+), 35 deletions(-) diff --git a/ghost/admin/mirage/config/members.js b/ghost/admin/mirage/config/members.js index 5c790ce02c..ac870fe646 100644 --- a/ghost/admin/mirage/config/members.js +++ b/ghost/admin/mirage/config/members.js @@ -3,10 +3,32 @@ import moment from 'moment-timezone'; import nql from '@tryghost/nql'; import {Response} from 'miragejs'; import {extractFilterParam, paginateModelCollection} from '../utils'; +import {getContext} from '@ember/test-helpers'; import {underscore} from '@ember/string'; +function hasInvalidPermissions() { + const {owner} = getContext(); + const session = owner.lookup('service:session'); + + if (!session?.user?.isAdmin) { + return new Response(403, {}, { + errors: [{ + type: 'NoPermissionError', + message: 'You do not have permission to perform this action' + }] + }); + } +} + +function withPermissionsCheck(fn) { + return function () { + const boundFn = fn.bind(this); + return hasInvalidPermissions() || boundFn(...arguments); + }; +} + export function mockMembersStats(server) { - server.get('/members/stats/count', function (db, {queryParams}) { + server.get('/members/stats/count', withPermissionsCheck(function (db, {queryParams}) { let {days} = queryParams; let firstSubscriberDays = faker.datatype.number({min: 30, max: 600}); @@ -58,13 +80,16 @@ export function mockMembersStats(server) { }; }) }; - }); + })); } export default function mockMembers(server) { - server.post('/members/'); + server.post('/members/', withPermissionsCheck(function ({members}) { + const attrs = this.normalizedRequestAttrs(); + return members.create(attrs); + })); - server.get('/members/', function ({members}, {queryParams}) { + server.get('/members/', withPermissionsCheck(function ({members}, {queryParams}) { let {filter, search, page, limit} = queryParams; page = +page || 1; @@ -127,9 +152,9 @@ export default function mockMembers(server) { } return paginateModelCollection('members', collection, page, limit); - }); + })); - server.del('/members/', function ({members}, {queryParams}) { + server.del('/members/', withPermissionsCheck(function ({members}, {queryParams}) { if (!queryParams.filter && !queryParams.search && queryParams.all !== 'true') { return new Response(422, {}, {errors: [{ type: 'IncorrectUsageError', @@ -163,9 +188,9 @@ export default function mockMembers(server) { } } }; - }); + })); - server.get('/members/:id/', function ({members}, {params}) { + server.get('/members/:id/', withPermissionsCheck(function ({members}, {params}) { let {id} = params; let member = members.find(id); @@ -175,9 +200,9 @@ export default function mockMembers(server) { message: 'Member not found.' }] }); - }); + })); - server.put('/members/:id/', function ({members, tiers, subscriptions}, {params}) { + server.put('/members/:id/', withPermissionsCheck(function ({members, tiers, subscriptions}, {params}) { const attrs = this.normalizedRequestAttrs(); const member = members.find(params.id); @@ -245,19 +270,22 @@ export default function mockMembers(server) { delete attrs.subscriptions; return member.update(attrs); - }); + })); - server.del('/members/:id/'); + server.del('/members/:id/', withPermissionsCheck(function ({members}, request) { + const id = request.params.id; + members.find(id).destroy(); + })); - server.get('/members/upload/', function () { + server.get('/members/upload/', withPermissionsCheck(function () { return new Response(200, { 'Content-Disposition': 'attachment', filename: `members.${moment().format('YYYY-MM-DD')}.csv`, 'Content-Type': 'text/csv' }, ''); - }); + })); - server.post('/members/upload/', function ({labels}, request) { + server.post('/members/upload/', withPermissionsCheck(function ({labels}, request) { const label = labels.create(); // TODO: parse CSV and create member records @@ -272,9 +300,9 @@ export default function mockMembers(server) { stats: {imported: 1, invalid: []} } }); - }); + })); - server.get('/members/events/', function ({memberActivityEvents}, {queryParams}) { + server.get('/members/events/', withPermissionsCheck(function ({memberActivityEvents}, {queryParams}) { let {limit} = queryParams; limit = +limit || 15; @@ -284,7 +312,7 @@ export default function mockMembers(server) { }).slice(0, limit); return collection; - }); + })); mockMembersStats(server); } diff --git a/ghost/admin/tests/acceptance/editor/publish-flow-test.js b/ghost/admin/tests/acceptance/editor/publish-flow-test.js index 5a397c36f1..b767cf5a33 100644 --- a/ghost/admin/tests/acceptance/editor/publish-flow-test.js +++ b/ghost/admin/tests/acceptance/editor/publish-flow-test.js @@ -157,11 +157,10 @@ describe('Acceptance: Publish flow', function () { // at least one member is required for publish+send to be available this.server.createList('member', 3, {status: 'free'}); this.server.createList('member', 4, {status: 'paid'}); - - await loginAsRole('Administrator', this.server); }); it('can publish+send with single newsletter', async function () { + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -212,7 +211,9 @@ describe('Acceptance: Publish flow', function () { this.server.create('member', {newsletters: [newsletter], status: 'free'}); + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); + await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -249,6 +250,7 @@ describe('Acceptance: Publish flow', function () { }); it('can schedule publish+send', async function () { + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -300,6 +302,7 @@ describe('Acceptance: Publish flow', function () { }); it('can send', async function () { + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -325,6 +328,7 @@ describe('Acceptance: Publish flow', function () { }); it('can schedule send', async function () { + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -369,6 +373,7 @@ describe('Acceptance: Publish flow', function () { it('respects default recipient settings - usually nobody', async function () { // switch to "usually nobody" setting // - doing it this way so we're not testing potentially stale mocked setting keys/values + await loginAsRole('Administrator', this.server); await visit('/settings/newsletters'); await click('[data-test-toggle-membersemail]'); await selectChoose('[data-test-select="default-recipients"]', 'Usually nobody'); @@ -402,7 +407,9 @@ describe('Acceptance: Publish flow', function () { it('handles Mailgun not being set up', async function () { disableMailgun(this.server); + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); + await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -425,7 +432,9 @@ describe('Acceptance: Publish flow', function () { this.server.db.members.remove(); this.server.db.newsletters.update({memberIds: []}); + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); + await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -470,8 +479,10 @@ describe('Acceptance: Publish flow', function () { }; }); - // try to publish post + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); + + // try to publish post await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -483,6 +494,7 @@ describe('Acceptance: Publish flow', function () { }); it('handles over-member limit when confirming', async function () { + await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); await visit(`/editor/post/${post.id}`); await click('[data-test-button="publish-flow"]'); @@ -517,6 +529,59 @@ describe('Acceptance: Publish flow', function () { .to.have.trimmed.text('Your plan supports up to 1,000 members. Please upgrade to reenable publishing.'); }); + it('(as editor) handles over-member limits', async function () { + // set members limit + const config = this.server.db.configs.find(1); + config.hostSettings = { + limits: { + members: { + max: 9, + error: 'Your plan supports up to {{max}} members. Please upgrade to reenable publishing.' + } + } + }; + this.server.db.configs.update(1, config); + + // go over limit (7 created by default in beforeEach) + this.server.createList('member', 3); + + await loginAsRole('Editor', this.server); + const post = this.server.create('post', {status: 'draft'}); + + // try to publish post + await visit(`/editor/post/${post.id}`); + await click('[data-test-button="publish-flow"]'); + await click('[data-test-button="continue"]'); + + this.server.put('/posts/:id/', function () { + return { + errors: [ + { + message: 'Host Limit error, cannot edit post.', + context: 'Your plan supports up to 1,000 members. Please upgrade to reenable publishing.', + type: 'HostLimitError', + details: { + name: 'members', + limit: 1000, + total: 37406 + }, + property: null, + help: 'https://ghost.org/help/', + code: null, + id: '212d9110-3db6-11ed-9651-e9a82ad49a7a', + ghostErrorCode: null + } + ] + }; + }); + + await click('[data-test-button="confirm-publish"]'); + + expect(find('[data-test-confirm-error]'), 'confirm error').to.exist; + expect(find('[data-test-confirm-error]'), 'confirm error') + .to.have.trimmed.text('Your plan supports up to 1,000 members. Please upgrade to reenable publishing.'); + }); + it('handles server error when confirming'); it('handles email sending error'); }); diff --git a/ghost/admin/tests/acceptance/members-test.js b/ghost/admin/tests/acceptance/members-test.js index 3543ff8a73..86bdbc4424 100644 --- a/ghost/admin/tests/acceptance/members-test.js +++ b/ghost/admin/tests/acceptance/members-test.js @@ -1,11 +1,10 @@ import moment from 'moment-timezone'; import {authenticateSession, invalidateSession} from 'ember-simple-auth/test-support'; import {beforeEach, describe, it} from 'mocha'; -import {blur, click, currentURL, fillIn, find, findAll, settled} from '@ember/test-helpers'; +import {blur, click, currentURL, fillIn, find, findAll} from '@ember/test-helpers'; import {expect} from 'chai'; import {setupApplicationTest} from 'ember-mocha'; import {setupMirage} from 'ember-cli-mirage/test-support'; -import {timeout} from 'ember-concurrency'; import {visit} from '../helpers/visit'; describe('Acceptance: Members', function () { @@ -47,8 +46,6 @@ describe('Acceptance: Members', function () { await visit('/members'); - await settled(); - // lands on correct page expect(currentURL(), 'currentURL').to.equal('/members'); @@ -69,9 +66,6 @@ describe('Acceptance: Members', function () { await visit(`/members/${member1.id}`); - // // second wait is needed for the member details to settle - await settled(); - // it shows selected member form expect(find('[data-test-input="member-name"]').value, 'loads correct member into form') .to.equal(member1.name); @@ -85,10 +79,6 @@ describe('Acceptance: Members', function () { await click('[data-test-button="save"]'); - // extra timeout needed for Travis - sometimes it doesn't update - // quick enough and an extra wait() call doesn't help - await timeout(100); - await click('[data-test-link="members-back"]'); // lands on correct page @@ -100,8 +90,6 @@ describe('Acceptance: Members', function () { await visit('/members'); - await settled(); - // lands on correct page expect(currentURL(), 'currentURL').to.equal('/members'); diff --git a/ghost/admin/tests/helpers/login-as-role.js b/ghost/admin/tests/helpers/login-as-role.js index 3b3c654657..282446aa33 100644 --- a/ghost/admin/tests/helpers/login-as-role.js +++ b/ghost/admin/tests/helpers/login-as-role.js @@ -1,6 +1,6 @@ import {authenticateSession, invalidateSession} from 'ember-simple-auth/test-support'; -export default async function loginRole(roleName, server) { +export default async function loginAsRole(roleName, server) { const role = server.create('role', {name: roleName}); const user = server.create('user', {roles: [role], slug: 'test-user'}); await invalidateSession();