Merge branch 'master' into combined-prs

* master:
  fix: improve wildcard handling in authorizer policy resource parser (dherault#1797)
  fix: ensure resource policy matches the whole arn (dherault#1798)

src/events/authMatchPolicyResource.js

@@ -1,7 +1,8 @@
 function parseResource(resource) {
-  const [, region, accountId, restApiId, path] = resource.match(
-    /arn:aws:execute-api:(.*?):(.*?):(.*?)\/(.*)/,
-  )
+  const [, region = "*", accountId = "*", restApiId = "*", path = "*"] =
+    resource.match(
+      /arn:aws:execute-api:([^\s:]+)(?::([^\s:]+))?(?::([^\s/:]+))?(?:\/(.*))?/,
+    )
 
   return {
     accountId,
@@ -26,10 +27,6 @@ export default function authMatchPolicyResource(policyResource, resource) {
     return true
   }
 
-  if (policyResource === "arn:aws:execute-api:*:*:*") {
-    return true
-  }
-
   if (policyResource.includes("*") || policyResource.includes("?")) {
     // Policy contains a wildcard resource
 
@@ -61,7 +58,7 @@ export default function authMatchPolicyResource(policyResource, resource) {
     // for the requested resource and the resource defined in the policy
     // Need to create a regex replacing ? with one character and * with any number of characters
     const regExp = new RegExp(
-      parsedPolicyResource.path.replaceAll("*", ".*").replaceAll("?", "."),
+      `${parsedPolicyResource.path.replaceAll("*", ".*").replaceAll("?", ".")}$`,
     )
 
     return regExp.test(parsedResource.path)

tests/old-unit/authMatchPolicyResource.test.js

@@ -25,6 +25,81 @@ describe("authMatchPolicyResource", () => {
     })
   })
 
+  describe("when the resource defines all segments with a wildcard", () => {
+    const wildcardResource = "arn:aws:execute-api:*:*:*"
+
+    it("matches anything", () => {
+      for (const resource of [
+        "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs",
+        "arn:aws:execute-api:us-west-1:123456:random-api-id/development/GET/diinosaurs",
+        "arn:aws:execute-api:eu-west-2:123abc:random-api-id/development/PUT/dinosaurs",
+        "arn:aws:execute-api:eu-west-1:random-account-id:123abc/development/GET/dinosaurs:extinct",
+        "arn:aws:execute-api:ap-southeast-1:random-account-id:random-api-id/development/GET/diinosaurs",
+      ]) {
+        assert.strictEqual(
+          authMatchPolicyResource(wildcardResource, resource),
+          true,
+        )
+      }
+    })
+  })
+
+  describe("when the resource ends with a wildcarded region segment", () => {
+    const wildcardResource = "arn:aws:execute-api:*"
+
+    it("matches anything", () => {
+      for (const resource of [
+        "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs",
+        "arn:aws:execute-api:us-west-1:123456:random-api-id/development/GET/diinosaurs",
+        "arn:aws:execute-api:eu-west-2:123abc:random-api-id/development/PUT/dinosaurs",
+        "arn:aws:execute-api:eu-west-1:random-account-id:123abc/development/GET/dinosaurs:extinct",
+        "arn:aws:execute-api:ap-southeast-1:random-account-id:random-api-id/development/GET/diinosaurs",
+      ]) {
+        assert.strictEqual(
+          authMatchPolicyResource(wildcardResource, resource),
+          true,
+        )
+      }
+    })
+  })
+
+  describe("when the resource ends with a wildcarded account-id segment", () => {
+    const wildcardResource = "arn:aws:execute-api:eu-west-1:*"
+
+    describe("and the resource is in the same region", () => {
+      it("matches regardless of what comes afterwards", () => {
+        for (const resource of [
+          "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs",
+          "arn:aws:execute-api:eu-west-1:123456:random-api-id/development/GET/diinosaurs",
+          "arn:aws:execute-api:eu-west-1:123abc:random-api-id/development/PUT/dinosaurs",
+          "arn:aws:execute-api:eu-west-1:random-account-id:123abc/development/GET/dinosaurs:extinct",
+        ]) {
+          assert.strictEqual(
+            authMatchPolicyResource(wildcardResource, resource),
+            true,
+          )
+        }
+      })
+    })
+
+    describe("and the resource is in a different region", () => {
+      it("does not match regardless of what comes afterwards", () => {
+        for (const resource of [
+          "arn:aws:execute-api:eu-west-2:random-account-id:random-api-id/development/GET/dinosaurs",
+          "arn:aws:execute-api:us-west-1:123456:random-api-id/development/GET/diinosaurs",
+          "arn:aws:execute-api:eu-west-2:123abc:random-api-id/development/PUT/dinosaurs",
+          "arn:aws:execute-api:eu-west-2:random-account-id:123abc/development/GET/dinosaurs:extinct",
+          "arn:aws:execute-api:ap-southeast-1:random-account-id:random-api-id/development/GET/diinosaurs",
+        ]) {
+          assert.strictEqual(
+            authMatchPolicyResource(wildcardResource, resource),
+            false,
+          )
+        }
+      })
+    })
+  })
+
   describe("when the resource has wildcards", () => {
     describe("and it matches", () => {
       const wildcardResource =
@@ -155,6 +230,38 @@ describe("authMatchPolicyResource", () => {
         })
       })
     })
+
+    describe("when the resource has segment wildcards", () => {
+      const wildcardResource =
+        "arn:aws:execute-api:*:*:random-api-id/local/GET/organizations"
+
+      describe("and it matches", () => {
+        it("returns true", () => {
+          const resource =
+            "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/local/GET/organizations"
+
+          assert.strictEqual(
+            authMatchPolicyResource(wildcardResource, resource),
+            true,
+          )
+        })
+      })
+
+      describe("and it does not match", () => {
+        it("returns false", () => {
+          for (const resource of [
+            "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/local/GET/me",
+            "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/local/GET/organisations",
+            "arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/local/GET/organizations/1",
+          ]) {
+            assert.strictEqual(
+              authMatchPolicyResource(wildcardResource, resource),
+              false,
+            )
+          }
+        })
+      })
+    })
   })
 
   describe("when the resource has single character wildcards", () => {