From 1ba2988e98f1f677739a0734e4e476737519029a Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Tue, 19 Jul 2022 15:19:08 +0200 Subject: [PATCH 1/4] Added `v5.*` to workflow triggers - this is needed so we run CI tests on release branches --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2fcc54d280..7f7bdf2906 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,7 +5,7 @@ on: branches: - main - '5.0' - - 'v4.*' + - 'v5.*' - 3.x - 2.x - 'renovate/*' From 309ead610842ffd5b2c2c2f582f3960e3f0918a2 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Tue, 19 Jul 2022 15:13:29 +0200 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20sending=20multiple?= =?UTF-8?q?=20support=20email=20verification=20emails=20(#15044)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Team/issues/1686 - When the settings are updated with the `members_support_address` key present, it would always send a verification email - Root cause is that the service failed to check if the email was changed or not. Due to a bug it always thought the email was changed, triggering the verification flow. - The admin app will always send all the settings keys when changing some other value. This causes a lot of email verification emails. - Added tests and email count checks in tests --- .../settings/settings-bread-service.js | 2 +- .../admin/__snapshots__/settings.test.js.snap | 289 ++++++++++++++++++ test/e2e-api/admin/settings.test.js | 36 ++- 3 files changed, 325 insertions(+), 2 deletions(-) diff --git a/core/server/services/settings/settings-bread-service.js b/core/server/services/settings/settings-bread-service.js index f0f1cb2ba0..e2e138d169 100644 --- a/core/server/services/settings/settings-bread-service.js +++ b/core/server/services/settings/settings-bread-service.js @@ -297,7 +297,7 @@ class SettingsBREADService { if (EMAIL_KEYS.includes(setting.key)) { const email = setting.value; const key = setting.key; - const hasChanged = getSetting(setting) !== email; + const hasChanged = getSetting(setting).value !== email; if (await this.requiresEmailVerification({email, hasChanged})) { emailsToVerify.push({email, key}); diff --git a/test/e2e-api/admin/__snapshots__/settings.test.js.snap b/test/e2e-api/admin/__snapshots__/settings.test.js.snap index 7f57a0feab..7727c5620c 100644 --- a/test/e2e-api/admin/__snapshots__/settings.test.js.snap +++ b/test/e2e-api/admin/__snapshots__/settings.test.js.snap @@ -916,6 +916,295 @@ Object { } `; +exports[`Settings API Edit does not trigger email verification flow if members_support_address remains the same 1: [body] 1`] = ` +Object { + "meta": Object {}, + "settings": Array [ + Object { + "key": "title", + "value": "[]", + }, + Object { + "key": "description", + "value": "Thoughts, stories and ideas", + }, + Object { + "key": "logo", + "value": "", + }, + Object { + "key": "cover_image", + "value": "https://static.ghost.org/v4.0.0/images/publication-cover.jpg", + }, + Object { + "key": "icon", + "value": "http://127.0.0.1:2369/content/images/size/w256h256/2019/07/icon.png", + }, + Object { + "key": "accent_color", + "value": "#FF1A75", + }, + Object { + "key": "locale", + "value": "ua", + }, + Object { + "key": "timezone", + "value": "Pacific/Auckland", + }, + Object { + "key": "codeinjection_head", + "value": null, + }, + Object { + "key": "codeinjection_foot", + "value": "", + }, + Object { + "key": "facebook", + "value": "ghost", + }, + Object { + "key": "twitter", + "value": "@ghost", + }, + Object { + "key": "navigation", + "value": "[{\\"label\\":\\"label1\\"}]", + }, + Object { + "key": "secondary_navigation", + "value": "[{\\"label\\":\\"Data & privacy\\",\\"url\\":\\"/privacy/\\"},{\\"label\\":\\"Contact\\",\\"url\\":\\"/contact/\\"},{\\"label\\":\\"Contribute →\\",\\"url\\":\\"/contribute/\\"}]", + }, + Object { + "key": "meta_title", + "value": "SEO title", + }, + Object { + "key": "meta_description", + "value": "SEO description", + }, + Object { + "key": "og_image", + "value": "http://127.0.0.1:2369/content/images/2019/07/facebook.png", + }, + Object { + "key": "og_title", + "value": "facebook title", + }, + Object { + "key": "og_description", + "value": "facebook description", + }, + Object { + "key": "twitter_image", + "value": "http://127.0.0.1:2369/content/images/2019/07/twitter.png", + }, + Object { + "key": "twitter_title", + "value": "twitter title", + }, + Object { + "key": "twitter_description", + "value": "twitter description", + }, + Object { + "key": "active_theme", + "value": "casper", + }, + Object { + "key": "is_private", + "value": false, + }, + Object { + "key": "password", + "value": "", + }, + Object { + "key": "public_hash", + "value": StringMatching /\\[a-z0-9\\]\\{30\\}/, + }, + Object { + "key": "default_content_visibility", + "value": "public", + }, + Object { + "key": "default_content_visibility_tiers", + "value": "[]", + }, + Object { + "key": "members_signup_access", + "value": "all", + }, + Object { + "key": "members_support_address", + "value": "support@example.com", + }, + Object { + "key": "stripe_secret_key", + "value": null, + }, + Object { + "key": "stripe_publishable_key", + "value": null, + }, + Object { + "key": "stripe_plans", + "value": "[]", + }, + Object { + "key": "stripe_connect_publishable_key", + "value": "pk_test_for_stripe", + }, + Object { + "key": "stripe_connect_secret_key", + "value": "••••••••", + }, + Object { + "key": "stripe_connect_livemode", + "value": null, + }, + Object { + "key": "stripe_connect_display_name", + "value": null, + }, + Object { + "key": "stripe_connect_account_id", + "value": null, + }, + Object { + "key": "members_monthly_price_id", + "value": null, + }, + Object { + "key": "members_yearly_price_id", + "value": null, + }, + Object { + "key": "portal_name", + "value": true, + }, + Object { + "key": "portal_button", + "value": true, + }, + Object { + "key": "portal_plans", + "value": "[\\"free\\"]", + }, + Object { + "key": "portal_products", + "value": "[]", + }, + Object { + "key": "portal_button_style", + "value": "icon-and-text", + }, + Object { + "key": "portal_button_icon", + "value": null, + }, + Object { + "key": "portal_button_signup_text", + "value": "Subscribe", + }, + Object { + "key": "mailgun_domain", + "value": null, + }, + Object { + "key": "mailgun_api_key", + "value": null, + }, + Object { + "key": "mailgun_base_url", + "value": null, + }, + Object { + "key": "email_track_opens", + "value": true, + }, + Object { + "key": "email_verification_required", + "value": false, + }, + Object { + "key": "amp", + "value": false, + }, + Object { + "key": "amp_gtag_id", + "value": null, + }, + Object { + "key": "firstpromoter", + "value": false, + }, + Object { + "key": "firstpromoter_id", + "value": null, + }, + Object { + "key": "labs", + "value": "{\\"members\\":true}", + }, + Object { + "key": "slack_url", + "value": "", + }, + Object { + "key": "slack_username", + "value": "New Slack Username", + }, + Object { + "key": "unsplash", + "value": false, + }, + Object { + "key": "shared_views", + "value": "[]", + }, + Object { + "key": "editor_default_email_recipients", + "value": "visibility", + }, + Object { + "key": "editor_default_email_recipients_filter", + "value": "all", + }, + Object { + "key": "members_enabled", + "value": true, + }, + Object { + "key": "members_invite_only", + "value": false, + }, + Object { + "key": "paid_members_enabled", + "value": true, + }, + Object { + "key": "firstpromoter_account", + "value": null, + }, + ], +} +`; + +exports[`Settings API Edit does not trigger email verification flow if members_support_address remains the same 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": "3376", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Origin, Accept-Encoding", + "x-cache-invalidate": "/*", + "x-powered-by": "Express", +} +`; + exports[`Settings API Edit editing members_support_address triggers email verification flow 1: [body] 1`] = ` Object { "meta": Object { diff --git a/test/e2e-api/admin/settings.test.js b/test/e2e-api/admin/settings.test.js index 8036ce8422..cf263d4cb3 100644 --- a/test/e2e-api/admin/settings.test.js +++ b/test/e2e-api/admin/settings.test.js @@ -167,6 +167,8 @@ describe('Settings API', function () { .matchHeaderSnapshot({ etag: anyEtag }); + + mockManager.assert.sentEmailCount(0); }); it('removes image size prefixes when setting the icon', async function () { @@ -197,6 +199,8 @@ describe('Settings API', function () { // Check if not changed (also check internal ones) const afterValue = settingsCache.get('icon'); assert.equal(afterValue, 'http://127.0.0.1:2369/content/images/2019/07/icon.png'); + + mockManager.assert.sentEmailCount(0); }); it('cannot edit uneditable settings', async function () { @@ -215,6 +219,7 @@ describe('Settings API', function () { const emailVerificationRequired = body.settings.find(setting => setting.key === 'email_verification_required'); assert.strictEqual(emailVerificationRequired.value, false); }); + mockManager.assert.sentEmailCount(0); }); it('editing members_support_address triggers email verification flow', async function () { @@ -237,12 +242,40 @@ describe('Settings API', function () { 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', + value: 'support@example.com' + }); + + 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 + }) + .expect(({body}) => { + const membersSupportAddress = body.settings.find(setting => setting.key === 'members_support_address'); + assert.strictEqual(membersSupportAddress.value, 'support@example.com'); + + assert.deepEqual(body.meta, {}); + }); + + mockManager.assert.sentEmailCount(0); + }); }); describe('verify key update', function () { @@ -263,6 +296,7 @@ describe('Settings API', function () { const membersSupportAddress = body.settings.find(setting => setting.key === 'members_support_address'); assert.strictEqual(membersSupportAddress.value, 'support@example.com'); }); + mockManager.assert.sentEmailCount(0); }); it('cannot update invalid keys via token', async function () { From 445d5b4da20e8bd252c186beeeb23d8c08b60268 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Tue, 19 Jul 2022 15:34:33 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20new=20member=20count?= =?UTF-8?q?=20helpers=20not=20included=20in=20GScan=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - this was due to GScan not being published and bumped before the member count helpers were merged into Ghost and released --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index f3afbf4d90..3afdc28321 100644 --- a/package.json +++ b/package.json @@ -144,7 +144,7 @@ "ghost-storage-base": "1.0.0", "glob": "8.0.3", "got": "9.6.0", - "gscan": "4.31.2", + "gscan": "4.32.0", "html-to-text": "8.2.0", "human-number": "2.0.0", "image-size": "1.0.2", diff --git a/yarn.lock b/yarn.lock index a777b5fb2a..25164a2320 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6213,10 +6213,10 @@ grunt@1.5.3: nopt "~3.0.6" rimraf "~3.0.2" -gscan@4.31.2: - version "4.31.2" - resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.31.2.tgz#d8dd1f156f7a59f655625c3422cfd89bb42df8ac" - integrity sha512-NuFcXRbSUSWOl6FuGPKT24dcepI/oMiUF6EGzMEGbY5ndl6Ba/BZrK9dT7hMHeyaJCn+T4fpxTPQqfXHkM2qcg== +gscan@4.32.0: + version "4.32.0" + resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.32.0.tgz#eb3211d7dd9c3b1cc3e9da883fe69283fb3bcbc8" + integrity sha512-ntNUO3J7Rc9KLnaJQ+QoPrrPWfoWa+8AiuYfynu7QWTq/YebC5zmuYgZMyPVvRc7C669E5A0U3VOxyBEbeth8w== dependencies: "@sentry/node" "6.19.7" "@tryghost/config" "0.2.9" From e39937ae21aacdb4a921e287405bf06bcfb72105 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 19 Jul 2022 14:57:42 +0100 Subject: [PATCH 4/4] v5.4.1 --- core/admin | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/admin b/core/admin index 0ab069218b..65f136737d 160000 --- a/core/admin +++ b/core/admin @@ -1 +1 @@ -Subproject commit 0ab069218b953836aecba1c2e75d5d36b716b9dd +Subproject commit 65f136737dbb6ae44ebaa57536552c58d0f9b94a diff --git a/package.json b/package.json index 3afdc28321..dfd6fb94e5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "5.4.0", + "version": "5.4.1", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org",