From 39f183f04199dc3a93596519047fd1dec8b638aa Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Wed, 2 Jun 2021 06:40:35 +0000 Subject: [PATCH] revise based on PR comments Signed-off-by: Anan Zhuang --- config/opensearch_dashboards.yml | 39 ++++++++++-- src/plugins/vis_type_timeline/config.ts | 4 +- src/plugins/vis_type_timeline/server/index.ts | 1 + .../server/lib/config_manager.ts | 14 ++--- .../vis_type_timeline/server/plugin.ts | 5 +- .../vis_type_timeline/server/routes/run.ts | 2 +- .../series_functions/fixtures/tl_config.js | 6 +- .../server/series_functions/graphite.js | 37 +++++------ .../server/series_functions/graphite.test.js | 62 ++++++++++++------- 9 files changed, 110 insertions(+), 60 deletions(-) diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml index 751b79469758..27f84aac3c4a 100644 --- a/config/opensearch_dashboards.yml +++ b/config/opensearch_dashboards.yml @@ -106,8 +106,37 @@ # Supported languages are the following: English - en , by default , Chinese - zh-CN . #i18n.locale: "en" -# Set the allowlist to check input graphite url. -vis_type_timeline.graphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'] - -# Set the blocklist to check input graphite url. -#vis_type_timeline.blocklist: [] +# Set the allowlist to check input graphite Url. Allowlist is the default check list. +vis_type_timeline.graphiteAllowedUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'] + +# Set the blocklist to check input graphite Url/IP. Blocklist is an IP list. +# Below is an example for reference +# vis_type_timeline.graphiteBlockedIPs: [ +# //Loopback +# '127.0.0.0/8', +# '::1/128', +# // Link-local Address for IPv6 +# 'fe80::/10', +# //Private IP address for IPv4 +# '10.0.0.0/8', +# '172.16.0.0/12', +# '192.168.0.0/16', +# //Unique local address (ULA) +# 'fc00::/7', +# //Reserved IP address +# '0.0.0.0/8', +# '100.64.0.0/10', +# '192.0.0.0/24', +# '192.0.2.0/24', +# '198.18.0.0/15', +# '192.88.99.0/24', +# '198.51.100.0/24', +# '203.0.113.0/24', +# '224.0.0.0/4', +# '240.0.0.0/4', +# '255.255.255.255/32', +# '::/128', +# '2001:db8::/32', +# 'ff00::/8', +# ] +#vis_type_timeline.graphiteBlockedIPs: [] diff --git a/src/plugins/vis_type_timeline/config.ts b/src/plugins/vis_type_timeline/config.ts index 608524010888..71765da00699 100644 --- a/src/plugins/vis_type_timeline/config.ts +++ b/src/plugins/vis_type_timeline/config.ts @@ -36,8 +36,8 @@ export const configSchema = schema.object( { enabled: schema.boolean({ defaultValue: true }), ui: schema.object({ enabled: schema.boolean({ defaultValue: false }) }), - graphiteUrls: schema.maybe(schema.arrayOf(schema.string())), - blocklist: schema.maybe(schema.arrayOf(schema.string())), + graphiteAllowedUrls: schema.maybe(schema.arrayOf(schema.string())), + graphiteBlockedIPs: schema.maybe(schema.arrayOf(schema.string())), }, // This option should be removed as soon as we entirely migrate config from legacy Timeline plugin. { unknowns: 'allow' } diff --git a/src/plugins/vis_type_timeline/server/index.ts b/src/plugins/vis_type_timeline/server/index.ts index a843ceccee97..a0e8ec745b15 100644 --- a/src/plugins/vis_type_timeline/server/index.ts +++ b/src/plugins/vis_type_timeline/server/index.ts @@ -45,6 +45,7 @@ export const config: PluginConfigDescriptor = { renameFromRoot('timeline_vis.enabled', 'vis_type_timeline.enabled'), renameFromRoot('timeline.enabled', 'vis_type_timeline.enabled'), renameFromRoot('timeline.graphiteUrls', 'vis_type_timeline.graphiteUrls'), + renameFromRoot('vis_type_timeline.graphiteUrls', 'vis_type_timeline.graphiteAllowedUrls'), renameFromRoot('timeline.ui.enabled', 'vis_type_timeline.ui.enabled', true), ], }; diff --git a/src/plugins/vis_type_timeline/server/lib/config_manager.ts b/src/plugins/vis_type_timeline/server/lib/config_manager.ts index 79926df0c88f..2f73e1afc20e 100644 --- a/src/plugins/vis_type_timeline/server/lib/config_manager.ts +++ b/src/plugins/vis_type_timeline/server/lib/config_manager.ts @@ -36,16 +36,16 @@ import { configSchema } from '../../config'; export class ConfigManager { private opensearchShardTimeout: number = 0; - private graphiteUrls: string[] = []; - private blockedUrls: string[] = []; + private graphiteAllowedUrls: string[] = []; + private graphiteBlockedIPs: string[] = []; constructor(config: PluginInitializerContext['config']) { config.create>().subscribe((configUpdate) => { - this.graphiteUrls = configUpdate.graphiteUrls || []; + this.graphiteAllowedUrls = configUpdate.graphiteAllowedUrls || []; }); config.create>().subscribe((configUpdate) => { - this.blockedUrls = configUpdate.blocklist || []; + this.graphiteBlockedIPs = configUpdate.graphiteBlockedIPs || []; }); config.legacy.globalConfig$.subscribe((configUpdate) => { @@ -58,10 +58,10 @@ export class ConfigManager { } getGraphiteUrls() { - return this.graphiteUrls; + return this.graphiteAllowedUrls; } - getBlockedUrls() { - return this.blockedUrls; + getGraphiteBlockedIPs() { + return this.graphiteBlockedIPs; } } diff --git a/src/plugins/vis_type_timeline/server/plugin.ts b/src/plugins/vis_type_timeline/server/plugin.ts index b12dd8f2e13b..12842e50184d 100644 --- a/src/plugins/vis_type_timeline/server/plugin.ts +++ b/src/plugins/vis_type_timeline/server/plugin.ts @@ -171,7 +171,10 @@ export class Plugin { description: 'The URL should be in the form of https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite', }), - value: config.graphiteUrls && config.graphiteUrls.length ? config.graphiteUrls[0] : null, + value: + config.graphiteAllowedUrls && config.graphiteAllowedUrls.length + ? config.graphiteAllowedUrls[0] + : null, description: i18n.translate('timeline.uiSettings.graphiteURLDescription', { defaultMessage: '{experimentalLabel} The URL of your graphite host', values: { experimentalLabel: `[${experimentalLabel}]` }, diff --git a/src/plugins/vis_type_timeline/server/routes/run.ts b/src/plugins/vis_type_timeline/server/routes/run.ts index 873264bb6242..6229e689c006 100644 --- a/src/plugins/vis_type_timeline/server/routes/run.ts +++ b/src/plugins/vis_type_timeline/server/routes/run.ts @@ -102,7 +102,7 @@ export function runRoute( getFunction, getStartServices: core.getStartServices, allowedGraphiteUrls: configManager.getGraphiteUrls(), - blockedGraphiteUrls: configManager.getBlockedUrls(), + blockedGraphiteIPs: configManager.getGraphiteBlockedIPs(), opensearchShardTimeout: configManager.getOpenSearchShardTimeout(), savedObjectsClient: context.core.savedObjects.client, }); diff --git a/src/plugins/vis_type_timeline/server/series_functions/fixtures/tl_config.js b/src/plugins/vis_type_timeline/server/series_functions/fixtures/tl_config.js index 2adf06da66aa..611cddc8f347 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/fixtures/tl_config.js +++ b/src/plugins/vis_type_timeline/server/series_functions/fixtures/tl_config.js @@ -56,7 +56,7 @@ export default function () { opensearchShardTimeout: moment.duration(30000), allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], - blockedGraphiteUrls: [], + blockedGraphiteIPs: [], }); tlConfig.time = { @@ -68,6 +68,10 @@ export default function () { tlConfig.settings = timelineDefaults(); + tlConfig.allowedGraphiteUrls = timelineDefaults(); + + tlConfig.blockedGraphiteIPs = timelineDefaults(); + tlConfig.setTargetSeries(); return tlConfig; diff --git a/src/plugins/vis_type_timeline/server/series_functions/graphite.js b/src/plugins/vis_type_timeline/server/series_functions/graphite.js index 3b18640c0d34..9f020502b29f 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/graphite.js +++ b/src/plugins/vis_type_timeline/server/series_functions/graphite.js @@ -41,7 +41,7 @@ import IPCIDR from 'ip-cidr'; const MISS_CHECKLIST_MESSAGE = `Please configure on the opensearch_dashbpards.yml file. You can always enable the default allowlist configuration.`; -const INVALID_URL_MESSAGE = `The Graphite URL provided by you is invalid. +const INVALID_URL_MESSAGE = `The Graphite URL/IP provided by you is invalid. Please update your config from OpenSearch Dashboards's Advanced Setting.`; /** @@ -65,7 +65,7 @@ function getIpAddress(urlObject) { return null; } -function isValidURL(configuredUrl, blockedUrls) { +function isValidURL(configuredUrl, blockedIPs) { // Check the format of URL, URL has be in the format as // scheme://server/path/resource otherwise an TypeError // would be thrown @@ -85,32 +85,27 @@ function isValidURL(configuredUrl, blockedUrls) { // range of an IP address block // @param {string} bl // @returns {object} cidr - for (const bl of blockedUrls) { - const cidr = new IPCIDR(bl); - if (cidr.contains(ip)) { - return false; - } - } - return true; + const isBlocked = blockedIPs.some((blockedIP) => new IPCIDR(blockedIP).contains(ip)); + return !isBlocked; } /** * Check configured url using blocklist and allowlist * If allowlist is used, return false if allowlist does not contain configured url * If blocklist is used, return false if blocklist contains configured url - * If both allowlist and blocklist are used, return false if allowlist does not contain or if blocklist contains configured url - * @param {Array|string} blockedUrls + * If both allowlist and blocklist are used, check blocklist first then allowlist + * @param {Array|string} blockedIPs * @param {Array|string} allowedUrls * @param {string} configuredUrls * @returns {boolean} true if the configuredUrl is valid */ -function checkConfigUrls(blockedUrls, allowedUrls, configuredUrl) { - if (blockedUrls.length === 0) { +function isValidConfig(blockedIPs, allowedUrls, configuredUrl) { + if (blockedIPs.length === 0) { if (!allowedUrls.includes(configuredUrl)) return false; } else if (allowedUrls.length === 0) { - if (!isValidURL(configuredUrl, blockedUrls)) return false; + if (!isValidURL(configuredUrl, blockedIPs)) return false; } else { - if (!allowedUrls.includes(configuredUrl) || !isValidURL(configuredUrl, blockedUrls)) + if (!isValidURL(configuredUrl, blockedIPs) || !allowedUrls.includes(configuredUrl)) return false; } return true; @@ -139,20 +134,22 @@ export default new Datasource('graphite', { min: moment(tlConfig.time.from).format('HH:mm[_]YYYYMMDD'), max: moment(tlConfig.time.to).format('HH:mm[_]YYYYMMDD'), }; + const allowedUrls = tlConfig.allowedGraphiteUrls; - const blockedUrls = tlConfig.blockedGraphiteUrls; + const blockedIPs = tlConfig.blockedGraphiteIPs; const configuredUrl = tlConfig.settings['timeline:graphite.url']; - if (allowedUrls.length === 0 && blockedUrls.length === 0) { + + if (allowedUrls.length === 0 && blockedIPs.length === 0) { throw new Error( - i18n.translate('timeline.help.functions.missCheckGraphiteUrl', { + i18n.translate('timeline.help.functions.missCheckGraphiteConfig', { defaultMessage: MISS_CHECKLIST_MESSAGE, }) ); } - if (!checkConfigUrls(blockedUrls, allowedUrls, configuredUrl)) { + if (!isValidConfig(blockedIPs, allowedUrls, configuredUrl)) { throw new Error( - i18n.translate('timeline.help.functions.invalidGraphiteUrl', { + i18n.translate('timeline.help.functions.invalidGraphiteConfig', { defaultMessage: INVALID_URL_MESSAGE, }) ); diff --git a/src/plugins/vis_type_timeline/server/series_functions/graphite.test.js b/src/plugins/vis_type_timeline/server/series_functions/graphite.test.js index 150c704f2d63..972e62e1a75a 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/graphite.test.js +++ b/src/plugins/vis_type_timeline/server/series_functions/graphite.test.js @@ -35,12 +35,15 @@ const expect = require('chai').expect; import fn from './graphite'; const MISS_CHECKLIST_MESSAGE = `Please configure on the opensearch_dashbpards.yml file. -You can always enable the default allowlist configuration. `; +You can always enable the default allowlist configuration.`; -const INVALID_URL_MESSAGE = `The Graphite URL provided by you is invalid. +const INVALID_URL_MESSAGE = `The Graphite URL/IP provided by you is invalid. Please update your config from OpenSearch Dashboards's Advanced Setting.`; -jest.mock('node-fetch', () => () => { +jest.mock('node-fetch', () => (url) => { + if (url.includes('redirect')) { + return Promise.reject(new Error('maximum redirect reached at: ' + url)); + } return Promise.resolve({ json: function () { return [ @@ -62,28 +65,38 @@ import invoke from './helpers/invoke_series_fn.js'; describe('graphite', function () { it('should wrap the graphite response up in a seriesList', function () { - return invoke(fn, []).then(function (result) { + return invoke(fn, [], { + allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], + blockedGraphiteIPs: [], + }).then(function (result) { expect(result.output.list[0].data[0][1]).to.eql(3); expect(result.output.list[0].data[1][1]).to.eql(14); }); }); it('should convert the seconds to milliseconds', function () { - return invoke(fn, []).then(function (result) { + return invoke(fn, [], { + allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], + blockedGraphiteIPs: [], + }).then(function (result) { expect(result.output.list[0].data[1][0]).to.eql(2000 * 1000); }); }); it('should set the label to that of the graphite target', function () { - return invoke(fn, []).then(function (result) { + return invoke(fn, [], { + allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], + blockedGraphiteIPs: [], + }).then(function (result) { expect(result.output.list[0].label).to.eql('__beer__'); }); }); it('should return error message if both allowlist and blocklist are disabled', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'http://127.0.0.1' }, + settings: { 'timeline:graphite.url': 'http://127.0.0.1' }, allowedGraphiteUrls: [], + blockedGraphiteIPs: [], }).catch((e) => { expect(e.message).to.eql(MISS_CHECKLIST_MESSAGE); }); @@ -92,17 +105,20 @@ describe('graphite', function () { it('setting with matched allowlist url should return result ', function () { return invoke(fn, [], { settings: { - 'timelion:graphite.url': 'https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite', + 'timeline:graphite.url': 'https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite', }, + allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], + blockedGraphiteIPs: [], }).then((result) => { - console.log(result); expect(result.output.list.length).to.eql(1); }); }); it('setting with unmatched allowlist url should return error message ', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'http://127.0.0.1' }, + settings: { 'timeline:graphite.url': 'http://127.0.0.1' }, + allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'], + blockedGraphiteIPs: [], }).catch((e) => { expect(e.message).to.eql(INVALID_URL_MESSAGE); }); @@ -110,9 +126,9 @@ describe('graphite', function () { it('setting with matched blocklist url should return error message', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'http://127.0.0.1' }, + settings: { 'timeline:graphite.url': 'http://127.0.0.1' }, allowedGraphiteUrls: [], - blockedGraphiteUrls: ['127.0.0.0/8'], + blockedGraphiteIPs: ['127.0.0.0/8'], }).catch((e) => { expect(e.message).to.eql(INVALID_URL_MESSAGE); }); @@ -120,9 +136,9 @@ describe('graphite', function () { it('setting with matched blocklist localhost should return error message', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'http://localhost' }, + settings: { 'timeline:graphite.url': 'http://localhost' }, allowedGraphiteUrls: [], - blockedGraphiteUrls: ['127.0.0.0/8'], + blockedGraphiteIPs: ['127.0.0.0/8'], }).catch((e) => { expect(e.message).to.eql(INVALID_URL_MESSAGE); }); @@ -130,31 +146,29 @@ describe('graphite', function () { it('setting with unmatched blocklist https url should return result', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'https://www.amazon.com' }, + settings: { 'timeline:graphite.url': 'https://www.amazon.com' }, allowedGraphiteUrls: [], - blockedGraphiteUrls: ['127.0.0.0/8'], + blockedGraphiteIPs: ['127.0.0.0/8'], }).then((result) => { - console.log(result); expect(result.output.list.length).to.eql(1); }); }); it('setting with unmatched blocklist ftp url should return result', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'ftp://www.amazon.com' }, + settings: { 'timeline:graphite.url': 'ftp://www.amazon.com' }, allowedGraphiteUrls: [], - blockedGraphiteUrls: ['127.0.0.0/8'], + blockedGraphiteIPs: ['127.0.0.0/8'], }).then((result) => { - console.log(result); expect(result.output.list.length).to.eql(1); }); }); it('setting with invalid url should return error message', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'www.amazon.com' }, + settings: { 'timeline:graphite.url': 'www.amazon.com' }, allowedGraphiteUrls: [], - blockedGraphiteUrls: ['127.0.0.0/8'], + blockedGraphiteIPs: ['127.0.0.0/8'], }).catch((e) => { expect(e.message).to.eql(INVALID_URL_MESSAGE); }); @@ -162,7 +176,9 @@ describe('graphite', function () { it('setting with redirection error message', function () { return invoke(fn, [], { - settings: { 'timelion:graphite.url': 'https://amazon.com/redirect' }, + settings: { 'timeline:graphite.url': 'https://amazon.com/redirect' }, + allowedGraphiteUrls: [], + blockedGraphiteIPs: ['127.0.0.0/8'], }).catch((e) => { expect(e.message).to.includes('maximum redirect reached'); });