diff --git a/README.md b/README.md index 231b92bf..9ea332ca 100644 --- a/README.md +++ b/README.md @@ -260,6 +260,8 @@ This is the current list: | Endpoints |   |   |   |   | |   |:white_check_mark:| EP001 | CORS Policy attachment | Check for multiple CORS policies, or attachment to Target Endpoint. | |   |:white_check_mark:| EP002 | Misplaced Elements | Check for commonly misplaced configuration elements in Proxy and Target Endpoints. | +| Features |   |   |   |   | +|   |:white_check_mark:| FE001 | Use of Authentication element | Check for the Authentication element in policies or in Target Endpoints. | | Deprecation |   |   |   |   | |   |:white_check_mark:| DC001 | ConcurrentRateLimit Policy Deprecation | Check usage of deprecated policy ConcurrentRateLimit. | |   |:white_check_mark:| DC002 | OAuth V1 Policies Deprecation | Check usage of deprecated OAuth V1 policies. | diff --git a/lib/package/plugins/FE001-authentication.js b/lib/package/plugins/FE001-authentication.js new file mode 100644 index 00000000..6a29c3db --- /dev/null +++ b/lib/package/plugins/FE001-authentication.js @@ -0,0 +1,176 @@ +/* + Copyright 2019-2023 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 myUtil = require("../myUtil.js"), + xpath = require("xpath"), + //util = require("util"), + ruleId = myUtil.getRuleId(), + debug = require("debug")("apigeelint:" + ruleId); + +const plugin = { + ruleId, + name: "Check for use of Authentication element in ServiceCallout, ExternalCallout and TargetEndpoint", + fatal: false, + severity: 2, //1= warning, 2=error + nodeType: "Feature", + enabled: true +}; + +const policiesToCheck = { + ServiceCallout: { + path: `/ServiceCallout/HTTPTargetConnection/Authentication`, + validChildren: ["HeaderName", "GoogleIDToken", "GoogleAccessToken"] + }, + ExternalCallout: { + path: `/ExternalCallout/GrpcConnection/Authentication`, + validChildren: ["HeaderName", "GoogleIDToken"] + } +}; + +let bundleProfile = "apigee"; +const onBundle = function (bundle, cb) { + if (bundle.profile) { + bundleProfile = bundle.profile; + } + if (typeof cb == "function") { + cb(null, false); + } +}; + +const checkAuthElements = function ( + selection, + addMessage, + label, + validChildren +) { + if (selection.length > 1) { + addMessage( + selection[1].lineNumber, + selection[1].columnNumber, + `${label} has multiple Authentication elements. You should have a maximum of one.` + ); + } + const authNode = selection[0]; + const children = xpath.select("*", authNode); + if (children.length == 0) { + addMessage( + authNode.lineNumber, + authNode.columnNumber, + `misconfigured (empty) Authentication element.` + ); + } else { + children.forEach((child) => { + if (!validChildren.includes(child.nodeName)) { + addMessage( + child.lineNumber, + child.columnNumber, + `Authentication element has unsupported child (${child.nodeName}).` + ); + } + }); + + const tokenNodes = children + .filter((child) => validChildren.includes(child.nodeName)) + .filter((child) => child.nodeName.endsWith("Token")); + if (tokenNodes.length == 0) { + const possibleElements = validChildren.filter((c) => c != "HeaderName"); + const message = + possibleElements.length == 1 + ? `Authentication element needs a child ${possibleElements[0]}` + : `Authentication element needs a child, one of [${possibleElements.join( + "," + )}]`; + addMessage(authNode.lineNumber, authNode.columnNumber, message); + } else if (tokenNodes.length > 1) { + addMessage( + tokenNodes[1].lineNumber, + tokenNodes[1].columnNumber, + `element ${tokenNodes[1].nodeName} is invalid here, because there is more than one *Token element.` + ); + } + } +}; + +const onPolicy = function (policy, cb) { + let foundIssue = false; + const policyType = policy.getType(); + const check = policiesToCheck[policyType]; + if (check) { + const selection = policy.select(check.path); + if (selection && selection[0]) { + if (bundleProfile != "apigeex") { + foundIssue = true; + policy.addMessage({ + plugin, + line: selection[0].lineNumber, + column: selection[0].columnNumber, + message: `Policy uses an Authentication element. Not supported in this profile.` + }); + } else { + const addMessage = (line, column, message) => { + foundIssue = true; + policy.addMessage({ plugin, line, column, message }); + }; + checkAuthElements(selection, addMessage, "Policy", check.validChildren); + } + } + } + if (typeof cb == "function") { + cb(null, foundIssue); + } +}; + +const onTargetEndpoint = function (endpoint, cb) { + debug("onTargetEndpoint"); + let foundIssue = false; + const httpTargets = xpath.select("HTTPTargetConnection", endpoint.element); + if (httpTargets && httpTargets[0]) { + debug("found HTTPTargetConnection"); + const authNodes = xpath.select("Authentication", httpTargets[0]); + if (authNodes && authNodes[0]) { + debug("found Authentication"); + if (bundleProfile != "apigeex") { + foundIssue = true; + endpoint.addMessage({ + plugin, + line: authNodes[0].lineNumber, + column: authNodes[0].columnNumber, + message: `Endpoint uses an Authentication element. Not supported in this profile.` + }); + } else { + const addMessage = (line, column, message) => { + foundIssue = true; + endpoint.addMessage({ plugin, line, column, message }); + }; + checkAuthElements(authNodes, addMessage, "TargetEndpoint", [ + "GoogleIDToken", + "GoogleAccessToken" + ]); + } + } + } + if (typeof cb == "function") { + cb(null, foundIssue); + } + return foundIssue; +}; + +module.exports = { + plugin, + onBundle, + onPolicy, + onTargetEndpoint +}; diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/flightdata.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/flightdata.xml new file mode 100644 index 00000000..c7125d72 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/flightdata.xml @@ -0,0 +1,12 @@ + + + 1491498008040 + + flightdata + 1491517153495 + + + + + false + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Clean-Response-Headers.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Clean-Response-Headers.xml new file mode 100644 index 00000000..f5a8223b --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Clean-Response-Headers.xml @@ -0,0 +1,20 @@ + + + +
+
+
+
+
+
+
+
+
+
+
+
+
+ + + false + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml new file mode 100644 index 00000000..02ed60b3 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml @@ -0,0 +1,7 @@ + + + +
{apiproxy.name} r{apiproxy.revision}
+ + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-1.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-1.xml new file mode 100644 index 00000000..16579101 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-1.xml @@ -0,0 +1,6 @@ + + + bq_query + + + \ No newline at end of file diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-2.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-2.xml new file mode 100644 index 00000000..9a9dcd45 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-2.xml @@ -0,0 +1,6 @@ + + + bq_query + + + \ No newline at end of file diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-3.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-3.xml new file mode 100644 index 00000000..d87001f6 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-3.xml @@ -0,0 +1,6 @@ + + + bq_query + + + \ No newline at end of file diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-4.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-4.xml new file mode 100644 index 00000000..4e2ef9aa --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-PreparedQuery-4.xml @@ -0,0 +1,6 @@ + + + bq_query + + + \ No newline at end of file diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Query.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Query.xml new file mode 100644 index 00000000..f297a84e --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/AM-Query.xml @@ -0,0 +1,15 @@ + + request + + target.copy.pathsuffix + false + + + +
application/json
+ + {"query": "{bq_query}"} + + POST + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-1-valid.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-1-valid.xml new file mode 100644 index 00000000..95fe0f1d --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-1-valid.xml @@ -0,0 +1,30 @@ + + + + + + + + + https://external-callout-2s6vjmoabq-uw.a.run.app + false + + + + + + 5000 + + true + true + true + true + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-2-accesstoken.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-2-accesstoken.xml new file mode 100644 index 00000000..a1cf3870 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EC-2-accesstoken.xml @@ -0,0 +1,27 @@ + + + + + + + + + + SCOPE + + + + + + + 5000 + + true + true + true + true + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EV-PathParams-4.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EV-PathParams-4.xml new file mode 100644 index 00000000..391d31f6 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/EV-PathParams-4.xml @@ -0,0 +1,7 @@ + + request + + /airports/{param1}/counts/{param2} + + true + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/JS-Convert-Response.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/JS-Convert-Response.xml new file mode 100644 index 00000000..0e2ee29c --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/JS-Convert-Response.xml @@ -0,0 +1,21 @@ + + 0) { + var fieldnames = c.schema.fields.map(function(field){ + return field.name; + }); + var rows = c.rows.map(function(row) { + var values = row.f.map(function(f){ return f.v; }); + var result = {}; + fieldnames.forEach(function(name, ix){ result[name] = values[ix]; }); + return result; + }); + c.rows = rows; +} +delete c.kind; +delete c.jobReference; +context.setVariable('response.content', JSON.stringify(c,null,2)+'\n'); +]]> + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/RF-Unknown-Request.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/RF-Unknown-Request.xml new file mode 100644 index 00000000..9f5f7fe1 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/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/FE001-Authentication/apiproxy/policies/SC-1-valid.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/SC-1-valid.xml new file mode 100644 index 00000000..425c4de7 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/SC-1-valid.xml @@ -0,0 +1,32 @@ + + + + +
anything here
+ + POST + + + tokenResponse + + + + + + + SCOPE + + + + + + true + true + ref://truststore-1 + + + 2xx, 3xx + + https://www.my-site.com/service + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/policies/SC-2-multiple-token-nodes.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/SC-2-multiple-token-nodes.xml new file mode 100644 index 00000000..f206769c --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/policies/SC-2-multiple-token-nodes.xml @@ -0,0 +1,37 @@ + + + + +
anything here
+ + POST + + + tokenResponse + + + + + + + SCOPE + + + + + SCOPE + + + + + + true + true + ref://truststore-1 + + + 2xx, 3xx + + https://www.my-site.com/service + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/proxies/endpoint1.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/proxies/endpoint1.xml new file mode 100644 index 00000000..9ba46a16 --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/proxies/endpoint1.xml @@ -0,0 +1,128 @@ + + + + /flightdata + + + + + + AM-Inject-Proxy-Revision-Header + + true + + + + + + + + + + + + + JS-Convert-Response + + + AM-Inject-Proxy-Revision-Header + + + + + + + + + + + + + + + + AM-PreparedQuery-1 + + + SC-1-valid + + + SC-2-multiple-token-nodes + + + EC-1-valid + + + EC-2-accesstoken + + + + + proxy.pathsuffix MatchesPath "/airlines32" and request.verb = "GET" + + + + + + + AM-PreparedQuery-2 + + + + + proxy.pathsuffix MatchesPath "/airlines100" and request.verb = "GET" + + + + + + + AM-PreparedQuery-3 + + + + + proxy.pathsuffix MatchesPath "/airlines500" and request.verb = "GET" + + + + + + + + EV-PathParams-4 + + + + AM-PreparedQuery-4 + + + + + proxy.pathsuffix MatchesPath "/airports/*/counts/*" and request.verb = "GET" + + + + + + + RF-Unknown-Request + + + + + + + + + + + target-2 + proxy.pathsuffix MatchesPath "/airlines32" and request.verb = "GET" + + + + target-1 + + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-1.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-1.xml new file mode 100644 index 00000000..ba9e6a7f --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-1.xml @@ -0,0 +1,43 @@ + + + + + AM-Query + + + + + AM-Clean-Response-Headers + + + + + + + + + + + + + + + + + + + https://www.googleapis.com/auth/cloud-platform + + + + + + true + false + ref://truststore-1 + + + + https://bigquery.googleapis.com/bigquery/v2/projects/infinite-chain-292422/queries + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-2.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-2.xml new file mode 100644 index 00000000..9fc4faad --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-2.xml @@ -0,0 +1,38 @@ + + + + + AM-Query + + + + + AM-Clean-Response-Headers + + + + + + + + + + + + + + + + + + + + true + false + ref://truststore-1 + + + + https://bigquery.googleapis.com/bigquery/v2/projects/infinite-chain-292422/queries + + diff --git a/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-3.xml b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-3.xml new file mode 100644 index 00000000..bf8fbffc --- /dev/null +++ b/test/fixtures/resources/FE001-Authentication/apiproxy/targets/target-3.xml @@ -0,0 +1,47 @@ + + + + + AM-Query + + + + + AM-Clean-Response-Headers + + + + + + + + + + + + + + + + + + + SCOPE + + + + https://external-callout-2s6vjmoabq-uw.a.run.app + false + + + + + true + false + ref://truststore-1 + + + + https://bigquery.googleapis.com/bigquery/v2/projects/infinite-chain-292422/queries + + diff --git a/test/specs/FE001-Authentication-element.js b/test/specs/FE001-Authentication-element.js new file mode 100644 index 00000000..1ea4f4ef --- /dev/null +++ b/test/specs/FE001-Authentication-element.js @@ -0,0 +1,119 @@ +/* + Copyright 2019-2023 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 */ + +const ruleId = "FE001", + assert = require("assert"), + path = require("path"), + util = require("util"), + debug = require("debug")(`apigeelint:${ruleId}`), + bl = require("../../lib/package/bundleLinter.js"); + +describe(`${ruleId} - bundle with Authentication elements`, () => { + const profiles = ["apigee", "apigeex"]; + + profiles.forEach((profile) => + it(`should generate the expected errors for profile ${profile}`, () => { + const configuration = { + debug: true, + source: { + type: "filesystem", + path: path.resolve( + __dirname, + "../fixtures/resources/FE001-Authentication/apiproxy" + ), + bundleType: "apiproxy" + }, + profile, + excluded: {}, + setExitCode: false, + output: () => {} // suppress output + }; + + bl.lint(configuration, (bundle) => { + const items = bundle.getReport(); + assert.ok(items); + assert.ok(items.length); + const fe001Issues = items.filter( + (item) => + item.messages && + item.messages.length && + item.messages.find((m) => m.ruleId == ruleId) + ); + if (profile == "apigeex") { + assert.equal(fe001Issues.length, 4); + debug(util.format(fe001Issues)); + + const ep2 = fe001Issues.find((e) => + e.filePath.endsWith("target-2.xml") + ); + assert.ok(ep2); + debug(util.format(ep2.messages)); + let messages = ep2.messages.filter((m) => m.ruleId == ruleId); + assert.equal(messages.length, 1); + assert.ok(messages[0].message); + assert.equal( + messages[0].message, + "misconfigured (empty) Authentication element." + ); + + const policy1 = fe001Issues.find((e) => + e.filePath.endsWith("SC-2-multiple-token-nodes.xml") + ); + assert.ok(policy1); + debug(util.format(policy1.messages)); + messages = policy1.messages.filter((m) => m.ruleId == ruleId); + assert.equal(messages.length, 1); + assert.ok(messages[0].message); + + const policy2 = fe001Issues.find((e) => + e.filePath.endsWith("EC-2-accesstoken.xml") + ); + assert.ok(policy2); + debug(util.format(policy2.messages)); + messages = policy2.messages.filter((m) => m.ruleId == ruleId); + + assert.equal(messages.length, 2); + assert.ok(messages[0].message); + assert.ok(messages[1].message); + } else { + assert.equal(fe001Issues.length, 7); + debug(util.format(fe001Issues)); + const itemsFlagged = fe001Issues.map((issue) => + issue.filePath.substring(issue.filePath.lastIndexOf("/") + 1) + ); + const expectedItemsFlagged = [ + "target-1.xml", + "target-2.xml", + "target-3.xml", + "SC-1-valid.xml", + "EC-1-valid.xml", + "EC-2-accesstoken.xml", + "SC-2-multiple-token-nodes.xml" + ]; + for (let i = 0; i < expectedItemsFlagged; i++) { + assert.ok( + itemsFlagged.includes(expectedItemsFlagged[i]), + expectedItemsFlagged[i] + ); + } + assert.equal(itemsFlagged.length, expectedItemsFlagged.length); + } + }); + }) + ); +});