Skip to content

Commit d2163f5

Browse files
Removed gift links permission from the Author role (#28910)
ref https://linear.app/ghost/issue/BER-3748/ The gift links permission fixtures were too broad: Authors were granted the "Manage gift links" permission, but Authors don't have permission to change post visibility — so they shouldn't be able to manage gift links either. - Removed `gift_link: manage` from the Author role in the permission fixtures - Added a migration to drop the existing "Manage gift links" grant from the Author role on sites that already ran the original gift links permission migration - Exported the existing `removePermissionFromRole` migration helper (the natural inverse of `addPermissionToRole`) so the migration can use it - Updated the migration integration assertion and the test-utils fixtures copy to match, and extended the gift-links API test to assert Authors (alongside Contributors) get a 403
1 parent 983582f commit d2163f5

8 files changed

Lines changed: 33 additions & 13 deletions

File tree

ghost/core/core/server/data/migrations/utils/permissions.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ function createRemovePermissionMigration(config, roles) {
272272
module.exports = {
273273
addPermission,
274274
addPermissionToRole,
275+
removePermissionFromRole,
275276
addPermissionWithRoles,
276277
createRemovePermissionMigration
277278
};
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const {removePermissionFromRole} = require('../../utils');
2+
3+
// Authors cannot change post visibility, so they should not be able to manage gift
4+
// links either. Drop the over-broad grant added in the original gift links rollout.
5+
module.exports = removePermissionFromRole({
6+
permission: 'Manage gift links',
7+
role: 'Author'
8+
});

ghost/core/core/server/data/schema/fixtures/fixtures.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,8 +1090,7 @@
10901090
"product": ["browse", "read"],
10911091
"newsletter": ["browse", "read"],
10921092
"collection": ["browse", "read", "add"],
1093-
"recommendation": ["browse", "read"],
1094-
"gift_link": "manage"
1093+
"recommendation": ["browse", "read"]
10951094
},
10961095
"Contributor": {
10971096
"post": ["browse", "read", "edit", "add", "destroy"],

ghost/core/test/e2e-api/admin/gift-links.test.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ describe('Gift Links Admin API', function () {
99
put: (_url: string) => any;
1010
post: (_url: string) => any;
1111
loginAsOwner: () => Promise<void>;
12+
loginAsAuthor: () => Promise<void>;
1213
loginAsContributor: () => Promise<void>;
1314
};
1415
let postId: string;
@@ -64,12 +65,6 @@ describe('Gift Links Admin API', function () {
6465
assert.notEqual(body.gift_links[0].token, first);
6566
});
6667

67-
it('403s for a role without gift-link permission', async function () {
68-
await agent.loginAsContributor();
69-
await agent.get(`${name}/${id()}/gift_links/`).expectStatus(403);
70-
await agent.loginAsOwner();
71-
});
72-
7368
it('supports the full lifecycle', async function () {
7469
// empty
7570
let body = (await agent.get(`${name}/${id()}/gift_links/`).expectStatus(200)).body;
@@ -95,6 +90,24 @@ describe('Gift Links Admin API', function () {
9590
});
9691
});
9792

93+
// Permission is granted at the role level, independent of the post/page, so these
94+
// run once rather than per-entity.
95+
describe('without gift-link permission', function () {
96+
afterEach(async function () {
97+
await agent.loginAsOwner();
98+
});
99+
100+
it('403s for an Author', async function () {
101+
await agent.loginAsAuthor();
102+
await agent.get(`posts/${postId}/gift_links/`).expectStatus(403);
103+
});
104+
105+
it('403s for a Contributor', async function () {
106+
await agent.loginAsContributor();
107+
await agent.get(`posts/${postId}/gift_links/`).expectStatus(403);
108+
});
109+
});
110+
98111
describe('remove_all', function () {
99112
it('returns the count in a meta block, not as a resource', async function () {
100113
await agent.put(`posts/${postId}/gift_links/`).expectStatus(200);

ghost/core/test/integration/migrations/migration.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ describe('Migrations', function () {
111111
assertHavePermission(permissions, 'Delete posts', ['Administrator', 'Editor', 'Author', 'Contributor', 'Admin Integration', 'Super Editor']);
112112
assertHavePermission(permissions, 'Publish posts', ['Administrator', 'Editor', 'Admin Integration', 'Scheduler Integration', 'Super Editor']);
113113
assertHavePermission(permissions, 'Flush gift reminders', ['Scheduler Integration']);
114-
assertHavePermission(permissions, 'Manage gift links', ['Administrator', 'Editor', 'Author', 'Admin Integration', 'Super Editor']);
114+
assertHavePermission(permissions, 'Manage gift links', ['Administrator', 'Editor', 'Admin Integration', 'Super Editor']);
115115
assertHavePermission(permissions, 'Remove all gift links', ['Administrator']);
116116

117117
assertHavePermission(permissions, 'Browse settings', ['Administrator', 'Editor', 'Author', 'Contributor', 'Admin Integration', 'Super Editor']);

ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ describe('Migration Fixture Utils', function () {
398398
const rolesAllStub = sinon.stub(models.Role, 'findAll').returns(Promise.resolve(dataMethodStub));
399399

400400
const result = await fixtureManager.addFixturesForRelation(fixtures.relations[0]);
401-
const FIXTURE_COUNT = 148;
401+
const FIXTURE_COUNT = 147;
402402
assertExists(result);
403403
assert(_.isPlainObject(result));
404404
assert.equal(result.expected, FIXTURE_COUNT);

ghost/core/test/unit/server/data/schema/integrity.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route
3636
describe('DB version integrity', function () {
3737
// Only these variables should need updating
3838
const currentSchemaHash = '83bd80e87f81e4663242965e5048fc7a';
39-
const currentFixturesHash = 'f4941d9d92b59075e0c2a1cc3fc4c44a';
39+
const currentFixturesHash = '16c0d239e8d04682ccb1894124179289';
4040
const currentSettingsHash = '397be8628c753b1959b8954d5610f83f';
4141
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';
4242

ghost/core/test/utils/fixtures/fixtures.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,8 +1220,7 @@
12201220
"product": ["browse", "read"],
12211221
"newsletter": ["browse", "read"],
12221222
"collection": ["browse", "read", "add"],
1223-
"recommendation": ["browse", "read"],
1224-
"gift_link": "manage"
1223+
"recommendation": ["browse", "read"]
12251224
},
12261225
"Super Editor": {
12271226
"notification": "all",

0 commit comments

Comments
 (0)