From e398557a7537f62ee0d1c3b4e80100a637758316 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 13 Apr 2022 19:34:48 +0100 Subject: [PATCH] Added sender email verification flow for newsletters refs https://github.com/TryGhost/Team/issues/584 refs https://github.com/TryGhost/Team/issues/1498 - updated newsletter save routine in `edit-newsletter` modal to open an email confirmation modal if the API indicates one was sent - modal indicates that the previously set or default email will continue to be used until verified - response from API when saving looks like `{newsletters: [{...}], meta: {sent_email_verification: ['sender_name]}}` - added custom newsletter serializer and updated model so that the `meta` property returned in the response when saving posts is exposed - Ember Data only exposes meta on array-response find/query methods - https://github.com/emberjs/data/issues/2905 - added `/settings/members-email-labs/?verifyEmail=xyz` query param handling - opens email verification modal if param is set and instantly clears the query param to avoid problems with sticky params - when the modal opens it makes a `PUT /newsletters/verify-email/` request with the token in the body params, on the API side this works the same as a newsletter update request returning the fully updated newsletter record which is then pushed into the store - removed unused from/reply address code from `` component and controller - setting the values now handled per-newsletter in the edit-newsletter modal - verifying email change is handled in the members-email-labs controller - fixed mirage not outputting pluralized root for "singular" endpoints such as POST/PUT requests to better match our API behaviour --- .../app/components/modals/edit-newsletter.js | 13 ++++ .../confirm-newsletter-email.hbs | 26 +++++++ .../verify-newsletter-email.hbs | 34 +++++++++ .../verify-newsletter-email.js | 49 +++++++++++++ .../components/settings/members-email-labs.js | 67 ----------------- .../settings/members-email-labs.js | 27 +------ ghost/admin/app/models/newsletter.js | 4 + .../app/routes/settings/members-email-labs.js | 28 ++++--- ghost/admin/app/serializers/newsletter.js | 27 +++++++ .../templates/settings/members-email-labs.hbs | 6 +- ghost/admin/mirage/config/newsletters.js | 73 ++++++++++++++++++- ghost/admin/mirage/scenarios/default.js | 26 +++++++ ghost/admin/mirage/serializers/application.js | 4 +- 13 files changed, 272 insertions(+), 112 deletions(-) create mode 100644 ghost/admin/app/components/modals/edit-newsletter/confirm-newsletter-email.hbs create mode 100644 ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.hbs create mode 100644 ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.js create mode 100644 ghost/admin/app/serializers/newsletter.js diff --git a/ghost/admin/app/components/modals/edit-newsletter.js b/ghost/admin/app/components/modals/edit-newsletter.js index 0a058ad24e..324143f387 100644 --- a/ghost/admin/app/components/modals/edit-newsletter.js +++ b/ghost/admin/app/components/modals/edit-newsletter.js @@ -1,9 +1,13 @@ import Component from '@glimmer/component'; +import ConfirmNewsletterEmailModal from './edit-newsletter/confirm-newsletter-email'; import {action} from '@ember/object'; +import {inject as service} from '@ember/service'; import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; export default class EditNewsletterModal extends Component { + @service modals; + static modalOptions = { className: 'fullscreen-modal-full-overlay fullscreen-modal-portal-settings' }; @@ -31,8 +35,17 @@ export default class EditNewsletterModal extends Component { @task *saveTask() { try { + const newEmail = this.args.data.newsletter.senderEmail; + const result = yield this.args.data.newsletter.save(); + if (result._meta?.sent_email_verification) { + yield this.modals.open(ConfirmNewsletterEmailModal, { + newEmail, + currentEmail: this.args.data.newsletter.senderEmail + }); + } + this.args.data.afterSave?.(result); return result; diff --git a/ghost/admin/app/components/modals/edit-newsletter/confirm-newsletter-email.hbs b/ghost/admin/app/components/modals/edit-newsletter/confirm-newsletter-email.hbs new file mode 100644 index 0000000000..6ec8e7010a --- /dev/null +++ b/ghost/admin/app/components/modals/edit-newsletter/confirm-newsletter-email.hbs @@ -0,0 +1,26 @@ +
+
+

Confirm newsletter email address

+
+ + +
+

+ We've sent a confirmation email to {{@data.newEmail}}. + Until the address has been verified newsletters will be sent from the + {{if @data.currentEmail "previous" "default"}} email address + ({{full-email-address (or @data.currentEmail "noreply")}}). +

+
+ +
+ +
+
\ No newline at end of file diff --git a/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.hbs b/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.hbs new file mode 100644 index 0000000000..9bf758b998 --- /dev/null +++ b/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.hbs @@ -0,0 +1,34 @@ +
+
+

Verifying newsletter email address

+
+ + +
+ {{#if this.verifyEmailTask.isRunning}} +
+ +
+ {{else if this.newsletter}} +

+ Success! From address for newsletter + "{{this.newsletter.name}}" + changed to {{this.newsletter.senderEmail}} +

+ {{else if this.error}} +

Verification failed:

+

{{this.error}}

+ {{/if}} +
+ +
+ +
+
\ No newline at end of file diff --git a/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.js b/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.js new file mode 100644 index 0000000000..b06c627028 --- /dev/null +++ b/ghost/admin/app/components/modals/edit-newsletter/verify-newsletter-email.js @@ -0,0 +1,49 @@ +import Component from '@glimmer/component'; +import {action} from '@ember/object'; +import {inject as service} from '@ember/service'; +import {task} from 'ember-concurrency'; +import {tracked} from '@glimmer/tracking'; + +export default class VerifyNewsletterEmail extends Component { + @service ajax; + @service ghostPaths; + @service router; + @service store; + + @tracked error = null; + @tracked newsletter = null; + + constructor() { + super(...arguments); + this.verifyEmailTask.perform(this.args.data.token); + + this.router.on('routeDidChange', this.handleRouteChange); + } + + willDestroy() { + super.willDestroy(...arguments); + this.router.off('routeDidChange', this.handleRouteChange); + } + + @task + *verifyEmailTask(token) { + try { + const url = this.ghostPaths.url.api('newsletters', 'verify-email'); + + const response = yield this.ajax.put(url, {data: {token}}); + + if (response.newsletters) { + this.store.pushPayload('newsletter', response); + const newsletter = this.store.peekRecord('newsletter', response.newsletters[0].id); + this.newsletter = newsletter; + } + } catch (e) { + this.error = e.message; + } + } + + @action + handleRouteChange() { + this.args.close(); + } +} diff --git a/ghost/admin/app/components/settings/members-email-labs.js b/ghost/admin/app/components/settings/members-email-labs.js index eb273a3f93..4c9dfb5b4c 100644 --- a/ghost/admin/app/components/settings/members-email-labs.js +++ b/ghost/admin/app/components/settings/members-email-labs.js @@ -1,7 +1,6 @@ import Component from '@glimmer/component'; import {action} from '@ember/object'; import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; const US = {flag: 'πŸ‡ΊπŸ‡Έ', name: 'US', baseUrl: 'https://api.mailgun.net/v3'}; @@ -9,8 +8,6 @@ const EU = {flag: 'πŸ‡ͺπŸ‡Ί', name: 'EU', baseUrl: 'https://api.eu.mailgun.net/v export default class MembersEmailLabs extends Component { @service config; - @service ghostPaths; - @service ajax; @service settings; // set recipientsSelectValue as a static property because within this @@ -19,41 +16,12 @@ export default class MembersEmailLabs extends Component { // from settings as it would equate to "none" @tracked recipientsSelectValue = this._getDerivedRecipientsSelectValue(); - @tracked showFromAddressConfirmation = false; - mailgunRegions = [US, EU]; - replyAddresses = [ - { - label: 'Newsletter email address (' + this.fromAddress + ')', - value: 'newsletter' - }, - { - label: 'Support email address (' + this.supportAddress + ')', - value: 'support' - } - ]; - get emailNewsletterEnabled() { return this.settings.get('editorDefaultEmailRecipients') !== 'disabled'; } - get emailPreviewVisible() { - return this.recipientsSelectValue !== 'none'; - } - - get selectedReplyAddress() { - return this.replyAddresses.findBy('value', this.settings.get('membersReplyAddress')); - } - - get disableUpdateFromAddressButton() { - const savedFromAddress = this.settings.get('membersFromAddress') || ''; - if (!savedFromAddress.includes('@') && this.config.emailDomain) { - return !this.fromAddress || (this.fromAddress === `${savedFromAddress}@${this.config.emailDomain}`); - } - return !this.fromAddress || (this.fromAddress === savedFromAddress); - } - get mailgunRegion() { if (!this.settings.get('mailgunBaseUrl')) { return US; @@ -72,11 +40,6 @@ export default class MembersEmailLabs extends Component { }; } - @action - toggleFromAddressConfirmation() { - this.showFromAddressConfirmation = !this.showFromAddressConfirmation; - } - @action setMailgunDomain(event) { this.settings.set('mailgunDomain', event.target.value); @@ -98,11 +61,6 @@ export default class MembersEmailLabs extends Component { this.settings.set('mailgunBaseUrl', region.baseUrl); } - @action - setFromAddress(fromAddress) { - this.setEmailAddress('fromAddress', fromAddress); - } - @action toggleEmailTrackOpens(event) { if (event) { @@ -129,13 +87,6 @@ export default class MembersEmailLabs extends Component { this.recipientsSelectValue = this._getDerivedRecipientsSelectValue(); } - @action - setReplyAddress(event) { - const newReplyAddress = event.value; - - this.settings.set('membersReplyAddress', newReplyAddress); - } - @action setDefaultEmailRecipients(value) { // Update the underlying setting properties to match the selected recipients option @@ -169,24 +120,6 @@ export default class MembersEmailLabs extends Component { this.settings.set('editorDefaultEmailRecipientsFilter', filter); } - @task({drop: true}) - *updateFromAddress() { - let url = this.ghostPaths.url.api('/settings/members/email'); - try { - const response = yield this.ajax.post(url, { - data: { - email: this.fromAddress, - type: 'fromAddressUpdate' - } - }); - this.toggleFromAddressConfirmation(); - return response; - } catch (e) { - // Failed to send email, retry - return false; - } - } - _getDerivedRecipientsSelectValue() { const defaultEmailRecipients = this.settings.get('editorDefaultEmailRecipients'); const defaultEmailRecipientsFilter = this.settings.get('editorDefaultEmailRecipientsFilter'); diff --git a/ghost/admin/app/controllers/settings/members-email-labs.js b/ghost/admin/app/controllers/settings/members-email-labs.js index c73308202f..4eac4cbac1 100644 --- a/ghost/admin/app/controllers/settings/members-email-labs.js +++ b/ghost/admin/app/controllers/settings/members-email-labs.js @@ -1,37 +1,14 @@ import Controller from '@ember/controller'; -import {action} from '@ember/object'; import {inject as service} from '@ember/service'; import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; export default class MembersEmailLabsController extends Controller { - @service config; - @service session; @service settings; - // from/supportAddress are set here so that they can be reset to saved values on save - // to avoid it looking like they've been saved when they have a separate update process - @tracked fromAddress = ''; - @tracked supportAddress = ''; + queryParams = ['verifyEmail']; - @action - setEmailAddress(property, email) { - this[property] = email; - } - - parseEmailAddress(address) { - const emailAddress = address || 'noreply'; - // Adds default domain as site domain - if (emailAddress.indexOf('@') < 0 && this.config.emailDomain) { - return `${emailAddress}@${this.config.emailDomain}`; - } - return emailAddress; - } - - resetEmailAddresses() { - this.fromAddress = this.parseEmailAddress(this.settings.get('membersFromAddress')); - this.supportAddress = this.parseEmailAddress(this.settings.get('membersSupportAddress')); - } + @tracked verifyEmail = null; @task({drop: true}) *saveSettings() { diff --git a/ghost/admin/app/models/newsletter.js b/ghost/admin/app/models/newsletter.js index 1557bdcc77..9a69254add 100644 --- a/ghost/admin/app/models/newsletter.js +++ b/ghost/admin/app/models/newsletter.js @@ -27,4 +27,8 @@ export default class Newsletter extends Model.extend(ValidationEngine) { @attr({defaultValue: 'sans_serif'}) bodyFontCategory; @attr() footerContent; @attr({defaultValue: true}) showBadge; + + // HACK - not a real model attribute but a workaround for Ember Data not + // exposing meta from save responses + @attr _meta; } diff --git a/ghost/admin/app/routes/settings/members-email-labs.js b/ghost/admin/app/routes/settings/members-email-labs.js index 32c9c5a225..9ef9d9c3d0 100644 --- a/ghost/admin/app/routes/settings/members-email-labs.js +++ b/ghost/admin/app/routes/settings/members-email-labs.js @@ -1,5 +1,6 @@ import AdminRoute from 'ghost-admin/routes/admin'; import ConfirmUnsavedChangesModal from '../../components/modals/confirm-unsaved-changes'; +import VerifyNewsletterEmail from '../../components/modals/edit-newsletter/verify-newsletter-email'; import {action} from '@ember/object'; import {inject as service} from '@ember/service'; @@ -9,30 +10,37 @@ export default class MembersEmailLabsRoute extends AdminRoute { @service notifications; @service settings; + queryParams = { + verifyEmail: { + replace: true + } + }; + confirmModal = null; hasConfirmed = false; - beforeModel(transition) { + beforeModel() { super.beforeModel(...arguments); if (!this.feature.multipleNewsletters) { return this.transitionTo('settings.members-email'); } - - if (transition.to.queryParams?.fromAddressUpdate === 'success') { - this.notifications.showAlert( - `Newsletter email address has been updated`, - {type: 'success', key: 'members.settings.from-address.updated'} - ); - } } model() { return this.settings.reload(); } - setupController(controller) { - controller.resetEmailAddresses(); + afterModel(model, transition) { + if (transition.to.queryParams.verifyEmail) { + this.modals.open(VerifyNewsletterEmail, { + token: transition.to.queryParams.verifyEmail + }); + + // clear query param so it doesn't linger and cause problems re-entering route + transition.abort(); + return this.transitionTo('settings.members-email-labs', {queryParams: {verifyEmail: null}}); + } } @action diff --git a/ghost/admin/app/serializers/newsletter.js b/ghost/admin/app/serializers/newsletter.js new file mode 100644 index 0000000000..d09f69783f --- /dev/null +++ b/ghost/admin/app/serializers/newsletter.js @@ -0,0 +1,27 @@ +/* eslint-disable camelcase */ +import ApplicationSerializer from './application'; + +export default class MemberSerializer extends ApplicationSerializer { + // HACK: Ember Data doesn't expose `meta` properties consistently + // - https://github.com/emberjs/data/issues/2905 + // + // We need the `meta` data returned when saving so we extract it and dump + // it onto the model as an attribute then delete it again when serializing. + normalizeResponse() { + const json = super.normalizeResponse(...arguments); + + if (json.meta && json.data.attributes) { + json.data.attributes._meta = json.meta; + } + + return json; + } + + serialize() { + const json = super.serialize(...arguments); + + delete json._meta; + + return json; + } +} diff --git a/ghost/admin/app/templates/settings/members-email-labs.hbs b/ghost/admin/app/templates/settings/members-email-labs.hbs index 1bcc332c3b..4c2511febf 100644 --- a/ghost/admin/app/templates/settings/members-email-labs.hbs +++ b/ghost/admin/app/templates/settings/members-email-labs.hbs @@ -20,11 +20,7 @@
- +
\ No newline at end of file diff --git a/ghost/admin/mirage/config/newsletters.js b/ghost/admin/mirage/config/newsletters.js index 1ef90ccd40..add2231310 100644 --- a/ghost/admin/mirage/config/newsletters.js +++ b/ghost/admin/mirage/config/newsletters.js @@ -1,8 +1,77 @@ +import {camelize} from '@ember/string'; import {paginatedResponse} from '../utils'; export default function mockNewsletters(server) { - server.post('/newsletters/'); server.get('/newsletters/', paginatedResponse('newsletters')); server.get('/newsletters/:id/'); - server.put('/newsletters/:id/'); + + server.post('/newsletters/', function ({newsletters}) { + const attrs = this.normalizedRequestAttrs(); + + // sender email can't be set without verification + const senderEmail = attrs.senderEmail; + attrs.senderEmail = null; + + const newsletter = newsletters.create(attrs); + + // workaround for mirage output of meta + const collection = newsletters.where({id: newsletter.id}); + + if (senderEmail) { + collection.meta = { + sent_email_verification: ['sender_email'] + }; + } + + return collection; + }); + + server.put('/newsletters/:id/', function ({newsletters}, {params}) { + const attrs = this.normalizedRequestAttrs(); + const newsletter = newsletters.find(params.id); + + const previousSenderEmail = newsletter.senderEmail; + const newSenderEmail = attrs.senderEmail; + + // sender email can't be changed without verification + if (newSenderEmail && newSenderEmail !== previousSenderEmail) { + attrs.senderEmail = previousSenderEmail; + } + + newsletter.update(attrs); + + // workaround for mirage output of meta + const collection = newsletters.where({id: newsletter.id}); + + if (newSenderEmail && newSenderEmail !== previousSenderEmail) { + collection.meta = { + sent_email_verification: ['sender_email'] + }; + + const tokenData = { + id: newsletter.id, + email: newSenderEmail, + type: 'sender_email' + }; + const token = btoa(JSON.stringify(tokenData)); + const baseUrl = window.location.href.replace(window.location.hash, ''); + const verifyUrl = `${baseUrl}settings/members-email-labs/?verifyEmail=${token}`; + // eslint-disable-next-line + console.warn('Verification email sent. Mocked verification URL:', verifyUrl); + } + + return collection; + }); + + // verify email update + server.put('/newsletters/verify-email/', function ({newsletters}, request) { + const requestBody = JSON.parse(request.requestBody); + const tokenData = JSON.parse(atob(requestBody.token)); + + const newsletter = newsletters.find(tokenData.id); + + newsletter[camelize(tokenData.type)] = tokenData.email; + + return newsletter.save(); + }); } diff --git a/ghost/admin/mirage/scenarios/default.js b/ghost/admin/mirage/scenarios/default.js index 8ebf793537..c2a38f0a80 100644 --- a/ghost/admin/mirage/scenarios/default.js +++ b/ghost/admin/mirage/scenarios/default.js @@ -9,4 +9,30 @@ export default function (server) { server.create('integration', {name: 'Demo'}); server.createList('member', 125); + + // sites always have a default newsletter + server.create('newsletter', { + name: 'Site title', + slug: 'site-title', + description: 'Default newsletter created during setup', + + senderName: 'Site title', + senderEmail: null, + senderReplyTo: 'newsletter', + + status: 'active', + recipientFilter: null, + subscribeOnSignup: true, + sortOrder: 0, + + headerImage: null, + showHeaderIcon: true, + showHeaderTitle: true, + titleFontCategory: 'sans_serif', + titleAlignment: 'center', + showFeatureImage: true, + bodyFontCategory: 'sans_serif', + footerContent: null, + showBadge: true + }); } diff --git a/ghost/admin/mirage/serializers/application.js b/ghost/admin/mirage/serializers/application.js index 6874381bcd..1cc58e128a 100644 --- a/ghost/admin/mirage/serializers/application.js +++ b/ghost/admin/mirage/serializers/application.js @@ -24,9 +24,7 @@ export default RestSerializer.extend({ }, serialize(object, request) { - // Ember expects pluralized responses for the post, user, and invite models, - // and this shortcut will ensure that those models are pluralized - if (this.isModel(object) && ['post', 'user', 'invite'].includes(object.modelName)) { + if (this.isModel(object)) { object = new Collection(object.modelName, [object]); }