From b0ff1e7cacf619deed1d4f07c3cb0e7731707597 Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 27 Feb 2020 11:48:02 +0800 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Added=20member=20login=20resource?= =?UTF-8?q?=20to=20Admin=20API=20(#11607)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - Adds 'GET /members/:id/signin_urls' endpoint to Admin API allowing to fetch login URL for member. This URL allows to log in as a member which is useful in situations when you need to impersonate a member (for example to debug some issue they are having) - Added member_signin_urls permission with migrations. Only the "Owner" user can read "signin_urls" resource. Admin and other users will be denied access --- core/server/api/canary/index.js | 4 + core/server/api/canary/memberSigninUrls.js | 30 +++++++ .../canary/utils/serializers/output/index.js | 4 + .../serializers/output/member-signin_urls.js | 7 ++ .../01-add-member-sigin-url-permissions.js | 58 ++++++++++++++ .../server/data/schema/fixtures/fixtures.json | 5 ++ core/server/web/api/canary/admin/routes.js | 2 + .../canary/admin/members_signin_url_spec.js | 80 +++++++++++++++++++ .../test/regression/api/canary/admin/utils.js | 1 + core/test/unit/data/schema/integrity_spec.js | 2 +- 10 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 core/server/api/canary/memberSigninUrls.js create mode 100644 core/server/api/canary/utils/serializers/output/member-signin_urls.js create mode 100644 core/server/data/migrations/versions/3.9/01-add-member-sigin-url-permissions.js create mode 100644 core/test/regression/api/canary/admin/members_signin_url_spec.js diff --git a/core/server/api/canary/index.js b/core/server/api/canary/index.js index a4fe39a672..8de45184cd 100644 --- a/core/server/api/canary/index.js +++ b/core/server/api/canary/index.js @@ -71,6 +71,10 @@ module.exports = { return shared.pipeline(require('./members'), localUtils); }, + get memberSigninUrls() { + return shared.pipeline(require('./memberSigninUrls.js'), localUtils); + }, + get labels() { return shared.pipeline(require('./labels'), localUtils); }, diff --git a/core/server/api/canary/memberSigninUrls.js b/core/server/api/canary/memberSigninUrls.js new file mode 100644 index 0000000000..f464dcc4a9 --- /dev/null +++ b/core/server/api/canary/memberSigninUrls.js @@ -0,0 +1,30 @@ +const models = require('../../models'); +const common = require('../../lib/common'); +const membersService = require('../../services/members'); + +module.exports = { + docName: 'member_signin_urls', + permissions: true, + read: { + data: [ + 'id' + ], + permissions: true, + async query(frame) { + let model = await models.Member.findOne(frame.data, frame.options); + + if (!model) { + throw new common.errors.NotFoundError({ + message: common.i18n.t('errors.api.members.memberNotFound') + }); + } + + const magicLink = membersService.api.getMagicLink(model.get('email')); + + return { + member_id: model.get('id'), + url: magicLink + }; + } + } +}; diff --git a/core/server/api/canary/utils/serializers/output/index.js b/core/server/api/canary/utils/serializers/output/index.js index 7d7990225b..4039e3b5bd 100644 --- a/core/server/api/canary/utils/serializers/output/index.js +++ b/core/server/api/canary/utils/serializers/output/index.js @@ -63,6 +63,10 @@ module.exports = { return require('./members'); }, + get member_signin_urls() { + return require('./member-signin_urls'); + }, + get images() { return require('./images'); }, diff --git a/core/server/api/canary/utils/serializers/output/member-signin_urls.js b/core/server/api/canary/utils/serializers/output/member-signin_urls.js new file mode 100644 index 0000000000..38bfc66b09 --- /dev/null +++ b/core/server/api/canary/utils/serializers/output/member-signin_urls.js @@ -0,0 +1,7 @@ +module.exports = { + read(data, apiConfig, frame) { + frame.response = { + member_signin_urls: [data] + }; + } +}; diff --git a/core/server/data/migrations/versions/3.9/01-add-member-sigin-url-permissions.js b/core/server/data/migrations/versions/3.9/01-add-member-sigin-url-permissions.js new file mode 100644 index 0000000000..5a5b72e110 --- /dev/null +++ b/core/server/data/migrations/versions/3.9/01-add-member-sigin-url-permissions.js @@ -0,0 +1,58 @@ +const _ = require('lodash'); +const utils = require('../../../schema/fixtures/utils'); +const permissions = require('../../../../services/permissions'); +const logging = require('../../../../lib/common/logging'); + +const resources = ['member_signin_url']; +const _private = {}; + +_private.getPermissions = function getPermissions(resource) { + return utils.findModelFixtures('Permission', {object_type: resource}); +}; + +_private.getRelations = function getRelations(resource) { + return utils.findPermissionRelationsForObject(resource); +}; + +_private.printResult = function printResult(result, message) { + if (result.done === result.expected) { + logging.info(message); + } else { + logging.warn(`(${result.done}/${result.expected}) ${message}`); + } +}; + +module.exports.config = { + transaction: true +}; + +module.exports.up = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToAdd = _private.getPermissions(resource); + const relationToAdd = _private.getRelations(resource); + + return utils.addFixturesForModel(modelToAdd, localOptions) + .then(result => _private.printResult(result, `Adding permissions fixtures for ${resource}s`)) + .then(() => utils.addFixturesForRelation(relationToAdd, localOptions)) + .then(result => _private.printResult(result, `Adding permissions_roles fixtures for ${resource}s`)) + .then(() => permissions.init(localOptions)); + }); +}; + +module.exports.down = (options) => { + const localOptions = _.merge({ + context: {internal: true} + }, options); + + return Promise.map(resources, (resource) => { + const modelToRemove = _private.getPermissions(resource); + + // permission model automatically cleans up permissions_roles on .destroy() + return utils.removeFixturesForModel(modelToRemove, localOptions) + .then(result => _private.printResult(result, `Removing permissions fixtures for ${resource}s`)); + }); +}; diff --git a/core/server/data/schema/fixtures/fixtures.json b/core/server/data/schema/fixtures/fixtures.json index f0b8b53d76..fc09d9e31f 100644 --- a/core/server/data/schema/fixtures/fixtures.json +++ b/core/server/data/schema/fixtures/fixtures.json @@ -412,6 +412,11 @@ "name": "Delete labels", "action_type": "destroy", "object_type": "label" + }, + { + "name": "Read member signin urls", + "action_type": "read", + "object_type": "member_signin_url" } ] }, diff --git a/core/server/web/api/canary/admin/routes.js b/core/server/web/api/canary/admin/routes.js index 85a2abbf71..1563a2fa7a 100644 --- a/core/server/web/api/canary/admin/routes.js +++ b/core/server/web/api/canary/admin/routes.js @@ -100,6 +100,8 @@ module.exports = function apiRoutes() { router.put('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.edit)); router.del('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.destroy)); + router.get('/members/:id/signin_urls', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.memberSigninUrls.read)); + // ## Labels router.get('/labels', mw.authAdminApi, http(apiCanary.labels.browse)); router.get('/labels/:id', mw.authAdminApi, http(apiCanary.labels.read)); diff --git a/core/test/regression/api/canary/admin/members_signin_url_spec.js b/core/test/regression/api/canary/admin/members_signin_url_spec.js new file mode 100644 index 0000000000..b75c14689e --- /dev/null +++ b/core/test/regression/api/canary/admin/members_signin_url_spec.js @@ -0,0 +1,80 @@ +const path = require('path'); +const should = require('should'); +const supertest = require('supertest'); +const sinon = require('sinon'); +const testUtils = require('../../../../utils'); +const localUtils = require('./utils'); +const config = require('../../../../../server/config'); +const labs = require('../../../../../server/services/labs'); + +const ghost = testUtils.startGhost; + +let request; + +describe('Members Sigin URL API', function () { + before(function () { + sinon.stub(labs, 'isSet').withArgs('members').returns(true); + }); + + after(function () { + sinon.restore(); + }); + + describe('As Owner', function () { + before(function () { + return ghost() + .then(function () { + request = supertest.agent(config.get('url')); + }) + .then(function () { + return localUtils.doAuth(request, 'member'); + }); + }); + + it('Can read', function () { + return request + .get(localUtils.API.getApiQuery(`members/${testUtils.DataGenerator.Content.members[0].id}/signin_urls/`)) + .set('Origin', config.get('url')) + .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'); + }); + }); + }); + + describe('As non-Owner', function () { + before(function () { + return ghost() + .then(function (_ghostServer) { + request = supertest.agent(config.get('url')); + }) + .then(function () { + return testUtils.createUser({ + user: testUtils.DataGenerator.forKnex.createUser({email: 'admin+1@ghost.org'}), + role: testUtils.DataGenerator.Content.roles[0].name + }); + }) + .then(function (admin) { + request.user = admin; + + return localUtils.doAuth(request, 'member'); + }); + }); + + it('Cannot read', function () { + return request + .get(localUtils.API.getApiQuery(`members/${testUtils.DataGenerator.Content.members[0].id}/signin_urls/`)) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(403); + }); + }); +}); diff --git a/core/test/regression/api/canary/admin/utils.js b/core/test/regression/api/canary/admin/utils.js index 71e718e7c7..9714c384a7 100644 --- a/core/test/regression/api/canary/admin/utils.js +++ b/core/test/regression/api/canary/admin/utils.js @@ -59,6 +59,7 @@ const expectedProperties = { .concat('comped') .concat('labels') , + member_signin_url: ['member_id', 'url'], role: _(schema.roles) .keys() , diff --git a/core/test/unit/data/schema/integrity_spec.js b/core/test/unit/data/schema/integrity_spec.js index 29bdb51057..91e33a186b 100644 --- a/core/test/unit/data/schema/integrity_spec.js +++ b/core/test/unit/data/schema/integrity_spec.js @@ -20,7 +20,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = '7cd198f085844aa5725964069b051189'; - const currentFixturesHash = '0ca1c9a6d3dab21d8a1e0b6a988fd83f'; + const currentFixturesHash = 'b2e26827d712513907054782a0be5735'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation