Skip to content

Commit

Permalink
Fixed members/signin_urls endpoint to take admin api key (#21284)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
2 people authored and dvdwinden committed Oct 24, 2024
1 parent ff78512 commit 2aa6852
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const {addPermissionToRole} = require('../../utils');

module.exports = addPermissionToRole({
permission: 'Read member signin urls',
role: 'Admin Integration'
});
3 changes: 2 additions & 1 deletion ghost/core/core/server/data/schema/fixtures/fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,8 @@
"link": "all",
"mention": "browse",
"collection": "all",
"recommendation": "all"
"recommendation": "all",
"member_signin_url": "read"
},
"Editor": {
"notification": "all",
Expand Down
1 change: 1 addition & 0 deletions ghost/core/test/integration/migrations/migration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
36 changes: 36 additions & 0 deletions ghost/core/test/regression/api/admin/members-signin-url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion ghost/core/test/unit/server/data/schema/integrity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
1 change: 1 addition & 0 deletions ghost/core/test/utils/fixtures/fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,7 @@
"webhook": "all",
"action": "all",
"member": "all",
"member_signin_url": "read",
"label": "all",
"email_preview": "all",
"email": "all",
Expand Down

0 comments on commit 2aa6852

Please sign in to comment.