Ep002 e

README.md

@@ -202,6 +202,7 @@ This is the current list:
 |   |:white_check_mark:| CC006 | Detect logical absurdities |  Conditions should not have internal logic conflicts - warn when these are detected. |
 | Endpoints |   |   |   |   |
 |   |:white_check_mark:| EP001 | CORS Policy attachment | Check for multiple CORS policies, or attachment to Target Endpoint. |
+|   |:white_check_mark:| EP002 | Element placement within Endpoints | Check for commonly misplaced elements within Proxy or 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. |

lib/package/plugins/EP002-misplacedElements.js

@@ -0,0 +1,159 @@
+/*
+  Copyright 2019-2022 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 plugin = {
+        ruleId,
+        name: "Check for commonly misplaced elements",
+        message:
+        "For example, a Flow element should be a child of Flows parent.",
+        fatal: false,
+        severity: 2, // error
+        nodeType: "Endpoint",
+        enabled: true
+      };
+
+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 _markEndpoint = (endpoint, message, line, column) => {
+        var result = {
+              ruleId: plugin.ruleId,
+              severity: plugin.severity,
+              nodeType: plugin.nodeType,
+              message,
+              line,
+              column,
+            };
+        endpoint.addMessage(result);
+      };
+
+const allowedParents = {
+        Step: ['Request', 'Response', 'FaultRule', 'DefaultFaultRule'],
+        Request: ['PreFlow', 'PostFlow', 'Flow', 'PostClientFlow'],
+        Response: ['PreFlow', 'PostFlow', 'Flow', 'PostClientFlow'],
+        Flows: ['ProxyEndpoint', 'TargetEndpoint'],
+        Flow: ['Flows'],
+        RouteRule: ['ProxyEndpoint'],
+        DefaultFaultRule: ['ProxyEndpoint', 'TargetEndpoint']
+      };
+
+const allowedChildren = {
+        Step: ['Name', 'Condition'],
+        Request: ['Step'],
+        Response: ['Step'],
+        Flows: ['Flow'],
+        Flow: ['Description', 'Request', 'Response', 'Condition'],
+        RouteRule: ['Condition', 'TargetEndpoint'],
+        DefaultFaultRule: ['Step', 'AlwaysEnforce'],
+        ProxyEndpoint: ['PreFlow', 'PostFlow', 'Flows', 'RouteRule', 'PostClientFlow',
+                       'Description', 'FaultRules', 'DefaultFaultRule', 'HTTPProxyConnection'],
+        TargetEndpoint: ['PreFlow', 'PostFlow', 'Flows',
+                         'Description', 'FaultRules', 'DefaultFaultRule', 'HTTPTargetConnection']
+      };
+
+class EndpointChecker {
+  constructor(endpoint, isProxyEndpoint) {
+    debug('EndpointChecker ctor (%s)', endpoint.getName());
+    this.endpoint = endpoint;
+    this.isProxyEndpoint = isProxyEndpoint;
+    this.flagged = false;
+  }
+
+  check() {
+    try {
+      const self = this;
+
+      let ep = self.isProxyEndpoint? 'ProxyEndpoint': 'TargetEndpoint';
+      let topLevelChildren = xpath.select("*", self.endpoint.element);
+      topLevelChildren.forEach(child => {
+        if (allowedChildren[ep].indexOf(child.tagName)< 0) {
+          self.flagged = true;
+          _markEndpoint(self.endpoint, `Invalid ${child.tagName} element`, child.lineNumber, child.columnNumber);
+        }
+      });
+
+      // 1st level children that must have at most one instance: Flows, DFR
+      ['Flows', 'DefaultFaultRule', 'FaultRules'].forEach( elementName => {
+        let elements = xpath.select(`${elementName}`, self.endpoint.element);
+        if (elements.length != 0 && elements.length != 1) {
+          self.flagged = true;
+          elements.slice(1)
+            .forEach(element =>
+                     _markEndpoint(self.endpoint, `Extra ${elementName} element`,
+                                   element.lineNumber, element.columnNumber));
+        }
+      });
+
+      // Request, Response, Step, Flow, DefaultFaultRule, RouteRule
+      let condition = Object.keys(allowedParents).map(n => `self::${n}`).join(' or ');
+      let elements = xpath.select(`//*[${condition}]`, self.endpoint.element);
+      elements.forEach(element => {
+        let tagName = element.tagName;
+        // misplaced children of toplevel elements are covered above
+        if (element.parentNode.tagName != 'ProxyEndpoint' &&
+            element.parentNode.tagName != 'TargetEndpoint') {
+          if (allowedParents[tagName].indexOf(element.parentNode.tagName)<0) {
+            self.flagged = true;
+            _markEndpoint(self.endpoint, `Misplaced ${tagName} element child of ${element.parentNode.tagName}`, element.lineNumber, element.columnNumber);
+          }
+        }
+        let children = xpath.select("*", element);
+        children.forEach(child => {
+          if (allowedChildren[tagName].indexOf(child.tagName)<0) {
+            self.flagged = true;
+            _markEndpoint(self.endpoint, `Misplaced '${child.tagName}' element child of ${tagName}`,
+                          child.lineNumber, child.columnNumber);
+          }
+        });
+      });
+
+
+      // future: add other checks here.
+
+      return this.flagged;
+    }
+    catch(e) {
+      console.log(e);
+      return false;
+    }
+  }
+}
+
+
+module.exports = {
+  plugin,
+  onProxyEndpoint,
+  onTargetEndpoint
+};

test/fixtures/resources/EP002/apiproxy/policies/AM-Clean-Response-Headers.xml

@@ -0,0 +1,6 @@
+<AssignMessage name='AM-Clean-Response-Headers' enabled='true'>
+  <Remove>
+    <Headers/>
+  </Remove>
+  <IgnoreUnresolvedVariables>false</IgnoreUnresolvedVariables>
+</AssignMessage>

test/fixtures/resources/EP002/apiproxy/policies/AM-Inject-Proxy-Revision-Header.xml

@@ -0,0 +1,7 @@
+<AssignMessage name="AM-Inject-Proxy-Revision-Header">
+  <Set>
+    <Headers>
+      <Header name="APIProxy">{apiproxy.name} r{apiproxy.revision}</Header>
+    </Headers>
+  </Set>
+</AssignMessage>

test/fixtures/resources/EP002/apiproxy/policies/AM-Response.xml

@@ -0,0 +1,9 @@
+<AssignMessage name="AM-Response">
+  <Set>
+    <Payload contentType="text/plain">
+request verb: {request.verb}
+</Payload>
+    <StatusCode>200</StatusCode>
+    <ReasonPhrase>OK</ReasonPhrase>
+  </Set>
+</AssignMessage>

test/fixtures/resources/EP002/apiproxy/policies/JS-Remove-Copied-Headers.xml

@@ -0,0 +1,13 @@
+<Javascript name="JS-Remove-Copied-Headers" enabled='true'>
+  <!--
+       This policy removes the request headers that Apigee
+       implicitly copies to the response, on a loopback proxy.
+  -->
+  <Source>
+var names = String(context.getVariable('request.headers.names'));
+names = names.substring(1, names.length-1);
+names.split(',').forEach(function(name) {
+  context.removeVariable('response.header.' + name.trim());
+});
+  </Source>
+</Javascript>

test/fixtures/resources/EP002/apiproxy/policies/RF-Invalid-Request.xml

@@ -0,0 +1,10 @@
+<RaiseFault name="RF-Invalid-Request">
+  <FaultResponse>
+    <Set>
+      <Payload contentType="text/plain">Bad Request</Payload>
+      <StatusCode>400</StatusCode>
+      <ReasonPhrase>Bad Request</ReasonPhrase>
+    </Set>
+  </FaultResponse>
+  <IgnoreUnresolvedVariables>true</IgnoreUnresolvedVariables>
+</RaiseFault>

test/fixtures/resources/EP002/apiproxy/proxies/endpoint1.xml

@@ -0,0 +1,90 @@
+<ProxyEndpoint name="endpoint1">
+  <HTTPProxyConnection>
+    <BasePath>/EP002</BasePath>
+  </HTTPProxyConnection>
+
+  <FaultRules/>
+  <FaultRules/> <!-- duplicate, will error -->
+
+  <Framjo> <!-- invalid -->
+    <!-- misplaced -->
+    <DefaultFaultRule name="default-fault-rule">
+      <Step>
+        <Name>AM-Inject-Proxy-Revision-Header</Name>
+      </Step>
+      <AlwaysEnforce>true</AlwaysEnforce>
+    </DefaultFaultRule>
+  </Framjo>
+
+  <DefaultFaultRule name="default-fault-rule">
+    <Step>
+      <Name>AM-Inject-Proxy-Revision-Header</Name>
+    </Step>
+    <AlwaysEnforce>true</AlwaysEnforce>
+  </DefaultFaultRule>
+
+  <PreFlow name="PreFlow">
+    <Request>
+    </Request>
+    <Response>
+      <Step>
+        <Name>AM-Clean-Response-Headers</Name>
+      </Step>
+      <Step>
+        <Name>JS-Remove-Copied-Headers</Name>
+      </Step>
+      <Step>
+      <Name>AM-Inject-Proxy-Revision-Header</Name>
+      </Step>
+    </Response>
+  </PreFlow>
+
+  <PostClientFlow name='PostClientFlow'>
+    <!-- error: misplaced Step -->
+    <Step>
+      <Name>AM-Inject-Proxy-Revision-Header</Name>
+    </Step>
+  </PostClientFlow>
+
+  <!-- misplaced -->
+  <Flow name="this-is-misplaced">
+    <Description></Description>
+    <Request>
+      <Step>
+        <Name>AM-Inject-Proxy-Revision-Header</Name>
+      </Step>
+    </Request>
+    <Response/>
+    <Condition>(proxy.pathsuffix MatchesPath "/urlpath") and (request.verb = "HEAD")</Condition>
+  </Flow>
+
+  <Flows>
+    <Flow name="f1">
+      <Description>purposefully raise a fault</Description>
+      <Request>
+        <Step>
+          <Name>RF-Invalid-Request</Name>
+        </Step>
+      </Request>
+      <Response/>
+      <Condition>proxy.pathsuffix MatchesPath "/fault"</Condition>
+    </Flow>
+  </Flows>
+
+  <PostFlow name="PostFlow">
+    <Request/>
+    <Response>
+      <Step>
+        <Name>AM-Response</Name>
+        <Condition>request.verb != "OPTIONS"</Condition>
+      </Step>
+    </Response>
+  </PostFlow>
+
+  <RouteRule name="http-1">
+    <Condition>proxy.pathsuffix MatchesPath "/t1"</Condition>
+    <TargetEndpoint>http-1</TargetEndpoint>
+  </RouteRule>
+
+  <RouteRule name="noroute"/>
+</ProxyEndpoint>

test/fixtures/resources/EP002/apiproxy/targets/http-1.xml

@@ -0,0 +1,27 @@
+<TargetEndpoint name="http-1">
+  <PreFlow name="PreFlow">
+    <Request>
+    </Request>
+    <Response/>
+  </PreFlow>
+  <PostFlow name="PostFlow">
+    <Request/>
+    <Response/>
+  </PostFlow>
+  <Flows/>
+
+  <Flows/>
+
+  <MisPlaced/>
+
+  <HTTPTargetConnection>
+    <LoadBalancer>
+      <Server name="target1" />
+      <Server name="target2" />
+    </LoadBalancer>
+    <Path>/test</Path>
+  </HTTPTargetConnection>
+
+  <RouteRule name="noroute"/>
+
+</TargetEndpoint>