From 715d658e11837db9ade69b5ed51e110d81096379 Mon Sep 17 00:00:00 2001
From: Simon Backx <simon@ghost.org>
Date: Wed, 20 Sep 2023 17:35:10 +0200
Subject: [PATCH] Disabled one-click subscribe when Portal checkbox is required
 (#18252)

fixes https://github.com/TryGhost/Product/issues/3911

For now we decided that we don't want to enable one-click-subscribe in
case a site has a required checkbox (which isn't shown during the
one-click-subscribe flow)
---
 .../src/api/external-ghost-site.ts            | 19 +++++-
 .../AddRecommendationModal.tsx                |  2 +-
 .../utils/serializers/output/site.js          |  2 +-
 .../server/services/public-config/site.js     |  2 +-
 .../admin/__snapshots__/site.test.js.snap     |  2 +-
 .../members/__snapshots__/site.test.js.snap   | 60 ++++++++++++++++++-
 ghost/core/test/e2e-api/members/site.test.js  | 42 ++++++++++++-
 .../shared/__snapshots__/version.test.js.snap | 10 ++--
 8 files changed, 123 insertions(+), 16 deletions(-)

diff --git a/apps/admin-x-settings/src/api/external-ghost-site.ts b/apps/admin-x-settings/src/api/external-ghost-site.ts
index 9ccc071b43..52e61be47e 100644
--- a/apps/admin-x-settings/src/api/external-ghost-site.ts
+++ b/apps/admin-x-settings/src/api/external-ghost-site.ts
@@ -7,7 +7,7 @@ export type GhostSiteResponse = {
         logo: URL | null,
         icon: URL | null,
         cover_image : URL | null,
-        allow_self_signup: boolean,
+        allow_external_signup: boolean,
         url: URL,
     }
 }
@@ -35,7 +35,20 @@ export const useExternalGhostSite = () => {
                 });
 
                 // We need to validate all data types here for extra safety
-                if (typeof result !== 'object' || !result.site || typeof result.site !== 'object' || typeof result.site.title !== 'string' || typeof result.site.allow_self_signup !== 'boolean' || typeof result.site.url !== 'string') {
+                if (typeof result !== 'object' || !result.site || typeof result.site !== 'object') {
+                    // eslint-disable-next-line no-console
+                    console.warn('Received invalid response from external Ghost site API', result);
+                    return null;
+                }
+
+                // Temporary mapping (should get removed!)
+                // allow_self_signup was replaced by allow_external_signup
+                if (typeof result.site.allow_self_signup === 'boolean' && typeof result.site.allow_external_signup !== 'boolean') {
+                    result.site.allow_external_signup = result.site.allow_self_signup;
+                }
+
+                // We need to validate all data types here for extra safety
+                if (typeof result.site.title !== 'string' || typeof result.site.allow_external_signup !== 'boolean' || typeof result.site.url !== 'string') {
                     // eslint-disable-next-line no-console
                     console.warn('Received invalid response from external Ghost site API', result);
                     return null;
@@ -74,7 +87,7 @@ export const useExternalGhostSite = () => {
                             logo: result.site.logo ? new URL(result.site.logo) : null,
                             icon: result.site.icon ? new URL(result.site.icon) : null,
                             cover_image: result.site.cover_image ? new URL(result.site.cover_image) : null,
-                            allow_self_signup: result.site.allow_self_signup,
+                            allow_external_signup: result.site.allow_external_signup,
                             url: new URL(result.site.url)
                         }
                     };
diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx
index 0ea2a0d1fc..0cd615986c 100644
--- a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx
+++ b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx
@@ -58,7 +58,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
                 updatedRecommendation.excerpt = externalGhostSite.site.description ?? formState.excerpt ?? null;
                 updatedRecommendation.featured_image = externalGhostSite.site.cover_image?.toString() ?? formState.featured_image ?? null;
                 updatedRecommendation.favicon = externalGhostSite.site.icon?.toString() ?? externalGhostSite.site.logo?.toString() ?? formState.favicon ?? null;
-                updatedRecommendation.one_click_subscribe = externalGhostSite.site.allow_self_signup;
+                updatedRecommendation.one_click_subscribe = externalGhostSite.site.allow_external_signup;
             } else {
                 // For non-Ghost sites, we use the Oemebd API to fetch metadata
                 const oembed = await queryOembed({
diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js
index dcdbf9abac..d5c49a4337 100644
--- a/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js
+++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js
@@ -16,7 +16,7 @@ module.exports = {
                 'locale',
                 'url',
                 'version',
-                'allow_self_signup',
+                'allow_external_signup',
                 'sentry_dsn',
                 'sentry_env'
             ])
diff --git a/ghost/core/core/server/services/public-config/site.js b/ghost/core/core/server/services/public-config/site.js
index fda63ccdc0..b94672af8b 100644
--- a/ghost/core/core/server/services/public-config/site.js
+++ b/ghost/core/core/server/services/public-config/site.js
@@ -14,7 +14,7 @@ module.exports = function getSiteProperties() {
         locale: settingsCache.get('locale'),
         url: urlUtils.urlFor('home', true),
         version: ghostVersion.safe,
-        allow_self_signup: settingsCache.get('allow_self_signup')
+        allow_external_signup: settingsCache.get('allow_self_signup') && !(settingsCache.get('portal_signup_checkbox_required') && settingsCache.get('portal_signup_terms_html'))
     };
 
     if (config.get('client_sentry') && !config.get('client_sentry').disabled) {
diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap
index f4a9615adf..d2439a2368 100644
--- a/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap
+++ b/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap
@@ -4,7 +4,7 @@ exports[`Site API can retrieve config and all expected properties 1: [body] 1`]
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
diff --git a/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap b/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap
index 1d0e0e47a4..00df132c28 100644
--- a/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap
+++ b/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap
@@ -4,7 +4,7 @@ exports[`Site Public Settings Can retrieve site pubic config 1: [body] 1`] = `
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
@@ -29,6 +29,64 @@ Object {
 }
 `;
 
+exports[`Site Public Settings Sets allow_external_signup to false when members are invite only 1: [body] 1`] = `
+Object {
+  "site": Object {
+    "accent_color": "#FF1A75",
+    "allow_external_signup": false,
+    "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
+    "description": "Thoughts, stories and ideas",
+    "icon": null,
+    "locale": "en",
+    "logo": null,
+    "title": "Ghost",
+    "url": "http://127.0.0.1:2369/",
+    "version": StringMatching /\\\\d\\+\\\\\\.\\\\d\\+/,
+  },
+}
+`;
+
+exports[`Site Public Settings Sets allow_external_signup to false when members are invite only 2: [headers] 1`] = `
+Object {
+  "access-control-allow-origin": "*",
+  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
+  "content-length": StringMatching /\\\\d\\+/,
+  "content-type": "application/json; charset=utf-8",
+  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
+  "vary": "Accept-Encoding",
+  "x-powered-by": "Express",
+}
+`;
+
+exports[`Site Public Settings Sets allow_external_signup to false when portal requires checkbox 1: [body] 1`] = `
+Object {
+  "site": Object {
+    "accent_color": "#FF1A75",
+    "allow_external_signup": false,
+    "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
+    "description": "Thoughts, stories and ideas",
+    "icon": null,
+    "locale": "en",
+    "logo": null,
+    "title": "Ghost",
+    "url": "http://127.0.0.1:2369/",
+    "version": StringMatching /\\\\d\\+\\\\\\.\\\\d\\+/,
+  },
+}
+`;
+
+exports[`Site Public Settings Sets allow_external_signup to false when portal requires checkbox 2: [headers] 1`] = `
+Object {
+  "access-control-allow-origin": "*",
+  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
+  "content-length": StringMatching /\\\\d\\+/,
+  "content-type": "application/json; charset=utf-8",
+  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
+  "vary": "Accept-Encoding",
+  "x-powered-by": "Express",
+}
+`;
+
 exports[`Site Public Settings Sets allow_self_signup to false when members are invite only 1: [body] 1`] = `
 Object {
   "site": Object {
diff --git a/ghost/core/test/e2e-api/members/site.test.js b/ghost/core/test/e2e-api/members/site.test.js
index 6c44ac55c6..ba94c9653b 100644
--- a/ghost/core/test/e2e-api/members/site.test.js
+++ b/ghost/core/test/e2e-api/members/site.test.js
@@ -11,6 +11,13 @@ describe('Site Public Settings', function () {
         await fixtureManager.init();
     });
 
+    afterEach(async function () {
+        await await models.Settings.edit({
+            key: 'members_signup_access',
+            value: 'all'
+        }, {context: {internal: true}});
+    });
+
     it('Can retrieve site pubic config', async function () {
         const {body} = await membersAgent
             .get('/api/site')
@@ -23,10 +30,10 @@ describe('Site Public Settings', function () {
                 etag: anyEtag,
                 'content-length': anyContentLength
             });
-        assert.equal(body.site.allow_self_signup, true);
+        assert.equal(body.site.allow_external_signup, true);
     });
 
-    it('Sets allow_self_signup to false when members are invite only', async function () {
+    it('Sets allow_external_signup to false when members are invite only', async function () {
         await await models.Settings.edit({
             key: 'members_signup_access',
             value: 'invite'
@@ -43,6 +50,35 @@ describe('Site Public Settings', function () {
                 etag: anyEtag,
                 'content-length': anyContentLength
             });
-        assert.equal(body.site.allow_self_signup, false);
+        assert.equal(body.site.allow_external_signup, false);
+    });
+
+    it('Sets allow_external_signup to false when portal requires checkbox', async function () {
+        const {body: initialBody} = await membersAgent
+            .get('/api/site');
+        assert.equal(initialBody.site.allow_external_signup, true, 'This test requires the initial state to allow external signups');
+
+        await await models.Settings.edit({
+            key: 'portal_signup_checkbox_required',
+            value: true
+        }, {context: {internal: true}});
+
+        await await models.Settings.edit({
+            key: 'portal_signup_terms_html',
+            value: 'I agree to the terms and conditions'
+        }, {context: {internal: true}});
+
+        const {body} = await membersAgent
+            .get('/api/site')
+            .matchBodySnapshot({
+                site: {
+                    version: stringMatching(/\d+\.\d+/)
+                }
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                'content-length': anyContentLength
+            });
+        assert.equal(body.site.allow_external_signup, false);
     });
 });
diff --git a/ghost/core/test/e2e-api/shared/__snapshots__/version.test.js.snap b/ghost/core/test/e2e-api/shared/__snapshots__/version.test.js.snap
index 58ed9e5475..c90b85a3d4 100644
--- a/ghost/core/test/e2e-api/shared/__snapshots__/version.test.js.snap
+++ b/ghost/core/test/e2e-api/shared/__snapshots__/version.test.js.snap
@@ -4,7 +4,7 @@ exports[`API Versioning Admin API Does an internal rewrite for canary URLs with
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
@@ -83,7 +83,7 @@ exports[`API Versioning Admin API allows invalid accept-version header 1: [body]
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
@@ -231,7 +231,7 @@ exports[`API Versioning Admin API responds with content version header even when
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
@@ -261,7 +261,7 @@ exports[`API Versioning Admin API responds with current content version header w
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,
@@ -291,7 +291,7 @@ exports[`API Versioning Admin API responds with current content version header w
 Object {
   "site": Object {
     "accent_color": "#FF1A75",
-    "allow_self_signup": true,
+    "allow_external_signup": true,
     "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg",
     "description": "Thoughts, stories and ideas",
     "icon": null,