From a89e2d0bf495be756b999d54b190ad0d5950d26c Mon Sep 17 00:00:00 2001 From: Cathy Sarisky <42299862+cathysarisky@users.noreply.github.com> Date: Wed, 16 Oct 2024 09:30:36 -0400 Subject: [PATCH] Fixed members/signin_urls endpoint to take admin api key (#21284) closes #16748 The members/:member_id/signin_urls endpoint currently only does cookie-based authentication. When #21249 is merged, turning on 2FA is going to break any 3rd party processes that use it (including my social sign-in offering). This patch gives admin API keys 'read' permission on this endpoint, and enables 3rd party processes to handle user logins the right way, instead of via a staff member's email/password. Migration included. Feedback appreciated. I have the wrong name on my migration. I can see it doesn't follow the naming convention, but I'm not sure how the names are generated. --------- Co-authored-by: Michael Barrett --- ...10-01-02-03-add-signin-urls-permissions.js | 6 ++++ .../server/data/schema/fixtures/fixtures.json | 3 +- .../integration/migrations/migration.test.js | 1 + .../api/admin/members-signin-url.test.js | 36 +++++++++++++++++++ .../schema/fixtures/fixture-manager.test.js | 2 +- .../unit/server/data/schema/integrity.test.js | 2 +- ghost/core/test/utils/fixtures/fixtures.json | 1 + 7 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/5.97/2024-10-10-01-02-03-add-signin-urls-permissions.js diff --git a/ghost/core/core/server/data/migrations/versions/5.97/2024-10-10-01-02-03-add-signin-urls-permissions.js b/ghost/core/core/server/data/migrations/versions/5.97/2024-10-10-01-02-03-add-signin-urls-permissions.js new file mode 100644 index 00000000000..629010d7089 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.97/2024-10-10-01-02-03-add-signin-urls-permissions.js @@ -0,0 +1,6 @@ +const {addPermissionToRole} = require('../../utils'); + +module.exports = addPermissionToRole({ + permission: 'Read member signin urls', + role: 'Admin Integration' +}); \ No newline at end of file diff --git a/ghost/core/core/server/data/schema/fixtures/fixtures.json b/ghost/core/core/server/data/schema/fixtures/fixtures.json index e6dd4b8d017..1d83925ad0f 100644 --- a/ghost/core/core/server/data/schema/fixtures/fixtures.json +++ b/ghost/core/core/server/data/schema/fixtures/fixtures.json @@ -907,7 +907,8 @@ "link": "all", "mention": "browse", "collection": "all", - "recommendation": "all" + "recommendation": "all", + "member_signin_url": "read" }, "Editor": { "notification": "all", diff --git a/ghost/core/test/integration/migrations/migration.test.js b/ghost/core/test/integration/migrations/migration.test.js index 0da8cf8d35b..8cde2edc96b 100644 --- a/ghost/core/test/integration/migrations/migration.test.js +++ b/ghost/core/test/integration/migrations/migration.test.js @@ -242,6 +242,7 @@ describe('Migrations', function () { permissions.should.havePermission('Edit collections', ['Administrator', 'Editor', 'Admin Integration']); permissions.should.havePermission('Add collections', ['Administrator', 'Editor', 'Author', 'Admin Integration']); permissions.should.havePermission('Delete collections', ['Administrator', 'Editor', 'Admin Integration']); + permissions.should.havePermission('Read member signin urls', ['Administrator', 'Admin Integration']); }); describe('Populate', function () { diff --git a/ghost/core/test/regression/api/admin/members-signin-url.test.js b/ghost/core/test/regression/api/admin/members-signin-url.test.js index a9d27bda5a0..2f74949ab70 100644 --- a/ghost/core/test/regression/api/admin/members-signin-url.test.js +++ b/ghost/core/test/regression/api/admin/members-signin-url.test.js @@ -99,4 +99,40 @@ describe('Members Sigin URL API', function () { .expect(403); }); }); + describe('With an admin API key', function () { + let key, token; + before(async function () { + await localUtils.startGhost(); + request = supertest.agent(config.get('url')); + await testUtils.initFixtures('members', 'api_keys'); + + key = testUtils.DataGenerator.Content.api_keys[0]; + token = localUtils.getValidAdminToken('/admin/', key); + }); + it('Cannot read without the key', function () { + return request + .get(localUtils.API.getApiQuery(`members/${testUtils.DataGenerator.Content.members[0].id}/signin_urls/`)) + .set('Origin', config.get('url')) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(403); + }); + it('Can read with a key', function () { + return request + .get(localUtils.API.getApiQuery(`members/${testUtils.DataGenerator.Content.members[0].id}/signin_urls/`)) + .set('Origin', config.get('url')) + .set('Content-Type', 'application/json') + .set('Authorization', `Ghost ${token}`) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .then((res) => { + should.not.exist(res.headers['x-cache-invalidate']); + const jsonResponse = res.body; + should.exist(jsonResponse); + should.exist(jsonResponse.member_signin_urls); + jsonResponse.member_signin_urls.should.have.length(1); + localUtils.API.checkResponse(jsonResponse.member_signin_urls[0], 'member_signin_url'); + }); + }); + }); }); diff --git a/ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js b/ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js index 149c3e60857..db93ef9d931 100644 --- a/ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js +++ b/ghost/core/test/unit/server/data/schema/fixtures/fixture-manager.test.js @@ -191,7 +191,7 @@ describe('Migration Fixture Utils', function () { const rolesAllStub = sinon.stub(models.Role, 'findAll').returns(Promise.resolve(dataMethodStub)); fixtureManager.addFixturesForRelation(fixtures.relations[0]).then(function (result) { - const FIXTURE_COUNT = 111; + const FIXTURE_COUNT = 112; should.exist(result); result.should.be.an.Object(); result.should.have.property('expected', FIXTURE_COUNT); diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 8242d198c28..30ebf3ac67f 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -36,7 +36,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = 'a4f016480ff73c6f52ee4c86482b45a7'; - const currentFixturesHash = 'a489d615989eab1023d4b8af0ecee7fd'; + const currentFixturesHash = '475f488105c390bb0018db90dce845f1'; const currentSettingsHash = '051ef2a50e2edb8723e89461448313cb'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01'; diff --git a/ghost/core/test/utils/fixtures/fixtures.json b/ghost/core/test/utils/fixtures/fixtures.json index c6f10459ba0..67248e2d37e 100644 --- a/ghost/core/test/utils/fixtures/fixtures.json +++ b/ghost/core/test/utils/fixtures/fixtures.json @@ -1071,6 +1071,7 @@ "webhook": "all", "action": "all", "member": "all", + "member_signin_url": "read", "label": "all", "email_preview": "all", "email": "all",