diff --git a/api/api.js b/api/api.js index c2c2d75f621..b09ae9ea22c 100644 --- a/api/api.js +++ b/api/api.js @@ -90,6 +90,43 @@ plugins.connectToAllDatabases().then(function() { trim_trailing_ending_spaces: false }); + //What a non global admin may read from the api namespace. The first group is what + //App Management renders: the dashboard infers each input's widget from the type of + //the value here, so a key missing from this list means that setting disappears from + //the screen. Keep it in step with showInAppManagment in the plugins frontend. + // + //domain is separate: crash symbolication builds its return URL from it, and falls + //back to the browser's origin when it is absent, which is wrong behind a custom + //domain rather than merely cosmetic. + plugins.setReadableConfigs("api", { + safe: true, + session_duration_limit: true, + country_data: true, + city_data: true, + event_limit: true, + event_segmentation_limit: true, + event_segmentation_value_limit: true, + metric_limit: true, + session_cooldown: true, + total_users: true, + prevent_duplicate_requests: true, + metric_changes: true, + trim_trailing_ending_spaces: true, + domain: true + }); + + //the dashboard reads the password policy to validate a new password before sending + //it. Nothing else from this namespace is readable, which is what keeps the proxy + //credentials out of the response. + plugins.setReadableConfigs("security", { + password_min: true, + password_char: true, + password_number: true, + password_symbol: true, + password_expiration: true, + password_autocomplete: true + }); + /** * Set Plugins APPs Config */ @@ -124,6 +161,15 @@ plugins.connectToAllDatabases().then(function() { proxy_type: "https" }); + //the outbound proxy credentials. The whole security namespace used to be + //serialized into the dashboard page and returned to any app admin, so these were + //readable well below the operator who set them. The dashboard only uses the + //password policy keys from this namespace, so nothing needs them. + plugins.setSecretConfigs("security", { + proxy_username: true, + proxy_password: true + }); + /** * Set tracking config */ diff --git a/frontend/express/app.js b/frontend/express/app.js index b64ed4503c8..293b775cc5f 100644 --- a/frontend/express/app.js +++ b/frontend/express/app.js @@ -992,9 +992,15 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_ csrf_token: req.csrfToken(), auth_token: req.session.auth_token, member: member, - config: req.config, - security: plugins.getConfig("security"), - tracking: plugins.getConfig("tracking"), + //these namespaces are serialized into the page, so every logged in + //user can read them in the source regardless of their permissions. + //Secret values are dropped rather than masked: the page has no use + //for a placeholder. Anything the browser genuinely needs must not + //be marked secret, since masking what is already in page source + //would achieve nothing. + config: plugins.omitSecretConfigs("frontend", req.config), + security: plugins.omitSecretConfigs("security", plugins.getConfig("security")), + tracking: plugins.omitSecretConfigs("tracking", plugins.getConfig("tracking")), plugins: plugins.getPlugins(), pluginsFull: plugins.getPlugins(true), path: countlyConfig.path || "", diff --git a/plugins/alerts/api/api.js b/plugins/alerts/api/api.js index 6bf378e155e..7f71891a4ff 100644 --- a/plugins/alerts/api/api.js +++ b/plugins/alerts/api/api.js @@ -10,6 +10,14 @@ const FEATURE_NAME = 'alerts'; const commonLib = require("./parts/common-lib.js"); const moment = require('moment-timezone'); +//the alert drawer reads concurrent_users.alert_interval to bound the interval it +//offers, falling back to a hardcoded 3 minutes when it cannot. The namespace belongs +//to an enterprise plugin, so it is declared here by the consumer that needs it rather +//than by its owner. +plugins.setReadableConfigs("concurrent_users", { + alert_interval: true +}); + /** * Alerts that can be triggered when an event is received. * see module file for details. diff --git a/plugins/pluginManager.js b/plugins/pluginManager.js index 5b44727de75..4113128adcd 100644 --- a/plugins/pluginManager.js +++ b/plugins/pluginManager.js @@ -26,6 +26,11 @@ var pluginDependencies = require('./pluginDependencies.js'), configextender = require('../api/configextender'); var pluginConfig = {}; +//stands in for a secret configuration value when configuration is read by someone +//who may know the value is set but not what it is. Deliberately not a plausible +//value, so it is obvious in a UI and in a bug report. +const SECRET_CONFIG_MASK = "********"; + /** * This module handles communicaton with plugins * @module "plugins/pluginManager" @@ -41,6 +46,12 @@ var pluginManager = function pluginManager() { var defaultConfigs = {}; var configsOnchanges = {}; var excludeFromUI = {plugins: true}; + //configuration values that hold credentials rather than settings, by namespace. + //see setSecretConfigs() + var secretConfigs = {}; + //configuration values a caller who is not a global admin may read, by namespace. + //Nothing is readable unless it appears here. See setReadableConfigs() + var readableConfigs = {}; var finishedSyncing = true; var expireList = []; var masking = {}; @@ -388,6 +399,163 @@ var pluginManager = function pluginManager() { } }; + /** + * Declare which configuration values a caller who is not a global admin may read. + * + * Nothing is readable unless declared here. That direction is deliberate: marking + * secrets instead would mean every new setting is exposed until somebody remembers + * to mark it, so one forgotten annotation is a leaked credential. Declaring what is + * needed fails the other way, and a missing entry costs a missing input in the UI, + * which someone notices and fixes. + * + * Declare the keys your own code reads. A plugin may declare a key from another + * namespace when it is the consumer: the reader knows what it needs, and the + * namespace owner may live in a different repository. + * + * This governs reads by other people. It has nothing to do with what the server + * itself can read, which is always the real configuration through getConfig(). + * + * @param {string} namespace - namespace of configuration, usually plugin name + * @param {object} conf - object whose keys are readable when the value is true + **/ + this.setReadableConfigs = function(namespace, conf) { + if (!readableConfigs[namespace]) { + readableConfigs[namespace] = {}; + } + for (let i in conf) { + readableConfigs[namespace][i] = conf[i]; + } + }; + + /** + * Whether a caller who is not a global admin may read a configuration value. + * @param {string} namespace - namespace of configuration + * @param {string} key - configuration key + * @returns {boolean} true when the value may be handed out + **/ + this.isReadableConfig = function(namespace, key) { + return !!(Object.prototype.hasOwnProperty.call(readableConfigs, namespace) + && readableConfigs[namespace][key] === true); + }; + + /** + * Copy of a full configuration object reduced to the values a caller who is not a + * global admin may read. A namespace with nothing declared is dropped entirely. + * + * The input is not modified. + * + * @param {object} confs - object of namespace -> configuration object + * @returns {object} copy containing only declared values + **/ + this.filterReadableConfigs = function(confs) { + var out = {}; + for (let namespace in confs) { + var conf = confs[namespace]; + if (!conf || typeof conf !== "object" || Array.isArray(conf)) { + continue; + } + for (let key in conf) { + if (this.isReadableConfig(namespace, key)) { + if (!out[namespace]) { + out[namespace] = {}; + } + out[namespace][key] = conf[key]; + } + } + } + return out; + }; + + /** + * Mark configuration values as secrets, so they are not handed out on read. + * + * Some configuration holds credentials rather than settings: a proxy password, an + * API key, the key report subscribe tokens are signed with. Those are readable by + * anyone who can read configuration at all, which is a lower bar than the + * operator who set them. Marking them here masks the value everywhere + * configuration is returned to a caller who is not a global admin, and keeps it + * out of the values exposed to the dashboard page. + * + * A value the browser needs is not a secret and must not be marked: once it is in + * page source, masking is theatre. tracking.self_tracking_app_key and + * recaptcha.site_key are deliberately not marked for that reason. + * + * getConfig() is unaffected, so server code keeps reading the real value. + * + * @param {string} namespace - namespace of configuration, usually plugin name + * @param {object} conf - object whose keys are secret when the value is true + **/ + this.setSecretConfigs = function(namespace, conf) { + if (!secretConfigs[namespace]) { + secretConfigs[namespace] = {}; + } + for (let i in conf) { + secretConfigs[namespace][i] = conf[i]; + } + }; + + /** + * Whether a configuration value is marked secret. + * @param {string} namespace - namespace of configuration + * @param {string} key - configuration key + * @returns {boolean} true when the value must not be handed out + **/ + this.isSecretConfig = function(namespace, key) { + return !!(secretConfigs[namespace] && secretConfigs[namespace][key] === true); + }; + + /** + * Copy of a full configuration object with every secret value replaced by a + * placeholder. Used for callers who may see that configuration exists but not what + * it is set to. + * + * The input is not modified, so the caller cannot accidentally hand a masked + * object to code that needs the real values. + * + * @param {object} confs - object of namespace -> configuration object + * @returns {object} copy with secret values masked + **/ + this.maskSecretConfigs = function(confs) { + var masked = {}; + for (let namespace in confs) { + var conf = confs[namespace]; + if (!conf || typeof conf !== "object" || Array.isArray(conf)) { + masked[namespace] = conf; + continue; + } + masked[namespace] = {}; + for (let key in conf) { + masked[namespace][key] = this.isSecretConfig(namespace, key) && conf[key] !== "" && conf[key] !== null && typeof conf[key] !== "undefined" + ? SECRET_CONFIG_MASK + : conf[key]; + } + } + return masked; + }; + + /** + * Copy of one namespace's configuration with secret keys removed entirely, for + * values that are serialized into the dashboard page. Removed rather than masked: + * the page has no use for a placeholder, and an absent key cannot be mistaken for + * a real one. + * + * @param {string} namespace - namespace of configuration + * @param {object} conf - configuration object for that namespace + * @returns {object} copy without the secret keys + **/ + this.omitSecretConfigs = function(namespace, conf) { + if (!conf || typeof conf !== "object" || Array.isArray(conf)) { + return conf; + } + var out = {}; + for (let key in conf) { + if (!this.isSecretConfig(namespace, key)) { + out[key] = conf[key]; + } + } + return out; + }; + /** * Get configuration from specific namespace and populate empty values with provided defaults * @param {string} namespace - namespace of configuration, usually plugin name diff --git a/plugins/plugins/api/api.js b/plugins/plugins/api/api.js index 02165726e8a..2be8344b2eb 100644 --- a/plugins/plugins/api/api.js +++ b/plugins/plugins/api/api.js @@ -282,6 +282,30 @@ var plugin = {}, plugins.loadConfigs(common.db, function() { var confs = plugins.getAllConfigs(); delete confs.services; + //A caller who is not a global admin gets only the values some part of + //the dashboard needs in order to work: the app management settings and + //a handful of display values. Nothing else, whether or not anybody + //remembered it was sensitive. + // + //Two mechanisms on purpose, in this order. The allow-list is the + //control: a value nobody declared is not returned, so a new setting is + //private by default and forgetting costs a missing input rather than a + //leak. Masking is the backstop: if a credential is ever declared + //readable by mistake, its value is still withheld. + if (!params.member.global_admin) { + var before = Object.keys(confs); + confs = plugins.maskSecretConfigs(plugins.filterReadableConfigs(confs)); + //a namespace disappearing here is how an undeclared value looks + //from the outside, and the UI degrades silently when it happens, so + //leave a trail rather than nothing + var dropped = before.filter(function(ns) { + return !confs[ns]; + }); + if (dropped.length) { + log.d("Withheld configuration namespaces from a non global admin" + + common.reqInfo(params) + ": " + dropped.join(", ")); + } + } common.returnOutput(params, confs); }); }); diff --git a/plugins/plugins/tests.js b/plugins/plugins/tests.js index 1ce26c98f7d..58f7abfc883 100644 --- a/plugins/plugins/tests.js +++ b/plugins/plugins/tests.js @@ -1,5 +1,6 @@ -/*global describe,it */ +/*global describe,it,before,after */ var request = require('supertest'); +var should = require('should'); var testUtils = require("../../test/testUtils"); request = request(testUtils.url); @@ -38,4 +39,278 @@ describe('Testing Plugins', function() { done(); }); }); -}); \ No newline at end of file +}); + +// Configuration read by someone who is not a global admin is reduced twice over. +// +// The allow-list is the control: setReadableConfigs declares what some part of the +// dashboard needs, and nothing else is returned. A setting added tomorrow is private +// until somebody declares it, so forgetting costs a missing input rather than a leak. +// +// Masking is the backstop: a value marked with setSecretConfigs is withheld even if it +// is declared readable by mistake. +// +// /o/configs is validateAppAdmin, so an app admin reaches it; /i/configs is +// validateGlobalAdmin, so only a global admin can write. That is why masking needs no +// write guard: the only callers who can write are the only callers who see real values. +describe('Testing configs read reduction', function() { + var MASK = "********"; + var API_KEY_ADMIN = ""; + var APP_ID = ""; + var memberApiKey = ""; + var memberUserId = ""; + var uniq = Date.now(); + + it('should give a global admin the full configuration', function(done) { + API_KEY_ADMIN = testUtils.get("API_KEY_ADMIN"); + APP_ID = testUtils.get("APP_ID"); + request + .get('/o/configs?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + // the signing key self-generates, so it is always set + ob.should.have.property("reports"); + ob.reports.secretKey.should.not.equal(MASK); + ob.reports.secretKey.length.should.be.above(0); + // and the namespaces a non-global admin will not get + ob.should.have.property("security"); + ob.security.should.have.property("proxy_password"); + done(); + }); + }); + + it('should create an app admin who is not a global admin', function(done) { + var permission = { + _: {a: [APP_ID], u: [[APP_ID]]}, + c: {}, + r: {}, + u: {}, + d: {} + }; + var userParams = { + full_name: "cfgadmin" + uniq, + username: "cfgadmin" + uniq, + password: "p4ssw0rD!", + email: "cfgadmin" + uniq + "@mail.test", + permission: permission + }; + request + .get('/i/users/create?api_key=' + API_KEY_ADMIN + '&args=' + encodeURIComponent(JSON.stringify(userParams))) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + memberApiKey = res.body.api_key; + memberUserId = res.body._id; + memberApiKey.should.not.equal(API_KEY_ADMIN); + done(); + }); + }); + + it('should withhold undeclared namespaces from an app admin entirely', function(done) { + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + // nothing in these is needed by the dashboard, so the whole namespace + // goes, credential or not + ob.should.not.have.property("reports"); + ob.should.not.have.property("push"); + ob.should.not.have.property("recaptcha"); + // no value anywhere in the response looks like the signing key + JSON.stringify(ob).should.not.match(/secretKey/); + done(); + }); + }); + + it('should withhold the proxy credentials while keeping the password policy', function(done) { + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property("security"); + ob.security.should.not.have.property("proxy_password"); + ob.security.should.not.have.property("proxy_username"); + // the dashboard validates a new password against these before sending it + ob.security.should.have.property("password_min"); + ob.security.password_min.should.not.equal(MASK); + done(); + }); + }); + + it('should still return every setting App Management renders', function(done) { + // App Management infers each input's widget from the type of the value here, so + // a key missing from the allow-list makes that setting disappear from the screen + // with no error. The list is read from the frontend rather than duplicated, so + // adding a key there without declaring it readable fails this test. + var fs = require('fs'); + var path = require('path'); + var viewsFile = path.resolve(__dirname, 'frontend/public/javascripts/countly.views.js'); + var src = fs.readFileSync(viewsFile, 'utf8'); + var block = /var showInAppManagment\s*=\s*\{\s*"api"\s*:\s*\{([\s\S]*?)\}/.exec(src); + should.exist(block); + var wanted = (block[1].match(/"([a-z_]+)"\s*:\s*true/g) || []).map(function(m) { + return /"([a-z_]+)"/.exec(m)[1]; + }); + wanted.length.should.be.above(0); + + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + ob.should.have.property("api"); + wanted.forEach(function(key) { + ob.api.should.have.property(key); + }); + done(); + }); + }); + + // Happy paths. The reduction is only correct if everything that legitimately reads + // configuration still works, so these assert the values actually arrive, with their + // real contents, rather than merely that secrets are gone. + + it('should give the app admin real values, not placeholders, for what it may read', function(done) { + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + // types matter: App Management infers each input's widget from them, so a + // masked or missing value would render the wrong control + ob.api.event_limit.should.be.a.Number(); + ob.api.session_duration_limit.should.be.a.Number(); + ob.api.country_data.should.be.a.Boolean(); + ob.api.safe.should.be.a.Boolean(); + ob.security.password_min.should.be.a.Number(); + JSON.stringify(ob).should.not.match(/\*{8}/); + done(); + }); + }); + + it('should agree with the global admin on every value the app admin can read', function(done) { + // the app settings screen shows how an app differs from the server default, which + // is only meaningful if the defaults it sees are the real ones + request + .get('/o/configs?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, adminRes) { + if (err) { + return done(err); + } + var full = JSON.parse(adminRes.text); + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err2, memberRes) { + if (err2) { + return done(err2); + } + var reduced = JSON.parse(memberRes.text); + Object.keys(reduced).forEach(function(ns) { + Object.keys(reduced[ns]).forEach(function(key) { + JSON.stringify(reduced[ns][key]) + .should.equal(JSON.stringify(full[ns][key])); + }); + }); + done(); + }); + }); + }); + + it('should give the app admin a plugin namespace it declared, when that plugin is on', function(done) { + // feedback belongs to star-rating. getAllConfigs omits a namespace whose plugin + // is not enabled, so in a test run without star-rating there is nothing to + // reduce and nothing to assert. Asserting the namespace exists would be + // asserting which plugins the run enabled, which is not what this covers. + request + .get('/o/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var ob = JSON.parse(res.text); + if (!ob.feedback) { + return done(); + } + // present, so the declared keys must have arrived with real values + ob.feedback.should.have.property("main_color"); + ob.feedback.should.have.property("font_color"); + ob.feedback.main_color.length.should.be.above(0); + ob.feedback.main_color.should.not.equal(MASK); + done(); + }); + }); + + it('should let a global admin still write configuration', function(done) { + // masking needs no write guard only because writing stays global-admin only, so + // that has to keep working. Writes the value back unchanged, to avoid leaving the + // test environment altered. + request + .get('/o/configs?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID) + .expect(200) + .end(function(err, res) { + if (err) { + return done(err); + } + var current = JSON.parse(res.text).api.event_limit; + var payload = {api: {event_limit: current}}; + request + .get('/i/configs?api_key=' + API_KEY_ADMIN + '&app_id=' + APP_ID + '&configs=' + encodeURIComponent(JSON.stringify(payload))) + .expect(200) + .end(function(err2, res2) { + if (err2) { + return done(err2); + } + var after = JSON.parse(res2.text); + after.api.event_limit.should.equal(current); + done(); + }); + }); + }); + + it('should not let the app admin write configuration', function(done) { + // the reason a mask can never be saved back over a real value + request + .get('/i/configs?api_key=' + memberApiKey + '&app_id=' + APP_ID + '&configs=' + encodeURIComponent(JSON.stringify({api: {event_limit: 1}}))) + .expect(401) + .end(function(err) { + if (err) { + return done(err); + } + done(); + }); + }); + + after(function(done) { + if (!memberUserId) { + return done(); + } + request + .get('/i/users/delete?api_key=' + API_KEY_ADMIN + '&args=' + encodeURIComponent(JSON.stringify({user_ids: [memberUserId]}))) + .end(function() { + done(); + }); + }); +}); diff --git a/plugins/push/api/api.js b/plugins/push/api/api.js index 7ebc47aaa6e..7674343cd89 100644 --- a/plugins/push/api/api.js +++ b/plugins/push/api/api.js @@ -70,6 +70,12 @@ plugins.setConfigs(FEATURE_NAME, { default_content_available: false, // sets content-available: 1 by default for ios }); +//outbound proxy credentials for sending pushes +plugins.setSecretConfigs(FEATURE_NAME, { + proxyuser: true, + proxypass: true +}); + plugins.internalEvents.push('[CLY]_push_sent'); plugins.internalEvents.push('[CLY]_push_action'); plugins.internalDrillEvents.push('[CLY]_push_action'); diff --git a/plugins/recaptcha/frontend/app.js b/plugins/recaptcha/frontend/app.js index ec32327a47c..0637ea95cb8 100644 --- a/plugins/recaptcha/frontend/app.js +++ b/plugins/recaptcha/frontend/app.js @@ -9,6 +9,12 @@ plugins.setConfigs("recaptcha", { secret_key: "" }); +//only secret_key. site_key is public by design: it goes to the browser to render the +//challenge, so marking it secret would achieve nothing. +plugins.setSecretConfigs("recaptcha", { + secret_key: true +}); + (function(plugin) { plugin.init = function(app, countlyDb) { plugins.loadConfigs(countlyDb, function() { diff --git a/plugins/reports/api/reports.js b/plugins/reports/api/reports.js index 1cc57ae2f0c..a439e5eec44 100644 --- a/plugins/reports/api/reports.js +++ b/plugins/reports/api/reports.js @@ -24,6 +24,13 @@ plugins.setConfigs("reports", { secretKey: countlyApiConfig?.encryption?.reports_key || "Ydqa7Omkd3yhV33M3iWV1oFcOEk898h9", }); +//this key signs the subscribe/unsubscribe tokens, and /subscribe_report and +///unsubscribe_report accept those without authentication. It self-generates, so +//unlike the other secrets it is always set on every install. +plugins.setSecretConfigs("reports", { + secretKey: true +}); + versionInfo.page = (!versionInfo.title) ? "https://count.ly" : null; versionInfo.title = versionInfo.title || "Countly"; var metrics = { diff --git a/plugins/star-rating/api/api.js b/plugins/star-rating/api/api.js index 4ca7d12cc21..6a9f18b6410 100644 --- a/plugins/star-rating/api/api.js +++ b/plugins/star-rating/api/api.js @@ -28,6 +28,14 @@ if (!surveysEnabled) { feedback_logo: "" }); + + //the ratings and surveys widget previews style themselves from these when the app + //has no per-app override, so a non global admin needs to read them + plugins.setReadableConfigs("feedback", { + main_color: true, + font_color: true, + feedback_logo: true + }); } const FEATURE_NAME = 'star_rating'; diff --git a/test/unit-tests/plugins.pluginManager.secret-configs.js b/test/unit-tests/plugins.pluginManager.secret-configs.js new file mode 100644 index 00000000000..6192973ea92 --- /dev/null +++ b/test/unit-tests/plugins.pluginManager.secret-configs.js @@ -0,0 +1,184 @@ +require("should"); +var plugins = require("../../plugins/pluginManager.js"); + +// Configuration values that hold credentials rather than settings are marked with +// setSecretConfigs(). Marking one masks it wherever configuration is returned to a +// caller who is not a global admin, and drops it from the values serialized into the +// dashboard page. +// +// The two invariants that matter: +// - getConfig() is unaffected, or server code would start authenticating with the +// placeholder instead of the password; +// - the input object is never mutated, or masking one response would corrupt the +// live configuration for everything else. +describe("pluginManager secret configs", function() { + before(function() { + plugins.setConfigs("unittest_secret", { + limit: 10, + enabled: true, + proxy_password: "REAL_PASSWORD", + api_key: "REAL_KEY", + site_key: "public-site-key", + unset_secret: "" + }); + plugins.setSecretConfigs("unittest_secret", { + proxy_password: true, + api_key: true, + unset_secret: true + }); + }); + + describe("isSecretConfig", function() { + it("is true only for marked keys", function() { + plugins.isSecretConfig("unittest_secret", "proxy_password").should.equal(true); + plugins.isSecretConfig("unittest_secret", "api_key").should.equal(true); + }); + it("is false for unmarked keys in the same namespace", function() { + plugins.isSecretConfig("unittest_secret", "limit").should.equal(false); + plugins.isSecretConfig("unittest_secret", "site_key").should.equal(false); + }); + it("is false for an unknown namespace", function() { + plugins.isSecretConfig("unittest_no_such_namespace", "proxy_password").should.equal(false); + }); + it("is false for inherited Object.prototype keys", function() { + plugins.isSecretConfig("unittest_secret", "constructor").should.equal(false); + plugins.isSecretConfig("unittest_secret", "toString").should.equal(false); + plugins.isSecretConfig("constructor", "constructor").should.equal(false); + }); + }); + + describe("getConfig", function() { + it("still returns the real value, so server code keeps working", function() { + var conf = plugins.getConfig("unittest_secret"); + conf.proxy_password.should.equal("REAL_PASSWORD"); + conf.api_key.should.equal("REAL_KEY"); + }); + }); + + describe("maskSecretConfigs", function() { + it("masks marked values and leaves the rest alone", function() { + var masked = plugins.maskSecretConfigs({unittest_secret: plugins.getConfig("unittest_secret")}); + masked.unittest_secret.proxy_password.should.not.equal("REAL_PASSWORD"); + masked.unittest_secret.api_key.should.not.equal("REAL_KEY"); + masked.unittest_secret.limit.should.equal(10); + masked.unittest_secret.enabled.should.equal(true); + masked.unittest_secret.site_key.should.equal("public-site-key"); + }); + it("does not modify the object it was given", function() { + var input = {unittest_secret: plugins.getConfig("unittest_secret")}; + plugins.maskSecretConfigs(input); + input.unittest_secret.proxy_password.should.equal("REAL_PASSWORD"); + }); + it("leaves an unset secret empty, so 'not configured' stays visible", function() { + var masked = plugins.maskSecretConfigs({unittest_secret: plugins.getConfig("unittest_secret")}); + masked.unittest_secret.unset_secret.should.equal(""); + }); + it("leaves namespaces it does not know untouched", function() { + var masked = plugins.maskSecretConfigs({other: {password: "not marked"}}); + masked.other.password.should.equal("not marked"); + }); + it("tolerates a non-object namespace value", function() { + var masked = plugins.maskSecretConfigs({a: null, b: "str", c: [1, 2]}); + (masked.a === null).should.equal(true); + masked.b.should.equal("str"); + masked.c.should.eql([1, 2]); + }); + it("masks every marked key, not just the first", function() { + var masked = plugins.maskSecretConfigs({unittest_secret: plugins.getConfig("unittest_secret")}); + masked.unittest_secret.proxy_password.should.equal(masked.unittest_secret.api_key); + }); + }); + + describe("omitSecretConfigs", function() { + it("removes marked keys entirely rather than masking them", function() { + var exposed = plugins.omitSecretConfigs("unittest_secret", plugins.getConfig("unittest_secret")); + exposed.should.not.have.property("proxy_password"); + exposed.should.not.have.property("api_key"); + exposed.should.have.property("limit", 10); + exposed.should.have.property("site_key", "public-site-key"); + }); + it("does not modify the object it was given", function() { + var conf = plugins.getConfig("unittest_secret"); + plugins.omitSecretConfigs("unittest_secret", conf); + conf.proxy_password.should.equal("REAL_PASSWORD"); + }); + it("returns a namespace with no secrets unchanged in content", function() { + var exposed = plugins.omitSecretConfigs("unittest_no_such_namespace", {a: 1, b: 2}); + exposed.should.eql({a: 1, b: 2}); + }); + it("tolerates a missing config object", function() { + (plugins.omitSecretConfigs("unittest_secret", null) === null).should.equal(true); + (typeof plugins.omitSecretConfigs("unittest_secret", undefined)).should.equal("undefined"); + }); + }); + + describe("setReadableConfigs / filterReadableConfigs", function() { + before(function() { + plugins.setConfigs("unittest_readable", { + needed: 1, + also_needed: "yes", + not_needed: "internal", + brand_new_api_key: "NOBODY_MARKED_THIS" + }); + plugins.setReadableConfigs("unittest_readable", { + needed: true, + also_needed: true + }); + }); + + it("returns declared values", function() { + var out = plugins.filterReadableConfigs({unittest_readable: plugins.getConfig("unittest_readable")}); + out.unittest_readable.should.have.property("needed", 1); + out.unittest_readable.should.have.property("also_needed", "yes"); + }); + it("withholds anything not declared", function() { + var out = plugins.filterReadableConfigs({unittest_readable: plugins.getConfig("unittest_readable")}); + out.unittest_readable.should.not.have.property("not_needed"); + }); + it("withholds a newly added credential nobody marked secret", function() { + // the point of the allow-list. Marking secrets is the other direction and + // leaks until someone remembers; this leaks only if someone opts in. + var out = plugins.filterReadableConfigs({unittest_readable: plugins.getConfig("unittest_readable")}); + out.unittest_readable.should.not.have.property("brand_new_api_key"); + }); + it("drops a namespace with nothing declared, rather than returning it empty", function() { + var out = plugins.filterReadableConfigs({unittest_undeclared_ns: {a: 1, b: 2}}); + out.should.not.have.property("unittest_undeclared_ns"); + }); + it("does not modify the object it was given", function() { + var input = {unittest_readable: plugins.getConfig("unittest_readable")}; + plugins.filterReadableConfigs(input); + input.unittest_readable.should.have.property("not_needed", "internal"); + }); + it("is false for inherited Object.prototype keys", function() { + plugins.isReadableConfig("unittest_readable", "constructor").should.equal(false); + plugins.isReadableConfig("constructor", "needed").should.equal(false); + }); + it("tolerates non-object namespace values", function() { + var out = plugins.filterReadableConfigs({a: null, b: "str", c: [1]}); + Object.keys(out).length.should.equal(0); + }); + }); + + describe("the two mechanisms together", function() { + before(function() { + plugins.setConfigs("unittest_both", {shown: 1, credential: "REAL_VALUE"}); + plugins.setSecretConfigs("unittest_both", {credential: true}); + }); + + it("masks a credential even when it is wrongly declared readable", function() { + // the allow-list is the control and the mask is the backstop, so declaring a + // credential readable by mistake still does not hand out its value + plugins.setReadableConfigs("unittest_both", {shown: true, credential: true}); + var out = plugins.maskSecretConfigs(plugins.filterReadableConfigs({unittest_both: plugins.getConfig("unittest_both")})); + out.unittest_both.should.have.property("shown", 1); + out.unittest_both.credential.should.not.equal("REAL_VALUE"); + }); + }); + + // Which keys this codebase actually declares is deliberately not asserted here: a + // unit test requiring pluginManager directly never loads api/api.js or the plugin + // modules that declare them, so every such assertion would pass or fail for the + // wrong reason. That belongs in plugins/plugins/tests.js, which runs against a + // started server with the declarations in place. +});