From fdb0e3d44db8bbe7d33cf0c112e1e604a22ffef4 Mon Sep 17 00:00:00 2001 From: Thibaut Patel Date: Thu, 31 Mar 2022 10:52:04 +0200 Subject: [PATCH] Added the newsletter API permissions refs https://github.com/TryGhost/Team/issues/1463 - Allow admins to perform all newsletter operations - We can adjust and be more permissive in the future if needed - Added the tests back as permissions are configured correctly now --- ...-03-30-15-44-add-newsletter-permissions.js | 28 ++++ test/e2e-api/admin/newsletters.test.js | 132 +++++++++--------- test/integration/migrations/migration.test.js | 8 +- .../schema/fixtures/fixture-manager.test.js | 12 +- test/utils/fixtures/fixtures.json | 18 ++- 5 files changed, 123 insertions(+), 75 deletions(-) create mode 100644 core/server/data/migrations/versions/4.42/2022-03-30-15-44-add-newsletter-permissions.js diff --git a/core/server/data/migrations/versions/4.42/2022-03-30-15-44-add-newsletter-permissions.js b/core/server/data/migrations/versions/4.42/2022-03-30-15-44-add-newsletter-permissions.js new file mode 100644 index 0000000000..d5f1e9112c --- /dev/null +++ b/core/server/data/migrations/versions/4.42/2022-03-30-15-44-add-newsletter-permissions.js @@ -0,0 +1,28 @@ +const { + addPermissionWithRoles, + combineTransactionalMigrations +} = require('../../utils'); + +module.exports = combineTransactionalMigrations( + addPermissionWithRoles({ + name: 'Browse newsletters', + action: 'browse', + object: 'newsletter' + }, [ + 'Administrator' + ]), + addPermissionWithRoles({ + name: 'Add newsletters', + action: 'add', + object: 'newsletter' + }, [ + 'Administrator' + ]), + addPermissionWithRoles({ + name: 'Edit newsletters', + action: 'edit', + object: 'newsletter' + }, [ + 'Administrator' + ]) +); diff --git a/test/e2e-api/admin/newsletters.test.js b/test/e2e-api/admin/newsletters.test.js index 786a729532..81b00aabe2 100644 --- a/test/e2e-api/admin/newsletters.test.js +++ b/test/e2e-api/admin/newsletters.test.js @@ -21,76 +21,76 @@ describe('Newsletters API', function () { mockManager.restore(); }); - // it('Can add a newsletter', async function () { - // const newsletter = { - // name: 'My test newsletter', - // sender_name: 'Test', - // sender_email: 'test@example.com', - // sender_reply_to: 'test@example.com', - // default: false, - // status: 'active', - // recipient_filter: '', - // subscribe_on_signup: true, - // sort_order: 0 - // }; + it('Can add a newsletter', async function () { + const newsletter = { + name: 'My test newsletter', + sender_name: 'Test', + sender_email: 'test@example.com', + sender_reply_to: 'test@example.com', + default: false, + status: 'active', + recipient_filter: '', + subscribe_on_signup: true, + sort_order: 0 + }; - // await agent - // .post(`newsletters/`) - // .body({newsletters: [newsletter]}) - // .expectStatus(201) - // .matchBodySnapshot({ - // newsletters: [newsletterSnapshot] - // }) - // .matchHeaderSnapshot({ - // etag: anyEtag, - // location: anyString - // }); + await agent + .post(`newsletters/`) + .body({newsletters: [newsletter]}) + .expectStatus(201) + .matchBodySnapshot({ + newsletters: [newsletterSnapshot] + }) + .matchHeaderSnapshot({ + etag: anyEtag, + location: anyString + }); - // await agent.get('newsletters/') - // .expectStatus(200) - // .matchBodySnapshot({ - // newsletters: [newsletterSnapshot] - // }) - // .matchHeaderSnapshot({ - // etag: anyEtag - // }); - // }); + await agent.get('newsletters/') + .expectStatus(200) + .matchBodySnapshot({ + newsletters: [newsletterSnapshot] + }) + .matchHeaderSnapshot({ + etag: anyEtag + }); + }); - // it('Can browse newsletters', async function () { - // await agent.get('newsletters/') - // .expectStatus(200) - // .matchBodySnapshot({ - // newsletters: [newsletterSnapshot] - // }) - // .matchHeaderSnapshot({ - // etag: anyEtag - // }); - // }); + it('Can browse newsletters', async function () { + await agent.get('newsletters/') + .expectStatus(200) + .matchBodySnapshot({ + newsletters: [newsletterSnapshot] + }) + .matchHeaderSnapshot({ + etag: anyEtag + }); + }); - // it('Can edit newsletters', async function () { - // const res = await agent.get('newsletters?limit=1') - // .expectStatus(200) - // .matchBodySnapshot({ - // newsletters: [newsletterSnapshot] - // }) - // .matchHeaderSnapshot({ - // etag: anyEtag - // }); + it('Can edit newsletters', async function () { + const res = await agent.get('newsletters?limit=1') + .expectStatus(200) + .matchBodySnapshot({ + newsletters: [newsletterSnapshot] + }) + .matchHeaderSnapshot({ + etag: anyEtag + }); - // const id = res.body.newsletters[0].id; + const id = res.body.newsletters[0].id; - // await agent.put(`newsletters/${id}`) - // .body({ - // newsletters: [{ - // name: 'Updated newsletter name' - // }] - // }) - // .expectStatus(200) - // .matchBodySnapshot({ - // newsletters: [newsletterSnapshot] - // }) - // .matchHeaderSnapshot({ - // etag: anyEtag - // }); - // }); + await agent.put(`newsletters/${id}`) + .body({ + newsletters: [{ + name: 'Updated newsletter name' + }] + }) + .expectStatus(200) + .matchBodySnapshot({ + newsletters: [newsletterSnapshot] + }) + .matchHeaderSnapshot({ + etag: anyEtag + }); + }); }); diff --git a/test/integration/migrations/migration.test.js b/test/integration/migrations/migration.test.js index 99e7417913..ee3d170a7a 100644 --- a/test/integration/migrations/migration.test.js +++ b/test/integration/migrations/migration.test.js @@ -45,7 +45,7 @@ describe('Database Migration (special functions)', function () { const permissions = this.obj; // If you have to change this number, please add the relevant `havePermission` checks below - permissions.length.should.eql(92); + permissions.length.should.eql(95); permissions.should.havePermission('Export database', ['Administrator', 'DB Backup Integration']); permissions.should.havePermission('Import database', ['Administrator', 'DB Backup Integration']); @@ -162,6 +162,10 @@ describe('Database Migration (special functions)', function () { permissions.should.havePermission('Browse custom theme settings', ['Administrator']); permissions.should.havePermission('Edit custom theme settings', ['Administrator']); + + permissions.should.havePermission('Browse newsletters', ['Administrator']); + permissions.should.havePermission('Edit newsletters', ['Administrator']); + permissions.should.havePermission('Add newsletters', ['Administrator']); }); describe('Populate', function () { @@ -219,7 +223,7 @@ describe('Database Migration (special functions)', function () { result.roles.at(7).get('name').should.eql('Scheduler Integration'); // Permissions - result.permissions.length.should.eql(92); + result.permissions.length.should.eql(95); result.permissions.toJSON().should.be.CompletePermissions(); }); }); diff --git a/test/unit/server/data/schema/fixtures/fixture-manager.test.js b/test/unit/server/data/schema/fixtures/fixture-manager.test.js index a27a94b363..5c7f57e34f 100644 --- a/test/unit/server/data/schema/fixtures/fixture-manager.test.js +++ b/test/unit/server/data/schema/fixtures/fixture-manager.test.js @@ -162,18 +162,18 @@ describe('Migration Fixture Utils', function () { fixtureManager.addFixturesForRelation(fixtures.relations[0]).then(function (result) { should.exist(result); result.should.be.an.Object(); - result.should.have.property('expected', 82); - result.should.have.property('done', 82); + result.should.have.property('expected', 83); + result.should.have.property('done', 83); // Permissions & Roles permsAllStub.calledOnce.should.be.true(); rolesAllStub.calledOnce.should.be.true(); - dataMethodStub.filter.callCount.should.eql(82); + dataMethodStub.filter.callCount.should.eql(83); dataMethodStub.find.callCount.should.eql(7); - baseUtilAttachStub.callCount.should.eql(82); + baseUtilAttachStub.callCount.should.eql(83); - fromItem.related.callCount.should.eql(82); - fromItem.find.callCount.should.eql(82); + fromItem.related.callCount.should.eql(83); + fromItem.find.callCount.should.eql(83); done(); }).catch(done); diff --git a/test/utils/fixtures/fixtures.json b/test/utils/fixtures/fixtures.json index e2e31e6bb6..dc6f3e3e17 100644 --- a/test/utils/fixtures/fixtures.json +++ b/test/utils/fixtures/fixtures.json @@ -532,6 +532,21 @@ "name": "Edit custom theme settings", "action_type": "edit", "object_type": "custom_theme_setting" + }, + { + "name": "Browse newsletters", + "action_type": "browse", + "object_type": "newsletter" + }, + { + "name": "Add newsletters", + "action_type": "add", + "object_type": "newsletter" + }, + { + "name": "Edit newsletters", + "action_type": "edit", + "object_type": "newsletter" } ] }, @@ -792,7 +807,8 @@ "custom_theme_setting": "all", "offer": "all", "authentication": "resetAllPasswords", - "members_stripe_connect": "auth" + "members_stripe_connect": "auth", + "newsletter": "all" }, "DB Backup Integration": { "db": "all"