From 1901d3e659baac46fc045e9064193e5e4a002f86 Mon Sep 17 00:00:00 2001 From: Dino Chiesa Date: Wed, 18 Sep 2024 22:51:24 -0700 Subject: [PATCH 1/3] feat: BN014 duplicate policy checker --- .../plugins/BN014-duplicate-policies.js | 254 ++++++++++++++++++ .../resources/BN014/cors-test/README.md | 7 + .../BN014/cors-test/apiproxy/cors-test.xml | 10 + ...AM-Clean-Request-Headers-From-Response.xml | 21 ++ .../AM-Inject-Proxy-Revision-Header.xml | 7 + .../apiproxy/policies/AM-Response-2.xml | 21 ++ .../apiproxy/policies/AM-Response-3.xml | 19 ++ .../apiproxy/policies/AM-Response.xml | 12 + .../cors-test/apiproxy/policies/CORS-1.xml | 10 + .../cors-test/apiproxy/policies/CORS-A.xml | 21 ++ .../apiproxy/policies/RF-Unknown-Request.xml | 16 ++ .../apiproxy/policies/VerifyAPIKey-1.xml | 3 + .../cors-test/apiproxy/proxies/endpoint1.xml | 74 +++++ test/specs/BN014-duplicate-policies.js | 92 +++++++ test/specs/PO037-DataCapture-hygiene.js | 2 +- 15 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 lib/package/plugins/BN014-duplicate-policies.js create mode 100644 test/fixtures/resources/BN014/cors-test/README.md create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/cors-test.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Clean-Request-Headers-From-Response.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-2.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-3.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-1.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-A.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/RF-Unknown-Request.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/policies/VerifyAPIKey-1.xml create mode 100644 test/fixtures/resources/BN014/cors-test/apiproxy/proxies/endpoint1.xml create mode 100644 test/specs/BN014-duplicate-policies.js diff --git a/lib/package/plugins/BN014-duplicate-policies.js b/lib/package/plugins/BN014-duplicate-policies.js new file mode 100644 index 00000000..bb02967a --- /dev/null +++ b/lib/package/plugins/BN014-duplicate-policies.js @@ -0,0 +1,254 @@ +/* + Copyright 2019-2020,2024 Google LLC + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +const ruleId = require("../myUtil.js").getRuleId(), + debug = require("debug")("apigeelint:" + ruleId), + xpath = require("xpath"); + +const ELEMENT_NODE = 1; +//const ATTRIBUTE_NODE = 2; +const TEXT_NODE = 3; +const CDATA_SECTION_NODE = 4; + +const plugin = { + ruleId, + name: "Check for duplicate policies", + message: "Multiple identically configured policies.", + fatal: false, + severity: 1, // 1=warning, 2=error + nodeType: "Endpoint", + enabled: true +}; + +const onBundle = function (bundle, cb) { + debug("onBundle"); + const flagged = check(bundle); + if (typeof cb == "function") { + cb(null, flagged); + } +}; + +const _markPolicy = (policy, msg) => + policy.addMessage({ + ruleId: plugin.ruleId, + severity: plugin.severity, + source: policy.getSource(), + line: policy.getElement().lineNumber, + column: policy.getElement().columnNumber, + nodeType: "Policy", + message: msg + }); + +/* + * Recursive fn that returns true if XML elements are different. Ignores any intervening whitespace or indents. + * + **/ +const _diffXmlNode = (isTopLevel) => (indent, elt1, elt2) => { + debug(`${indent}elt1: TYPE(${elt1.nodeType}) tag(${elt1.tagName})`); + debug(`${indent}elt2: TYPE(${elt2.nodeType}) tag(${elt2.tagName})`); + // verify same node type + if (elt1.nodeType != elt2.nodeType) { + debug(`${indent}different nodeType`); + return true; + } + + if (elt1.nodeType == TEXT_NODE) { + debug( + `${indent}test '${elt1.nodeValue.trim()}' ==? '${elt2.nodeValue.trim()}'` + ); + return elt1.nodeValue.trim() != elt2.nodeValue.trim(); + } + + if (elt1.nodeType == CDATA_SECTION_NODE) { + debug(`${indent}test '${elt1.data}' ==? '${elt2.data}'`); + return elt1.data != elt2.data; + } + + if (elt1.nodeType == ELEMENT_NODE) { + // compare tagname on element nodes + debug(`${indent}test '${elt1.tagName}' ==? '${elt2.tagName}'`); + if (elt1.tagName != elt2.tagName) { + return true; + } + + // compare attrs, maybe excepting name + const attrsToCompare = (elt) => { + let attrs = xpath.select("@*", elt); + if (isTopLevel) { + attrs = attrs.filter((attr) => attr.name != "name"); + } + return attrs; + }; + + // compare attrs without respect to ordering + const attrs1 = attrsToCompare(elt1); + const attrs2 = attrsToCompare(elt2); + let diff = + attrs1.length != attrs2.length || + !!attrs1.find( + (attr1, _i) => + !attrs2.find( + (attr2) => attr2.name == attr1.name && attr2.value == attr1.value + ) + ); + + // compare child nodes, respecting ordering + if (!diff) { + const childrenToCompare = (elt) => + xpath + .select("node()|text()", elt) + .filter( + (node) => + node.nodeType == ELEMENT_NODE || + node.nodeType == TEXT_NODE || + node.nodeType == CDATA_SECTION_NODE + ); + const children1 = childrenToCompare(elt1); + const children2 = childrenToCompare(elt2); + + if ( + children1.length == children2.length && + children2.length == 1 && + children2[0].nodeType == TEXT_NODE + ) { + diff = children1[0].nodeValue.trim() != children2[0].nodeValue.trim(); + } else { + debug(`${indent}recurse`); + /* + * It is too simplistic to use the lengths of the node sets as a basis of + * difference. If there is a comment in one of the documents, and not in the other, + * it results in multiple text nodes in one of the nodesets and just one in the + * other. Instead, we effectively ignore toplevel text nodes in either + * document. This is ok, as there are no cases in which Apigee uses complex XML in + * which a child nodeset includes both TEXT nodes and elements. In other words, in + * all valid Apigee configuration, if the node is an element, it either has a single + * TEXT child, or it has one or more element children. Never both. + **/ + const reducer = (a) => (accumulator, elt, _index) => { + // if they are already different, skip all further checks + if (!accumulator.different) { + // skip all TEXT nodes in children1 + if (elt.nodeType != TEXT_NODE) { + // likewise, skip all successive TEXT nodes in children2 + let ix2 = accumulator.ix2; + while (a[ix2] && a[ix2].nodeType == TEXT_NODE) { + ix2++; + } + const elt2 = a[ix2]; + debug(`${indent} elt2: ${elt2}`); + accumulator.different = + !elt2 || _diffXmlNode(false)(indent + " ", elt, elt2); + accumulator.ix2 = ++ix2; + } + } + return accumulator; + }; + + const result = children1.reduce(reducer(children2), { + ix2: 0, + different: false + }); + diff = result.different; + } + } + return diff; + } + throw new Error("unhandled node type"); +}; + +/* + * Returns true if XML elements are different. This ignores the toplevel name attribute, + * and any intervening whitespace or indents. + * + **/ +const diffXml = (elt1, elt2) => _diffXmlNode(true)("", elt1, elt2); + +const _checkForDuplicatePolicies = (policies) => { + const xpath = "/*/*"; + let flagged = false; + /** + * Check each policy for duplicates. + * + * This sort of works, but it's naive in that it compares the XML + * configuration directly, including whitespace. A better comparison would + * ignore whitespace and just compare the digest of the XML. That is an + * unrealizable dream as the digest is determined by how Apigee interprets + * the XML. We might be able to get close by comparing the XML infoset. + * + **/ + const previouslyDetected = []; + + policies.slice(0, -1).forEach((policy1, i, a) => { + if (!previouslyDetected.includes(i)) { + try { + //const p1 = policy1.select(xpath).toString().trim(); + debug( + `looking at index ${i}: type(${policy1.getType()}) name(${policy1.getName()})` + ); + const dupesForI = []; + a.slice(i + 1).forEach((c, j) => { + // debug(` comparing to ${c}...`); + if (!diffXml(policy1.getElement(), c.getElement())) { + const actualIndex = j + i + 1; + debug(` duplicate found at index ${actualIndex}`); + dupesForI.push(actualIndex); + } + }); + + if (dupesForI.length) { + debug(` dupes found for ${i}: ${dupesForI}`); + dupesForI.forEach((ix) => + _markPolicy( + policies[ix], + `Policy ${policies[ix].getName()} is a duplicate of Policy ${policies[i].getName()}. Eliminate duplicates and attach a single policy in multiple places.` + ) + ); + flagged = true; + previouslyDetected.push(...dupesForI); + debug(`\n dupes found so far: ${previouslyDetected}`); + } + } catch (e) { + console.log(e.stack); + _markPolicy( + policy1, + `Error processing Policy ${policy1.getName()}: ${e.message}.` + ); + flagged = true; + } + } + }); + return flagged; +}; + +const check = (bundle) => { + let flagged = false; + if (bundle.policies) { + debug("number of policies: " + bundle.policies.length); + if (bundle.policies.length > 1) { + flagged = _checkForDuplicatePolicies( + bundle.policies.toSorted((a, b) => + a.getName().localeCompare(b.getName()) + ) + ); + } + } + return flagged; +}; + +module.exports = { + plugin, + onBundle +}; diff --git a/test/fixtures/resources/BN014/cors-test/README.md b/test/fixtures/resources/BN014/cors-test/README.md new file mode 100644 index 00000000..328df9b1 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/README.md @@ -0,0 +1,7 @@ +20240424-1159 + +A loopback proxy to test CORS policy and DefaultFaultRule + +https://www.googlecloudcommunity.com/gc/Apigee/Default-Fault-Rule-not-adding-CORS-headers-in-Raise-Fault/m-p/742799#M79285 + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/cors-test.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/cors-test.xml new file mode 100644 index 00000000..6dd4a574 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/cors-test.xml @@ -0,0 +1,10 @@ + + + 1489339923158 + dino + + cors-test + 1489340418969 + orgAdmin + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Clean-Request-Headers-From-Response.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Clean-Request-Headers-From-Response.xml new file mode 100644 index 00000000..0705cbde --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Clean-Request-Headers-From-Response.xml @@ -0,0 +1,21 @@ + + + +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml new file mode 100644 index 00000000..fcf6c0ae --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml @@ -0,0 +1,7 @@ + + + +
{apiproxy.name} r{apiproxy.revision}
+
+
+
diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-2.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-2.xml new file mode 100644 index 00000000..072d4f27 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-2.xml @@ -0,0 +1,21 @@ + + true + + + { + "status" : "ok" +} + + OK + 200 + + + response + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-3.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-3.xml new file mode 100644 index 00000000..9fa0aae3 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response-3.xml @@ -0,0 +1,19 @@ + + true + + + { + "status" : "ok" +} + + + NOTOK + 200 + + + response + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response.xml new file mode 100644 index 00000000..f976727b --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/AM-Response.xml @@ -0,0 +1,12 @@ + + true + + { + "status" : "ok" +} + + OK + 200 + + response + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-1.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-1.xml new file mode 100644 index 00000000..ca18cfa0 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-1.xml @@ -0,0 +1,10 @@ + + {request.header.origin:*} + GET, PUT, POST, DELETE, OPTIONS + origin, x-requested-with + * + 1800 + true + true + true + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-A.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-A.xml new file mode 100644 index 00000000..59ba682e --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/CORS-A.xml @@ -0,0 +1,21 @@ + + + + + {request.header.origin:*} + + GET, PUT, POST, DELETE, OPTIONS + + origin, x-requested-with + + * + + 1800 + + true + + true + + true + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/RF-Unknown-Request.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/RF-Unknown-Request.xml new file mode 100644 index 00000000..9f5f7fe1 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/RF-Unknown-Request.xml @@ -0,0 +1,16 @@ + + true + + + { + "error" : { + "code" : 404.01, + "message" : "that request was unknown; try a different request." + } +} + + 404 + Not Found + + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/policies/VerifyAPIKey-1.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/VerifyAPIKey-1.xml new file mode 100644 index 00000000..f48908ec --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/policies/VerifyAPIKey-1.xml @@ -0,0 +1,3 @@ + + + diff --git a/test/fixtures/resources/BN014/cors-test/apiproxy/proxies/endpoint1.xml b/test/fixtures/resources/BN014/cors-test/apiproxy/proxies/endpoint1.xml new file mode 100644 index 00000000..1f8f44b8 --- /dev/null +++ b/test/fixtures/resources/BN014/cors-test/apiproxy/proxies/endpoint1.xml @@ -0,0 +1,74 @@ + + Proxy Endpoint 1 + + /cors-loopback + + + + + + AM-Inject-Proxy-Revision-Header + + true + + + + + + CORS-1 + + + CORS-A + + + + + AM-Clean-Request-Headers-From-Response + + + + + + + + + + AM-Inject-Proxy-Revision-Header + + + AM-Response + + + + + + + + + + + + + + + + VerifyAPIKey-1 + + + + (proxy.pathsuffix MatchesPath "/t1") and (request.verb = "GET") + + + + + + RF-Unknown-Request + + + + + + + + + diff --git a/test/specs/BN014-duplicate-policies.js b/test/specs/BN014-duplicate-policies.js new file mode 100644 index 00000000..85c24c0c --- /dev/null +++ b/test/specs/BN014-duplicate-policies.js @@ -0,0 +1,92 @@ +/* + Copyright 2019-2024 Google LLC + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +/* global describe, it, __dirname */ + +const assert = require("assert"), + path = require("path"), + util = require("util"), + ruleId = "BN014", + debug = require("debug")("apigeelint:" + ruleId), + bl = require("../../lib/package/bundleLinter.js"); + +const expectedErrors = { + "CORS-A.xml": [ + "Policy CORS-A is a duplicate of Policy CORS-1. Eliminate duplicates and attach a single policy in multiple places." + ], + "AM-Response-2.xml": [ + "Policy AM-Response-2 is a duplicate of Policy AM-Response. Eliminate duplicates and attach a single policy in multiple places." + ] +}; + +describe(`BN014 - Duplicate policies`, () => { + const configuration = { + debug: true, + source: { + type: "filesystem", + path: path.resolve( + __dirname, + "../fixtures/resources/BN014/cors-test", + "apiproxy" + ), + bundleType: "apiproxy", + profile: "apigeex" + }, + excluded: {}, + setExitCode: false, + output: () => {} // suppress output + }; + + debug(`BN014 configuration: ${util.format(configuration)}`); + bl.lint(configuration, (bundle) => { + const items = bundle.getReport(); + assert.ok(items); + assert.ok(items.length); + const bn014Items = items.filter((item) => + item.messages.some((m) => m.ruleId == "BN014") + ); + it(`should generate the expected number of errors`, () => { + debug(`bn014Items: ${util.format(bn014Items.map((i) => i.filePath))}`); + + assert.equal(bn014Items.length, Object.keys(expectedErrors).length); + }); + + Object.keys(expectedErrors).forEach((policyName, caseNum) => { + it(`should generate the expected errors for ${policyName}`, () => { + debug(`policyName: ${policyName}`); + const expected = expectedErrors[policyName]; + const policyItems = bn014Items.filter((item) => + item.filePath.endsWith(policyName) + ); + debug(`policyItems: ${util.format(policyItems)}`); + + assert.equal(policyItems.length, 1); + const bn014Messages = policyItems[0].messages.filter( + (m) => m.ruleId == "BN014" + ); + debug(`po035Messages: ${util.format(bn014Messages)}`); + assert.equal(bn014Messages.length, expected.length); + assert.equal(bn014Messages.length, 1); + + assert.equal( + bn014Messages[0].message, + expected[0], + `${policyName} case(${caseNum})` + ); + }); + }); + }); +}); diff --git a/test/specs/PO037-DataCapture-hygiene.js b/test/specs/PO037-DataCapture-hygiene.js index ee8bb6f2..f8b7149f 100644 --- a/test/specs/PO037-DataCapture-hygiene.js +++ b/test/specs/PO037-DataCapture-hygiene.js @@ -146,7 +146,7 @@ describe(`PO037 - DataCapture Source usage`, () => { it(`should generate no errors other than PO037`, () => { const nonPO37Items = items.filter((item) => - item.messages.some((m) => m.ruleId != "PO037") + item.messages.some((m) => m.ruleId != "PO037" && m.ruleId != "BN014") ); debug( `nonPO37Items: ${util.format(nonPO37Items.map((i) => i.filePath))}` From ad3a05d06cfb21e349740d7efaf1df01d93d7e74 Mon Sep 17 00:00:00 2001 From: Dino Chiesa Date: Wed, 18 Sep 2024 22:54:32 -0700 Subject: [PATCH 2/3] add README update --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 7b0bbb8f..86fa8594 100644 --- a/README.md +++ b/README.md @@ -320,6 +320,7 @@ This is the current list: |   |:white_check_mark:| BN011 | Check each XML file for well-formedness.| |   |:white_check_mark:| BN012 | unreferrenced Target Endpoints | Check that each TargetEndpoint can be reached. | |   |:white_check_mark:| BN013 | Unreferenced resources. | Warn for resources that not referenced in any policy. Unreferenced resources are dead code. | +|   |:white_check_mark:| BN014 | Duplicate policies. | Warn if there are identically configured, if differently named, policies. | | Proxy Definition |   |   |   |   | |   |:white_check_mark:| PD001 | RouteRules to Targets | RouteRules should map to defined Targets. | |   |:white_check_mark:| PD002 | Unreachable Route Rules - defaults | Only one RouteRule should be present without a condition. | From 3e768a210286b993fd1ce2dfa251362809331a2d Mon Sep 17 00:00:00 2001 From: Dino Chiesa Date: Wed, 18 Sep 2024 22:58:41 -0700 Subject: [PATCH 3/3] remove the code in EP001 for detecting duplicates --- .../plugins/EP001-corsPolicyAttachment.js | 239 ++++++------------ 1 file changed, 74 insertions(+), 165 deletions(-) diff --git a/lib/package/plugins/EP001-corsPolicyAttachment.js b/lib/package/plugins/EP001-corsPolicyAttachment.js index 73b898b9..80f897fb 100644 --- a/lib/package/plugins/EP001-corsPolicyAttachment.js +++ b/lib/package/plugins/EP001-corsPolicyAttachment.js @@ -1,5 +1,5 @@ /* - Copyright 2019-2020 Google LLC + Copyright 2019-2020,2024 Google LLC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,150 +15,63 @@ */ const ruleId = require("../myUtil.js").getRuleId(), - debug = require("debug")("apigeelint:" + ruleId); + debug = require("debug")("apigeelint:" + ruleId); const plugin = { - ruleId, - name: "Check for multiple CORS policies", - message: - "Only one CORS Policy is necessary. It should attach in the Proxy Preflow.", - fatal: false, - severity: 2, // error - nodeType: "Endpoint", - enabled: true - }; - -const onBundle = function(bundle, cb) { - debug('onBundle'); - let checker = new BundleChecker(bundle); - let flagged = checker.check(); - - if (typeof(cb) == 'function') { - cb(null, flagged); - } - }; - -const onProxyEndpoint = function(endpoint, cb) { - debug('onProxyEndpoint'); - let checker = new EndpointChecker(endpoint, true); - let flagged = checker.check(); - if (typeof(cb) == 'function') { - cb(null, flagged); - } - }; - -const onTargetEndpoint = function(endpoint, cb) { - debug('onTargetEndpoint'); - let checker = new EndpointChecker(endpoint, false); - let flagged = checker.check(); - if (typeof(cb) == 'function') { - cb(null, flagged); - } - }; - -const _markPolicy = (policy, msg) => { - let result = { - ruleId: plugin.ruleId, - severity: plugin.severity, - source: policy.getSource(), - line: policy.getElement().lineNumber, - column: policy.getElement().columnNumber, - nodeType: "Policy", - message: msg - }; - policy.addMessage(result); - }; - - -const _markStep = (step, msg) => { - let result = { - ruleId: plugin.ruleId, - severity: plugin.severity, - entity: step, - line: step.getElement().lineNumber, - column: step.getElement().columnNumber, - nodeType: "Step", - message: msg - }; - step.addMessage(result); - }; - -const _markEndpoint = (endpoint, msg) => { - var result = { - ruleId: plugin.ruleId, - severity: plugin.severity, - nodeType: plugin.nodeType, - message: msg - }; - endpoint.addMessage(result); - }; - -class BundleChecker { - constructor(bundle) { - debug('BundleChecker ctor'); - this.bundle = bundle; - this.flagged = false; - } - - check() { - if (this.bundle.policies) { - - debug("number of policies: " + this.bundle.policies.length); - let corsPolicies = this.bundle.policies.filter(policy => (policy.getType() === "CORS")); + ruleId, + name: "Check for multiple CORS policies", + message: + "Only one CORS Policy is necessary. It should attach in the Proxy Preflow.", + fatal: false, + severity: 2, // error + nodeType: "Endpoint", + enabled: true +}; - debug("number of CORS policies: " + corsPolicies.length); - if (corsPolicies.length > 1) { - this.flagged = this._checkForDuplicatePolicies(corsPolicies); - } - return this.flagged; - } +const onProxyEndpoint = function (endpoint, cb) { + debug("onProxyEndpoint"); + const checker = new EndpointChecker(endpoint, true); + const flagged = checker.check(); + if (typeof cb == "function") { + cb(null, flagged); } +}; - _checkForDuplicatePolicies(policies) { - let dupes = BundleChecker._getDuplicates('/CORS', policies); - if (dupes.length > 0) { - debug("there are " + dupes.length + " repeated policies."); - let duplicatePolicyNames = dupes.map( p => p.getName()).join(" "); - debug("duplicate policies: " + duplicatePolicyNames); - policies.forEach(policy => { - debug("duplicate policy warning for " + policy.getName()); - _markPolicy( - policy, - "The following CORS policies are configured: " + - duplicatePolicyNames - ); - }); - return true; - } +const onTargetEndpoint = function (endpoint, cb) { + debug("onTargetEndpoint"); + const checker = new EndpointChecker(endpoint, false); + const flagged = checker.check(); + if (typeof cb == "function") { + cb(null, flagged); } +}; - static _getDuplicates(xpath, policies) { - let dupes = []; - for (var i = 0; i < policies.length - 1; i++) { - for (var j = i + 1; j < policies.length; j++) { - var p1 = policies[i].select(xpath).toString().trim(); - var p2 = policies[j].select(xpath).toString().trim(); - debug("comparing -> \n1st policy:\n" + p1 + "\n2nd policy:\n" + p2); - if (p1 === p2) { - if (dupes.indexOf(policies[i]) === -1) { - dupes.push(policies[i]); - debug(policies[i].getName() + " is a duplicate!"); - } - if (dupes.indexOf(policies[j]) === -1) { - dupes.push(policies[j]); - debug(policies[j].getName() + " is a duplicate!"); - } - } - } - } - return dupes; - } -} +const _markStep = (step, msg) => { + let result = { + ruleId: plugin.ruleId, + severity: plugin.severity, + entity: step, + line: step.getElement().lineNumber, + column: step.getElement().columnNumber, + nodeType: "Step", + message: msg + }; + step.addMessage(result); +}; +const _markEndpoint = (endpoint, msg) => { + var result = { + ruleId: plugin.ruleId, + severity: plugin.severity, + nodeType: plugin.nodeType, + message: msg + }; + endpoint.addMessage(result); +}; class EndpointChecker { constructor(endpoint, isProxyEndpoint) { - debug('EndpointChecker ctor (%s)', endpoint.getName()); + debug("EndpointChecker ctor (%s)", endpoint.getName()); this.endpoint = endpoint; this.bundle = endpoint.parent; this.isProxyEndpoint = isProxyEndpoint; @@ -170,62 +83,60 @@ class EndpointChecker { this.corsPoliciesInBundle = this.bundle.policies && this.bundle.policies - .filter(policy => (policy.getType() === "CORS")) - .reduce(function(obj, policy) { - obj[policy.getName()] = policy; - return obj; - }, {}); + .filter((policy) => policy.getType() === "CORS") + .reduce(function (obj, policy) { + obj[policy.getName()] = policy; + return obj; + }, {}); if (this.corsPoliciesInBundle) { let keys = Object.keys(this.corsPoliciesInBundle); - debug('CORS policies in endpoint: ' + JSON.stringify(keys)); + debug("CORS policies in endpoint: " + JSON.stringify(keys)); // Ensure at most one attachment for any CORS policy, // or, conditions on all CORS policies. let corsStepsInEndpoint = this.endpoint .getSteps() - .filter( step => keys.indexOf(step.getName())>=0); + .filter((step) => keys.indexOf(step.getName()) >= 0); if (this.isProxyEndpoint) { if (corsStepsInEndpoint.length > 1) { - this.flagged = this._checkStepsForMissingConditions(corsStepsInEndpoint); - } - else { - debug('zero or one CORS policies in this endpoint'); + this.flagged = + this._checkStepsForMissingConditions(corsStepsInEndpoint); + } else { + debug("zero or one CORS policies in this endpoint"); } - } - else { + } else { if (corsStepsInEndpoint.length > 0) { - corsStepsInEndpoint.forEach(step => { + corsStepsInEndpoint.forEach((step) => { _markStep( step, - "There is a CORS policy attached to a TargetEndpoint. Attach CORS policies to a ProxyEndpoint."); - + "There is a CORS policy attached to a TargetEndpoint. Attach CORS policies to a ProxyEndpoint." + ); }); this.flagged = true; } } } return this.flagged; - } - catch(e) { + } catch (e) { console.log(e); return false; } } _checkStepsForMissingConditions(steps) { - let anyStepsLackCondition = - steps.filter( step => { - if ( ! step.getCondition()) { - let name = step.getName(); - _markStep( - step, - //this.corsPoliciesInBundle[name], - `There are multiple CORS policies and policy ${name} is attached to a Step without a Condition.`); - return true; - } - }); + let anyStepsLackCondition = steps.filter((step) => { + if (!step.getCondition()) { + let name = step.getName(); + _markStep( + step, + //this.corsPoliciesInBundle[name], + `There are multiple CORS policies and policy ${name} is attached to a Step without a Condition.` + ); + return true; + } + }); if (anyStepsLackCondition.length > 0) { _markEndpoint( @@ -237,10 +148,8 @@ class EndpointChecker { } } - module.exports = { plugin, - onBundle, onProxyEndpoint, onTargetEndpoint };