Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-08 02:52:39 -05:00
Code Issues Activity

Cleaned up newEmailAddresses feature flag (#22001)

Browse source 
ref https://linear.app/ghost/issue/ENG-1416

- "New email addresses" feature was released in [Ghost
v5.78.0](https://github.com/TryGhost/Ghost/releases/tag/v5.78.0)
(commit:
7d0be3f1a9)
- In the context of DMARC changes from February 2024, we've allowed
self-hosters to change their sender and reply-to email addresses without
verification (cf. [Investigation For
Self-hosters](https://www.notion.so/ghost/Investigation-on-FROM-addresses-3f07d724e6044179b38e2793e1d9e797)
and [DMARC Product
Changes](https://www.notion.so/ghost/Working-Document-DMARC-Product-Changes-4cf1e435d8f2452f83cd92dddeaf9d67?pvs=4))
This commit is contained in:
Sag 2025-01-15 10:56:47 +07:00 • committed by GitHub
parent 6ca066c8c3
commit 2cc1e28eca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 118 additions and 584 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx
View file

@ -1,6 +1,6 @@
import NewsletterPreview from './NewsletterPreview';
import NiceModal from '@ebay/nice-modal-react';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import React, {useCallback, useEffect, useState} from 'react';
import useFeatureFlag from '../../../../hooks/useFeatureFlag';
import useSettingGroup from '../../../../hooks/useSettingGroup';
import validator from 'validator';
@ -25,46 +25,18 @@ const ReplyToEmailField: React.FC<{
}> = ({newsletter, updateNewsletter, errors, clearError}) => {
const {settings, config} = useGlobalData();
const [defaultEmailAddress, supportEmailAddress] = getSettingValues<string>(settings, ['default_email_address', 'support_email_address']);
const newEmailAddressesFlag = useFeatureFlag('newEmailAddresses');
// When editing the senderReplyTo, we use a state, so we don't cause jumps when the 'rendering' method decides to change the value
// Because 'newsletter' 'support' or an empty value can be mapped to a default value, we don't want those changes to happen when entering text
const [senderReplyTo, setSenderReplyTo] = useState(renderReplyToEmail(newsletter, config, supportEmailAddress, defaultEmailAddress) || '');
let newsletterAddress = renderSenderEmail(newsletter, config, defaultEmailAddress);
const replyToEmails = useMemo(() => [
{label: `Newsletter address (${newsletterAddress})`, value: 'newsletter'},
{label: `Support address (${supportEmailAddress})`, value: 'support'}
], [newsletterAddress, supportEmailAddress]);
useEffect(() => {
if (!isManagedEmail(config) && !newEmailAddressesFlag) {
// Autocorrect invalid values
const foundValue = replyToEmails.find(option => option.value === newsletter.sender_reply_to);
if (!foundValue) {
updateNewsletter({sender_reply_to: 'newsletter'});
}
}
}, [config, replyToEmails, updateNewsletter, newsletter.sender_reply_to, newEmailAddressesFlag]);
const onChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
setSenderReplyTo(e.target.value);
updateNewsletter({sender_reply_to: e.target.value || 'newsletter'});
}, [updateNewsletter, setSenderReplyTo]);
// Self-hosters, or legacy Pro users
if (!isManagedEmail(config) && !newEmailAddressesFlag) {
// Only allow some choices
return (
<Select
options={replyToEmails}
selectedOption={replyToEmails.find(option => option.value === newsletter.sender_reply_to)}
title="Reply-to email"
onSelect={option => updateNewsletter({sender_reply_to: option?.value})}
/>
);
}
const onBlur = () => {
// Update the senderReplyTo to the rendered value again
const rendered = renderReplyToEmail(newsletter, config, supportEmailAddress, defaultEmailAddress) || '';
@ -189,7 +161,7 @@ const Sidebar: React.FC<{
};
const renderSenderEmailField = () => {
// Self-hosters, or legacy Pro users
// Self-hosters
if (!isManagedEmail(config)) {
return (
<TextField

4
apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterPreviewContent.tsx
View file

@ -3,7 +3,6 @@ import LatestPosts1 from '../../../../assets/images/latest-posts-1.png';
import LatestPosts2 from '../../../../assets/images/latest-posts-2.png';
import LatestPosts3 from '../../../../assets/images/latest-posts-3.png';
import clsx from 'clsx';
import useFeatureFlag from '../../../../hooks/useFeatureFlag';
import {GhostOrb, Icon} from '@tryghost/admin-x-design-system';
import {isManagedEmail} from '@tryghost/admin-x-framework/api/config';
import {textColorForBackgroundColor} from '@tryghost/color-utils';
@ -76,7 +75,6 @@ const NewsletterPreviewContent: React.FC<{
}) => {
const showHeader = headerIcon || headerTitle;
const {config} = useGlobalData();
const hasNewEmailAddresses = useFeatureFlag('newEmailAddresses');
const currentDate = new Date().toLocaleDateString('default', {
year: 'numeric',
@ -89,7 +87,7 @@ const NewsletterPreviewContent: React.FC<{
let emailHeader;
if ({hasNewEmailAddresses} || isManagedEmail(config)) {
if (isManagedEmail(config)) {
emailHeader = <><p className="leading-normal"><span className="font-semibold text-grey-900">From: </span><span>{senderName} ({senderEmail})</span></p>
<p className="leading-normal">
<span className="font-semibold text-grey-900">Reply-to: </span>{senderReplyTo ? senderReplyTo : senderEmail}

2
apps/admin-x-settings/src/utils/newsletterEmails.ts
View file

@ -12,7 +12,7 @@ export const renderSenderEmail = (newsletter: Newsletter, config: Config, defaul
export const renderReplyToEmail = (newsletter: Newsletter, config: Config, supportEmailAddress: string|undefined, defaultEmailAddress: string|undefined) => {
if (newsletter.sender_reply_to === 'newsletter') {
if (isManagedEmail(config) || !!config.labs.newEmailAddresses) {
if (isManagedEmail(config)) {
// No reply-to set
// sender_reply_to currently doesn't allow empty values, we need to set it to 'newsletter'
return '';

52
ghost/core/core/server/services/mail/GhostMailer.js
View file

@ -1,14 +1,12 @@
// # Mail
// Handles sending email for Ghost
const _ = require('lodash');
const validator = require('@tryghost/validator');
const config = require('../../../shared/config');
const errors = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');
const settingsCache = require('../../../shared/settings-cache');
const urlUtils = require('../../../shared/url-utils');
const metrics = require('@tryghost/metrics');
const settingsHelpers = require('../settings-helpers');
const emailAddress = require('../email-address');
const messages = {
title: 'Ghost at {domain}',
@ -19,7 +17,6 @@ const messages = {
messageSent: 'Message sent. Double check inbox and spam folder!'
};
const {EmailAddressParser} = require('@tryghost/email-addresses');
const logging = require('@tryghost/logging');
function getDomain() {
const domain = urlUtils.urlFor('home', true).match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i'));
@ -32,45 +29,24 @@ function getDomain() {
* @returns {{from: string, replyTo?: string|null}}
*/
function getFromAddress(requestedFromAddress, requestedReplyToAddress) {
if (settingsHelpers.useNewEmailAddresses()) {
if (!requestedFromAddress) {
// Use the default config
requestedFromAddress = emailAddress.service.defaultFromEmail;
}
// Clean up email addresses (checks whether sending is allowed + email address is valid)
const addresses = emailAddress.service.getAddressFromString(requestedFromAddress, requestedReplyToAddress);
// fill in missing name if not set
const defaultSiteTitle = settingsCache.get('title') ? settingsCache.get('title') : tpl(messages.title, {domain: getDomain()});
if (!addresses.from.name) {
addresses.from.name = defaultSiteTitle;
}
return {
from: EmailAddressParser.stringify(addresses.from),
replyTo: addresses.replyTo ? EmailAddressParser.stringify(addresses.replyTo) : null
};
}
const configAddress = config.get('mail') && config.get('mail').from;
const address = requestedFromAddress || configAddress;
// If we don't have a from address at all
if (!address) {
// Default to noreply@[blog.url]
return getFromAddress(`noreply@${getDomain()}`, requestedReplyToAddress);
if (!requestedFromAddress) {
// Use the default config
requestedFromAddress = emailAddress.service.defaultFromEmail;
}
// If we do have a from address, and it's just an email
if (validator.isEmail(address, {require_tld: false})) {
const defaultSiteTitle = settingsCache.get('title') ? settingsCache.get('title').replace(/"/g, '\\"') : tpl(messages.title, {domain: getDomain()});
return {
from: `"${defaultSiteTitle}" <${address}>`
};
// Clean up email addresses (checks whether sending is allowed + email address is valid)
const addresses = emailAddress.service.getAddressFromString(requestedFromAddress, requestedReplyToAddress);
// fill in missing name if not set
const defaultSiteTitle = settingsCache.get('title') ? settingsCache.get('title') : tpl(messages.title, {domain: getDomain()});
if (!addresses.from.name) {
addresses.from.name = defaultSiteTitle;
}
logging.warn(`Invalid from address used for sending emails: ${address}`);
return {from: address};
return {
from: EmailAddressParser.stringify(addresses.from),
replyTo: addresses.replyTo ? EmailAddressParser.stringify(addresses.replyTo) : null
};
}
/**

9
ghost/core/core/server/services/members/service.js
View file

@ -92,13 +92,8 @@ const initVerificationTrigger = () => {
isVerificationRequired: () => settingsCache.get('email_verification_required') === true,
sendVerificationEmail: async ({subject, message, amountTriggered}) => {
const escalationAddress = config.get('hostSettings:emailVerification:escalationAddress');
let fromAddress = config.get('user_email');
let replyTo = undefined;
if (settingsHelpers.useNewEmailAddresses()) {
replyTo = fromAddress;
fromAddress = settingsHelpers.getNoReplyAddress();
}
const replyTo = config.get('user_email');
const fromAddress = settingsHelpers.getDefaultEmailAddress();
if (escalationAddress) {
await ghostMailer.send({

32
ghost/core/core/server/services/newsletters/NewslettersService.js
View file

@ -268,20 +268,9 @@ class NewslettersService {
{property: 'sender_email', type: 'from', emptyable: true, error: messages.senderEmailNotAllowed}
];
if (!this.emailAddressService.service.useNewEmailAddresses) {
// Validate reply_to is either newsletter or support
if (cleanedAttrs.sender_reply_to !== undefined) {
if (!['newsletter', 'support'].includes(cleanedAttrs.sender_reply_to)) {
throw new errors.ValidationError({
message: tpl(messages.replyToNotAllowed, {email: cleanedAttrs.sender_reply_to})
});
}
}
} else {
if (cleanedAttrs.sender_reply_to !== undefined) {
if (!['newsletter', 'support'].includes(cleanedAttrs.sender_reply_to)) {
emailProperties.push({property: 'sender_reply_to', type: 'replyTo', emptyable: false, error: messages.replyToNotAllowed});
}
if (cleanedAttrs.sender_reply_to !== undefined) {
if (!['newsletter', 'support'].includes(cleanedAttrs.sender_reply_to)) {
emailProperties.push({property: 'sender_reply_to', type: 'replyTo', emptyable: false, error: messages.replyToNotAllowed});
}
}
@ -355,20 +344,7 @@ class NewslettersService {
* @private
*/
async sendEmailVerificationMagicLink({id, email, property = 'sender_from'}) {
const [,toDomain] = email.split('@');
let fromEmail = `noreply@${toDomain}`;
if (fromEmail === email) {
fromEmail = `no-reply@${toDomain}`;
}
if (this.emailAddressService.service.useNewEmailAddresses) {
// Gone with the old logic: always use the default email address here
// We don't need to validate the FROM address, only the to address
// Also because we are not only validating FROM addresses, but also possible REPLY-TO addresses, which we won't send FROM
fromEmail = this.emailAddressService.service.defaultFromAddress;
}
const fromEmail = this.emailAddressService.service.defaultFromAddress;
const {ghostMailer} = this;
this.magicLinkService.transporter = {

26
ghost/core/core/server/services/settings-helpers/SettingsHelpers.js
View file

@ -117,7 +117,7 @@ class SettingsHelpers {
getMembersSupportAddress() {
let supportAddress = this.settingsCache.get('members_support_address');
if (!supportAddress && this.useNewEmailAddresses()) {
if (!supportAddress) {
// In the new flow, we make a difference between an empty setting (= use default) and a 'noreply' setting (=use noreply @ domain)
// Also keep the name of the default email!
return EmailAddressParser.stringify(this.getDefaultEmail());
@ -144,18 +144,16 @@ class SettingsHelpers {
}
getDefaultEmail() {
if (this.useNewEmailAddresses()) {
// parse the email here and remove the sender name
// E.g. when set to "bar" <from@default.com>
const configAddress = this.config.get('mail:from');
const parsed = EmailAddressParser.parse(configAddress);
if (parsed) {
return parsed;
}
// For missing configs, we default to the old flow
logging.warn('Missing mail.from config, falling back to a generated email address. Please update your config file and set a valid from address');
// parse the email here and remove the sender name
// E.g. when set to "bar" <from@default.com>
const configAddress = this.config.get('mail:from');
const parsed = EmailAddressParser.parse(configAddress);
if (parsed) {
return parsed;
}
// For missing configs, we default to the old flow
logging.warn('Missing mail.from config, falling back to a generated email address. Please update your config file and set a valid from address');
return {
address: this.getLegacyNoReplyAddress()
};
@ -173,10 +171,6 @@ class SettingsHelpers {
return this.isStripeConnected();
}
useNewEmailAddresses() {
return this.#managedEmailEnabled() || this.labs.isSet('newEmailAddresses');
}
createUnsubscribeUrl(uuid, options = {}) {
const siteUrl = this.urlUtils.urlFor('home', true);
const unsubscribeUrl = new URL(siteUrl);

15
ghost/core/core/server/services/settings/SettingsBREADService.js
View file

@ -369,20 +369,7 @@ class SettingsBREADService {
* @private
*/
async sendEmailVerificationMagicLink({email, key}) {
const [,toDomain] = email.split('@');
let fromEmail = `noreply@${toDomain}`;
if (fromEmail === email) {
fromEmail = `no-reply@${toDomain}`;
}
if (this.emailAddressService.service.useNewEmailAddresses) {
// Gone with the old logic: always use the default email address here
// We don't need to validate the FROM address, only the to address
// Also because we are not only validating FROM addresses, but also possible REPLY-TO addresses, which we won't send FROM
fromEmail = this.emailAddressService.service.defaultFromAddress;
}
const fromEmail = this.emailAddressService.service.defaultFromAddress;
const {ghostMailer} = this;
this.magicLinkService.transporter = {

1
ghost/core/core/shared/labs.js
View file

@ -27,7 +27,6 @@ const GA_FEATURES = [
'themeErrorsNotification',
'outboundLinkTagging',
'announcementBar',
'newEmailAddresses',
'customFonts'
];

1
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap
View file

@ -27,7 +27,6 @@ Object {
"lexicalIndicators": true,
"mailEvents": true,
"members": true,
"newEmailAddresses": true,
"outboundLinkTagging": true,
"postsX": true,
"stripeAutomaticTax": true,

62
ghost/core/test/e2e-api/admin/__snapshots__/newsletters.test.js.snap
View file

@ -236,7 +236,7 @@ Object {
"show_post_title_section": true,
"show_subscription_details": false,
"slug": "new-newsletter-with-existing-members-subscribed",
"sort_order": 8,
"sort_order": 7,
"status": "active",
"subscribe_on_signup": true,
"title_alignment": "center",
@ -1039,6 +1039,66 @@ Object {
}
`;
exports[`Newsletters API Can add a newsletter and subscribe existing members 1: [body] 1`] = `
Object {
"meta": Object {
"opted_in_member_count": 6,
},
"newsletters": Array [
Object {
"background_color": "light",
"body_font_category": "serif",
"border_color": null,
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"description": null,
"feedback_enabled": false,
"footer_content": null,
"header_image": null,
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"name": "My test newsletter where I want to subscribe existing members",
"sender_email": null,
"sender_name": null,
"sender_reply_to": "newsletter",
"show_badge": true,
"show_comment_cta": true,
"show_excerpt": false,
"show_feature_image": true,
"show_header_icon": true,
"show_header_name": true,
"show_header_title": true,
"show_latest_posts": false,
"show_post_title_section": true,
"show_subscription_details": false,
"slug": "my-test-newsletter-where-i-want-to-subscribe-existing-members",
"sort_order": 9,
"status": "active",
"subscribe_on_signup": true,
"title_alignment": "center",
"title_color": null,
"title_font_category": "serif",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"visibility": "members",
},
],
}
`;
exports[`Newsletters API Can add a newsletter and subscribe existing members 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "998",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/newsletters\\\\/\\[a-f0-9\\]\\{24\\}\\\\//,
"vary": "Accept-Version, Origin, Accept-Encoding",
"x-cache-invalidate": "/*",
"x-powered-by": "Express",
}
`;
exports[`Newsletters API Can add multiple newsletters 1: [body] 1`] = `
Object {
"newsletters": Array [

2
ghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snap
View file

@ -1155,7 +1155,7 @@ exports[`Settings API Edit Can edit a setting 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4466",
"content-length": "4439",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,

13
ghost/core/test/e2e-api/admin/members.test.js
View file

@ -21,7 +21,7 @@ const urlUtils = require('../../../core/shared/url-utils');
const settingsCache = require('../../../core/shared/settings-cache');
const DomainEvents = require('@tryghost/domain-events');
const logging = require('@tryghost/logging');
const {stripeMocker, mockLabsDisabled} = require('../../utils/e2e-framework-mock-manager');
const {stripeMocker} = require('../../utils/e2e-framework-mock-manager');
const settingsHelpers = require('../../../core/server/services/settings-helpers');
/**
@ -197,7 +197,6 @@ describe('Members API without Stripe', function () {
beforeEach(function () {
mockManager.mockMail();
mockLabsDisabled('newEmailAddresses');
});
afterEach(function () {
@ -490,10 +489,10 @@ describe('Members API', function () {
agent = await agentProvider.getAdminAPIAgent();
await fixtureManager.init('posts', 'newsletters', 'members:newsletters', 'comments', 'redirects', 'clicks');
await agent.loginAsOwner();
newsletters = await getNewsletters();
});
beforeEach(function () {
mockManager.mockStripe();
emailMockReceiver = mockManager.mockMail();
@ -544,7 +543,7 @@ describe('Members API', function () {
etag: anyEtag
});
});
it('Can browse with more than maximum allowed limit', async function () {
await agent
.get('/members/?limit=300')
@ -566,7 +565,7 @@ describe('Members API', function () {
etag: anyEtag
});
});
it('Can browse with limit=all', async function () {
await agent
.get('/members/?limit=all')
@ -588,7 +587,7 @@ describe('Members API', function () {
etag: anyEtag
});
});
it('Can browse with filter', async function () {
await agent
.get('/members/?filter=label:label-1')

252
ghost/core/test/e2e-api/admin/newsletters.test.js
View file

@ -5,7 +5,6 @@ const {anyContentVersion, anyEtag, anyObjectId, anyUuid, anyErrorId, anyISODateT
const {queryStringToken} = regexes;
const models = require('../../../core/server/models');
const logging = require('@tryghost/logging');
const {mockLabsDisabled, mockLabsEnabled} = require('../../utils/e2e-framework-mock-manager');
const settingsHelpers = require('../../../core/server/services/settings-helpers');
const assertMemberRelationCount = async (newsletterId, expectedCount) => {
@ -51,7 +50,6 @@ describe('Newsletters API', function () {
beforeEach(function () {
emailMockReceiver = mockManager.mockMail();
mockLabsDisabled('newEmailAddresses');
});
afterEach(function () {
@ -222,51 +220,6 @@ describe('Newsletters API', function () {
});
});
it('Can add a newsletter - with custom sender_email', async function () {
const newsletter = {
name: 'My test newsletter with custom sender_email',
sender_name: 'Test',
sender_email: 'test@example.com',
sender_reply_to: 'newsletter',
status: 'active',
subscribe_on_signup: true,
title_font_category: 'serif',
body_font_category: 'serif',
show_header_icon: true,
show_header_title: true,
show_badge: true,
sort_order: 0
};
await agent
.post(`newsletters/`)
.body({newsletters: [newsletter]})
.expectStatus(201)
.matchBodySnapshot({
newsletters: [newsletterSnapshot],
meta: {
sent_email_verification: ['sender_email']
}
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag,
location: anyLocationFor('newsletters')
});
emailMockReceiver
.assertSentEmailCount(1)
.matchMetadataSnapshot()
.matchHTMLSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}])
.matchPlaintextSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}]);
});
it('Can add a newsletter - and subscribe existing members', async function () {
const newsletter = {
name: 'New newsletter with existing members subscribed',
@ -336,182 +289,6 @@ describe('Newsletters API', function () {
});
});
it('Can edit a newsletters and update the sender_email when already set', async function () {
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
name: 'Updated newsletter name',
sender_email: 'updated@example.com'
}]
})
.expectStatus(200)
.matchBodySnapshot({
newsletters: [newsletterSnapshot],
meta: {
sent_email_verification: ['sender_email']
}
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
emailMockReceiver
.assertSentEmailCount(1)
.matchMetadataSnapshot()
.matchHTMLSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}])
.matchPlaintextSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}]);
});
it('[Legacy] Can only set newsletter reply to to newsletter or support value', async function () {
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
sender_reply_to: 'support'
}]
})
.expectStatus(200)
.matchBodySnapshot({
newsletters: [newsletterSnapshot]
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
sender_reply_to: 'newsletter'
}]
})
.expectStatus(200)
.matchBodySnapshot({
newsletters: [newsletterSnapshot]
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
});
it('[Legacy] Cannot set newsletter clear sender_reply_to', async function () {
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
sender_reply_to: ''
}]
})
.expectStatus(422)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
});
it('[Legacy] Cannot set newsletter reply-to to any email address', async function () {
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
sender_reply_to: 'hello@acme.com'
}]
})
.expectStatus(422)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
});
it('[Legacy] Cannot set newsletter sender_email to invalid email address', async function () {
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
sender_email: 'notvalid'
}]
})
.expectStatus(422)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});
});
it('Can verify property updates', async function () {
const cheerio = require('cheerio');
const id = fixtureManager.get('newsletters', 0).id;
await agent.put(`newsletters/${id}`)
.body({
newsletters: [{
name: 'Updated newsletter name',
sender_email: 'verify@example.com'
}]
})
.expectStatus(200);
// @NOTE: need a way to return snapshot of sent email from email mock receiver
const mail = mockManager.assert.sentEmail([]);
emailMockReceiver
.assertSentEmailCount(1)
.matchMetadataSnapshot()
.matchHTMLSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}])
.matchPlaintextSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}]);
const $mailHtml = cheerio.load(mail.html);
const verifyUrl = new URL($mailHtml('[data-test-verify-link]').attr('href'));
// convert Admin URL hash to native URL for easier token param extraction
const token = (new URL(verifyUrl.hash.replace('#', ''), 'http://example.com')).searchParams.get('verifyEmail');
await agent.put(`newsletters/verifications`)
.body({
token
})
.expectStatus(200)
.matchBodySnapshot({
newsletters: [newsletterSnapshot]
});
});
describe('Host Settings: newsletter limits', function () {
after(function () {
configUtils.set('hostSettings:limits', undefined);
@ -797,17 +574,9 @@ describe('Newsletters API', function () {
});
});
it('Can add a newsletter - with custom sender_email and subscribe existing members', async function () {
if (dbUtils.isSQLite()) {
// This breaks snapshot tests if you don't update snapshot tests on MySQL + make sure this is the last ADD test
return;
}
it('Can add a newsletter and subscribe existing members', async function () {
const newsletter = {
name: 'My test newsletter with custom sender_email and subscribe existing',
sender_name: 'Test',
sender_email: 'test@example.com',
sender_reply_to: 'newsletter',
name: 'My test newsletter where I want to subscribe existing members',
status: 'active',
subscribe_on_signup: true,
title_font_category: 'serif',
@ -824,27 +593,13 @@ describe('Newsletters API', function () {
.expectStatus(201)
.matchBodySnapshot({
newsletters: [newsletterSnapshot],
meta: {
sent_email_verification: ['sender_email']
}
meta: {opted_in_member_count: 6}
})
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag,
location: anyLocationFor('newsletters')
});
emailMockReceiver
.assertSentEmailCount(1)
.matchMetadataSnapshot()
.matchHTMLSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}])
.matchPlaintextSnapshot([{
pattern: queryStringToken('verifyEmail'),
replacement: 'verifyEmail=REPLACED_TOKEN'
}]);
});
it(`Can't edit multiple newsletters to existing name`, async function () {
@ -1697,7 +1452,6 @@ describe('Newsletters API', function () {
this.beforeEach(function () {
configUtils.set('hostSettings:managedEmail:enabled', false);
configUtils.set('hostSettings:managedEmail:sendingDomain', '');
mockLabsEnabled('newEmailAddresses');
});
it('Can set newsletter reply-to to newsletter or support', async function () {

34
ghost/core/test/e2e-api/admin/settings.test.js
View file

@ -6,7 +6,6 @@ const settingsCache = require('../../../core/shared/settings-cache');
const {agentProvider, fixtureManager, mockManager, matchers, configUtils} = require('../../utils/e2e-framework');
const {stringMatching, anyEtag, anyUuid, anyContentLength, anyContentVersion} = matchers;
const models = require('../../../core/server/models');
const {mockLabsDisabled, mockLabsEnabled} = require('../../utils/e2e-framework-mock-manager');
const {anyErrorId} = matchers;
const CURRENT_SETTINGS_COUNT = 87;
@ -52,7 +51,6 @@ describe('Settings API', function () {
beforeEach(function () {
mockManager.mockMail();
mockLabsDisabled('newEmailAddresses');
});
afterEach(function () {
@ -257,37 +255,6 @@ describe('Settings API', function () {
mockManager.assert.sentEmailCount(0);
});
it('[LEGACY] editing members_support_address triggers email verification flow', async function () {
await agent.put('settings/')
.body({
settings: [{key: 'members_support_address', value: 'support@example.com'}]
})
.expectStatus(200)
.matchBodySnapshot({
settings: matchSettingsArray(CURRENT_SETTINGS_COUNT)
})
.matchHeaderSnapshot({
etag: anyEtag,
// Special rule for this test, as the labs setting changes a lot
'content-length': anyContentLength,
'content-version': anyContentVersion
})
.expect(({body}) => {
const membersSupportAddress = body.settings.find(setting => setting.key === 'members_support_address');
assert.equal(membersSupportAddress.value, 'noreply');
assert.deepEqual(body.meta, {
sent_email_verification: ['members_support_address']
});
});
mockManager.assert.sentEmailCount(1);
mockManager.assert.sentEmail({
subject: 'Verify email address',
to: 'support@example.com'
});
});
it('does not trigger email verification flow if members_support_address remains the same', async function () {
await models.Settings.edit({
key: 'members_support_address',
@ -649,7 +616,6 @@ describe('Settings API', function () {
this.beforeEach(function () {
configUtils.set('hostSettings:managedEmail:enabled', false);
configUtils.set('hostSettings:managedEmail:sendingDomain', '');
mockLabsEnabled('newEmailAddresses');
});
it('editing members_support_address does not trigger email verification flow', async function () {

75
ghost/core/test/integration/services/q-email-addresses.test.js
View file

@ -4,7 +4,7 @@ const mentionsService = require('../../../core/server/services/mentions');
const assert = require('assert/strict');
const {agentProvider, fixtureManager, mockManager} = require('../../utils/e2e-framework');
const configUtils = require('../../utils/configUtils');
const {mockLabsDisabled, mockLabsEnabled, mockSetting} = require('../../utils/e2e-framework-mock-manager');
const {mockSetting} = require('../../utils/e2e-framework-mock-manager');
const ObjectId = require('bson-objectid').default;
const {sendEmail, getDefaultNewsletter, getLastEmail} = require('../../utils/batch-email-utils');
const urlUtils = require('../../utils/urlUtils');
@ -125,7 +125,6 @@ describe('Email addresses', function () {
beforeEach(async function () {
emailMockReceiver = mockManager.mockMail();
mockManager.mockMailgun();
mockLabsDisabled('newEmailAddresses');
configureSite({
siteUrl: 'http://blog.acme.com'
@ -142,77 +141,6 @@ describe('Email addresses', function () {
mockManager.restore();
});
describe('Legacy setup', function () {
it('[STAFF] sends recommendation notification emails from mail.from', async function () {
await sendRecommendationNotification();
assertFromAddress('"Postmaster" <postmaster@examplesite.com>');
});
it('[STAFF] sends new member notification emails from ghost@domain', async function () {
await sendFreeMemberSignupNotification();
assertFromAddress('"Example Site" <ghost@blog.acme.com>');
});
it('[MEMBERS] send a comment reply notification from the generated noreply email address if support address is set to noreply', async function () {
mockSetting('members_support_address', 'noreply');
await sendCommentNotification();
assertFromAddress('"Example Site" <noreply@blog.acme.com>');
});
it('[MEMBERS] send a comment reply notification from the generated noreply email address if no support address is set', async function () {
mockSetting('members_support_address', '');
await sendCommentNotification();
assertFromAddress('"Example Site" <noreply@blog.acme.com>');
});
it('[MEMBERS] send a comment reply notification from the support address', async function () {
await sendCommentNotification();
assertFromAddress('"Example Site" <support@address.com>');
});
it('[NEWSLETTER] Allows to send a newsletter from any configured email address', async function () {
await configureNewsletter({
sender_email: 'anything@possible.com',
sender_name: 'Anything Possible',
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <anything@possible.com>', '"Anything Possible" <anything@possible.com>');
});
it('[NEWSLETTER] Sends from a generated noreply by default', async function () {
await configureNewsletter({
sender_email: null,
sender_name: 'Anything Possible',
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <noreply@blog.acme.com>', '"Anything Possible" <noreply@blog.acme.com>');
});
it('[NEWSLETTER] Can set the reply to to the support address', async function () {
await configureNewsletter({
sender_email: null,
sender_name: 'Anything Possible',
sender_reply_to: 'support'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Anything Possible" <noreply@blog.acme.com>', 'support@address.com');
});
it('[NEWSLETTER] Uses site title as default sender name', async function () {
await configureNewsletter({
sender_email: null,
sender_name: null,
sender_reply_to: 'newsletter'
});
await sendNewsletter();
await assertFromAddressNewsletter('"Example Site" <noreply@blog.acme.com>', '"Example Site" <noreply@blog.acme.com>');
});
});
describe('Custom sending domain', function () {
beforeEach(async function () {
configUtils.set('hostSettings:managedEmail:enabled', true);
@ -401,7 +329,6 @@ describe('Email addresses', function () {
describe('Self-hosted', function () {
beforeEach(async function () {
mockLabsEnabled('newEmailAddresses');
configUtils.set('hostSettings:managedEmail:enabled', false);
configUtils.set('hostSettings:managedEmail:sendingDomain', undefined);
configUtils.set('mail:from', '"Default Address" <default@sendingdomain.com>');

61
ghost/core/test/unit/server/services/newsletters/service.test.js
View file

@ -182,24 +182,6 @@ describe('NewslettersService', function () {
sinon.assert.calledOnceWithExactly(findOneStub, {id: 'test'}, {foo: 'bar', require: true});
});
it('will trigger verification when sender_email is provided', async function () {
const data = {name: 'hello world', sender_email: 'test@example.com'};
const options = {foo: 'bar'};
const result = await newsletterService.add(data, options);
assert.deepEqual(result.meta, {
sent_email_verification: [
'sender_email'
]
});
sinon.assert.calledOnceWithExactly(addStub, {name: 'hello world', sort_order: 1}, options);
mockManager.assert.sentEmail({to: 'test@example.com'});
sinon.assert.calledOnceWithExactly(tokenProvider.create, {id: 'test', property: 'sender_email', value: 'test@example.com'});
sinon.assert.notCalled(fetchMembersStub);
sinon.assert.calledOnceWithExactly(findOneStub, {id: 'test'}, {foo: 'bar', require: true});
});
it('will try to find existing members when opt_in_existing is provided', async function () {
const data = {name: 'hello world'};
const options = {opt_in_existing: true};
@ -268,49 +250,6 @@ describe('NewslettersService', function () {
sinon.assert.calledWithExactly(findOneStub.firstCall, {id: 'test'}, {require: true});
sinon.assert.calledWithExactly(findOneStub.secondCall, {id: 'test'}, {...options, require: true});
});
it('will trigger verification when sender_email is provided', async function () {
const data = {name: 'hello world', sender_email: 'test@example.com'};
const options = {id: 'test', foo: 'bar'};
// Explicitly set the old value to a different value
getStub.withArgs('sender_email').returns('old@example.com');
const result = await newsletterService.edit(data, options);
assert.deepEqual(result.meta, {
sent_email_verification: [
'sender_email'
]
});
sinon.assert.calledOnceWithExactly(editStub, {name: 'hello world'}, options);
sinon.assert.calledTwice(findOneStub);
sinon.assert.calledWithExactly(findOneStub.firstCall, {id: 'test'}, {require: true});
sinon.assert.calledWithExactly(findOneStub.secondCall, {id: 'test'}, {...options, require: true});
mockManager.assert.sentEmail({to: 'test@example.com'});
sinon.assert.calledOnceWithExactly(tokenProvider.create, {id: 'test', property: 'sender_email', value: 'test@example.com'});
});
it('will NOT trigger verification when sender_email is provided but is already verified', async function () {
const data = {name: 'hello world', sender_email: 'test@example.com'};
const options = {foo: 'bar', id: 'test'};
// The model says this is already verified
getStub.withArgs('sender_email').returns('test@example.com');
const result = await newsletterService.edit(data, options);
assert.deepEqual(result.meta, undefined);
sinon.assert.calledOnceWithExactly(editStub, {name: 'hello world', sender_email: 'test@example.com'}, options);
sinon.assert.calledTwice(findOneStub);
sinon.assert.calledWithExactly(findOneStub.firstCall, {id: 'test'}, {require: true});
sinon.assert.calledWithExactly(findOneStub.secondCall, {id: 'test'}, {...options, require: true});
mockManager.assert.sentEmailCount(0);
});
});
describe('verifyPropertyUpdate', function () {

6
ghost/core/test/unit/server/services/settings/settings-bread-service.test.js
View file

@ -5,11 +5,12 @@ const SettingsBreadService = require('../../../../../core/server/services/settin
const urlUtils = require('../../../../../core/shared/url-utils.js');
const {mockManager} = require('../../../../utils/e2e-framework');
const should = require('should');
const emailAddress = require('../../../../../core/server/services/email-address');
describe('UNIT > Settings BREAD Service:', function () {
let emailMockReceiver;
beforeEach(function () {
emailAddress.init();
emailMockReceiver = mockManager.mockMail();
});
@ -195,7 +196,8 @@ describe('UNIT > Settings BREAD Service:', function () {
allowed: true,
verificationEmailRequired: true
};
}
},
defaultFromAddress: 'noreply@example.com'
}
}
});

6
ghost/email-addresses/src/EmailAddressService.ts
View file

@ -48,10 +48,6 @@ export class EmailAddressService {
return this.#getManagedEmailEnabled();
}
get useNewEmailAddresses() {
return this.managedEmailEnabled || this.#labs.isSet('newEmailAddresses');
}
get defaultFromEmail(): EmailAddress {
return this.#getDefaultEmail();
}
@ -152,7 +148,7 @@ export class EmailAddressService {
// Self hoster or legacy Ghost Pro
return {
allowed: true,
verificationEmailRequired: !this.useNewEmailAddresses // Self hosters don't need to verify email addresses
verificationEmailRequired: false // Self hosters don't need to verify email addresses
};
}

5
ghost/staff-service/lib/StaffServiceEmails.js
View file

@ -429,10 +429,7 @@ class StaffServiceEmails {
}
get fromEmailAddress() {
if (this.settingsHelpers.useNewEmailAddresses()) {
return EmailAddressParser.stringify(this.settingsHelpers.getDefaultEmail());
}
return `ghost@${this.defaultEmailDomain}`;
return EmailAddressParser.stringify(this.settingsHelpers.getDefaultEmail());
}
extractInitials(name = '') {

12
ghost/staff-service/test/staff-service.test.js
View file

@ -18,7 +18,7 @@ function testCommonMailData({mailStub, getEmailAlertUsersStub}) {
// has right from/to address
mailStub.calledWith(sinon.match({
from: 'ghost@ghost.example',
from: '"Default" <default@email.com>',
to: 'owner@ghost.org'
})).should.be.true();
@ -152,12 +152,10 @@ describe('StaffService', function () {
};
const settingsHelpers = {
getDefaultEmailDomain: () => {
return 'ghost.example';
},
useNewEmailAddresses: () => {
return false;
}
getDefaultEmail: () => ({
address: 'default@email.com',
name: 'Default'
})
};
beforeEach(function () {