diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 41a650a5dc..a20921bb03 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -37,6 +37,7 @@ jobs: with: node-version: ${{ matrix.node-version }} - run: yarn + - run: yarn lint - run: yarn run test:coverage env: CI: true diff --git a/lib/addons/addon-schema.js b/lib/addons/addon-schema.js index bfbe57147d..ea92147d7c 100644 --- a/lib/addons/addon-schema.js +++ b/lib/addons/addon-schema.js @@ -18,6 +18,7 @@ const addonDefinitionSchema = joi.object().keys({ description: joi.string(), placeholder: joi.string().allow(''), required: joi.boolean().default(false), + sensitive: joi.boolean().default(false), }), ), events: joi diff --git a/lib/addons/jira-comment-definition.js b/lib/addons/jira-comment-definition.js index 152bb890e3..5844a79543 100644 --- a/lib/addons/jira-comment-definition.js +++ b/lib/addons/jira-comment-definition.js @@ -24,6 +24,7 @@ module.exports = { 'Add a new key at https://id.atlassian.com/manage-profile/security/api-tokens when logged in as the user you want Unleash to use', type: 'text', required: true, + sensitive: true, }, { name: 'user', diff --git a/lib/addons/slack-definition.js b/lib/addons/slack-definition.js index b09b77c409..9a81a4bdd5 100644 --- a/lib/addons/slack-definition.js +++ b/lib/addons/slack-definition.js @@ -18,6 +18,7 @@ module.exports = { displayName: 'Slack webhook URL', type: 'url', required: true, + sensitive: true, }, { name: 'username', diff --git a/lib/addons/webhook-definition.js b/lib/addons/webhook-definition.js index 544095b9c3..3bbc11e463 100644 --- a/lib/addons/webhook-definition.js +++ b/lib/addons/webhook-definition.js @@ -19,6 +19,7 @@ module.exports = { '(Required) Unleash will perform a HTTP Post to the specified URL (one retry if first attempt fails)', type: 'url', required: true, + sensitive: true, }, { name: 'contentType', diff --git a/lib/services/addon-service.js b/lib/services/addon-service.js index a02a94122b..c01decdfc0 100644 --- a/lib/services/addon-service.js +++ b/lib/services/addon-service.js @@ -10,6 +10,8 @@ const SUPPORTED_EVENTS = Object.keys(events).map(k => events[k]); const ADDONS_CACHE_TIME = 60 * 1000; // 60s +const MASKED_VALUE = '*****'; + class AddonService { constructor( { addonStore, eventStore, featureToggleStore }, @@ -23,7 +25,21 @@ class AddonService { this.logger = getLogger('services/addon-service.js'); this.tagTypeService = tagTypeService; - this.addonProviders = addonProvidersClasses.reduce((map, Provider) => { + this.addonProviders = this.loadProviders(getLogger); + this.sensitiveParams = this.loadSensitiveParams(this.addonProviders); + if (addonStore) { + this.registerEventHandler(); + } + + // Memoized private function + this.fetchAddonConfigs = memoize( + () => addonStore.getAll({ enabled: true }), + { promise: true, maxAge: ADDONS_CACHE_TIME }, + ); + } + + loadProviders(getLogger) { + return addonProvidersClasses.reduce((map, Provider) => { try { const provider = new Provider({ getLogger }); // eslint-disable-next-line no-param-reassign @@ -33,15 +49,21 @@ class AddonService { } return map; }, {}); - if (addonStore) { - this.registerEventHandler(); - } + } - // Memoized function - this.fetchAddonConfigs = memoize( - () => addonStore.getAll({ enabled: true }), - { promise: true, maxAge: ADDONS_CACHE_TIME }, + loadSensitiveParams(addonProviders) { + const providerDefinitions = Object.values(addonProviders).map( + p => p.definition, ); + return providerDefinitions.reduce((obj, definition) => { + const sensitiveParams = definition.parameters + .filter(p => p.sensitive) + .map(p => p.name); + + const o = { ...obj }; + o[definition.name] = sensitiveParams; + return o; + }, {}); } registerEventHandler() { @@ -67,12 +89,31 @@ class AddonService { }; } + // Should be used by the controller. async getAddons() { - return this.addonStore.getAll(); + const addonConfigs = await this.addonStore.getAll(); + return addonConfigs.map(a => this.filterSensitiveFields(a)); + } + + filterSensitiveFields(addonConfig) { + const { sensitiveParams } = this; + const a = { ...addonConfig }; + a.parameters = Object.keys(a.parameters).reduce((obj, paramKey) => { + const o = { ...obj }; + if (sensitiveParams[a.provider].includes(paramKey)) { + o[paramKey] = MASKED_VALUE; + } else { + o[paramKey] = a.parameters[paramKey]; + } + + return o; + }, {}); + return a; } async getAddon(id) { - return this.addonStore.get(id); + const addonConfig = await this.addonStore.get(id); + return this.filterSensitiveFields(addonConfig); } getProviderDefinition() { @@ -122,6 +163,21 @@ class AddonService { async updateAddon(id, data, userName) { const addonConfig = await addonSchema.validateAsync(data); + if (this.sensitiveParams[addonConfig.provider].length > 0) { + const existingConfig = await this.addonStore.get(id); + addonConfig.parameters = Object.keys(addonConfig.parameters).reduce( + (params, key) => { + const o = { ...params }; + if (addonConfig.parameters[key] === MASKED_VALUE) { + o[key] = existingConfig.parameters[key]; + } else { + o[key] = addonConfig.parameters[key]; + } + return o; + }, + {}, + ); + } await this.addonStore.update(id, addonConfig); await this.eventStore.store({ type: events.ADDON_CONFIG_UPDATED, diff --git a/lib/services/addon-service.test.js b/lib/services/addon-service.test.js index e52e9347e5..f9750ad360 100644 --- a/lib/services/addon-service.test.js +++ b/lib/services/addon-service.test.js @@ -18,6 +18,8 @@ const { ADDON_CONFIG_DELETED, } = require('../event-type'); +const MASKED_VALUE = '*****'; + const definition = { name: 'simple', displayName: 'Simple ADdon', @@ -36,6 +38,14 @@ const definition = { type: 'text', required: false, }, + { + name: 'sensitiveParam', + displayName: 'Some sensitive param', + description: 'Some variable to inject', + type: 'text', + required: false, + sensitive: true, + }, ], events: [ FEATURE_CREATED, @@ -259,3 +269,53 @@ test('should store ADDON_CONFIG_REMOVE event', async t => { t.is(events[2].type, ADDON_CONFIG_DELETED); t.is(events[2].data.id, addonConfig.id); }); + +test('should hide sensitive fields when fetching', async t => { + const { addonService } = getSetup(); + + const config = { + provider: 'simple', + enabled: true, + parameters: { + url: 'http://localhost/wh', + var: 'some-value', + sensitiveParam: 'should be hidden when fetching', + }, + events: [FEATURE_CREATED], + }; + + const createdConfig = await addonService.createAddon(config, 'me@mail.com'); + const addons = await addonService.getAddons(); + const addonRetrieved = await addonService.getAddon(createdConfig.id); + + t.is(addons.length, 1); + t.is(addons[0].parameters.sensitiveParam, MASKED_VALUE); + t.is(addonRetrieved.parameters.sensitiveParam, MASKED_VALUE); +}); + +test('should not overwrite masked values when updating', async t => { + const { addonService, stores } = getSetup(); + + const config = { + provider: 'simple', + enabled: true, + parameters: { + url: 'http://localhost/wh', + var: 'some-value', + }, + events: [FEATURE_CREATED], + }; + + const addonConfig = await addonService.createAddon(config, 'me@mail.com'); + + const updated = { + ...addonConfig, + parameters: { url: MASKED_VALUE, var: 'some-new-value' }, + description: 'test', + }; + await addonService.updateAddon(addonConfig.id, updated, 'me@mail.com'); + + const updatedConfig = await stores.addonStore.get(addonConfig.id); + t.is(updatedConfig.parameters.url, 'http://localhost/wh'); + t.is(updatedConfig.parameters.var, 'some-new-value'); +}); diff --git a/package.json b/package.json index b21f1617d8..21188a9fe4 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,6 @@ "start:dev": "NODE_ENV=development supervisor --ignore ./node_modules/,website server-dev.js", "db-migrate": "db-migrate", "lint": "eslint .", - "pretest": "yarn run lint", "test": "NODE_ENV=test PORT=4243 ava", "test:docker": "./scripts/docker-postgres.sh", "test:watch": "yarn test --watch", diff --git a/test/e2e/api/admin/addon.e2e.test.js b/test/e2e/api/admin/addon.e2e.test.js index 2fe21f2f66..3f0c7f7dc3 100644 --- a/test/e2e/api/admin/addon.e2e.test.js +++ b/test/e2e/api/admin/addon.e2e.test.js @@ -6,6 +6,8 @@ const dbInit = require('../../helpers/database-init'); const { setupApp } = require('../../helpers/test-helper'); const getLogger = require('../../../fixtures/no-logger'); +const MASKED_VALUE = '*****'; + let stores; test.before(async () => { @@ -84,7 +86,7 @@ test.serial('should delete addon configuration', async t => { }); test.serial('should update addon configuration', async t => { - t.plan(1); + t.plan(2); const request = await setupApp(stores); const config = { @@ -122,7 +124,11 @@ test.serial('should update addon configuration', async t => { .send(config) .expect(200) .expect(r => { - t.is(r.body.parameters.url, updatedConfig.parameters.url); + t.is(r.body.parameters.url, MASKED_VALUE); + t.is( + r.body.parameters.bodyTemplate, + updatedConfig.parameters.bodyTemplate, + ); }); }); @@ -145,7 +151,7 @@ test.serial('should not update with invalid addon configuration', async t => { .expect(400); }); -test.serial('should not update unknwn addon configuration', async t => { +test.serial('should not update unknown addon configuration', async t => { t.plan(0); const request = await setupApp(stores); @@ -166,7 +172,7 @@ test.serial('should not update unknwn addon configuration', async t => { }); test.serial('should get addon configuration', async t => { - t.plan(1); + t.plan(3); const request = await setupApp(stores); const config = { @@ -191,10 +197,15 @@ test.serial('should get addon configuration', async t => { .expect(200) .expect(r => { t.is(r.body.provider, config.provider); + t.is( + r.body.parameters.bodyTemplate, + config.parameters.bodyTemplate, + ); + t.is(r.body.parameters.url, MASKED_VALUE); }); }); -test.serial('should not get unkown addon configuration', async t => { +test.serial('should not get unknown addon configuration', async t => { t.plan(0); const request = await setupApp(stores); diff --git a/test/fixtures/fake-addon-store.js b/test/fixtures/fake-addon-store.js index ff0a801447..fa8f6d2c04 100644 --- a/test/fixtures/fake-addon-store.js +++ b/test/fixtures/fake-addon-store.js @@ -18,7 +18,7 @@ module.exports = () => { Promise.resolve(); }, get: async id => { - return _addons.find(id); + return _addons[id]; }, getAll: () => Promise.resolve(_addons), };