From dd214d71dc618f110092d93dcb0eff5e06cd55b0 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Wed, 9 Oct 2019 14:14:26 +0700 Subject: [PATCH 01/33] Decoupled `add` from `importCSV` queries no-issue --- core/server/api/canary/members.js | 40 ++++++++++++------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/core/server/api/canary/members.js b/core/server/api/canary/members.js index bfd6231cc1..59431644cc 100644 --- a/core/server/api/canary/members.js +++ b/core/server/api/canary/members.js @@ -60,30 +60,20 @@ const members = { } }, permissions: true, - query(frame) { - // NOTE: Promise.resolve() is here for a reason! Method has to return an instance - // of a Bluebird promise to allow reflection. If decided to be replaced - // with something else, e.g: async/await, CSV export function - // would need a deep rewrite (see failing tests if this line is removed) - return Promise.resolve() - .then(() => { - return membersService.api.members.create(frame.data.members[0], { - sendEmail: frame.options.send_email, - emailType: frame.options.email_type - }); - }) - .then((member) => { - if (member) { - return Promise.resolve(member); - } - }) - .catch((error) => { - if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { - return Promise.reject(new common.errors.ValidationError({message: common.i18n.t('errors.api.members.memberAlreadyExists')})); - } - - return Promise.reject(error); + async query(frame) { + try { + const member = await membersService.api.members.create(frame.data.members[0], { + sendEmail: frame.options.send_email, + emailType: frame.options.email_type }); + return member; + } catch (error) { + if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { + throw new common.errors.ValidationError({message: common.i18n.t('errors.api.members.memberAlreadyExists')}); + } + + throw error; + } } }, @@ -168,7 +158,7 @@ const members = { return Promise.all(result.map((entry) => { const api = require('./index'); - return api.members.add.query({ + return Promise.resolve(api.members.add.query({ data: { members: [{ email: entry.email, @@ -179,7 +169,7 @@ const members = { context: frame.options.context, options: {send_email: false} } - }).reflect(); + })).reflect(); })).each((inspection) => { if (inspection.isFulfilled()) { fulfilled = fulfilled + 1; From 58651caa32532f04b5fd11f4105827ee4223aa3c Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Wed, 9 Oct 2019 14:16:13 +0700 Subject: [PATCH 02/33] Removed members endpoint from admin v2 api no-issue --- core/server/api/v2/index.js | 4 - core/server/api/v2/members.js | 209 -------------- .../v2/utils/serializers/output/members.js | 77 ------ .../api/v2/utils/validators/input/members.js | 15 -- .../validators/input/schemas/members-add.json | 22 -- .../input/schemas/members-edit.json | 21 -- .../validators/input/schemas/members.json | 42 --- core/server/web/api/v2/admin/routes.js | 17 -- .../regression/api/v2/admin/members_spec.js | 255 ------------------ 9 files changed, 662 deletions(-) delete mode 100644 core/server/api/v2/members.js delete mode 100644 core/server/api/v2/utils/serializers/output/members.js delete mode 100644 core/server/api/v2/utils/validators/input/members.js delete mode 100644 core/server/api/v2/utils/validators/input/schemas/members-add.json delete mode 100644 core/server/api/v2/utils/validators/input/schemas/members-edit.json delete mode 100644 core/server/api/v2/utils/validators/input/schemas/members.json delete mode 100644 core/test/regression/api/v2/admin/members_spec.js diff --git a/core/server/api/v2/index.js b/core/server/api/v2/index.js index ce50008de0..2932b32461 100644 --- a/core/server/api/v2/index.js +++ b/core/server/api/v2/index.js @@ -71,10 +71,6 @@ module.exports = { return shared.pipeline(require('./subscribers'), localUtils); }, - get members() { - return shared.pipeline(require('./members'), localUtils); - }, - get images() { return shared.pipeline(require('./images'), localUtils); }, diff --git a/core/server/api/v2/members.js b/core/server/api/v2/members.js deleted file mode 100644 index bfd6231cc1..0000000000 --- a/core/server/api/v2/members.js +++ /dev/null @@ -1,209 +0,0 @@ -// NOTE: We must not cache references to membersService.api -// as it is a getter and may change during runtime. -const Promise = require('bluebird'); -const membersService = require('../../services/members'); -const common = require('../../lib/common'); -const fsLib = require('../../lib/fs'); - -const members = { - docName: 'members', - browse: { - options: [ - 'limit', - 'fields', - 'filter', - 'order', - 'debug', - 'page' - ], - permissions: true, - validation: {}, - query(frame) { - return membersService.api.members.list(frame.options); - } - }, - - read: { - headers: {}, - data: [ - 'id', - 'email' - ], - validation: {}, - permissions: true, - async query(frame) { - const member = await membersService.api.members.get(frame.data, frame.options); - if (!member) { - throw new common.errors.NotFoundError({ - message: common.i18n.t('errors.api.members.memberNotFound') - }); - } - return member; - } - }, - - add: { - statusCode: 201, - headers: {}, - options: [ - 'send_email', - 'email_type' - ], - validation: { - data: { - email: {required: true} - }, - options: { - email_type: { - values: ['signin', 'signup', 'subscribe'] - } - } - }, - permissions: true, - query(frame) { - // NOTE: Promise.resolve() is here for a reason! Method has to return an instance - // of a Bluebird promise to allow reflection. If decided to be replaced - // with something else, e.g: async/await, CSV export function - // would need a deep rewrite (see failing tests if this line is removed) - return Promise.resolve() - .then(() => { - return membersService.api.members.create(frame.data.members[0], { - sendEmail: frame.options.send_email, - emailType: frame.options.email_type - }); - }) - .then((member) => { - if (member) { - return Promise.resolve(member); - } - }) - .catch((error) => { - if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { - return Promise.reject(new common.errors.ValidationError({message: common.i18n.t('errors.api.members.memberAlreadyExists')})); - } - - return Promise.reject(error); - }); - } - }, - - edit: { - statusCode: 200, - headers: {}, - options: [ - 'id' - ], - validation: { - options: { - id: { - required: true - } - } - }, - permissions: true, - async query(frame) { - const member = await membersService.api.members.update(frame.data.members[0], frame.options); - return member; - } - }, - - destroy: { - statusCode: 204, - headers: {}, - options: [ - 'id' - ], - validation: { - options: { - id: { - required: true - } - } - }, - permissions: true, - async query(frame) { - frame.options.require = true; - await membersService.api.members.destroy(frame.options); - return null; - } - }, - - exportCSV: { - headers: { - disposition: { - type: 'csv', - value() { - const datetime = (new Date()).toJSON().substring(0, 10); - return `members.${datetime}.csv`; - } - } - }, - response: { - format: 'plain' - }, - permissions: { - method: 'browse' - }, - validation: {}, - query(frame) { - return membersService.api.members.list(frame.options); - } - }, - - importCSV: { - statusCode: 201, - permissions: { - method: 'add' - }, - async query(frame) { - let filePath = frame.file.path, - fulfilled = 0, - invalid = 0, - duplicates = 0; - - return fsLib.readCSV({ - path: filePath, - columnsToExtract: [{name: 'email', lookup: /email/i}, {name: 'name', lookup: /name/i}] - }).then((result) => { - return Promise.all(result.map((entry) => { - const api = require('./index'); - - return api.members.add.query({ - data: { - members: [{ - email: entry.email, - name: entry.name - }] - }, - options: { - context: frame.options.context, - options: {send_email: false} - } - }).reflect(); - })).each((inspection) => { - if (inspection.isFulfilled()) { - fulfilled = fulfilled + 1; - } else { - if (inspection.reason() instanceof common.errors.ValidationError) { - duplicates = duplicates + 1; - } else { - invalid = invalid + 1; - } - } - }); - }).then(() => { - return { - meta: { - stats: { - imported: fulfilled, - duplicates: duplicates, - invalid: invalid - } - } - }; - }); - } - } -}; - -module.exports = members; diff --git a/core/server/api/v2/utils/serializers/output/members.js b/core/server/api/v2/utils/serializers/output/members.js deleted file mode 100644 index 4eeca69f5e..0000000000 --- a/core/server/api/v2/utils/serializers/output/members.js +++ /dev/null @@ -1,77 +0,0 @@ -const common = require('../../../../../lib/common'); -const debug = require('ghost-ignition').debug('api:v2:utils:serializers:output:members'); - -module.exports = { - browse(data, apiConfig, frame) { - debug('browse'); - - frame.response = data; - }, - - add(data, apiConfig, frame) { - debug('add'); - - frame.response = { - members: [data] - }; - }, - - edit(data, apiConfig, frame) { - debug('edit'); - - frame.response = { - members: [data] - }; - }, - - read(data, apiConfig, frame) { - debug('read'); - - if (!data) { - return Promise.reject(new common.errors.NotFoundError({ - message: common.i18n.t('errors.api.members.memberNotFound') - })); - } - - frame.response = { - members: [data] - }; - }, - - exportCSV(models, apiConfig, frame) { - debug('exportCSV'); - - const fields = ['id', 'email', 'name', 'created_at', 'deleted_at']; - - function formatCSV(data) { - let csv = `${fields.join(',')}\r\n`, - entry, - field, - j, - i; - - for (j = 0; j < data.length; j = j + 1) { - entry = data[j]; - - for (i = 0; i < fields.length; i = i + 1) { - field = fields[i]; - csv += entry[field] !== null ? entry[field] : ''; - if (i !== fields.length - 1) { - csv += ','; - } - } - csv += '\r\n'; - } - - return csv; - } - - frame.response = formatCSV(models.members); - }, - - importCSV(data, apiConfig, frame) { - debug('importCSV'); - - frame.response = data; - } -}; diff --git a/core/server/api/v2/utils/validators/input/members.js b/core/server/api/v2/utils/validators/input/members.js deleted file mode 100644 index aa01b06aeb..0000000000 --- a/core/server/api/v2/utils/validators/input/members.js +++ /dev/null @@ -1,15 +0,0 @@ -const jsonSchema = require('../utils/json-schema'); - -module.exports = { - add(apiConfig, frame) { - const schema = require('./schemas/members-add'); - const definitions = require('./schemas/members'); - return jsonSchema.validate(schema, definitions, frame.data); - }, - - edit(apiConfig, frame) { - const schema = require('./schemas/members-edit'); - const definitions = require('./schemas/members'); - return jsonSchema.validate(schema, definitions, frame.data); - } -}; diff --git a/core/server/api/v2/utils/validators/input/schemas/members-add.json b/core/server/api/v2/utils/validators/input/schemas/members-add.json deleted file mode 100644 index d6370b6595..0000000000 --- a/core/server/api/v2/utils/validators/input/schemas/members-add.json +++ /dev/null @@ -1,22 +0,0 @@ - -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "members.add", - "title": "members.add", - "description": "Schema for members.add", - "type": "object", - "additionalProperties": false, - "properties": { - "members": { - "type": "array", - "minItems": 1, - "maxItems": 1, - "items": { - "type": "object", - "allOf": [{"$ref": "members#/definitions/member"}], - "required": ["email"] - } - } - }, - "required": ["members"] - } diff --git a/core/server/api/v2/utils/validators/input/schemas/members-edit.json b/core/server/api/v2/utils/validators/input/schemas/members-edit.json deleted file mode 100644 index a1a9adf791..0000000000 --- a/core/server/api/v2/utils/validators/input/schemas/members-edit.json +++ /dev/null @@ -1,21 +0,0 @@ - -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "members.edit", - "title": "members.edit", - "description": "Schema for members.edit", - "type": "object", - "additionalProperties": false, - "properties": { - "members": { - "type": "array", - "minItems": 1, - "maxItems": 1, - "items": { - "type": "object", - "allOf": [{"$ref": "members#/definitions/member"}] - } - } - }, - "required": ["members"] - } diff --git a/core/server/api/v2/utils/validators/input/schemas/members.json b/core/server/api/v2/utils/validators/input/schemas/members.json deleted file mode 100644 index 1a2dc9b7ac..0000000000 --- a/core/server/api/v2/utils/validators/input/schemas/members.json +++ /dev/null @@ -1,42 +0,0 @@ - -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "members", - "title": "members", - "description": "Base members definitions", - "definitions": { - "member": { - "type": "object", - "additionalProperties": false, - "properties": { - "name": { - "type": "string", - "minLength": 1, - "maxLength": 191, - "pattern": "^([^,]|$)" - }, - "email": { - "type": "string", - "minLength": 1, - "maxLength": 191, - "pattern": "^([^,]|$)" - }, - "id": { - "strip": true - }, - "created_at": { - "strip": true - }, - "created_by": { - "strip": true - }, - "updated_at": { - "strip": true - }, - "updated_by": { - "strip": true - } - } - } - } -} diff --git a/core/server/web/api/v2/admin/routes.js b/core/server/web/api/v2/admin/routes.js index a17e3bdc8e..b99a42af25 100644 --- a/core/server/web/api/v2/admin/routes.js +++ b/core/server/web/api/v2/admin/routes.js @@ -101,23 +101,6 @@ module.exports = function apiRoutes() { router.del('/subscribers/:id', shared.middlewares.labs.subscribers, mw.authAdminApi, http(apiv2.subscribers.destroy)); router.del('/subscribers/email/:email', shared.middlewares.labs.subscribers, mw.authAdminApi, http(apiv2.subscribers.destroy)); - // ## Members - router.get('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.browse)); - router.post('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.add)); - - router.get('/members/csv', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.exportCSV)); - router.post('/members/csv', - shared.middlewares.labs.members, - mw.authAdminApi, - upload.single('membersfile'), - shared.middlewares.validation.upload({type: 'members'}), - http(apiv2.members.importCSV) - ); - - router.get('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.read)); - router.put('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.edit)); - router.del('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.destroy)); - // ## Roles router.get('/roles/', mw.authAdminApi, http(apiv2.roles.browse)); diff --git a/core/test/regression/api/v2/admin/members_spec.js b/core/test/regression/api/v2/admin/members_spec.js deleted file mode 100644 index 779f2ac1f2..0000000000 --- a/core/test/regression/api/v2/admin/members_spec.js +++ /dev/null @@ -1,255 +0,0 @@ -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 API', function () { - before(function () { - sinon.stub(labs, 'isSet').withArgs('members').returns(true); - }); - - after(function () { - sinon.restore(); - }); - - before(function () { - return ghost() - .then(function () { - request = supertest.agent(config.get('url')); - }) - .then(function () { - return localUtils.doAuth(request, 'member'); - }); - }); - - it('Can browse', function () { - return request - .get(localUtils.API.getApiQuery('members/')) - .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.members); - jsonResponse.members.should.have.length(1); - localUtils.API.checkResponse(jsonResponse.members[0], 'member'); - - testUtils.API.isISO8601(jsonResponse.members[0].created_at).should.be.true(); - jsonResponse.members[0].created_at.should.be.an.instanceof(String); - - jsonResponse.meta.pagination.should.have.property('page', 1); - jsonResponse.meta.pagination.should.have.property('limit', 15); - jsonResponse.meta.pagination.should.have.property('pages', 1); - jsonResponse.meta.pagination.should.have.property('total', 1); - jsonResponse.meta.pagination.should.have.property('next', null); - jsonResponse.meta.pagination.should.have.property('prev', null); - }); - }); - - it('Can read', function () { - return request - .get(localUtils.API.getApiQuery(`members/${testUtils.DataGenerator.Content.members[0].id}/`)) - .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.members); - jsonResponse.members.should.have.length(1); - localUtils.API.checkResponse(jsonResponse.members[0], 'member', 'stripe'); - }); - }); - - it('Can add', function () { - const member = { - name: 'test', - email: 'memberTestAdd@test.com' - }; - - return request - .post(localUtils.API.getApiQuery(`members/`)) - .send({members: [member]}) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(201) - .then((res) => { - should.not.exist(res.headers['x-cache-invalidate']); - const jsonResponse = res.body; - should.exist(jsonResponse); - should.exist(jsonResponse.members); - jsonResponse.members.should.have.length(1); - - jsonResponse.members[0].name.should.equal(member.name); - jsonResponse.members[0].email.should.equal(member.email); - }) - .then(() => { - return request - .post(localUtils.API.getApiQuery(`members/`)) - .send({members: [member]}) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(422); - }); - }); - - it('Should fail when passing incorrect email_type query parameter', function () { - const member = { - name: 'test', - email: 'memberTestAdd@test.com' - }; - - return request - .post(localUtils.API.getApiQuery(`members/?send_email=true&email_type=lel`)) - .send({members: [member]}) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(422); - }); - - it('Can edit by id', function () { - const memberToChange = { - name: 'change me', - email: 'member2Change@test.com' - }; - - const memberChanged = { - name: 'changed', - email: 'cantChangeMe@test.com' - }; - - return request - .post(localUtils.API.getApiQuery(`members/`)) - .send({members: [memberToChange]}) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(201) - .then((res) => { - should.not.exist(res.headers['x-cache-invalidate']); - const jsonResponse = res.body; - should.exist(jsonResponse); - should.exist(jsonResponse.members); - jsonResponse.members.should.have.length(1); - - return jsonResponse.members[0]; - }) - .then((newMember) => { - return request - .put(localUtils.API.getApiQuery(`members/${newMember.id}/`)) - .send({members: [memberChanged]}) - .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.members); - jsonResponse.members.should.have.length(1); - localUtils.API.checkResponse(jsonResponse.members[0], 'member'); - jsonResponse.members[0].name.should.equal(memberChanged.name); - jsonResponse.members[0].email.should.not.equal(memberChanged.email); - jsonResponse.members[0].email.should.equal(memberToChange.email); - }); - }); - }); - - it('Can destroy', function () { - const member = { - name: 'test', - email: 'memberTestDestroy@test.com' - }; - - return request - .post(localUtils.API.getApiQuery(`members/`)) - .send({members: [member]}) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(201) - .then((res) => { - should.not.exist(res.headers['x-cache-invalidate']); - - const jsonResponse = res.body; - - should.exist(jsonResponse); - should.exist(jsonResponse.members); - - return jsonResponse.members[0]; - }) - .then((newMember) => { - return request - .delete(localUtils.API.getApiQuery(`members/${newMember.id}`)) - .set('Origin', config.get('url')) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(204) - .then(() => newMember); - }) - .then((newMember) => { - return request - .get(localUtils.API.getApiQuery(`members/${newMember.id}/`)) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(404); - }); - }); - - it('Can export CSV', function () { - return request - .get(localUtils.API.getApiQuery(`members/csv/`)) - .set('Origin', config.get('url')) - .expect('Content-Type', /text\/csv/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(200) - .then((res) => { - should.not.exist(res.headers['x-cache-invalidate']); - res.headers['content-disposition'].should.match(/Attachment;\sfilename="members/); - res.text.should.match(/id,email,name,created_at,deleted_at/); - res.text.should.match(/member1@test.com/); - res.text.should.match(/Mr Egg/); - }); - }); - - it('Can import CSV', function () { - return request - .post(localUtils.API.getApiQuery(`members/csv/`)) - .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/valid-members-import.csv')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(201) - .then((res) => { - should.not.exist(res.headers['x-cache-invalidate']); - const jsonResponse = res.body; - - should.exist(jsonResponse); - should.exist(jsonResponse.meta); - should.exist(jsonResponse.meta.stats); - - jsonResponse.meta.stats.imported.should.equal(2); - jsonResponse.meta.stats.duplicates.should.equal(0); - jsonResponse.meta.stats.invalid.should.equal(0); - }); - }); -}); From 0a40d11af9739d94ffdad4b78178bd6518a9b30c Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 11:18:09 +0700 Subject: [PATCH 03/33] Added note column to members table no-issue --- core/server/data/schema/schema.js | 1 + core/test/unit/data/schema/integrity_spec.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index b519e63c7d..654de757e4 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -386,6 +386,7 @@ module.exports = { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, email: {type: 'string', maxlength: 191, nullable: false, unique: true, validations: {isEmail: true}}, name: {type: 'string', maxlength: 191, nullable: true}, + note: {type: 'string', maxlength: 2000, nullable: true}, created_at: {type: 'dateTime', nullable: false}, created_by: {type: 'string', maxlength: 24, nullable: false}, updated_at: {type: 'dateTime', nullable: true}, diff --git a/core/test/unit/data/schema/integrity_spec.js b/core/test/unit/data/schema/integrity_spec.js index 891814c867..84e1b851bb 100644 --- a/core/test/unit/data/schema/integrity_spec.js +++ b/core/test/unit/data/schema/integrity_spec.js @@ -19,7 +19,7 @@ var should = require('should'), */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'a5341373370dc25009806963c1bc236f'; + const currentSchemaHash = 'c4a06e34d7792fd23d94f613b7d3d4fb'; const currentFixturesHash = 'c7b485fe2f16517295bd35c761129729'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, From 035cb55ca9d56c3543de9e4f23d82b20ae77ab26 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 11:24:31 +0700 Subject: [PATCH 04/33] Added migration for note column on members table no-issue --- .../2.35/01-add-note-to-members-table.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 core/server/data/migrations/versions/2.35/01-add-note-to-members-table.js diff --git a/core/server/data/migrations/versions/2.35/01-add-note-to-members-table.js b/core/server/data/migrations/versions/2.35/01-add-note-to-members-table.js new file mode 100644 index 0000000000..fbd51464f2 --- /dev/null +++ b/core/server/data/migrations/versions/2.35/01-add-note-to-members-table.js @@ -0,0 +1,28 @@ +const commands = require('../../../schema').commands; + +module.exports = { + + up: commands.createColumnMigration({ + table: 'members', + column: 'note', + dbIsInCorrectState(hasColumn) { + return hasColumn === true; + }, + operation: commands.addColumn, + operationVerb: 'Adding' + }), + + down: commands.createColumnMigration({ + table: 'members', + column: 'note', + dbIsInCorrectState(hasColumn) { + return hasColumn === false; + }, + operation: commands.dropColumn, + operationVerb: 'Dropping' + }), + + config: { + transaction: true + } +}; From fe59613867bbdc385665be029f54db61614c4d05 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 11:32:42 +0700 Subject: [PATCH 05/33] Wired up the note property to members-api no-issue --- core/server/services/members/api.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 901d3f452c..5e66590e43 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -7,10 +7,11 @@ const ghostVersion = require('../../lib/ghost-version'); const mail = require('../mail'); const models = require('../../models'); -function createMember({email, name}) { +function createMember({email, name, note}) { return models.Member.add({ email, - name + name, + note }).then((member) => { return member.toJSON(); }); @@ -70,8 +71,8 @@ async function getMetadata(module, member) { }; } -function updateMember({name}, options) { - return models.Member.edit({name}, options); +function updateMember({name, note}, options) { + return models.Member.edit({name, note}, options); } function deleteMember(options) { From f3a8119870556d54e1fa9eb78d23288f51e93f09 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 11:51:27 +0700 Subject: [PATCH 06/33] Added note column to csv import/export for members no-issue --- core/server/api/canary/members.js | 5 +++-- core/server/api/canary/utils/serializers/output/members.js | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/server/api/canary/members.js b/core/server/api/canary/members.js index 59431644cc..44f5c793cc 100644 --- a/core/server/api/canary/members.js +++ b/core/server/api/canary/members.js @@ -153,7 +153,7 @@ const members = { return fsLib.readCSV({ path: filePath, - columnsToExtract: [{name: 'email', lookup: /email/i}, {name: 'name', lookup: /name/i}] + columnsToExtract: [{name: 'email', lookup: /email/i}, {name: 'name', lookup: /name/i}, {name: 'note', lookup: /note/i}] }).then((result) => { return Promise.all(result.map((entry) => { const api = require('./index'); @@ -162,7 +162,8 @@ const members = { data: { members: [{ email: entry.email, - name: entry.name + name: entry.name, + note: entry.note }] }, options: { diff --git a/core/server/api/canary/utils/serializers/output/members.js b/core/server/api/canary/utils/serializers/output/members.js index 312a8429e6..be7b35f16e 100644 --- a/core/server/api/canary/utils/serializers/output/members.js +++ b/core/server/api/canary/utils/serializers/output/members.js @@ -41,7 +41,7 @@ module.exports = { exportCSV(models, apiConfig, frame) { debug('exportCSV'); - const fields = ['id', 'email', 'name', 'created_at', 'deleted_at']; + const fields = ['id', 'email', 'name', 'note', 'created_at', 'deleted_at']; function formatCSV(data) { let csv = `${fields.join(',')}\r\n`, From 38832d5c6b72527c3967cb56db6bd7e218771e5b Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 13:39:32 +0700 Subject: [PATCH 07/33] Added note to member json schema no-issue --- .../api/canary/utils/validators/input/schemas/members.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/server/api/canary/utils/validators/input/schemas/members.json b/core/server/api/canary/utils/validators/input/schemas/members.json index 1a2dc9b7ac..fdfaa42dd8 100644 --- a/core/server/api/canary/utils/validators/input/schemas/members.json +++ b/core/server/api/canary/utils/validators/input/schemas/members.json @@ -21,6 +21,11 @@ "maxLength": 191, "pattern": "^([^,]|$)" }, + "note": { + "type": "string", + "minLength": 0, + "maxLength": 2000 + }, "id": { "strip": true }, From 99681e692a7a48a094b591e1b0b9fbd06d672a99 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 13:45:43 +0700 Subject: [PATCH 08/33] Updated the create,get&update member functions no-issue This updates them to async functions, and defaults falsy name and note to null --- core/server/services/members/api.js | 36 +++++++++++++++++------------ 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 5e66590e43..cc3329b847 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -7,26 +7,26 @@ const ghostVersion = require('../../lib/ghost-version'); const mail = require('../mail'); const models = require('../../models'); -function createMember({email, name, note}) { - return models.Member.add({ +async function createMember({email, name, note}, options = {}) { + const model = await models.Member.add({ email, - name, - note - }).then((member) => { - return member.toJSON(); + name: name || null, + note: note || null }); + const member = model.toJSON(options); + return member; } -function getMember(data, options = {}) { +async function getMember(data, options = {}) { if (!data.email && !data.id) { return Promise.resolve(null); } - return models.Member.findOne(data, options).then((model) => { - if (!model) { - return null; - } - return model.toJSON(options); - }); + const model = await models.Member.findOne(data, options); + if (!model) { + return null; + } + const member = model.toJSON(options); + return member; } async function setMetadata(module, metadata) { @@ -71,8 +71,14 @@ async function getMetadata(module, member) { }; } -function updateMember({name, note}, options) { - return models.Member.edit({name, note}, options); +async function updateMember({name, note}, options = {}) { + const model = await models.Member.edit({ + name: name || null, + note: note || null + }, options); + + const member = model.toJSON(options); + return member; } function deleteMember(options) { From 6b4e6fb4004f61afc1382de2f4cefbb75e63d286 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 13:48:20 +0700 Subject: [PATCH 09/33] Removed unused stripe_customers relationship no-issue --- core/server/models/member.js | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/core/server/models/member.js b/core/server/models/member.js index 2532218625..dcf96473a7 100644 --- a/core/server/models/member.js +++ b/core/server/models/member.js @@ -1,24 +1,7 @@ const ghostBookshelf = require('./base'); const Member = ghostBookshelf.Model.extend({ - tableName: 'members', - - relationships: ['stripe_customers'], - relationshipBelongsTo: { - stripe_customers: 'members_stripe_customers' - }, - - permittedAttributes(...args) { - return ghostBookshelf.Model.prototype.permittedAttributes.apply(this, args).concat(this.relationships); - }, - - stripe_customers() { - return this.hasMany('MemberStripeCustomer', 'member_id'); - } -}, { - permittedOptions(...args) { - return ghostBookshelf.Model.permittedOptions.apply(this, args).concat(['withRelated']); - } + tableName: 'members' }); const Members = ghostBookshelf.Collection.extend({ From 4f0ca2914fc1183484c41589fd3542743ca12083 Mon Sep 17 00:00:00 2001 From: Rish Date: Thu, 10 Oct 2019 17:02:18 +0530 Subject: [PATCH 10/33] Updated members schema validation for name no issue - Removed minimum length requirement for `name` as its possible to have empty name for a member --- .../api/canary/utils/validators/input/schemas/members.json | 1 - 1 file changed, 1 deletion(-) diff --git a/core/server/api/canary/utils/validators/input/schemas/members.json b/core/server/api/canary/utils/validators/input/schemas/members.json index fdfaa42dd8..9a2e0a63a6 100644 --- a/core/server/api/canary/utils/validators/input/schemas/members.json +++ b/core/server/api/canary/utils/validators/input/schemas/members.json @@ -11,7 +11,6 @@ "properties": { "name": { "type": "string", - "minLength": 1, "maxLength": 191, "pattern": "^([^,]|$)" }, From 6b3c4a59b4d357362db90bc074d724efd3792153 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Oct 2019 16:35:29 +0100 Subject: [PATCH 11/33] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20rendering=20and=20?= =?UTF-8?q?url=20transformation=20of=20v1=20"card-markdown"=20aliased=20ca?= =?UTF-8?q?rds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - Ghost 1.x stored markdown cards with the name `card-markdown`, this was changed in Ghost 2.x to be `markdown`. To keep compatibility with the older mobiledoc content the `markdown` card was aliased using a straightforward `Object.assign()`. Unfortunately this failed to work adequately when the url transformation functions were added to cards and resulted in corrupted data being returned in API responses - moved the markdown card definition into a factory function so that a clean card definition object can be used for both the `markdown` and `card-markdown` cards --- core/server/lib/mobiledoc/cards/_markdown.js | 40 +++++++++ .../lib/mobiledoc/cards/card-markdown.js | 9 +- core/server/lib/mobiledoc/cards/markdown.js | 40 +-------- .../lib/mobiledoc/cards/card-markdown_spec.js | 83 +++++++++++++++++++ 4 files changed, 132 insertions(+), 40 deletions(-) create mode 100644 core/server/lib/mobiledoc/cards/_markdown.js create mode 100644 core/test/unit/lib/mobiledoc/cards/card-markdown_spec.js diff --git a/core/server/lib/mobiledoc/cards/_markdown.js b/core/server/lib/mobiledoc/cards/_markdown.js new file mode 100644 index 0000000000..ca807c081b --- /dev/null +++ b/core/server/lib/mobiledoc/cards/_markdown.js @@ -0,0 +1,40 @@ +// this is a function so that when it's aliased across multiple cards we do not +// end up modifying the object by reference +module.exports = function markdownCardDefinition() { + return { + name: 'markdown', + type: 'dom', + render: function (opts) { + let converters = require('../converters'); + let payload = opts.payload; + let version = opts.options && opts.options.version || 2; + // convert markdown to HTML ready for insertion into dom + let html = converters.markdownConverter.render(payload.markdown || ''); + + if (!html) { + return ''; + } + + /** + * @deprecated Ghost 1.0's markdown-only renderer wrapped cards. Remove in Ghost 3.0 + */ + if (version === 1) { + html = `
${html}
`; + } + + // use the SimpleDOM document to create a raw HTML section. + // avoids parsing/rendering of potentially broken or unsupported HTML + return opts.env.dom.createRawHTMLSection(html); + }, + + absoluteToRelative(urlUtils, payload, options) { + payload.markdown = payload.markdown && urlUtils.markdownAbsoluteToRelative(payload.markdown, options); + return payload; + }, + + relativeToAbsolute(urlUtils, payload, options) { + payload.markdown = payload.markdown && urlUtils.markdownRelativeToAbsolute(payload.markdown, options); + return payload; + } + }; +}; diff --git a/core/server/lib/mobiledoc/cards/card-markdown.js b/core/server/lib/mobiledoc/cards/card-markdown.js index ca1a61ee4f..310d880876 100644 --- a/core/server/lib/mobiledoc/cards/card-markdown.js +++ b/core/server/lib/mobiledoc/cards/card-markdown.js @@ -1,8 +1,9 @@ // this card is just an alias of the `markdown` card which is necessary because // our markdown-only editor was using the `card-markdown` card name -const markdownCard = require('./markdown'); +const markdownCard = require('./_markdown'); const createCard = require('../create-card'); -module.exports = createCard( - Object.assign({}, markdownCard, {name: 'card-markdown'}) -); +const v1CompatMarkdownCard = markdownCard(); +v1CompatMarkdownCard.name = 'card-markdown'; + +module.exports = createCard(v1CompatMarkdownCard); diff --git a/core/server/lib/mobiledoc/cards/markdown.js b/core/server/lib/mobiledoc/cards/markdown.js index 1cf7ba2374..b577c76b86 100644 --- a/core/server/lib/mobiledoc/cards/markdown.js +++ b/core/server/lib/mobiledoc/cards/markdown.js @@ -1,38 +1,6 @@ +const markdownCard = require('./_markdown'); const createCard = require('../create-card'); -module.exports = createCard({ - name: 'markdown', - type: 'dom', - render: function (opts) { - let converters = require('../converters'); - let payload = opts.payload; - let version = opts.options && opts.options.version || 2; - // convert markdown to HTML ready for insertion into dom - let html = converters.markdownConverter.render(payload.markdown || ''); - - if (!html) { - return ''; - } - - /** - * @deprecated Ghost 1.0's markdown-only renderer wrapped cards. Remove in Ghost 3.0 - */ - if (version === 1) { - html = `
${html}
`; - } - - // use the SimpleDOM document to create a raw HTML section. - // avoids parsing/rendering of potentially broken or unsupported HTML - return opts.env.dom.createRawHTMLSection(html); - }, - - absoluteToRelative(urlUtils, payload, options) { - payload.markdown = payload.markdown && urlUtils.markdownAbsoluteToRelative(payload.markdown, options); - return payload; - }, - - relativeToAbsolute(urlUtils, payload, options) { - payload.markdown = payload.markdown && urlUtils.markdownRelativeToAbsolute(payload.markdown, options); - return payload; - } -}); +// uses the _markdown card definition function so that the card definition +// can be re-used in aliased cards +module.exports = createCard(markdownCard()); diff --git a/core/test/unit/lib/mobiledoc/cards/card-markdown_spec.js b/core/test/unit/lib/mobiledoc/cards/card-markdown_spec.js new file mode 100644 index 0000000000..2c0b9ef87f --- /dev/null +++ b/core/test/unit/lib/mobiledoc/cards/card-markdown_spec.js @@ -0,0 +1,83 @@ +const should = require('should'); +const card = require('../../../../../server/lib/mobiledoc/cards/card-markdown'); +const SimpleDom = require('simple-dom'); +const serializer = new SimpleDom.HTMLSerializer(SimpleDom.voidMap); + +describe('Markdown card (v1 compatibility card)', function () { + it('renders', function () { + let opts = { + env: { + dom: new SimpleDom.Document() + }, + payload: { + markdown: '#HEADING\r\n- list\r\n- items' + } + }; + + serializer.serialize(card.render(opts)).should.eql('

HEADING

\n
    \n
  • list
    • \n
  • items
    • \n
\n'); + }); + + it('Accepts invalid HTML in markdown', function () { + let opts = { + env: { + dom: new SimpleDom.Document() + }, + payload: { + markdown: '#HEADING\r\n

Heading 2>' + } + }; + + serializer.serialize(card.render(opts)).should.eql('

HEADING

\n

Heading 2>'); + }); + + it('Renders nothing when payload is undefined', function () { + let opts = { + env: { + dom: new SimpleDom.Document() + }, + payload: { + markdown: undefined + } + }; + + serializer.serialize(card.render(opts)).should.eql(''); + }); + + it('[deprecated] version 1', function () { + let opts = { + env: { + dom: new SimpleDom.Document() + }, + payload: { + markdown: '#HEADING\r\n- list\r\n- items' + }, + options: { + version: 1 + } + }; + + serializer.serialize(card.render(opts)).should.eql('

HEADING

\n
    \n
  • list
    • \n
  • items
    • \n
\n
'); + }); + + it('transforms urls absolute to relative', function () { + let payload = { + markdown: 'A link to [an internal post](http://127.0.0.1:2369/post)' + }; + + const transformed = card.absoluteToRelative(payload, {}); + + transformed.markdown + .should.equal('A link to [an internal post](/post)'); + }); + + it('transforms urls relative to absolute', function () { + let payload = { + markdown: 'A link to [an internal post](/post)' + }; + + const transformed = card.relativeToAbsolute(payload, {}); + + transformed.markdown + .should.equal('A link to [an internal post](http://127.0.0.1:2369/post)'); + }); +}); From 1725f538be8f01a1b898218d1172a17d78f3ecf2 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Oct 2019 16:57:15 +0100 Subject: [PATCH 12/33] Fixed characters being lost at the end of markdown card content no issue - bumped @tryghost/url-utils with relevant fix --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 1781c75011..887f913857 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "@tryghost/members-ssr": "0.7.0", "@tryghost/social-urls": "0.1.2", "@tryghost/string": "^0.1.3", - "@tryghost/url-utils": "0.6.5", + "@tryghost/url-utils": "0.6.6", "ajv": "6.10.2", "amperize": "0.6.0", "analytics-node": "3.3.0", diff --git a/yarn.lock b/yarn.lock index 738256d8c0..f006520bcd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -297,10 +297,10 @@ dependencies: unidecode "^0.1.8" -"@tryghost/url-utils@0.6.5": - version "0.6.5" - resolved "https://registry.yarnpkg.com/@tryghost/url-utils/-/url-utils-0.6.5.tgz#a393e67e60daf265fd5d3e4ff6423bd0573bcd10" - integrity sha512-Ah6gmFo79a4p/lpzGC/EMwKgw5jEiURJAtaj3RSAiOaDF8VIAdjg7G4zO6CY8ShnMCCo7xizIGUMETHePRBgVA== +"@tryghost/url-utils@0.6.6": + version "0.6.6" + resolved "https://registry.yarnpkg.com/@tryghost/url-utils/-/url-utils-0.6.6.tgz#45fe815419df325cc07571c39739b04296a1cc09" + integrity sha512-U3JHMK5c2OVpuXHoJAOMjomYOVoyCY3ZGMYJ8yyby7eq2mMAHIBbwWsg4IkKuYKQ+/hGxK7DLGS7A3xb/Rxxbg== dependencies: cheerio "0.22.0" moment "2.24.0" From c0533b75ace16a5a19b4aa0be0ed9085b85c2bed Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Oct 2019 17:45:47 +0100 Subject: [PATCH 13/33] Fixed regression tests --- core/test/regression/api/canary/admin/members_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/regression/api/canary/admin/members_spec.js b/core/test/regression/api/canary/admin/members_spec.js index 779f2ac1f2..6ab0c73f0f 100644 --- a/core/test/regression/api/canary/admin/members_spec.js +++ b/core/test/regression/api/canary/admin/members_spec.js @@ -225,7 +225,7 @@ describe('Members API', function () { .then((res) => { should.not.exist(res.headers['x-cache-invalidate']); res.headers['content-disposition'].should.match(/Attachment;\sfilename="members/); - res.text.should.match(/id,email,name,created_at,deleted_at/); + res.text.should.match(/id,email,name,note,created_at,deleted_at/); res.text.should.match(/member1@test.com/); res.text.should.match(/Mr Egg/); }); From 0cc8b019d18e87c512c5431c68f18036a1f6d7d5 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Oct 2019 18:18:45 +0100 Subject: [PATCH 14/33] Updated Ghost-Admin to 2.35.0 --- core/client | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/client b/core/client index 2e91cb09bf..1705a622d3 160000 --- a/core/client +++ b/core/client @@ -1 +1 @@ -Subproject commit 2e91cb09bf9ea348552eae7daede465f227cb4b3 +Subproject commit 1705a622d386bb034aecebeaa4857463c3c3c3a4 From 816bec28c25db017fc09528f3afb6781427f3e1e Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Oct 2019 18:18:45 +0100 Subject: [PATCH 15/33] Version bump to 2.35.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 887f913857..82c8de5ba3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "2.34.0", + "version": "2.35.0", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org", From 1b04b48ffd1cbacefe647947f203e14fb7e93519 Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Fri, 11 Oct 2019 06:21:53 +0200 Subject: [PATCH 16/33] Added `from` parameter for member emails (#11222) * Added from parameter for member emails no issue - Passed in the `from` parameter when initializing members mailer to be able to customize outgoing address - Extends GhsotMailer to accept a from parameter from the outside --- core/server/services/mail/GhostMailer.js | 2 +- core/server/services/members/api.js | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/core/server/services/mail/GhostMailer.js b/core/server/services/mail/GhostMailer.js index df013a0aaf..38787bfa3e 100644 --- a/core/server/services/mail/GhostMailer.js +++ b/core/server/services/mail/GhostMailer.js @@ -37,7 +37,7 @@ function getFromAddress(requestedFromAddress) { function createMessage(message) { return Object.assign({}, message, { - from: getFromAddress(), + from: getFromAddress(message.from), generateTextFromHTML: true, encoding: 'base64' }); diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index cc3329b847..c7528e99c9 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -158,6 +158,13 @@ function getRequirePaymentSetting() { return !!subscriptionSettings.requirePaymentForSignup; } +// NOTE: the function is an exact duplicate of one in GhostMailer should be extracted +// into a common lib once it needs to be reused anywhere else again +function getDomain() { + const domain = urlUtils.urlFor('home', true).match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i')); + return domain && domain[1]; +} + module.exports = createApiInstance; function createApiInstance() { @@ -182,7 +189,16 @@ function createApiInstance() { if (process.env.NODE_ENV !== 'production') { common.logging.warn(message.text); } - return ghostMailer.send(Object.assign({subject: 'Signin'}, message)); + + let msg = Object.assign({subject: 'Signin'}, message); + const subscriptionSettings = settingsCache.get('members_subscription_settings'); + + if (subscriptionSettings && subscriptionSettings.fromAddress) { + let from = `${subscriptionSettings.fromAddress}@${getDomain()}`; + msg = Object.assign({from: from}, msg); + } + + return ghostMailer.send(msg); } }, getText(url, type) { From b030081a4ba22bd1e98d461b404c789a245dcc9f Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:09:23 +0700 Subject: [PATCH 17/33] Updated GhostMailer to allow forcing text content no-issue This is so that we can pass our own customised text content --- core/server/services/mail/GhostMailer.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/server/services/mail/GhostMailer.js b/core/server/services/mail/GhostMailer.js index 38787bfa3e..d8be219f5e 100644 --- a/core/server/services/mail/GhostMailer.js +++ b/core/server/services/mail/GhostMailer.js @@ -36,10 +36,12 @@ function getFromAddress(requestedFromAddress) { } function createMessage(message) { + const encoding = 'base64'; + const generateTextFromHTML = !message.forceTextContent; return Object.assign({}, message, { from: getFromAddress(message.from), - generateTextFromHTML: true, - encoding: 'base64' + generateTextFromHTML, + encoding }); } From 6070ebaf02068b182643191ccb19cd20f5f1a0ea Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:23:43 +0700 Subject: [PATCH 18/33] Installed @tryghost/members-api@0.8.1 no-issue This includes changes to support custom subjects in emails and access to the email of the recipient in the getHTML and getText functions --- package.json | 2 +- yarn.lock | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 82c8de5ba3..b399cc1380 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "dependencies": { "@nexes/nql": "0.3.0", "@tryghost/helpers": "1.1.15", - "@tryghost/members-api": "0.8.0", + "@tryghost/members-api": "0.8.1", "@tryghost/members-ssr": "0.7.0", "@tryghost/social-urls": "0.1.2", "@tryghost/string": "^0.1.3", diff --git a/yarn.lock b/yarn.lock index f006520bcd..50d6b36bbb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -227,22 +227,22 @@ dependencies: "@tryghost/kg-clean-basic-html" "^0.1.3" -"@tryghost/magic-link@^0.2.1": - version "0.2.1" - resolved "https://registry.yarnpkg.com/@tryghost/magic-link/-/magic-link-0.2.1.tgz#c729bf5d2fe7fa1330eccbba51ba3579834784fc" - integrity sha512-bqlZndOXwU3b9FXvMtHIep1EradDnsfQ+4vvINQ+QsCOWKH1EDbPhjYS9f2G0xNx8BVyG4e1eMxZ5lBhJ6lBCA== +"@tryghost/magic-link@^0.2.2": + version "0.2.2" + resolved "https://registry.yarnpkg.com/@tryghost/magic-link/-/magic-link-0.2.2.tgz#fa4dbe58d0389db558fee70ac065ce0a78cb29e7" + integrity sha512-5OdnC1sEDGA1ioF7NJpTT14p1nlnaHOr/EPtiDxhgND1+vd09LQe+EB6JoefLPonzSEvSexdIYRGj1Gh+zfj6A== dependencies: bluebird "^3.5.5" ghost-ignition "^3.1.0" jsonwebtoken "^8.5.1" lodash "^4.17.15" -"@tryghost/members-api@0.8.0": - version "0.8.0" - resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.8.0.tgz#e1f0b67371b6b61f6cf4f64e5b62c92ba3777c2a" - integrity sha512-THPd9HUyqo1WdroWFH8X+KcNfyy586dVSOYRQWIjF+URoYHmlrf2AlxBrdHMq5R8mQVCKIO0t3pmZwRnafucVA== +"@tryghost/members-api@0.8.1": + version "0.8.1" + resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.8.1.tgz#45453acbbe8f464e8bd45273d2e74902b150bce5" + integrity sha512-6lQbr47xR/bJHlOXTpU2dipjKWKX6pUA48rvpdJRA6xvRYQHGezXg8Ebg1K08llJgTEjpA/oFrOnA0FAf1i/vA== dependencies: - "@tryghost/magic-link" "^0.2.1" + "@tryghost/magic-link" "^0.2.2" bluebird "^3.5.4" body-parser "^1.19.0" cookies "^0.7.3" From 6f160518d12e28776dd48f69397a32e3ebcfef88 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:25:48 +0700 Subject: [PATCH 19/33] Ensured that members emails include our text version no-issue --- core/server/services/members/api.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index c7528e99c9..1078d2d58e 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -189,8 +189,10 @@ function createApiInstance() { if (process.env.NODE_ENV !== 'production') { common.logging.warn(message.text); } - - let msg = Object.assign({subject: 'Signin'}, message); + let msg = Object.assign({ + subject: 'Signin', + forceTextContent: true + }, message); const subscriptionSettings = settingsCache.get('members_subscription_settings'); if (subscriptionSettings && subscriptionSettings.fromAddress) { From 98f27c1c339bc4c6f7991631cb76142d5233ac45 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:26:18 +0700 Subject: [PATCH 20/33] Added getSubject function for members emails no-issue --- core/server/services/members/api.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 1078d2d58e..858c37af51 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -203,6 +203,18 @@ function createApiInstance() { return ghostMailer.send(msg); } }, + getSubject(type) { + const siteTitle = settingsCache.get('title'); + switch (type) { + case 'subscribe': + return `📫 Confirm your subscription to ${siteTitle}`; + case 'signup': + return `🙌 Complete your sign up to ${siteTitle}!`; + case 'signin': + default: + return `🔑 Secure sign in link for ${siteTitle}`; + } + }, getText(url, type) { switch (type) { case 'subscribe': From f4d202d7c59c500bdf27e10239d777a3a5611f8d Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:28:02 +0700 Subject: [PATCH 21/33] Added member email templates no-issue --- core/server/services/members/emails/signin.js | 175 +++++++++++++++++ core/server/services/members/emails/signup.js | 175 +++++++++++++++++ .../services/members/emails/subscribe.js | 176 ++++++++++++++++++ 3 files changed, 526 insertions(+) create mode 100644 core/server/services/members/emails/signin.js create mode 100644 core/server/services/members/emails/signup.js create mode 100644 core/server/services/members/emails/subscribe.js diff --git a/core/server/services/members/emails/signin.js b/core/server/services/members/emails/signin.js new file mode 100644 index 0000000000..dc47211144 --- /dev/null +++ b/core/server/services/members/emails/signin.js @@ -0,0 +1,175 @@ +module.exports = ({siteTitle, email, url}) => ` + + + + + + 🔑 Secure sign in link for ${siteTitle} + + + + + + + + + +
  +
+ + + + + + + + + + + +
+ + + + +
+

Hey there,

+

Welcome back! Use this link to securely sign in to your ${siteTitle} account:

+ + + + + + +
+ + + + + + +
Sign in to ${siteTitle}
+
+

For your security, the link will expire in 10 minutes time.

+

See you soon!
The team at ${siteTitle}

+
+

You can also copy & paste this URL into your browser:

+

${url}

+
+
+ + +
+ + + + + + + +
+ If you did not make this request, you can safely ignore this email. +
+ Sent to ${email} +
+
+ + + +
+ 		 
+ + +`; diff --git a/core/server/services/members/emails/signup.js b/core/server/services/members/emails/signup.js new file mode 100644 index 0000000000..0d482bb070 --- /dev/null +++ b/core/server/services/members/emails/signup.js @@ -0,0 +1,175 @@ +module.exports = ({siteTitle, email, url}) => ` + + + + + + 🙌 Complete your sign up to ${siteTitle}! + + + + + + + + + +
  +
+ + + + + + + + + + + +
+ + + + +
+

Hey there!

+

Thanks for signing up for ${siteTitle} — use this link to complete the sign up process and be automatically signed in:

+ + + + + + +
+ + + + + + +
Activate my account
+
+

For your security, the link will expire in 10 minutes time.

+

See you soon!
The team at ${siteTitle}

+
+

You can also copy & paste this URL into your browser:

+

${url}

+
+
+ + +
+ + + + + + + +
+ If you did not make this request, you can simply delete this message.
You will not be signed up, and no account will be created for you. +
+ Sent to ${email} +
+
+ + + +
+ 		 
+ + +`; diff --git a/core/server/services/members/emails/subscribe.js b/core/server/services/members/emails/subscribe.js new file mode 100644 index 0000000000..ec8dc02780 --- /dev/null +++ b/core/server/services/members/emails/subscribe.js @@ -0,0 +1,176 @@ +module.exports = ({siteTitle, email, url}) => ` + + + + + + 📫 Confirm your subscription to ${siteTitle} + + + + + + + + + +
  +
+ + + + + + + + + + + +
+ + + + +
+

Hey there,

+

You're one tap away from subscribing to ${siteTitle} — please confirm your email address with this link:

+ + + + + + +
+ + + + + + +
Yes, I want to subscribe
+
+

For your security, the link will expire in 10 minutes time.

+

All the best!
The team at ${siteTitle}

+
+

You can also copy & paste this URL into your browser:

+

${url}

+
+
+ + +
+ + + + + + + +
+ If you did not make this request, you can simply delete this message.
You will not be subscribed. +
+ Sent to ${email} +
+
+ + + +
+ 		 
+ + +`; From 257bebbb3944cd3be62fbd32f8102d3a6013da92 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:28:20 +0700 Subject: [PATCH 22/33] Wired up the members emails templates no-issue --- core/server/services/members/api.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 858c37af51..fb8aa16bb9 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -6,6 +6,9 @@ const common = require('../../lib/common'); const ghostVersion = require('../../lib/ghost-version'); const mail = require('../mail'); const models = require('../../models'); +const signinEmail = require('./emails/signin'); +const signupEmail = require('./emails/signup'); +const subscribeEmail = require('./emails/subscribe'); async function createMember({email, name, note}, options = {}) { const model = await models.Member.add({ @@ -226,15 +229,16 @@ function createApiInstance() { return `Click here to sign in ${url}`; } }, - getHTML(url, type) { + getHTML(url, type, email) { + const siteTitle = settingsCache.get('title'); switch (type) { case 'subscribe': - return `Click here to confirm your subscription`; + return subscribeEmail({url, email, siteTitle}); case 'signup': - return `Click here to confirm your email address and sign up`; + return signupEmail({url, email, siteTitle}); case 'signin': default: - return `Click here to sign in`; + return signinEmail({url, email, siteTitle}); } } }, From 3062ec76901d405c6f38f419d0e52a34c5b7b83f Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 20:43:30 +0700 Subject: [PATCH 23/33] Wired up members plaintext emails no-issue --- core/server/services/members/api.js | 57 +++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index fb8aa16bb9..6bdc43754b 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -218,15 +218,64 @@ function createApiInstance() { return `🔑 Secure sign in link for ${siteTitle}`; } }, - getText(url, type) { + getText(url, type, email) { + const siteTitle = settingsCache.get('title'); switch (type) { case 'subscribe': - return `Click here to confirm your subscription ${url}`; + return ` + Hey there, + + You're one tap away from subscribing to ${siteTitle} — please confirm your email address with this link: + + ${url} + + For your security, the link will expire in 10 minutes time. + + All the best! + The team at ${siteTitle} + + --- + + Sent to ${email} + If you did not make this request, you can simply delete this message. You will not be subscribed. + `; case 'signup': - return `Click here to confirm your email address and sign up ${url}`; + return ` + Hey there! + + Thanks for signing up for ${siteTitle} — use this link to complete the sign up process and be automatically signed in: + + ${url} + + For your security, the link will expire in 10 minutes time. + + See you soon! + The team at ${siteTitle} + + --- + + Sent to ${email} + If you did not make this request, you can simply delete this message. You will not be signed up, and no account will be created for you. + `; case 'signin': default: - return `Click here to sign in ${url}`; + return ` + Hey there, + + Welcome back! Use this link to securely sign in to your ${siteTitle} account: + + ${url} + + For your security, the link will expire in 10 minutes time. + + See you soon! + The team at ${siteTitle} + + --- + + Sent to ${email} + If you did not make this request, you can safely ignore this email. + `; } }, getHTML(url, type, email) { From 527632f287bfefb520bf7adc9ce56be15dde9467 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 09:59:16 +0700 Subject: [PATCH 24/33] Updated members email templates no-issue These changes fix come colors and styles --- core/server/services/members/emails/signin.js | 30 ++++++++----------- core/server/services/members/emails/signup.js | 30 ++++++++----------- .../services/members/emails/subscribe.js | 29 +++++++----------- 3 files changed, 35 insertions(+), 54 deletions(-) diff --git a/core/server/services/members/emails/signin.js b/core/server/services/members/emails/signin.js index dc47211144..621be389a1 100644 --- a/core/server/services/members/emails/signin.js +++ b/core/server/services/members/emails/signin.js @@ -50,6 +50,7 @@ module.exports = ({siteTitle, email, url}) => ` width: auto !important; } } + /* ------------------------------------- PRESERVE THESE STYLES IN THE HEAD ------------------------------------- */ @@ -81,13 +82,6 @@ module.exports = ({siteTitle, email, url}) => ` font-weight: inherit; line-height: inherit; } - .btn-primary table td:hover { - background-color: #34495e !important; - } - .btn-primary a:hover { - background-color: #34495e !important; - border-color: #34495e !important; - } } hr { border-width: 0; @@ -103,8 +97,8 @@ module.exports = ({siteTitle, email, url}) => ` - -
  -
+
+
@@ -112,20 +106,20 @@ module.exports = ({siteTitle, email, url}) => `
+ @@ -151,12 +145,12 @@ module.exports = ({siteTitle, email, url}) => `

Hey there,

-

Welcome back! Use this link to securely sign in to your ${siteTitle} account:

+

Welcome back! Use this link to securely sign in to your ${siteTitle} account:

-
+ - +
Sign in to ${siteTitle} Sign in to ${siteTitle}
@@ -133,10 +127,10 @@ module.exports = ({siteTitle, email, url}) => `
-

For your security, the link will expire in 10 minutes time.

-

See you soon!
The team at ${siteTitle}

+

For your security, the link will expire in 10 minutes time.

+

See you soon!
The team at ${siteTitle}

-

You can also copy & paste this URL into your browser:

+

You can also copy & paste this URL into your browser:

${url}
- - diff --git a/core/server/services/members/emails/signup.js b/core/server/services/members/emails/signup.js index 0d482bb070..fc26e1c7f3 100644 --- a/core/server/services/members/emails/signup.js +++ b/core/server/services/members/emails/signup.js @@ -50,6 +50,7 @@ module.exports = ({siteTitle, email, url}) => ` width: auto !important; } } + /* ------------------------------------- PRESERVE THESE STYLES IN THE HEAD ------------------------------------- */ @@ -81,13 +82,6 @@ module.exports = ({siteTitle, email, url}) => ` font-weight: inherit; line-height: inherit; } - .btn-primary table td:hover { - background-color: #34495e !important; - } - .btn-primary a:hover { - background-color: #34495e !important; - border-color: #34495e !important; - } } hr { border-width: 0; @@ -103,8 +97,8 @@ module.exports = ({siteTitle, email, url}) => `
+ If you did not make this request, you can safely ignore this email.
+ Sent to ${email}
- -
  -
+
+
@@ -112,20 +106,20 @@ module.exports = ({siteTitle, email, url}) => `
+ @@ -151,12 +145,12 @@ module.exports = ({siteTitle, email, url}) => `

Hey there!

-

Thanks for signing up for ${siteTitle} — use this link to complete the sign up process and be automatically signed in:

+

Thanks for signing up for ${siteTitle} — use this link to complete the sign up process and be automatically signed in:

-
+ - +
Activate my account Activate my account
@@ -133,10 +127,10 @@ module.exports = ({siteTitle, email, url}) => `
-

For your security, the link will expire in 10 minutes time.

-

See you soon!
The team at ${siteTitle}

+

For your security, the link will expire in 10 minutes time.

+

See you soon!
The team at ${siteTitle}

-

You can also copy & paste this URL into your browser:

+

You can also copy & paste this URL into your browser:

${url}
- - diff --git a/core/server/services/members/emails/subscribe.js b/core/server/services/members/emails/subscribe.js index ec8dc02780..ac2c5e700b 100644 --- a/core/server/services/members/emails/subscribe.js +++ b/core/server/services/members/emails/subscribe.js @@ -82,13 +82,6 @@ module.exports = ({siteTitle, email, url}) => ` font-weight: inherit; line-height: inherit; } - .btn-primary table td:hover { - background-color: #34495e !important; - } - .btn-primary a:hover { - background-color: #34495e !important; - border-color: #34495e !important; - } } hr { border-width: 0; @@ -104,8 +97,8 @@ module.exports = ({siteTitle, email, url}) => `
+ If you did not make this request, you can simply delete this message.
You will not be signed up, and no account will be created for you.
+ Sent to ${email}
- -
  -
+
+
@@ -113,20 +106,20 @@ module.exports = ({siteTitle, email, url}) => `
+ @@ -152,12 +145,12 @@ module.exports = ({siteTitle, email, url}) => `

Hey there,

-

You're one tap away from subscribing to ${siteTitle} — please confirm your email address with this link:

+

You're one tap away from subscribing to ${siteTitle} — please confirm your email address with this link:

-
+ - +
Yes, I want to subscribe Yes, I want to subscribe
@@ -134,10 +127,10 @@ module.exports = ({siteTitle, email, url}) => `
-

For your security, the link will expire in 10 minutes time.

-

All the best!
The team at ${siteTitle}

+

For your security, the link will expire in 10 minutes time.

+

All the best!
The team at ${siteTitle}

-

You can also copy & paste this URL into your browser:

+

You can also copy & paste this URL into your browser:

${url}
- - From ef5e6f7e5beb1715da4abb2c2e4df6d9f2edf6c9 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 11:37:00 +0700 Subject: [PATCH 25/33] Removed text-transform: capitalize from buttons no-issue Button text should be sentence case not title case --- core/server/services/members/emails/signin.js | 2 +- core/server/services/members/emails/signup.js | 2 +- core/server/services/members/emails/subscribe.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/server/services/members/emails/signin.js b/core/server/services/members/emails/signin.js index 621be389a1..51fa8fbd13 100644 --- a/core/server/services/members/emails/signin.js +++ b/core/server/services/members/emails/signin.js @@ -119,7 +119,7 @@ module.exports = ({siteTitle, email, url}) => `
+ If you did not make this request, you can simply delete this message.
You will not be subscribed.
+ Sent to ${email}
- +
Sign in to ${siteTitle} Sign in to ${siteTitle}
diff --git a/core/server/services/members/emails/signup.js b/core/server/services/members/emails/signup.js index fc26e1c7f3..b0db9db093 100644 --- a/core/server/services/members/emails/signup.js +++ b/core/server/services/members/emails/signup.js @@ -119,7 +119,7 @@ module.exports = ({siteTitle, email, url}) => ` - +
Activate my account Activate my account
diff --git a/core/server/services/members/emails/subscribe.js b/core/server/services/members/emails/subscribe.js index ac2c5e700b..7d1c979548 100644 --- a/core/server/services/members/emails/subscribe.js +++ b/core/server/services/members/emails/subscribe.js @@ -119,7 +119,7 @@ module.exports = ({siteTitle, email, url}) => ` - +
Yes, I want to subscribe Yes, I want to subscribe
From 8b0130193c570252559ccc31a90cab67c516f4a3 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 12:07:44 +0700 Subject: [PATCH 26/33] Installed @tryghost/members-api@0.8.2 no-issue This version uses HS256 signed tokens for magic-links and provides much smaller links but requires a 256bit (64 byte) secret --- package.json | 2 +- yarn.lock | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index b399cc1380..693c5c521f 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "dependencies": { "@nexes/nql": "0.3.0", "@tryghost/helpers": "1.1.15", - "@tryghost/members-api": "0.8.1", + "@tryghost/members-api": "0.8.2", "@tryghost/members-ssr": "0.7.0", "@tryghost/social-urls": "0.1.2", "@tryghost/string": "^0.1.3", diff --git a/yarn.lock b/yarn.lock index 50d6b36bbb..a2a4dc160d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -227,22 +227,22 @@ dependencies: "@tryghost/kg-clean-basic-html" "^0.1.3" -"@tryghost/magic-link@^0.2.2": - version "0.2.2" - resolved "https://registry.yarnpkg.com/@tryghost/magic-link/-/magic-link-0.2.2.tgz#fa4dbe58d0389db558fee70ac065ce0a78cb29e7" - integrity sha512-5OdnC1sEDGA1ioF7NJpTT14p1nlnaHOr/EPtiDxhgND1+vd09LQe+EB6JoefLPonzSEvSexdIYRGj1Gh+zfj6A== +"@tryghost/magic-link@^0.3.0": + version "0.3.0" + resolved "https://registry.yarnpkg.com/@tryghost/magic-link/-/magic-link-0.3.0.tgz#52e4326efba5756a39d219990d8fcbcbd3ba813a" + integrity sha512-Ahd4KG8Hz/zndRjx0MQt6cY8Lq8TL3UQEXbNIhEO1zOzgyq7FKGpBoSfaVzYb6aSx1pAG/EP60VuBKetHBmUSw== dependencies: bluebird "^3.5.5" ghost-ignition "^3.1.0" jsonwebtoken "^8.5.1" lodash "^4.17.15" -"@tryghost/members-api@0.8.1": - version "0.8.1" - resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.8.1.tgz#45453acbbe8f464e8bd45273d2e74902b150bce5" - integrity sha512-6lQbr47xR/bJHlOXTpU2dipjKWKX6pUA48rvpdJRA6xvRYQHGezXg8Ebg1K08llJgTEjpA/oFrOnA0FAf1i/vA== +"@tryghost/members-api@0.8.2": + version "0.8.2" + resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.8.2.tgz#2d72cb80909c571fb2e6c9de868d438b5319e54d" + integrity sha512-maqhvyNpvw0kWG20UpSG0637YXteO5cc/ts+KWiSk4oIDuaH1sycgNXQ98q4Z+MAKzMye0iGoom7NcXQOFh8/Q== dependencies: - "@tryghost/magic-link" "^0.2.2" + "@tryghost/magic-link" "^0.3.0" bluebird "^3.5.4" body-parser "^1.19.0" cookies "^0.7.3" From cbb13904b845967bbf4e4b7a2d31b022169a6256 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 17:31:19 +0700 Subject: [PATCH 27/33] Added members_email_auth_secret setting no-issue This will be used for signing HS256 JWTs it's a 64 byte (256 bit) hex string --- core/server/data/schema/default-settings.json | 3 +++ core/server/models/settings.js | 3 ++- core/test/unit/models/settings_spec.js | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index 88b3c8c966..12c79739bb 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -194,6 +194,9 @@ "members_session_secret": { "defaultValue": null }, + "members_email_auth_secret": { + "defaultValue": null + }, "default_content_visibility": { "defaultValue": "public" }, diff --git a/core/server/models/settings.js b/core/server/models/settings.js index 5de36857b5..ef8fae67ed 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -37,7 +37,8 @@ function parseDefaultSettings() { members_session_secret: () => crypto.randomBytes(32).toString('hex'), theme_session_secret: () => crypto.randomBytes(32).toString('hex'), members_public_key: () => getMembersKey('public'), - members_private_key: () => getMembersKey('private') + members_private_key: () => getMembersKey('private'), + members_email_auth_secret: () => crypto.randomBytes(64).toString('hex') }; _.each(defaultSettingsInCategories, function each(settings, categoryName) { diff --git a/core/test/unit/models/settings_spec.js b/core/test/unit/models/settings_spec.js index ced845cb2d..d1f751027b 100644 --- a/core/test/unit/models/settings_spec.js +++ b/core/test/unit/models/settings_spec.js @@ -113,7 +113,7 @@ describe('Unit: models/settings', function () { return models.Settings.populateDefaults() .then(() => { - eventSpy.callCount.should.equal(80); + eventSpy.callCount.should.equal(82); eventSpy.args[1][0].should.equal('settings.db_hash.added'); eventSpy.args[1][1].attributes.type.should.equal('core'); @@ -137,7 +137,7 @@ describe('Unit: models/settings', function () { return models.Settings.populateDefaults() .then(() => { - eventSpy.callCount.should.equal(78); + eventSpy.callCount.should.equal(80); eventSpy.args[13][0].should.equal('settings.logo.added'); }); From 0c602976c064adcc2e982be36780bcd65d735b11 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 10 Oct 2019 17:31:38 +0700 Subject: [PATCH 28/33] Passed members_email_auth_secret to members-api no-issue --- core/server/services/members/api.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 6bdc43754b..7efde87625 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -1,3 +1,4 @@ +const crypto = require('crypto'); const {URL} = require('url'); const settingsCache = require('../settings/cache'); const urlUtils = require('../../lib/url-utils'); @@ -156,6 +157,20 @@ function getStripePaymentConfig() { }; } +function getAuthSecret() { + const hexSecret = settingsCache.get('members_email_auth_secret'); + if (!hexSecret) { + common.logging.warn('Could not find members_email_auth_secret, using dynamically generated secret'); + return crypto.randomBytes(64); + } + const secret = Buffer.from(hexSecret, 'hex'); + if (secret.length < 64) { + common.logging.warn('members_email_auth_secret not large enough (64 bytes), using dynamically generated secret'); + return crypto.randomBytes(64); + } + return secret; +} + function getRequirePaymentSetting() { const subscriptionSettings = settingsCache.get('members_subscription_settings'); return !!subscriptionSettings.requirePaymentForSignup; @@ -184,7 +199,8 @@ function createApiInstance() { signinURL.searchParams.set('action', type); return signinURL.href; }, - allowSelfSignup: !getRequirePaymentSetting() + allowSelfSignup: !getRequirePaymentSetting(), + secret: getAuthSecret() }, mail: { transporter: { From 1500881923a2e85369903d2820f1667f51b16f1d Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 12:16:57 +0700 Subject: [PATCH 29/33] Renamed getRequirePaymentSetting no-issue The negation before this function call was a little easy to miss for me --- core/server/services/members/api.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index 7efde87625..e8346a3f33 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -171,9 +171,9 @@ function getAuthSecret() { return secret; } -function getRequirePaymentSetting() { +function getAllowSelfSignup() { const subscriptionSettings = settingsCache.get('members_subscription_settings'); - return !!subscriptionSettings.requirePaymentForSignup; + return !subscriptionSettings.requirePaymentForSignup; } // NOTE: the function is an exact duplicate of one in GhostMailer should be extracted @@ -199,7 +199,7 @@ function createApiInstance() { signinURL.searchParams.set('action', type); return signinURL.href; }, - allowSelfSignup: !getRequirePaymentSetting(), + allowSelfSignup: getAllowSelfSignup(), secret: getAuthSecret() }, mail: { From 95fa815eb3d56e1ebc009f8793a41f0debbd7715 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 13:41:36 +0700 Subject: [PATCH 30/33] Improved settings model tests no-issue This makes them way less brittle and reliant on correctly indexing an array --- core/test/unit/models/settings_spec.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/core/test/unit/models/settings_spec.js b/core/test/unit/models/settings_spec.js index d1f751027b..05b7d3e581 100644 --- a/core/test/unit/models/settings_spec.js +++ b/core/test/unit/models/settings_spec.js @@ -114,16 +114,14 @@ describe('Unit: models/settings', function () { return models.Settings.populateDefaults() .then(() => { eventSpy.callCount.should.equal(82); + const eventsEmitted = eventSpy.args.map(args => args[0]); + const checkEventEmitted = event => should.ok(eventsEmitted.includes(event), `${event} event should be emitted`); - eventSpy.args[1][0].should.equal('settings.db_hash.added'); - eventSpy.args[1][1].attributes.type.should.equal('core'); + checkEventEmitted('settings.db_hash.added'); + checkEventEmitted('settings.description.added'); - eventSpy.args[13][0].should.equal('settings.description.added'); - eventSpy.args[13][1].attributes.type.should.equal('blog'); - eventSpy.args[13][1].attributes.value.should.equal('The professional publishing platform'); - - eventSpy.args[77][0].should.equal('settings.default_content_visibility.added'); - eventSpy.args[79][0].should.equal('settings.members_subscription_settings.added'); + checkEventEmitted('settings.default_content_visibility.added'); + checkEventEmitted('settings.members_subscription_settings.added'); }); }); From 3eb4427888561f76834e70545633c4c16026b823 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 13:58:50 +0700 Subject: [PATCH 31/33] Exposed visibility prop for posts on canary api (#11229) no-issue This is required by the theme layer to style member only posts differently --- core/server/api/canary/utils/serializers/output/utils/clean.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/server/api/canary/utils/serializers/output/utils/clean.js b/core/server/api/canary/utils/serializers/output/utils/clean.js index cf2b775111..bd2d53470d 100644 --- a/core/server/api/canary/utils/serializers/output/utils/clean.js +++ b/core/server/api/canary/utils/serializers/output/utils/clean.js @@ -98,8 +98,6 @@ const post = (attrs, frame) => { if (attrs.og_description === '') { attrs.og_description = null; } - - delete attrs.visibility; } else { delete attrs.page; From 85d83b4f088a99d3917e09f6b6cf42e7f828f91d Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Fri, 11 Oct 2019 14:14:46 +0700 Subject: [PATCH 32/33] Fixed regression test for canary content api no-issue We now return the visibility field on the content api --- core/test/regression/api/canary/content/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/regression/api/canary/content/utils.js b/core/test/regression/api/canary/content/utils.js index 404f62d161..ab1cad5db5 100644 --- a/core/test/regression/api/canary/content/utils.js +++ b/core/test/regression/api/canary/content/utils.js @@ -20,7 +20,7 @@ const expectedProperties = { // and always returns computed properties: url, primary_tag, primary_author .concat('url', 'primary_tag', 'primary_author') // canary API doesn't return unused fields - .without('locale', 'visibility') + .without('locale') // These fields aren't useful as they always have known values .without('status') // @TODO: https://github.com/TryGhost/Ghost/issues/10335 From cd02fd5c6301283549e3ae5286f5cc2e711b39e1 Mon Sep 17 00:00:00 2001 From: Rish Date: Fri, 11 Oct 2019 14:08:31 +0530 Subject: [PATCH 33/33] Renamed member requirePayment setting no issue Renames member setting `requirePaymentForSignup` -> `allowSelfSignup` to match members API usage --- core/server/data/schema/default-settings.json | 2 +- core/server/services/members/api.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index 12c79739bb..778e72b2c8 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -201,7 +201,7 @@ "defaultValue": "public" }, "members_subscription_settings": { - "defaultValue": "{\"isPaid\":false,\"fromAddress\":\"noreply\",\"requirePaymentForSignup\":false,\"paymentProcessors\":[{\"adapter\":\"stripe\",\"config\":{\"secret_token\":\"\",\"public_token\":\"\",\"product\":{\"name\":\"Ghost Subscription\"},\"plans\":[{\"name\":\"Monthly\",\"currency\":\"usd\",\"interval\":\"month\",\"amount\":\"\"},{\"name\":\"Yearly\",\"currency\":\"usd\",\"interval\":\"year\",\"amount\":\"\"}]}}]}" + "defaultValue": "{\"isPaid\":false,\"fromAddress\":\"noreply\",\"allowSelfSignup\":true,\"paymentProcessors\":[{\"adapter\":\"stripe\",\"config\":{\"secret_token\":\"\",\"public_token\":\"\",\"product\":{\"name\":\"Ghost Subscription\"},\"plans\":[{\"name\":\"Monthly\",\"currency\":\"usd\",\"interval\":\"month\",\"amount\":\"\"},{\"name\":\"Yearly\",\"currency\":\"usd\",\"interval\":\"year\",\"amount\":\"\"}]}}]}" } } } diff --git a/core/server/services/members/api.js b/core/server/services/members/api.js index e8346a3f33..dc826bdc7e 100644 --- a/core/server/services/members/api.js +++ b/core/server/services/members/api.js @@ -173,7 +173,7 @@ function getAuthSecret() { function getAllowSelfSignup() { const subscriptionSettings = settingsCache.get('members_subscription_settings'); - return !subscriptionSettings.requirePaymentForSignup; + return subscriptionSettings.allowSelfSignup; } // NOTE: the function is an exact duplicate of one in GhostMailer should be extracted