fix: correctly identify scopes for paths containing arguments (#57)

.circleci/config.yml

@@ -104,6 +104,9 @@ jobs:
       - set_aws_prefix
       - checkout
       - node/install-packages
+      - run:
+          name: Run unit tests
+          command: npm run unit-test
       - run:
           name: Run infra tests
           command: npm run infra-test

examples/simple-authenticated-api/.gitignore

@@ -10,3 +10,5 @@ cdk.context.json
 
 !src/lambda/route1.js
 !src/lambda/route2.js
+!src/lambda/route3.js
+!src/lambda/route4.js

examples/simple-authenticated-api/lib/simple-authenticated-api-stack.ts

@@ -85,6 +85,32 @@ export class SimpleAuthenticatedApiStack extends cdk.Stack {
       }
     );
 
+    const route3Handler = new AuthenticatedApiFunction(
+      this,
+      `${prefix}simple-authenticated-api-route3-handler`,
+      {
+        name: `${prefix}route3-handler`,
+        entry: "src/lambda/route3.js",
+        environment: {},
+        handler: "route",
+        timeout: cdk.Duration.seconds(30),
+        securityGroups: lambdaSecurityGroups,
+      }
+    );
+
+    const route4Handler = new AuthenticatedApiFunction(
+      this,
+      `${prefix}simple-authenticated-api-route4-handler`,
+      {
+        name: `${prefix}route4-handler`,
+        entry: "src/lambda/route4.js",
+        environment: {},
+        handler: "route",
+        timeout: cdk.Duration.seconds(30),
+        securityGroups: lambdaSecurityGroups,
+      }
+    );
+
     const api = new AuthenticatedApi(
       this,
       `${prefix}simple-authenticated-api`,
@@ -127,6 +153,20 @@ export class SimpleAuthenticatedApiStack extends cdk.Stack {
             lambda: route2Handler,
             isPublic: true,
           },
+          {
+            name: "route3",
+            path: "/1/route3/{id}",
+            method: apigatewayv2.HttpMethod.GET,
+            lambda: route3Handler,
+            requiredScope: "analytics:admin",
+          },
+          {
+            name: "route4",
+            path: "/1/route4/{id}/route4",
+            method: apigatewayv2.HttpMethod.GET,
+            lambda: route4Handler,
+            requiredScope: "analytics:admin",
+          },
         ],
       }
     );

examples/simple-authenticated-api/src/lambda/route3.js

@@ -0,0 +1,20 @@
+class Route {
+  constructor(event) {
+    this.event = event;
+  }
+
+  async handle() {
+    console.log("Route 3 processing event.");
+
+    return {
+      statusCode: 200,
+      headers: {},
+      body: "route 3",
+    };
+  }
+}
+
+module.exports.route = async (event) => {
+  const route = new Route(event);
+  return await route.handle();
+};

examples/simple-authenticated-api/src/lambda/route4.js

@@ -0,0 +1,20 @@
+class Route {
+  constructor(event) {
+    this.event = event;
+  }
+
+  async handle() {
+    console.log("Route 4 processing event.");
+
+    return {
+      statusCode: 200,
+      headers: {},
+      body: "route 4",
+    };
+  }
+}
+
+module.exports.route = async (event) => {
+  const route = new Route(event);
+  return await route.handle();
+};

lib/authenticated-api/authenticated-api.ts

@@ -80,7 +80,7 @@ export class AuthenticatedApi extends cdk.Construct {
     if (props.lambdaRoutes) {
       for (const routeProps of props.lambdaRoutes) {
         if (routeProps.requiredScope) {
-          scopeConfig[`^${routeProps.path}$`] = routeProps.requiredScope;
+          scopeConfig[routeProps.path] = routeProps.requiredScope;
         }
       }
     }

package.json

@@ -13,6 +13,7 @@
   "scripts": {
     "build": "tsc",
     "watch": "tsc -w",
+    "unit-test": "npm run build && jest test/unit",
     "infra-test": "npm run build && jest test/infra",
     "integration-test": "npm run build && npm run jest-integration-test",
     "jest-integration-test": "jest test/integration/",

src/lambda/api/authorizer.d.ts

@@ -1 +1,46 @@
+import { PersonaClient } from "talis-node";
+declare type ParsedArn = {
+    method: string;
+    resourcePath: string;
+    apiOptions: {
+        region: string;
+        restApiId: string;
+        stage: string;
+    };
+    awsAccountId: string;
+};
+export declare class PersonaAuthorizer {
+    event: any;
+    context: any;
+    personaClient: PersonaClient | undefined;
+    constructor(event: any, context: any);
+    handle(): Promise<any>;
+    validateToken(validationOpts: any): Promise<Record<string, any>>;
+    /**
+     * Break down an API gateway method ARN into it's constituent parts.
+     * Method ARNs take the following format:
+     *
+     *   arn:aws:execute-api:<Region id>:<Account id>:<API id>/<Stage>/<Method>/<Resource path>
+     *
+     * e.g:
+     *
+     *   arn:aws:execute-api:eu-west-1:123:abc/development/GET/2/works
+     *
+     * @param methodArn {string} The method ARN provided by the event handed to a Lambda function
+     * @returns {{
+     *   method: string,
+     *   resourcePath: string,
+     *   apiOptions: {
+     *     region: string,
+     *     restApiId: string,
+     *     stage: string
+     *   },
+     *   awsAccountId: string
+     *   }}
+     */
+    parseMethodArn(methodArn: string): ParsedArn;
+    getScope(parsedMethodArn: ParsedArn): any;
+    getPersonaClient(): PersonaClient;
+    pathMatch(pathDefinition: string, path: string): boolean;
+}
 export {};

src/lambda/api/authorizer.ts

@@ -48,7 +48,7 @@ const API_GATEWAY_ARN_INDEXES = [
   RESOURCE_PATH_INDEX,
 ];
 
-class PersonaAuthorizer {
+export class PersonaAuthorizer {
   event: any;
   context: any;
   personaClient: PersonaClient | undefined;
@@ -215,9 +215,9 @@ class PersonaAuthorizer {
     const scopeConfig = process.env["SCOPE_CONFIG"];
     if (scopeConfig != undefined) {
       const conf = JSON.parse(scopeConfig);
-      for (const pathRegEx of Object.keys(conf)) {
-        if (parsedMethodArn.resourcePath.match(pathRegEx)) {
-          return conf[pathRegEx];
+      for (const path of Object.keys(conf)) {
+        if (this.pathMatch(path, parsedMethodArn.resourcePath)) {
+          return conf[path];
         }
       }
     }
@@ -241,6 +241,34 @@ class PersonaAuthorizer {
 
     return this.personaClient;
   }
+
+  pathMatch(pathDefinition: string, path: string): boolean {
+    const pathDefinitionParts = pathDefinition.split("/");
+    const pathParts = path.split("/");
+
+    if (pathDefinitionParts.length !== pathParts.length) {
+      return false;
+    }
+
+    for (let i = 0; i < pathDefinitionParts.length; i++) {
+      const pathDefinitionSegment = pathDefinitionParts[i];
+      const pathSegment = pathParts[i];
+
+      if (
+        pathDefinitionSegment.startsWith("{") &&
+        pathDefinitionSegment.endsWith("}")
+      ) {
+        // Matches path argument
+      } else {
+        // Should match directly
+        if (pathDefinitionSegment !== pathSegment) {
+          return false;
+        }
+      }
+    }
+
+    return true;
+  }
 }
 
 module.exports.validateToken = async (event: any, context: any) => {

test/integration/authenticated-api/authenticated-api.test.ts

@@ -156,4 +156,66 @@ describe("AuthenticatedApi", () => {
     expect(response.status).toBe(200);
     expect(response.data).toBe("Simple Authenticated Api Documentation");
   });
+
+  test("returns 200 when routing to a url ending in a path argument", async () => {
+    const token = await getOAuthToken(
+      TALIS_CDK_AUTH_API_VALID_CLIENT,
+      TALIS_CDK_AUTH_API_VALID_SECRET
+    );
+    const axiosAuthInstance = axios.create({
+      headers: { Authorization: `Bearer ${token}` },
+      baseURL: `https://${apiGatewayId}.execute-api.eu-west-1.amazonaws.com/1/`,
+    });
+    const response = await axiosAuthInstance.get("route3/1234");
+    expect(response.status).toBe(200);
+    expect(response.data).toBe("route 3");
+  });
+
+  test("returns 403 when routing to a url ending in a path argument", async () => {
+    const token = await getOAuthToken(
+      TALIS_CDK_AUTH_API_MISSING_SCOPE_CLIENT,
+      TALIS_CDK_AUTH_API_MISSING_SCOPE_SECRET
+    );
+    try {
+      const axiosBadAuthInstance = axios.create({
+        headers: { Authorization: `Bearer ${token}` },
+        baseURL: `https://${apiGatewayId}.execute-api.eu-west-1.amazonaws.com/1/`,
+      });
+      await axiosBadAuthInstance.get("route3/1234");
+      throw Error("Expected a 403 response");
+    } catch (err) {
+      expect(err.message).toBe("Request failed with status code 403");
+    }
+  });
+
+  test("returns 200 when routing to a url containing a path argument", async () => {
+    const token = await getOAuthToken(
+      TALIS_CDK_AUTH_API_VALID_CLIENT,
+      TALIS_CDK_AUTH_API_VALID_SECRET
+    );
+    const axiosAuthInstance = axios.create({
+      headers: { Authorization: `Bearer ${token}` },
+      baseURL: `https://${apiGatewayId}.execute-api.eu-west-1.amazonaws.com/1/`,
+    });
+    const response = await axiosAuthInstance.get("route4/1234/route4");
+    expect(response.status).toBe(200);
+    expect(response.data).toBe("route 4");
+  });
+
+  test("returns 403 when routing to a url containing a path argument", async () => {
+    const token = await getOAuthToken(
+      TALIS_CDK_AUTH_API_MISSING_SCOPE_CLIENT,
+      TALIS_CDK_AUTH_API_MISSING_SCOPE_SECRET
+    );
+    try {
+      const axiosBadAuthInstance = axios.create({
+        headers: { Authorization: `Bearer ${token}` },
+        baseURL: `https://${apiGatewayId}.execute-api.eu-west-1.amazonaws.com/1/`,
+      });
+      await axiosBadAuthInstance.get("route4/1234/route4");
+      throw Error("Expected a 403 response");
+    } catch (err) {
+      expect(err.message).toBe("Request failed with status code 403");
+    }
+  });
 });

test/unit/lambda/api/authorizer.test.ts

@@ -0,0 +1,58 @@
+import { PersonaAuthorizer } from "../../../../src/lambda/api/authorizer";
+
+describe("authorizer", () => {
+  describe("pathMatch", () => {
+    const pathMatchTests = [
+      {
+        title: "matches simple paths",
+        pathDefinition: "/1/route1",
+        path: "/1/route1",
+        expectedResult: true,
+      },
+      {
+        title: "does not match different simple paths",
+        pathDefinition: "/1/route1",
+        path: "/1/route2",
+        expectedResult: false,
+      },
+      {
+        title: "matches long paths",
+        pathDefinition: "/1/a/b/route1",
+        path: "/1/a/b/route1",
+        expectedResult: true,
+      },
+      {
+        title: "matches paths terminated by argument",
+        pathDefinition: "/1/route1/{id}",
+        path: "/1/route1/test_id",
+        expectedResult: true,
+      },
+      {
+        title: "does not matche paths when argument incorrect syntax",
+        pathDefinition: "/1/route1/:id",
+        path: "/1/route1/test_id",
+        expectedResult: false,
+      },
+      {
+        title: "matches paths containing an argument",
+        pathDefinition: "/1/a/{id}/route1",
+        path: "/1/a/test_id/route1",
+        expectedResult: true,
+      },
+      {
+        title: "does not match when number of segments don't match",
+        pathDefinition: "/a/b/route1",
+        path: "/a/b/c/route1",
+        expectedResult: false,
+      },
+    ];
+    pathMatchTests.forEach((testSpec) => {
+      test(`${testSpec.title}`, async () => {
+        const authorizer = new PersonaAuthorizer(null, null);
+        expect(
+          authorizer.pathMatch(testSpec.pathDefinition, testSpec.path)
+        ).toBe(testSpec.expectedResult);
+      });
+    });
+  });
+});