fix: improve wildcard handling in authorizer policy resource parser (#…

…1797) * fix: improve wildcard handling in authorizer policy resource parser * refactor: remove unneeded condition as it's handled by the new parsing logic * refactor: remove unneeded escape * refactor: optimize regexp
dherault · Jun 18, 2024 · 0203d04 · 0203d04
1 parent 600c93e
commit 0203d04
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 7 deletions.
diff --git a/src/events/authMatchPolicyResource.js b/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
 

diff --git a/tests/old-unit/authMatchPolicyResource.test.js b/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 =