Repository: serverless/serverless-azure-functions
Pull request #394 (2019-11-25 20:54:43)

feat: Adds support for APIM check-header policy
<!--
1. Please check out and follow our Contributing Guidelines: https://github.com/serverless/serverless-azure-functions/CONTRIBUTING.md
2. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the PR
3. Please follow the template, otherwise we'll have to ask you to update it and it will take longer until your PR is merged
-->

## What did you implement:
- Adds support for API Management `check-header` policy
- Adds support for multiple API Management `ip-filter` policies

<!--
Briefly describe the feature if no issue exists for this PR
-->
### How
- Adds new builder methods for `checkHeader`
- Adds new API Management configuration for `checkHeaders`

<!--
If this is a nontrivial change please briefly describe your implementation so its easy for us to understand and review your code.
-->

## How can we verify it:

Run new unit tests to validate changes to Policy builder and APIM service deployments.

<!--
Add any applicable config, commands, screenshots or other resources
to make it easy for us to verify this works. The easier you make it for us
to review a PR, the faster we can review and merge it.

Examples:
* serverless.yml - Fully functioning to easily deploy changes
* Screenshots - Showing the difference between your output and the master
* Cloud Configuration - List cloud resources and show that the correct configuration is in place
* Other - Anything else that comes to mind to help us evaluate
-->

## Todos:

_**Note: Run `npm run test:ci` to run all validation checks on proposed changes**_

- [x] Ensure there are no lint errors.  
       **Validate via `npm run lint`**  
       _Note: Some reported issues can be automatically fixed by running `npm run lint:fix`_
- [x] Write tests and confirm existing functionality is not broken.  
       **Validate via `npm test`**
- [x] Write documentation
- [ ] Provide verification config / commands / resources
- [x] Enable "Allow edits from maintainers" for this PR
- [x] Update the messages below

**_Is this ready for review?:_** YES  
**_Is it a breaking change?:_** NO


pjlittle: Left a couple comments - looks good. Nice to see this support 👍 

Patch:
diff --git a/docs/examples/apim.md b/docs/examples/apim.md
index ab6c57b..67f33c2 100644
--- a/docs/examples/apim.md
+++ b/docs/examples/apim.md
@@ -111,6 +111,54 @@ provider:
       exposeHeaders:
         - "*"
 
+    # JWT validation APIM policy
+    jwtValidate:
+      headerName: authorization
+      scheme: bearer
+      failedStatusCode: 401
+      failedErrorMessage: "Authorization token is missing or invalid"
+      openId:
+        metadataUrl: "https://path/to/openid/metadata/config"
+      audiences:
+        - "audience1"
+        - "audience2"
+      issuers:
+        - "https://path/to/openid/issuer"
+
+    # Header validation APIM policy
+    checkHeaders:
+      - headerName: x-example-header-1
+        failedStatusCode: 400
+        failedErrorMessage: Not Authorized
+        values: # List of allowed values, otherwise returns error code/message
+          - value1
+          - value2
+      - headerName: x-example-header-2
+        failedStatusCode: 403
+        failedErrorMessage: Forbidden
+        values: # List of allowed values, otherwise returns error code/message
+          - value1
+          - value2
+
+    # IP Validation APIM policies
+    ipFilters:
+      - action: allow
+        addresses: # List of allowed IP addresses
+          - 1.1.1.1
+          - 2.2.2.2
+        addressRange: # Also optionally support range of IP addresses
+          from: 1.1.1.1
+          to: 2.2.2.2
+      - action: forbid
+        addresses: # List of forbidden IP addresses
+          - 3.3.3.3
+          - 4.4.4.4
+        addressRange: # Also optionally support range of IP addresses
+          from: 3.3.3.3
+          to: 4.4.4.4
+
+
+
 plugins:
   - serverless-azure-functions
 
diff --git a/src/models/apiManagement.ts b/src/models/apiManagement.ts
index d21e21d..e445e54 100644
--- a/src/models/apiManagement.ts
+++ b/src/models/apiManagement.ts
@@ -16,6 +16,10 @@ export interface ApiManagementConfig {
   jwtValidate?: ApiJwtValidatePolicy;
   /** The API's IP Filter policy */
   ipFilter?: ApiIpFilterPolicy;
+  /** The API's IP Filter policies */
+  ipFilters?: ApiIpFilterPolicy[];
+  /** The API's header policies */
+  checkHeaders?: ApiCheckHeaderPolicy[];
   /** The pricing SKU for the APIM instance */
   sku?: {
     /** The SKU name, (consumption | developer | basic | standard | premium) */
@@ -102,7 +106,7 @@ export interface ApiJwtClaim {
 }
 
 /**
- * A IP Filter validation policy
+ * An IP Filter validation policy
  */
 export interface ApiIpFilterPolicy {
   /** Whether the policy should allow or forbid the address specification */
@@ -114,3 +118,19 @@ export interface ApiIpFilterPolicy {
     to: string;
   };
 }
+
+/**
+ * A header validation policy
+ */
+export interface ApiCheckHeaderPolicy {
+  /** The name of the HTTP header that to validate */
+  headerName: string;
+  /** The HTTP status code to return for a failed response */
+  failedStatusCode?: number;
+  /** The error message to return for a failed response */
+  failedErrorMessage?: string;
+  /** Performs a case insensitive comparison to the allowed values */
+  ignoreCase?: boolean;
+  /** List of allowed values to compare against */
+  values: string[];
+}
diff --git a/src/services/apimPolicyBuilder.ts b/src/services/apimPolicyBuilder.ts
index c86f1c6..404dead 100644
--- a/src/services/apimPolicyBuilder.ts
+++ b/src/services/apimPolicyBuilder.ts
@@ -1,7 +1,9 @@
-import { ApiJwtValidatePolicy, ApiJwtClaim, ApiCorsPolicy, ApiIpFilterPolicy } from "../models/apiManagement";
+import { ApiJwtValidatePolicy, ApiJwtClaim, ApiCorsPolicy, ApiIpFilterPolicy, ApiCheckHeaderPolicy } from "../models/apiManagement";
 import { Guard } from "../shared/guard";
 import { Builder, Parser } from "xml2js";
 
+declare type PolicyType = "inbound" | "outbound" | "backend" | "on-error";
+
 /**
  * APIM Policy build that can be used to build polices for APIs, Backends & operations
  */
@@ -13,7 +15,7 @@ export class ApimPolicyBuilder {
   }
 
   /**
-   * Parses an APIM xml policy and loads it into a Policy Bulder
+   * Parses an APIM xml policy and loads it into a Policy Builder
    * @param xml The xml policy
    */
   public static async parse(xml: string): Promise<ApimPolicyBuilder> {
@@ -80,19 +82,21 @@ export class ApimPolicyBuilder {
   public ipFilter(ipConfig: ApiIpFilterPolicy): ApimPolicyBuilder {
     Guard.null(ipConfig, "ipConfig");
 
-    const element = {
+    const policy = {
       $: { action: ipConfig.action },
       addressRange: ipConfig.addressRange,
       address: ipConfig.addresses
     }
 
-    Object.keys(element).forEach((key) => {
-      if (!element[key]) {
-        delete element[key];
-      }
-    });
+    ApimPolicyBuilder.cleanElement(policy);
 
-    this.policyRoot.policies.inbound[0]["ip-filter"] = [element];
+    this.updatePolicy(
+      "inbound",
+      "ip-filter",
+      policy,
+      // We are keying the ipFilter policy by action
+      (policy) => policy.$.action === ipConfig.action
+    );
 
     return this;
   }
@@ -109,8 +113,9 @@ export class ApimPolicyBuilder {
     const audiences = jwtPolicy.audiences ? [jwtPolicy.audiences.map((audience) => ({ audience }))] : null;
     const issuers = jwtPolicy.issuers ? [jwtPolicy.issuers.map((issuer) => ({ issuer }))] : null;
     const oidConfig = jwtPolicy.openId && jwtPolicy.openId.metadataUrl ? [{ $: { url: jwtPolicy.openId.metadataUrl } }] : null;
-    const claims = jwtPolicy.requiredClaims ? [jwtPolicy.requiredClaims.map(this.createClaimElement)] : null;
+    const claims = jwtPolicy.requiredClaims ? [jwtPolicy.requiredClaims.map(ApimPolicyBuilder.createClaimElement)] : null;
 
+    // Maps yaml config names to xml attribute names
     const attributeMap = {
       headerName: "header-name",
       tokenValue: "token-value",
@@ -124,15 +129,9 @@ export class ApimPolicyBuilder {
       outputTokenVariableName: "output-token-variable-name"
     };
 
-    const attributes = {};
-
-    Object.keys(attributeMap).forEach((key) => {
-      if (jwtPolicy[key]) {
-        attributes[attributeMap[key]] = jwtPolicy[key];
-      }
-    });
+    const attributes = ApimPolicyBuilder.createAttributes(attributeMap, jwtPolicy);
 
-    const elements = {
+    const policy = {
       $: attributes,
       "openid-config": oidConfig,
       "issuer-signing-keys": signingKeys,
@@ -142,13 +141,37 @@ export class ApimPolicyBuilder {
       "required-claims": claims
     };
 
-    Object.keys(elements).forEach((key) => {
-      if (!elements[key]) {
-        delete elements[key];
-      }
-    });
+    ApimPolicyBuilder.cleanElement(policy);
+    this.policyRoot.policies.inbound[0]["validate-jwt"] = [policy];
 
-    this.policyRoot.policies.inbound[0]["validate-jwt"] = [elements];
+    return this;
+  }
+
+  public checkHeader(checkHeaderPolicy: ApiCheckHeaderPolicy): ApimPolicyBuilder {
+    Guard.null(checkHeaderPolicy, "checkHeaderPolicy");
+
+    // Maps yaml config names to xml attribute names
+    const attributeMap = {
+      headerName: "name",
+      failedStatusCode: "failed-check-httpcode",
+      failedErrorMessage: "failed-check-error-message",
+      ignoreCase: "ignore-case"
+    };
+
+    const attributes = ApimPolicyBuilder.createAttributes(attributeMap, checkHeaderPolicy);
+
+    const policy = {
+      $: attributes,
+      value: checkHeaderPolicy.values
+    };
+
+    this.updatePolicy(
+      "inbound",
+      "check-header",
+      policy,
+      // We are keying the check-header policy by header name
+      (policy) => policy.$.name === checkHeaderPolicy.headerName
+    );
 
     return this;
   }
@@ -168,11 +191,69 @@ export class ApimPolicyBuilder {
     }
   }
 
+  /**
+   * Updates the matching policy or appends a new policy
+   * @param policyType The policy type (inbound | outbound | backend | on-error)
+   * @param policyName The xml element name of the policy
+   * @param newPolicy The new policy XML object
+   * @param predicate The condition to check for existing policy
+   */
+  private updatePolicy(policyType: PolicyType, policyName: string, newPolicy: any, predicate: (policy: any) => boolean): void {
+    const existing = this.policyRoot.policies[policyType][0][policyName];
+
+    // Add new policy to empty list
+    if (!existing) {
+      this.policyRoot.policies[policyType][0][policyName] = [newPolicy];
+      return;
+    }
+
+    // Find and replace the first matching policy
+    for (let i = 0; i < existing.length; i++) {
+      const policy = existing[i];
+      if (predicate(policy)) {
+        existing[i] = newPolicy;
+        return;
+      }
+    }
+
+    // Append new policy if not found
+    existing.push(newPolicy);
+  }
+
+  /**
+   * Creates a map of XML attributes for the APIM policy
+   * @param attributeMap The attribute mapping from yaml -> xml
+   * @param policy The APIM yaml policy
+   */
+  private static createAttributes(attributeMap: any, policy: any): any {
+    const attributes = {};
+
+    Object.keys(attributeMap).forEach((key) => {
+      if (policy[key]) {
+        attributes[attributeMap[key]] = policy[key];
+      }
+    });
+
+    return attributes;
+  }
+
+  /**
+   * Cleans an element so that when its converted to XML it doesn't generate empty tags
+   * @param element The pseudo XML element to clean
+   */
+  private static cleanElement(element: any) {
+    Object.keys(element).forEach((key) => {
+      if (!element[key]) {
+        delete element[key];
+      }
+    });
+  }
+
   /**
    * Create a claim element for a JWT validation policy
    * @param claim The JWT claim
    */
-  private createClaimElement(claim: ApiJwtClaim) {
+  private static createClaimElement(claim: ApiJwtClaim) {
     return {
       claim: {
         $: { name: claim.name, match: claim.match, separator: claim.separator },
diff --git a/src/services/apimService.ts b/src/services/apimService.ts
index 3c243ca..c3b5a7d 100644
--- a/src/services/apimService.ts
+++ b/src/services/apimService.ts
@@ -361,9 +361,23 @@ export class ApimService extends BaseService {
       requireUpdate = true;
     }
 
+    // Support backwards compatibility for single IP filter
     if (this.apimConfig.ipFilter) {
-      builder.ipFilter(this.apimConfig.ipFilter);
-      requireUpdate = true;
+      this.apimConfig.ipFilters = [this.apimConfig.ipFilter];
+    }
+
+    if (this.apimConfig.ipFilters) {
+      this.apimConfig.ipFilters.forEach((policy) => {
+        builder.ipFilter(policy);
+        requireUpdate = true;
+      });
+    }
+
+    if (this.apimConfig.checkHeaders) {
+      this.apimConfig.checkHeaders.forEach((policy) => {
+        builder.checkHeader(policy);
+        requireUpdate = true;
+      });
     }
 
     if (requireUpdate) {


Test patch:
diff --git a/src/services/apimPolicyBuilder.test.ts b/src/services/apimPolicyBuilder.test.ts
index c5b30e0..cc7df5d 100644
--- a/src/services/apimPolicyBuilder.test.ts
+++ b/src/services/apimPolicyBuilder.test.ts
@@ -1,7 +1,7 @@
 import { MockFactory } from "../test/mockFactory";
 import { ApimPolicyBuilder } from "./apimPolicyBuilder";
 import fs from "fs";
-import { ApiIpFilterPolicy } from "../models/apiManagement";
+import { ApiIpFilterPolicy, ApiCheckHeaderPolicy } from "../models/apiManagement";
 
 declare global {
   interface String {
@@ -100,8 +100,8 @@ describe("APIM PolicyBuilder", () => {
     expect(actual).toEqual(expected);
   });
 
-  it("can create an IP filter API policy", () => {
-    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/ip-filter.xml`)
+  it("can create an IP filter API allow policy", () => {
+    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/ip-filter-allow.xml`)
       .toString()
       .trim()
       .cleanXml();
@@ -126,4 +126,104 @@ describe("APIM PolicyBuilder", () => {
 
     expect(actual).toEqual(expected);
   });
+
+  it("can create an IP filter API forbid policy", () => {
+    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/ip-filter-forbid.xml`)
+      .toString()
+      .trim()
+      .cleanXml();
+
+    const ipPolicy: ApiIpFilterPolicy = {
+      action: "forbid",
+      addressRange: {
+        from: "1.1.1.1",
+        to: "1.1.1.255"
+      },
+      addresses: [
+        "2.2.2.2",
+        "3.3.3.3"
+      ]
+    };
+
+    const policyBuilder = new ApimPolicyBuilder();
+    const actual = policyBuilder
+      .ipFilter(ipPolicy)
+      .build()
+      .cleanXml();
+
+    expect(actual).toEqual(expected);
+  });
+
+  it("can create a policy with allow and forbid constraints", () => {
+    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/ip-filter-combined.xml`)
+      .toString()
+      .trim()
+      .cleanXml();
+
+    const allowPolicy: ApiIpFilterPolicy = {
+      action: "allow",
+      addresses: ["1.1.1.1"]
+    };
+    const forbidPolicy: ApiIpFilterPolicy = {
+      action: "forbid",
+      addresses: ["2.2.2.2"]
+    };
+
+    const policyBuilder = new ApimPolicyBuilder();
+    const actual = policyBuilder
+      .ipFilter(allowPolicy)
+      .ipFilter(forbidPolicy)
+      .build()
+      .cleanXml();
+
+    expect(actual).toEqual(expected);
+  });
+
+  it("can create a check header policy", () => {
+    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/check-header.xml`)
+      .toString()
+      .trim()
+      .cleanXml();
+
+    const checkHeaderPolicy: ApiCheckHeaderPolicy = {
+      headerName: "authorization",
+      failedStatusCode: 401,
+      failedErrorMessage: "The authorization header is either missing or invalid",
+      values: ["value1", "value2", "value3"],
+      ignoreCase: true
+    };
+
+    const policyBuilder = new ApimPolicyBuilder();
+    const actual = policyBuilder
+      .checkHeader(checkHeaderPolicy)
+      .build()
+      .cleanXml();
+
+    expect(actual).toEqual(expected);
+  });
+
+  it("can append a new check header policy", async () => {
+    const expected = fs.readFileSync(`${process.cwd()}/src/test/policies/append-check-header.xml`)
+      .toString()
+      .trim()
+      .cleanXml();
+
+    const policyXml = fs.readFileSync(`${process.cwd()}/src/test/policies/check-header.xml`).toString();
+    const policyBuilder = await ApimPolicyBuilder.parse(policyXml);
+
+    const anotherPolicy: ApiCheckHeaderPolicy = {
+      headerName: "foo",
+      values: ["bar"],
+      failedStatusCode: 400,
+      failedErrorMessage: "Bad Request",
+      ignoreCase: true
+    };
+
+    const actual = policyBuilder
+      .checkHeader(anotherPolicy)
+      .build()
+      .cleanXml();
+
+    expect(actual).toEqual(expected);
+  });
 });
diff --git a/src/services/apimService.test.ts b/src/services/apimService.test.ts
index 49f7458..afa89d2 100644
--- a/src/services/apimService.test.ts
+++ b/src/services/apimService.test.ts
@@ -21,7 +21,7 @@ import {
   ApiPolicyGetResponse
 } from "@azure/arm-apimanagement/esm/models";
 import { AzureNamingService } from "./namingService";
-import { ApiManagementConfig, ApiIpFilterPolicy } from "../models/apiManagement";
+import { ApiManagementConfig, ApiIpFilterPolicy, ApiCheckHeaderPolicy } from "../models/apiManagement";
 import { ApimPolicyBuilder } from "./apimPolicyBuilder";
 
 describe("APIM Service", () => {
@@ -388,6 +388,61 @@ describe("APIM Service", () => {
           }
         );
       });
+
+      it("deploys API IP filter policies when defined within configuration", async () => {
+        jest.spyOn(ApimPolicyBuilder.prototype, "ipFilter");
+
+        const ipFilterPolicy: ApiIpFilterPolicy = {
+          action: "allow",
+          addresses: [
+            "1.1.1.1",
+            "2.2.2.2"
+          ]
+        }
+
+        serverless.service.provider["apim"]["ipFilters"] = [ipFilterPolicy];
+
+        const apimService = new ApimService(serverless);
+        await apimService.deploy();
+
+        expect(ApimPolicyBuilder.prototype.ipFilter).toBeCalledWith(ipFilterPolicy);
+        expect(ApiPolicy.prototype.createOrUpdate).toBeCalledWith(
+          resourceGroupName,
+          serviceName,
+          apiName,
+          {
+            format: "rawxml",
+            value: expect.stringContaining("ip-filter"),
+          }
+        );
+      });
+
+      it("deploys API check header policies when defined within configuration", async () => {
+        jest.spyOn(ApimPolicyBuilder.prototype, "checkHeader");
+
+        const checkHeaderPolicy: ApiCheckHeaderPolicy = {
+          headerName: "authorization",
+          failedStatusCode: 401,
+          failedErrorMessage: "Not Authorized",
+          values: ["value1"]
+        };
+
+        serverless.service.provider["apim"]["checkHeaders"] = [checkHeaderPolicy];
+
+        const apimService = new ApimService(serverless);
+        await apimService.deploy();
+
+        expect(ApimPolicyBuilder.prototype.checkHeader).toBeCalledWith(checkHeaderPolicy);
+        expect(ApiPolicy.prototype.createOrUpdate).toBeCalledWith(
+          resourceGroupName,
+          serviceName,
+          apiName,
+          {
+            format: "rawxml",
+            value: expect.stringContaining("check-header"),
+          }
+        );
+      });
     });
 
     it("returns null when APIM is not configured", async () => {
diff --git a/src/test/policies/append-check-header.xml b/src/test/policies/append-check-header.xml
new file mode 100644
index 0000000..aed12f9
--- /dev/null
+++ b/src/test/policies/append-check-header.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<policies>
+  <inbound>
+    <base/>
+    <check-header name="authorization" failed-check-httpcode="401" failed-check-error-message="The authorization header is either missing or invalid" ignore-case="true">
+      <value>value1</value>
+      <value>value2</value>
+      <value>value3</value>
+    </check-header>
+    <check-header name="foo" failed-check-httpcode="400" failed-check-error-message="Bad Request" ignore-case="true">
+      <value>bar</value>
+    </check-header>
+  </inbound>
+  <backend>
+    <base/>
+  </backend>
+  <outbound>
+    <base/>
+  </outbound>
+  <on-error>
+    <base/>
+  </on-error>
+</policies>
diff --git a/src/test/policies/check-header.xml b/src/test/policies/check-header.xml
new file mode 100644
index 0000000..1f2bdbd
--- /dev/null
+++ b/src/test/policies/check-header.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<policies>
+  <inbound>
+    <base/>
+    <check-header name="authorization" failed-check-httpcode="401" failed-check-error-message="The authorization header is either missing or invalid" ignore-case="true">
+      <value>value1</value>
+      <value>value2</value>
+      <value>value3</value>
+    </check-header>
+  </inbound>
+  <backend>
+    <base/>
+  </backend>
+  <outbound>
+    <base/>
+  </outbound>
+  <on-error>
+    <base/>
+  </on-error>
+</policies>
diff --git a/src/test/policies/ip-filter.xml b/src/test/policies/ip-filter-allow.xml
similarity index 100%
rename from src/test/policies/ip-filter.xml
rename to src/test/policies/ip-filter-allow.xml
diff --git a/src/test/policies/ip-filter-combined.xml b/src/test/policies/ip-filter-combined.xml
new file mode 100644
index 0000000..22765e8
--- /dev/null
+++ b/src/test/policies/ip-filter-combined.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<policies>
+  <inbound>
+    <base/>
+    <ip-filter action="allow">
+      <address>1.1.1.1</address>
+    </ip-filter>
+    <ip-filter action="forbid">
+      <address>2.2.2.2</address>
+    </ip-filter>
+  </inbound>
+  <backend>
+    <base/>
+  </backend>
+  <outbound>
+    <base/>
+  </outbound>
+  <on-error>
+    <base/>
+  </on-error>
+</policies>
diff --git a/src/test/policies/ip-filter-forbid.xml b/src/test/policies/ip-filter-forbid.xml
new file mode 100644
index 0000000..981f7d3
--- /dev/null
+++ b/src/test/policies/ip-filter-forbid.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<policies>
+  <inbound>
+    <base/>
+    <ip-filter action="forbid">
+      <addressRange>
+        <from>1.1.1.1</from>
+        <to>1.1.1.255</to>
+      </addressRange>
+      <address>2.2.2.2</address>
+      <address>3.3.3.3</address>
+    </ip-filter>
+  </inbound>
+  <backend>
+    <base/>
+  </backend>
+  <outbound>
+    <base/>
+  </outbound>
+  <on-error>
+    <base/>
+  </on-error>
+</policies>