🔒 Added a way to hide the secret settings once they are set

issue https://github.com/TryGhost/Team/issues/621

core/server/api/canary/settings.js

@@ -26,12 +26,14 @@ module.exports = {
                 }));
             }
 
-            // CASE: omit core settings unless internal request
             if (!frame.options.context.internal) {
+                // CASE: omit core settings unless internal request
                 settings = _.filter(settings, (setting) => {
                     const isCore = setting.group === 'core';
                     return !isCore;
                 });
+                // CASE: omit secret settings unless internal request
+                settings = settings.map(settingsService.hideValueIfSecret);
             }
 
             return settings;
@@ -83,6 +85,8 @@ module.exports = {
                 }));
             }
 
+            setting = settingsService.hideValueIfSecret(setting);
+
             return {
                 [frame.options.key]: setting
             };
@@ -230,8 +234,8 @@ module.exports = {
         async query(frame) {
             const stripeConnectIntegrationToken = frame.data.settings.find(setting => setting.key === 'stripe_connect_integration_token');
 
-            // The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
             const settings = frame.data.settings.filter((setting) => {
+                // The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
                 return ![
                     'stripe_connect_integration_token',
                     'stripe_connect_publishable_key',
@@ -239,7 +243,9 @@ module.exports = {
                     'stripe_connect_livemode',
                     'stripe_connect_account_id',
                     'stripe_connect_display_name'
-                ].includes(setting.key);
+                ].includes(setting.key)
+                // Remove obfuscated settings
+                && !(setting.value === settingsService.obfuscatedSetting && settingsService.isSecretSetting(setting));
             });
 
             const getSetting = setting => settingsCache.get(setting.key, {resolve: false});

core/server/api/v2/settings.js

@@ -24,12 +24,14 @@ module.exports = {
                 }));
             }
 
-            // CASE: omit core settings unless internal request
             if (!frame.options.context.internal) {
+                // CASE: omit core settings unless internal request
                 settings = _.filter(settings, (setting) => {
                     const isCore = setting.group === 'core';
                     return !isCore;
                 });
+                // CASE: omit secret settings unless internal request
+                settings = settings.map(settingsService.hideValueIfSecret);
             }
 
             return settings;
@@ -83,6 +85,8 @@ module.exports = {
                 }));
             }
 
+            setting = settingsService.hideValueIfSecret(setting);
+
             return {
                 [frame.options.key]: setting
             };
@@ -123,7 +127,10 @@ module.exports = {
             }
 
             frame.data.settings = _.reject(frame.data.settings, (setting) => {
-                return setting.key === 'type';
+                return setting.key === 'type'
+                    // Remove obfuscated settings
+                    || (setting.value === settingsService.obfuscatedSetting
+                        && settingsService.isSecretSetting(setting));
             });
 
             const errors = [];

core/server/api/v3/settings.js

@@ -26,12 +26,14 @@ module.exports = {
                 }));
             }
 
-            // CASE: omit core settings unless internal request
             if (!frame.options.context.internal) {
+                // CASE: omit core settings unless internal request
                 settings = _.filter(settings, (setting) => {
                     const isCore = setting.group === 'core';
                     return !isCore;
                 });
+                // CASE: omit secret settings unless internal request
+                settings = settings.map(settingsService.hideValueIfSecret);
             }
 
             return settings;
@@ -83,6 +85,8 @@ module.exports = {
                 }));
             }
 
+            setting = settingsService.hideValueIfSecret(setting);
+
             return {
                 [frame.options.key]: setting
             };
@@ -230,8 +234,8 @@ module.exports = {
         async query(frame) {
             const stripeConnectIntegrationToken = frame.data.settings.find(setting => setting.key === 'stripe_connect_integration_token');
 
-            // The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
             const settings = frame.data.settings.filter((setting) => {
+                // The `stripe_connect_integration_token` "setting" is only used to set the `stripe_connect_*` settings.
                 return ![
                     'stripe_connect_integration_token',
                     'stripe_connect_publishable_key',
@@ -239,7 +243,9 @@ module.exports = {
                     'stripe_connect_livemode',
                     'stripe_connect_account_id',
                     'stripe_connect_display_name'
-                ].includes(setting.key);
+                ].includes(setting.key)
+                // Remove obfuscated settings
+                && !(setting.value === settingsService.obfuscatedSetting && settingsService.isSecretSetting(setting));
             });
 
             const getSetting = setting => settingsCache.get(setting.key, {resolve: false});

core/server/services/settings/index.js

@@ -5,6 +5,22 @@
 const models = require('../../models');
 const SettingsCache = require('./cache');
 
+// The string returned when a setting is set as write-only
+const obfuscatedSetting = '••••••••';
+
+// The function used to decide whether a setting is write-only
+function isSecretSetting(setting) {
+    return /secret/.test(setting.key);
+}
+
+// The function that obfuscates a write-only setting
+function hideValueIfSecret(setting) {
+    if (setting.value && isSecretSetting(setting)) {
+        return {...setting, value: obfuscatedSetting};
+    }
+    return setting;
+}
+
 module.exports = {
     async init() {
         const settingsCollection = await models.Settings.populateDefaults();
@@ -44,5 +60,9 @@ module.exports = {
                 value: currentRoutesHash
             }], {context: {internal: true}});
         }
-    }
+    },
+
+    obfuscatedSetting,
+    isSecretSetting,
+    hideValueIfSecret
 };

test/regression/api/canary/admin/settings_spec.js

@@ -210,6 +210,66 @@ describe('Settings API (canary)', function () {
                 .expect(403);
         });
 
+        it('Can\'t read secret setting', function (done) {
+            const key = 'stripe_secret_key';
+            request
+                .get(localUtils.API.getApiQuery(`settings/${key}/`))
+                .set('Origin', config.get('url'))
+                .expect('Content-Type', /json/)
+                .expect('Cache-Control', testUtils.cacheRules.private)
+                .expect(200)
+                .end(function (err, res) {
+                    if (err) {
+                        return done(err);
+                    }
+
+                    const json = res.body;
+                    should.exist(json);
+                    should.exist(json.settings);
+
+                    json.settings.length.should.eql(1);
+                    json.settings[0].key.should.eql('stripe_secret_key');
+                    should(json.settings[0].value).be.null();
+
+                    done();
+                });
+        });
+
+        it('Can\'t read secret setting', function (done) {
+            const key = 'stripe_secret_key';
+            request.put(localUtils.API.getApiQuery('settings/'))
+                .set('Origin', config.get('url'))
+                .send({
+                    settings: [{
+                        key,
+                        value: 'sk_test_4eC39HqLyjWDarjtT1zdp7dc'
+                    }]
+                })
+                .then(() => {
+                    request
+                        .get(localUtils.API.getApiQuery(`settings/${key}/`))
+                        .set('Origin', config.get('url'))
+                        .expect('Content-Type', /json/)
+                        .expect('Cache-Control', testUtils.cacheRules.private)
+                        .expect(200)
+                        .end(function (err, res) {
+                            if (err) {
+                                return done(err);
+                            }
+
+                            const json = res.body;
+                            should.exist(json);
+                            should.exist(json.settings);
+
+                            json.settings.length.should.eql(1);
+                            json.settings[0].key.should.eql('stripe_secret_key');
+                            json.settings[0].value.should.eql('••••••••');
+
+                            done();
+                        });
+                });
+        });
+
         it('Can\'t read permalinks', function (done) {
             request.get(localUtils.API.getApiQuery('settings/permalinks/'))
                 .set('Origin', config.get('url'))