fix: requiresAppAuth was matching unintended URLs.

* test: Test to validate if a URL that does not exist in PATHS matches. * fix: requiresAppAuth was matching unintended URLs. * fix: requiresAppAuth that it did not work correctly when a URL with a query string was given. --------- Co-authored-by: Nick Floyd <[email protected]>
octokit · May 8, 2023 · 0ad8d6a · 0ad8d6a
1 parent 6466404
commit 0ad8d6a
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 3 deletions.
diff --git a/src/requires-app-auth.ts b/src/requires-app-auth.ts
@@ -42,10 +42,10 @@ function routeMatcher(paths: string[]) {
       '/repos/(?:.+?)/(?:.+?)/collaborators/(?:.+?)'
   ] */
 
-  const regex = `^(?:${regexes.map((r) => `(?:${r})`).join("|")})[^/]*$`;
+  const regex = `^(?:${regexes.map((r) => `(?:${r})`).join("|")})$`;
   // 'regex' would contain:
   /*
-    ^(?:(?:\/orgs\/(?:.+?)\/invitations)|(?:\/repos\/(?:.+?)\/(?:.+?)\/collaborators\/(?:.+?)))[^\/]*$
+    ^(?:(?:\/orgs\/(?:.+?)\/invitations)|(?:\/repos\/(?:.+?)\/(?:.+?)\/collaborators\/(?:.+?)))$
 
     It may look scary, but paste it into https://www.debuggex.com/
     and it will make a lot more sense!
@@ -57,5 +57,5 @@ function routeMatcher(paths: string[]) {
 const REGEX = routeMatcher(PATHS);
 
 export function requiresAppAuth(url: string | undefined): Boolean {
-  return !!url && REGEX.test(url);
+  return !!url && REGEX.test(url.split("?")[0]);
 }
diff --git a/test/index.test.ts b/test/index.test.ts
@@ -2383,3 +2383,83 @@ it("throws helpful error if `installationId` is set to a falsy value in createAp
     });
   }).toThrowError("[@octokit/auth-app] installationId is set to a falsy value");
 });
+
+test("auth.hook() uses installation auth for /orgs/{org}/installations requests (#374)", async () => {
+  const mock = fetchMock
+    .sandbox()
+    .post("https://api.github.com/app/installations/123/access_tokens", {
+      token: "secret123",
+      expires_at: "1970-01-01T01:00:00.000Z",
+      permissions: {
+        metadata: "read",
+      },
+      repository_selection: "all",
+      repeat: 2,
+    })
+    .getOnce("https://api.github.com/orgs/octocat/installations", {
+      headers: {
+        authorization: "token secret123",
+      },
+    });
+
+  const auth = createAppAuth({
+    appId: APP_ID,
+    privateKey: PRIVATE_KEY,
+    installationId: 123,
+  });
+
+  const requestWithMock = request.defaults({
+    headers: {
+      "user-agent": "test",
+    },
+    request: {
+      fetch: mock,
+    },
+  });
+  const requestWithAuth = requestWithMock.defaults({
+    request: {
+      hook: auth.hook,
+    },
+  });
+
+  await requestWithAuth("GET /orgs/{org}/installations", {
+    org: "octocat",
+  });
+
+  expect(mock.done()).toBe(true);
+});
+
+test("auth.hook() uses app auth even for requests with query strings. (#374)", async () => {
+  const mock = fetchMock
+    .sandbox()
+    .getOnce("https://api.github.com/orgs/octocat/installation?per_page=100", {
+      headers: {
+        authorization: `bearer ${BEARER}`,
+      },
+    });
+
+  const auth = createAppAuth({
+    appId: APP_ID,
+    privateKey: PRIVATE_KEY,
+  });
+
+  const requestWithMock = request.defaults({
+    headers: {
+      "user-agent": "test",
+    },
+    request: {
+      fetch: mock,
+    },
+  });
+  const requestWithAuth = requestWithMock.defaults({
+    request: {
+      hook: auth.hook,
+    },
+  });
+
+  await requestWithAuth("GET /orgs/{org}/installation?per_page=100", {
+    org: "octocat",
+  });
+
+  expect(mock.done()).toBe(true);
+});